Skip to content

fix(runtime): stream and fold non-streaming Codex OAuth calls - #5723

Open
liugddx wants to merge 3 commits into
apache:mainfrom
liugddx:fix/codex-auxiliary-stream
Open

liugddx wants to merge 3 commits into
apache:mainfrom
liugddx:fix/codex-auxiliary-stream

Conversation

@liugddx

@liugddx liugddx commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Summary

On an openai-codex (ChatGPT subscription) connection every tool-free auxiliary call failed: Session titles, goal evaluation, recap, Daily Review and prompt suggestions. Main turns kept working. As a result, Codex Sessions never get a generated title; they fall back to the truncated first message.

The backend rejects the non-streaming request generateText sends, for three separate reasons:

  • 400 "Stream must be set to true"
  • 400 "Unsupported parameter: max_output_tokens"
  • once the request streams, the terminal response.completed event can carry an empty output

The fix stays inside buildOpenAiCodexFetch, the adapter that already reshapes Codex requests. For a caller that did not ask to stream, it:

  • sends stream: true and drops max_output_tokens;
  • reads the stream to its terminal event and restores output from response.output_item.done when the terminal event omits it;
  • returns the single Responses body the non-streaming parser expects.

A failed, incomplete or truncated stream surfaces as an error rather than an empty or partial success. Streaming callers (main turns) are forwarded exactly as built, including any configured max_output_tokens; see #5736 for that separate failure.

The stream is recognised by its body, not by content-type. Against the live backend, the stream did not reach this layer with an event-stream content type, and a first version gated on the header never folded anything.

Trade-off: Codex auxiliary calls no longer carry an output cap. They are bounded by their prompts and by each caller's own result validation.

Fixes #5711

