diff --git a/CHANGELOG.md b/CHANGELOG.md index 10252f14f..9d0368fb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **wrap/codex:** `headroom unwrap codex` now removes the Headroom rtk instruction block from the Codex global `AGENTS.md`. `wrap codex` injects it there, but unwrap only restored `config.toml` and MCP state, so a plain `codex` launch kept following the "prefix shell commands with `rtk`" guidance and failed once the managed rtk binary was off PATH. Unwrap now strips the marker-fenced block (preserving the rest of the file), mirroring `unwrap copilot` ([#1421](https://github.com/headroomlabs-ai/headroom/issues/1421)). * **proxy/auth:** classify real Anthropic OAuth tokens correctly. `classify_auth_mode` matched OAuth on the `sk-ant-oat-` prefix, but real access tokens are `sk-ant-oat01-...` (a version number, no dash after `oat`), so every real subscription/OAuth token fell through to the `sk-` branch and was tagged `PAYG` — enabling aggressive lossy compression, auto `cache_control`, and `prompt_cache_key` injection on subscription-bound requests the classifier is meant to route to the passthrough-prefer path. The prefix is now the dash-less `sk-ant-oat` (still matches the legacy dashed shape). The existing parity tests only passed because they used a synthetic `sk-ant-oat-01-` fixture; a regression test now covers the real `sk-ant-oat01-` format. * **install:** stop leaking a file descriptor on every `headroom install start`. `start_detached_agent()` opened the agent log file and handed it to `subprocess.Popen` but never closed the parent's copy, so each call leaked one fd (and pinned the log file open against rotation). The parent now closes its copy in a `try/finally` once the child has inherited it — the close also runs if `Popen` raises ([#1554](https://github.com/headroomlabs-ai/headroom/issues/1554)). +* **memory/sync:** stop the Codex AGENTS.md sync adapter from erasing previously-synced memories on every export. `sync_export` hands each adapter only the *delta* (memories the agent lacks), but `CodexAdapter.write_memories` rebuilt its whole managed section from just that delta — so each sync overwrote the section with only the new items, thrashing the file between disjoint subsets and never accumulating. It now merges the delta into the facts already present (deduped), matching the additive contract the ClaudeCode adapter already follows. * **cli/proxy:** honor `HEADROOM_MIN_TOKENS=0` / `HEADROOM_MAX_ITEMS=0`. The Click `proxy` command built these with `_get_env_int_optional(name) or 500`/`or 50`, so an explicit `0` — a legitimate value (`min_tokens_to_crush=0` means "crush every item") — was treated as falsy and silently replaced with the default. The `headroom proxy` argparse path already preserved `0` via `_get_env_int`, so the two entry points disagreed. The Click path now uses the same None-checking helper. * **proxy:** include the system prompt, tools, and the response-shaping request fields in the SemanticCache key. `_compute_key` hashed only `{model, messages}`, so two non-streaming requests with identical messages but a different top-level `system` prompt, tool set, sampling config, or output-shaping field collided on one key and the second caller was served the first's cached response — generated under different request semantics, in the default config (`cache_enabled` defaults on). The key now folds the request fields that shape generation — `temperature`/`top_p`/`top_k`/`max_tokens`/`stop`, plus OpenAI `tool_choice`/`response_format`/`parallel_tool_calls`/`seed`/`presence_penalty`/`frequency_penalty`/`logit_bias`/`n`/`logprobs`/`top_logprobs`/`reasoning_effort`/`verbosity`/`modalities` and Anthropic `thinking`/`tool_choice`/`output_config` — canonicalizing `system`/`tools` so a moved `cache_control` breakpoint does not fragment it, and the handlers snapshot the fields once at the cache read and reuse them at write so a body mutated by the pipeline cannot diverge the key. Non-streaming path only. * **learn (verbosity):** `--verbosity --apply --all` now aggregates the savings baseline across every project instead of overwriting it per project (last-project-wins), which previously left the output shaper with a tiny, unrepresentative baseline. The applied verbosity level comes from the project with the most samples ([#1288](https://github.com/headroomlabs-ai/headroom/pull/1288)). diff --git a/headroom/memory/sync_adapters/codex_agent.py b/headroom/memory/sync_adapters/codex_agent.py index 993ebf5a1..01f9da07a 100644 --- a/headroom/memory/sync_adapters/codex_agent.py +++ b/headroom/memory/sync_adapters/codex_agent.py @@ -68,32 +68,61 @@ class CodexAdapter(AgentMemoryAdapter): return memories async def write_memories(self, memories: list[dict[str, Any]]) -> int: - """Write memories into the headroom section of AGENTS.md.""" + """Merge memories into the headroom section of AGENTS.md. + + ``sync_export`` hands this adapter only the *delta* — memories the + agent doesn't already have (see ``AgentMemoryAdapter`` contract; the + sibling ClaudeCode adapter is additive for the same reason). So this + must accumulate: rebuilding the section from just ``memories`` would + erase every previously-synced fact on each run, thrashing the file + between disjoint subsets and never converging. + """ if not memories: return 0 - # Build section content - lines = ["## Headroom Shared Memory", ""] - for mem in memories: - content = mem["content"].split("\n")[0].strip() # First line only - lines.append(f"- {content}") - lines.append("") + existing_content = self._path.read_text(encoding="utf-8") if self._path.exists() else "" + # Facts already in the managed section — preserve them (dedup by the + # rendered first-line, matching how read_memories reconstructs them). + facts: list[str] = [] + seen: set[str] = set() + existing_match = _MARKER_PATTERN.search(existing_content) + if existing_match: + for line in existing_match.group(1).split("\n"): + stripped = line.strip() + if stripped.startswith("- "): + fact = stripped[2:].strip() + if fact and fact not in seen: + seen.add(fact) + facts.append(fact) + + added = 0 + for mem in memories: + fact = mem["content"].split("\n")[0].strip() # First line only + if fact and fact not in seen: + seen.add(fact) + facts.append(fact) + added += 1 + + lines = ["## Headroom Shared Memory", ""] + lines.extend(f"- {fact}" for fact in facts) + lines.append("") section = f"{_MARKER_START}\n" + "\n".join(lines) + f"{_MARKER_END}" - # Merge into AGENTS.md - if self._path.exists(): - content = self._path.read_text(encoding="utf-8") - if _MARKER_START in content: - content = _MARKER_PATTERN.sub(lambda _match: section, content) + # Splice the section back in. Use a function replacement (not a string + # template) so literal backslashes / \\u in a memory aren't treated as + # regex escapes. + if existing_content: + if _MARKER_START in existing_content: + content = _MARKER_PATTERN.sub(lambda _match: section, existing_content) else: - content = content.rstrip() + "\n\n" + section + "\n" + content = existing_content.rstrip() + "\n\n" + section + "\n" else: self._path.parent.mkdir(parents=True, exist_ok=True) content = section + "\n" self._path.write_text(content, encoding="utf-8") - return len(memories) + return added def fingerprint(self) -> str: """Hash of AGENTS.md contents.""" diff --git a/tests/test_memory_sync.py b/tests/test_memory_sync.py index 3ff0f0346..26802f4c8 100644 --- a/tests/test_memory_sync.py +++ b/tests/test_memory_sync.py @@ -536,11 +536,14 @@ class TestCodexAdapter: assert "Existing instructions" in content # Preserved @pytest.mark.asyncio - async def test_write_replaces_existing_section(self, agents_md): + async def test_write_merges_into_existing_section(self, agents_md): + """Additive: an existing managed fact is preserved when a new one is + written. ``sync_export`` hands the adapter only the delta, so a + replace-the-whole-section write would erase prior memories.""" agents_md.write_text( "# Instructions\n\n" "\n" - "## Old\n- old fact\n" + "## Headroom Shared Memory\n\n- old fact\n" "\n" ) @@ -549,14 +552,14 @@ class TestCodexAdapter: content = agents_md.read_text() assert "new fact" in content - assert "old fact" not in content + assert "old fact" in content # preserved, not clobbered @pytest.mark.asyncio - async def test_write_replaces_existing_section_with_literal_backslashes(self, agents_md): + async def test_write_preserves_existing_fact_with_literal_backslashes(self, agents_md): agents_md.write_text( "# Instructions\n\n" "\n" - "## Old\n- old fact\n" + "## Headroom Shared Memory\n\n- old fact\n" "\n" ) @@ -564,9 +567,30 @@ class TestCodexAdapter: await adapter.write_memories([{"content": r"Use C:\Users\john.doe\repo and literal \u"}]) content = agents_md.read_text() + # Backslashes / \u land literally (function replacement, not a template). assert r"C:\Users\john.doe\repo" in content assert r"literal \u" in content - assert "old fact" not in content + assert "old fact" in content # preserved + + @pytest.mark.asyncio + async def test_write_accumulates_across_syncs(self, agents_md): + """Regression: exporting deltas across successive syncs must accumulate, + not thrash between disjoint subsets.""" + adapter = CodexAdapter(agents_md) + + await adapter.write_memories([{"content": "fact A"}, {"content": "fact B"}]) + # Second sync only sees the new memory as a delta. + added = await adapter.write_memories([{"content": "fact C"}]) + + content = agents_md.read_text() + assert "fact A" in content + assert "fact B" in content + assert "fact C" in content + assert added == 1 + # Re-writing an already-present fact adds nothing and keeps the rest. + again = await adapter.write_memories([{"content": "fact A"}]) + assert again == 0 + assert (await adapter.read_memories()).__len__() == 3 @pytest.mark.asyncio async def test_read_empty_agents_md(self, agents_md):