Skip to content

Send x-opencode-session on OpenCode requests - #501

Open
aebrer wants to merge 3 commits into
masterfrom
feature/issue-500-opencode-session-header
Open

Send x-opencode-session on OpenCode requests#501
aebrer wants to merge 3 commits into
masterfrom
feature/issue-500-opencode-session-header

Conversation

@aebrer

@aebrer aebrer commented Sep 3, 2026

Copy link
Copy Markdown
Owner

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

aebrer commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

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

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Vitest coverage

Metric Covered Total Coverage
Statements 41082 56768 72.36%
Branches 22318 35765 62.4%
Functions 8713 11908 73.16%
Lines 29655 40807 72.67%

View full coverage run

@aebrer

aebrer commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

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
aebrer marked this pull request as ready for review September 3, 2026 19:54
@aebrer

aebrer commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

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: silentgenerateSoul'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:32withOpenCodeSessionHeader(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

aebrer commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

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

aebrer commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

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

aebrer commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

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 completeSimplestreamSimple. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Send x-opencode-session on OpenCode requests

1 participant