Skip to content

fix(opencode): synthesize x-opencode-session when client sends none — 2026-09-06 upstream enforcement - #12719

Closed
Moseyuh333 wants to merge 2 commits into
diegosouzapw:release/v3.8.51from
Moseyuh333:fix/opencode-session-09-06-3851
Closed

Moseyuh333 wants to merge 2 commits into
diegosouzapw:release/v3.8.51from
Moseyuh333:fix/opencode-session-09-06-3851

Conversation

@Moseyuh333

@Moseyuh333 Moseyuh333 commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Rescoped after review (commit 46eb14bd4). Under the default configuration, applyCliDefaults() (step 4, #10571) already backfills x-opencode-session with ||=, so the default path is covered — thanks for the careful audit. The reproducible gap this PR actually closes is the opt-out path: with OPENCODE_SYNTHESIZE_CLI_HEADERS=false, step 4 never runs and, before this patch, step 3 did not synthesize either — an OpencodeExecutor request whose client sent no session-ish header went outbound without x-opencode-session at all.

  • What: forwardOpencodeClientHeaders() step 3 (OpencodeExecutor path only) now synthesizes x-opencode-session when the client provides none of x-opencode-session / x-session-affinity / x-session-id: a conversation-stable fingerprint via generateSessionId(sessionBody) (model, system prompt, first user message, tools), falling back to a random UUID when no body is available. +8/-3; explicit client sessions still win and forward untouched.
  • Why merge: (1) opencode.ai errs on requests missing x-opencode-session since 2026-09-06 — the opt-out path trips exactly that check. (2) The opt-out is a documented escape hatch (fix(opencode): session stability, free-tier routing, and CLI defaults #10571 flipped the default on; OPENCODE_SYNTHESIZE_CLI_HEADERS=false lets operators who don't want fabricated CLI identity still customize env defaults), and those operators still need a session header for the enforcement and upstream prompt caching. (3) It future-proofs step 3 for any path where cliDefaults is absent.
  • Scope: step-3 only. CLI-identity synthesis (fix(providers): opencode-go — inject OpenCode CLI headers on VPS #5997/fix(opencode): session stability, free-tier routing, and CLI defaults #10571) unchanged; DefaultExecutor pass-through untouched; the body plumbing (buildHeaders(..., body), sessionBody) and the Muse responses UUID guard already exist on the base — this closes the last missing branch.

Related Issues

Validation

Choose the change type and focused loop from the
Contribution Golden Path. The full unit suite,
Vitest, the 60% coverage gate, and the production build all run in CI on this PR (#8329):

  • Change type: provider / sse
  • Focused tests and category gates from the golden path
  • npm run lint — not runnable in this contributor environment (local node_modules/eslint fails to load, pre-existing, unrelated to this diff; CI runs the real lint gate)
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR
  • SonarQube is temporarily opt-in while the private project has no quota; it is not a PR gate.

Isolation proof — current tip 152d95108, unpatched vs patched

# Unpatched tip, new test files only:
DISABLE_SQLITE_AUTO_BACKUP=true node --import tsx/esm \
  --import ./open-sse/utils/setupPolyfill.ts \
  --import ./tests/_setup/isolateDataDir.ts \
  --test tests/unit/runtime/opencode-session-headers.audit.test.ts
# > tests 11 | pass 10 | fail 1
#   ✖ opt-out (OPENCODE_SYNTHESIZE_CLI_HEADERS=false): step 3 alone still fills x-opencode-session

# Same file set with this PR's opencodeHeaders.ts applied:
#   audit + e2e: > tests 14 | pass 14 | fail 0
  • Also verified on the branch's own base c3945a724 (identical opencodeHeaders.ts to the tip): unpatched 10/11 — only the new test red — patched 11/11.
  • tests/unit/opencode-executor.test.ts + tests/unit/opencode-cli-headers-synthesis-5997.test.ts on the branch: 66/66 pass.
  • npx prettier --check tests/unit/runtime/opencode-session-headers.audit.test.ts: clean.
  • Reconciliation: all four files this PR touches are identical between the branch base and the current tip — the diff applies cleanly.

Tests Added Or Updated

  • tests/unit/runtime/opencode-session-headers.audit.test.ts — new file, 11 tests, including the opt-out isolation test that fails on the unpatched tip (verified) and passes with the patch.
  • tests/unit/runtime/opencode-session-e2e.test.ts — new file, 3 tests (echo-server e2e through OpencodeExecutor.execute()).
  • tests/unit/opencode-executor.test.ts — 3 subtests updated to pin the new behavior (synthesize when the client sends no matching keys).

Coverage Notes

  • open-sse/utils/opencodeHeaders.ts step-3 behavior is covered by the audit file; the outbound wiring by the e2e file; the executor-level path by the updated subtests. No coverage movement expected.

Reviewer Notes

  • Direct answer to the review: correct — in the default configuration step 4 already backfills the header, and none of the original tests could observe the step-3 gap. Took option (a): the description is now scoped to the opt-out edge case, and the new test
    opt-out (OPENCODE_SYNTHESIZE_CLI_HEADERS=false): step 3 alone still fills x-opencode-session
    isolates it — red on the unpatched tip (10/11), green with the patch (11/11; 14/14 with e2e). Happy to re-review.
  • No feature flag, no migration.

… 2026-09-06 upstream enforcement

Starting 2026-09-06 opencode.ai errors on requests missing
x-opencode-session. Requests sent through OmniRoute without any
session-ish client header produced NO session header at all
(only x-session-affinity/x-session-id were mapped), which trips the
new upstream check and loses upstream prompt caching.

forwardOpencodeClientHeaders() step-3 now synthesizes, for
OpencodeExecutor only, when no session was provided:
- a conversation-stable fingerprint via generateSessionId(sessionBody)
  (model, system prompt, first user message, tools) so consecutive
  agent turns share one session -> upstream prompt cache hits;
- a random UUID when no body is available (same behavior as the diegosouzapw#5997
  CLI-identity synth).

Explicit client sessions still win (forwarded untouched); the
OPENCODE_SYNTHESIZE_CLI_HEADERS opt-in path is unchanged. Plumbing
(buildHeaders body param, Muse responses UUID guard) already exists in
this release; this closes the last missing branch.

Tests: unit (opencode-executor, refactor-buildHeaders, 5997 synthesis)
+ new audit suite (10) + end-to-end execute() suite (3) through a real
local fetch target: 97/97 pass. Client session is forwarded unchanged;
same conversation prefix keeps one synthesized session across turns.
Copilot AI lite review requested due to automatic review settings September 4, 2026 08:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A newly added unit test currently passes the request body in the wrong buildHeaders() parameter position, so it can “pass” without actually exercising the fingerprint-based session synthesis logic.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Ensures the OpencodeExecutor path always sends x-opencode-session upstream (synthesizing a deterministic conversation fingerprint when the client provides no session-ish header), to comply with the announced 2026-09-06 opencode.ai enforcement and preserve prompt-cache affinity.

Changes:

  • Update forwardOpencodeClientHeaders() to synthesize x-opencode-session when synthesizeRequestId is enabled and neither a direct session header nor an affinity/session-id fallback is present.
  • Add new unit + execute()-level tests to pin the new outbound header contract (synthesized session/request IDs, stability across turns, and Muse Responses UUID guard).
  • Update existing OpencodeExecutor tests to reflect the new “always send session” behavior when the client provides none.
File summaries
File Description
open-sse/utils/opencodeHeaders.ts Add synthesis branch to guarantee x-opencode-session (fingerprint w/ body, UUID fallback w/o body).
tests/unit/runtime/opencode-session-headers.audit.test.ts New audit-style unit tests for header forwarding/mapping/synthesis + stability across turns.
tests/unit/runtime/opencode-session-e2e.test.ts New local-echo execute()-level tests verifying outbound wire headers.
tests/unit/opencode-executor.test.ts Update existing suite expectations to match the new synthesis contract.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +103 to +111
const h1 = executor.buildHeaders({ accessToken: "k" } as any, true, {}, "m", {
model: "m",
messages: [{ role: "user", content: "conversation A" }],
}) as Record<string, string>;
const h2 = executor.buildHeaders({ accessToken: "k" } as any, true, {}, "m", {
model: "m",
messages: [{ role: "user", content: "conversation B" }],
}) as Record<string, string>;
assert.notEqual(h1["x-opencode-session"], h2["x-opencode-session"]);
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for the thorough test coverage here — 13 well-written tests. But I need to flag
something before this can merge: I dropped your own new test files onto the unpatched
release/v3.8.51 tip (no code changes from this PR applied) and all 13 passed. That's because
OpencodeExecutor.buildHeaders() already runs applyCliDefaults() unconditionally (step 4,
from #10571) whenever CLI-identity synthesis is enabled — which it is by default — and that
step already backfills x-opencode-session with ||= regardless of what step 3 (the code this
PR touches) does. So under the default configuration, the gap you're describing doesn't
currently exist on the tip.

The one place your patch does change behavior is when an operator sets
OPENCODE_SYNTHESIZE_CLI_HEADERS=false (disabling CLI-identity synthesis) — in that case step 4
never runs, and your fix to step 3 would be the only thing filling the gap. None of your 13
tests isolate that specific scenario though. Could you either (a) rescope the PR description to
that opt-out edge case specifically and add a test for it, or (b) let us know if I'm missing
something about how this reaches the default path? Happy to re-review either way.

Triage note: this is the review recommendation — the close itself happens only after the maintainer's per-PR sign-off (and, where a superseding PR is named, after it has landed). Nothing is being closed by this comment.

…e only filler

Review follow-up (PR diegosouzapw#12719): under the default configuration applyCliDefaults
(step 4, diegosouzapw#10571) backfills x-opencode-session, so the step-3 gap this PR fixes
is only observable when an operator sets OPENCODE_SYNTHESIZE_CLI_HEADERS=false
— step 4 never runs there and step 3 is the only filler left.

The new test isolates exactly that scenario and fails on the unpatched tip
(verified: 10/11 pass there, only this test red), proving it pins the fix.
@Moseyuh333

Copy link
Copy Markdown
Contributor Author

Took option (a) — thanks for the precise audit; you're right that step 4's ||= backfill covers the default path and none of the original tests could observe the step-3 gap.

Changes since your review (commit 46eb14b):

  • PR description rescoped to the opt-out edge case (OPENCODE_SYNTHESIZE_CLI_HEADERS=false, where step 4 never runs).
  • Added an isolation test: opt-out (OPENCODE_SYNTHESIZE_CLI_HEADERS=false): step 3 alone still fills x-opencode-session — it also asserts x-opencode-client/x-opencode-project stay unset, proving it exercised step 3 only.

Isolation proof, run on the current tip 152d951 with the new test files dropped on unpatched vs patched:

  • unpatched: 10/11 — only the new test red
  • patched: 14/14 (audit 11 + e2e 3)

Ready for re-review.

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.

3 participants