🤖 fix: send GPT-6 Sol/Luna reasoning effort none and repair main CI - #4350
Conversation
--- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$26.31`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=26.31 -->
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a3d24a0a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replace the bun patch (not applied to npm installs) with a post-SDK body rewrite.
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fd72e8c35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Coordination note from #4259 (same defect, different history):
No action needed from this PR; just flagging so we don't both edit the same call sites. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
… correct their Codex context cap Stacked on #4350, which makes a requested reasoning_effort "none" reach the wire for GPT-6 Sol/Luna. Headless tool loops (Dream consolidation, memory harvest, refine, sidebar status) call streamText with tools and no provider options, so nothing requests "none" and Chat Completions defaults to medium, which rejects function calling. A model-boundary middleware fills in "none" for tool-bearing Chat Completions requests that asked for no effort, wrapped outside #4350's body-patching wrapper so the injected value survives the SDK; explicit caller efforts are left alone. The Coder openai-compat path resolves scoped "Treat as" aliases through resolveModelForMetadata before clamping. The assumed 372K Codex OAuth cap for Sol/Luna becomes the 272K published in the pinned Codex catalog. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$174.60`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=174.60 -->
|
Thanks @ThomasK33, acknowledged. This PR won't touch the headless tool-loop clamp, the Coder |
|
Status for whoever queues this: |
… correct their Codex context cap Stacked on #4350, which makes a requested reasoning_effort "none" reach the wire for GPT-6 Sol/Luna. Headless tool loops (Dream consolidation, memory harvest, refine, sidebar status) call streamText with tools and no provider options, so nothing requests "none" and Chat Completions defaults to medium, which rejects function calling. A model-boundary middleware fills in "none" for tool-bearing Chat Completions requests that asked for no effort, wrapped outside #4350's body-patching wrapper so the injected value survives the SDK; explicit caller efforts are left alone. The Coder openai-compat path resolves scoped "Treat as" aliases through resolveModelForMetadata before clamping. The assumed 372K Codex OAuth cap for Sol/Luna becomes the 272K published in the pinned Codex catalog. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$174.60`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=174.60 -->
## Summary Bumps `packages/mux-compat/package.json` (`version` and the `@coder/xum` dependency pin) from 0.29.0 to 0.30.0 to match the root package after the `release: v0.30.0` commit (`81b0b744`), which bumped only the root `package.json`. ## Background `src/common/compat/productIdentity.test.ts` › "keeps the published mux forwarding package version-locked to @coder/xum" asserts `legacyPackageJson.version === packageJson.version`. Since the release commit it fails on `main` ([run 35783501719](https://github.com/coder/xum/actions/runs/35783501719)) and on every PR's merge commit (e.g. coder#4259 [run 35785546631](https://github.com/coder/xum/actions/runs/35785546631), coder#4350), so `Required` is red repo-wide and the merge queue cannot admit anything. Same fix as coder#4048 for v0.28.3; release commits since (e.g. v0.28.5, coder#4175) bumped both files together. `@coder/xum@0.30.0` is already published (Publish to NPM run 35783501943), so the new pin resolves. ## Validation `bun test src/common/compat/productIdentity.test.ts` on this branch: 8 pass (the version-lock case fails on `main` without it). --- _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$174.60`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=174.60 -->
… correct their Codex context cap Stacked on #4350, which makes a requested reasoning_effort "none" reach the wire for GPT-6 Sol/Luna. Headless tool loops (Dream consolidation, memory harvest, refine, sidebar status) call streamText with tools and no provider options, so nothing requests "none" and Chat Completions defaults to medium, which rejects function calling. A model-boundary middleware fills in "none" for tool-bearing Chat Completions requests that asked for no effort, wrapped outside #4350's body-patching wrapper so the injected value survives the SDK; explicit caller efforts are left alone. The Coder openai-compat path resolves scoped "Treat as" aliases through resolveModelForMetadata before clamping. The assumed 372K Codex OAuth cap for Sol/Luna becomes the 272K published in the pinned Codex catalog. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$174.60`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=174.60 -->
… correct their Codex context cap (coder#4259) ## Summary Follow-up to coder#4350 (landed as `7358fa80`), which makes a *requested* `reasoning_effort: "none"` reach the wire for GPT-6 Sol/Luna. This PR closes the three gaps left after coder#4348 and coder#4350: headless tool loops that request no effort at all, Coder-scoped aliases on `openai-compat` instances, and the assumed 372K Codex OAuth context cap (272K in the pinned Codex catalog). ## Background - Sol/Luna Chat Completions accepts function calling only with `reasoning_effort: "none"`; an omitted effort defaults to `medium`. `buildProviderOptions` clamps agent turns (coder#4348) and coder#4350 makes that clamp serialize, but Dream consolidation, memory harvest, refine and sidebar-status generation call `streamText` with tools and **no provider options**, so nothing requests `none` and their tool calls are rejected on Chat Completions routes (direct `wireFormat: chatCompletions`, Coder `openai-compat` instances). Raised by Codex on the earlier revision of this PR; the Coder alias case is its round-6 finding. - Codex catalog pin: [openai/codex@04fc75ad `codex-rs/models-manager/models.json`](https://github.com/openai/codex/blob/04fc75adbe67a612a1cb0fc469533f24b24fa499/codex-rs/models-manager/models.json) — `gpt-6-sol` / `gpt-6-luna` `context_window: 272000`. `main` copies Astra's 372K with an "assumed" comment. ## Implementation - `clampGpt6ChatCompletionsToolReasoning` (providerModelFactory.ts): a `transformParams` middleware that sets `reasoningEffort: "none"` when the request carries tools **and no effort was requested**, for Sol/Luna capability identities on Chat Completions. Explicit caller efforts are preserved (this keeps coder#4350's "requested `high` stays `high`" contract intact). It wraps *outside* `createOpenAIModelWithPreservedOptions` so the injected `none` is seen by that wrapper's body patch instead of being stripped by the SDK. - Applied on the direct OpenAI path (Chat Completions wire only) and the Coder gateway Chat path, where the capability identity is `resolveModelForMetadata(coder:<instance>/<model>)` so scoped "Treat as" aliases are clamped too. - `CODEX_OAUTH_CONTEXT_WINDOW_OVERRIDES`: `gpt-6-sol` / `gpt-6-luna` 372K → 272K with the pin cited inline; Astra and the GPT-5.6 family are unchanged (coder#4347). Dropped from earlier revisions: catalog/aliases/pricing/labels/migration/docs (landed in coder#4348), the `@ai-sdk/openai` Bun patch (Bun `patchedDependencies` never reach npm installs of `@coder/xum`; coder#4350's runtime rewrite is the right fix), and the `hasTools` plumbing through `buildProviderOptions`. ## Validation Repo-pinned Bun 1.3.5, on `main` + this commit with a freshly reinstalled, unpatched `@ai-sdk/openai` (so a green body assertion proves the composition with coder#4350's wrapper). - `providerModelFactory.test.ts` › "clamps %s tool requests … at the model boundary" (Sol raw id, mapped `team-luna`, Astra control, Responses control) and › "clamps %s tool requests through a Coder openai-compat instance" (`coder:chat-proxy/team-luna` mapped, `coder:chat-proxy/gpt-6-sol` raw, `team-astra` control). Red proof: with the clamp disabled, exactly the four Sol/Luna Chat Completions cases fail (`Expected "none" / Received undefined`); coder#4350's 11 wire cases are unaffected. Reverting the Coder capability argument to the raw `originModelId` fails exactly the mapped `team-luna` case. - `codexOAuth.test.ts` asserts Astra (372K) and Sol/Luna (272K) separately. `contextLimit`, `tokenMeterUtils`, `thinking`, `turnRequestBuilder`, `providerOptions` suites pass. `make static-check` green. ## Risks - The middleware fires only for Sol/Luna capability identities, Chat Completions wire, a non-empty `tools` array, and no requested effort. Responses, tool-free requests, explicit efforts, Astra, GPT-5.6 and non-OpenAI providers are untouched. - Lower Codex OAuth cap (272K) starts limit-driven compaction earlier for OAuth-routed Sol/Luna; API-key routes keep the public limits. ## Readiness and follow-ups - Originally stacked on coder#4350; rebased onto `main` after coder#4350 landed (`7358fa80`). The residual diff is unchanged (+183/−21). - `main` unblock for `Test / Unit` (release commit left `packages/mux-compat` at 0.29.0): coder#4354 standalone, also folded into coder#4350; whichever becomes empty is closed. - Round-6 security finding (budget accounting ignores priority-tier pricing) is pre-existing/generic → coder#4352. coder#4347 tracks GPT-5.6 Sol promotional pricing and Astra's Codex cap. <details> <summary>Preserved review and delivery record — fifteen Codex executions including this head, one code-advisor pass</summary> | Round | Reviewed head (before rebase / re-scope) | Executions | Findings / disposition | | --- | --- | --- | --- | | 1 | `01ca053` | Codex code + security | Hidden-model compaction exposure and zero pricing bypassing CLI budget; fixed in `252bba9` / `cb5ac4b`, replied and resolved | | 2 | `cb5ac4b` | Codex code + security | Migration-marker flip and hiding prior opt-ins; fixed in `64169a3`, replied and resolved | | 3 | `64169a3` | Codex code + security | Clean completion on both loops | | 4, renewed authorization | `40e4b2a6` | Automatic Codex code + security on undraft | Security clean; code P2 Chat Completions default-effort issue → fixed in `ce20ecd0`, replied and resolved | | 5, explicit "get it merged" authorization | `bfc0906f` (superseding `ce20ecd0`) | Automatic Codex code review on undraft | P2: headless tool loops bypass provider options → fixed in `a4c7c156`, replied and resolved | | 6 | `a4c7c156` | Explicit `@codex review` (code + security) | Code P2: Coder aliases not resolved before clamping → fixed, replied and resolved. Security P2: priority-tier budget accounting → pre-existing/generic, coder#4352, replied and resolved | | 7 | `2c4803c0` (residual on `main` after coder#4348) | Explicit `@codex review` (code + security) | Both clean ("Didn't find any major issues" / "No security issues") | | 8 | `c9a0f572` (stacked on coder#4350; rebased onto `main` without changes) | Explicit `@codex review` (code + security) | Both clean ("Didn't find any major issues" 21:57Z / "No security issues" 21:59Z, 👍) | | 9 | `76bff932` (rebased onto `main` after coder#4350 landed) | Explicit `@codex review` (code + security) | Both clean; merge blocked by the repo-wide `compaction1MRetry` content-filter failure (coder#4398), fixed by coder#4401 | | 10 | `bde7c0fe` (rebased onto `main` after coder#4401 landed; diff unchanged) | Explicit `@codex review` (code + security) | Security clean. Code P3: stale Astra comment claimed the catalog couples Astra and Sol → fixed in `7deeea6e`, replied and resolved | | 11 | this head (`7deeea6e`, comment-only fix) | Explicit `@codex review` (code + security) | Pending | | Advisory | — | One independent advisor | Recommended scope reduction and accuracy fixes before official release. Scope reduction realised by coder#4348 and coder#4350 landing first; this PR is the residual. | Post-cap work, retained in order: `f04892f`→`fcf5d0f` budget-comment accuracy; `782cca6`→`04bad77` maintainer-authorized provisional estimate; `d24d776` rebase integration; `870259a` official Sol launch + authorized Luna expansion; `40e4b2a6` verified Codex defaults, labels, route/pricing regressions, docs; `ce20ecd0` tool-aware `none` clamp + SDK patch; `bfc0906f` Nix hash refresh; `a4c7c156` model-boundary clamp; `2c4803c0` residual on `main` (SDK patch + clamp + Coder alias + Codex cap). Other rebase mappings: `01ca053`→`c47ba9c`, `252bba9`→`5363ca0`, `cb5ac4b`→`a866d96`, `64169a3`→`c0d5def`. The cap did not reset on rebase, handoff, scope expansion or re-scope. Backups of superseded heads: `a4c7c15635f3a865baa4b937618da17459b48209`, `2c4803c0c9481924ad682f969542a7a6db71e9e7`. Historical stop reason: implementation and validation were complete for both models, but the final head lacked review coverage under the exhausted cap. The later direct conditional merge instruction reopened delivery for one automatic final-head pair; its findings were then fixed under the user's explicit "get it merged" authorization through the normal gated merge path. When coder#4348 and then coder#4350 landed the same work first, this PR was reduced to the residual fixes above rather than closed. </details> --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh` • Cost: `$281.40`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh costs=281.40 -->
Summary
Follow-up to #4348. GPT-6 Sol/Luna now actually send
reasoning_effort: "none". The PR also fixes two post-mergeTest / Unitfailures onmain: a unit test file that crashed Bun, and the v0.30.0 version-lock contract.Background
none:@ai-sdk/openai@4.0.71, including the latest 4.0.72, allows onlylow/medium/high/xhigh/maxfor everygpt-6-*ID and silently strips any other effort from Chat Completions and Responses requests. That list is correct for Astra, but Sol and Luna acceptnone. OpenAI documents that an omitted effort defaults tomediumand that Chat Completions function calling requiresnone. So after 🤖 feat: add GPT-6 Sol and Luna model support #4348:gpt,solandlunahad their tool calls rejected.buildProviderOptionsoutput, not the serialized request, so they missed this.Test / Unitjob inmainrun 35782976621 exited with code 133. Bun hit an allocator panic (pas panic: deallocation did fail) just asmcpIconDecodeClient.test.tsstarted, with no failing assertions. That file loads the nativesharpaddon into the shared coverage process. The crash is not caused by 🤖 feat: add GPT-6 Sol and Luna model support #4348's model changes.release: v0.30.0(81b0b74) bumpedpackage.jsonbut notpackages/mux-compat.productIdentity.test.ts, which requires the legacymuxforwarding package to stay version-locked to@coder/xum, therefore fails onmain.Implementation
createOpenAIModelWithServiceTierintocreateOpenAIModelWithPreservedOptions. For Sol/Luna wire IDs (including dated IDs) with effortnone, it removes the effort from the SDK options and writes it into the serialized body after the SDK's capability checks. This reuses the per-call fetch-wrapper pattern already used for service tiers.openai-responsesproviders and the Coder gateway OpenAI route.mcpIconDecodeClient.test.tsintoisolated_unit_tests, following the existing pattern, so it runs in its own process with the signal-exit retry.packages/mux-compat(version and its@coder/xumdependency) to 0.30.0.Validation
noneis sent. Two controls check that Solhighpasses through and that Astranoneis still dropped. With the fix disabled, the 6nonecases fail.Risks
Low. Only requests to Sol/Luna wire IDs with effort
noneare rewritten. Other models, and Sol/Luna at other efforts, keep the existing path.Deferred (pre-existing): on Responses, the SDK treats opaque aliases (for example
team-model→openai:gpt-6-sol) as non-reasoning models and drops every reasoning effort, not justnone. This already affected aliases of any reasoning model before #4348. On Chat Completions, which is wherenonematters for tool calls, those aliases already serializenone, and a test now covers that.Review history
The first revision fixed this with a
bun patchof@ai-sdk/openai. Codex pointed out thatpatchedDependenciesdoes not apply to npm installs of the published package, so the fix now lives in Xum runtime code and the patch has been removed.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$26.31