Verification

  • subscription-model-fetch.test.ts: 22/22 pass. The five new cases fail without the change:

    • fold with an empty terminal output, through doGenerate;
    • a streamed turn passed through unfolded, keeping its configured output cap;
    • truncated stream;
    • response.failed;
    • response.incomplete with partial output (content_filter).
  • The test SSE carries no content-type, matching what the live backend delivered.

  • Real backend. I ran a Runtime Host from this branch on a copy of a Desktop workspace with a ChatGPT-subscription connection, did one turn, then read the Session name and the session_title usage row:

    model main this branch
    gpt-6-astra error, HTTP 400 {"detail":"Stream must be set to true"}; name = truncated first message success (82 in / 14 out); "Planning a Small Refactor of a Large React Component"
    gpt-5.6-sol (same 400 in the workspace's usage history) success; "Planning a Large React Component Refactor"
    gpt-5.5 (same 400 in the workspace's usage history) success; "Refactoring a Large React Component"

    That workspace's usage_llm_calls had 18 Codex auxiliary rows (titles and goal evaluation), all failed, none succeeded.

  • biome check on the changed files passes, and so does tsc --noEmit for @maka/runtime.

  • The full @maka/runtime suite ran on Windows. Its failures are all in unrelated files and are environmental: EBUSY on temp SQLite cleanup, EPERM on symlink creation, macOS seatbelt tests, and shell tests. I did not run the other workspaces' suites.

  • The pre-commit hook could not run on this Windows machine. scripts/biome-staged-check.mjs spawns node_modules/.bin/biome.cmd with spawnSync and no shell, which Node 22 rejects with EINVAL. I ran its steps manually and all passed: biome check, asf-license-headers check-staged, protocol-epoch-check --staged and git diff --cached --check. The hook problem itself is not part of this PR.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code (Claude Opus) diagnosed the backend responses, implemented the fix and tests, and ran the verification above; liugddx reviewed. The commit carries a Generated-by trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

🤖 Generated with Claude Code

The ChatGPT Codex backend accepts only streamed requests and rejects
max_output_tokens. Main turns stream and never send the cap, but every
tool-free auxiliary call (Session titles, goal evaluation, recap, Daily
Review, prompt suggestions) goes through generateText and failed with
HTTP 400 "Stream must be set to true".

For a non-streaming caller, the Codex fetch now sends stream: true, drops
max_output_tokens, and folds the event stream into the single Responses
body the caller parses. The terminal event can carry an empty output, so
items from response.output_item.done are restored. A failed or truncated
stream surfaces as an error. Streaming callers are passed through.

Fixes apache#5711

Generated-by: Claude Code
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@hqhq1025 hqhq1025 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.

Reviewed the current two-file Codex OAuth request adapter change. It streams auxiliary Responses requests and folds their terminal result for non-streaming callers. Current-head CI test/label, build, and 47 focused tests pass, but one output-budget regression below needs resolution before merge. I did not validate a live Codex subscription endpoint or every model-limit setting.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

// streamed here and folded back into the one Responses body a
// non-streaming caller parses.
const foldStream = parsedBody.stream !== true;
const { max_output_tokens: _unsupportedOutputCap, ...forwardedBody } = parsedBody;

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.

[P2] This strips max_output_tokens from every Codex request, including stream: true main turns (foldStream is false on that path). Desktop exposes an output-token limit and the model adapter can pass it into the Responses body, so the selected cap is silently discarded. The comment assumes main turns never send it, but the adapter path does. Please preserve/enforce the configured limit for main turns, or explicitly reject an unsupported limit rather than silently ignoring the user's setting; add a streamed-turn regression case.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right, thanks. A per-model output limit (modelOverrides[model].maxOutputTokens) reaches a streamed main turn as max_output_tokens, and this PR was dropping it silently.

Fixed in e5a66e3: max_output_tokens is now dropped only on the folded path, i.e. a caller that did not ask to stream. Those are the tool-free auxiliary calls, whose cap is an internal constant. A streaming request is forwarded exactly as the caller built it, so main turns behave as on main.

Added a streamed-turn regression case. It goes through the OpenAI Responses adapter's doStream with maxOutputTokens: 64 and asserts that the body keeps stream: true and max_output_tokens: 64. It fails against the previous head and passes now.

One observation, left out of scope here: when I sent the auxiliary request with the cap, the Codex backend answered 400 {"detail":"Unsupported parameter: max_output_tokens"}. A main turn with that override set will probably hit the same 400 on main. If so, rejecting the override up front for openai-codex, as you suggest, seems like the right follow-up. I'd rather not guess at it in this PR.

Only the folded auxiliary path drops max_output_tokens: its cap is an
internal constant the Codex backend rejects. A streaming request is
forwarded as the caller built it, so a per-model output limit on a main
turn is no longer silently discarded.

Generated-by: Claude Code
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@hqhq1025 hqhq1025 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.

I reviewed the exact current head. The two-file change streams and folds non-streaming Codex auxiliary requests while preserving max_output_tokens on streamed main turns. Node 24 npm ci, build:test, 57 focused runtime tests, and current-head CI test pass; the PR is mergeable and git diff --check is clean. The focused test proves the forwarded JSON, but its mock accepts any request. I could not test this with a real ChatGPT Codex backend, so the compatibility issue below needs a real streamed request with a configured output limit before this is considered ready. No schema or migration change.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

// here.
const foldStream = parsedBody.stream !== true;
const { max_output_tokens: _auxiliaryOutputCap, ...uncappedBody } = parsedBody;
const forwardedBody = foldStream ? { ...uncappedBody, stream: true } : parsedBody;

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.

[P2] Check the configured main-turn path against the actual Codex backend. This now forwards max_output_tokens whenever a streamed turn has a per-model output override (model-adapter.ts:724-725 → ai-sdk-turn.ts:1580-1622). The PR's real-backend evidence reports HTTP 400 Unsupported parameter: max_output_tokens, and this file still says the backend rejects that field. The new test only checks a permissive mock's captured JSON; it cannot establish that a streamed request with the field is accepted. If the rejection applies to streamed requests too, a user-set output cap changes a previously working turn into a failed turn. Please verify this exact case against the backend and handle unsupported limits explicitly rather than silently dropping the user's setting or forwarding a rejected parameter.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I checked this exact case against the real backend. I ran a Runtime Host on a copy of a Desktop workspace with a ChatGPT-subscription connection, with modelOverrides['gpt-6-astra'] = { maxOutputTokens: 4096 }, and ran one streamed main turn ("Reply with exactly: ok"):

build per-model output limit turn
this PR (e5a66e322) 4096 failed, request_rejected: Codex OAuth request failed: HTTP 400 {"detail":"Unsupported parameter: max_output_tokens"}
main (87fc9f69c) 4096 failed, identical 400
this PR none completed

So the backend rejects the field on streamed requests too. A Codex turn with a per-model output limit already fails on main, and this PR leaves that path byte-for-byte as main sends it: it forwards the streaming body unchanged. The previous head silently dropped the limit; the current head no longer does. Neither version turns a working turn into a failing one.

Handling the unsupported limit explicitly is a real fix, but it is a different change from making auxiliary calls work. There are two options, and they have different product consequences:

  • reject the override at configuration time for openai-codex, in the connection settings and codec;
  • drop it in selectedModelMaxOutputTokens and surface that to the user.

I've opened #5736 with the reproduction above, and I'm happy to take it as a follow-up. I'd prefer to keep this PR scoped to the auxiliary path unless you think the two should land together.

if (!terminal) {
throw new Error('Codex OAuth request failed: the stream ended without a terminal response');
}
if (terminalType === 'response.failed') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P2] Please handle response.incomplete explicitly before returning the folded JSON. It is accepted as a terminal event above, but this branch throws only for response.failed. If the backend emits response.incomplete with incomplete_details.reason: "content_filter" and a partial output item, the fold returns that partial item as a normal Responses body. I verified against the repository’s @ai-sdk/openai version that doGenerate resolves with the partial text and a content-filter finish reason rather than throwing; runHostAuxiliaryModelCall then records the call as success. That contradicts the PR’s failed/truncated-stream contract and can use partial text for titles or other auxiliary work. Please add an incomplete-event regression test and reject incomplete responses here (or define and test a narrowly justified reason for accepting partial output). This is a synthetic SDK-boundary case; I have not observed this event on the live Codex backend.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agreed, thanks. The fold accepted response.incomplete as terminal but returned its partial output as a normal body. A content-filtered fragment would have resolved as a Session title or suggestion and been metered as success, which contradicts the stated contract.

Fixed in c231148. response.incomplete is now rejected like response.failed, and the error carries the reason (Codex OAuth request incomplete: content_filter). I don't see a narrow reason to accept partial output here. This path only serves tool-free auxiliary calls, which already drop max_output_tokens, so a length stop isn't expected. Every caller already treats an error as "no result": titles fall back, suggestions show nothing, recap and Daily Review report failure.

Regression test: a stream with a partial output_item.done followed by response.incomplete (incomplete_details.reason: "content_filter") must reject with that reason. It fails without the change. subscription-model-fetch is 22/22, and runtime tsc and Biome pass.

I've also updated the PR description: a failed, incomplete or truncated stream now surfaces as an error.

A response.incomplete terminal event carries partial output, for
example when a content filter cuts it off. Folded into a normal
Responses body, the fragment resolved as the caller's result and was
recorded as a success. Reject it like response.failed, with the
incomplete reason in the error.

Generated-by: Claude Code
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@hqhq1025 hqhq1025 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.

Reviewed the current head (c231148b). The new change rejects a terminal response.incomplete rather than returning partial auxiliary output as a successful folded response; the regression test covers a partial output item followed by a content_filter incomplete event (packages/runtime/src/subscription-model-fetch.ts:257-261, packages/runtime/src/__tests__/subscription-model-fetch.test.ts:212-251). I found no remaining substantiated P0–P3 in this change. The previous incomplete-stream P2 is addressed; earlier-head reviews do not apply to this head.

Node 24 build:test, 22 focused fetch tests, Biome, git diff --check, current-head CI test, and merge-tree checks against current main and PR #5738 passed. This PR handles auxiliary calls; PR #5738 separately handles main-turn output limits. I did not run a live Codex backend or packaged Desktop end-to-end test, so those remain validation gaps. No schema or migration changes.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

This branch has not been deployed

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

Labels

effort/M Under 500 readable lines

Projects

None yet

3 participants