Skip to content

fix(responses): recover expired forward continuation over WebSocket - #3889

Closed
y2ambition-ai wants to merge 1 commit into
lidge-jun:devfrom
y2ambition-ai:fix/codex-continuation-cache-recovery
Closed

fix(responses): recover expired forward continuation over WebSocket#3889
y2ambition-ai wants to merge 1 commit into
lidge-jun:devfrom
y2ambition-ai:fix/codex-continuation-cache-recovery

Conversation

@y2ambition-ai

Copy link
Copy Markdown
Contributor

Summary

A Codex WebSocket task stops with a non-retryable 400 when its canonical ChatGPT forward replay entry expires or is missing. The proxy correctly refuses to forward a context-free delta, but reports invalid_request_error, which Codex treats as terminal.

Return the existing previous_response_not_found classification instead. Codex can then reconnect and resend its full retained input, including completed tool calls and results. Rejection still happens before authentication, adapter construction, or upstream I/O; the cache TTL and capacity bounds remain unchanged. HTTP clients must explicitly replay their full context.

This is a standalone fix to existing upstream behavior, independent of the context-history and quota feature PRs. English and Korean integration guidance describe the recovery boundary.

Verification

At revision e8d82a18:

  • bun scripts/test.ts --changed=upstream/dev (base 76436a3e): 4,466 passed, 1 skipped, 0 failed across 177 files.
  • Focused replay regression: 7 passed. Covers expired/missing state through a real WebSocket, no upstream request for the rejected delta, reconnect/full replay with tool-call/result pairing, fresh replay, and native API-key continuation.
  • Regression proved red before the production change: expiry HTTP and both WebSocket cases reported the old generic code.
  • bun run typecheck, bun run privacy:scan, and git diff --check passed.
  • Documentation frozen install/build passed: 425 pages.
  • Installed official Codex CLI 0.153.4, isolated homes and a synthetic upstream: one real tool execution, forced cache expiry, recoverable WebSocket error, a second handshake, full-input replay, then the same codex exec completed with exit 0. The pre-fix control failed with exit 1, one handshake and no replay. All external requests were blocked and test children/listeners were cleaned up. This verifies the real client/proxy recovery protocol, not live-model or account failover behavior.
  • Independent read-only review found no actionable issues. Full repository-wide and hosted CI are not claimed; this remains a draft.

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

  • All CI tests are green on my local testing.
  • I pushed my PR to the latest dev commit.
  • I resolved all correct Codex and CodeRabbit findings.
  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 7, 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 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 68 / 80

이 PR은 캐노니컬 ChatGPT forward에서 로컬 continuation 캐시가 만료·누락됐을 때 나는 오류 코드를 바꿉니다. 지금 devsrc/server/responses/core.tshasUnexpandedPreviousResponse + 캐노니컬 forward일 때 invalid_request_error를 돌려줍니다. 업스트림으로 보내기 전에 막는 #702 가드는 맞지만, Codex WebSocket 클라이언트는 그 일반 코드를 터미널로 보고 재시도를 포기합니다. 같은 자리의 콤보/corrupt 경로들은 이미 previous_response_not_found를 씁니다. 이 PR은 forward 만료 경로도 그 코드로 맞추고, 메시지를 “전체 대화를 다시 보내라”로 바꿉니다. 캐시 TTL·용량·인증 전 거부는 그대로입니다.

영문·한글 codex-integration 가이드에 복구 경계를 적어 두었고, tests/codex-integration/issue-702-expired-replay-state.test.ts에 WebSocket 재연결·툴 히스토리 풀 리플레이 회귀를 넣었습니다. 공식 Codex CLI 0.153.4로 캐시 만료 후 복구를 실제로 확인했다는 점도 본문에 있습니다. types/config 분할과 무관한 좁은 서버/문서 수정입니다.

src/server/responses/core.ts (forward 만료 formatErrorResponse) - 코드만 previous_response_not_found로 바뀌고 HTTP 상태는 400 유지입니다. 콤보 경로(같은 파일의 다른 previous_response_not_found)와 맞습니다. HTTP 전용 클라이언트가 예전 문자열/타입에 의존했다면 깨질 수 있으니, 가이드가 말한 대로 “같은 ID만 재시도하면 안 된다”는 점이 중요합니다.

tests/codex-integration/issue-702-expired-replay-state.test.ts (HTTP 기대값) - 기존 HTTP 테스트의 type은 여전히 invalid_request_error이고 codeprevious_response_not_found로 바뀝니다. OpenAI 스타일 봉투에서 type/code 분리가 의도인지, HTTP도 type을 같이 바꿀지 한 번만 확인하세요.

docs-site/.../codex-integration.md (EN/KO) - “1시간 캐시”를 문서에 적습니다. 코드의 REPLAY_TTL_MS와 숫자가 어긋나면 나중에 문서만 낡습니다. 상수와 맞춰 두거나 “bounded cache”처럼 숫자 없이 쓰세요.

메인테이너의 판단이 필요한 지점

  • HTTP 응답의 error.type을 코드와 같이 previous_response_not_found로 통일할지, type은 일반·code만 특수 코드로 둘지
  • 문서에 TTL 숫자를 박을지

너의 추천
베이스를 현재 dev(522ce5f8c)에 맞춘 뒤 머지하세요. #702 가드의 의미를 바꾸지 않고 Codex WS 복구만 열어 주는 좋은 한 줄 수정입니다. 문서 TTL 숫자만 코드와 대조하면 됩니다.

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

@lidge-jun

Copy link
Copy Markdown
Owner

Carried into the integration stack as PR #3937.

Your commit e8d82a181 was cherry-picked onto current dev with its original authorship intact, and the branch commit carries a Co-authored-by: ykvv <229483879+y2ambition-ai@users.noreply.github.com> trailer so the credit survives the squash. The layer branch is codex/b-stack-l1-continuation-recovery; #3937 is stacked on top of it and carries both layers.

An independent read-only audit checked the parts this change depends on: formatErrorResponse's second argument is a classifyError input whose previous_response_not_found branch produces type: invalid_request_error with code: previous_response_not_found (src/bridge.ts:2130, src/lib/errors.ts:179), which matches your updated assertion. The three existing uses in core.ts carry the same recovery meaning, no other consumer depends on the old string, and the "one hour" in your documentation matches RESPONSE_TTL_MS = 3_600_000 (src/responses/state.ts:27). The #702 rejection boundary is preserved.

Nothing is wrong with this PR — it is being landed through the stack so both forward-path fixes share a single CI run. This PR will be closed once #3937 merges into dev; the code lands either way. Thanks for the WebSocket reproduction with the real Codex CLI, which is what made the recovery path convincing.

lidge-jun added a commit that referenced this pull request Sep 7, 2026
…ebSocket quota (#3937)

* docs(devlog): plan the B-track continuation/quota stack delivery

Records the dependency-ordered branch chain for #3889 and #3934, the CI trigger contract that keeps the lower layer PR-free, and the original-author preservation requirement.

* fix(responses): recover expired forward continuation over WebSocket

A Codex WebSocket task stopped with a non-retryable 400 when its canonical
ChatGPT forward replay entry expired or was missing. The proxy correctly
refuses to forward a context-free delta, but reported invalid_request_error,
which Codex treats as terminal.

Return the existing previous_response_not_found classification instead, so
Codex reconnects and resends its full retained input, including completed tool
calls and their results. Rejection still happens before provider credential
resolution, adapter construction, and upstream I/O; the cache TTL and capacity
bounds are unchanged. HTTP clients must explicitly replay their full context.

(cherry picked from commit e8d82a1)

Co-authored-by: ykvv <229483879+y2ambition-ai@users.noreply.github.com>

* fix(codex): fence late WebSocket quota by credential generation

A streaming Codex request could keep receiving WebSocket quota metadata after
its pool credential was replaced under the same local account ID. A late frame
then repopulated the quota cleared for that replacement, making the new
credential look exhausted.

Capture the selected pool credential generation when the response's quota
observer is created and ignore later observations once that generation is no
longer live. Existing configuration-generation fencing and the main-pool
writer are unchanged: main-pool carries no credential generation and keeps its
own mainQuotaWriter identity check.

(cherry picked from commit e5c01f4)

Co-authored-by: luvs01 <luvs01@hanmail.net>

---------

Co-authored-by: t <a@b.com>
Co-authored-by: ykvv <229483879+y2ambition-ai@users.noreply.github.com>
Co-authored-by: luvs01 <luvs01@hanmail.net>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev in ca381ea764cfbc63bec978f53eb58e96c00c0c64 via the integration stack (#3937).

Your change is in dev byte-for-byte: src/server/responses/core.ts, tests/codex-integration/issue-702-expired-replay-state.test.ts, and both Codex integration guides match this branch exactly. The squash commit carries Co-authored-by: ykvv <229483879+y2ambition-ai@users.noreply.github.com>, so the contribution is attributed to you.

Cross-platform CI passed on the stack tip a06bfa2f2: all four Linux shards, both macOS lanes, gates, packaging, keyring, and Docker smoke, with zero failures.

Closing as delivered, not rejected. Thank you.

@lidge-jun lidge-jun closed this Sep 7, 2026
@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 7, 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 landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants