Skip to content

fix(responses): preserve caller cancellation in eager SSE relay - #3613

Merged
lidge-jun merged 3 commits into
devfrom
codex/win-8-eager-caller-cancel
Sep 5, 2026
Merged

lidge-jun merged 3 commits into
devfrom
codex/win-8-eager-caller-cancel

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 5, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Carry caller-abort provenance into the eager SSE relay independently of turn/shutdown cancellation. A caller abort no longer becomes synthetic upstream 502 when the fetch read rejects before body cancellation.
  • Preserve real-terminal precedence, bounded discard-drain, and genuine upstream-reset failure accounting. Clean up the caller listener and close signal-cancelled downstream streams.
  • Add deterministic cancellation, terminal, silent-source, paused-source and re-entrant serialization regressions.
  • Stack: fix(quota): repair Windows reset fixtures and route inventory #3610 quota integration -> this eager cancellation layer. Merge bottom-up; review this layer only.

Verification

  • New caller regression: original source failed with synthetic failed; patched source passes with one cancellation and a closed downstream.
  • Focused checks: 71 eager tests; 73 passthrough/failed-tail/capability tests; 40 WebSocket upstream tests with one existing skip; original caller/reset pair 2 tests. All passed.
  • Typecheck passed. Documentation build passed (425 pages). Independent implementation review passed.
  • Windows run 33943295449 on exact head 0449c8df022095393c926a76e3e6ed071d40f476: all six shards SUCCESS, 18,147 pass, 79 skip, 0 fail across 1,080 files. Slowest job 22m38s within the unchanged 25-minute ceiling.
  • The original caller-cancel and upstream-reset server tests both passed on Windows (1.34s and 1.46s), preserving 499 versus 502 and pool-health assertions.
  • No repository-wide local suite; macOS completion is explicitly excluded by the maintainer. This is Windows-only acceptance, not aggregate multi-OS green.
  • Subsequent dev reconciliation preserves the Windows-tested eager implementation and regression files byte-for-byte; the inherited core changes were reviewed, and 150 focused tests plus the original pair and typecheck passed. Independent merge-resolution review PASS. Final commit only archives the unit and records evidence; do not relabel the Windows run as post-reconciliation CI.

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.

@coderabbitai

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

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 75 / 80

이 PR은 Windows에서 eager SSE 릴레이가 호출자 abort를 502(합성 upstream 실패)로 잘못 기록하던 문제를 고칩니다. base는 dev가 아니라 #3610 브랜치(codex/win-7-postmerge-stability)입니다. 지금 dev HEAD f008a553d에는 아직 이 패치가 없고, Windows rewrite가 eager를 고르면 #3541이 tee에 넣어 둔 caller provenance가 eager 경로에는 전달되지 않습니다.

증상은 명확합니다. core.ts는 fetch controller에 caller abort를 연결하지만, eager에는 별도 turn controller를 줍니다. read가 reject된 뒤 body.cancel보다 먼저 신호가 오면 !cancelled && !upstream.signal.aborted 가드가 합성을 허용해 499 대신 502/terminal synthetic이 남습니다. 이 PR은 EagerRelayOptions.clientGoneSignal을 추가하고, markClientGone / canDeliver로 호출자 상태를 재검사하며, 생산 시작 전 리스너를 등록합니다. core.ts의 단일 생산 호출은 항상 clientGoneSignal: options.abortSignal을 넘깁니다.

테스트는 tests/server/relay-eager.test.ts에 결정적 시나리오를 넣습니다. read reject+caller abort, abort 없는 reject, 실청크 선도착, delimiter-less terminal, silent producer 등입니다. 문서(proxy-formats.md, structure/04_...)에 한 줄씩 계약을 남겼고, decade doc 110_eager_caller_provenance.md에 closeout을 붙입니다. auth·Bun pin·Windows safety selector는 건드리지 않습니다.

#3610이 픽스처/목록을 맞춘 뒤, 이 층이 실제 프로덕트 버그를 고칩니다. Windows 스위트 초록의 핵심 축이라 우선순위가 높습니다. 다만 base가 #3610이므로 단독으로 dev에 머지하면 충돌·의존이 생깁니다.

src/server/relay-eager.ts - canDeliver가 합성/enqueue 가드를 한곳으로 모읍니다. catch 진입과 encode 후에도 abort를 다시 보는 점이 “serialization이 cancel을 재진입”하는 기존 주석과 맞습니다.

src/server/relay-eager.ts - 최종 controllerRef?.close()를 cancelled와 무관하게 가드된 close로 바꾼 것은 signal-only cancel(body.cancel 없음)을 위한 것입니다. 이미 닫힌 스트림에서 예외를 삼키는 try는 유지됩니다.

src/server/responses/core.ts - inline rewrite budget과 clientGoneSignal을 한 options 객체로 합쳤습니다. 다른 직접 호출자는 테스트뿐이라 필드 생략이 허용됩니다.

tests/server/relay-eager.test.ts - sleep 없는 결정적 행이 Windows flake를 줄입니다. 음성 twin(upstream reset → 502)이 유지되는지 리뷰 체크리스트에 남는 것이 중요합니다.

라인 없음 - draft이며 CI QUEUED입니다. #3610 없이 머지하면 base 가정이 깨집니다.

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

  • #3610과 #3613을 스택 그대로 연속 머지할지
  • Windows 6샤드 전체 초록을 머지 게이트로 강제할지, focused+이 테스트만으로 먼저 넣을지
  • Darwin/명시 eager·Codex WS 경로에도 동일 신호 전달이 충분한지(본문은 기존 abortSignal 체인을 재사용)

너의 추천
#3610이 먼저 합쳐진 뒤 이 PR을 바로 이어서 머지하세요. exact-head에서 relay-eager 신규 테스트와 Windows shard의 499/502 쌍이 초록인지 확인하고 draft를 해제하세요. #3610을 닫지 않은 채 이 브랜치만 dev에 리베이스해 올리는 것은 비추천입니다.

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

@github-actions

github-actions Bot commented Sep 5, 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 5, 2026
Base automatically changed from codex/win-7-postmerge-stability to dev September 5, 2026 04:38
@lidge-jun
lidge-jun marked this pull request as ready for review September 5, 2026 04:39
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 5, 2026 04:39
@lidge-jun
lidge-jun merged commit be81013 into dev Sep 5, 2026
8 of 11 checks passed
@lidge-jun
lidge-jun deleted the codex/win-8-eager-caller-cancel branch September 5, 2026 04:39
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-05T04:40:51.653072Z b9c3b77 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…ller-cancel

fix(responses): preserve caller cancellation in eager SSE relay
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.

1 participant