Send x-opencode-session on OpenCode requests

[aebrer]

Closes #500

Ensure OpenCode Zen and Go requests carry a stable per-conversation session header across all supported provider protocols and internal LLM request paths.

Implementation plan posted as a comment below.

[aebrer]

Implementation Plan

Problem analysis

dreb already creates the right stable conversation identifier: SessionManager persists a UUID, restores it on resume, and replaces it for new sessions and forks. The main Agent forwards that UUID as StreamOptions.sessionId, but none of the OpenCode transports convert it into the required x-opencode-session HTTP header.

OpenCode models span OpenAI Completions, OpenAI Responses, Anthropic Messages, and Google Generative AI, so protocol-local fixes would be easy to miss or drift apart. Several coding-agent features also call completeSimple() outside the main agent loop and currently omit the session ID: compaction, branch summaries, tab titles, subagent model probes, buddy generation, and the dispatch arbiter.

The fix should derive the header per request rather than mutating shared model definitions. This preserves isolation between concurrent sessions and gives explicit configured/request headers higher precedence.

Deliverables

1. Central OpenCode session-header normalization in @dreb/ai

   • Add a small provider-specific helper that recognizes the built-in opencode and opencode-go providers plus models whose parsed base-URL hostname is exactly opencode.ai.
   • When sessionId is present, add x-opencode-session before API dispatch so every supported protocol receives the same behavior.
   • Merge case-insensitively, with generated headers at lowest precedence and explicit model/request headers winning without duplicate differently-cased names.
   • Leave non-OpenCode providers and OpenCode calls without a supplied session ID unchanged; do not invent a random per-request value that would violate the stable-per-conversation contract.

2. Complete session-ID propagation through coding-agent request paths

   • Keep the existing main-agent flow unchanged except for consuming the header normalization.
   • Pass the parent AgentSession UUID into manual/automatic compaction and turn-prefix summarization.
   • Pass it into branch summarization.
   • Expose it to tab-title generation in both interactive and RPC modes, including title-model fallback probes.
   • Thread it through subagent model-resolution probes and dispatch arbitration.
   • Pass it into OpenCode-capable buddy hatch/reroll generation; leave the local Ollama-only reaction path untouched.
   • Use the parent session UUID for these auxiliary one-shot requests because they belong to that conversation. Spawned child agents continue using their own persisted child-session UUIDs.

3. Regression coverage across layers

   • Verify final HTTP headers for representative OpenCode models on each supported API protocol.
   • Verify exact-provider and exact-host matching, mixed-case override precedence, no duplicate header names, absent-session behavior, and non-OpenCode isolation.
   • Verify each auxiliary caller forwards the supplied session ID.
   • Retain the existing session lifecycle tests proving stability across turns/resume and replacement on new/fork.
   • Update the opt-in live OpenCode smoke test to supply one stable ID for its requests.

Acceptance criteria

• OpenCode Zen and Go requests with a dreb session include x-opencode-session on every API protocol used by their current model catalog.
• Main turns and auxiliary requests belonging to one AgentSession use that persisted session UUID.
• The UUID remains stable across turns and resume and changes on new sessions/forks, as established by the existing lifecycle behavior.
• Explicit model/request values override the generated default case-insensitively, with one effective header value.
• Non-OpenCode providers and lookalike hostnames never receive the header.
• Every production completeSimple() path that can select an OpenCode model either forwards the parent session ID or is intentionally non-OpenCode-only.
• Targeted tests, the full test suite, type checking, build, and workspace-link verification pass.

Files to create or modify

@dreb/ai

• Create packages/ai/src/providers/opencode-headers.ts — OpenCode matching and case-insensitive low-precedence session-header merge helper.
• Modify packages/ai/src/stream.ts — apply the helper consistently in both full and simple stream dispatch paths.
• Create packages/ai/test/opencode-session-headers.test.ts — protocol, provider/hostname matching, precedence, duplicate-casing, missing-session, and isolation coverage.
• Modify packages/ai/test/zen.test.ts — give the opt-in live smoke conversation a stable session ID.

@dreb/coding-agent production

• Modify packages/coding-agent/src/core/agent-session.ts — pass the live session UUID into compaction, branch-summary, subagent-probe, and dispatch-arbiter dependencies.
• Modify packages/coding-agent/src/core/compaction/compaction.ts — accept and forward sessionId for summary and turn-prefix requests.
• Modify packages/coding-agent/src/core/compaction/branch-summarization.ts — accept and forward sessionId in branch-summary options.
• Modify packages/coding-agent/src/core/tab-title.ts — add a session-ID dependency and forward it to title completions and fallback probes.
• Modify packages/coding-agent/src/core/tools/subagent.ts — carry session ID through model resolution and availability probes.
• Modify packages/coding-agent/src/core/dispatch-arbiter.ts — obtain and forward the parent session ID for arbiter requests.
• Modify packages/coding-agent/src/core/buddy/buddy-manager.ts — accept and forward session ID for parent-model soul generation.
• Modify packages/coding-agent/src/modes/interactive/interactive-mode.ts — provide session ID to tab-title and buddy flows.
• Modify packages/coding-agent/src/modes/rpc/rpc-mode.ts — provide session ID to tab-title and buddy flows.

@dreb/coding-agent tests

• Modify packages/coding-agent/test/compaction-summary-reasoning.test.ts and compaction.test.ts — assert summary request options retain the session ID in reasoning and non-reasoning paths.
• Create packages/coding-agent/test/branch-summarization.test.ts — directly verify branch-summary forwarding.
• Modify packages/coding-agent/test/tab-title.test.ts — verify title completion and fallback probing receive the same session ID.
• Modify packages/coding-agent/test/subagent-model-fallback.test.ts — verify availability probes receive the supplied parent session ID.
• Modify packages/coding-agent/test/dispatch-arbiter.test.ts — verify both arbiter attempts retain the session ID.
• Modify packages/coding-agent/test/buddy-manager.test.ts — verify hatch/reroll parent-model requests receive the session ID while the Ollama-only path is unaffected.
• Modify the relevant agent-session compaction/branching tests if needed to cover the final wiring from AgentSession.sessionId into those helpers, rather than testing only helper-level options.

No public configuration, root README, or provider documentation change is planned: the work restores an upstream compatibility requirement using an existing internal session option.

Testing approach

1. Use mocked SDK/fetch transports in the new AI test to inspect actual outgoing headers for representative OpenAI Completions, OpenAI Responses, Anthropic Messages, and Google Generative AI models.
2. Use existing mocked completeSimple() patterns in coding-agent tests to inspect auxiliary request options without network access.
3. Exercise edge cases:
   • exact built-in provider IDs;
   • custom provider pointed at exact opencode.ai hostname;
   • malformed and lookalike URLs;
   • absent/empty session ID;
   • explicit mixed-case header override;
   • non-OpenCode provider with a session ID;
   • repeated/retried auxiliary calls retaining one ID.
4. Run formatting/linting on touched files, then npm run build, targeted Vitest files, npx tsgo --noEmit, npm test, and npm run verify-workspace-links.
5. If an OpenCode key is available, run the existing opt-in zen.test.ts live smoke test as supplemental evidence; automated mocked-header coverage remains mandatory.

Risks and open questions

• Header precedence: dreb currently uses case-sensitive object spreads in several SDK clients. The helper must explicitly normalize only the affected header so an override such as X-OpenCode-Session cannot coexist with the generated lowercase key.
• Session leakage: hostname fallback must use parsed exact-host equality, not substring matching, to avoid sending the UUID to lookalike domains. Exact built-in provider IDs still receive it when intentionally routed through a configured proxy.
• Concurrent sessions: never write the session header into shared Model.headers; derive immutable per-request options instead.
• Auxiliary semantics: parent-bound helper calls use the parent UUID; child agent conversations use their child UUID. This matches the issue's conversation grouping without introducing extra persistent IDs.
• Standalone @dreb/ai callers: the library cannot infer conversation boundaries. Callers that use OpenCode directly must pass the already-supported sessionId; generating a fresh fallback on each invocation would satisfy header presence while defeating the stated optimization requirement.
• Catalog churn: central dispatch normalization avoids maintaining a hardcoded model list as OpenCode moves models between protocols.

---

Plan created by mach6

[github-actions]

Vitest coverage

Metric	Covered	Total	Coverage
Statements	41091	56768	72.38%
Branches	22322	35765	62.41%
Functions	8716	11908	73.19%
Lines	29662	40807	72.68%

View full coverage run

[aebrer]

Progress Update

Implementation complete.

@dreb/ai — central header normalization

• New packages/ai/src/providers/opencode-headers.ts: recognizes built-in opencode/opencode-go providers plus models whose base-URL hostname is exactly opencode.ai (parsed-host equality, lookalike domains and malformed URLs rejected).
• withOpenCodeSessionHeader() adds x-opencode-session at lowest, case-insensitive precedence — explicit model/request headers win, inputs are never mutated, non-OpenCode models and missing/empty session IDs pass through untouched (same-object identity preserved).
• Applied in both stream() and streamSimple() dispatch paths, so every supported protocol (OpenAI Completions/Responses, Anthropic Messages, Google Generative AI) receives identical behavior.

@dreb/coding-agent — session ID threading

• Compaction (summary + turn-prefix), branch summarization, tab-title (completion + fallback probe), subagent availability probes (parent session UUID; children keep their own), dispatch arbiter (both retry attempts), and buddy hatch/reroll now forward the stable AgentSession UUID.
• Wired from AgentSession.sessionId plus interactive/RPC mode tab-title and buddy dependencies.

Tests

• New opencode-session-headers.test.ts (20 tests): wire-level header assertions via real SDK clients with stubbed fetch on all four protocols, provider/hostname matching, mixed-case override precedence, no-mutation, absent-session, non-OpenCode isolation, repeated-call stability.
• 12 new forwarding tests + 3 mock fixes across compaction, branch-summarization (new file), tab-title, subagent fallback, dispatch arbiter, and buddy-manager suites.
• Live zen smoke test now supplies one stable session ID.

Verification: full pre-commit gate green — biome, tsgo --noEmit, and the complete test suite (6001 passed / 0 failed).

Commit: df5067e

---

Progress tracked by mach6

[aebrer]

Unverified Review Candidates — Pending Assessment

Review round: 1
Reviewed commit: df5067e

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

None.

Important

Finding 1 — Session ID loaded from a session file is used unsanitized as an HTTP header value (error-auditor, confidence 85)

SessionManager.setSessionFile() adopts the session-file header ID verbatim (session-manager.ts:736, this.sessionId = header?.id ?? randomUUID()); the only validation is a typeof === "string" check, so an ID containing CRLF, NUL, or non-Latin-1 characters is accepted. That value now flows into the x-opencode-session header via withOpenCodeSessionHeader (no value validation). The auditor reproduced this against a local server with the built @dreb/ai: sessionId: "evil\r\nX-Injected: 1" makes undici throw Headers.append: ... is an invalid header value, so after this PR, resuming / --session / --fork of a tampered or corrupted session file with an OpenCode model turns every request into a hard error:

• main conversation: stopReason: "error" with a misleading message that never mentions the session ID/file;
• compaction: throws Summarization failed: Headers.append: ... mid-compaction;
• dispatch arbiter: both attempts fail;
• subagent probes: every OpenCode candidate model reported unavailable;
• buddy hatch/reroll: silent — generateSoul's pre-existing catch swallows the error and falls back to the default species soul.

Before this PR the same file worked fine (no header was generated). Trigger is local file tampering/corruption only (all legitimate write paths use randomUUID()), no remote vector. Suggested fix is fail-closed at the value: reject IDs outside a conservative charset at session-file load (the existing "corrupt header" branch already resets to a fresh UUID), or only emit the header when the ID matches a strict allow-list in withOpenCodeSessionHeader.

Finding 2 — No test covers the final AgentSession → auxiliary call-site wiring (test-reviewer, confidence 92)

Every new call site that injects this.sessionId is untested at the level where it was added. All new coding-agent tests pass sessionId explicitly into helpers (generateBranchSummary(entries, {sessionId}), hatch(model, key, uuid), …) with @dreb/ai mocked, so deleting this.sessionId from any AgentSession call site (manual/auto compaction, branch summarization, subagent tool dep, mode-level tab-title/buddy getters) would keep the entire suite green while silently stripping the header from real OpenCode requests. The plan's stated promise to "update the relevant agent-session compaction/branching tests if needed to cover the final wiring from AgentSession.sessionId" was not honored — the pre-existing agent-session-compaction.test.ts / agent-session-branching.test.ts are live-API-gated (skipped in CI) and were not updated. Suggested fix follows the existing pattern in agent-session-auto-compaction-queue.test.ts (real AgentSession + mocked compaction index), asserting the captured sessionId argument equals session.sessionId.

Suggestions

Finding 3 — compact() sessionId fan-out is untested; the turn-prefix summary LLM path has zero coverage (test-reviewer, confidence 85)

New compaction tests exercise generateSummary(...) directly, but nothing calls compact() with a sessionId. compact() fans the parameter out to three call sites (split-turn generateSummary, split-turn generateTurnPrefixSummary, normal-path generateSummary); dropping the argument on any one — most plausibly generateTurnPrefixSummary, whose LLM path has no tests at all — passes the whole suite. Same regression class as finding 2, one layer down; cheap to close with the existing vi.mock("@dreb/ai") pattern.

Finding 4 — Request-level explicit header override is tested only at unit level, not on the wire (test-reviewer, confidence 82)

The acceptance criterion is about explicit request-level x-opencode-session overriding the generated default with one effective header value. Wire tests prove the model.headers case only; the options.headers case is asserted only against the pure withOpenCodeSessionHeader function. A provider header-merge refactor (e.g. options headers no longer folded into client defaults) could silently drop a user's explicit request-level header, or double-send a differently-cased duplicate, without any test failing. Suggested fix: one wire test with { apiKey, sessionId, headers: { "X-OpenCode-Session": "explicit" } } asserting exactly one effective value on the captured request.

Finding 5 — Unnecessary as TOptions cast in withOpenCodeSessionHeader (simplifier, confidence 90)

opencode-headers.ts final return: the object literal is assignable to TOptions without the cast — verified that removing it type-checks clean under both tsc and tsgo on a scratch copy, and the 20-test suite passes. Pure verbosity.

Finding 6 — Unnecessary as StreamOptions cast in stream() plus now-unused import (simplifier, confidence 90)

stream.ts:32 — withOpenCodeSessionHeader(model, options) infers TOptions = ProviderStreamOptions | undefined, which is assignable to the options?: StreamOptions parameter (the sibling streamSimple call already passes the helper result without a cast). Removing the cast type-checks clean under both tsc and tsgo; it also makes the StreamOptions import unused, so the import line should be trimmed in the same change. Net −2 lines, zero runtime change.

Strengths

• Central dispatch is the right shape: header injection in stream()/streamSimple() covers all four protocols (openai-completions, openai-responses, anthropic-messages, google-generative-ai — the exact protocol set in the built-in OpenCode catalogs, verified in models.generated.ts) and every completeSimple() call site at once; no protocol-local drift.
• Protocol coverage proven at the wire: 20 new @dreb/ai tests capture outgoing headers through the real SDK clients with stubbed fetch — all four protocols, lookalike-hostname rejection (including userinfo/percent-encoding tricks, empirically verified safe), mixed-case override precedence, non-OpenCode isolation, absent-session behavior, no-mutation, and stable-ID-on-shared-options.
• Completeness of the auxiliary wiring is real: full production call-site census (all packages) shows every AgentSession-bound LLM path now forwards the parent UUID, both retry attempts in the dispatch arbiter, and both tab-title completion + fallback probe; the Ollama-only buddy reaction path is correctly untouched per the approved plan.
• Non-mutation and concurrency safety: the helper returns the same object when no header is added and a fresh object when it is; verified pure and idempotent.
• Conservative matching: exact hostname equality (case-insensitive, no substring) with the header omitted as the safe direction on malformed URLs — no leak path to lookalike hosts found.

Agents run: code-reviewer (no candidates), error-auditor (1), test-reviewer (3), completeness-checker (no candidates), simplifier (2)

---

Reviewed by mach6

[aebrer]

Review Assessment

#501 (comment)

Classifications

Finding	Classification	Reasoning
1 — Session ID loaded from a session file used unsanitized as header value	useful follow-up	Factual: holds (reproduced: CRLF/NUL ID from a tampered session file makes undici throw in both openai and anthropic SDK clients; the "non-Latin-1" sub-claim does not hold on this build). Scope: in as a PR-introduced failure mode (pre-PR no header was generated, so such files worked with OpenCode; post-PR they hard-fail), but trigger is local file tampering only — all legitimate ID write paths use randomUUID(). Practical: borderline pass — local, self-inflicted DoS that fails closed (request rejected client-side; no injection, no cross-session impact); the identical risk pattern already ships pre-PR for mistral x-affinity and codex session_id. Cheap fix available: only emit the header in withOpenCodeSessionHeader when the ID matches a strict allow-list (UUIDs always pass, so no acceptance criterion is affected).
2 — No test covers the final AgentSession → auxiliary wiring	merge blocker	Factual: holds — all new coding-agent tests are helper-level with explicitly-passed IDs; agent-session-compaction.test.ts:24 and agent-session-branching.test.ts:24 are describe.skipIf(!API_KEY) (skipped in CI) and assert summary text, never the sessionId argument; agent-session-auto-compaction-queue.test.ts mocks the entire compaction module so it is argument-blind; zero test files reference parentSessionId. Deleting this.sessionId at any of the six wiring sites keeps the suite green. Scope: in — the approved plan explicitly promised to "update the relevant agent-session compaction/branching tests if needed to cover the final wiring from AgentSession.sessionId, rather than testing only helper-level options"; the PR shipped exactly the rejected state, and no non-live test exists to update, which is precisely the need. Practical: passes — per the verbatim OpenCode notice, requests missing the header may error starting 09/06 (days after merge), so a one-argument regression at any wiring site silently turns OpenCode compaction/branch-summary/tab-title/subagent-probe/buddy requests from "optimized" to "possibly rejected," with the full suite green.
3 — compact() sessionId fan-out untested; turn-prefix LLM path has zero coverage	useful follow-up	Factual: holds — compact() fans the parameter to three completeSimple sites (split-turn summary, split-turn turn-prefix, normal-path summary); the only real-compact() tests are live-gated and pass no sessionId; generateTurnPrefixSummary's LLM path is untested (pre-existing gap). Scope: partially in — the fan-out arguments are this PR's code, but the plan's promise was one level up (AgentSession). Practical: real regression risk, but fully subsumed by implementing finding 2's fix at the LLM boundary, which drives the real compact() and catches all three sites at once. Track as folded into the finding-2 fix.
4 — Request-level options.headers override tested only at unit level	useful follow-up	Factual: holds — the wire suite proves the model.headers override case only; the options.headers case is asserted only against the pure helper. Scope: not explicitly required — the "override precedence" test criterion is met at unit level for both header sources. Practical: weak pass — the guarded regression would be a future refactor of untouched provider header-merging code, would simultaneously break all documented custom-header usage (immediately visible), and the dedup logic itself is unit-tested at its implementation site. A ~15-line wire test is cheap low-priority insurance.
5 — Unnecessary as TOptions cast in opencode-headers.ts	nitpick	Factual: holds — removal type-checks clean under both tsc and tsgo repo-wide, and the 20-test suite passes. Scope: out (no criterion or plan item involved). Practical: fails — zero runtime change.
6 — Unnecessary as StreamOptions cast + unused import in stream.ts	nitpick	Factual: holds — removal plus import trim type-checks clean under both compilers; streamSimple already passes the helper result cast-free, confirming assignability. Scope: out. Practical: fails — zero runtime change (−2 lines).

Action Plan

1. Finding 2 (merge blocker), with finding 3 folded in: add non-live regression test(s) proving the final wiring — construct a real AgentSession using the existing in-memory harness pattern from agent-session-auto-compaction-queue.test.ts, mock at the LLM boundary (not the compaction-module boundary), and assert sessionId reaches manual compaction, auto compaction, and branch summarization with session.sessionId. Implementing at the LLM boundary drives the real compact(), which also covers all three fan-out sites including the previously untested generateTurnPrefixSummary path (finding 3) in the same pass.

Non-blocking follow-ups, in priority order: (1) session-ID header allow-list in withOpenCodeSessionHeader (finding 1); (3) options.headers wire test (finding 4); cast cleanups (findings 5, 6) are optional and can ride along with the finding-2 commit.

---

Assessment by mach6

[aebrer]

Progress Update

Addressed findings 2 and 3 from round 2 of the mach6 review: the earlier session-ID coverage was helper-level, so dropping this.sessionId from any AgentSession call site would have kept the whole suite green.

Added packages/coding-agent/test/agent-session-session-id-wiring.test.ts — non-live (no API keys) regression tests that build a real AgentSession in the in-memory harness and mock only the LLM boundary (completeSimple), asserting the stable session ID reaches the outgoing LLM call on:

• manual compaction (normal path)
• manual compaction (split turn — covers the previously zero-coverage generateTurnPrefixSummary fan-out, closing finding 3)
• auto compaction
• branch summarization via navigateTree

Mutation-checked each wiring site: removing the session ID at any one of the four sites (manual compaction, auto compaction, branch summarization, and the turn-prefix fan-out inside compact()) fails the corresponding test(s). All mutations reverted; only the new test file changed.

Commit: 8446436

---

Progress tracked by mach6

[aebrer]

Unverified Review Candidates — Pending Assessment

Review round: 2
Reviewed commit: 8446436

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

None.

Important

Finding 1 — Subagent tool parentSessionId wiring at the AgentSession level is untested (test-reviewer, confidence 90)

packages/coding-agent/src/core/agent-session.ts:3254 adds parentSessionId: () => this.sessionId to the subagent tool deps built by _buildRuntime. Deleting that line keeps the entire suite green: every subagent test either calls the helpers directly with an explicit sessionId (subagent-model-fallback.test.ts) or hand-constructs createSubagentToolDefinition deps itself; no test drives an AgentSession-built subagent tool and inspects the probe options. Regression that ships green: spawn-time availability probes to OpenCode models go out headerless — after OpenCode's 2026-09-06 enforcement, subagents with explicit model specs (the norm for mach6/custom agents) fail every probe and the spawn dies with "None of the fallback models passed availability checks".

Finding 2 — DispatchArbiter getSessionId wiring at the AgentSession level is untested (test-reviewer, confidence 90)

packages/coding-agent/src/core/agent-session.ts:469 adds getSessionId: () => this.sessionId to the DispatchArbiter constructor deps. Deleting that line keeps the suite green: dispatch-arbiter.test.ts constructs the arbiter directly with explicit deps, and the two tests that build a real AgentSession (subagent-arbiter.test.ts:572, 671) inject fakes that discard the options argument where the sessionId lives (async (_model, context) / bare vi.fn()). Regression that ships green: every arbitration request to an OpenCode model is headerless → rejected after 2026-09-06 → arbitration fails (inference_failed) and gated spawns break. Cheapest close: capture the 3rd argument in the existing injected fake and assert options.sessionId === session.sessionId (plumbing already exists).

Finding 3 — Wire-level protocol tests never exercise streamSimple/completeSimple, the entry point every product flow uses (test-reviewer, confidence 85)

All four protocol wire tests in packages/ai/test/opencode-session-headers.test.ts call complete() → stream() (the line 32 injection). streamSimple() (stream.ts:43-51, the line 50 injection) is a separate entry point with zero wire-level coverage — the only existing test driving real streamSimple with a sessionId (openai-completions-kimi.test.ts) uses a Kimi model where the helper is a no-op. Meanwhile the production agent loop uses streamSimple by default (packages/agent/src/agent.ts default streamFn, agent-loop.ts:342), and every auxiliary flow (compaction, branch summary, arbiter, tab title, probes, buddy) calls completeSimple → streamSimple. Deleting withOpenCodeSessionHeader(...) from stream.ts:50 while leaving line 32 keeps the entire suite green: header injection would be dead on the actual product path for the main conversation and all auxiliary requests while all 20 new tests pass. The code is correct today (three independent reviewers verified the call site); this is the highest-value regression guard the suite is missing. Fix is ~4 near-copy wire tests calling completeSimple instead of complete.

Suggestions

Finding 4 — Tab-title mode-level wiring untested in both modes (test-reviewer, confidence 85)

Deleting getSessionId: () => this.session.sessionId at interactive-mode.ts:699 or rpc-mode.ts:1875 keeps the suite green (tab-title tests use hand-built deps). Harm if it regresses: tab-title completions to OpenCode go headerless → rejected after 2026-09-06 → titles fall back to the default/first-word title (cosmetic).

Finding 5 — Buddy hatch/reroll mode-level wiring untested in both modes (test-reviewer, confidence 85)

Dropping the third argument at any of the four sites (interactive-mode.ts:324, 331; rpc-mode.ts:2090, 2108) keeps the suite green — the buddy-manager forwarding tests pass sessionId explicitly to the manager (helper-level only). Harm if it regresses: buddy hatch/reroll on an OpenCode parent model goes headerless → rejected after 2026-09-06 → visible error to the user (cosmetic feature).

Finding 6 — Session ID loaded from a session file flows unsanitized into the header value (error-auditor, confidence 95)

Round-1 finding 1, unchanged in this commit: SessionManager.setSessionFile adopts the file header id verbatim (only a typeof === "string" check, session-manager.ts:736); the value now lands in the x-opencode-session header via withOpenCodeSessionHeader (no value validation). Empirically re-verified: an id containing CRLF/NUL makes undici throw Headers.append: ... is an invalid header value, so resuming a tampered/corrupted session file with an OpenCode model turns every request in that session into a client-side hard error (main conversation, compaction, arbiter, probes all fail; mostly loud, one silent sub-case where leading/trailing whitespace is silently stripped). Trigger is local file tampering only (all legitimate write paths use randomUUID()); fails closed client-side, no injection or cross-session impact; identical risk pattern already ships pre-PR for mistral x-affinity and codex session_id. Round 1 classified this a useful follow-up — reclassified for consistency unless new evidence.

Finding 7 — Buddy soul generation swallows the new failure mode silently (error-auditor, confidence 85)

The pre-existing bare catch in generateSoul (buddy-manager.ts, fallback to species-named soul) has no logging; this PR adds the invalid-header TypeError (finding 6's trigger, OpenCode parent model) to the set of causes that degrade hatch to a default soul with zero signal. Suggested fix: log the caught error before falling back.

Finding 8 — New wiring test duplicates the existing assistantMsg() test utility (simplifier, confidence 88)

agent-session-session-id-wiring.test.ts:42-57 defines a 16-line local mockSummaryResponse that duplicates assistantMsg() from test/utilities.ts (already imported in this file); the flows under test read only stopReason/errorMessage/content, and no assertion touches the response's model/usage/timestamp fields. Replacing with assistantMsg("Mock summary") is behavior-preserving (test file already green).

Finding 9 — Same fixture duplication in branch-summarization.test.ts (simplifier, confidence 88)

Same analysis: the local mockSummaryResponse (lines 33-48) can become assistantMsg("Branch summary"); only the text is asserted.

Nitpick (round-1 item 5, still open) — Unnecessary as TOptions cast (simplifier, confidence 95)

opencode-headers.ts:59: the returned object literal is assignable to TOptions without the cast — re-verified by a scratch tsc --noEmit over the whole packages/ai (exit 0). Zero runtime change.

Nitpick (round-1 item 6, still open) — Unnecessary as StreamOptions cast plus now-unused import (simplifier, confidence 95)

stream.ts:32: withOpenCodeSessionHeader(model, options) infers to a type assignable to the options?: StreamOptions parameter (sibling streamSimple already passes the helper result cast-free); scratch typecheck clean, which also lets the StreamOptions import line be trimmed. −2 lines, zero runtime change.

Strengths

• Round-1 merge blocker genuinely closed: commit 8446436's agent-session-session-id-wiring.test.ts was mutation-checked by two independent agents — deleting this.sessionId at any of the compaction/branch-summary sites (or inside compact()'s fan-out, including the previously untested generateTurnPrefixSummary) fails a specific test; the UUID-shape guard prevents vacuous assertions. Runs green (4/4) at this commit.
• Wire-level test design is the right altitude: stubbing only global.fetch through the real SDK clients and SSE parsers proves the header for all four protocols, mixed-case override (exactly one effective value), non-OpenCode isolation, and no-mutation at once.
• Structural concurrency safety: the helper returns the same object on no-ops and a fresh object when injecting; every wiring site builds fresh option literals — no shared-mutable-options hazard across concurrent sessions.
• Completeness verified, not assumed: full production call-site census across all packages shows every completeSimple/stream/streamSimple entry that can select an OpenCode model now forwards the parent UUID or is intentionally non-OpenCode-only (Ollama buddy reaction path, per the approved plan); the built-in catalogs use exactly the four issue protocols and the dispatch-level injection covers them all; no production code bypasses the instrumented dispatch.
• Idiomatic and minimal: reuses the pre-existing StreamOptions.sessionId field and the conditional-spread pattern already established by the main conversation path; main-conversation wiring confirmed (Agent → loop config → streamSimple), stable across turns/resume, fresh UUID on new/fork (pre-existing SessionManager behavior, verified at the persistence layer).

Agents run: code-reviewer (no candidates; first attempt hit a context limit and was retried once), error-auditor (2), test-reviewer (5), completeness-checker (no candidates), simplifier (4)

---

Reviewed by mach6

[aebrer]

Review Assessment

#501 (comment)

Classifications

Finding	Classification	Reasoning
1 — Subagent tool parentSessionId wiring untested at the AgentSession level	merge blocker	Factual: confirmed — agent-session.ts:3254 exists; no test drives an AgentSession-built subagent tool into a probe; empirically mutation-verified (line deleted → full coding-agent suite 3241 passed / 0 failed). Scope: in — the acceptance criteria name "subagent model probes" explicitly in both the header-presence and test-coverage clauses, and the plan promised final-wiring coverage "rather than testing only helper-level options"; the author's own 8446436 commit established the standard that deleting an AgentSession call-site line must not keep the suite green. Practical: passes — actor: any future commit deleting that one line (suite stays green, verified); trigger: user on OpenCode spawns a subagent with an explicit model spec after 2026-09-06; harm: headerless availability probes rejected → models skipped → wrong-model silent fallback or "None of the fallback models passed availability checks" spawn failure; safeguards: none (empirically green); material outcome of fix: one test reusing the existing in-memory AgentSession harness.
2 — DispatchArbiter getSessionId wiring untested at the AgentSession level	merge blocker	Factual: confirmed — agent-session.ts:469 exists; dispatch-arbiter.test.ts builds the arbiter with hand-made deps and the real-AgentSession tests' fakes discard the options argument where the sessionId lives (verified async (_model, context) at subagent-arbiter.test.ts:572, bare vi.fn() at :661); mutation-verified (line deleted → 3241/0). Scope: in — same criteria clauses; the plan named dispatch arbitration (both attempts) as an aux path. Practical: passes — same deletion trigger; trigger: arbiter-enabled session spawns a subagent on OpenCode after 2026-09-06; harm: both arbiter attempts rejected → inference_failed → "Dispatch arbitration failed" blocks gated spawns; fix is ~3 lines (capture the 3rd argument in the existing injected fake).
3 — Wire-level protocol tests never exercise streamSimple/completeSimple, the entry point every product flow uses	merge blocker (highest priority)	Factual: confirmed — all wire fetch-captures call complete()→stream() (line-32 injection only); streamSimple (line-50 injection) has zero wire coverage; the only real streamSimple+sessionId test uses a Kimi model where the helper is a no-op; mutation-verified (line-50 injection deleted → zero failures in either package); independently verified that production imports only streamSimple from @dreb/ai (agent.ts, agent-loop.ts) — the main conversation loop and every aux flow run the untested path. Scope: in — the plan promised the helper "applied to BOTH stream() and streamSimple() dispatch paths" and promised wire-level header verification; the product path's header presence is currently proven by code reading alone. Practical: passes — one-line deletion ships green (verified) and after 2026-09-06 every OpenCode request including the main conversation goes headerless; this is the broadest regression surface in the set; fix is ~4 near-copy wire tests calling completeSimple instead of complete (existing harness works unchanged).
4 — Tab-title mode-level wiring untested (both modes)	useful follow-up	Factual: confirmed — deleting interactive-mode.ts:699 / rpc-mode.ts:1875 ships green (mutation-verified 3241/0); helper-level forwarding is tested with explicit IDs. Scope: partially in — the criteria name tab-title explicitly, but the plan's final-wiring test promise was limited to "agent-session compaction/branching tests"; forwarding at the tab-title caller is verified (deliverable 3, helper level). Practical: fails the materiality bar — harm is cosmetic (auto tab titles fall back to the default/first-word title; no data loss, no functional break, trivially recoverable); the guard requires new, heavy InteractiveMode/RPC-mode test infrastructure (existing mode tests use prototype-call fakes precisely because full mode construction is expensive), disproportionate to a cosmetic-flow glue line.
5 — Buddy hatch/reroll mode-level wiring untested (both modes)	useful follow-up	Factual: confirmed — dropping the 3rd arg at interactive-mode.ts:324,331 / rpc-mode.ts:2090,2108 ships green (mutation-verified 3241/0); manager-level forwarding tested with explicit IDs. Scope: partially in — same basis as finding 4. Practical: fails the materiality bar — hatch/reroll failure degrades a cosmetic companion feature (visible error or fallback to default soul, compounding with finding 7); same infrastructure-cost asymmetry as finding 4. Both 4/5 stay tracked so the guard is not lost, without blocking the merge for cosmetic-flow glue lines when the functional flows (1/2/3) are guarded.
6 — Session ID loaded from a session file flows unsanitized into the header value	useful follow-up	Factual: confirmed — setSessionFile adopts the file id verbatim (only a typeof === "string" check); no value validation at injection; CRLF/NUL ids empirically make undici throw before the request leaves. Scope: out — no criterion or plan item requires id sanitization; the validation gap predates this PR (identical pattern already ships for mistral x-affinity and codex session_id); the PR widens an existing gap by one provider surface. Practical: fails as blocker — trigger is local tampering/corruption of the user's own session file (all legitimate write paths use randomUUID()); fails closed client-side (no injection, no cross-session impact); self-inflicted DoS on an already-corrupted session. Track: validate/normalize the id in setSessionFile or in the header helper.
7 — Buddy soul generation swallows the new failure mode silently	useful follow-up (low priority)	Factual: confirmed — pre-existing bare catch in generateSoul (verified present pre-PR); the PR adds exactly one more swallowed cause (invalid-header TypeError from finding 6's trigger, OpenCode parent model only). Scope: out — no criterion or plan item requires logging. Practical: fails — marginal diagnosability loss on a cosmetic feature. Track: one-line console.warn in the catch.
8 — Wiring test duplicates the existing assistantMsg() utility	nitpick	Factual: confirmed — 16-line local mockSummaryResponse duplicates assistantMsg() already imported in the same file; drop-in replacement, behavior-preserving. Scope: out. Practical: fails — zero runtime impact.
9 — Same fixture duplication in branch-summarization.test.ts	nitpick	Factual: confirmed — same drop-in applies; only the text is asserted. Scope: out. Practical: fails — zero runtime impact.
10 — Unnecessary as TOptions cast (round-1 item 5)	nitpick	Factual: confirmed — scratch tsgo --noEmit over all of packages/ai exits 0 with the cast removed; cast erased at compile time. Scope: out. Practical: fails — zero runtime change.
11 — Unnecessary as StreamOptions cast + now-unused import (round-1 item 6)	nitpick	Factual: confirmed — typecheck clean with cast removed; the StreamOptions import is used only by that cast (grep-verified), so the import line orphans. Scope: out. Practical: fails — zero runtime change (−2 lines).

Action Plan

1. Finding 3 — Add wire-level coverage for the simple-stream dispatch path: in packages/ai/test/opencode-session-headers.test.ts, mirror the existing four protocol wire tests but call completeSimple() instead of complete() (the harness — stubbed global.fetch, SSE fixtures, makeModel — works unchanged). This is the path the main conversation loop (agent.ts default streamFn, agent-loop.ts:342) and every auxiliary flow actually use.
2. Finding 1 — In packages/coding-agent/test/agent-session-session-id-wiring.test.ts (or a sibling using the same in-memory AgentSession harness), drive the AgentSession-built subagent tool with an explicit model spec and assert the spawn-time availability probe's completeSimple options carry session.sessionId — guarding agent-session.ts:3254.
3. Finding 2 — In the existing real-AgentSession tests in subagent-arbiter.test.ts, make the injected dispatchArbiterComplete fake capture the 3rd argument (options) and assert options.sessionId === session.sessionId — guarding agent-session.ts:469.

Non-blocking follow-ups, in priority order: (4) tab-title mode-level wiring tests; (5) buddy mode-level wiring tests; (6) session-file id validation in setSessionFile/header helper; (7) log in generateSoul's catch. Nitpicks 8-11 (fixture dedup via assistantMsg(), two cast/import cleanups) can ride along with the action-plan commit at zero risk.

---

Assessment by mach6

[aebrer]

Round-2 merge blockers resolved in 51109c3 — findings 1, 2, 3 (test-coverage gaps; all were "suite stays green when the wiring line is deleted" class).

Finding 1 — subagent spawn probe session ID
test/agent-session-session-id-wiring.test.ts: new test drives a real subagent spawn (fake child process, no real CLI). The probe agent file declares a two-model list (claude-sonnet-5, claude-opus-5), which forces the runtime availability-probe branch that a single model silently skips. Asserts the probe LLM call (identified by its "Reply with the single word OK." system prompt — so it can't be mistaken for a summary call) carries sessionId === session.sessionId, and that the spawn actually happened.

Finding 2 — dispatch arbiter session ID
test/subagent-arbiter.test.ts: the existing real-AgentSession test now captures the injected dispatchArbiterComplete options and asserts every arbiter LLM call (2 of them, one per task) carries sessionId === session.sessionId.

Finding 3 — wire tests never hit the production entry point
packages/ai/test/opencode-session-headers.test.ts: the four protocol wire tests now run again through completeSimple (→ streamSimple, where production imports land — agent.ts / agent-loop.ts). 20 → 24 tests.

Verification

• Mutation check on all three: deleting the wiring line (agent-session.ts subagent parentSessionId option / arbiter getSessionId dep; stream.ts streamSimple injection) makes the corresponding new test fail; the other suites stay green — i.e. each new test guards exactly its path.
• Full suites: packages/ai 390 passed / 0 failed, packages/coding-agent 3242 passed / 0 failed; tsgo --noEmit and biome check clean (pre-commit hook gate also passed: 6010 tests, 0 failed).

Findings 4–7 remain as follow-ups (tab-title, buddy, unsanitized session-file id, generateSoul catch); findings 8–11 are nitpicks left for author preference.