feat(agent): deterministic memory-note carry + verbatim-specifics summarizer#27
Closed
jaredLunde wants to merge 9 commits into
Closed
feat(agent): deterministic memory-note carry + verbatim-specifics summarizer#27jaredLunde wants to merge 9 commits into
jaredLunde wants to merge 9 commits into
Conversation
…inistically The recall reminder (#25) tells the model to consult `/session` after a compaction, and the pressure nudge (#26) tells it to save there before — but both still lean on the summarizing model not erasing the fact that a note *exists*. Close that: make the model's memory writes ride the existing deterministic-carry channel, so every summary lists them verbatim regardless of what the summarizer chooses to say. - `CompactionProvenance` gains `memory_notes: Vec<String>` — the logical paths the model authored via the `memory` tool (create/str_replace/ insert), deduped, oldest-first, folded forward every round by `merge_provenance`, exactly like the read/modified-file lists. - `extract_memory_notes` keys on the `memory` tool name only (like `extract_todos` keys on `todo`) — agnostic to which root a note lives under, so agent-core never learns the host's `/session` vs `/memories` convention. - `format_memory_notes` renders a `<memory-notes>` block appended to every summary; `Agent::compact` appends it alongside the file/todo blocks, and `previous_summary` strips it (via `CARRY_BLOCKS`) so it never accumulates across incremental rounds. - Persisted on the `Entry::Compaction` record and restored on reopen, so a serve restart past a compaction keeps the awareness. This is the guaranteed, zero-effort half of the save→carry→recall arc: after a compaction the model is *shown* what it wrote and where, not just told to go look. Proven e2e (serve_session_memory): a `/session` note the model wrote survives a compaction as a `<memory-notes>` block in the summary. Plus unit tests for extract/format/merge/strip. ARCHITECTURE.md updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… at the source) The deterministic carry guarantees a few structured artifacts survive a compaction; the summary prose still loses everything else the summarizer chooses to smooth over. Strengthen the summarization instructions so the specifics most often dropped survive the cut in the first place. `SUMMARY_SYSTEM` (shared by compaction and branch summaries) and every per-mode template (`SUMMARY_INSTRUCTION`, `UPDATE_INSTRUCTION`, `SPLIT_TURN_INSTRUCTION`, `BRANCH_SUMMARY_INSTRUCTION`) now explicitly demand that file:line references, exact literal values (numbers, ports, versions, config keys, IDs, flags), commands and their key outputs, and error messages be copied *verbatim* rather than paraphrased or rounded — "a specific you drop is gone for good." The pi-parity structural shape (section headings, checkbox/`(none)` conventions, numbered Next Steps) is unchanged; only the preserve directive is sharpened. A unit test pins that every summary-shaping prompt demands verbatim specifics and names file:line references, so this can't silently regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jaredLunde
changed the title
…window `compaction_pressure_point` collapsed to 0 when the reserve met or exceeded the context window (a tiny-window or test config): since the check is `live_prompt > point`, a 0 point is true on every turn, firing the pre-compaction nudge (and its extra steer turn) spuriously from turn one — which, against a fixed-response test server, can exhaust or stall it. There is no proactive window to warn within when reserve >= window (compaction fires the instant any usage exists), so return a never-fires sentinel (u32::MAX) in that case instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes surfaced by the crates/agent safety audit. Each is independent and
self-contained; the serve.rs hunks are only the OutSink/get_messages changes
(a concurrent workstream's in-progress edits in the same file are intentionally
left unstaged).
1. mcp_auth_store: create mcp_auth.json 0600 atomically + self-heal on every
write. It holds live MCP OAuth access/refresh tokens; write_atomic only
preserves an existing file's mode, so a brand-new file previously landed at
the umask default (world-readable). Mirrors auth_store's auth.json handling.
2. serve get_messages{since}: guard the split_off with arr.len()==msg_ids.len().
msg_ids (active tree path) includes Custom entries that session.messages
excludes, so after an append_custom RPC an idx past messages' end made
split_off(idx+1) panic — a remotely-triggerable per-session crash.
3. worktree patch_paths: parse `git apply --numstat -z` (NUL-terminated, raw).
Without -z git C-quotes non-ASCII paths ("s\303\251crets.env"), which slips
past the --deny-path merge-back re-check (a **/*.env glob can't match a
trailing .env"), merging a denied file back into the parent repo.
4. deps: bump quinn-proto 0.11.14->0.11.16 (RUSTSEC-2026-0185, CVSS 7.5) and
crossbeam-epoch 0.9.18->0.9.20 (RUSTSEC-2026-0204).
5. session_store create_private: unlink-then-create_new instead of
create(true).truncate(true) on the deterministic .tmp path, so a pre-planted
file/symlink can't be opened-and-truncated (keeping a loose mode) or followed.
6. serve/serve_ws: bound each network connection's output channel (OutSink enum;
1024-frame cap, disconnect-on-full). A stalled mobile socket previously let
the session buffer streamed frames without limit. stdio stays unbounded.
7. exec: prune completed receivers from PENDING_GROUP_KILLS on push, so the
registry (otherwise drained only before process::exit) can't grow for the
lifetime of a long-lived serve daemon.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `serve_auth` abort-login test hung intermittently under load (2-core CI runners hit it ~every run; it killed the runner at ~46min with no logs). Two independent bugs, found by reproducing under `taskset -c 0,1` and live-inspecting the stuck serve with gdb: 1. Test-ordering bug (the actual hang). `abort_login`'s own response and the aborted login's async terminal response are produced by different tasks and race — but the test read them in a fixed order. When the login terminal won the race, the abort-response read consumed and discarded it, and the following terminal-response read then hung forever on a frame already thrown away. Fixed with an order-independent `read_frames_matching_all` for both racing pairs. 2. A real serve race the fixed test then exposed. `abort_login` cleared the `pending_login` slot only asynchronously, via the detached task's `Drop` guard, so the next `login` could arrive before the slot freed and be spuriously rejected as "already in flight" — hanging a client that reasonably expects abort's success to mean the slot is free. Now `abort_login` clears the slot synchronously (`take`), and the guard is generation-tagged so a late-winding-down task can't wipe a successor's slot (which would make the next abort a silent no-op, leaving that login uncancellable). Verified: the previously-hanging test now passes 60/60 under 2-core concurrency (unfixed hung within 1-9), and the full integration suite runs clean 3x on 2 cores. New unit test pins the generation-guard behavior. This is the flake that was blocking #27's CI — pre-existing, unrelated to the memory work, surfaced because CI's slow runners lose the race almost every time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A safety audit of `agent-core` found the mechanical surface already clean (`unsafe_code = "forbid"`, panic lints denied workspace-wide, `overflow-checks` on in release), so every finding here is behavioral. The two that mattered: - **An un-terminated SSE event grew without bound.** `LineFramer`'s 32 MiB cap bounds a single *line* and resets on every terminator, but `SseEventBuffer` only drains on a blank line — so a relay that strips blank lines grew one `String` per `data:` line forever, under the line cap the whole way, until the process OOMed. Capped the *event*, not just the line. - **The `MaxTokens` silent-truncation retry could spin forever.** Its `overflow_recovered` guard was reset on the `Ok` path *upstream* of the check that read it, so it was dead code; the retry then decremented `steps_this_call`, so `max_steps` couldn't bound it either. Each round billed two model calls and made no progress. The flag now has a survives-exactly-one-iteration lifetime (`mem::replace` into a per-turn local), and `compact()` refuses the one cut that provably can't shrink (a re-summarized summary), so `Compacted` stays an honest signal that the caller made progress. Also: - Compaction budgets now scale with the model's real window (`CompactionConfig::for_window`). The 200k absolutes left a 32k model keeping more (20_000) than the threshold that triggers a compaction (16_384) — so it re-triggered every turn and never got back under the line. Windows >= ~64k are byte-for-byte unchanged. - `openai_responses` no longer defaults a missing `output_index` to `-1` (i.e. `usize::MAX` once cast): two tool calls collapsed onto that one synthetic index and the accumulator silently overwrote the first. Now refuses to guess, matching `anthropic::usize_at`'s posture. - Provider token counts clamp instead of truncating (`as u32` is not covered by `overflow-checks`, so `u32::MAX + 2` landed as `1` — making a full context look empty and *suppressing* the compaction that would have saved the turn). - The Codex WebSocket `send()` was the module's one unbounded await; idle cached connections are now swept rather than reaped lazily per-key. - `CheckpointHook::checkpoint` is panic-guarded like every other host seam — it's the one most likely to touch failing I/O, and a host that unwrapped on a full disk took the whole run down. - Steering lanes are bounded (they're fed straight from remote RPC); a full lane refuses the newest message and `serve` now acks that honestly instead of claiming `success: true` for a message it dropped. - Tool results are capped at the loop layer — the 50 KiB bound was a per-tool convention, and `Tool` is a public trait MCP servers implement. - A cancelled `write_lock` waiter no longer orphans its map entry; schema coercion has a work budget. Not done: `Error::Transport(String)` still flattens its source chain. `MID_STREAM_NETWORK_ERROR` is a *deliberate* transport-agnostic classification channel (a concrete `#[source]` would couple `agent-core` to `reqwest` and break the network-blind-core property), and a `Box<dyn Error>` source means converting a tuple variant matched across two crates — diagnosability, not safety. Left for a focused change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…export A leak hunt across `crates/agent` (daemon, subprocess tools, export). The two that mattered: - **An abandoned `login` leaked an entire session, permanently.** `login_cancel` was a fresh *root* token, the login ran as a detached task holding a clone of the session's `out_tx`, and teardown is `drop(out_tx); writer.await` — where the writer ends only once every sender drops. `pending_login` was never cancelled on the break path (`abort_login` is a *command*, which an abandoned login never sends). So a client that starts a login and disconnects wedged `serve_session` forever: an OS thread + its runtime, the `Agent`, the `GatewayClient` and its pool, the message history, and the OAuth callback server's *fixed* port (53692 / 1455) — which then made every subsequent login fail with `PortBindFailed`. In stdio `serve` it also made graceful shutdown hang outright. The root cause was that `ServeLoginCallbacks::prompt_text` was a bare `rx.await`. CLAUDE.md's human-in-the-loop rule says the wait "races the run's `CancellationToken` and a timeout, and fails closed" — `ApprovalGate` does exactly that; this second instance of the same seam did not. It now does, and teardown cancels a pending login unconditionally. `CallbackServer` also gained a `Drop` that releases the listener without depending on anyone to cancel first. - **`bash` output spilled to `/tmp` with no cap, and `/tmp` here is a 14 GB tmpfs** — i.e. "spill to disk" is spill to RAM. The 50 KiB cap is a display/tail cap; post-spill the writer did `write_all` unconditionally. With a 30-minute default timeout, `bash: yes` fills RAM. The spill file now has a byte budget and the reported truncation metadata stays honest about what was dropped. Also: - The daemon's idle reaper was **opt-in** (`Option`, no default), so every connection minted a session held for the daemon's life. It now defaults to 1h; `0` explicitly disables. Dead handles (a session whose loop already exited) were *excluded* from reaping by an inverted predicate and leaked forever — a closed `input_tx` is now the strongest reason to reap, not a reason to skip. - A wedged session permanently burned a tokio blocking-pool thread (the `timeout` abandoned the await, not the `j.join()`); the join now polls `is_finished()`. - The inbound command channel was unbounded while outbound was carefully capped. It's now bounded with real backpressure — *not* drop-on-full: a dropped command is a correctness bug (the client waits on a response that never comes), so a full queue stops the reader and lets TCP carry the backpressure to the client. - `git apply` wrote the whole patch to stdin before reading stdout/stderr — >64 KiB of git output deadlocked the parent, holding a child and three pipe fds. Write and drain are now concurrent. - `lcs_diff` allocated an uncapped O(n·m) table (a 25k-line rewrite is ~2.5 GB at export time); past a cell budget it falls back to a linear rendering. - `wait_for_pending_group_kills` is now called before every `process::exit` that follows a cancellation, as its own doc comment always asked — three sites were missing it, leaving backgrounded grandchildren alive. Not included: `session_store.rs`. A concurrent session is mid-refactor there (`Node::content` → `Arc<NodeContent>`), and the `append_custom` size cap this hunt found belongs on top of that, not underneath it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Measured first, then fixed — and the measurements killed half the original audit. `benches/persistence.rs` is new and covers the half of this crate that had no benchmark at all; every number below is from it or from `benches/search.rs`. The constraint: `read_listing` fully parses every line of every session file to recover a preview, a message count, and a max timestamp. Cost is O(total transcript bytes), and transcripts are append-only and never pruned — the one cost here that grows without bound with a user's history. A sidecar `.listings.json` per session directory now caches each file's derived `SessionMeta` against the `(size, mtime)` it was computed from. Session files are append-only, so any write moves both and a stale entry cannot survive one. Listing a 14 MB history: 15.1 ms -> 11.4 µs, and — the actual point — flat in transcript size (11.1 µs at 1.4 MB vs 11.4 µs at 14 MB). It is a pure cache: every read is best-effort and any failure falls back to a full scan, so a lost or clobbered index costs time, never correctness. The derived fields are carried explicitly in the index rather than inside `SessionMeta`, which marks all four `#[serde(skip)]` so they can't go stale in the on-disk *header*. Right call there, exactly wrong here — the first cut serialized a `SessionMeta` straight in and silently dropped the only fields the index exists to remember. The tests caught it. Two cheaper fixes were tried and reverted, because a control benchmark (`listing_read_floor`) showed only ~15% of a listing is reading bytes and ~85% is serde *tokenizing* payload it discards. Skipping materialization doesn't skip tokenization: a peek-parse cut allocations 75% and bought 0% wall time. The only way to make the scan cheaper was to not do it. Also fixed, each measured: - `edit`: ASCII fast path in normalization. NFKC is the identity on ASCII and every `fold_char` arm is >= U+00A0, so for ASCII input — which source code overwhelmingly is — normalization provably reduces to dropping trailing whitespace, with no `char` decoding at all. normalize 2.82 ms -> 107 µs (26x); full `edit::run` ~2.7 ms -> ~0.5 ms (~5x). A test pins that the fast path yields byte-identical text *and* an identical offset map — it feeds the splice, so a divergence would corrupt files, not just mismatch. - `edit`: onto `spawn_blocking`. On a 4 MB file it pinned a `current_thread` runtime — what `serve_ws` gives every session — for 72.9 ms, stalling that session's event pump and its abort/steer command loop. This is the invariant `serve.rs::persist_blocking` already states; `edit` just hadn't followed. `tests/tool_reactor_stall.rs` now pins it. `write` and `ls` were measured on the same harness and do NOT stall, so they are deliberately left alone rather than wrapped on principle. - `session_store::write_line`: single-pass `to_writer` instead of `Entry` -> owned `Value` tree -> `String` — the same two-pass shape `serve::event_frame` was already fixed out of and `benches/serve_events.rs` was written to condemn; the fix had never reached this module. Plus a borrowing `EntryRef` so `append_new` stops cloning each `Message` purely to hand it to serde. Serializer 2.9x, allocations 29 -> 1; `append_new` -10% end-to-end (fsync-dominated, so that's the ceiling). Also included, from a parallel session working in this same tree, NOT part of the above perf work: `Node::content` moves behind an `Arc<NodeContent>` so `rewrite_compacted` — which keeps the pre-compaction chain intact *and* re-emits the surviving suffix under fresh ids — no longer holds every kept message twice per compaction round. Committed together because the two changes interleave in `session_store.rs`; it is green under the full suite. cargo test -p beyond-ai-agent: all green (1275 lib + every integration suite). clippy --all-targets: clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jaredLunde
added a commit
that referenced
this pull request
Jul 13, 2026
…nal (#28) * feat(agent): carry the model's memory writes across compaction deterministically The recall reminder (#25) tells the model to consult `/session` after a compaction, and the pressure nudge (#26) tells it to save there before — but both still lean on the summarizing model not erasing the fact that a note *exists*. Close that: make the model's memory writes ride the existing deterministic-carry channel, so every summary lists them verbatim regardless of what the summarizer chooses to say. - `CompactionProvenance` gains `memory_notes: Vec<String>` — the logical paths the model authored via the `memory` tool (create/str_replace/ insert), deduped, oldest-first, folded forward every round by `merge_provenance`, exactly like the read/modified-file lists. - `extract_memory_notes` keys on the `memory` tool name only (like `extract_todos` keys on `todo`) — agnostic to which root a note lives under, so agent-core never learns the host's `/session` vs `/memories` convention. - `format_memory_notes` renders a `<memory-notes>` block appended to every summary; `Agent::compact` appends it alongside the file/todo blocks, and `previous_summary` strips it (via `CARRY_BLOCKS`) so it never accumulates across incremental rounds. - Persisted on the `Entry::Compaction` record and restored on reopen, so a serve restart past a compaction keeps the awareness. This is the guaranteed, zero-effort half of the save→carry→recall arc: after a compaction the model is *shown* what it wrote and where, not just told to go look. Proven e2e (serve_session_memory): a `/session` note the model wrote survives a compaction as a `<memory-notes>` block in the summary. Plus unit tests for extract/format/merge/strip. ARCHITECTURE.md updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(agent): summarizer preserves specifics verbatim (shrink the loss at the source) The deterministic carry guarantees a few structured artifacts survive a compaction; the summary prose still loses everything else the summarizer chooses to smooth over. Strengthen the summarization instructions so the specifics most often dropped survive the cut in the first place. `SUMMARY_SYSTEM` (shared by compaction and branch summaries) and every per-mode template (`SUMMARY_INSTRUCTION`, `UPDATE_INSTRUCTION`, `SPLIT_TURN_INSTRUCTION`, `BRANCH_SUMMARY_INSTRUCTION`) now explicitly demand that file:line references, exact literal values (numbers, ports, versions, config keys, IDs, flags), commands and their key outputs, and error messages be copied *verbatim* rather than paraphrased or rounded — "a specific you drop is gone for good." The pi-parity structural shape (section headings, checkbox/`(none)` conventions, numbered Next Steps) is unchanged; only the preserve directive is sharpened. A unit test pins that every summary-shaping prompt demands verbatim specifics and names file:line references, so this can't silently regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: dprint markdown fmt (underscore emphasis in ARCHITECTURE.md) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agent): pressure point never fires on every turn when reserve >= window `compaction_pressure_point` collapsed to 0 when the reserve met or exceeded the context window (a tiny-window or test config): since the check is `live_prompt > point`, a 0 point is true on every turn, firing the pre-compaction nudge (and its extra steer turn) spuriously from turn one — which, against a fixed-response test server, can exhaust or stall it. There is no proactive window to warn within when reserve >= window (compaction fires the instant any usage exists), so return a never-fires sentinel (u32::MAX) in that case instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agent): production-safety audit remediations (7 findings) Fixes surfaced by the crates/agent safety audit. Each is independent and self-contained; the serve.rs hunks are only the OutSink/get_messages changes (a concurrent workstream's in-progress edits in the same file are intentionally left unstaged). 1. mcp_auth_store: create mcp_auth.json 0600 atomically + self-heal on every write. It holds live MCP OAuth access/refresh tokens; write_atomic only preserves an existing file's mode, so a brand-new file previously landed at the umask default (world-readable). Mirrors auth_store's auth.json handling. 2. serve get_messages{since}: guard the split_off with arr.len()==msg_ids.len(). msg_ids (active tree path) includes Custom entries that session.messages excludes, so after an append_custom RPC an idx past messages' end made split_off(idx+1) panic — a remotely-triggerable per-session crash. 3. worktree patch_paths: parse `git apply --numstat -z` (NUL-terminated, raw). Without -z git C-quotes non-ASCII paths ("s\303\251crets.env"), which slips past the --deny-path merge-back re-check (a **/*.env glob can't match a trailing .env"), merging a denied file back into the parent repo. 4. deps: bump quinn-proto 0.11.14->0.11.16 (RUSTSEC-2026-0185, CVSS 7.5) and crossbeam-epoch 0.9.18->0.9.20 (RUSTSEC-2026-0204). 5. session_store create_private: unlink-then-create_new instead of create(true).truncate(true) on the deterministic .tmp path, so a pre-planted file/symlink can't be opened-and-truncated (keeping a loose mode) or followed. 6. serve/serve_ws: bound each network connection's output channel (OutSink enum; 1024-frame cap, disconnect-on-full). A stalled mobile socket previously let the session buffer streamed frames without limit. stdio stays unbounded. 7. exec: prune completed receivers from PENDING_GROUP_KILLS on push, so the registry (otherwise drained only before process::exit) can't grow for the lifetime of a long-lived serve daemon. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(serve): flaky login-abort hang (the CI-blocking heisenbug) The `serve_auth` abort-login test hung intermittently under load (2-core CI runners hit it ~every run; it killed the runner at ~46min with no logs). Two independent bugs, found by reproducing under `taskset -c 0,1` and live-inspecting the stuck serve with gdb: 1. Test-ordering bug (the actual hang). `abort_login`'s own response and the aborted login's async terminal response are produced by different tasks and race — but the test read them in a fixed order. When the login terminal won the race, the abort-response read consumed and discarded it, and the following terminal-response read then hung forever on a frame already thrown away. Fixed with an order-independent `read_frames_matching_all` for both racing pairs. 2. A real serve race the fixed test then exposed. `abort_login` cleared the `pending_login` slot only asynchronously, via the detached task's `Drop` guard, so the next `login` could arrive before the slot freed and be spuriously rejected as "already in flight" — hanging a client that reasonably expects abort's success to mean the slot is free. Now `abort_login` clears the slot synchronously (`take`), and the guard is generation-tagged so a late-winding-down task can't wipe a successor's slot (which would make the next abort a silent no-op, leaving that login uncancellable). Verified: the previously-hanging test now passes 60/60 under 2-core concurrency (unfixed hung within 1-9), and the full integration suite runs clean 3x on 2 cores. New unit test pins the generation-guard behavior. This is the flake that was blocking #27's CI — pre-existing, unrelated to the memory work, surfaced because CI's slow runners lose the race almost every time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agent-core): production-safety audit remediations (14 findings) A safety audit of `agent-core` found the mechanical surface already clean (`unsafe_code = "forbid"`, panic lints denied workspace-wide, `overflow-checks` on in release), so every finding here is behavioral. The two that mattered: - **An un-terminated SSE event grew without bound.** `LineFramer`'s 32 MiB cap bounds a single *line* and resets on every terminator, but `SseEventBuffer` only drains on a blank line — so a relay that strips blank lines grew one `String` per `data:` line forever, under the line cap the whole way, until the process OOMed. Capped the *event*, not just the line. - **The `MaxTokens` silent-truncation retry could spin forever.** Its `overflow_recovered` guard was reset on the `Ok` path *upstream* of the check that read it, so it was dead code; the retry then decremented `steps_this_call`, so `max_steps` couldn't bound it either. Each round billed two model calls and made no progress. The flag now has a survives-exactly-one-iteration lifetime (`mem::replace` into a per-turn local), and `compact()` refuses the one cut that provably can't shrink (a re-summarized summary), so `Compacted` stays an honest signal that the caller made progress. Also: - Compaction budgets now scale with the model's real window (`CompactionConfig::for_window`). The 200k absolutes left a 32k model keeping more (20_000) than the threshold that triggers a compaction (16_384) — so it re-triggered every turn and never got back under the line. Windows >= ~64k are byte-for-byte unchanged. - `openai_responses` no longer defaults a missing `output_index` to `-1` (i.e. `usize::MAX` once cast): two tool calls collapsed onto that one synthetic index and the accumulator silently overwrote the first. Now refuses to guess, matching `anthropic::usize_at`'s posture. - Provider token counts clamp instead of truncating (`as u32` is not covered by `overflow-checks`, so `u32::MAX + 2` landed as `1` — making a full context look empty and *suppressing* the compaction that would have saved the turn). - The Codex WebSocket `send()` was the module's one unbounded await; idle cached connections are now swept rather than reaped lazily per-key. - `CheckpointHook::checkpoint` is panic-guarded like every other host seam — it's the one most likely to touch failing I/O, and a host that unwrapped on a full disk took the whole run down. - Steering lanes are bounded (they're fed straight from remote RPC); a full lane refuses the newest message and `serve` now acks that honestly instead of claiming `success: true` for a message it dropped. - Tool results are capped at the loop layer — the 50 KiB bound was a per-tool convention, and `Tool` is a public trait MCP servers implement. - A cancelled `write_lock` waiter no longer orphans its map entry; schema coercion has a work budget. Not done: `Error::Transport(String)` still flattens its source chain. `MID_STREAM_NETWORK_ERROR` is a *deliberate* transport-agnostic classification channel (a concrete `#[source]` would couple `agent-core` to `reqwest` and break the network-blind-core property), and a `Box<dyn Error>` source means converting a tuple variant matched across two crates — diagnosability, not safety. Left for a focused change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agent): resource-leak remediations across the daemon, tools, and export A leak hunt across `crates/agent` (daemon, subprocess tools, export). The two that mattered: - **An abandoned `login` leaked an entire session, permanently.** `login_cancel` was a fresh *root* token, the login ran as a detached task holding a clone of the session's `out_tx`, and teardown is `drop(out_tx); writer.await` — where the writer ends only once every sender drops. `pending_login` was never cancelled on the break path (`abort_login` is a *command*, which an abandoned login never sends). So a client that starts a login and disconnects wedged `serve_session` forever: an OS thread + its runtime, the `Agent`, the `GatewayClient` and its pool, the message history, and the OAuth callback server's *fixed* port (53692 / 1455) — which then made every subsequent login fail with `PortBindFailed`. In stdio `serve` it also made graceful shutdown hang outright. The root cause was that `ServeLoginCallbacks::prompt_text` was a bare `rx.await`. CLAUDE.md's human-in-the-loop rule says the wait "races the run's `CancellationToken` and a timeout, and fails closed" — `ApprovalGate` does exactly that; this second instance of the same seam did not. It now does, and teardown cancels a pending login unconditionally. `CallbackServer` also gained a `Drop` that releases the listener without depending on anyone to cancel first. - **`bash` output spilled to `/tmp` with no cap, and `/tmp` here is a 14 GB tmpfs** — i.e. "spill to disk" is spill to RAM. The 50 KiB cap is a display/tail cap; post-spill the writer did `write_all` unconditionally. With a 30-minute default timeout, `bash: yes` fills RAM. The spill file now has a byte budget and the reported truncation metadata stays honest about what was dropped. Also: - The daemon's idle reaper was **opt-in** (`Option`, no default), so every connection minted a session held for the daemon's life. It now defaults to 1h; `0` explicitly disables. Dead handles (a session whose loop already exited) were *excluded* from reaping by an inverted predicate and leaked forever — a closed `input_tx` is now the strongest reason to reap, not a reason to skip. - A wedged session permanently burned a tokio blocking-pool thread (the `timeout` abandoned the await, not the `j.join()`); the join now polls `is_finished()`. - The inbound command channel was unbounded while outbound was carefully capped. It's now bounded with real backpressure — *not* drop-on-full: a dropped command is a correctness bug (the client waits on a response that never comes), so a full queue stops the reader and lets TCP carry the backpressure to the client. - `git apply` wrote the whole patch to stdin before reading stdout/stderr — >64 KiB of git output deadlocked the parent, holding a child and three pipe fds. Write and drain are now concurrent. - `lcs_diff` allocated an uncapped O(n·m) table (a 25k-line rewrite is ~2.5 GB at export time); past a cell budget it falls back to a linear rendering. - `wait_for_pending_group_kills` is now called before every `process::exit` that follows a cancellation, as its own doc comment always asked — three sites were missing it, leaving backgrounded grandchildren alive. Not included: `session_store.rs`. A concurrent session is mid-refactor there (`Node::content` → `Arc<NodeContent>`), and the `append_custom` size cap this hunt found belongs on top of that, not underneath it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(agent): make session listing O(sessions), not O(transcript bytes) Measured first, then fixed — and the measurements killed half the original audit. `benches/persistence.rs` is new and covers the half of this crate that had no benchmark at all; every number below is from it or from `benches/search.rs`. The constraint: `read_listing` fully parses every line of every session file to recover a preview, a message count, and a max timestamp. Cost is O(total transcript bytes), and transcripts are append-only and never pruned — the one cost here that grows without bound with a user's history. A sidecar `.listings.json` per session directory now caches each file's derived `SessionMeta` against the `(size, mtime)` it was computed from. Session files are append-only, so any write moves both and a stale entry cannot survive one. Listing a 14 MB history: 15.1 ms -> 11.4 µs, and — the actual point — flat in transcript size (11.1 µs at 1.4 MB vs 11.4 µs at 14 MB). It is a pure cache: every read is best-effort and any failure falls back to a full scan, so a lost or clobbered index costs time, never correctness. The derived fields are carried explicitly in the index rather than inside `SessionMeta`, which marks all four `#[serde(skip)]` so they can't go stale in the on-disk *header*. Right call there, exactly wrong here — the first cut serialized a `SessionMeta` straight in and silently dropped the only fields the index exists to remember. The tests caught it. Two cheaper fixes were tried and reverted, because a control benchmark (`listing_read_floor`) showed only ~15% of a listing is reading bytes and ~85% is serde *tokenizing* payload it discards. Skipping materialization doesn't skip tokenization: a peek-parse cut allocations 75% and bought 0% wall time. The only way to make the scan cheaper was to not do it. Also fixed, each measured: - `edit`: ASCII fast path in normalization. NFKC is the identity on ASCII and every `fold_char` arm is >= U+00A0, so for ASCII input — which source code overwhelmingly is — normalization provably reduces to dropping trailing whitespace, with no `char` decoding at all. normalize 2.82 ms -> 107 µs (26x); full `edit::run` ~2.7 ms -> ~0.5 ms (~5x). A test pins that the fast path yields byte-identical text *and* an identical offset map — it feeds the splice, so a divergence would corrupt files, not just mismatch. - `edit`: onto `spawn_blocking`. On a 4 MB file it pinned a `current_thread` runtime — what `serve_ws` gives every session — for 72.9 ms, stalling that session's event pump and its abort/steer command loop. This is the invariant `serve.rs::persist_blocking` already states; `edit` just hadn't followed. `tests/tool_reactor_stall.rs` now pins it. `write` and `ls` were measured on the same harness and do NOT stall, so they are deliberately left alone rather than wrapped on principle. - `session_store::write_line`: single-pass `to_writer` instead of `Entry` -> owned `Value` tree -> `String` — the same two-pass shape `serve::event_frame` was already fixed out of and `benches/serve_events.rs` was written to condemn; the fix had never reached this module. Plus a borrowing `EntryRef` so `append_new` stops cloning each `Message` purely to hand it to serde. Serializer 2.9x, allocations 29 -> 1; `append_new` -10% end-to-end (fsync-dominated, so that's the ceiling). Also included, from a parallel session working in this same tree, NOT part of the above perf work: `Node::content` moves behind an `Arc<NodeContent>` so `rewrite_compacted` — which keeps the pre-compaction chain intact *and* re-emits the surviving suffix under fresh ids — no longer holds every kept message twice per compaction round. Committed together because the two changes interleave in `session_store.rs`; it is green under the full suite. cargo test -p beyond-ai-agent: all green (1275 lib + every integration suite). clippy --all-targets: clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agent-core): stop leaking foreign content-block fields onto the Anthropic wire A session started on an OpenAI model and continued on a Claude one 400s outright: messages.1.content.0.text.id: Extra inputs are not permitted `build_body` builds each message from exactly `role`/`content` so a stray *message* field is structurally unreachable — but `content` is then serialized straight off `ContentBlock`, which carries fields no Anthropic block has: `Text::id`/`Text::phase` (OpenAI Responses item ids) and `ToolUse::thought_signature`. They are `None` on anything Anthropic produced, and populated the moment a transcript crosses dialects — which a session outlives. Apply the same argument one level down: prune each block to the keys its type is actually allowed to carry. An allowlist, not a blocklist, for the reason `build_body`'s own comment already gives — a blocklist that lags a new `ContentBlock` field is a 400 on every request carrying it, while an allowlist that lags drops a field Anthropic wanted, which the dialect's tests catch loudly. `tool_result.images` stays in the allowlist: a later pass consumes it into the real content-array shape. `cache_control` is absent because a later pass adds it. Pre-existing (reproduced on main), but cross-provider switching is about to become an ordinary thing to do, so it stops being a corner. Does not fix the *next* error underneath it — a signed `thinking` block from a foreign model. `Session::scrub_cross_model_state` handles exactly that and has zero production callers; wiring it into the model-switch paths is a separate change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: route directly to model providers — the AI gateway is now optional Export `ANTHROPIC_API_KEY` and run the agent. That's it. With no gateway configured, the request goes straight to `api.anthropic.com` with `x-api-key`; an `OPENAI_API_KEY` goes to `api.openai.com` with `Authorization: Bearer`. Nobody spells out an auth header — it falls out of the provider row. Before this, a developer with no gateway deployed had exactly one escape: a hand-written `~/.claude/models.json` with `base_url` + `auth_header`. `AI_GATEWAY_URL` defaulted to `http://ai.internal`, so the agent always believed it had a gateway. The seam already existed. `RouteOverride::Direct` replaces `GatewayClient`'s base URL outright at request time, so this needed no new transport: a direct credential just carries a route, and the gateway URL is never dialed. ## crates/providers — one table, two consumers A new zero-dependency crate: one row per upstream, carrying its gateway `authority`, its direct `base_url`, its wire format, its auth scheme, the env var that conventionally holds a user's key, the model-id prefixes it serves natively, and any headers it requires. The gateway proxies on those rows; the agent routes directly on them. A provider's auth scheme cannot be right in one and wrong in the other. It absorbs four tables that were previously kept in sync by hand: the gateway's `KNOWN_PROVIDERS`, `agent_core`'s `AggregatorHost` (now a re-export of `providers::ProviderId`), the agent's `aggregator_host_for_base_url`, and its `default_headers_for_base_url`. ## Provider resolves before dialect — never the reverse A model id cannot tell you the wire format. `anthropic/claude-sonnet-4.5` is an *OpenAI*-wire request when OpenRouter serves it. `Dialect::for_model`'s name heuristic ("contains anthropic" => Anthropic wire) is right for a native route and wrong for an aggregator, so deriving the dialect from the id and only then picking a provider inverts the dependency and POSTs an Anthropic body to a Chat Completions endpoint. This was already latent on main for a `models.json` OpenRouter entry with no explicit `dialect`. `Dialect::for_model_via_provider` takes the provider and suppresses the name heuristic for an OpenAI-wire row. The genuine per-model exceptions still win (Fireworks really does serve 14 ids over the Anthropic wire; OpenCode Zen/Go really do disagree). `provider: None` is bit-identical to the old heuristic — that's the regression guard. Aggregators match no model-id prefix, deliberately: they serve other vendors' ids, so any prefix that identified them would steal that vendor's native route. Name them (`AI_PROVIDER=openrouter`). ## Precedence A gateway is *configured* if `AI_GATEWAY_URL`, a stored `default_gateway_url`, or `AI_AGENT_KEY` is set. `DEFAULT_GATEWAY` is a fallback, *not* configuration — that one distinction is what makes the gateway optional. With one configured it is used; `AI_DIRECT=1` forces direct anyway. A configured gateway is never silently rerouted just because a key is in someone's shell — that would move traffic off the gateway and off its metering with nothing on screen to say so. Direct mode, in order: `models.json` override > `AI_BASE_URL`+`AI_API_KEY` > `AI_PROVIDER` > stored OAuth > the provider key for whichever provider natively serves the model id. OAuth sits *above* the ambient key on purpose: `agent login` is explicit and durable, while `ANTHROPIC_API_KEY` is often exported for an unrelated tool, and ranking it higher would silently move a subscriber onto pay-per-token billing. `OPENAI_BASE_URL` is honored only as the OpenAI row's base URL, never as a global escape hatch — it is widely exported, and pointing it at a proxy for one tool must not redirect Anthropic traffic. And a row's env var only ever pays for that row: `ANTHROPIC_API_KEY` never travels to an arbitrary `AI_BASE_URL`. ## Two inferences that stopped holding Both read the *route shape* to guess the provider, which worked only while a provider was reachable exactly one way. Both now ask the routing which provider it points at: - `is_oauth` was "has no direct override => genuine direct-to-Anthropic". A direct route to `api.anthropic.com` is the most first-party request there is, and suppressing the identity headers on it makes Anthropic's OAuth endpoint reject the token. - `is_codex` was `RouteOverride::Prefixed`. Codex is now also reachable as a plain `Direct` route, so the prefix no longer identifies it. Relatedly: an OAuth token is not an API key. The Anthropic row's `x-api-key` scheme describes how Anthropic takes an *API key*; a subscription token goes in `Authorization: Bearer`. The row is right about host, wire, and path — the credential decides the header. Error messages no longer say "the gateway rejected your key" when there is no gateway; they name the host that actually answered. ## Verified Live, with no gateway, against real keys: `ANTHROPIC_API_KEY` -> api.anthropic.com/x-api-key; `OPENAI_API_KEY` -> api.openai.com/Bearer; `AI_PROVIDER=openrouter` + `--model anthropic/claude-sonnet-4.5` -> a real completion, which is only possible over the OpenAI wire. Plus the regression: a configured gateway with `ANTHROPIC_API_KEY` also exported still went to the gateway. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agent-core): actually call scrub_cross_model_state on a model switch `Session::scrub_cross_model_state` was written for exactly the cross-model resume problem — downgrade a foreign model's signed `Thinking` blocks to plain text, drop opaque `RedactedThinking`, truncate OpenAI-Responses combined `"call_id|item_id"` tool ids and replay the rewrite onto the paired `ToolResult` — and was called by nothing but its own tests. So it never ran, and resuming a session on another provider 400s: Invalid `signature` in `thinking` block Wire it into `Agent::run_events_steered`. Not into each caller that can change the active model (`run --continue`, and serve's `set_model`/`cycle_model`/`switch_session`/`fork`/ `clone`/`switch_branch`) — every one of those funnels into this function, and this is the single point where a model and the transcript it is about to be shown actually meet. At the choke point it is structurally impossible for a new model-switching entry point to forget it, which is precisely how it came to have no callers at all. Gated on `Session::needs_cross_model_scrub`, a new read-only scan: the scrub `Arc::make_mut`s the message vec, which deep-clones the whole transcript whenever that `Arc` is shared (it is — persistence holds a handle). Paying that on every turn of a long same-model session to change nothing would be a real regression. The gate is deliberately narrower than "any message with a foreign `model_id`": every `User` message has `model_id: None` and is foreign by the scrub's own rule, so that predicate is true of essentially every session and would gate nothing. What matters is whether a foreign message carries state that does not survive the crossing — a `Thinking`/`RedactedThinking` block, or a tool-call id containing `|`. Nothing else the scrub touches can differ. Verified live, both directions, with no gateway: run --model gpt-5 "think, then say BANANA" run --continue --model claude-opus-4-8 "what word did you just say?" -> BANANA run --model claude-opus-4-8 "read secret.txt …" (tools: signed thinking + tool ids) run --continue --model gpt-5 "what was the secret word?" -> ZEBRA Claude reads the prior turn's content back, so the reasoning text survives as context rather than being erased — the downgrade-not-discard behavior the scrub was designed for, finally running. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Landed in main via #28 (squash |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Two complementary pieces that make a compaction lose less — the "guarantee what you can, and shrink what you can't" half of the arc from #25 (session memory) and #26 (pressure nudge). Both land together.
1. Deterministic carry of the model's memory writes
The recall reminder tells the model to consult
/sessionafter a compaction, and the pressure nudge tells it to save there before — but both still lean on the summarizing model not quietly erasing the fact that a note exists. Now the model's memory writes ride the existing deterministic-carry channel (parallel to the read/modified-file andtodolists):CompactionProvenancegainsmemory_notes: Vec<String>— logical paths the model authored via thememorytool (create/str_replace/insert), deduped, oldest-first, folded forward each round bymerge_provenance.extract_memory_noteskeys on thememorytool name only (likeextract_todoskeys ontodo), so agent-core stays agnostic to the host's/sessionvs/memoriesconvention.format_memory_notesrenders a<memory-notes>block appended to every summary;Agent::compactappends it alongside the file/todo blocks, andprevious_summarystrips it (viaCARRY_BLOCKS) so it never accumulates across incremental rounds.Entry::Compactionrecord and restored on reopen.After a compaction the model is shown what it wrote and where, guaranteed, zero effort.
2. Summarizer preserves specifics verbatim
The carry guarantees a few structured artifacts; the summary prose still loses whatever the summarizer smooths over.
SUMMARY_SYSTEM(shared by compaction and branch summaries) and every per-mode template (SUMMARY_INSTRUCTION,UPDATE_INSTRUCTION,SPLIT_TURN_INSTRUCTION,BRANCH_SUMMARY_INSTRUCTION) now explicitly demand that the specifics most often dropped —file:linereferences, exact literal values (numbers, ports, versions, config keys, IDs, flags), commands and their key outputs, error messages — be copied verbatim. The pi-parity structural shape is untouched; only the preserve directive is sharpened.Verification
serve_session_memory: a/sessionnote survives a compaction as a<memory-notes>block in the persisted summary.file:line.🤖 Generated with Claude Code