Skip to content

fix(opencode): keep the target format and client session per request, not on the shared executor - #14149

Merged
diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.51from
maxmad64bis:fix/opencode-request-format-race
Sep 22, 2026
Merged

diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.51from
maxmad64bis:fix/opencode-request-format-race

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #14148. Only the last commit is this PR's; I'll rebase once it lands.

⚠️ base-red inherited: #13866

Summary

Default change: none. Behavior change: a JSON caller of a Responses model no longer gets the raw event stream back when another request finishes while it waits. The shared executor kept target format and client session as instance fields written at request start and read after later awaits, so a slow Responses JSON request got the event stream (event: response.completed …) once a Chat request finished first; both values now live in a per-execute() context, with the public fields unchanged outside execute().

Related Issues

Validation

  • Change type: provider
  • Focused tests and category gates from the golden path
  • npm run lint — ESLint on the touched files is clean; the full run is red on the base (5 errors in files this PR doesn't touch)
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/unit/opencode-request-format-race.test.ts (new, 4 cases): slow Responses JSON caller with a Chat request finishing meanwhile (fails before the change), two in flight each keeping session and format, a request ending not clearing what another still uses, plain fields unchanged outside execute().

Coverage Notes

  • opencodeRequestContext.ts (new) and the accessors in opencode.ts: mutating the code (format or session always in the shared field, execute without a context) makes the named tests fail. The 41 test files that touch the executor pass unchanged. No coverage moved down.

Reviewer Notes

  • It depends on fix(opencode): replay a request in the other tool shape when its tools are refused #14148: the wrapper it hooks into, withRequestShapeRetry around executeOnce, is introduced there. The public _requestFormat and _clientSession stay as accessors: inside execute() they use the request's own context, outside it plain fields, keeping the ten existing tests that assign them unchanged.
  • The Muse Spark output-budget helpers move to opencodeMuseSpark.ts unchanged and re-exported, taking opencode.ts from 1195 to 1125 lines; isResponsesTerminalLine is now exported and reportedCompletionTokens leaves normalizeMuseSparkFinishReason, putting its complexity under the limit. Not changed here: the account list and cursor are also rebuilt per request on the shared instance (same class of problem), and reads of _requestFormat after a request ended still see the cleared value, as before.
  • CI reds are inherited from the red base (🔴 Release branch not green: release/v3.8.51 #13866), not from this diff: the third-party PR fix(sse): treat antigravity empty completions with a normal stop as valid 200s (#14160) #14243 on the same base release/v3.8.51 fails the same 9 jobs (API Route Typecheck, Docs Gates, Fast Quality Gates, Merge integrity, ESLint, Unit fast-path 1-4/4); every file cited by the failing gates is outside this diff. Non-blocking for this PR.

@maxmad64bis
maxmad64bis force-pushed the fix/opencode-request-format-race branch from 76bbed3 to 0834882 Compare September 18, 2026 22:04
@maxmad64bis
maxmad64bis marked this pull request as ready for review September 18, 2026 22:04
@maxmad64bis
maxmad64bis force-pushed the fix/opencode-request-format-race branch 2 times, most recently from 295287d to f5c7941 Compare September 18, 2026 22:17
Max added 2 commits September 20, 2026 11:13
…s are refused

Tool-shape refusal replays once in the other shape and remembers the working shape per prompt digest.
… not on the shared executor

Request format and client session now live in a per-request context instead of shared executor fields.
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @maxmad64bis — merging via the release merge-train. Validated in local merge-train (mt-train10c) on the devbox @ train tip 4d841aa1c740bbaa03868dc0a403c62099a99a42 with the 72 sibling PRs of this batch: typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity green; changed-area node:test 831/831 (0 failing) and vitest 480/482 — the two reds are autoCombo/provider-family-combos.test.ts timing out at 20s, which reproduces on the PURE release tip under the full vitest suite (and is already tracked by the Release-Green issue #13866), so it is inherited, not this batch's. Merged --admin per merge-gates §3/§4/§7.

The diegosouzapw#14148 commit this branch carries is already on the release tip under a
different SHA (771c376), so every conflict was the same content on both
sides: keep the tip's copy of the shape-retry module, the free-tier contract,
their tests and the 14148 changelog entry, and re-apply this branch's own
delta (per-request format/session context, muse-spark helpers extracted)
on top.
@diegosouzapw
diegosouzapw merged commit 59de50e into diegosouzapw:release/v3.8.51 Sep 22, 2026
9 of 16 checks passed
diegosouzapw pushed a commit that referenced this pull request Sep 22, 2026
Merged. Thank you, @maxmad64bis — this and #14149 are the same class of bug caught twice, and both were worth catching.

Two requests sharing one executor walking one shared member list means one request's re-sync can replace the other's picks — silently, and only under concurrency, which is exactly why it never shows up in a single-request test. Keying the list on request body identity while keeping the shared pick index (clamped to the local list) preserves the dispatch order for a single request, so the fix costs nothing in the common case. Per-member health in a named store with write-back is the right place for the state that genuinely should outlive the request.

Validation before merge: re-reconciled after #14149 landed, which is where the real work was. #14149 had extracted the MuseSpark block out of `opencode.ts` into its own module and wrapped `execute()` in `runInRequestContext(...)`, while #14464 had added the free-tier completion-retry — so `execute()` had to carry three things at once. Final shape:

```ts
async execute(input: ExecuteInput) {
  try {
    return await runInRequestContext(() =>
      withRequestShapeRetry(input, (i) => this.executeOnce(i))
    );
  } finally {
    releaseRequestList(input.body, this.accountHealth);
  }
}
```

The `finally` is outermost on purpose: it runs once, after every shape replay and after the request context exits, against the same `input.body` the list was keyed on.

I checked the things a clean-looking merge can quietly break here: MuseSpark is not duplicated back inline (it is only imported and re-exported), #14149's `_requestFormat`/`_clientSession` accessors and context wrap are intact, and #14464's four wiring points are byte-identical to the tip. The diff against the tip contains only your delta — no tip line reverted.

`opencode-accounts-per-request` 5/5, plus the sibling suites that would catch a botched resolution: `opencode-request-format-race` 4/4, `opencode-request-shape-retry` 34/34, `opencode-free-tier-refusal-rotation` 9/9, `opencode-free-tier-request-contract` 38/38 — **90 pass / 0 fail**. `typecheck:core` clean, `check:file-size` OK (`opencode.ts` at 1224 against the 1303 ceiling, no rebaseline needed), `check:changelog-integrity` OK.

Note: the release tip is currently base-red on `open-sse/executors/auggie.ts` (#14496). Inherited, unrelated to this diff.
diegosouzapw added a commit to maxmad64bis/OmniRoute that referenced this pull request Sep 22, 2026
…he reconciled tip

opencode.ts 1386->1324 and chat.ts 2561->2559 were measured before diegosouzapw#14149
moved the MuseSpark block out of the executor and before Prettier reflowed
chat.ts; pin both to the gate's own count on this tree.
@maxmad64bis
maxmad64bis deleted the fix/opencode-request-format-race branch September 24, 2026 21:14
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.

2 participants