Skip to content

fix(responses): strip max_output_tokens and metadata in forward mode#489

Merged
Ingwannu merged 1 commit into
lidge-jun:devfrom
elppaaa:fix/strip-max-output-tokens-forward
Jul 26, 2026
Merged

fix(responses): strip max_output_tokens and metadata in forward mode#489
Ingwannu merged 1 commit into
lidge-jun:devfrom
elppaaa:fix/strip-max-output-tokens-forward

Conversation

@elppaaa

@elppaaa elppaaa commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

The ChatGPT backend (authMode: "forward") enforces a strict parameter allowlist and rejects unknown top-level fields with HTTP 400 {"detail":"Unsupported parameter: …"}.

The stripPreviousResponseId doc-comment (line 452) already documents that the backend also rejects metadata and max_output_tokens, but only previous_response_id was actually stripped. Third-party Responses API clients (e.g. GJC, SDK wrappers) send max_output_tokens per the public OpenAI /v1/responses spec, causing every request through a forward-mode provider to fail with 400.

Codex CLI never sends these fields — it controls output length via reasoning.effort — so the strip is a no-op for the primary client. Claude Code uses the anthropic.ts adapter and is unaffected. Key-auth mode preserves both fields (the platform API supports them).

Change: add stripUnsupportedForwardParams() inside the existing if (forward) block in buildRequest(), removing both max_output_tokens and metadata before the wire request is serialized.

Verification

  • bun test tests/openai-responses-passthrough.test.ts — 29/29 pass (3 new tests)
  • bun run typecheck — clean
  • Live reproduction: the exact request shape that returned 400 (max_output_tokens: 32000, reasoning.effort: "low", store: false) now returns HTTP 200 with response.completed through a forward-mode provider
  • New tests cover: forward strips both fields, forward no-op when absent, key-auth preserves both fields

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • Bug Fixes

    • Improved forwarding compatibility by removing unsupported max_output_tokens and metadata request parameters before submission.
    • Preserved supported request fields and ensured parameters remain unchanged when using key-based authentication.
  • Tests

    • Added coverage for parameter removal, no-op behavior when fields are absent, and preservation in key-authenticated requests.

The ChatGPT backend (authMode: "forward") enforces a strict parameter
allowlist and rejects unknown top-level fields with HTTP 400
{"detail":"Unsupported parameter: …"}. The stripPreviousResponseId
doc-comment already documents that the backend rejects metadata and
max_output_tokens, but only previous_response_id was actually stripped.

Third-party Responses API clients (GJC, SDK wrappers) send
max_output_tokens per the public OpenAI spec, causing every request
through a forward-mode provider to fail with 400. Codex CLI never sends
these fields (it uses reasoning.effort for output control), so the
strip is a no-op for the primary client.

Add stripUnsupportedForwardParams() inside the existing forward block,
removing both max_output_tokens and metadata before the wire request
is built. Key-auth mode is unaffected.

Verified live: the exact request that returned 400 (max_output_tokens:
32000 + reasoning.effort: low) now returns 200 with response.completed.
@github-actions github-actions Bot added the bug Something isn't working label Jul 26, 2026
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f2704ee7-8279-425f-ad81-70229dde7488

📥 Commits

Reviewing files that changed from the base of the PR and between 2f3fa58 and 112b8c6.

📒 Files selected for processing (2)
  • src/adapters/openai-responses.ts
  • tests/openai-responses-passthrough.test.ts

📝 Walkthrough

Walkthrough

The Responses passthrough adapter now strips max_output_tokens and metadata from forward-mode requests. Tests cover removal, no-op behavior, and preservation of both fields in key-auth mode.

Changes

Forward request sanitization

Layer / File(s) Summary
Forward-mode parameter sanitizer
src/adapters/openai-responses.ts
Adds a helper that removes top-level max_output_tokens and metadata and applies it during forward-mode request construction.
Mode-specific request tests
tests/openai-responses-passthrough.test.ts
Verifies forward-mode stripping and no-op behavior, while confirming key-auth requests preserve both fields.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: ingwannu, lidge-jun, alexanderxcmo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: stripping max_output_tokens and metadata in Responses forward mode.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed exact head 112b8c6. The change is correctly limited to forward-mode Responses requests; persisted configuration reserves that mode for the canonical built-in ChatGPT backend, while key-auth providers retain max_output_tokens and metadata. The sanitizer is top-level only, allocation-free when absent, and covered by forward-strip/no-op/key-preserve regressions. All 29 focused tests and TypeScript typecheck pass locally. Approved pending required CI.

@Ingwannu
Ingwannu merged commit 9dff083 into lidge-jun:dev Jul 26, 2026
12 checks passed
lidge-jun added a commit that referenced this pull request Jul 26, 2026
The prior 260726_pr_close_rework map is stale: eight of its nine targets are
already CLOSED or MERGED. This unit re-inventories the 14 currently open PRs,
applies the MAINTAINERS.md security boundary, and writes diff-level decade docs
for the three that a maintainer can land directly (#489, #464, #483).

Eight PRs stay held for security review (OAuth, credential routing, UAC,
release automation) and three are held for their authors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants