Skip to content

fix(responses): allow canonical ChatGPT upstream WebSocket opt-out - #5704

Closed
ildunari wants to merge 1 commit into
lidge-jun:devfrom
ildunari:fix/codex-chatgpt-http-transport
Closed

ildunari wants to merge 1 commit into
lidge-jun:devfrom
ildunari:fix/codex-chatgpt-http-transport

Conversation

@ildunari

@ildunari ildunari commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Allow the canonical ChatGPT openai provider to set upstreamWebsocket: false and send streaming Responses turns over HTTP/SSE. Omitted keeps the current WebSocket default; explicit true remains outside the canonical provider seed.
  • Keep client-facing websockets independent and disable WebSocket-only native steering/injection when the upstream opt-out is active.
  • Document the setting and cover provider persistence, transport selection, and native-control eligibility.

This addresses a field report of intermittent upstream WebSocket close 1006 with no response frame on requests around 2.7–3.6 MB. The proxy currently returns 502 for those ambiguous post-send closes; it cannot safely replay the same turn. The option lets affected hosts choose HTTP before sending without changing credentials or destination. It does not claim that every 1006 has the same cause or that HTTP cannot have its own transport failures.

Verification

  • bun test tests/responses/ws-upstream.test.ts tests/responses/ws-native-injection.test.ts — 172 passed, 1 skipped.
  • bun test tests/server/management-provider-validation.test.ts — 137 passed.
  • bun run typecheck — passed.
  • bun run structure:check — passed.
  • bun run privacy:scan — passed.
  • cd docs-site && bun install --frozen-lockfile && bun run build — passed, 505 pages built.
  • bun run test:changed — 10,369 passed, 5 skipped, 2 failed under four-worker parallelism. Both failures passed when rerun individually: native-codex-toggle.test.ts and openai-provider-option-e2e.test.ts.
  • bun run test:changed --parallel=2 — timed out at 15 minutes while tests/server/audio-dictation.test.ts had been running for 826 seconds. The changed-area gate is not green; this test is outside the modified transport and provider-validation files.

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.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

An explicit upstreamWebsocket: false now routes streaming canonical ChatGPT turns over HTTP/SSE before sending. This gives operators a supported escape from intermittent post-send WebSocket closes without replaying ambiguous turns. The default remains WebSocket and native WS controls are unavailable when HTTP is selected.

Verified: focused Responses/provider suites, typecheck, structure check, privacy scan, docs build. Changed-area suite rerun pending.
@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.

@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • hygiene: unsponsored_surface.

What to do

  • Fix unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.
  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@ildunari Tick the boxes once required local validation has passed with commands, results, and any full-suite exception documented, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

@ildunari ildunari changed the title Allow canonical ChatGPT upstream WebSocket opt-out fix(responses): allow canonical ChatGPT upstream WebSocket opt-out Sep 23, 2026
@github-actions github-actions Bot added the bug Something isn't working label Sep 23, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 54 / 80

이 PR은 ChatGPT 기본 공급자 openai가 긴 답을 보낼 때 길을 고를 수 있게 한다. 지금은 조건이 맞으면 웹소켓으로 보낸다. 현장에서는 질문이 약 2.7~3.6MB일 때 웹소켓이 답을 주기 전에 끊기기도 한다. 끊김 번호는 1006이다. 이미 보낸 질문을 다시 보내면 같은 작업이 두 번 나갈 수 있어서, 프록시는 그때 502를 주고 다시 보내지 않는다. 그래서 ~/.opencodex/config.json의 providers.openai.upstreamWebsocket를 false로 적으면, 그 공급자의 스트리밍은 HTTP로 나간다. 칸을 비워 두면 기본값인 웹소켓이 유지된다. true는 이 기본 openai 줄에서 거절된다. 사용자가 켜는 websockets 스위치와 ChatGPT 계정은 그대로다. 웹소켓으로만 되는 중간 조종과 주입은 이 값을 false로 두면 빠진다. 계정과 주소는 바뀌지 않는다.

라인 - docs-site/src/content/docs/reference/configuration/providers.md의 upstreamWebsocket 칸, src/types/provider.ts 주석, src/config/schema/leaf-validators.ts 주석은 아직 "정식 ChatGPT 전송과 무관"이라고 적혀 있다. 이번 코드에서 false는 그 전송을 HTTP로 바꾼다. 새 설명은 안내 글 codex-integration.md와 structure/config.md, structure/transports/streaming-health.md에만 있다. 같은 참고 문서의 번역본도 옛 문장이다. 설정을 찾는 사람은 이 칸이 아무 일도 안 한다고 읽게 된다.

라인 - src/server/management/provider-routes.ts의 GET /api/providers는 upstreamWebsocket를 값이 정확히 true인지만 돌려준다. 칸이 없어도 응답은 false다. 다른 공급자는 빈 칸과 false가 둘 다 HTTP라서 괜찮다. 기본 openai는 이번 PR 이후 빈 칸이 웹소켓이고 false가 HTTP다. 이 응답을 그대로 다시 저장하면 웹소켓이 꺼진다. 저장 쪽은 요청에 칸이 없을 때만 디스크의 예전 값을 남긴다.

