From ce40b54648e8da9ebda407f8f304c85eeaa8425f Mon Sep 17 00:00:00 2001 From: Jared Lunde Date: Sat, 15 Aug 2026 13:56:40 -0700 Subject: [PATCH 1/2] fix(agent): a session id is an address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every distinct session id in one directory resolved to the same session, and there was no way to ask for a new one. `SessionRepo::resume_or_create` matched on `cwd` first and consulted the caller's id only in the no-match branch, so once any session existed for a directory every `--session-id` pointed at it was silently discarded. Everything that opens a session in repo mode went through it — a bare `run`, `run --continue`, `serve` with or without `--session-dir` — so N ids collapsed to 1. Split the API into the two things it was conflating, and fix the precedence: - `open_or_create_id` — open exactly this id, or mint it under exactly this id. Never another session, and never by unique prefix (`find_path_exact`): resolving `abc` onto an existing `abcdef` would both return the wrong session and suppress creating the one asked for. - `resume_latest_or_create` — this cwd's most recent session. Now reached only via `--continue`. `--session-id` > `--continue` > fresh, identically in `run` and `serve` (`serve::SessionSelect`). `serve` gains `--continue` to spell what its default used to do implicitly. A bare launch now starts its own session rather than reattaching. Two shells in one repo previously drove the same store, and `append_new` is count-keyed, so neither could observe the other's writes and the transcript interleaved. Daemon: `session_cfg` rewrote every WS session into single-file mode purely to escape the cwd collapse. With ids taking precedence that workaround is gone, which fixes what it cost — daemon files were `.jsonl` where the repo names `_.jsonl`, so `find_path` couldn't resolve them and a daemon session was listable but not openable by id. Sessions are now durable across a full daemon restart, not just a reap. Because the id is a routing key, `new_session` on an addressed session keeps it and archives the outgoing conversation into a sibling session (`parent` = the slot) instead of overwriting it — "start a new session" no longer doubles as "destroy the old one". Creating a genuinely new session over the daemon is a routing operation: connect with a new `?session_id=`. Also fixes exact-match id lookup, which compared an `_.jsonl` suffix. Ids may legally contain `_`, so a lookup for `b` matched a session named `a_b`. Harmless for a convenience lookup; not harmless once it decides what an address resolves to. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 20 ++ crates/agent/ARCHITECTURE.md | 230 +++++++++------- crates/agent/src/main.rs | 97 ++++--- crates/agent/src/serve.rs | 182 ++++++++++--- crates/agent/src/serve_ws.rs | 57 ++-- crates/agent/src/session_store.rs | 244 +++++++++++++---- crates/agent/tests/run_session_management.rs | 112 +++++++- .../agent/tests/serve_session_addressing.rs | 248 ++++++++++++++++++ crates/agent/tests/serve_session_lifecycle.rs | 28 +- crates/agent/tests/serve_websocket.rs | 36 +-- 10 files changed, 971 insertions(+), 283 deletions(-) create mode 100644 crates/agent/tests/serve_session_addressing.rs diff --git a/README.md b/README.md index 7bf6bf8..94f6623 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,26 @@ configured, the gateway is used (unless `AI_DIRECT=1`). Otherwise, routing is di Tools: `read`, `write`, `edit`, `bash`, `ls`, `grep`, `find` (pi's coding set), plus `todo` and `web`. See [crates/agent-core/ARCHITECTURE.md](crates/agent-core/ARCHITECTURE.md). +### Which session you get + +A session id is an **address**, and it's the only thing that names a session outright. Both `run` and +`serve` take the same three: + +```sh +agent run "..." # a new session, persisted under ~/.claude/sessions// +agent run --continue "..." # reattach to this directory's most recent session +agent run --session-id build-42 "…" # open session build-42, or create it — same session every time +``` + +`--session-id` outranks `--continue`, never resolves to a session it wasn't given, and is idempotent, so +it's the right flag for anything supervised or scripted: a restarted `serve --session-id X` lands back on +the same conversation deterministically, where `--continue` depends on whatever else touched the +directory meanwhile. Distinct ids in one directory are distinct sessions — which is what makes `serve` +multi-tenant, and what the daemon's `?session_id=` routing is built on. + +A bare launch starts its own session rather than reattaching, so two shells (or two servers) working in +one directory don't silently drive the same transcript. Use `--continue` when you want the old one. + ### Running the tools against a remote exec endpoint `--exec-url ` makes the agent's tools — `read`, `write`, `edit`, `ls`, `grep`, `find` **and diff --git a/crates/agent/ARCHITECTURE.md b/crates/agent/ARCHITECTURE.md index c13a9cb..8003c15 100644 --- a/crates/agent/ARCHITECTURE.md +++ b/crates/agent/ARCHITECTURE.md @@ -76,7 +76,15 @@ The harness layers several capabilities over the bare tools + loop: can't burn a blocking-pool thread for the daemon's life). The default has to be finite: a connection that omits `?session_id=` mints a fresh id, and every id owns a thread, an `Agent`, and a gateway pool until something reclaims it. A reconnect to a just-reaped id respawns it from disk, and the `catchup` - frame seeded on attach replays its restored history. A third daemon facility, **shared upstream + frame seeded on attach replays its restored history. That respawn works because the routing key is + handed to the session as `--session-id`, which _addresses_ an ordinary repo session — so the key + survives a full process restart, not just a reap. `session_cfg` used to rewrite each session into + single-file mode at `/.jsonl` purely to escape repo mode's old cwd-collapse; that cost + it a naming split the repo's own `find_path` (`_.jsonl`) couldn't resolve, so daemon sessions were + listable but not openable by id. Because the key is an address something routes on, `new_session` + **keeps** it: the outgoing conversation is archived into a sibling session (`parent` = the slot) and the + slot is blanked in place, so a client's address never goes stale. Creating a genuinely new session is a + routing operation — connect with a new `?session_id=`. A third daemon facility, **shared upstream pooling** (`--upstream-http2 `, off by default): instead of each session building its own `reqwest::Client` (so N sessions ≈ N connections to the gateway on the plaintext HTTP/1.1 hop), `serve_ws` builds **one** client and injects it into every session via @@ -397,8 +405,28 @@ The harness layers several capabilities over the bare tools + loop: multi-session `SessionRepo` (list-with-metadata/create/open/soft-delete-to-`.trash`/fork); `--session-file` is the single-session form; neither flag defaults to `~/.claude/sessions//` rather than silent - in-memory-only (`--no-session-persistence` opts out explicitly). Every cwd this module ever records - into or matches against a session — `serve`'s own startup reattach, `run --continue` — is passed + in-memory-only (`--no-session-persistence` opts out explicitly). + + **Which session a launch opens** is one rule, shared by `run` and `serve` (`serve::SessionSelect`), in + precedence order: + + 1. `--session-id ` (and the daemon's `?session_id=`) **addresses** a session: + `SessionRepo::open_or_create_id` opens exactly that session, or mints it under exactly that id. It + never resolves to a different session, and never by unique prefix — `find_path_exact`, not + `find_path`, since resolving `abc` onto an existing `abcdef` would both return the wrong session and + suppress creating the one asked for. Idempotent: same id, same conversation, every time. + 2. `--continue` reattaches to this cwd's most recent session (`resume_latest_or_create`), else creates + one. The only implicit reattach there is. + 3. Otherwise a **fresh** session, persisted but its own. + + The precedence is the point. `--session-id` used to be consulted only in `resume_or_create`'s no-match + branch, so a cwd match silently discarded it and every distinct id in a shared directory collapsed onto + one shared session — which is also why `serve_ws` used to rewrite each daemon session into single-file + mode to escape it. And (3) used to reattach like (2), so two shells in one repo drove one store, where + `append_new`'s count-keyed dedup meant neither could observe the other's writes. + + Every cwd this module ever records + into or matches against a session — `serve --continue`, `run --continue` — is passed through `canonical_cwd` first (resolves symlinks/`.`/`..`, drops a trailing separator), so a project reached through a symlink one time and its real path another still matches the same session instead of silently fragmenting into two; a path that can't be resolved (removed out from under the process) @@ -615,7 +643,8 @@ The harness layers several capabilities over the bare tools + loop: `meta.model`/`meta.thinking_level` mean. **`serve`'s own startup now applies the identical restoration, not just RPC-triggered transitions** - (Task #5, pi-parity fix) — reattaching to an existing session (repo-mode cwd match, or an explicit + (Task #5, pi-parity fix) — reattaching to an existing session (an addressed `--session-id`, a + `--continue` cwd match, or an explicit `--session-file`/`--session-dir` that already has content) used to always seed `current_model`/ `starting_level` from `cfg.model`/`cfg.reasoning_effort` (the CLI flag, or the stored global default when the flag was absent), even when that session's own active tip was actually last driven on @@ -1764,24 +1793,24 @@ spawn ──► Booting ──writer task up + "ready" frame sent──► Ready Closed ``` -| From | Event | To | Guard | What Actually Happens | -| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Booting | writer task spawned, `ready` frame sent | Ready | `out_tx.send` succeeds | `Session` restored from persistence (file/dir/none); `session_id` minted; static system prompt built | -| Ready | `{"type":"prompt"}` | RunningTurn | — | `ack` frame sent immediately; message pushed as a user turn; `Agent::run_events_steered` streams `event` frames live | -| RunningTurn | `{"type":"abort"}` / `"stop_after_turn"` / `"steer"` / `"follow_up"` | RunningTurn | — | cancels the run / requests a graceful stop at the next turn boundary / queues a mid-run steer / queues a stop-boundary follow-up — the run keeps going until its current turn's tool calls (if any) finish | -| RunningTurn | `{"type":"prompt", streaming_behavior:"steer"\|"follow_up"}` | RunningTurn | — | accepted (not rejected as busy) and routed through the same `Steering` queue as an explicit `steer`/`follow_up` | -| RunningTurn | any other command | RunningTurn | — | rejected: `response{success:false, error:"busy…"}` — the session is borrowed by the in-flight run | -| RunningTurn | model ends turn (no more `tool_use`, not a refusal) | Ready | persist succeeds | session persisted (rewrite/rewrite_compacted if compaction fired); `response{success:true, data:{steps,…, refused:false}}` | -| RunningTurn | model ends turn, but persisting the transcript fails (disk full, permission error, …) | Ready | — | `response{success:false, data:{steps,…}, error:"run completed but failed to persist: …"}` — the run itself is not retried or lost, but the client is told the transcript may not be durable yet, rather than a false success | -| RunningTurn | model refuses (`StopReason::Refusal`) | Ready | — | run ends immediately _without_ draining queued steering; `response{success:true, data:{…, refused:true}}` | -| RunningTurn | `Error::MaxSteps` / transport error | Ready | — | session still persisted; `response{success:false, error}` — the process keeps serving | -| Ready | `{"type":"new_session", parent_session?}` | Ready | on-disk reset/create succeeds | history replaced; `steering.clear()`; `response{success:true, data:{session_id,…}}` — `parent` in the response is `parent_session` when given, else whatever was active before this call | -| Ready | `{"type":"new_session"}`, on-disk reset/create fails | Ready | — | `response{success:false, error}` — the _previous_ session stays active (nothing is swapped for an empty in-memory session that was never actually persisted, which would otherwise desync `SessionStore`'s persisted-message count from what the caller believes is live, silently dropping the next turn's `append_new`) | -| Ready | `{"type":"switch_session"}` / `"fork"` / `"clone"` / `"switch_branch"` | Ready | — | history replaced/switched; `steering.clear()` — a message queued for the old session's next turn can't leak into the new one | -| Ready | `{"type":"set_model"}` / `"cycle_model"` / `"set_reasoning_effort"` / `"cycle_thinking_level"`, recording the change fails to persist | Ready | — | `response{success:false, error}`; the live model/thinking-level is _not_ switched either — a failed persist never leaves live state ahead of what's durably recorded | -| Ready | invalid JSON / unknown `type` | Ready | — | `response{success:false, error}`; loop continues, no state change | -| Ready/RunningTurn | stdin EOF | Closed | — | `out_tx` dropped → writer drains its queue → awaited → process returns `Ok(())` | -| any | stdout write fails (broken pipe) | Closed | — | writer task `break`s its receive loop; the next `emit!` send fails → main loop `break`s | +| From | Event | To | Guard | What Actually Happens | +| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Booting | writer task spawned, `ready` frame sent | Ready | `out_tx.send` succeeds | `Session` restored from persistence (file/dir/none); `session_id` minted; static system prompt built | +| Ready | `{"type":"prompt"}` | RunningTurn | — | `ack` frame sent immediately; message pushed as a user turn; `Agent::run_events_steered` streams `event` frames live | +| RunningTurn | `{"type":"abort"}` / `"stop_after_turn"` / `"steer"` / `"follow_up"` | RunningTurn | — | cancels the run / requests a graceful stop at the next turn boundary / queues a mid-run steer / queues a stop-boundary follow-up — the run keeps going until its current turn's tool calls (if any) finish | +| RunningTurn | `{"type":"prompt", streaming_behavior:"steer"\|"follow_up"}` | RunningTurn | — | accepted (not rejected as busy) and routed through the same `Steering` queue as an explicit `steer`/`follow_up` | +| RunningTurn | any other command | RunningTurn | — | rejected: `response{success:false, error:"busy…"}` — the session is borrowed by the in-flight run | +| RunningTurn | model ends turn (no more `tool_use`, not a refusal) | Ready | persist succeeds | session persisted (rewrite/rewrite_compacted if compaction fired); `response{success:true, data:{steps,…, refused:false}}` | +| RunningTurn | model ends turn, but persisting the transcript fails (disk full, permission error, …) | Ready | — | `response{success:false, data:{steps,…}, error:"run completed but failed to persist: …"}` — the run itself is not retried or lost, but the client is told the transcript may not be durable yet, rather than a false success | +| RunningTurn | model refuses (`StopReason::Refusal`) | Ready | — | run ends immediately _without_ draining queued steering; `response{success:true, data:{…, refused:true}}` | +| RunningTurn | `Error::MaxSteps` / transport error | Ready | — | session still persisted; `response{success:false, error}` — the process keeps serving | +| Ready | `{"type":"new_session", parent_session?}` | Ready | on-disk reset/create succeeds | history replaced; `steering.clear()`; `response{success:true, data:{session_id,…}}` — `parent` in the response is `parent_session` when given, else whatever was active before this call. An **addressed** session (`--session-id`, the daemon's `?session_id=`) keeps its own id instead of minting one, since something is routing on it: the outgoing conversation is archived into a sibling session (`parent` = this id) and this one is blanked in place, so "start a new session" is never also "destroy the old one" | +| Ready | `{"type":"new_session"}`, on-disk reset/create fails | Ready | — | `response{success:false, error}` — the _previous_ session stays active (nothing is swapped for an empty in-memory session that was never actually persisted, which would otherwise desync `SessionStore`'s persisted-message count from what the caller believes is live, silently dropping the next turn's `append_new`) | +| Ready | `{"type":"switch_session"}` / `"fork"` / `"clone"` / `"switch_branch"` | Ready | — | history replaced/switched; `steering.clear()` — a message queued for the old session's next turn can't leak into the new one | +| Ready | `{"type":"set_model"}` / `"cycle_model"` / `"set_reasoning_effort"` / `"cycle_thinking_level"`, recording the change fails to persist | Ready | — | `response{success:false, error}`; the live model/thinking-level is _not_ switched either — a failed persist never leaves live state ahead of what's durably recorded | +| Ready | invalid JSON / unknown `type` | Ready | — | `response{success:false, error}`; loop continues, no state change | +| Ready/RunningTurn | stdin EOF | Closed | — | `out_tx` dropped → writer drains its queue → awaited → process returns `Ok(())` | +| any | stdout write fails (broken pipe) | Closed | — | writer task `break`s its receive loop; the next `emit!` send fails → main loop `break`s | --- @@ -2127,89 +2156,90 @@ backend an operator points it at. ## Package Structure -| File | What It Does | -| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `src/main.rs` | CLI entry point (`run`/`serve`/`tools`/`list-models`/`trust`/`untrust`/`clear-trust`/`trust-status`/`settings`/`export`/`login`/`logout`/`auth-status`/`mcp-login`/`mcp-logout` subcommands (the model-provider trio: `crate::auth_store`/`crate::oauth`'s CLI surface, same dispatch shape as `trust`/`settings`; `mcp-login`/`mcp-logout` are the MCP-server-OAuth analogue — see `mcp_auth_store.rs`'s own row below) — `list-models` prints a table (`model`/`context`/`max-out`/`thinking`/`vision` columns) over `serve::available_models()`, no gateway/key needed, each row's capability data pulled straight from `agent_core::capabilities` — pi's own `--list-models` table, minus a `provider` column this crate has no separate field for (a model id is forwarded verbatim; see `agent_core::models`'s own module doc comment); `context`/`max-out` are humanized (`format_token_count`: `"200K"`/`"1M"`, one decimal place only when not a whole unit), not raw integers, matching pi's own `formatTokenCount` (Task #39 pi-parity fix, cosmetic); an optional positional `search` (Task #51 pi-parity fix) fuzzy-filters/ranks rows via `fuzzy_match` — a ported, non-contiguous, order-preserving, word-boundary-scored subsequence match (plus a letters/digits-swapped fallback try for a query typed in the opposite order, e.g. "5sonnet") matching pi's own `fuzzyFilter`/`fuzzyMatch` (`packages/tui/src/fuzzy.ts`), so e.g. "sn5" finds "claude-sonnet-4-5" — previously a plain case-insensitive substring `contains` check, which that query would never match at all; `settings` views/updates `settings::SettingsStore`'s stored defaults, printing every field's current value after any requested update); `expand_short_aliases` rewrites pi's own hand-rolled multi-character short flags (`-np`/`-nc`/`-na`, …) to their long form before clap sees them, including (Task #43 pi-parity fix) a lowercase `-v` alias for `--version` — clap's auto-generated version flag only binds capital `-V`; `DEFAULT_MODEL`, `DEFAULT_GATEWAY`, `default_system_prompt(®istry)` (generated from the actually-registered, filtered tool set); renders streamed text + `[tool: name]` markers (followed live by each `InputJsonDelta` fragment — a | -| growing preview of the call's arguments as they stream in, not just its name) to stdout for `run` in its default text mode, or (`run --json`) one `AgentEvent` object per line via `run_events`/`serde_json::to_string` — the same full observation surface (tool calls/results, turn boundaries) `serve`'s NDJSON protocol streams, preceded by a `{"kind":"session", id, model, cwd}` header line — for a scripting caller that wants structured output without spawning `serve`; `run` composes its first message from piped stdin (trimmed; a whitespace-only pipe is treated as nothing piped at all — Task #37 pi-parity fix, matching pi's own `readPipedStdin`) + `@file` references (`partition_tasks`/`read_file_refs_with_home`, which tilde-expands each ref via `tools::expand_tilde` before joining with `cwd` — Task #20 pi-parity fix — and skips a zero-byte file entirely rather than emitting an empty `` block — Task #38 pi-parity fix, matching pi's own `file-processor.ts`) + the first positional message, runs any further positional messages as separate sequential turns, and — via `--session `/`--continue` (reusing `SessionStore`/`SessionRepo::resume_or_create`) — can persist and resume a transcript across invocations exactly like `serve`'s own repo/file modes; `--session ` (Task #24 pi-parity fix, matching pi's own `resolveSessionPath`) accepts either a literal path (path-like — has a `/`, or a leading `./`/`~` — or one that already exists on disk, used as-is, creating a fresh session there when absent exactly as always) _or_ a bare session id/prefix, resolved (`open_session_by_id`) against the current project's own repo first, then cross-project — the identical two-tier search `--fork ` already does via `fork_by_arg` — and reopened **in place**, continuing that session rather than forking a new one (unlike `--fork`, which always copies); previously `arg` was always treated as a literal filesystem path, so a bare id silently created an empty, wrongly-named session file instead of reopening the one actually meant; `--fork `(pi's own cross-project`--fork`, wins over`--session`/`--continue`) instead copies an *existing* session's transcript into a brand-new one under the current project and continues from there — a path opens that`.jsonl`file directly regardless of which project it belongs to, an id is searched in the current project's own repo first and then, by filename rather than by recomputing a path from the source's recorded`cwd`(`session_store::find_session_path_under`— the recompute approach broke under a non-default`--session-dir`, whose directory name need not encode any`cwd`at all), across every project's own directory under the session root, so a session started in one project can be picked up and continued from a different one (`session_store::fork_by_arg`/`fork_from_path`);`--session-dir`/`AI_AGENT_SESSION_DIR`(matching`serve`'s identical flag) redirects that root — both the repo`--continue`reopens and the root`--fork`'s cross-project search spans (that search then scopes to the given directory's own *parent*, the same convention`serve`'s`list_all_sessions`uses) — away from the default`~/.claude/sessions//`;`--session-id `(matching pi's own flag) overrides the freshly-generated id with a caller-chosen one, wherever a *new*`SessionMeta`is actually minted (a fresh`--session `or a plain ephemeral run) — a script/test harness gets a known id to correlate against instead of parsing one back out of the run's own output;`run --export `renders the finished transcript via`export::export_html`after a live run completes, while the standalone`export [output.html]`subcommand renders an *already-persisted* session file straight off disk — no gateway, key, or model involved at all (`SessionStore::open`+`export::export_html`) | -| `src/lib.rs` | Library root; re-exports `serve`/`tools`/`resources`/`skills`/`prompts`/`session_store`/`trust_store`/`export`/`auth_store`/`auth_credential_source`/`oauth` for tests/benches | -| `src/serve.rs` | NDJSON control protocol: single stdout-writer task, `Persistence` (file/dir/none, default-per-cwd directory), a large command set (session/branch nav, `reload`, model/thinking/tool/auto-compaction tuning, `bash`/`abort_bash`, `export_html`) — see the module's own doc comment for the exhaustive list; prompt runs concurrently with stdin routing `steer`/`follow_up` (also accepted while idle, via a persistent `Steering` handle) | -| `src/export.rs` | `export_html`/`render_html` — renders a session's transcript as one self-contained, dependency-free HTML file (inline CSS, no client-side JS, images inlined as data URIs); every abandoned branch (passed in as `SessionStore::abandoned_branches`'s output) renders inline as a collapsible `
` block right after the message it actually diverged from (`render_branches_diverging_at`, native HTML — no script) — only the divergent suffix, not the shared prefix already shown above, and numbered sequentially across the whole document; every built-in tool call (`edit`/`write`/`bash`/`read`/`grep`/`find`/`ls`, and the Beyond platform tools `fork`/`sync`/`logs`) gets a dedicated renderer (`render_tool_call`) instead of raw pretty-printed JSON — `edit` reuses the diff-coloring machinery (`diff_pair_html`) to show its before/after as a real diff, falling back to generic JSON if `input` doesn't parse as a valid edit shape; only a genuinely unrecognized tool name (a third-party extension) stays generic JSON; message text is rendered as markdown (`render_markdown`, `pulldown-cmark`, server-side at export time — not pi's client-side `marked`/`highlight.js`) — a bare `\n` not preceded by two-plus trailing spaces (CommonMark's own soft break) now renders as a hard `
`, matching pi's own `marked.use({ breaks: true })` (previously the two collapsed into a run-on paragraph with no visible break at all) — with raw HTML defused to visible text and link/image URLs scheme-allow-listed (`sanitize_url` — `http(s):`/`mailto:` plus, as of a pi-parity fix, `tel:`/`ftp:`; everything else, `javascript:` included, is neutralized); a `[Earlier conversation compacted…]`/branch-summary marker line (`parse_summary_marker`/`render_summary_marker`) renders as its own distinct styled block rather than plain markdown, and — for a real compaction specifically — surfaces the token count `compaction::apply_summary` now embeds as a leading `"Compacted from {N} tokens\n\n"` line in the summary body (`parse_compaction_tokens_before` strips and parses it back out, `format_thousands` for display; `None`/absent for a branch summary, which never carries that line) as a `· Compacted from N tokens` note on the block's label; `serve.rs`'s host-bash-command marker (a `[Host bash command, run outside the model's own turn]\n$ …` prefix — commands run out-of-band of the model's own turn — or, Fix 9 pi-parity gap, the `exclude_from_context: true` counterpart `[Host bash command, excluded from model context]\n$ …`, recorded the same way and rendered identically except for a "hidden from the model" note in its title, since hiding a command from the _model_ is a separate concern from hiding it from the exported _transcript_ — see `serve.rs::ServeHooks`) is recognized the same way (`parse_host_bash_marker`/`render_host_bash_marker`) and rendered as its own styled `tool-call host-bash` block instead of falling through to plain markdown, with a distinct `.error` variant; a fenced `` ```diff `` block or diff-shaped tool-result content (`looks_like_diff`) gets per-line +/- coloring (`diff_html`) instead of real syntax highlighting, which is deliberately not implemented (would need a heavy crate like `syntect`, bloating every build of this CLI for a nice-to-have); reuses `skills::xml_escape` for HTML-text escaping; `export_html_full`/`render_html_full` additionally render the system prompt and tool set (each a collapsed-by-default `
` section) plus token-usage totals — pi's own always-included `systemPrompt`/`tools` fields, previously omitted entirely — with `serve`'s `export_html` RPC command and `run --export` (Task #44) both passing the actually-running agent's real system prompt/`agent_core::ToolRegistry::definitions`/session usage; the standalone `export` subcommand still passes `None` for `system_prompt`/`tools` (no live `Agent`/`ToolRegistry` to pull either from — a genuine absence, not an oversight) but, as of Fix 6 (pi-parity gap), no longer for usage — `serve::message_export_usage_totals` sums each message's own `usage` straight out of `sess.messages`, no live `Session` counters needed, so all three export entry points now report the same totals for the same session instead of the standalone path silently omitting the line; shared by `serve`'s `export_html` RPC command, `run --export`, and the standalone `export` subcommand | -| `src/policy.rs` | `ToolPolicy` — the concrete `agent_core::AgentHooks` implementation this crate installs: `--deny-tool`/`--deny-bash-pattern`/`--deny-path` (all three repeatable, comma-separated, `serve` also env-var-backed via `AI_AGENT_DENY_PATH` etc.) build a static deny-list gating `before_tool_call` — `--deny-path` compiles each pattern to a `globset::GlobMatcher` once at construction and matches it against a `write`/`edit` call's canonicalized target path; installed on `Agent` only when non-empty, else the zero-cost `NoHooks` default; shared by `run` and `serve` (`ToolPolicy::from_lists`) | -| `src/trust_store.rs` | Tri-state (`Trust::{Trusted,Untrusted,Unknown}`), ancestor-inheriting trust allowlist (`~/.claude/trusted-projects.json`); `trust`/`distrust` record an explicit grant/denial, `clear` removes a directory's own entry (trusted _or_ untrusted) without recording a new one, reverting it to inheriting its nearest ancestor's decision; legacy bare-array files still parse (trusted-only); allowlist keys are resolved via `path_utils::resolved_path` (absolutize-and-lexically-normalize _before_ a real `canonicalize()` attempt) — matching pi's `resolvePath` → `canonicalizePath` order, so pre-trusting a not-yet-created directory via a relative or trailing-slash path (`agent trust newproj/`) still matches once the directory exists and a later `lookup()` canonicalizes an absolute `cwd`, instead of comparing against the stale literal string `canonicalize()` alone would have fallen back to on its `ENOENT`; `read_store_file` distinguishes a missing file (silent — nothing has ever been trusted yet) from an existing-but-unreadable one or one that fails to parse as either the current or legacy shape (both `warn!`, since either would otherwise silently discard every persisted trust/untrust decision with no signal anything was wrong) | -| `src/auth_store.rs` | `AuthStore` — OAuth/subscription-credential storage (`~/.claude/auth.json`), same shape family as `trust_store.rs`/`settings.rs` (own small `FileLock`, atomic writes, re-read-under-lock-before-mutate, no shared code with either); `refreshed()` is the one genuinely new piece — double-checked-locked refresh that holds the lock across the _entire_ refresh including the network call, preserves the credential bit-for-bit and records `last_refresh_error` (not deletion) on failure; the store file is created at `0600` atomically on first write (`write_atomic` only _preserves_ an existing file's mode, so this is a real, deliberate first-write step, not redundant with it); a local `Secret` type round-trips its real value through `serde` (unlike `agent_core::client::ApiKey`, never serialized, or `beyond-gateway::Secret`, which redacts on serialize) since persisting the real value to disk is this store's whole job; deliberately OAuth-only, no `api_key` credential variant — see "Provider keys" above | -| `src/mcp_auth_store.rs` | `McpAuthStore` — persisted MCP OAuth credentials (`~/.claude/mcp_auth.json`), keyed by MCP server _name_ rather than a fixed provider enum (contrast `auth_store.rs`); same shape family (own small `FileLock`, atomic writes, re-read-under-lock-before-mutate, no shared code with the other three stores). `ScopedMcpCredentialStore` implements `rmcp::transport::auth::CredentialStore` scoped to one server's own slot in the file — what `agent mcp-login`/`tools::mcp::connect_http` actually read and write through; see the "MCP Server OAuth" section above for the full mechanism. | -| `src/auth_credential_source.rs` | `OAuthCredentialSource` — the one `agent_core::client::CredentialSource` implementation this crate ships: an in-memory fast path for a still-valid cached token, else `spawn_blocking`s into `AuthStore::refreshed` (bridging its one async sub-step, the provider's token-endpoint call, back in via `Handle::current().block_on`) — kept off the tokio worker thread the same way `serve.rs`'s `persist_blocking` keeps a blocking session write off it | -| `src/oauth/` | The three providers' OAuth protocol flows — `anthropic.rs` (PKCE + local callback, port `53692`, the `state = verifier` quirk), `github_copilot.rs` (device-code, Copilot-internal-token re-derivation, model-availability discovery, plus `copilot_endpoint_path`/`CopilotRoutedCredentialSource` — the `agent_core::client::CredentialSource` wrapper `gateway_credential::resolve_gateway_credential` builds for a Copilot-routed model; unlike OpenAI Codex's fully-static routing, Copilot's `base_url` is re-derived via `base_url_from_token` fresh on every `credential()` call, from whichever access token is CURRENTLY on disk — re-read directly from the auth store rather than through the wrapped source's own opaque `Credential` — since GitHub can migrate an account to a different Copilot proxy pool mid-session, a real, previously observed occurrence a value frozen at construction would silently misroute after), `openai_codex.rs` (user-selectable browser PKCE / custom device-code, JWT-claim account-id extraction) — plus shared `pkce.rs`/`device_code.rs`/`callback_server.rs`/`callbacks.rs` (the `LoginCallbacks` trait a CLI or `serve` implements) /`error.rs`. Lives in `agent`, not `agent_core`: a second, direct-to-provider network surface `agent_core`'s own doc comment explicitly disclaims owning, and device-code polling/the local callback listener both need a live tokio runtime as a real (non-dev) dependency, which `agent_core` deliberately doesn't have. See "OAuth / Subscription-Credential Authentication" above for the full picture. | -| `src/path_utils.rs` | Shared path-normalization helpers, extracted from `trust_store.rs` once `skills.rs`/`prompts.rs` needed the identical logic: `absolutize`/`lexically_normalize` (pure, no filesystem access beyond reading the cwd — work even for a path that doesn't exist yet) and `resolved_path` (canonicalize on top of that, falling back to the absolutized-and-normalized form on failure); `push_unique_scoped_root` — push a `(discovery root, scope tag)` pair onto a `Vec<(PathBuf, T)>` only if the root's canonical form hasn't already been seen (tracked in a caller-owned `HashSet`), so a root reached by two different paths (a symlink, a relative-vs-absolute spelling, `cwd` itself equaling a global root) is scanned only once — used by `skills.rs`/`prompts.rs` to keep a real directory reached twice from double-counting its contents and self-colliding every name in it against a phantom duplicate, and to carry each root's own `"user"`/`"project"` scope tag (Task #39) alongside it without a second parallel structure; pi doesn't dedupe at the root-list level either (only file-level, in `skills.ts`'s own `realPathSet`) — a Beyond-specific hardening | -| `src/settings.rs` | `SettingsStore` — persisted global-tier `default_model`/`default_gateway_url`/`default_session_dir`/`default_project_trust`/`compaction_enabled`/`default_reasoning_effort`/`block_images`/`image_auto_resize`/`thinking_budget_overrides`/`default_bash_shell_path`/`default_bash_command_prefix`/`default_compaction_reserve_tokens`/`default_compaction_keep_recent_tokens`/`default_retry_max_retries`/`default_retry_base_delay_ms`/`default_provider_timeout_ms`/`default_models_list`/`default_skill_paths`/`default_prompt_template_paths`/`mcp_servers` (`~/.claude/settings.json`; see `tools/mcp.rs`'s own row above for what `mcp_servers`'s `McpServerConfig`/`McpTransport` shape does), consulted by `run_task`/`Command::Serve` as the last fallback before this crate's own built-in default; managed via `agent settings`, mirroring `trust_store.rs`'s own out-of-band management convention and reusing its exact on-disk pattern (own small `FileLock`, atomic writes, re-read-under-lock-before-mutate) without sharing code with it. `Settings::merge_over` + `effective_settings_for_cwd` (Round 3, pi-parity feature) layer a _project_-level `/.claude/settings.json` tier on top of the global one, field-by-field, gated on `trust_store::TrustStore::is_trusted(cwd)` specifically (a persisted grant only — see the "Persisted settings" section above for the full trust-gating rationale and the list-field replace-not-append semantics); `main.rs`'s `run_task`/`Command::Serve` call this instead of the bare `SettingsStore::open_default()` they used before. Also `ModelOverride`/`ModelOverrides` — a read-only, hand-edited `~/.claude/models.json` (model id → `{base_url, api_key, headers}`), consulted by `gateway_credential::resolve_gateway_credential`/`main.rs::model_override_extra_headers`; `api_key`/`headers` values resolve through `resolve_config_value`'s `!command`/`$VAR`/`${VAR}`/literal syntax (pi's own `resolveConfigValue`) | -| `src/gateway_credential.rs` | `resolve_gateway_credential(key, model)` — the one seam that decides how a given model id reaches the gateway (managed/BYO key, a `models.json` `base_url` override, or an inferred OAuth subscription login) and builds the matching `agent_core::client::CredentialSource`. Keyed on `model`, never cached across a model switch: called fresh by `main.rs` for `run` (once, since `run` never switches models mid-process) and by `serve::build_gateway_client` — at `serve` startup _and_ every runtime command that changes the active model (`set_model`/`cycle_model`/`switch_session`/`fork`/`clone`/`switch_branch`) — so a switch that crosses OAuth providers gets that provider's own credential/routing instead of silently reusing whichever one was resolved for the previously active model. `DirectRoutedCredentialSource` (OpenAI Codex — its `chatgpt-account-id`/gateway-prefix routing is genuinely fixed per login) and `StaticDirectCredentialSource` (a `models.json` override's fixed bearer + `base_url`) both live here; GitHub Copilot's own wrapper, `CopilotRoutedCredentialSource` (its proxy host is NOT fixed per login — see `oauth/github_copilot.rs`'s row below), lives with the rest of that provider's logic instead | -| `src/session_store.rs` | JSONL `SessionStore` (fsync'd append/atomic-rewrite/mid-file-corruption recovery, header metadata + durable `Entry::Compaction` provenance, version-migration guard, collision-safe ids; `create` initializes an existing but zero-byte file in place — `touch`'d ahead of time, or left over from a crash before the header write landed — rather than hard-failing, while still refusing (`AlreadyExists`) a genuinely non-empty one; `run`/`serve`'s own `--session ` open-vs-create decision checks file _content_, not just existence, so this path actually gets exercised) + multi-session `SessionRepo` (list-with-metadata, soft-delete-to-`.trash`, fork + read-only fork preview, `resume_or_create` — reopen the most recent session matching a `cwd` or make a fresh one, shared by `serve`'s startup reattach and `run --continue`; `fork_from_path` forks an arbitrary source session that need not live in `self.dir` at all, stamping the _target_ project's own `cwd` rather than the source's — the primitive `fork_by_arg` (module-level, not a `SessionRepo` method: it may need to open a _second_ repo for the source) builds `run --fork `CLAUDE.md` discovery, skill injection from an already-discovered `PromptOptions::skills` slice — the caller's job, not this function's, since every real caller already discovers skills separately for its own purposes and re-discovering here too would walk the same directories twice — expensive, cached) and `dynamic_footer` (local date/cwd — cheap, refreshed every turn); `build_system_prompt` composes both for a one-shot caller | -| `src/skills.rs` | Recursive skill discovery (`SKILL.md` frontmatter up to `MAX_DEPTH` — 8 — levels deep per root, a Beyond-specific bound pi's own unbounded walk doesn't have, `disable-model-invocation`, `/skill:` lookup — expands into a `` tag with the frontmatter stripped, not the raw file) across `~/.claude/skills`+`/.claude/skills` and the vendor-neutral `~/.agents/skills`+every `.agents/skills` between `cwd` and the enclosing git-repo root (`collect_ancestor_agents_skill_dirs`/`find_git_repo_root` — `.agents/skills` never recognizes the loose-root-`.md`-file shape `.claude/skills` does) + `` rendering + `discover_with_diagnostics` (name-collision reporting; `.claude/skills` wins over `.agents/skills`, and the `.agents/skills` level closest to `cwd` wins over one further up; every discovery root, standard or `--skill` extra, is deduped by canonical path via `path_utils::push_unique_scoped_root` before it's ever walked, so the same real directory reached twice can't double-count its skills into a phantom self-collision); `project_trusted` gates only the project-local roots, the user-global roots are always scanned; `validate_skill_name`/`validate_skill_description` — non-fatal, `warn!`-logged shape/length checks (a bad `name`, or a `description` past 1024 chars) that never block discovery | -| `src/prompts.rs` | `/name args` prompt-template discovery (gitignore-aware, non-recursive single-directory scan — `.gitignore`/`.ignore`/`.fdignore` only, no global excludes/`.git/info/exclude`, matching pi's own `IGNORE_FILE_NAMES`/`readdirSync` scope) + bash-style expansion (quote-aware args, `$N`, `${@:N:L}` slices, `${N:-default}`, `description`/`argument-hint` frontmatter — parsed via `skills::parse_frontmatter`, shared rather than reimplemented, so a YAML block scalar is understood the same way a skill's frontmatter already is) + `discover_with_diagnostics` (name-collision reporting; same canonical-path root dedup as `skills.rs`, via the identical `path_utils::push_unique_scoped_root`) | -| `src/timing.rs` | `StartupTiming` — `AI_AGENT_TIMING=1`-gated startup profiling (pi's own `PI_TIMING=1`/`timings.ts`); `mark(label)`/`print()` are no-ops (don't even read the clock) when unset, so it's safe to sprinkle through `run`/`serve`'s startup path unconditionally; prints to stderr only, never stdout | -| `src/tools/mod.rs` | `default_registry_with(bash_timeout_ms)` — assembles the base 10-tool `ToolRegistry`; `apply_filter(&mut registry, tools, exclude_tools, no_tools)` — allow/deny-list/no-tools filtering applied once at process build time; `default_registry_with_prefix_image_auto_resize_and_mcp_tools` additionally merges in already-connected `tools::mcp` tools (see that module's own row below) after every built-in, so `--tools`/`--exclude-tools` scope MCP-discovered tools too, not just built-ins | -| `src/tools/read.rs` | `read` — line-numbered read with `offset`/`limit` (explicit `limit` also capped at `DEFAULT_LIMIT`, 2000), byte budget, offset-past-EOF error, continuation hints; image files sniffed by magic bytes (`is_valid_bmp` structurally validates a `"BM"`-prefixed file's DIB header rather than trusting the bare 2-byte signature; a SOF55/JPEG-LS 4th byte excludes an undecodable JPEG variant) and downscaled/re-encoded (Lanczos3, PNG-then-JPEG, Exif orientation via `image`'s own generic decoder API — JPEG and WebP both) to fit a 4.5 MB base64 budget; an image already under that budget is still decode-validated (not just magic-byte-sniffed) before being sent as-is — matching pi's `resizeImageInProcess`, which always decodes even on its own already-fits fast path — so a truncated/bit-rotted/polyglot file that merely starts with a real image's magic bytes gets a clear tool error instead of being forwarded to the model unchecked; appends pi's `getNonVisionImageNote` when dispatch's injected `_model_supports_vision` field says the active model can't see images at all | -| `src/tools/write.rs` | `write` — create/overwrite a file, creating parent directories | -| `src/tools/edit.rs` | `edit` — exact-then-fuzzy (NFKC/quote/dash/space/trailing-ws) replacement matched in LF space (CRLF/BOM restored), against the original, overlap/no-op checks, `replace_all` | -| `src/tools/ls.rs` | `ls` — directory listing, directories-first sort, dotfile filtering, `limit` entry cap | -| `src/tools/grep.rs` | `grep` — parallel, gitignore-aware regex (or `literal`) search with `context`/`before`/`after` lines, a whole-output byte cap (`MAX_OUTPUT_BYTES`), deterministic sort+truncate | -| `src/tools/find.rs` | `find` — sequential, gitignore-aware glob search over files **and** dirs; deterministic sort+truncate | -| `src/tools/bash.rs` | `bash` — resolved real-`bash` (falling back to `sh`) execution with a 30-minute default timeout, streaming `OutputAccumulator` (tail-truncated display + full-output temp-file spill), output hygiene (ANSI strip/control sanitize) | -| `src/tools/output.rs` | Shared `OutputAccumulator`/`format_size`/`marker` — the bounded, spill-to-disk output-truncation machinery every truncating tool (`bash`/`read`/`grep`/`ls`/`find`) now shares; `snapshot_text` holds back an incomplete trailing UTF-8 sequence (`incomplete_utf8_suffix_len`) on a live (not-yet-`finish`ed) snapshot instead of transiently lossy-decoding it to `U+FFFD` at a chunk boundary | -| `src/tools/beyond.rs` | `fork`/`sync`/`logs` — shell out to the `beyond` platform CLI | -| `src/exec_endpoint.rs` | `HttpExecRunner` (POST a command to a URL — the whole remote contract, vendor-agnostic, no lifecycle) and `TemplateRunner` (argv template for `ssh`/`docker exec`/`kubectl exec`, `{}` expanding to separate argv entries, never a shell string) | -| `src/tools/fs/mod.rs` | `FsBackend` trait + `PathWorld`, `Meta`/`FileKind`/`DirEntry`, `SearchQuery`/`GlobQuery`, and the impl-independent tail every backend shares: `clip`, `trim_eol`, `finalize`, `finalize_glob` | -| `src/tools/fs/local.rs` | `LocalFs` — the host filesystem: ripgrep's parallel `ignore` walk + `grep-searcher` sink, `std::fs` reads/stats, `write_atomic`, and the FIFO-safe `stat` (the writability access check is gated on kind, because opening a FIFO for write blocks forever) | -| `src/tools/fs/shell.rs` | `ShellFs` — the same operations as commands over any `CommandRunner`: `Capabilities` probe, `rg`/POSIX-`grep` rungs, NUL-scanning output parsers (a filename may contain a newline), base64 for content (`ExecResult::stdout` is a `String`), and the readability guard that stops a failing `dd` being masked by a succeeding `base64` in the pipeline | -| `src/tools/exec.rs` | `CommandRunner` trait + `RealRunner` (stdin closed via `Stdio::null()`, `GroupKillGuard` process-group kill on timeout _or_ a dropped/cancelled future, bounded head+tail streaming capture, `ExecResult.truncated`) — the process-execution seam shared by `bash`/`beyond` tools and `serve`'s own `bash`/`abort_bash` RPC | -| `src/tools/mcp.rs` | MCP (Model Context Protocol) client — this crate's extension mechanism, deliberately MCP rather than pi's in-process `registerTool`/`registerCommand` TypeScript modules: a standardized, language-agnostic protocol with an existing server ecosystem, and no `unsafe`/`libloading` plugin-loading needed (the workspace forbids `unsafe_code`). Built on the official `rmcp` SDK (client-side features only: `client`, `transport-child-process`, `transport-streamable-http-client-reqwest` — this binary never acts as an MCP _server_). `connect_all(&[McpServerConfig]) -> (Vec>, Vec)` connects to every configured server exactly once (stdio via `TokioChildProcess`, streamable-HTTP via `StreamableHttpClientTransport`), lists each one's tools via `tools/list`, and wraps each into an `McpTool` (forwards `run()` straight to `tools/call`, maps `ContentBlock::Text`/`Image` into `ToolOutput.text`/`images`, `is_error: true` into `ToolError::Execution`) — registered as `mcp____` (Claude Code's own convention) so it can never collide with a built-in or another server's tool. A server that fails to connect is skipped with a warning (`connect_all`'s second return value, printed by both `run`/`serve` call sites), never failing the whole agent's startup — matches this crate's "skip and warn, don't silently lose data" convention (`settings::read_store_file`). Connecting happens once, up front (`main.rs::run_task`; `serve.rs` before its own session loop starts, cached on `ServeConfig::mcp_tools` so a `set_model`/`set_thinking` registry rebuild reuses the live connections rather than reconnecting) — both call sites are exercised in tests (`tests/mcp_client.rs`'s `..._through_serve_too` test, `tests/mcp_oauth.rs`'s `..._is_honored_by_serve_too_not_just_run`), not just `run`. There is, deliberately, no way to trigger `agent mcp-login`'s interactive flow from inside a live `serve` session (no RPC command analogous to `login`/`logout`/`auth_status`) — it's a one-shot CLI command run before `serve` starts, and a credential established after a `serve` session is already running won't be picked up until that session restarts. Out of scope for v1: MCP _resources_/_prompts_ (only `tools` is wired up), and per-session dynamic add/remove of a single server (matches the existing whole-registry-rebuild pattern for every other tool-set change). Configured exclusively via `mcp_servers` in `settings.rs`'s `Settings` (global `~/.claude/settings.json`, or a trusted project's own `/.claude/settings.json` — already trust-gated, see `settings.rs`'s row above) — no CLI flag, matching `models.json`'s own hand-edited-only convention; see `McpServerConfig`/`McpTransport` there for the exact JSON shape (`{"name", "transport": "stdio", "command", "args", "env"}` or `{"name", "transport": "http", "url", "headers"}`, each `env`/`headers` value resolved through the same `resolve_config_value` `!command`/`$VAR`/literal syntax `ModelOverride` uses). Tested end to end in `tests/mcp_client.rs` against a real subprocess (`src/bin/mcp_fixture_stdio_server.rs`, below) and a real TCP listener (streamable-HTTP) — never a mock of the MCP protocol itself. | -| `src/bin/mcp_fixture_stdio_server.rs` | Test fixture only, not a real MCP server: a ~150-line hand-rolled MCP server speaking newline-delimited JSON-RPC over stdio, with zero new dependencies (`tokio`/`serde_json`, already ordinary deps of this crate) rather than pulling in `rmcp`'s own server-side machinery just for a test double. Auto-discovered by Cargo as a sibling `[[bin]]` of this same package (any `.rs` file under `src/bin/` becomes its own binary target), so `tests/mcp_client.rs` locates it via `env!("CARGO_BIN_EXE_mcp_fixture_stdio_server")` exactly like every other e2e test locates the real `beyond-ai-agent` binary. Six tools (`echo`, `add`, `ping`, `fail`, `echo_env`, `image`), each proving one distinct thing the real client code must handle (see the file's own module doc comment). | -| `benches/search.rs` | Criterion macro-bench: `grep` (1 vs auto threads) and `find` (sequential) over a 5,000-file tree | -| `tests/common/mod.rs` | Shared test harness: mock Anthropic-SSE model server, `serve`/`run` command builders, `read_until_response`, gateway binary locator, port/connection helpers — used by every file below | -| `tests/run_*.rs` | `run` binary against a mock model server (no gateway in the loop), split by domain: `run_core_flow` (tool round trips, refusal exit codes, text/json mode, stdin/@file input), `run_skills_prompts` (skill/prompt-template expansion, trust gating), `run_session_management` (`--session`/`--continue`/`--name`/`--fork`, same- and cross-project), `run_cli_flags` (`--version`/`--help`/`list-models`/`export`, flags reaching the wire request, including `models.json` header/api_key overrides, `--idle-timeout-ms`, `--block-images`), `run_stdout_robustness` (Task #10: a closed stdout pipe, simulated by dropping the child's stdout handle mid-stream, must exit 0 rather than panic on `EPIPE`, in both text and `--json` mode) | -| `tests/serve_*.rs` | `serve` binary NDJSON protocol round-trip, split by domain: `serve_session_lifecycle` (startup/resume/crash recovery/jsonl framing/export), `serve_state_reporting` (`get_state`/`get_session_stats`/`cwd_stale`), `serve_session_tree` (branches/forks/`get_tree`), `serve_models_thinking` (model/thinking-level/`--models`), `serve_compaction_retry` (compaction + whole-run auto-retry), `serve_trust_prompts` (trust gating + system prompt), `serve_tools_bash` (tool exclusion + host `bash`), `serve_prompt_flow` (`prompt`/`steer`/`follow_up` queuing, busy semantics, refusal, abort) — each a separate Cargo test binary, ~400-1200 lines apiece rather than one ~6,200-line file | -| `tests/gateway_e2e.rs` | `run` binary → real gateway binary → mock upstream (proves key-swap + the virtual key never reaches upstream) | -| `tests/oauth_e2e.rs`, `tests/serve_oauth_model_switch.rs` | A seeded `~/.claude/auth.json` (no `--key`/`AI_AGENT_KEY`) driving `run`/`serve` against a mock model server: `oauth_e2e` proves a stored Anthropic credential alone is enough to authenticate a `run` turn, carrying its OAuth identity headers, and that no credential at all is a clean error naming `agent login`; `serve_oauth_model_switch` proves `set_model` across two _different_ stored OAuth logins (Anthropic → OpenAI Codex) re-derives the gateway credential/routing for the new provider on the very next turn rather than reusing whichever client was resolved at startup — regression coverage for `gateway_credential::resolve_gateway_credential` being re-run on every model switch, not just once at process start | -| `tests/smoke.rs` | Ignored-by-default live test: real gateway → real Anthropic/OpenAI across both providers (`mise run test:smoke:agent`) — tool/image round trips, cache, thinking-signature replay, auto-compaction, max_steps, concurrent tool calls, abort, follow-up, branch summary, cross-provider model switch, process-restart session resume, live fork, real provider-rejection fail-fast, manual-compact custom instructions | +| File | What It Does | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/main.rs` | CLI entry point (`run`/`serve`/`tools`/`list-models`/`trust`/`untrust`/`clear-trust`/`trust-status`/`settings`/`export`/`login`/`logout`/`auth-status`/`mcp-login`/`mcp-logout` subcommands (the model-provider trio: `crate::auth_store`/`crate::oauth`'s CLI surface, same dispatch shape as `trust`/`settings`; `mcp-login`/`mcp-logout` are the MCP-server-OAuth analogue — see `mcp_auth_store.rs`'s own row below) — `list-models` prints a table (`model`/`context`/`max-out`/`thinking`/`vision` columns) over `serve::available_models()`, no gateway/key needed, each row's capability data pulled straight from `agent_core::capabilities` — pi's own `--list-models` table, minus a `provider` column this crate has no separate field for (a model id is forwarded verbatim; see `agent_core::models`'s own module doc comment); `context`/`max-out` are humanized (`format_token_count`: `"200K"`/`"1M"`, one decimal place only when not a whole unit), not raw integers, matching pi's own `formatTokenCount` (Task #39 pi-parity fix, cosmetic); an optional positional `search` (Task #51 pi-parity fix) fuzzy-filters/ranks rows via `fuzzy_match` — a ported, non-contiguous, order-preserving, word-boundary-scored subsequence match (plus a letters/digits-swapped fallback try for a query typed in the opposite order, e.g. "5sonnet") matching pi's own `fuzzyFilter`/`fuzzyMatch` (`packages/tui/src/fuzzy.ts`), so e.g. "sn5" finds "claude-sonnet-4-5" — previously a plain case-insensitive substring `contains` check, which that query would never match at all; `settings` views/updates `settings::SettingsStore`'s stored defaults, printing every field's current value after any requested update); `expand_short_aliases` rewrites pi's own hand-rolled multi-character short flags (`-np`/`-nc`/`-na`, …) to their long form before clap sees them, including (Task #43 pi-parity fix) a lowercase `-v` alias for `--version` — clap's auto-generated version flag only binds capital `-V`; `DEFAULT_MODEL`, `DEFAULT_GATEWAY`, `default_system_prompt(®istry)` (generated from the actually-registered, filtered tool set); renders streamed text + `[tool: name]` markers (followed live by each `InputJsonDelta` fragment — a | +| growing preview of the call's arguments as they stream in, not just its name) to stdout for `run` in its default text mode, or (`run --json`) one `AgentEvent` object per line via `run_events`/`serde_json::to_string` — the same full observation surface (tool calls/results, turn boundaries) `serve`'s NDJSON protocol streams, preceded by a `{"kind":"session", id, model, cwd}` header line — for a scripting caller that wants structured output without spawning `serve`; `run` composes its first message from piped stdin (trimmed; a whitespace-only pipe is treated as nothing piped at all — Task #37 pi-parity fix, matching pi's own `readPipedStdin`) + `@file` references (`partition_tasks`/`read_file_refs_with_home`, which tilde-expands each ref via `tools::expand_tilde` before joining with `cwd` — Task #20 pi-parity fix — and skips a zero-byte file entirely rather than emitting an empty `` block — Task #38 pi-parity fix, matching pi's own `file-processor.ts`) + the first positional message, runs any further positional messages as separate sequential turns, and — via `--session `/`--continue` (reusing `SessionStore`/`SessionRepo`'s selection methods) — can persist and resume a transcript across invocations exactly like `serve`'s own repo/file modes; `--session ` (Task #24 pi-parity fix, matching pi's own `resolveSessionPath`) accepts either a literal path (path-like — has a `/`, or a leading `./`/`~` — or one that already exists on disk, used as-is, creating a fresh session there when absent exactly as always) _or_ a bare session id/prefix, resolved (`open_session_by_id`) against the current project's own repo first, then cross-project — the identical two-tier search `--fork ` already does via `fork_by_arg` — and reopened **in place**, continuing that session rather than forking a new one (unlike `--fork`, which always copies); previously `arg` was always treated as a literal filesystem path, so a bare id silently created an empty, wrongly-named session file instead of reopening the one actually meant; `--fork `(pi's own cross-project`--fork`, wins over`--session`/`--continue`) instead copies an *existing* session's transcript into a brand-new one under the current project and continues from there — a path opens that`.jsonl`file directly regardless of which project it belongs to, an id is searched in the current project's own repo first and then, by filename rather than by recomputing a path from the source's recorded`cwd`(`session_store::find_session_path_under`— the recompute approach broke under a non-default`--session-dir`, whose directory name need not encode any`cwd`at all), across every project's own directory under the session root, so a session started in one project can be picked up and continued from a different one (`session_store::fork_by_arg`/`fork_from_path`);`--session-dir`/`AI_AGENT_SESSION_DIR`(matching`serve`'s identical flag) redirects that root — both the repo`--continue`reopens and the root`--fork`'s cross-project search spans (that search then scopes to the given directory's own *parent*, the same convention`serve`'s`list_all_sessions`uses) — away from the default`~/.claude/sessions//`;`--session-id `(matching pi's own flag) *addresses* a session — open exactly that id or mint it under exactly that id, idempotently, outranking`--continue`— so a script/orchestrator/test harness gets a known id to route on instead of parsing one back out of the run's own output, and distinct ids in one directory stay distinct sessions;`run --export `renders the finished transcript via`export::export_html`after a live run completes, while the standalone`export [output.html]`subcommand renders an *already-persisted* session file straight off disk — no gateway, key, or model involved at all (`SessionStore::open`+`export::export_html`) | +| `src/lib.rs` | Library root; re-exports `serve`/`tools`/`resources`/`skills`/`prompts`/`session_store`/`trust_store`/`export`/`auth_store`/`auth_credential_source`/`oauth` for tests/benches | +| `src/serve.rs` | NDJSON control protocol: single stdout-writer task, `Persistence` (file/dir/none, default-per-cwd directory), a large command set (session/branch nav, `reload`, model/thinking/tool/auto-compaction tuning, `bash`/`abort_bash`, `export_html`) — see the module's own doc comment for the exhaustive list; prompt runs concurrently with stdin routing `steer`/`follow_up` (also accepted while idle, via a persistent `Steering` handle) | +| `src/export.rs` | `export_html`/`render_html` — renders a session's transcript as one self-contained, dependency-free HTML file (inline CSS, no client-side JS, images inlined as data URIs); every abandoned branch (passed in as `SessionStore::abandoned_branches`'s output) renders inline as a collapsible `
` block right after the message it actually diverged from (`render_branches_diverging_at`, native HTML — no script) — only the divergent suffix, not the shared prefix already shown above, and numbered sequentially across the whole document; every built-in tool call (`edit`/`write`/`bash`/`read`/`grep`/`find`/`ls`, and the Beyond platform tools `fork`/`sync`/`logs`) gets a dedicated renderer (`render_tool_call`) instead of raw pretty-printed JSON — `edit` reuses the diff-coloring machinery (`diff_pair_html`) to show its before/after as a real diff, falling back to generic JSON if `input` doesn't parse as a valid edit shape; only a genuinely unrecognized tool name (a third-party extension) stays generic JSON; message text is rendered as markdown (`render_markdown`, `pulldown-cmark`, server-side at export time — not pi's client-side `marked`/`highlight.js`) — a bare `\n` not preceded by two-plus trailing spaces (CommonMark's own soft break) now renders as a hard `
`, matching pi's own `marked.use({ breaks: true })` (previously the two collapsed into a run-on paragraph with no visible break at all) — with raw HTML defused to visible text and link/image URLs scheme-allow-listed (`sanitize_url` — `http(s):`/`mailto:` plus, as of a pi-parity fix, `tel:`/`ftp:`; everything else, `javascript:` included, is neutralized); a `[Earlier conversation compacted…]`/branch-summary marker line (`parse_summary_marker`/`render_summary_marker`) renders as its own distinct styled block rather than plain markdown, and — for a real compaction specifically — surfaces the token count `compaction::apply_summary` now embeds as a leading `"Compacted from {N} tokens\n\n"` line in the summary body (`parse_compaction_tokens_before` strips and parses it back out, `format_thousands` for display; `None`/absent for a branch summary, which never carries that line) as a `· Compacted from N tokens` note on the block's label; `serve.rs`'s host-bash-command marker (a `[Host bash command, run outside the model's own turn]\n$ …` prefix — commands run out-of-band of the model's own turn — or, Fix 9 pi-parity gap, the `exclude_from_context: true` counterpart `[Host bash command, excluded from model context]\n$ …`, recorded the same way and rendered identically except for a "hidden from the model" note in its title, since hiding a command from the _model_ is a separate concern from hiding it from the exported _transcript_ — see `serve.rs::ServeHooks`) is recognized the same way (`parse_host_bash_marker`/`render_host_bash_marker`) and rendered as its own styled `tool-call host-bash` block instead of falling through to plain markdown, with a distinct `.error` variant; a fenced `` ```diff `` block or diff-shaped tool-result content (`looks_like_diff`) gets per-line +/- coloring (`diff_html`) instead of real syntax highlighting, which is deliberately not implemented (would need a heavy crate like `syntect`, bloating every build of this CLI for a nice-to-have); reuses `skills::xml_escape` for HTML-text escaping; `export_html_full`/`render_html_full` additionally render the system prompt and tool set (each a collapsed-by-default `
` section) plus token-usage totals — pi's own always-included `systemPrompt`/`tools` fields, previously omitted entirely — with `serve`'s `export_html` RPC command and `run --export` (Task #44) both passing the actually-running agent's real system prompt/`agent_core::ToolRegistry::definitions`/session usage; the standalone `export` subcommand still passes `None` for `system_prompt`/`tools` (no live `Agent`/`ToolRegistry` to pull either from — a genuine absence, not an oversight) but, as of Fix 6 (pi-parity gap), no longer for usage — `serve::message_export_usage_totals` sums each message's own `usage` straight out of `sess.messages`, no live `Session` counters needed, so all three export entry points now report the same totals for the same session instead of the standalone path silently omitting the line; shared by `serve`'s `export_html` RPC command, `run --export`, and the standalone `export` subcommand | +| `src/policy.rs` | `ToolPolicy` — the concrete `agent_core::AgentHooks` implementation this crate installs: `--deny-tool`/`--deny-bash-pattern`/`--deny-path` (all three repeatable, comma-separated, `serve` also env-var-backed via `AI_AGENT_DENY_PATH` etc.) build a static deny-list gating `before_tool_call` — `--deny-path` compiles each pattern to a `globset::GlobMatcher` once at construction and matches it against a `write`/`edit` call's canonicalized target path; installed on `Agent` only when non-empty, else the zero-cost `NoHooks` default; shared by `run` and `serve` (`ToolPolicy::from_lists`) | +| `src/trust_store.rs` | Tri-state (`Trust::{Trusted,Untrusted,Unknown}`), ancestor-inheriting trust allowlist (`~/.claude/trusted-projects.json`); `trust`/`distrust` record an explicit grant/denial, `clear` removes a directory's own entry (trusted _or_ untrusted) without recording a new one, reverting it to inheriting its nearest ancestor's decision; legacy bare-array files still parse (trusted-only); allowlist keys are resolved via `path_utils::resolved_path` (absolutize-and-lexically-normalize _before_ a real `canonicalize()` attempt) — matching pi's `resolvePath` → `canonicalizePath` order, so pre-trusting a not-yet-created directory via a relative or trailing-slash path (`agent trust newproj/`) still matches once the directory exists and a later `lookup()` canonicalizes an absolute `cwd`, instead of comparing against the stale literal string `canonicalize()` alone would have fallen back to on its `ENOENT`; `read_store_file` distinguishes a missing file (silent — nothing has ever been trusted yet) from an existing-but-unreadable one or one that fails to parse as either the current or legacy shape (both `warn!`, since either would otherwise silently discard every persisted trust/untrust decision with no signal anything was wrong) | +| `src/auth_store.rs` | `AuthStore` — OAuth/subscription-credential storage (`~/.claude/auth.json`), same shape family as `trust_store.rs`/`settings.rs` (own small `FileLock`, atomic writes, re-read-under-lock-before-mutate, no shared code with either); `refreshed()` is the one genuinely new piece — double-checked-locked refresh that holds the lock across the _entire_ refresh including the network call, preserves the credential bit-for-bit and records `last_refresh_error` (not deletion) on failure; the store file is created at `0600` atomically on first write (`write_atomic` only _preserves_ an existing file's mode, so this is a real, deliberate first-write step, not redundant with it); a local `Secret` type round-trips its real value through `serde` (unlike `agent_core::client::ApiKey`, never serialized, or `beyond-gateway::Secret`, which redacts on serialize) since persisting the real value to disk is this store's whole job; deliberately OAuth-only, no `api_key` credential variant — see "Provider keys" above | +| `src/mcp_auth_store.rs` | `McpAuthStore` — persisted MCP OAuth credentials (`~/.claude/mcp_auth.json`), keyed by MCP server _name_ rather than a fixed provider enum (contrast `auth_store.rs`); same shape family (own small `FileLock`, atomic writes, re-read-under-lock-before-mutate, no shared code with the other three stores). `ScopedMcpCredentialStore` implements `rmcp::transport::auth::CredentialStore` scoped to one server's own slot in the file — what `agent mcp-login`/`tools::mcp::connect_http` actually read and write through; see the "MCP Server OAuth" section above for the full mechanism. | +| `src/auth_credential_source.rs` | `OAuthCredentialSource` — the one `agent_core::client::CredentialSource` implementation this crate ships: an in-memory fast path for a still-valid cached token, else `spawn_blocking`s into `AuthStore::refreshed` (bridging its one async sub-step, the provider's token-endpoint call, back in via `Handle::current().block_on`) — kept off the tokio worker thread the same way `serve.rs`'s `persist_blocking` keeps a blocking session write off it | +| `src/oauth/` | The three providers' OAuth protocol flows — `anthropic.rs` (PKCE + local callback, port `53692`, the `state = verifier` quirk), `github_copilot.rs` (device-code, Copilot-internal-token re-derivation, model-availability discovery, plus `copilot_endpoint_path`/`CopilotRoutedCredentialSource` — the `agent_core::client::CredentialSource` wrapper `gateway_credential::resolve_gateway_credential` builds for a Copilot-routed model; unlike OpenAI Codex's fully-static routing, Copilot's `base_url` is re-derived via `base_url_from_token` fresh on every `credential()` call, from whichever access token is CURRENTLY on disk — re-read directly from the auth store rather than through the wrapped source's own opaque `Credential` — since GitHub can migrate an account to a different Copilot proxy pool mid-session, a real, previously observed occurrence a value frozen at construction would silently misroute after), `openai_codex.rs` (user-selectable browser PKCE / custom device-code, JWT-claim account-id extraction) — plus shared `pkce.rs`/`device_code.rs`/`callback_server.rs`/`callbacks.rs` (the `LoginCallbacks` trait a CLI or `serve` implements) /`error.rs`. Lives in `agent`, not `agent_core`: a second, direct-to-provider network surface `agent_core`'s own doc comment explicitly disclaims owning, and device-code polling/the local callback listener both need a live tokio runtime as a real (non-dev) dependency, which `agent_core` deliberately doesn't have. See "OAuth / Subscription-Credential Authentication" above for the full picture. | +| `src/path_utils.rs` | Shared path-normalization helpers, extracted from `trust_store.rs` once `skills.rs`/`prompts.rs` needed the identical logic: `absolutize`/`lexically_normalize` (pure, no filesystem access beyond reading the cwd — work even for a path that doesn't exist yet) and `resolved_path` (canonicalize on top of that, falling back to the absolutized-and-normalized form on failure); `push_unique_scoped_root` — push a `(discovery root, scope tag)` pair onto a `Vec<(PathBuf, T)>` only if the root's canonical form hasn't already been seen (tracked in a caller-owned `HashSet`), so a root reached by two different paths (a symlink, a relative-vs-absolute spelling, `cwd` itself equaling a global root) is scanned only once — used by `skills.rs`/`prompts.rs` to keep a real directory reached twice from double-counting its contents and self-colliding every name in it against a phantom duplicate, and to carry each root's own `"user"`/`"project"` scope tag (Task #39) alongside it without a second parallel structure; pi doesn't dedupe at the root-list level either (only file-level, in `skills.ts`'s own `realPathSet`) — a Beyond-specific hardening | +| `src/settings.rs` | `SettingsStore` — persisted global-tier `default_model`/`default_gateway_url`/`default_session_dir`/`default_project_trust`/`compaction_enabled`/`default_reasoning_effort`/`block_images`/`image_auto_resize`/`thinking_budget_overrides`/`default_bash_shell_path`/`default_bash_command_prefix`/`default_compaction_reserve_tokens`/`default_compaction_keep_recent_tokens`/`default_retry_max_retries`/`default_retry_base_delay_ms`/`default_provider_timeout_ms`/`default_models_list`/`default_skill_paths`/`default_prompt_template_paths`/`mcp_servers` (`~/.claude/settings.json`; see `tools/mcp.rs`'s own row above for what `mcp_servers`'s `McpServerConfig`/`McpTransport` shape does), consulted by `run_task`/`Command::Serve` as the last fallback before this crate's own built-in default; managed via `agent settings`, mirroring `trust_store.rs`'s own out-of-band management convention and reusing its exact on-disk pattern (own small `FileLock`, atomic writes, re-read-under-lock-before-mutate) without sharing code with it. `Settings::merge_over` + `effective_settings_for_cwd` (Round 3, pi-parity feature) layer a _project_-level `/.claude/settings.json` tier on top of the global one, field-by-field, gated on `trust_store::TrustStore::is_trusted(cwd)` specifically (a persisted grant only — see the "Persisted settings" section above for the full trust-gating rationale and the list-field replace-not-append semantics); `main.rs`'s `run_task`/`Command::Serve` call this instead of the bare `SettingsStore::open_default()` they used before. Also `ModelOverride`/`ModelOverrides` — a read-only, hand-edited `~/.claude/models.json` (model id → `{base_url, api_key, headers}`), consulted by `gateway_credential::resolve_gateway_credential`/`main.rs::model_override_extra_headers`; `api_key`/`headers` values resolve through `resolve_config_value`'s `!command`/`$VAR`/`${VAR}`/literal syntax (pi's own `resolveConfigValue`) | +| `src/gateway_credential.rs` | `resolve_gateway_credential(key, model)` — the one seam that decides how a given model id reaches the gateway (managed/BYO key, a `models.json` `base_url` override, or an inferred OAuth subscription login) and builds the matching `agent_core::client::CredentialSource`. Keyed on `model`, never cached across a model switch: called fresh by `main.rs` for `run` (once, since `run` never switches models mid-process) and by `serve::build_gateway_client` — at `serve` startup _and_ every runtime command that changes the active model (`set_model`/`cycle_model`/`switch_session`/`fork`/`clone`/`switch_branch`) — so a switch that crosses OAuth providers gets that provider's own credential/routing instead of silently reusing whichever one was resolved for the previously active model. `DirectRoutedCredentialSource` (OpenAI Codex — its `chatgpt-account-id`/gateway-prefix routing is genuinely fixed per login) and `StaticDirectCredentialSource` (a `models.json` override's fixed bearer + `base_url`) both live here; GitHub Copilot's own wrapper, `CopilotRoutedCredentialSource` (its proxy host is NOT fixed per login — see `oauth/github_copilot.rs`'s row below), lives with the rest of that provider's logic instead | +| `src/session_store.rs` | JSONL `SessionStore` (fsync'd append/atomic-rewrite/mid-file-corruption recovery, header metadata + durable `Entry::Compaction` provenance, version-migration guard, collision-safe ids; `create` initializes an existing but zero-byte file in place — `touch`'d ahead of time, or left over from a crash before the header write landed — rather than hard-failing, while still refusing (`AlreadyExists`) a genuinely non-empty one; `run`/`serve`'s own `--session ` open-vs-create decision checks file _content_, not just existence, so this path actually gets exercised) + multi-session `SessionRepo` (list-with-metadata, soft-delete-to-`.trash`, fork + read-only fork preview; session selection splits into `open_or_create_id` — address exactly this id, opening it or minting it, never resolving to another session and never by prefix (`find_path_exact`) — and `resume_latest_or_create`, reopen the most recent session matching a `cwd` or make a fresh one, which is now only ever `--continue`; `fork_from_path` forks an arbitrary source session that need not live in `self.dir` at all, stamping the _target_ project's own `cwd` rather than the source's — the primitive `fork_by_arg` (module-level, not a `SessionRepo` method: it may need to open a _second_ repo for the source) builds `run --fork `CLAUDE.md` discovery, skill injection from an already-discovered `PromptOptions::skills` slice — the caller's job, not this function's, since every real caller already discovers skills separately for its own purposes and re-discovering here too would walk the same directories twice — expensive, cached) and `dynamic_footer` (local date/cwd — cheap, refreshed every turn); `build_system_prompt` composes both for a one-shot caller | +| `src/skills.rs` | Recursive skill discovery (`SKILL.md` frontmatter up to `MAX_DEPTH` — 8 — levels deep per root, a Beyond-specific bound pi's own unbounded walk doesn't have, `disable-model-invocation`, `/skill:` lookup — expands into a `` tag with the frontmatter stripped, not the raw file) across `~/.claude/skills`+`/.claude/skills` and the vendor-neutral `~/.agents/skills`+every `.agents/skills` between `cwd` and the enclosing git-repo root (`collect_ancestor_agents_skill_dirs`/`find_git_repo_root` — `.agents/skills` never recognizes the loose-root-`.md`-file shape `.claude/skills` does) + `` rendering + `discover_with_diagnostics` (name-collision reporting; `.claude/skills` wins over `.agents/skills`, and the `.agents/skills` level closest to `cwd` wins over one further up; every discovery root, standard or `--skill` extra, is deduped by canonical path via `path_utils::push_unique_scoped_root` before it's ever walked, so the same real directory reached twice can't double-count its skills into a phantom self-collision); `project_trusted` gates only the project-local roots, the user-global roots are always scanned; `validate_skill_name`/`validate_skill_description` — non-fatal, `warn!`-logged shape/length checks (a bad `name`, or a `description` past 1024 chars) that never block discovery | +| `src/prompts.rs` | `/name args` prompt-template discovery (gitignore-aware, non-recursive single-directory scan — `.gitignore`/`.ignore`/`.fdignore` only, no global excludes/`.git/info/exclude`, matching pi's own `IGNORE_FILE_NAMES`/`readdirSync` scope) + bash-style expansion (quote-aware args, `$N`, `${@:N:L}` slices, `${N:-default}`, `description`/`argument-hint` frontmatter — parsed via `skills::parse_frontmatter`, shared rather than reimplemented, so a YAML block scalar is understood the same way a skill's frontmatter already is) + `discover_with_diagnostics` (name-collision reporting; same canonical-path root dedup as `skills.rs`, via the identical `path_utils::push_unique_scoped_root`) | +| `src/timing.rs` | `StartupTiming` — `AI_AGENT_TIMING=1`-gated startup profiling (pi's own `PI_TIMING=1`/`timings.ts`); `mark(label)`/`print()` are no-ops (don't even read the clock) when unset, so it's safe to sprinkle through `run`/`serve`'s startup path unconditionally; prints to stderr only, never stdout | +| `src/tools/mod.rs` | `default_registry_with(bash_timeout_ms)` — assembles the base 10-tool `ToolRegistry`; `apply_filter(&mut registry, tools, exclude_tools, no_tools)` — allow/deny-list/no-tools filtering applied once at process build time; `default_registry_with_prefix_image_auto_resize_and_mcp_tools` additionally merges in already-connected `tools::mcp` tools (see that module's own row below) after every built-in, so `--tools`/`--exclude-tools` scope MCP-discovered tools too, not just built-ins | +| `src/tools/read.rs` | `read` — line-numbered read with `offset`/`limit` (explicit `limit` also capped at `DEFAULT_LIMIT`, 2000), byte budget, offset-past-EOF error, continuation hints; image files sniffed by magic bytes (`is_valid_bmp` structurally validates a `"BM"`-prefixed file's DIB header rather than trusting the bare 2-byte signature; a SOF55/JPEG-LS 4th byte excludes an undecodable JPEG variant) and downscaled/re-encoded (Lanczos3, PNG-then-JPEG, Exif orientation via `image`'s own generic decoder API — JPEG and WebP both) to fit a 4.5 MB base64 budget; an image already under that budget is still decode-validated (not just magic-byte-sniffed) before being sent as-is — matching pi's `resizeImageInProcess`, which always decodes even on its own already-fits fast path — so a truncated/bit-rotted/polyglot file that merely starts with a real image's magic bytes gets a clear tool error instead of being forwarded to the model unchecked; appends pi's `getNonVisionImageNote` when dispatch's injected `_model_supports_vision` field says the active model can't see images at all | +| `src/tools/write.rs` | `write` — create/overwrite a file, creating parent directories | +| `src/tools/edit.rs` | `edit` — exact-then-fuzzy (NFKC/quote/dash/space/trailing-ws) replacement matched in LF space (CRLF/BOM restored), against the original, overlap/no-op checks, `replace_all` | +| `src/tools/ls.rs` | `ls` — directory listing, directories-first sort, dotfile filtering, `limit` entry cap | +| `src/tools/grep.rs` | `grep` — parallel, gitignore-aware regex (or `literal`) search with `context`/`before`/`after` lines, a whole-output byte cap (`MAX_OUTPUT_BYTES`), deterministic sort+truncate | +| `src/tools/find.rs` | `find` — sequential, gitignore-aware glob search over files **and** dirs; deterministic sort+truncate | +| `src/tools/bash.rs` | `bash` — resolved real-`bash` (falling back to `sh`) execution with a 30-minute default timeout, streaming `OutputAccumulator` (tail-truncated display + full-output temp-file spill), output hygiene (ANSI strip/control sanitize) | +| `src/tools/output.rs` | Shared `OutputAccumulator`/`format_size`/`marker` — the bounded, spill-to-disk output-truncation machinery every truncating tool (`bash`/`read`/`grep`/`ls`/`find`) now shares; `snapshot_text` holds back an incomplete trailing UTF-8 sequence (`incomplete_utf8_suffix_len`) on a live (not-yet-`finish`ed) snapshot instead of transiently lossy-decoding it to `U+FFFD` at a chunk boundary | +| `src/tools/beyond.rs` | `fork`/`sync`/`logs` — shell out to the `beyond` platform CLI | +| `src/exec_endpoint.rs` | `HttpExecRunner` (POST a command to a URL — the whole remote contract, vendor-agnostic, no lifecycle) and `TemplateRunner` (argv template for `ssh`/`docker exec`/`kubectl exec`, `{}` expanding to separate argv entries, never a shell string) | +| `src/tools/fs/mod.rs` | `FsBackend` trait + `PathWorld`, `Meta`/`FileKind`/`DirEntry`, `SearchQuery`/`GlobQuery`, and the impl-independent tail every backend shares: `clip`, `trim_eol`, `finalize`, `finalize_glob` | +| `src/tools/fs/local.rs` | `LocalFs` — the host filesystem: ripgrep's parallel `ignore` walk + `grep-searcher` sink, `std::fs` reads/stats, `write_atomic`, and the FIFO-safe `stat` (the writability access check is gated on kind, because opening a FIFO for write blocks forever) | +| `src/tools/fs/shell.rs` | `ShellFs` — the same operations as commands over any `CommandRunner`: `Capabilities` probe, `rg`/POSIX-`grep` rungs, NUL-scanning output parsers (a filename may contain a newline), base64 for content (`ExecResult::stdout` is a `String`), and the readability guard that stops a failing `dd` being masked by a succeeding `base64` in the pipeline | +| `src/tools/exec.rs` | `CommandRunner` trait + `RealRunner` (stdin closed via `Stdio::null()`, `GroupKillGuard` process-group kill on timeout _or_ a dropped/cancelled future, bounded head+tail streaming capture, `ExecResult.truncated`) — the process-execution seam shared by `bash`/`beyond` tools and `serve`'s own `bash`/`abort_bash` RPC | +| `src/tools/mcp.rs` | MCP (Model Context Protocol) client — this crate's extension mechanism, deliberately MCP rather than pi's in-process `registerTool`/`registerCommand` TypeScript modules: a standardized, language-agnostic protocol with an existing server ecosystem, and no `unsafe`/`libloading` plugin-loading needed (the workspace forbids `unsafe_code`). Built on the official `rmcp` SDK (client-side features only: `client`, `transport-child-process`, `transport-streamable-http-client-reqwest` — this binary never acts as an MCP _server_). `connect_all(&[McpServerConfig]) -> (Vec>, Vec)` connects to every configured server exactly once (stdio via `TokioChildProcess`, streamable-HTTP via `StreamableHttpClientTransport`), lists each one's tools via `tools/list`, and wraps each into an `McpTool` (forwards `run()` straight to `tools/call`, maps `ContentBlock::Text`/`Image` into `ToolOutput.text`/`images`, `is_error: true` into `ToolError::Execution`) — registered as `mcp____` (Claude Code's own convention) so it can never collide with a built-in or another server's tool. A server that fails to connect is skipped with a warning (`connect_all`'s second return value, printed by both `run`/`serve` call sites), never failing the whole agent's startup — matches this crate's "skip and warn, don't silently lose data" convention (`settings::read_store_file`). Connecting happens once, up front (`main.rs::run_task`; `serve.rs` before its own session loop starts, cached on `ServeConfig::mcp_tools` so a `set_model`/`set_thinking` registry rebuild reuses the live connections rather than reconnecting) — both call sites are exercised in tests (`tests/mcp_client.rs`'s `..._through_serve_too` test, `tests/mcp_oauth.rs`'s `..._is_honored_by_serve_too_not_just_run`), not just `run`. There is, deliberately, no way to trigger `agent mcp-login`'s interactive flow from inside a live `serve` session (no RPC command analogous to `login`/`logout`/`auth_status`) — it's a one-shot CLI command run before `serve` starts, and a credential established after a `serve` session is already running won't be picked up until that session restarts. Out of scope for v1: MCP _resources_/_prompts_ (only `tools` is wired up), and per-session dynamic add/remove of a single server (matches the existing whole-registry-rebuild pattern for every other tool-set change). Configured exclusively via `mcp_servers` in `settings.rs`'s `Settings` (global `~/.claude/settings.json`, or a trusted project's own `/.claude/settings.json` — already trust-gated, see `settings.rs`'s row above) — no CLI flag, matching `models.json`'s own hand-edited-only convention; see `McpServerConfig`/`McpTransport` there for the exact JSON shape (`{"name", "transport": "stdio", "command", "args", "env"}` or `{"name", "transport": "http", "url", "headers"}`, each `env`/`headers` value resolved through the same `resolve_config_value` `!command`/`$VAR`/literal syntax `ModelOverride` uses). Tested end to end in `tests/mcp_client.rs` against a real subprocess (`src/bin/mcp_fixture_stdio_server.rs`, below) and a real TCP listener (streamable-HTTP) — never a mock of the MCP protocol itself. | +| `src/bin/mcp_fixture_stdio_server.rs` | Test fixture only, not a real MCP server: a ~150-line hand-rolled MCP server speaking newline-delimited JSON-RPC over stdio, with zero new dependencies (`tokio`/`serde_json`, already ordinary deps of this crate) rather than pulling in `rmcp`'s own server-side machinery just for a test double. Auto-discovered by Cargo as a sibling `[[bin]]` of this same package (any `.rs` file under `src/bin/` becomes its own binary target), so `tests/mcp_client.rs` locates it via `env!("CARGO_BIN_EXE_mcp_fixture_stdio_server")` exactly like every other e2e test locates the real `beyond-ai-agent` binary. Six tools (`echo`, `add`, `ping`, `fail`, `echo_env`, `image`), each proving one distinct thing the real client code must handle (see the file's own module doc comment). | +| `benches/search.rs` | Criterion macro-bench: `grep` (1 vs auto threads) and `find` (sequential) over a 5,000-file tree | +| `tests/common/mod.rs` | Shared test harness: mock Anthropic-SSE model server, `serve`/`run` command builders, `read_until_response`, gateway binary locator, port/connection helpers — used by every file below | +| `tests/run_*.rs` | `run` binary against a mock model server (no gateway in the loop), split by domain: `run_core_flow` (tool round trips, refusal exit codes, text/json mode, stdin/@file input), `run_skills_prompts` (skill/prompt-template expansion, trust gating), `run_session_management` (`--session`/`--continue`/`--name`/`--fork`, same- and cross-project), `run_cli_flags` (`--version`/`--help`/`list-models`/`export`, flags reaching the wire request, including `models.json` header/api_key overrides, `--idle-timeout-ms`, `--block-images`), `run_stdout_robustness` (Task #10: a closed stdout pipe, simulated by dropping the child's stdout handle mid-stream, must exit 0 rather than panic on `EPIPE`, in both text and `--json` mode) | +| `tests/serve_*.rs` | `serve` binary NDJSON protocol round-trip, split by domain: `serve_session_lifecycle` (startup/resume/crash recovery/jsonl framing/export), `serve_state_reporting` (`get_state`/`get_session_stats`/`cwd_stale`), `serve_session_tree` (branches/forks/`get_tree`), `serve_models_thinking` (model/thinking-level/`--models`), `serve_compaction_retry` (compaction + whole-run auto-retry), `serve_trust_prompts` (trust gating + system prompt), `serve_tools_bash` (tool exclusion + host `bash`), `serve_prompt_flow` (`prompt`/`steer`/`follow_up` queuing, busy semantics, refusal, abort) — each a separate Cargo test binary, ~400-1200 lines apiece rather than one ~6,200-line file | +| `tests/gateway_e2e.rs` | `run` binary → real gateway binary → mock upstream (proves key-swap + the virtual key never reaches upstream) | +| `tests/oauth_e2e.rs`, `tests/serve_oauth_model_switch.rs` | A seeded `~/.claude/auth.json` (no `--key`/`AI_AGENT_KEY`) driving `run`/`serve` against a mock model server: `oauth_e2e` proves a stored Anthropic credential alone is enough to authenticate a `run` turn, carrying its OAuth identity headers, and that no credential at all is a clean error naming `agent login`; `serve_oauth_model_switch` proves `set_model` across two _different_ stored OAuth logins (Anthropic → OpenAI Codex) re-derives the gateway credential/routing for the new provider on the very next turn rather than reusing whichever client was resolved at startup — regression coverage for `gateway_credential::resolve_gateway_credential` being re-run on every model switch, not just once at process start | +| `tests/smoke.rs` | Ignored-by-default live test: real gateway → real Anthropic/OpenAI across both providers (`mise run test:smoke:agent`) — tool/image round trips, cache, thinking-signature replay, auto-compaction, max_steps, concurrent tool calls, abort, follow-up, branch summary, cross-provider model switch, process-restart session resume, live fork, real provider-rejection fail-fast, manual-compact custom instructions | --- ## Configuration -| Variable / Flag | Default | What It Controls | -| --------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `--model` / `AI_AGENT_MODEL` | `claude-opus-4-8` | Model id sent in each `ModelRequest`; selects the wire dialect (`agent_core::Dialect::for_model`); `serve`'s `set_model`/`cycle_model` switch it at runtime | -| `--models` / `AI_AGENT_MODELS` (`serve`-only) | none (`cycle_model` steps through the full `available_models()` hint list) | Comma-separated patterns narrowing `cycle_model`'s candidate list — a literal id, a glob (`claude-*`, `*sonnet*`) expanded against `available_models()`, either optionally suffixed `:` to pin that entry's depth on cycle (`resolve_model_scope`); does **not** affect `set_model` or `get_available_models`, which always sees/accepts the full catalog | -| `--gateway-url` / `AI_GATEWAY_URL` | `http://ai.internal` | Base URL `GatewayClient` posts completions to | -| `--key` / `AI_AGENT_KEY` | none (required) | Bearer token sent to the gateway — a `bai_v1…` virtual key, or a BYO provider key forwarded as-is | -| `--max-steps` | none (unbounded) | Opt-in ceiling on loop iterations (`run`) or per-`prompt` iterations (`serve`) before `Error::MaxSteps` (resumable with a fresh call); omitted, the loop runs until the model ends its turn | -| `--max-tokens` / `AI_AGENT_MAX_TOKENS` | none (model's own capability-table `max_output`, floored at `DEFAULT_MAX_TOKENS`) | Per-turn output token ceiling (`Agent::with_max_tokens`); `serve` re-applies it on every `build_agent` rebuild (`set_model`/`set_thinking`/…), so it survives a model switch rather than resetting to that model's own default | -| `--context-window` / `AI_AGENT_CONTEXT_WINDOW` (`run` and `serve`) | none (model's own capability-table `context_window`) | Pins the compaction-trigger threshold (`CompactionConfig::context_window`) to a fixed budget regardless of which model ends up used — useful mainly for forcing/testing compaction on a small, deterministic value. `run` and `serve` share the identical flag/env var | -| `--tools` / `--exclude-tools` / `--no-tools` (+ `AI_AGENT_TOOLS`/`AI_AGENT_EXCLUDE_TOOLS`, `run` and `serve` alike) | none (full default registry) | Restrict/drop from the advertised tool set before the process's `Agent`/system prompt are built; `--no-tools` wins outright | -| `--sequential-tools` (`run` and `serve`) | `false` | Force every batch of tool calls in a turn to run one at a time instead of the default bounded-concurrent dispatch (`agent_core::Agent::with_sequential_tools`) — e.g. a deterministic repro, or a host policy that never wants two tool calls actually overlapping; matches pi's own `AgentOptions.toolExecution: "sequential"`, which pi's own CLI doesn't expose either (library-only there) | -| `--no-skills` / `--no-prompt-templates` (`run` and `serve`) | `false` | Skip _standard-root_ skill/prompt-template discovery outright (`~/.claude/skills`+`/.claude/skills`+`~/.agents/skills`+the `.agents/skills` ancestor walk, `~/.claude/prompts`+`/.claude/prompts`) — no `` listing from either, and a `/skill:name`/`/name` invocation passes through unexpanded unless it resolves against a `--skill`/`--prompt-template` extra path instead, which is still honored even so; matches pi's own flags (a documented, tested combination — `discover_extra_only`). On `serve`, applies on every `reload` too | -| `--skill` / `AI_AGENT_SKILL_PATH`, `--prompt-template` / `AI_AGENT_PROMPT_TEMPLATE_PATH` (`run` and `serve`) | none (only the two standard roots) | Extra discovery root(s) beyond the standard `.claude/skills`/`.claude/prompts` roots — repeatable on the CLI, or comma-separated in the env var, matching `--tools`/`AI_AGENT_TOOLS`'s identical convention (Task #40 pi-parity fix: previously CLI-only, with no env-var fallback unlike most other flags here) | -| `--no-context-files` (`run` and `serve`) | `false` | Skip AGENTS.md/CLAUDE.md project-instruction discovery/injection entirely; matches pi's own flag | -| `--system-prompt` / `AI_AGENT_SYSTEM_PROMPT` (`run` and `serve`) | none (built-in base identity) | Replaces the built-in base system prompt entirely (an on-disk project/user `SYSTEM.md` still overrides it — see `resources::build_static_system_prompt`) | -| `--append-system-prompt` / `AI_AGENT_APPEND_SYSTEM_PROMPT` (`run` and `serve`) | none (falls back to an on-disk `APPEND_SYSTEM.md`, if any) | Extra instructions appended after the base/override; an explicit flag wins outright over the on-disk file rather than combining with it | -| `--trust-project` | `false` | Trust the cwd for this run only (session-scoped), independent of `agent trust `'s persistent allowlist | -| `--force-untrusted` | `false` | Force the cwd _untrusted_ for this run only, overriding both `--trust-project` and a persisted `agent trust ` grant — pi's own `--no-approve`/`-na`; wins over `--trust-project` if both are given | -| `--session-file` / `--session-dir` / `AI_AGENT_SESSION_FILE` (`serve`) | per-cwd directory under `~/.claude/sessions/` | Where the `Session`/`SessionRepo` persists; `--no-session-persistence` opts out to pure in-memory | -| `--session-dir` / `AI_AGENT_SESSION_DIR` (`run`) | per-cwd directory under `~/.claude/sessions/` | `run`-only, same flag/env var and meaning as `serve`'s own `--session-dir` above: redirects the repo root `--continue`/`--fork` use, in place of the default; `--fork`'s cross-project search then scopes to this directory's own parent (matching how `serve`'s `list_all_sessions` scopes off `--session-dir`'s parent too). No effect on `--session ` (an exact file already) or a plain no-flag run (never persisted) | -| `--session-id` (`run` and `serve`) | none (freshly generated) | Pi-parity fix: `serve` previously had no equivalent at all, the one `run`-vs-`serve` gap that ran in the opposite direction from most this audit found. Uses this exact id in place of a freshly generated one wherever a _new_ `SessionMeta` is actually minted — a fresh `--session `/`--session-file`, `--no-session-persistence`, or the default/`--session-dir` repo mode when no session yet matches this `cwd` (`SessionRepo::resume_or_create`'s new `id` parameter) — ignored when reattaching to an existing one (already has a fixed id from disk), including `run --continue`, which always passes `None` regardless of `--session-id` (matches `run`'s own long-documented contract, deliberately left unchanged by this fix). Validated by `is_valid_session_id` before touching any files either way: embedded directly into a persisted filename, so an unsanitized value (`../../../tmp/pwned/evil`) would otherwise write outside the intended sessions directory | -| `--fork ` (`run`-only) | none | Forks an existing session into a brand-new one under the _current_ project and continues from there — pi's own cross-project `--fork`; a path opens that `.jsonl` file directly (any project), an id is searched in the current project first, then every other project's directory under the session root by filename (`session_store::fork_by_arg`/`find_session_path_under`/`fork_from_path`); wins over `--session`/`--continue` | -| `timeout_ms` (per-call, `bash` tool input) | `1_800_000` (30 min) | Wall-clock ceiling for one `bash` invocation — deliberately long; this runs unattended with no one watching a hung shell | -| `--bash-timeout-ms` / `AI_AGENT_BASH_TIMEOUT_MS` | same as above | Overrides `bash`'s own default when the model omits `timeout_ms` | -| `--bash-shell-path` / `AI_AGENT_BASH_SHELL_PATH` | none (auto-resolved: `/bin/bash`, else `bash` on `$PATH`, else `sh`) | Runs `bash` commands through this shell instead — for a non-standard environment (Cygwin, a container without `/bin/bash` at the expected path, a hardened/audited shell wrapper); matches pi's `shellPath` setting. Checked to exist once at CLI-argument time — `serve` fails to start rather than surfacing a confusing spawn error on the first `bash` call | -| `--bash-command-prefix` / `AI_AGENT_BASH_COMMAND_PREFIX` | none | Prepends this line to every `bash` command, in the same shell invocation (e.g. sourcing env setup, activating a venv) — matches pi's own `shellCommandPrefix` setting. Fixed for the process, like `--bash-shell-path`; both `run` and `serve` | -| `RUST_LOG` (`tracing_subscriber::EnvFilter::from_default_env`) | unset (no logs) | Verbosity of `tracing` spans/events emitted by the binary's subscriber | -| `AI_AGENT_TIMING` | unset (no timing output) | `=1` prints a startup-timing breakdown (resource discovery, system-prompt build, session open, agent construction) to stderr just before the first turn/`ready` frame — pi's own `PI_TIMING=1`; every checkpoint is a zero-cost no-op when unset | -| `--idle-timeout-ms` / `AI_AGENT_IDLE_TIMEOUT_MS` (`run`-only) | `agent_core::client::READ_TIMEOUT` (~600s) | Task #19 (pi-parity feature): overrides the gateway HTTP client's idle-read timeout between response chunks (`GatewayClient::with_idle_timeout`, previously unused anywhere in this codebase) — matters most for a direct-routed/custom `models.json` `base_url` override, which bypasses the gateway's own upstream-timeout assumption entirely | -| `--block-images` / `AI_AGENT_BLOCK_IMAGES` (`run`-only), persisted via `agent settings --block-images` | `false` (images allowed) | Task #26 (pi-parity feature): forces `Agent::with_block_images(true)` — every image is downgraded to the same text-placeholder path a vision-incapable model already gets, regardless of the active model's real `supports_vision` capability (bandwidth, compliance, a proxy that strips/rejects multipart image content) | -| `--no-image-auto-resize` / `AI_AGENT_NO_IMAGE_AUTO_RESIZE` (`run`-only), persisted via `agent settings --image-auto-resize` | `false` (resize enabled) | Task #4 (pi-parity feature): skips `read`'s image resize/downscale path entirely (`Read::with_image_auto_resize(false)`) — an oversized image ships its normalized bytes as-is, whatever their size or pixel dimensions, matching pi's `ImageSettings.autoResize`/`processImage` | -| `agent settings --thinking-budget EFFORT=TOKENS` (`run`-only) | none (built-in effort→budget ladder) | Task #36 (pi-parity feature): per-reasoning-effort-level thinking-token-budget override (`agent_core::models::budget_for_effort_with_override`), consulted when `run` derives a thinking budget from `--reasoning-effort` for a `Budget`/`Adaptive`-shape model with no explicit `--thinking` | -| `mcp_servers` (`settings.json` only — deliberately no CLI flag/env var, `run` and `serve` alike) | none (no MCP servers configured) | MCP client support (`tools/mcp.rs`): an array of `{name, transport: "stdio", command, args, env}` or `{name, transport: "http", url, headers}` objects, each contributing its own `tools/list` as `mcp____`-registered tools. Global tier applies unconditionally; project tier is trust-gated (see the Trust Boundaries section above). Connected once at startup — no runtime add/remove, no `agent settings` mutator (hand-edited, like `models.json`) | +| Variable / Flag | Default | What It Controls | +| --------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--model` / `AI_AGENT_MODEL` | `claude-opus-4-8` | Model id sent in each `ModelRequest`; selects the wire dialect (`agent_core::Dialect::for_model`); `serve`'s `set_model`/`cycle_model` switch it at runtime | +| `--models` / `AI_AGENT_MODELS` (`serve`-only) | none (`cycle_model` steps through the full `available_models()` hint list) | Comma-separated patterns narrowing `cycle_model`'s candidate list — a literal id, a glob (`claude-*`, `*sonnet*`) expanded against `available_models()`, either optionally suffixed `:` to pin that entry's depth on cycle (`resolve_model_scope`); does **not** affect `set_model` or `get_available_models`, which always sees/accepts the full catalog | +| `--gateway-url` / `AI_GATEWAY_URL` | `http://ai.internal` | Base URL `GatewayClient` posts completions to | +| `--key` / `AI_AGENT_KEY` | none (required) | Bearer token sent to the gateway — a `bai_v1…` virtual key, or a BYO provider key forwarded as-is | +| `--max-steps` | none (unbounded) | Opt-in ceiling on loop iterations (`run`) or per-`prompt` iterations (`serve`) before `Error::MaxSteps` (resumable with a fresh call); omitted, the loop runs until the model ends its turn | +| `--max-tokens` / `AI_AGENT_MAX_TOKENS` | none (model's own capability-table `max_output`, floored at `DEFAULT_MAX_TOKENS`) | Per-turn output token ceiling (`Agent::with_max_tokens`); `serve` re-applies it on every `build_agent` rebuild (`set_model`/`set_thinking`/…), so it survives a model switch rather than resetting to that model's own default | +| `--context-window` / `AI_AGENT_CONTEXT_WINDOW` (`run` and `serve`) | none (model's own capability-table `context_window`) | Pins the compaction-trigger threshold (`CompactionConfig::context_window`) to a fixed budget regardless of which model ends up used — useful mainly for forcing/testing compaction on a small, deterministic value. `run` and `serve` share the identical flag/env var | +| `--tools` / `--exclude-tools` / `--no-tools` (+ `AI_AGENT_TOOLS`/`AI_AGENT_EXCLUDE_TOOLS`, `run` and `serve` alike) | none (full default registry) | Restrict/drop from the advertised tool set before the process's `Agent`/system prompt are built; `--no-tools` wins outright | +| `--sequential-tools` (`run` and `serve`) | `false` | Force every batch of tool calls in a turn to run one at a time instead of the default bounded-concurrent dispatch (`agent_core::Agent::with_sequential_tools`) — e.g. a deterministic repro, or a host policy that never wants two tool calls actually overlapping; matches pi's own `AgentOptions.toolExecution: "sequential"`, which pi's own CLI doesn't expose either (library-only there) | +| `--no-skills` / `--no-prompt-templates` (`run` and `serve`) | `false` | Skip _standard-root_ skill/prompt-template discovery outright (`~/.claude/skills`+`/.claude/skills`+`~/.agents/skills`+the `.agents/skills` ancestor walk, `~/.claude/prompts`+`/.claude/prompts`) — no `` listing from either, and a `/skill:name`/`/name` invocation passes through unexpanded unless it resolves against a `--skill`/`--prompt-template` extra path instead, which is still honored even so; matches pi's own flags (a documented, tested combination — `discover_extra_only`). On `serve`, applies on every `reload` too | +| `--skill` / `AI_AGENT_SKILL_PATH`, `--prompt-template` / `AI_AGENT_PROMPT_TEMPLATE_PATH` (`run` and `serve`) | none (only the two standard roots) | Extra discovery root(s) beyond the standard `.claude/skills`/`.claude/prompts` roots — repeatable on the CLI, or comma-separated in the env var, matching `--tools`/`AI_AGENT_TOOLS`'s identical convention (Task #40 pi-parity fix: previously CLI-only, with no env-var fallback unlike most other flags here) | +| `--no-context-files` (`run` and `serve`) | `false` | Skip AGENTS.md/CLAUDE.md project-instruction discovery/injection entirely; matches pi's own flag | +| `--system-prompt` / `AI_AGENT_SYSTEM_PROMPT` (`run` and `serve`) | none (built-in base identity) | Replaces the built-in base system prompt entirely (an on-disk project/user `SYSTEM.md` still overrides it — see `resources::build_static_system_prompt`) | +| `--append-system-prompt` / `AI_AGENT_APPEND_SYSTEM_PROMPT` (`run` and `serve`) | none (falls back to an on-disk `APPEND_SYSTEM.md`, if any) | Extra instructions appended after the base/override; an explicit flag wins outright over the on-disk file rather than combining with it | +| `--trust-project` | `false` | Trust the cwd for this run only (session-scoped), independent of `agent trust `'s persistent allowlist | +| `--force-untrusted` | `false` | Force the cwd _untrusted_ for this run only, overriding both `--trust-project` and a persisted `agent trust ` grant — pi's own `--no-approve`/`-na`; wins over `--trust-project` if both are given | +| `--session-file` / `--session-dir` / `AI_AGENT_SESSION_FILE` (`serve`) | per-cwd directory under `~/.claude/sessions/` | Where the `Session`/`SessionRepo` persists; `--no-session-persistence` opts out to pure in-memory | +| `--session-dir` / `AI_AGENT_SESSION_DIR` (`run`) | per-cwd directory under `~/.claude/sessions/` | `run`-only, same flag/env var and meaning as `serve`'s own `--session-dir` above: redirects the repo root `--continue`/`--fork` use, in place of the default; `--fork`'s cross-project search then scopes to this directory's own parent (matching how `serve`'s `list_all_sessions` scopes off `--session-dir`'s parent too). No effect on `--session ` (an exact file already). A plain no-flag run uses it too — it persists, into a session of its own | +| `--session-id` (`run` and `serve`) | none (a fresh session is created) | **Addresses** a session: open exactly this id, or mint it under exactly this id (`SessionRepo::open_or_create_id`). Idempotent — same id, same conversation — which is what makes it the right flag for a supervised `serve` (deterministic across restarts, unlike `--continue`'s "most recent for this cwd") and what makes `serve` multi-tenant (distinct ids are distinct sessions even in one `--session-dir`; the daemon's `?session_id=` routing is exactly this). Outranks `--continue`. It previously applied only where a _new_ `SessionMeta` happened to be minted and was discarded whenever any session already matched the current `cwd`, so every id in a shared directory silently collapsed onto one conversation. Ignored under `--no-session-persistence` (names the in-memory session only) and `--session ` (already an exact file). Validated by `is_valid_session_id` before touching any files: embedded directly into a persisted filename, so an unsanitized value (`../../../tmp/pwned/evil`) would otherwise write outside the intended sessions directory | +| `--continue` / `-c` (`run` and `serve`) | off (each launch starts its own session) | Reattach to the most recent session for this `cwd` (`resume_latest_or_create`), creating one if this is the first launch here. The only implicit reattach there is: a bare launch persists but starts its own session, so two shells (or two servers) in one directory don't drive the same store — `append_new` is count-keyed, so neither process could observe the other's writes and the transcript interleaved. Ignored when `--session`/`--session-id` names a session outright | +| `--fork ` (`run`-only) | none | Forks an existing session into a brand-new one under the _current_ project and continues from there — pi's own cross-project `--fork`; a path opens that `.jsonl` file directly (any project), an id is searched in the current project first, then every other project's directory under the session root by filename (`session_store::fork_by_arg`/`find_session_path_under`/`fork_from_path`); wins over `--session`/`--continue` | +| `timeout_ms` (per-call, `bash` tool input) | `1_800_000` (30 min) | Wall-clock ceiling for one `bash` invocation — deliberately long; this runs unattended with no one watching a hung shell | +| `--bash-timeout-ms` / `AI_AGENT_BASH_TIMEOUT_MS` | same as above | Overrides `bash`'s own default when the model omits `timeout_ms` | +| `--bash-shell-path` / `AI_AGENT_BASH_SHELL_PATH` | none (auto-resolved: `/bin/bash`, else `bash` on `$PATH`, else `sh`) | Runs `bash` commands through this shell instead — for a non-standard environment (Cygwin, a container without `/bin/bash` at the expected path, a hardened/audited shell wrapper); matches pi's `shellPath` setting. Checked to exist once at CLI-argument time — `serve` fails to start rather than surfacing a confusing spawn error on the first `bash` call | +| `--bash-command-prefix` / `AI_AGENT_BASH_COMMAND_PREFIX` | none | Prepends this line to every `bash` command, in the same shell invocation (e.g. sourcing env setup, activating a venv) — matches pi's own `shellCommandPrefix` setting. Fixed for the process, like `--bash-shell-path`; both `run` and `serve` | +| `RUST_LOG` (`tracing_subscriber::EnvFilter::from_default_env`) | unset (no logs) | Verbosity of `tracing` spans/events emitted by the binary's subscriber | +| `AI_AGENT_TIMING` | unset (no timing output) | `=1` prints a startup-timing breakdown (resource discovery, system-prompt build, session open, agent construction) to stderr just before the first turn/`ready` frame — pi's own `PI_TIMING=1`; every checkpoint is a zero-cost no-op when unset | +| `--idle-timeout-ms` / `AI_AGENT_IDLE_TIMEOUT_MS` (`run`-only) | `agent_core::client::READ_TIMEOUT` (~600s) | Task #19 (pi-parity feature): overrides the gateway HTTP client's idle-read timeout between response chunks (`GatewayClient::with_idle_timeout`, previously unused anywhere in this codebase) — matters most for a direct-routed/custom `models.json` `base_url` override, which bypasses the gateway's own upstream-timeout assumption entirely | +| `--block-images` / `AI_AGENT_BLOCK_IMAGES` (`run`-only), persisted via `agent settings --block-images` | `false` (images allowed) | Task #26 (pi-parity feature): forces `Agent::with_block_images(true)` — every image is downgraded to the same text-placeholder path a vision-incapable model already gets, regardless of the active model's real `supports_vision` capability (bandwidth, compliance, a proxy that strips/rejects multipart image content) | +| `--no-image-auto-resize` / `AI_AGENT_NO_IMAGE_AUTO_RESIZE` (`run`-only), persisted via `agent settings --image-auto-resize` | `false` (resize enabled) | Task #4 (pi-parity feature): skips `read`'s image resize/downscale path entirely (`Read::with_image_auto_resize(false)`) — an oversized image ships its normalized bytes as-is, whatever their size or pixel dimensions, matching pi's `ImageSettings.autoResize`/`processImage` | +| `agent settings --thinking-budget EFFORT=TOKENS` (`run`-only) | none (built-in effort→budget ladder) | Task #36 (pi-parity feature): per-reasoning-effort-level thinking-token-budget override (`agent_core::models::budget_for_effort_with_override`), consulted when `run` derives a thinking budget from `--reasoning-effort` for a `Budget`/`Adaptive`-shape model with no explicit `--thinking` | +| `mcp_servers` (`settings.json` only — deliberately no CLI flag/env var, `run` and `serve` alike) | none (no MCP servers configured) | MCP client support (`tools/mcp.rs`): an array of `{name, transport: "stdio", command, args, env}` or `{name, transport: "http", url, headers}` objects, each contributing its own `tools/list` as `mcp____`-registered tools. Global tier applies unconditionally; project tier is trust-gated (see the Trust Boundaries section above). Connected once at startup — no runtime add/remove, no `agent settings` mutator (hand-edited, like `models.json`) | `serve`'s `set_thinking`/`cycle_thinking_level` tune the thinking budget at runtime; `set_auto_compaction`/`set_auto_retry` toggle threshold-triggered compaction / mid-stream retry at diff --git a/crates/agent/src/main.rs b/crates/agent/src/main.rs index a57d92e..8edc9c5 100644 --- a/crates/agent/src/main.rs +++ b/crates/agent/src/main.rs @@ -586,20 +586,23 @@ enum Command { /// over `--continue` if both are given. #[arg(long)] session: Option, - /// Use this exact session id instead of a freshly generated one — a caller (a script, a test - /// harness) that wants a known, predictable id to correlate against rather than parsing it back - /// out of the run's own output. Applies whenever a *new* `SessionMeta` is minted: a fresh - /// `--session ` (one that doesn't already exist) or a plain run with neither `--session` - /// nor `--continue` given (still persisted by default — see `--no-session-persistence`); ignored - /// when reopening an existing `--session ` or resuming via `--continue` (the id is already - /// fixed by whatever's on disk). Matches pi's own `--session-id` flag. + /// Address this exact session: continue it if it already exists, or create it under exactly this + /// id if it doesn't. Gives a caller (a script, an orchestrator, a test harness) a known, + /// predictable name to route on rather than parsing an id back out of the run's own output, and + /// re-running with the same id is idempotent — same conversation, every time. + /// + /// Outranks `--continue`, which only describes a session ("whatever ran here last") where this + /// names one. Distinct ids in one directory are distinct sessions: this used to be discarded + /// whenever *any* session already existed for the current cwd, which silently collapsed every id + /// onto one shared conversation. Ignored with `--no-session-persistence` (nothing is written, so + /// it only names the in-memory session) and with `--session ` (that already names a file). #[arg(long)] session_id: Option, /// Continue the most recent session for the current directory (the same /// `~/.claude/sessions//` repo `serve` defaults to), creating one if this is the - /// first run here. Ignored if `--session` is also given. Kept as an explicit, self-documenting - /// spelling of what a plain no-flag `run` now does by default too (pi-parity fix — see - /// `--no-session-persistence`); harmless to pass either way. + /// first run here. This is the *only* flag that reattaches implicitly — a plain no-flag `run` + /// starts a new session (still persisted; see `--no-session-persistence`). Ignored if + /// `--session`/`--session-id` is also given, both of which name a session outright. #[arg(long, short = 'c', default_value_t = false)] r#continue: bool, /// Use this directory as the session repo instead of the default `~/.claude/sessions/ @@ -614,11 +617,11 @@ enum Command { #[arg(long, env = "AI_AGENT_SESSION_DIR")] session_dir: Option, /// Skip persistence entirely, even without `--session`/`--continue`/`--fork`. Without this, a - /// plain no-flag `run` now defaults to the same per-cwd repo `serve` does + /// plain no-flag `run` writes a new session to the same per-cwd repo `serve` uses /// (`~/.claude/sessions//`, or `--session-dir`) rather than running in-memory-only — /// pass this for the rare case that's genuinely what you want (e.g. a short-lived script that /// mustn't leave a session file behind). Matches `serve`'s identical flag, so the CLI vocabulary - /// for opting out is the same either way. + /// for opting out is the same either way. `--continue` overrides it; `--session-id` does not. #[arg(long, default_value_t = false)] no_session_persistence: bool, /// Persistent-memory backend DSN. Absent ⇒ the stored `default_memory_backend` setting, else a @@ -710,16 +713,25 @@ enum Command { /// `--listen`/`--listen-uds`; ignored on the stdio path. #[arg(long, env = "AI_AGENT_UPSTREAM_HTTP2", value_parser = parse_upstream_http2, default_value = "off")] upstream_http2: serve::UpstreamHttp2, - /// Use this exact session id instead of a freshly generated one — a caller (a script, a test - /// harness) that wants a known, predictable id to correlate against rather than parsing it back - /// out of `get_state`/the startup `{"kind":"session", id, …}` banner. Applies only when a *new* - /// `SessionMeta` is actually minted: a brand-new `--session-file` (one that doesn't already - /// exist), `--no-session-persistence`, or the default/`--session-dir` repo mode when no existing - /// session matches this `cwd` yet; ignored when reattaching to an existing one (already has a - /// fixed id from disk) — matches `run`'s identical flag/contract exactly (`main.rs::Run:: - /// session_id`). + /// Address this exact session: reattach to it if it already exists, or create it under exactly + /// this id if it doesn't. Gives a caller a known, predictable name to route on rather than + /// parsing an id back out of `get_state`/the startup `{"kind":"session", id, …}` banner. + /// + /// This is the right flag for a supervised (systemd, container) `serve`: it's deterministic and + /// idempotent, so a restart lands back on the same conversation, where `--continue`'s "most + /// recent for this cwd" silently depends on whatever else touched the directory meanwhile. It is + /// also what makes `serve` multi-tenant — distinct ids are distinct sessions even in a shared + /// `--session-dir`, which is exactly what the daemon's own `?session_id=` routing relies on. + /// Outranks `--continue`. Matches `run`'s identical flag/contract (`main.rs::Run::session_id`). #[arg(long)] session_id: Option, + /// Reattach to the most recent session for the current directory instead of starting a fresh + /// one, creating one if this is the first `serve` here. The only flag that reattaches + /// implicitly: without it (and without `--session-id`/`--session-file`, both of which name a + /// session outright) each launch starts its own session, so two servers sharing a directory + /// don't silently drive the same on-disk transcript. Matches `run`'s identical flag. + #[arg(long, short = 'c', default_value_t = false)] + r#continue: bool, /// Skip persistence entirely, even without `--session-file`/`--session-dir`. Without this, /// `serve` defaults to `~/.claude/sessions//` rather than silently running /// in-memory-only — pass this for the rare case that's genuinely what you want (e.g. a @@ -1634,6 +1646,7 @@ async fn main() -> Result<(), Box> { session_idle_timeout, upstream_http2, session_id, + r#continue: continue_session, no_session_persistence, memory, no_memory, @@ -1954,6 +1967,7 @@ async fn main() -> Result<(), Box> { // session; the stdio/`run` path leaves it `None` and never pools. shared_http: None, session_id, + continue_session, no_session_persistence, context_window, cache_long, @@ -3771,28 +3785,33 @@ async fn run_task( (Some(store), session) } } - None if continue_session => { - let repo = SessionRepo::open(&repo_dir)?; - // `None` here, not `session_id` — see `resume_or_create`'s doc comment: `--session-id` - // is documented (and tested, above) to apply only to a genuinely fresh `--session ` - // or a plain ephemeral run, never `--continue`. - let (store, session) = repo.resume_or_create(&cwd_str, &model, None)?; - (Some(store), session) - } - // pi-parity fix: previously always `(None, Session::new())` — in-memory only. Matches - // `serve`'s own default (no `--session-file`/`--session-dir` given) exactly: the same - // per-cwd repo, reattaching to this directory's most recent session if one already exists - // rather than starting fresh every single invocation (`SessionRepo::resume_or_create`'s own - // doc comment). `--session-id` *does* apply here (unlike the `--continue` arm above) — this - // is exactly the "plain ephemeral run" case that flag's own doc comment already documents it - // for. - None if !no_session_persistence => { + // Pure in-memory. `--continue` still overrides `--no-session-persistence` (it always has: + // asking to continue a persisted session is a direct contradiction of not persisting, and + // the explicit verb wins), but `--session-id` does not — there it just names the ephemeral + // session for correlation, exactly as it does in `serve`'s own no-persistence branch. + None if no_session_persistence && !continue_session => (None, Session::new()), + // The selection ladder, most specific first — the same one `serve` applies via + // `serve::SessionSelect`, so the flags mean identically the same thing in both binaries. + None => { let repo = SessionRepo::open(&repo_dir)?; - let (store, session) = - repo.resume_or_create(&cwd_str, &model, session_id.as_deref())?; + let (store, session) = match (session_id.as_deref(), continue_session) { + // `--session-id` addresses one session outright: open it, or create it under + // exactly that id. It outranks `--continue`, which only *describes* a session + // ("whatever ran here last"). It used to be discarded outright whenever any session + // already existed for this cwd, which collapsed every distinct id in a shared + // directory onto one shared conversation. + (Some(id), _) => repo.open_or_create_id(id, &cwd_str, &model)?, + (None, true) => repo.resume_latest_or_create(&cwd_str, &model)?, + // A bare `run` starts a genuinely new session — persisted, so nothing is lost, but + // its own. It briefly reattached to this cwd's most recent session instead, which + // meant two shells in one repo drove the same store: `append_new` is count-keyed, + // so neither could observe the other's writes and the transcript interleaved into + // nonsense. `--continue` is how you ask for the old behavior, and it is now the + // only thing that reattaches implicitly. + (None, false) => (repo.create(fresh_meta())?, Session::new()), + }; (Some(store), session) } - None => (None, Session::new()), } }; diff --git a/crates/agent/src/serve.rs b/crates/agent/src/serve.rs index c5810bc..7266c12 100644 --- a/crates/agent/src/serve.rs +++ b/crates/agent/src/serve.rs @@ -80,7 +80,9 @@ //! - `{type:"new_session", parent_session?}` start a fresh session → `data: {session_id, parent}` //! (repo mode: `parent` is `parent_session` when given, else whatever session id was active //! immediately before this call — pi's own `parentSession` lineage marker, provenance only, not a -//! fork; `null` in single-file/in-memory mode, where there's no *new* id to link) +//! fork; `null` in single-file/in-memory mode, where there's no *new* id to link). An *addressed* +//! session — `--session-id`, the daemon's `?session_id=` — keeps its own id instead of minting one, +//! archiving the outgoing conversation into a sibling session; see [`Persistence::new_session`] //! - `{type:"list_sessions", query?}` (repo mode) → `data: {sessions: [SessionMeta + updated_at/ //! message_count/preview/search_text…]}` (via `SessionMeta::to_listing_json` — those four fields are //! `#[serde(skip)]` on the struct itself), this project's sessions only (matched by the default @@ -499,13 +501,20 @@ pub struct ServeConfig { /// Disable only the per-session `/session` working-memory mount (`--no-session-memory`), keeping the /// durable `/memories` store. On by default whenever memory is enabled. See [`crate::memory`]. pub no_session_memory: bool, - /// Use this exact session id instead of a freshly generated one, wherever a *new* `SessionMeta` is - /// actually minted by [`Persistence::open`] — already-validated by `main.rs` (embedded directly into - /// a persisted filename, so it must be sanitized before it ever reaches here). Matches `run`'s - /// identical `--session-id` flag/contract: ignored when reattaching to an existing session (already - /// has a fixed id from disk), whether that's an existing `session_file` or a repo-mode match on the - /// current `cwd`. + /// **Address** this exact session: open it if it already exists, else create it under exactly this + /// id. Already-validated by `main.rs` (embedded directly into a persisted filename, so it must be + /// sanitized before it ever reaches here). Matches `run`'s identical `--session-id` flag/contract. + /// + /// This is the most specific selector there is, so it wins over [`Self::continue_session`] and over + /// the default — see [`SessionSelect`]. It never resolves to a *different* session, which is what + /// makes it usable as a routing key for a multi-tenant `serve` (and is exactly what the daemon's + /// `?session_id=` relies on). Repeating the same id is idempotent: same session, every time. pub session_id: Option, + /// Reattach to this `cwd`'s most recent session instead of starting a fresh one (`--continue`). + /// Without it, `serve` starts a new session on every launch — two servers sharing a directory would + /// otherwise silently drive the same on-disk transcript. Ignored when [`Self::session_id`] is set + /// (that names one session outright) and in `session_file` mode (the path already names one). + pub continue_session: bool, /// Skip persistence entirely, even though neither `session_file` nor `session_dir` was set — /// without this, `Persistence::open` defaults to a per-cwd directory under /// `~/.claude/sessions//` rather than silently running in-memory-only (an operator @@ -989,6 +998,34 @@ fn fresh_meta(cwd: &str, model: &str, session_id: Option<&str>) -> SessionMeta { } } +/// How [`Persistence`] picks which session to open in repo mode. Spelled out as a choice rather than +/// inferred from a pile of `Option`s at the point of use, because the precedence between these is the +/// whole contract: an explicit id is the most specific selector and always wins, `--continue` is the +/// only thing that reattaches, and everything else starts clean. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SessionSelect<'a> { + /// Address one specific session: open it, or create it under exactly this id + /// ([`SessionRepo::open_or_create_id`]). `--session-id`, and the daemon's `?session_id=`. + Id(&'a str), + /// `--continue`: this cwd's most recent session, else a fresh one. + Latest, + /// The default: always a brand-new session. + Fresh, +} + +impl<'a> SessionSelect<'a> { + /// Resolve the selector from config. An explicit id beats `--continue` because it names exactly one + /// session while `--continue` only describes one, and a caller who supplied both has already been + /// more specific than "whatever ran here last". + fn from_cfg(cfg: &'a ServeConfig) -> Self { + match (&cfg.session_id, cfg.continue_session) { + (Some(id), _) => Self::Id(id), + (None, true) => Self::Latest, + (None, false) => Self::Fresh, + } + } +} + /// Where the server persists sessions: a multi-session [`SessionRepo`] (`--session-dir`), a single /// JSONL file (`--session-file`), or nowhere (in-memory). It always carries the current session's /// [`SessionMeta`] so the session id is stable across reattaches. @@ -996,17 +1033,24 @@ struct Persistence { repo: Option, store: Option, meta: SessionMeta, + /// Set when the caller *named* this session ([`ServeConfig::session_id`]), which makes the id a + /// routing key rather than an incidental label — the daemon reconnects `?session_id=` to it, and + /// a supervised `serve --session-id` expects the same id back after a restart. [`Self::new_session`] + /// honors it by blanking the session in place instead of minting a new id, so the address a client + /// holds never goes stale underneath it. `None` leaves `new_session` free to mint (repo mode's + /// natural behavior). + pinned_id: Option, } impl Persistence { - /// Open persistence and restore (or create) the active session. In repo mode, reopens the most - /// recent session or creates a fresh one; in file mode, opens the file or creates it. + /// Open persistence and select the active session: in repo mode per [`SessionSelect`], in file mode + /// by opening the named file (or creating it). fn open(cfg: &ServeConfig) -> std::io::Result<(Self, Session)> { let cwd = crate::session_store::canonical_cwd(&std::env::current_dir().unwrap_or_default()) .to_string_lossy() .into_owned(); if let Some(dir) = &cfg.session_dir { - return Self::open_repo(dir, &cwd, &cfg.model, cfg.session_id.as_deref()); + return Self::open_repo(dir, &cwd, &cfg.model, SessionSelect::from_cfg(cfg)); } if let Some(path) = &cfg.session_file { let path = std::path::PathBuf::from(path); @@ -1027,6 +1071,10 @@ impl Persistence { repo: None, store: Some(store), meta, + // Recorded for consistency, but single-file mode never consults it: the operator + // named the *file*, and `new_session`'s file-mode arm already resets in place, so + // the id can't move regardless of whether one was pinned. + pinned_id: cfg.session_id.clone(), }, session, )); @@ -1037,6 +1085,7 @@ impl Persistence { repo: None, store: None, meta: fresh_meta(&cwd, &cfg.model, cfg.session_id.as_deref()), + pinned_id: cfg.session_id.clone(), }, Session::new(), )); @@ -1048,28 +1097,40 @@ impl Persistence { crate::session_store::default_session_dir(&cwd), &cwd, &cfg.model, - cfg.session_id.as_deref(), + SessionSelect::from_cfg(cfg), ) } - /// Open (creating if needed) a multi-session repo at `dir` and reattach to the most recent session - /// whose recorded cwd matches `cwd` — not just the globally newest one, so a shared `--session-dir` - /// spanning multiple projects (or the shared default directory before cwd-encoding existed) doesn't - /// resume a stranger's unrelated session. No match (a fresh directory, or one with no session for - /// this cwd yet) creates a new one, using `session_id` in place of a freshly generated one when - /// given (see `ServeConfig::session_id`'s doc comment). + /// Open (creating if needed) a multi-session repo at `dir` and pick a session out of it per + /// `select` — see [`SessionSelect`] for the precedence, and the two `SessionRepo` methods it + /// dispatches to for what each one guarantees. + /// + /// Note that [`SessionSelect::Fresh`] (the default) genuinely creates a new session on every start. + /// A supervised `serve` that wants to pick its conversation back up across restarts should pin + /// `--session-id`: that's deterministic and idempotent, whereas `--continue`'s "most recent for this + /// cwd" silently depends on what else has touched the directory since. fn open_repo( dir: impl Into, cwd: &str, model: &str, - session_id: Option<&str>, + select: SessionSelect<'_>, ) -> std::io::Result<(Self, Session)> { let repo = SessionRepo::open(dir)?; - let (store, session) = repo.resume_or_create(cwd, model, session_id)?; + let (store, session) = match select { + SessionSelect::Id(id) => repo.open_or_create_id(id, cwd, model)?, + SessionSelect::Latest => repo.resume_latest_or_create(cwd, model)?, + SessionSelect::Fresh => (repo.create(SessionMeta::new(cwd, model))?, Session::new()), + }; let meta = store.meta().clone(); Ok(( Self { repo: Some(repo), + // Only an explicitly addressed session is pinned. `Latest`/`Fresh` produce an id the + // caller never asked for, so nothing is routing on it and `new_session` may mint freely. + pinned_id: match select { + SessionSelect::Id(id) => Some(id.to_string()), + SessionSelect::Latest | SessionSelect::Fresh => None, + }, store: Some(store), meta, }, @@ -1142,6 +1203,13 @@ impl Persistence { /// Start a fresh session. In repo mode this creates a new file (new id); in single-file mode it /// resets the existing file (keeping its id); in-memory it just mints new metadata. /// + /// **Pinned sessions are the exception** ([`Self::pinned_id`]): when the caller named this session, + /// that name is an address something is routing on, so `new_session` keeps it and blanks the session + /// in place. The outgoing conversation is *snapshotted into its own session first* — a real, listable + /// sibling whose `parent` points back here — so "start a new session" never doubles as "destroy the + /// old one". That archive-then-blank order matters: the copy is taken from what's on disk, so a + /// failure to archive aborts before anything is cleared. + /// /// In repo mode, the fresh session's `parent` records whatever session id was active immediately /// before this call, unless `parent_session` explicitly names a different one — pi's own /// `parentSession` lineage marker on a `/new`-equivalent reset (pi's own default, absent an @@ -1168,6 +1236,9 @@ impl Persistence { ) -> std::io::Result { let cwd = Self::cwd(); if let Some(repo) = &self.repo { + if let Some(pinned) = self.pinned_id.clone() { + return self.blank_pinned_session(&pinned); + } let mut meta = SessionMeta::new(&cwd, model); meta.parent = Some(parent_session.unwrap_or(&self.meta.id).to_string()); match repo.create(meta) { @@ -1191,6 +1262,41 @@ impl Persistence { Ok(Session::new()) } + /// [`Self::new_session`] for a pinned (caller-addressed) session: archive the current conversation + /// into a session of its own, then clear this one in place, keeping the id. + /// + /// The archive is a plain [`SessionRepo::fork`] of the whole active transcript, so it lands as an + /// ordinary session — it appears in `list_sessions`, can be `switch_session`ed into, and records + /// `parent = `, leaving the lineage back to this slot legible. An empty session has + /// nothing worth archiving, so that's skipped and blanking an already-blank slot stays cheap. + fn blank_pinned_session(&mut self, pinned: &str) -> std::io::Result { + let Self { + repo: Some(repo), + store: Some(store), + .. + } = self + else { + // Not repo mode with a live store: single-file and in-memory both keep their id anyway, so + // the caller's own arms already do the right thing. + return Ok(Session::new()); + }; + if !store.active_ids().is_empty() { + // Copied from what's on disk, *before* anything is cleared — a failure here leaves the + // session exactly as it was rather than half-reset with its history already gone. + if let Err(e) = repo.fork(pinned, usize::MAX) { + eprintln!("serve: failed to archive session {pinned} before reset: {e}"); + return Err(e); + } + } + if let Err(e) = store.reset_for_new_session() { + eprintln!("serve: failed to reset session: {e}"); + return Err(e); + } + let meta = store.meta().clone(); + self.meta = meta; + Ok(Session::new()) + } + /// Switch to another session by id (repo mode only). fn switch(&mut self, id: &str) -> std::io::Result { let repo = self.repo.as_ref().ok_or_else(not_in_repo_mode)?; @@ -8837,7 +8943,8 @@ mod tests { // full history rather than zero/current-process-only. let dir = tempfile::tempdir().unwrap(); let (mut persistence, mut session) = - Persistence::open_repo(dir.path(), "/w", "claude-opus-4-8", None).unwrap(); + Persistence::open_repo(dir.path(), "/w", "claude-opus-4-8", SessionSelect::Latest) + .unwrap(); session.user("go"); session.push( agent_core::Message::assistant(vec![agent_core::ContentBlock::text("ok")]) @@ -8856,7 +8963,8 @@ mod tests { // in-memory counters the original process accumulated (matching a real process restart exactly, // since those counters never persist regardless). let (_restarted, reloaded) = - Persistence::open_repo(dir.path(), "/w", "claude-opus-4-8", None).unwrap(); + Persistence::open_repo(dir.path(), "/w", "claude-opus-4-8", SessionSelect::Latest) + .unwrap(); assert_eq!( reloaded.input_tokens, 0, "sanity: the running counter itself stayed at zero" @@ -9335,6 +9443,7 @@ mod tests { // difference, not a gap to close), so the contract this test protects is simply "fails // cleanly, doesn't crash." let mut persistence = Persistence { + pinned_id: None, repo: None, store: None, meta: SessionMeta::new("/w", "claude-test"), @@ -9369,7 +9478,7 @@ mod tests { // navigation). let dir = tempfile::tempdir().unwrap(); let (mut persistence, _session) = - Persistence::open_repo(dir.path(), "/w", "claude-test", None).unwrap(); + Persistence::open_repo(dir.path(), "/w", "claude-test", SessionSelect::Latest).unwrap(); let ids = { let store = persistence.store.as_mut().unwrap(); let mut session = Session::new(); @@ -9417,7 +9526,7 @@ mod tests { // fully replace the default structured template with its own instructions had no way to do so. let dir = tempfile::tempdir().unwrap(); let (mut persistence, _session) = - Persistence::open_repo(dir.path(), "/w", "claude-test", None).unwrap(); + Persistence::open_repo(dir.path(), "/w", "claude-test", SessionSelect::Latest).unwrap(); let ids = { let store = persistence.store.as_mut().unwrap(); let mut session = Session::new(); @@ -9481,6 +9590,7 @@ mod tests { // `get_label` are the RPC handlers' entry point. Same "no tree, no label" contract as // `switch_branch` above. let mut persistence = Persistence { + pinned_id: None, repo: None, store: None, meta: SessionMeta::new("/w", "claude-test"), @@ -9507,6 +9617,7 @@ mod tests { // same as `delete`'s own repo-mode requirement — neither single-file nor in-memory-only mode has // a repo directory to consult. let persistence = Persistence { + pinned_id: None, repo: None, store: None, meta: SessionMeta::new("/w", "claude-test"), @@ -9527,7 +9638,7 @@ mod tests { fn list_trash_and_restore_session_round_trip_through_persistence() { let dir = tempfile::tempdir().unwrap(); let (persistence, _session) = - Persistence::open_repo(dir.path(), "/w", "claude-test", None).unwrap(); + Persistence::open_repo(dir.path(), "/w", "claude-test", SessionSelect::Latest).unwrap(); let repo = persistence.repo.as_ref().unwrap(); let other = repo.create(SessionMeta::new("/w", "claude-test")).unwrap(); let other_id = other.meta().id.clone(); @@ -9549,6 +9660,7 @@ mod tests { // surface at all — `Persistence::append_custom` is the RPC handler's entry point. Same // "no tree, nothing to append to" contract as `set_label`/`get_label` above. let mut persistence = Persistence { + pinned_id: None, repo: None, store: None, meta: SessionMeta::new("/w", "claude-test"), @@ -9570,6 +9682,7 @@ mod tests { // `repo` to fork within either (`--session-file`/`--no-session-persistence` both leave `repo` // `None`; only `--session-dir` sets it). let mut persistence = Persistence { + pinned_id: None, repo: None, store: None, meta: SessionMeta::new("/w", "claude-test"), @@ -9592,7 +9705,7 @@ mod tests { // before the fork swaps `self.store`. let dir = tempfile::tempdir().unwrap(); let (mut persistence, _session) = - Persistence::open_repo(dir.path(), "/w", "claude-a", None).unwrap(); + Persistence::open_repo(dir.path(), "/w", "claude-a", SessionSelect::Latest).unwrap(); let ids = { let store = persistence.store.as_mut().unwrap(); let mut session = Session::new(); @@ -9638,7 +9751,7 @@ mod tests { // path currently ends. let dir = tempfile::tempdir().unwrap(); let (mut persistence, _session) = - Persistence::open_repo(dir.path(), "/w", "claude-a", None).unwrap(); + Persistence::open_repo(dir.path(), "/w", "claude-a", SessionSelect::Latest).unwrap(); let ids = { let store = persistence.store.as_mut().unwrap(); let mut session = Session::new(); @@ -9738,7 +9851,8 @@ mod tests { // itself end to end. let dir = tempfile::tempdir().unwrap(); let (mut persistence, _session) = - Persistence::open_repo(dir.path(), "/w", "claude-original", None).unwrap(); + Persistence::open_repo(dir.path(), "/w", "claude-original", SessionSelect::Latest) + .unwrap(); let mut session = Session::new(); { let store = persistence.store.as_mut().unwrap(); @@ -9766,10 +9880,16 @@ mod tests { drop(persistence); // "Restart": a fresh `Persistence::open_repo` against the same directory/cwd, exactly like a - // brand-new `serve` process's `Persistence::open` would do — reattaching to the most recent - // session for this cwd rather than creating a new one. - let (restarted, _session) = - Persistence::open_repo(dir.path(), "/w", "claude-cli-default", None).unwrap(); + // brand-new `serve --continue` process's `Persistence::open` would do — reattaching to the most + // recent session for this cwd rather than creating a new one. (A bare `serve` now selects + // `Fresh` instead; reattach is what this test is about, so it asks for it explicitly.) + let (restarted, _session) = Persistence::open_repo( + dir.path(), + "/w", + "claude-cli-default", + SessionSelect::Latest, + ) + .unwrap(); let cfg_level = agent_core::ThinkingLevel::Off; let (session_model, session_level) = restarted.model_and_level_at_active(cfg_level); diff --git a/crates/agent/src/serve_ws.rs b/crates/agent/src/serve_ws.rs index 5883562..2b4734e 100644 --- a/crates/agent/src/serve_ws.rs +++ b/crates/agent/src/serve_ws.rs @@ -19,12 +19,22 @@ //! //! A connection names its session in the URL: `…/_beyond/agent?session_id=` (absent ⇒ a fresh id //! is minted; the client learns it from any `response`/`get_state` frame). That id is both the -//! supervisor's routing key **and** the persisted session id: each WS session is its own JSONL file -//! `/.jsonl` (via [`ServeConfig::session_id`]/`session_file`), so the id is stable -//! across reconnects and a cold reconnect after a process restart re-opens the same file. Without a -//! `--session-dir`, sessions are in-memory only (live re-attach still works for the process's -//! lifetime). Repo-mode multi-session commands (`list_sessions`, `switch_session`) are not used — each -//! connection is pinned to one session by its URL. +//! supervisor's routing key **and** the persisted session id: it's handed to the session as +//! [`ServeConfig::session_id`], which *addresses* it in the repo — open that session, or create it under +//! exactly that id ([`crate::session_store::SessionRepo::open_or_create_id`]). So the id is stable +//! across reconnects, and a cold reconnect after a full process restart reopens the same conversation +//! rather than a blank one. `--no-session-persistence` opts out into in-memory-only sessions, which +//! still live re-attach for the process's lifetime. +//! +//! Because the id is a routing key, `new_session` on a live connection **keeps** it: the conversation is +//! archived into a session of its own and this one is blanked in place, so the address a client holds +//! never goes stale (see [`crate::serve::Persistence::new_session`]). Creating a genuinely new session +//! is a routing operation — connect with a new `?session_id=`. +//! +//! One limit worth naming: `switch_session` (and `fork`/`clone`) move *this process's* view to another +//! session while the routing key stays put. That's fine while the session is live, but it isn't durable +//! — if the session is reaped and later respawned, the key re-opens the session it was named for. For a +//! durable move, reconnect at `?session_id=` instead. //! //! ## Auth //! @@ -142,9 +152,16 @@ struct Supervisor { } impl Supervisor { - /// Derive a per-session config: pin the id, drop `listen`, and give the session its own JSONL file - /// under the base `--session-dir` so its persisted id equals its routing key. No base dir ⇒ - /// in-memory only (live re-attach still works while the task lives). + /// Derive a per-session config: address the session by its routing key and drop `listen`. + /// + /// Pinning `session_id` is the whole mechanism — repo mode opens exactly that session or creates it + /// under exactly that id, so the persisted id always equals the routing key. This used to rewrite + /// each session into single-file mode at `/.jsonl` instead, purely to dodge repo + /// mode's old behavior of resolving by `cwd` and collapsing every session in a directory onto one. + /// With an id now taking precedence over the cwd match that workaround is unnecessary, and dropping + /// it fixes what it cost: daemon files were named `.jsonl` where the repo names its own + /// `_.jsonl`, so `find_path`'s `_.jsonl` lookup couldn't see them — a daemon + /// session appeared in `list_sessions` but `switch_session` reported it missing. fn session_cfg(&self, id: &str) -> ServeConfig { let mut c = self.cfg.clone(); c.listen = None; @@ -153,21 +170,13 @@ impl Supervisor { c.listen_uds = None; c.listen_uds_mode = None; c.session_id = Some(id.to_string()); - match &self.cfg.session_dir { - Some(dir) => { - c.session_file = Some( - std::path::Path::new(dir) - .join(format!("{id}.jsonl")) - .to_string_lossy() - .into_owned(), - ); - c.session_dir = None; - } - None => { - c.no_session_persistence = true; - c.session_file = None; - } - } + // An addressed session selects itself; `--continue`'s "most recent for this cwd" would only be + // able to disagree with the id the client actually routed on. + c.continue_session = false; + // Repo mode, always: one file can't hold the many sessions a daemon serves, so a `--session-file` + // meant for the stdio path can't carry over. `session_dir` (or, unset, the default per-cwd repo) + // is where they go; `--no-session-persistence` is still honored and keeps them in memory. + c.session_file = None; c } diff --git a/crates/agent/src/session_store.rs b/crates/agent/src/session_store.rs index 8e98305..4fdc4b3 100644 --- a/crates/agent/src/session_store.rs +++ b/crates/agent/src/session_store.rs @@ -2721,33 +2721,54 @@ impl SessionRepo { &self.dir } - /// Reopen the most recent session whose recorded `cwd` matches, or create a fresh one — not just - /// the globally newest session, so a shared repo directory spanning multiple projects doesn't - /// resume a stranger's unrelated session. Matching is an exact string comparison — callers are - /// expected to have already passed `cwd` through [`canonical_cwd`], so a symlinked or - /// trailing-slashed spelling of the same real directory still matches (`serve`'s own startup - /// reattach and `run --continue` both do). Shared by both, so they pick up "my last session in - /// this directory" the same way. `id`, when given, names the fresh session in the no-match branch - /// instead of a freshly generated one — a caller-chosen id (`serve`'s own `--session-id`); ignored - /// when an existing session is reattached instead (already has a fixed id from disk). `run - /// --continue` always passes `None` here, matching its own documented contract that `--session-id` - /// applies only to a genuinely fresh `--session ` or a plain ephemeral run, never `--continue`. - pub fn resume_or_create( + /// Open the session named `id`, or create it under exactly that id if it doesn't exist yet. + /// + /// **A session id is an address.** This resolves to the session named `id` or to a brand-new one + /// carrying that name — never to some *other* session, and never to a unique-prefix neighbour (see + /// [`Self::find_path_exact`]). That total, unambiguous mapping is what makes an id usable as a + /// routing key: a caller minting one id per tenant, per task, or per connection gets one session + /// apiece, and re-running with the same id is idempotent, landing back on the same conversation. + /// + /// Deliberately does **not** consult `cwd`. It's recorded on a freshly created session (and is what + /// [`Self::resume_latest_or_create`] matches on), but it never overrides an explicit name: an + /// earlier version of this API checked `cwd` first and silently discarded the caller's id, which + /// collapsed every distinct id in a shared directory onto one shared session. + pub fn open_or_create_id( + &self, + id: &str, + cwd: &str, + model: &str, + ) -> std::io::Result<(SessionStore, Session)> { + match self.find_path_exact(id)? { + Some(path) => SessionStore::open(path), + None => Ok(( + self.create(SessionMeta::with_id(id.to_string(), cwd, model))?, + Session::new(), + )), + } + } + + /// Reopen the most recent session whose recorded `cwd` matches, or create a fresh one — "my last + /// session in this directory", which is exactly what `--continue` means on both `run` and `serve`. + /// + /// Not just the globally newest session, so a shared repo directory spanning multiple projects + /// doesn't resume a stranger's unrelated session. Matching is an exact string comparison — callers + /// are expected to have already passed `cwd` through [`canonical_cwd`], so a symlinked or + /// trailing-slashed spelling of the same real directory still matches. + /// + /// Only ever reached when the caller explicitly asked to continue. It is deliberately **not** what a + /// bare invocation does: two shells running in one repo would both land on this same store and + /// interleave their transcripts into it, and since [`SessionStore::append_new`] is count-keyed + /// neither process can even observe the other's writes. Callers wanting a brand-new session call + /// [`Self::create`]; callers wanting a *named* one call [`Self::open_or_create_id`]. + pub fn resume_latest_or_create( &self, cwd: &str, model: &str, - id: Option<&str>, ) -> std::io::Result<(SessionStore, Session)> { match self.list()?.into_iter().find(|m| m.cwd == cwd) { Some(meta) => self.open_id(&meta.id), - None => { - let meta = match id { - Some(id) => SessionMeta::with_id(id.to_string(), cwd, model), - None => SessionMeta::new(cwd, model), - }; - let store = self.create(meta)?; - Ok((store, Session::new())) - } + None => Ok((self.create(SessionMeta::new(cwd, model))?, Session::new())), } } @@ -3291,6 +3312,40 @@ impl SessionRepo { Ok(self.fork_at_entry_prefix(id, entry_id, before)?.messages) } + /// Every `.jsonl`-or-otherwise path directly under this repo's directory, unfiltered. A missing + /// directory is an empty list, not an error — nothing has been persisted here yet, which every + /// lookup below already treats as "no match" rather than a failure. + fn session_paths(&self) -> std::io::Result> { + match fs::read_dir(&self.dir) { + Ok(entries) => Ok(entries.flatten().map(|e| e.path()).collect()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(Vec::new()), + Err(e) => Err(e), + } + } + + /// The path whose `` component is exactly `id`, from an already-gathered listing. Filename-only + /// — no file is opened, so this is a string compare per directory entry rather than a parse. + /// + /// Compares the parsed component, not an `_.jsonl` *suffix*: ids may legally contain `_` + /// ([`is_valid_session_id`]), so a suffix test lets a lookup for `b` match a session actually named + /// `a_b`. Harmless while this only backed a convenience lookup; not harmless now that it decides + /// which session an address resolves to. + fn exact_match(entries: &[PathBuf], id: &str) -> Option { + entries.iter().find(|p| file_id(p) == Some(id)).cloned() + } + + /// Resolve `id` to its on-disk path by **exact** match only, with no unique-prefix fallback. + /// + /// This is the lookup an *addressing* caller needs — `--session-id`, `serve`'s `?session_id=` — as + /// opposed to [`Self::find_path`]'s human-convenience resolution. The distinction is not cosmetic: + /// under prefix matching, asking for `abc` when `abcdef` already exists would both hand back the + /// wrong session *and* suppress creating the one actually asked for, so a caller minting ids that + /// happen to share a prefix would silently collapse onto whichever landed first. Exact-or-absent + /// keeps "open it, else create it under this exact name" total and unambiguous. + fn find_path_exact(&self, id: &str) -> std::io::Result> { + Ok(Self::exact_match(&self.session_paths()?, id)) + } + /// Resolve `id` to its on-disk path in this repo: an exact match first (cheap, unambiguous), then a /// unique-prefix match — pi's own convenience for typing a shortened id (`main.ts`'s /// `resolveSessionPath`), but *not* pi's own silent "pick whichever sorts first" when a prefix @@ -3299,28 +3354,15 @@ impl SessionRepo { /// candidate instead of a guess. `Ok(None)` when nothing matches at all — not found is not an error /// here, matching every existing caller's own "session may not exist" handling. fn find_path(&self, id: &str) -> std::io::Result> { - let entries: Vec = match fs::read_dir(&self.dir) { - Ok(entries) => entries.flatten().map(|e| e.path()).collect(), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), - Err(e) => return Err(e), - }; - let exact_suffix = format!("_{id}.jsonl"); - if let Some(path) = entries.iter().find(|p| { - p.file_name() - .and_then(|n| n.to_str()) - .is_some_and(|n| n.ends_with(&exact_suffix)) - }) { - return Ok(Some(path.clone())); + let entries = self.session_paths()?; + if let Some(path) = Self::exact_match(&entries, id) { + return Ok(Some(path)); } - // No exact match: fall back to a unique-prefix match over each file's own `` component - // (`split_once` on the *first* underscore only, since `` is always plain digits — - // never itself containing one — while a caller-supplied `--session-id` legally can). + // No exact match: fall back to a unique-prefix match over each file's own `` component. let matches: Vec<(&str, &PathBuf)> = entries .iter() .filter_map(|path| { - let name = path.file_name()?.to_str()?; - let rest = name.strip_suffix(".jsonl")?; - let (_, file_id) = rest.split_once('_')?; + let file_id = file_id(path)?; file_id.starts_with(id).then_some((file_id, path)) }) .collect(); @@ -3339,6 +3381,16 @@ impl SessionRepo { } } +/// The `` component of a `_.jsonl` session filename, or `None` for anything that +/// isn't shaped like one. `split_once` on the *first* underscore only: `` is always plain +/// digits and so never contains one, while a caller-supplied `--session-id` legally can — so everything +/// after that first separator is the id, `_` and all. +fn file_id(path: &Path) -> Option<&str> { + let name = path.file_name()?.to_str()?; + let (_, id) = name.strip_suffix(".jsonl")?.split_once('_')?; + Some(id) +} + /// Canonicalize `cwd` for session-matching purposes: resolves symlinks and `.`/`..` components (and, as /// a side effect, any trailing separator) so two different-but-equivalent spellings of the same real /// directory — a project reached through a symlink one time and its real path another, or a caller that @@ -6356,7 +6408,7 @@ mod tests { } #[test] - fn resume_or_create_reopens_the_session_matching_cwd() { + fn resume_latest_or_create_reopens_the_session_matching_cwd() { let dir = tmpdir(); let repo = SessionRepo::open(dir.path()).unwrap(); let mut other = repo.create(SessionMeta::new("/other", "m")).unwrap(); @@ -6369,34 +6421,35 @@ mod tests { sb.user("from my project"); mine.append_new(&sb.messages).unwrap(); - let (store, session) = repo.resume_or_create("/mine", "m", None).unwrap(); + let (store, session) = repo.resume_latest_or_create("/mine", "m").unwrap(); assert_eq!(store.meta().id, mine.meta().id); assert_eq!(session.messages.len(), 1); } #[test] - fn resume_or_create_makes_a_fresh_session_when_no_cwd_matches() { + fn resume_latest_or_create_makes_a_fresh_session_when_no_cwd_matches() { let dir = tmpdir(); let repo = SessionRepo::open(dir.path()).unwrap(); - let (store, session) = repo.resume_or_create("/brand/new", "m", None).unwrap(); + let (store, session) = repo.resume_latest_or_create("/brand/new", "m").unwrap(); assert_eq!(store.meta().cwd, "/brand/new"); assert!(session.messages.is_empty()); } #[test] - fn resume_or_create_uses_the_given_id_for_a_genuinely_fresh_session() { - // Backs `serve`'s own `--session-id` flag (pi-parity: `run` already had this, `serve` didn't). + fn open_or_create_id_mints_a_session_under_exactly_the_given_id() { let dir = tmpdir(); let repo = SessionRepo::open(dir.path()).unwrap(); let (store, session) = repo - .resume_or_create("/brand/new", "m", Some("my-chosen-id")) + .open_or_create_id("my-chosen-id", "/brand/new", "m") .unwrap(); assert_eq!(store.meta().id, "my-chosen-id"); assert!(session.messages.is_empty()); } #[test] - fn resume_or_create_ignores_the_given_id_when_an_existing_session_matches_the_cwd() { + fn open_or_create_id_wins_over_a_session_matching_the_cwd() { + // The regression that started this: an explicit id used to be silently discarded whenever *any* + // session already existed for the same cwd, so the caller got a stranger's conversation. let dir = tmpdir(); let repo = SessionRepo::open(dir.path()).unwrap(); let mut mine = repo.create(SessionMeta::new("/mine", "m")).unwrap(); @@ -6404,15 +6457,97 @@ mod tests { s.user("already here"); mine.append_new(&s.messages).unwrap(); - let (store, session) = repo - .resume_or_create("/mine", "m", Some("should-be-ignored")) - .unwrap(); + let (store, session) = repo.open_or_create_id("my-own-id", "/mine", "m").unwrap(); assert_eq!( store.meta().id, - mine.meta().id, - "an existing session's own id must win over a caller-supplied one" + "my-own-id", + "a caller-supplied id must never resolve to a different session" + ); + assert_ne!(store.meta().id, mine.meta().id); + assert!( + session.messages.is_empty(), + "the named session is brand new, so it must not inherit the cwd match's transcript" ); + } + + #[test] + fn distinct_ids_in_one_cwd_stay_distinct_sessions() { + // Multi-tenancy in one line: N ids in a shared directory must be N sessions, not one. This is + // the property `serve --session-id` and the daemon's `?session_id=` both route on. + let dir = tmpdir(); + let repo = SessionRepo::open(dir.path()).unwrap(); + for id in ["tenant-a", "tenant-b", "tenant-c"] { + let (mut store, mut session) = repo.open_or_create_id(id, "/shared", "m").unwrap(); + session.user(id); + store.append_new(&session.messages).unwrap(); + } + let ids: HashSet = repo.list().unwrap().into_iter().map(|m| m.id).collect(); + assert_eq!(ids.len(), 3, "each id must own its own session file"); + + // ...and each reopens to its own transcript, not a neighbour's. + for id in ["tenant-a", "tenant-b", "tenant-c"] { + let (_, session) = repo.open_or_create_id(id, "/shared", "m").unwrap(); + assert_eq!(session.messages.len(), 1); + assert!(format!("{:?}", session.messages[0]).contains(id)); + } + } + + #[test] + fn open_or_create_id_is_idempotent() { + // Re-running with the same id lands back on the same conversation rather than stacking up a new + // session per invocation — the property a supervised `serve --session-id` restart depends on. + let dir = tmpdir(); + let repo = SessionRepo::open(dir.path()).unwrap(); + let (mut store, mut session) = repo.open_or_create_id("pinned", "/w", "m").unwrap(); + session.user("first"); + store.append_new(&session.messages).unwrap(); + + let (reopened, session) = repo.open_or_create_id("pinned", "/w", "m").unwrap(); + assert_eq!(reopened.meta().id, "pinned"); assert_eq!(session.messages.len(), 1); + assert_eq!(repo.list().unwrap().len(), 1, "no duplicate was created"); + } + + #[test] + fn open_or_create_id_does_not_confuse_an_id_with_an_underscored_suffix_of_another() { + // Ids may legally contain `_`, and sessions are stored as `_.jsonl`. Matching on + // an `_.jsonl` *suffix* rather than the parsed component would resolve `b` onto `a_b`. + let dir = tmpdir(); + let repo = SessionRepo::open(dir.path()).unwrap(); + let (mut store, mut session) = repo.open_or_create_id("a_b", "/w", "m").unwrap(); + session.user("belongs to a_b"); + store.append_new(&session.messages).unwrap(); + + let (short, session) = repo.open_or_create_id("b", "/w", "m").unwrap(); + assert_eq!(short.meta().id, "b"); + assert!( + session.messages.is_empty(), + "`b` must not resolve onto the session named `a_b`" + ); + assert_eq!(repo.list().unwrap().len(), 2); + + // …and `a_b` still resolves to itself, by both lookups. + let (reopened, session) = repo.open_or_create_id("a_b", "/w", "m").unwrap(); + assert_eq!(reopened.meta().id, "a_b"); + assert_eq!(session.messages.len(), 1); + assert_eq!(repo.open_id("a_b").unwrap().0.meta().id, "a_b"); + } + + #[test] + fn open_or_create_id_matches_exactly_never_by_prefix() { + // `find_path`'s unique-prefix convenience is for a human typing a shortened id. Applying it to + // addressing would resolve `abc` onto `abcdef` *and* suppress creating `abc` — two wrongs that + // would silently collapse a set of prefix-sharing ids onto whichever landed first. + let dir = tmpdir(); + let repo = SessionRepo::open(dir.path()).unwrap(); + let (mut store, mut session) = repo.open_or_create_id("abcdef", "/w", "m").unwrap(); + session.user("the long one"); + store.append_new(&session.messages).unwrap(); + + let (short, session) = repo.open_or_create_id("abc", "/w", "m").unwrap(); + assert_eq!(short.meta().id, "abc"); + assert!(session.messages.is_empty()); + assert_eq!(repo.list().unwrap().len(), 2); } #[test] @@ -6433,7 +6568,8 @@ mod tests { } #[test] - fn resume_or_create_matches_a_session_recorded_under_a_symlinked_cwd_once_canonicalized() { + fn resume_latest_or_create_matches_a_session_recorded_under_a_symlinked_cwd_once_canonicalized() + { // The regression this guards: a project reached through a symlink one time and its real path // another must resolve to the same session, not silently fork into two. let dir = tmpdir(); @@ -6451,7 +6587,7 @@ mod tests { store.append_new(&s.messages).unwrap(); let link_cwd = canonical_cwd(&link).to_string_lossy().into_owned(); - let (reopened, session) = repo.resume_or_create(&link_cwd, "m", None).unwrap(); + let (reopened, session) = repo.resume_latest_or_create(&link_cwd, "m").unwrap(); assert_eq!( reopened.meta().id, store.meta().id, diff --git a/crates/agent/tests/run_session_management.rs b/crates/agent/tests/run_session_management.rs index e2c1920..8758dc4 100644 --- a/crates/agent/tests/run_session_management.rs +++ b/crates/agent/tests/run_session_management.rs @@ -75,12 +75,14 @@ fn run_binary_session_flag_persists_and_resumes_across_invocations() { } #[test] -fn run_binary_persists_and_resumes_a_session_by_default_with_no_session_flags_given() { - // pi-parity fix: a plain `run` with none of `--fork`/`--session`/`--continue` previously stayed - // in-memory-only — pi's own default (every mode, including one-shot print-mode) is a persisted, - // disk-backed session, matching `serve`'s own default repo-mode persistence. No `--continue` and no - // `--session` here at all: the second invocation must still pick up the first's history from the - // same per-cwd default repo `--continue` itself resolves against. +fn run_binary_persists_but_does_not_resume_by_default_with_no_session_flags_given() { + // Two halves of one contract. A plain `run` with none of `--fork`/`--session`/`--continue`: + // 1. *persists* — pi's own default (every mode, including one-shot print-mode) is a disk-backed + // session, so history isn't lost and `--continue` has something to find; but + // 2. does *not* resume — it starts its own session. It briefly did resume, which meant two shells + // in one repo drove the same store, and since `append_new` is count-keyed neither could see the + // other's writes: the transcript interleaved into nonsense. `--continue` is now the only thing + // that reattaches implicitly (covered by the `--continue` tests above). let home_dir = tempfile::tempdir().unwrap(); let project_dir = tempfile::tempdir().unwrap(); let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); @@ -138,10 +140,29 @@ fn run_binary_persists_and_resumes_a_session_by_default_with_no_session_flags_gi let bodies2 = bodies2.lock().unwrap(); assert!( - bodies2[0].contains("default-persist-99"), - "the second no-flag run must see the first no-flag run's history: {}", + !bodies2[0].contains("default-persist-99"), + "a second no-flag run must start its own session, not join the first's: {}", bodies2[0] ); + + // Both runs persisted — two separate sessions in the one per-cwd repo, neither discarded. + let repo = std::fs::read_dir(home_dir.path().join(".claude/sessions")) + .expect("sessions root") + .filter_map(Result::ok) + .map(|e| e.path()) + .find(|p| p.is_dir()) + .expect("a per-cwd repo directory"); + let sessions: Vec<_> = std::fs::read_dir(&repo) + .expect("repo dir") + .filter_map(Result::ok) + .map(|e| e.path()) + .filter(|p| p.extension().and_then(|e| e.to_str()) == Some("jsonl")) + .collect(); + assert_eq!( + sessions.len(), + 2, + "each no-flag run must own its own session file, and neither may be discarded: {sessions:?}" + ); } #[test] @@ -865,6 +886,81 @@ fn run_session_flag_warns_when_the_sessions_recorded_cwd_no_longer_matches() { ); } +#[test] +fn run_session_id_addresses_one_session_rather_than_whatever_matched_the_cwd() { + // `run`'s half of the same contract `serve` is covered for in `serve_session_addressing.rs`: an id + // names a session. It used to be discarded whenever any session already existed for this cwd, so a + // caller minting one id per task got a stranger's conversation instead of its own. + let home_dir = tempfile::tempdir().unwrap(); + let project_dir = tempfile::tempdir().unwrap(); + let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); + + let go = |base: &str, args: &[&str], msg: &str| { + let mut cmd = Command::new(bin); + cmd.env("HOME", home_dir.path()) + .args(["run", msg, "--gateway-url", base, "--key", "bai_v1.test"]) + .args(["--model", "claude-test"]) + .args(args) + .current_dir(project_dir.path()) + .stdin(Stdio::null()); + let out = cmd.output().expect("spawn binary"); + assert!( + out.status.success(), + "run failed.\nstderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + + // A plain run first, so the directory already holds a session for this cwd. + let (base1, _b1) = spawn_model_server(vec![turn_text("first answer")]); + go(&base1, &[], "remember the marker: squatter-3"); + + // Now an addressed run: it must open its *own* session, blind to the one above. + let (base2, bodies2) = spawn_model_server(vec![turn_text("second answer")]); + go(&base2, &["--session-id", "task-a"], "marker: task-a-1"); + assert!( + !bodies2.lock().unwrap()[0].contains("squatter-3"), + "--session-id must not inherit the cwd match's transcript" + ); + + // …and the same id again continues that session, idempotently. + let (base3, bodies3) = spawn_model_server(vec![turn_text("third answer")]); + go(&base3, &["--session-id", "task-a"], "what was the marker?"); + let body = &bodies3.lock().unwrap()[0]; + assert!( + body.contains("task-a-1"), + "the same id must reopen the same conversation: {body}" + ); + assert!( + !body.contains("squatter-3"), + "and still only that conversation: {body}" + ); + + // A second id in the same directory is a second session — 3 in total (squatter, task-a, task-b). + let (base4, bodies4) = spawn_model_server(vec![turn_text("fourth answer")]); + go(&base4, &["--session-id", "task-b"], "marker: task-b-1"); + assert!( + !bodies4.lock().unwrap()[0].contains("task-a-1"), + "distinct ids must be distinct sessions" + ); + + let repo = std::fs::read_dir(home_dir.path().join(".claude/sessions")) + .expect("sessions root") + .filter_map(Result::ok) + .map(|e| e.path()) + .find(|p| p.is_dir()) + .expect("a per-cwd repo directory"); + let ids: Vec = std::fs::read_dir(&repo) + .expect("repo dir") + .filter_map(Result::ok) + .map(|e| e.path()) + .filter(|p| p.extension().and_then(|e| e.to_str()) == Some("jsonl")) + .map(|p| session_id_of(&p)) + .collect(); + assert_eq!(ids.len(), 3, "three separate sessions on disk: {ids:?}"); + assert!(ids.contains(&"task-a".to_string()) && ids.contains(&"task-b".to_string())); +} + /// Reads a session `.jsonl` file's header line and returns its `id` field. fn session_id_of(path: &std::path::Path) -> String { let content = std::fs::read_to_string(path).unwrap(); diff --git a/crates/agent/tests/serve_session_addressing.rs b/crates/agent/tests/serve_session_addressing.rs new file mode 100644 index 0000000..a4fc002 --- /dev/null +++ b/crates/agent/tests/serve_session_addressing.rs @@ -0,0 +1,248 @@ +//! `serve` e2e: which session a launch opens — `--session-id` addressing, `--continue`, and the fresh +//! default. The contract under test is one sentence: **a session id is an address.** It resolves to the +//! session it names or to a brand-new one under exactly that name, never to some other session that +//! merely shares the directory. +//! +//! The regression behind this file: `--session-id` was consulted only when *no* session yet matched the +//! current `cwd`, so a single pre-existing session in a directory silently swallowed every id pointed at +//! it — every tenant, task, or connection collapsed onto one shared conversation. +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +mod common; + +use std::io::{BufReader, Write}; +use std::process::{Child, Command, Stdio}; + +use common::{ISOLATED_HOME, SpawnGuarded, read_until_response, spawn_model_server, turn_text}; +use serde_json::{Value, json}; + +/// A `serve` child bound to `--session-dir`, plus whatever extra selection flags a test wants +/// (`--session-id `, `--continue`). +fn serve_selecting(bin: &str, base: &str, session_dir: &str, extra: &[&str]) -> Command { + let mut c = Command::new(bin); + c.args([ + "serve", + "--gateway-url", + base, + "--key", + "bai_v1.test", + "--model", + "claude-test", + "--session-dir", + session_dir, + ]) + .args(extra) + .env("HOME", ISOLATED_HOME) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + c +} + +/// One command against a live `serve` child, returning the `data` object of its `response` frame. +fn command(child: &mut Child, cmd: Value) -> Value { + let name = cmd + .get("type") + .and_then(Value::as_str) + .expect("command needs a type") + .to_string(); + let mut stdin = child.stdin.as_ref().expect("stdin"); + writeln!(stdin, "{cmd}").unwrap(); + stdin.flush().unwrap(); + let mut stdout = BufReader::new(child.stdout.as_mut().expect("stdout")); + let frames = read_until_response(&mut stdout, &name); + frames + .into_iter() + .rfind(|f| f.get("type").and_then(Value::as_str) == Some("response")) + .and_then(|f| f.get("data").cloned()) + .unwrap_or_else(|| panic!("no response data for {name}")) +} + +/// `(session_id, message_count)` — the two facts every test here asserts on. +fn identity(child: &mut Child) -> (String, u64) { + let data = command(child, json!({ "type": "get_state" })); + ( + data["session_id"].as_str().expect("session_id").to_string(), + data["message_count"].as_u64().expect("message_count"), + ) +} + +/// Drive one prompt to completion, so the session has a real persisted turn behind it. +fn prompt(child: &mut Child, message: &str) { + command(child, json!({ "type": "prompt", "message": message })); +} + +#[test] +fn an_addressed_session_wins_over_an_unrelated_session_in_the_same_directory() { + // The exact collapse this fixes: a directory that already holds a session for this cwd must not + // capture a launch that named a *different* session. + let dir = tempfile::tempdir().unwrap(); + let session_dir = dir.path().to_string_lossy().into_owned(); + let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); + + let (base, _bodies) = spawn_model_server(vec![turn_text("first answer")]); + let mut squatter = serve_selecting(bin, &base, &session_dir, &[]).spawn_guarded(); + prompt(&mut squatter, "remember the marker: squatter-1"); + let (squatter_id, squatter_count) = identity(&mut squatter); + assert!(squatter_count > 0, "the squatter session recorded its turn"); + drop(squatter); + + let (base2, _bodies2) = spawn_model_server(vec![turn_text("second answer")]); + let mut addressed = + serve_selecting(bin, &base2, &session_dir, &["--session-id", "tenant-a"]).spawn_guarded(); + let (id, count) = identity(&mut addressed); + assert_eq!(id, "tenant-a", "the id asked for is the id opened"); + assert_ne!(id, squatter_id); + assert_eq!( + count, 0, + "an addressed session must start empty, not inherit the cwd match's transcript" + ); +} + +#[test] +fn distinct_session_ids_in_one_directory_are_distinct_sessions() { + // Multi-tenancy, end to end: N ids sharing one `--session-dir` are N conversations. + let dir = tempfile::tempdir().unwrap(); + let session_dir = dir.path().to_string_lossy().into_owned(); + let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); + + for tenant in ["tenant-a", "tenant-b"] { + let (base, _bodies) = spawn_model_server(vec![turn_text("ack")]); + let mut child = + serve_selecting(bin, &base, &session_dir, &["--session-id", tenant]).spawn_guarded(); + let (id, count) = identity(&mut child); + assert_eq!(id, tenant); + assert_eq!(count, 0, "{tenant} must not see the other tenant's history"); + prompt(&mut child, &format!("marker for {tenant}")); + } + + // Each reopens to its own transcript — the point of an address being stable. + for tenant in ["tenant-a", "tenant-b"] { + let (base, _bodies) = spawn_model_server(vec![turn_text("ack")]); + let mut child = + serve_selecting(bin, &base, &session_dir, &["--session-id", tenant]).spawn_guarded(); + let (id, count) = identity(&mut child); + assert_eq!(id, tenant); + assert_eq!(count, 2, "{tenant} reopens its own one-turn transcript"); + } +} + +#[test] +fn an_addressed_session_is_idempotent_across_restarts() { + // What a supervised (systemd, container) `serve --session-id` depends on: restart, same + // conversation — deterministically, rather than via "whatever touched this directory most recently". + let dir = tempfile::tempdir().unwrap(); + let session_dir = dir.path().to_string_lossy().into_owned(); + let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); + + let (base, _bodies) = spawn_model_server(vec![turn_text("first answer")]); + let mut first = + serve_selecting(bin, &base, &session_dir, &["--session-id", "pinned"]).spawn_guarded(); + prompt(&mut first, "remember the marker: pinned-42"); + drop(first); + + let (base2, _bodies2) = spawn_model_server(vec![turn_text("second answer")]); + let mut restarted = + serve_selecting(bin, &base2, &session_dir, &["--session-id", "pinned"]).spawn_guarded(); + let (id, count) = identity(&mut restarted); + assert_eq!(id, "pinned"); + assert_eq!(count, 2, "the restart picked the same conversation back up"); + + let sessions = jsonl_count(dir.path()); + assert_eq!(sessions, 1, "and did not stack up a second session file"); +} + +#[test] +fn a_bare_serve_starts_fresh_while_continue_reattaches() { + // The default flip. A bare launch owns its own session (two servers in one directory must not + // silently drive the same on-disk transcript); `--continue` is the one flag that reattaches. + let dir = tempfile::tempdir().unwrap(); + let session_dir = dir.path().to_string_lossy().into_owned(); + let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); + + let (base, _bodies) = spawn_model_server(vec![turn_text("first answer")]); + let mut first = serve_selecting(bin, &base, &session_dir, &[]).spawn_guarded(); + prompt(&mut first, "remember the marker: bare-7"); + let (first_id, _) = identity(&mut first); + drop(first); + + let (base2, _bodies2) = spawn_model_server(vec![turn_text("second answer")]); + let mut bare = serve_selecting(bin, &base2, &session_dir, &[]).spawn_guarded(); + let (bare_id, bare_count) = identity(&mut bare); + assert_ne!(bare_id, first_id, "a bare launch starts its own session"); + assert_eq!(bare_count, 0); + drop(bare); + + let (base3, _bodies3) = spawn_model_server(vec![turn_text("third answer")]); + let mut continued = serve_selecting(bin, &base3, &session_dir, &["--continue"]).spawn_guarded(); + let (continued_id, continued_count) = identity(&mut continued); + assert_eq!( + continued_id, bare_id, + "--continue reattaches to the most recent session for this cwd" + ); + assert_eq!( + continued_count, 0, + "which is the empty one the bare launch left" + ); +} + +#[test] +fn new_session_on_an_addressed_session_keeps_the_id_and_archives_the_old_transcript() { + // A pinned id is a routing key (`--session-id`, the daemon's `?session_id=`), so `new_session` must + // not move it out from under whoever holds it. It blanks in place — and the outgoing conversation is + // snapshotted into a session of its own rather than destroyed, so "start a new session" never + // doubles as "throw the old one away". + let dir = tempfile::tempdir().unwrap(); + let session_dir = dir.path().to_string_lossy().into_owned(); + let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); + + let (base, _bodies) = spawn_model_server(vec![turn_text("first answer")]); + let mut child = + serve_selecting(bin, &base, &session_dir, &["--session-id", "routed"]).spawn_guarded(); + prompt(&mut child, "remember the marker: routed-99"); + let (_, before) = identity(&mut child); + assert_eq!(before, 2); + + let data = command(&mut child, json!({ "type": "new_session" })); + assert_eq!( + data["session_id"].as_str(), + Some("routed"), + "the address a client routes on must survive new_session" + ); + let (id, count) = identity(&mut child); + assert_eq!(id, "routed"); + assert_eq!(count, 0, "…while the conversation itself is blank"); + + // The old transcript is still on disk as its own session, with lineage back to the slot. + assert_eq!( + jsonl_count(dir.path()), + 2, + "the outgoing conversation was archived, not overwritten" + ); + let archived = beyond_ai_agent::session_store::SessionRepo::open(dir.path()) + .unwrap() + .list() + .unwrap() + .into_iter() + .find(|m| m.id != "routed") + .expect("an archived sibling session"); + assert_eq!( + archived.parent.as_deref(), + Some("routed"), + "the archive records where it came from" + ); + assert!( + archived.message_count > 0, + "and carries the real transcript" + ); +} + +/// How many `.jsonl` sessions the repo directory holds. +fn jsonl_count(dir: &std::path::Path) -> usize { + std::fs::read_dir(dir) + .unwrap() + .filter_map(Result::ok) + .map(|e| e.path()) + .filter(|p| p.extension().and_then(|e| e.to_str()) == Some("jsonl")) + .count() +} diff --git a/crates/agent/tests/serve_session_lifecycle.rs b/crates/agent/tests/serve_session_lifecycle.rs index 7c32b54..fd8b29e 100644 --- a/crates/agent/tests/serve_session_lifecycle.rs +++ b/crates/agent/tests/serve_session_lifecycle.rs @@ -348,11 +348,12 @@ fn serve_resumes_newest_session_matching_cwd_not_globally_newest() { child.wait().unwrap(); } - // Reattach from project_a again — must resume A's transcript, not B's. + // Reattach from project_a again — must resume A's transcript, not B's. `--continue` is what asks + // to reattach at all; a bare `serve` would start its own session. { let (base, _bodies) = spawn_model_server(vec![]); let mut cmd = serve_dir_cmd(bin, &base, &session_dir); - cmd.current_dir(project_a.path()); + cmd.current_dir(project_a.path()).arg("--continue"); let mut child = cmd.spawn_guarded(); let mut stdin = child.stdin.take().unwrap(); let mut stdout = BufReader::new(child.stdout.take().unwrap()); @@ -410,7 +411,7 @@ fn serve_reattaches_through_a_symlinked_cwd_to_the_session_recorded_under_its_re { let (base, _bodies) = spawn_model_server(vec![]); let mut cmd = serve_dir_cmd(bin, &base, &session_dir); - cmd.current_dir(&link); + cmd.current_dir(&link).arg("--continue"); let mut child = cmd.spawn_guarded(); let mut stdin = child.stdin.take().unwrap(); let mut stdout = BufReader::new(child.stdout.take().unwrap()); @@ -1383,10 +1384,10 @@ fn serve_session_id_flag_applies_to_the_default_repo_mode_when_no_session_exists } #[test] -fn serve_session_id_flag_is_ignored_when_reattaching_to_an_existing_session() { - // Matches `run`'s own documented contract: a caller-chosen id only ever applies when a *new* - // `SessionMeta` is minted — reattaching to an existing session (already has a fixed id from disk) - // must not be silently renamed out from under it. +fn serve_session_id_flag_addresses_its_own_session_not_whatever_matched_the_cwd() { + // Matches `run`'s own contract: a caller-chosen id *addresses* a session. It used to be dropped + // whenever any session already matched this cwd, so a second process asking for a specific id was + // handed the first process's conversation instead — every id in a directory collapsing onto one. let session_dir_tmp = tempfile::tempdir().unwrap(); let session_dir = session_dir_tmp.path().to_string_lossy().into_owned(); let project = tempfile::tempdir().unwrap(); @@ -1412,12 +1413,12 @@ fn serve_session_id_flag_is_ignored_when_reattaching_to_an_existing_session() { id }; - // Second process, same cwd/session-dir, now with `--session-id` — must reattach to the same - // existing session rather than minting (or renaming to) the given id. + // Second process, same cwd/session-dir, now with `--session-id` — must open the session it named, + // not the unrelated one that happens to share the cwd. let (base, _bodies) = spawn_model_server(vec![turn_text("second")]); let mut cmd = serve_dir_cmd(bin, &base, &session_dir); cmd.current_dir(project.path()) - .args(["--session-id", "should-be-ignored"]); + .args(["--session-id", "mine-alone"]); let mut child = cmd.spawn_guarded(); let mut stdin = child.stdin.take().unwrap(); let mut stdout = BufReader::new(child.stdout.take().unwrap()); @@ -1426,9 +1427,14 @@ fn serve_session_id_flag_is_ignored_when_reattaching_to_an_existing_session() { stdin.flush().unwrap(); let state = read_until_response(&mut stdout, "get_state"); assert_eq!( + state.last().unwrap()["data"]["session_id"], + "mine-alone", + "--session-id must open the session it names, not one that merely shares the cwd" + ); + assert_ne!( state.last().unwrap()["data"]["session_id"], existing_id, - "reattaching to an existing session must keep its own id, not the --session-id argument" + "and must not be swallowed by the cwd match" ); drop(stdin); diff --git a/crates/agent/tests/serve_websocket.rs b/crates/agent/tests/serve_websocket.rs index e13b4a6..aeb5ce2 100644 --- a/crates/agent/tests/serve_websocket.rs +++ b/crates/agent/tests/serve_websocket.rs @@ -327,15 +327,18 @@ async fn ws_distinct_sessions_run_concurrently_and_stay_isolated() { "session bravo must be isolated: {db}" ); - // Each session persisted to its own file, named by id. - assert!( - dir.path().join("alpha.jsonl").exists(), - "alpha.jsonl should exist" - ); - assert!( - dir.path().join("bravo.jsonl").exists(), - "bravo.jsonl should exist" - ); + // Each session persisted as an ordinary repo session under its routing key. The daemon used to + // hand-build `.jsonl` while the repo names its own `_.jsonl`, so `find_path`'s + // `_.jsonl` lookup couldn't see daemon sessions at all: they showed up in `list_sessions` but + // `switch_session`/`open_id` reported them missing. Resolving by id is the assertion that matters — + // the filename is just how the repo spells it. + let repo = beyond_ai_agent::session_store::SessionRepo::open(dir.path()).unwrap(); + for id in ["alpha", "bravo"] { + let (store, _) = repo + .open_id(id) + .unwrap_or_else(|e| panic!("daemon session {id} must be openable by its id: {e}")); + assert_eq!(store.meta().id, id); + } let _ = child.kill(); let _ = child.wait(); @@ -390,13 +393,14 @@ async fn ws_sigterm_persists_in_flight_session_and_exits() { "serve --listen must exit promptly on SIGTERM, not hang" ); - // The in-flight session was persisted before exit (graceful, not killed mid-write). - let file = dir.path().join(format!("{SID}.jsonl")); - assert!( - file.exists(), - "the session file should exist after graceful shutdown" - ); - let content = std::fs::read_to_string(&file).unwrap(); + // The in-flight session was persisted before exit (graceful, not killed mid-write). Located + // through the repo by its routing key rather than by a hand-built filename — the daemon persists + // ordinary repo sessions (`_.jsonl`), so the id is what resolves it. + let repo = beyond_ai_agent::session_store::SessionRepo::open(dir.path()).unwrap(); + let (store, _) = repo + .open_id(SID) + .expect("the session should be on disk, and resolvable by id, after graceful shutdown"); + let content = std::fs::read_to_string(store.path()).unwrap(); assert!( content.contains("SIGMARKER"), "the in-flight prompt must be persisted on graceful shutdown, not lost: {content}" From e9d878bb716224e3d4989c8e988ba7493130c219 Mon Sep 17 00:00:00 2001 From: Jared Lunde Date: Sat, 15 Aug 2026 14:57:38 -0700 Subject: [PATCH 2/2] test(agent): fix two flakes in the new session-addressing e2e MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found by running the shard under CI's own nextest profile and filter, which plain `cargo test` doesn't reproduce. A fresh `BufReader` per command over one pipe is a latent hang: it reads ahead, so bytes of the next frame land in a buffer that's then dropped, and the following read blocks forever on data already consumed. A wedged test never drops its `ChildGuard`, which leaves a `serve` running — the failure mode `.github/workflows/ci.yml` documents as hanging the step rather than failing it. One reader per child now, with an explicit `shutdown()` that closes stdin and reaps. `a_bare_serve_starts_fresh_while_continue_reattaches` asserted *which* session `--continue` picks. `updated_at` is second-granularity, so a prompted session and an empty one written inside the same second tie, and the tie breaks on directory order — it failed (and retry-failed) under shard parallelism. It now asserts the deterministic contract: `--continue` reattaches to a session already present rather than minting one. "Most recent wins" stays pinned by serve_resumes_newest_session_matching_cwd_not_globally_newest, which separates its candidates by cwd rather than by time. Co-Authored-By: Claude Opus 5 (1M context) --- .../agent/tests/serve_session_addressing.rs | 233 ++++++++++++------ 1 file changed, 161 insertions(+), 72 deletions(-) diff --git a/crates/agent/tests/serve_session_addressing.rs b/crates/agent/tests/serve_session_addressing.rs index a4fc002..d4463c5 100644 --- a/crates/agent/tests/serve_session_addressing.rs +++ b/crates/agent/tests/serve_session_addressing.rs @@ -11,9 +11,11 @@ mod common; use std::io::{BufReader, Write}; -use std::process::{Child, Command, Stdio}; +use std::process::{Command, Stdio}; -use common::{ISOLATED_HOME, SpawnGuarded, read_until_response, spawn_model_server, turn_text}; +use common::{ + ChildGuard, ISOLATED_HOME, SpawnGuarded, read_until_response, spawn_model_server, turn_text, +}; use serde_json::{Value, json}; /// A `serve` child bound to `--session-dir`, plus whatever extra selection flags a test wants @@ -39,37 +41,74 @@ fn serve_selecting(bin: &str, base: &str, session_dir: &str, extra: &[&str]) -> c } -/// One command against a live `serve` child, returning the `data` object of its `response` frame. -fn command(child: &mut Child, cmd: Value) -> Value { - let name = cmd - .get("type") - .and_then(Value::as_str) - .expect("command needs a type") - .to_string(); - let mut stdin = child.stdin.as_ref().expect("stdin"); - writeln!(stdin, "{cmd}").unwrap(); - stdin.flush().unwrap(); - let mut stdout = BufReader::new(child.stdout.as_mut().expect("stdout")); - let frames = read_until_response(&mut stdout, &name); - frames - .into_iter() - .rfind(|f| f.get("type").and_then(Value::as_str) == Some("response")) - .and_then(|f| f.get("data").cloned()) - .unwrap_or_else(|| panic!("no response data for {name}")) +/// A live `serve` child together with its stdio. +/// +/// The `BufReader` is created **once** and reused for every command. A fresh one per command would be +/// a latent hang: `BufReader` reads ahead, so bytes of the *next* frame routinely land in its buffer, +/// and dropping it discards them — the following read then blocks forever on a response whose bytes +/// were already consumed. A wedged test never drops its [`ChildGuard`], which leaves a `serve` running +/// and (per `.github/workflows/ci.yml`) hangs the whole CI step rather than failing it. +struct Serve { + child: ChildGuard, + stdin: std::process::ChildStdin, + stdout: BufReader, } -/// `(session_id, message_count)` — the two facts every test here asserts on. -fn identity(child: &mut Child) -> (String, u64) { - let data = command(child, json!({ "type": "get_state" })); - ( - data["session_id"].as_str().expect("session_id").to_string(), - data["message_count"].as_u64().expect("message_count"), - ) -} +impl Serve { + fn start(cmd: &mut Command) -> Self { + let mut child = cmd.spawn_guarded(); + let stdin = child.stdin.take().expect("stdin"); + let stdout = BufReader::new(child.stdout.take().expect("stdout")); + Self { + child, + stdin, + stdout, + } + } + + /// Send one command, returning the `data` object of its `response` frame. + fn command(&mut self, cmd: Value) -> Value { + let name = cmd + .get("type") + .and_then(Value::as_str) + .expect("command needs a type") + .to_string(); + writeln!(self.stdin, "{cmd}").unwrap(); + self.stdin.flush().unwrap(); + read_until_response(&mut self.stdout, &name) + .into_iter() + .rfind(|f| f.get("type").and_then(Value::as_str) == Some("response")) + .and_then(|f| f.get("data").cloned()) + .unwrap_or_else(|| panic!("no response data for {name}")) + } -/// Drive one prompt to completion, so the session has a real persisted turn behind it. -fn prompt(child: &mut Child, message: &str) { - command(child, json!({ "type": "prompt", "message": message })); + /// `(session_id, message_count)` — the two facts every test here asserts on. + fn identity(&mut self) -> (String, u64) { + let data = self.command(json!({ "type": "get_state" })); + ( + data["session_id"].as_str().expect("session_id").to_string(), + data["message_count"].as_u64().expect("message_count"), + ) + } + + /// Drive one prompt to completion, so the session has a real persisted turn behind it. + fn prompt(&mut self, message: &str) { + self.command(json!({ "type": "prompt", "message": message })); + } + + /// Close stdin and reap, so the child is gone before the next one starts. `ChildGuard` would kill + /// it anyway, but an orderly EOF-then-exit is what these tests are actually asserting persisted + /// state after — and it leaves nothing behind to outlive the test. + fn shutdown(self) { + let Self { + mut child, + stdin, + stdout, + } = self; + drop(stdin); + drop(stdout); + let _ = child.wait(); + } } #[test] @@ -81,22 +120,27 @@ fn an_addressed_session_wins_over_an_unrelated_session_in_the_same_directory() { let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); let (base, _bodies) = spawn_model_server(vec![turn_text("first answer")]); - let mut squatter = serve_selecting(bin, &base, &session_dir, &[]).spawn_guarded(); - prompt(&mut squatter, "remember the marker: squatter-1"); - let (squatter_id, squatter_count) = identity(&mut squatter); + let mut squatter = Serve::start(&mut serve_selecting(bin, &base, &session_dir, &[])); + squatter.prompt("remember the marker: squatter-1"); + let (squatter_id, squatter_count) = squatter.identity(); assert!(squatter_count > 0, "the squatter session recorded its turn"); - drop(squatter); + squatter.shutdown(); let (base2, _bodies2) = spawn_model_server(vec![turn_text("second answer")]); - let mut addressed = - serve_selecting(bin, &base2, &session_dir, &["--session-id", "tenant-a"]).spawn_guarded(); - let (id, count) = identity(&mut addressed); + let mut addressed = Serve::start(&mut serve_selecting( + bin, + &base2, + &session_dir, + &["--session-id", "tenant-a"], + )); + let (id, count) = addressed.identity(); assert_eq!(id, "tenant-a", "the id asked for is the id opened"); assert_ne!(id, squatter_id); assert_eq!( count, 0, "an addressed session must start empty, not inherit the cwd match's transcript" ); + addressed.shutdown(); } #[test] @@ -108,22 +152,32 @@ fn distinct_session_ids_in_one_directory_are_distinct_sessions() { for tenant in ["tenant-a", "tenant-b"] { let (base, _bodies) = spawn_model_server(vec![turn_text("ack")]); - let mut child = - serve_selecting(bin, &base, &session_dir, &["--session-id", tenant]).spawn_guarded(); - let (id, count) = identity(&mut child); + let mut child = Serve::start(&mut serve_selecting( + bin, + &base, + &session_dir, + &["--session-id", tenant], + )); + let (id, count) = child.identity(); assert_eq!(id, tenant); assert_eq!(count, 0, "{tenant} must not see the other tenant's history"); - prompt(&mut child, &format!("marker for {tenant}")); + child.prompt(&format!("marker for {tenant}")); + child.shutdown(); } // Each reopens to its own transcript — the point of an address being stable. for tenant in ["tenant-a", "tenant-b"] { let (base, _bodies) = spawn_model_server(vec![turn_text("ack")]); - let mut child = - serve_selecting(bin, &base, &session_dir, &["--session-id", tenant]).spawn_guarded(); - let (id, count) = identity(&mut child); + let mut child = Serve::start(&mut serve_selecting( + bin, + &base, + &session_dir, + &["--session-id", tenant], + )); + let (id, count) = child.identity(); assert_eq!(id, tenant); assert_eq!(count, 2, "{tenant} reopens its own one-turn transcript"); + child.shutdown(); } } @@ -136,17 +190,26 @@ fn an_addressed_session_is_idempotent_across_restarts() { let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); let (base, _bodies) = spawn_model_server(vec![turn_text("first answer")]); - let mut first = - serve_selecting(bin, &base, &session_dir, &["--session-id", "pinned"]).spawn_guarded(); - prompt(&mut first, "remember the marker: pinned-42"); - drop(first); + let mut first = Serve::start(&mut serve_selecting( + bin, + &base, + &session_dir, + &["--session-id", "pinned"], + )); + first.prompt("remember the marker: pinned-42"); + first.shutdown(); let (base2, _bodies2) = spawn_model_server(vec![turn_text("second answer")]); - let mut restarted = - serve_selecting(bin, &base2, &session_dir, &["--session-id", "pinned"]).spawn_guarded(); - let (id, count) = identity(&mut restarted); + let mut restarted = Serve::start(&mut serve_selecting( + bin, + &base2, + &session_dir, + &["--session-id", "pinned"], + )); + let (id, count) = restarted.identity(); assert_eq!(id, "pinned"); assert_eq!(count, 2, "the restart picked the same conversation back up"); + restarted.shutdown(); let sessions = jsonl_count(dir.path()); assert_eq!(sessions, 1, "and did not stack up a second session file"); @@ -156,33 +219,54 @@ fn an_addressed_session_is_idempotent_across_restarts() { fn a_bare_serve_starts_fresh_while_continue_reattaches() { // The default flip. A bare launch owns its own session (two servers in one directory must not // silently drive the same on-disk transcript); `--continue` is the one flag that reattaches. + // + // Deliberately asserts *which set* `--continue` lands in, not which member. `updated_at` is + // second-granularity, so two sessions written inside the same second tie, and a tie is broken by + // directory order — nondeterministic. "Most recent wins" is pinned separately, and unambiguously, + // by `serve_resumes_newest_session_matching_cwd_not_globally_newest` (which distinguishes its + // candidates by `cwd` rather than by time). let dir = tempfile::tempdir().unwrap(); let session_dir = dir.path().to_string_lossy().into_owned(); let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); let (base, _bodies) = spawn_model_server(vec![turn_text("first answer")]); - let mut first = serve_selecting(bin, &base, &session_dir, &[]).spawn_guarded(); - prompt(&mut first, "remember the marker: bare-7"); - let (first_id, _) = identity(&mut first); - drop(first); + let mut first = Serve::start(&mut serve_selecting(bin, &base, &session_dir, &[])); + first.prompt("remember the marker: bare-7"); + let (first_id, _) = first.identity(); + first.shutdown(); let (base2, _bodies2) = spawn_model_server(vec![turn_text("second answer")]); - let mut bare = serve_selecting(bin, &base2, &session_dir, &[]).spawn_guarded(); - let (bare_id, bare_count) = identity(&mut bare); + let mut bare = Serve::start(&mut serve_selecting(bin, &base2, &session_dir, &[])); + let (bare_id, bare_count) = bare.identity(); assert_ne!(bare_id, first_id, "a bare launch starts its own session"); - assert_eq!(bare_count, 0); - drop(bare); + assert_eq!( + bare_count, 0, + "and does not inherit the earlier session's transcript" + ); + bare.shutdown(); + assert_eq!( + jsonl_count(dir.path()), + 2, + "two bare launches, two sessions" + ); let (base3, _bodies3) = spawn_model_server(vec![turn_text("third answer")]); - let mut continued = serve_selecting(bin, &base3, &session_dir, &["--continue"]).spawn_guarded(); - let (continued_id, continued_count) = identity(&mut continued); - assert_eq!( - continued_id, bare_id, - "--continue reattaches to the most recent session for this cwd" + let mut continued = Serve::start(&mut serve_selecting( + bin, + &base3, + &session_dir, + &["--continue"], + )); + let (continued_id, _) = continued.identity(); + assert!( + continued_id == bare_id || continued_id == first_id, + "--continue must reattach to a session already here, not mint one: {continued_id}" ); + continued.shutdown(); assert_eq!( - continued_count, 0, - "which is the empty one the bare launch left" + jsonl_count(dir.path()), + 2, + "--continue reattached rather than creating a third session" ); } @@ -197,21 +281,26 @@ fn new_session_on_an_addressed_session_keeps_the_id_and_archives_the_old_transcr let bin = env!("CARGO_BIN_EXE_beyond-ai-agent"); let (base, _bodies) = spawn_model_server(vec![turn_text("first answer")]); - let mut child = - serve_selecting(bin, &base, &session_dir, &["--session-id", "routed"]).spawn_guarded(); - prompt(&mut child, "remember the marker: routed-99"); - let (_, before) = identity(&mut child); + let mut child = Serve::start(&mut serve_selecting( + bin, + &base, + &session_dir, + &["--session-id", "routed"], + )); + child.prompt("remember the marker: routed-99"); + let (_, before) = child.identity(); assert_eq!(before, 2); - let data = command(&mut child, json!({ "type": "new_session" })); + let data = child.command(json!({ "type": "new_session" })); assert_eq!( data["session_id"].as_str(), Some("routed"), "the address a client routes on must survive new_session" ); - let (id, count) = identity(&mut child); + let (id, count) = child.identity(); assert_eq!(id, "routed"); assert_eq!(count, 0, "…while the conversation itself is blank"); + child.shutdown(); // The old transcript is still on disk as its own session, with lineage back to the slot. assert_eq!(