메인테이너의 판단이 필요한 지점
위생 검사가 src/server/auth-cors.ts 때문에 unsponsored_surface로 실패했다. 보안 리뷰 뒤 maintainer-sponsored가 필요하다. 바뀐 부분은 로그인 동작이 아니라, 기본 openai에 upstreamWebsocket: false만 남기고 true는 거절하는 검사다. PR은 아직 draft이고 본문 체크리스트 네 칸이 비어 있다. 작성자는 test:changed가 네 갈래에서 두 개 실패했고 개별로 다시 돌리면 통과했다고 적었다. --parallel=2는 tests/server/audio-dictation.test.ts에서 15분을 넘겼다. 그 실패를 이 변경과 무관한 것으로 볼지는 메인테이너 몫이다.

너의 추천
참고 문서와 코드 주석을 새 동작에 맞춘 뒤에 초안을 푸는 쪽이 맞다. GET /api/providers가 빈 칸과 false를 구분하게 하거나, 조회 결과의 false를 그대로 저장해도 기본 openai의 웹소켓이 꺼지지 않게 막으면 된다. 길 선택과 저장 테스트는 이 의도를 덮고 있다. 기본값은 웹소켓으로 남아 있고, HTTP는 적은 사람만 고른다. 1006이 모두 이 설정으로 사라진다고 말하지 않은 점도 맞다.

이 댓글은 grok-bot이 작성했습니다

lidge-jun added a commit that referenced this pull request Sep 24, 2026
…, #5714, #5704, #5707) (#5738)

* fix(claude): bound Responses user to 64 chars for long metadata.user_id

Claude Code sends metadata.user_id as a ~186-char JSON string. It was copied
verbatim into the Responses 'user' field, which Azure OpenAI and other
OpenAI-compatible backends cap at 64 chars, so every Claude Code request
routed to Azure failed with 400 "Invalid 'user': string too long".

Keep short ids as-is and send the SHA-256 hex digest (already computed for
prompt_cache_key) when the id exceeds 64 chars.

Fixes #5705

Co-authored-by: Giulio Leone <giulioleone097@gmail.com>

* Fix plaintext V2 SSE responses with missing content type

Co-authored-by: Jerry WANG <jerrywang@Jerrys-MacBook-Pro-2.local>

* fix(responses): exclude dropped chat reasoning from input admission

* test(admission): verify reasoning payload matches gate decisions

Co-authored-by: 정우철 <oocheol@naver.com>

* Allow HTTP upstream for canonical ChatGPT provider

An explicit upstreamWebsocket: false now routes streaming canonical ChatGPT turns over HTTP/SSE before sending. This gives operators a supported escape from intermittent post-send WebSocket closes without replaying ambiguous turns. The default remains WebSocket and native WS controls are unavailable when HTTP is selected.

Verified: focused Responses/provider suites, typecheck, structure check, privacy scan, docs build. Changed-area suite rerun pending.
Co-authored-by: kosta <kosta963@gmail.com>

* test(claude): pin exact user hash and the 64/65-char boundary

Refs #5705

Co-authored-by: Giulio Leone <giulioleone097@gmail.com>

* docs(architecture): attribute the admission fix to excluding unsent thinking

Split the preserved-reasoning statement from the refusal rationale in the English and Korean paragraphs, as review of #5714 asked.

Refs #5696

Co-authored-by: 정우철 <oocheol@naver.com>

* fix(anthropic): count ping events as upstream liveness

Anthropic streams may include any number of ping events. The adapter only turned SSE comments into heartbeats, so a long silent thinking phase that pinged was cut off at stallTimeoutSec with upstream_stall_timeout. Named and data-only ping records now yield the same heartbeat.

Refs #5707

* fix(management): report upstreamWebsocket as configured in GET /api/providers

The row coerced an unset value to false. After the canonical ChatGPT opt-out, unset means upstream WebSocket and false means HTTP/SSE, so a save built from the row could turn WebSocket off. The row now omits the key when it is unset.

Co-authored-by: kosta <kosta963@gmail.com>

* docs(providers): describe the canonical ChatGPT upstreamWebsocket opt-out

The reference row, its seven translations, and the provider type and schema comments still said the canonical ChatGPT transport ignores upstreamWebsocket. It now selects HTTP/SSE when set to false. The locale rows were also behind the English row on the first-party-only restriction; they are retranslated from it.

Co-authored-by: kosta <kosta963@gmail.com>

* fix(responses): bound the plaintext V2 SSE prefix probe by stallTimeoutSec

The carried probe read the first chunk of an unlabeled body with no deadline, before the passthrough stall guard is attached. An upstream that sent headers but no body held the request and its host lease until the client gave up, and a failed read escaped the classifier. Each probe read now races a per-read inactivity window and one total budget, both stallTimeoutSec, plus the client abort signal. Timeout, abort, and read errors cancel the reader and return the existing unsupported-content-type 502. Docs now scope the recovery to missing or unrecognized non-JSON content types.

Follow-up to #5683 review (maintainer, Codex, CodeRabbit).

Co-authored-by: Jerry WANG <jerrywang@Jerrys-MacBook-Pro-2.local>

---------

Co-authored-by: Giulio Leone <giulioleone097@gmail.com>
Co-authored-by: Jerry WANG <jerrywang@Jerrys-MacBook-Pro-2.local>
Co-authored-by: 정우철 <oocheol@naver.com>
Co-authored-by: kosta <kosta963@gmail.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Carried into bundle #5738, which is now on dev (squash-merged as df61bce) with a Co-authored-by trailer for you, so this PR is closing as landed. Thank you for the fix. If something from this branch did not make it in, the bundle description lists what was changed during the carry.

@lidge-jun lidge-jun closed this Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants