Skip to content

fix(responses): expose bounded recovery refusal reasons - #3794

Merged
lidge-jun merged 2 commits into
devfrom
codex/track1-04-recovery-592d
Sep 6, 2026
Merged

fix(responses): expose bounded recovery refusal reasons#3794
lidge-jun merged 2 commits into
devfrom
codex/track1-04-recovery-592d

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

Verification

  • Full Cross-platform CI lane=all: all 25 jobs passed at final descendant bf94d8dfa7b91c0e4acb96b2afce64d3c9a5ddde, including Linux, all six Windows shards and the macOS full-control run.
  • This layer's exact head is bf94d8dfa7b91c0e4acb96b2afce64d3c9a5ddde and is an ancestor of that tested final head. Lower-head suites were intentionally not run separately, per the maintainer's final-first validation instruction; they are not represented as independently passing checks.
  • Relevant coverage: Boolean compatibility, admission/privacy failures, shared-flight isolation, cancellation/capacity, input invalidation and direct/combo error serialization.
  • Independent Astra high security/correctness reviews completed; valid automated findings were fixed. Local tests, typecheck, builds and installation were not run, per explicit maintainer instruction.
  • Protocol cases were checked against the local Codex implementation and official WebSocket documentation.

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.

Manual stack

Layer Pull request Head branch
01 #3791 codex/track1-01-quota-592d
02 #3792 codex/track1-02-compact-592d
03 #3793 codex/track1-03-websocket-592d
04 #3794 codex/track1-04-recovery-592d

Maintainer integration

@lidge-jun explicitly authorized admin integration into dev without a second maintainer approval under MAINTAINERS.md. This is maintainer integration, not self-approval. The final aggregate evidence above and its ancestry cover this stack; lower-head execution is explicitly deferred. Merge bottom-up with merge commits, preserving contributor attribution. Because automatic branch deletion is enabled, move a direct child's base to dev immediately before merging its parent. No native GitHub stack registration or repository-policy change is used.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^dev$
  • ^preview$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 7ba9d058-de5c-4b8c-9fd3-f7bad74a0fd5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

리뷰 · 우선순위 66 / 80

설명

이 PR은 Track1 스택의 넷째(마지막 코드) 층입니다. base=codex/track1-03-websocket-592d / head=codex/track1-04-recovery-592d(a4edcc294), DRAFT입니다. 열려 있는 #3661(unreadable_encrypted_agent_task가 routed V2 subagent에서 간헐 실패)에 대한 진단만 추가하고, boolean API·캐시·admission gate·재시도 금지는 유지합니다. 지금 dev(24c761a05)의 recoverEncryptedAgentTaskPromise<boolean>만 돌려서, 400 unreadable_encrypted_agent_task를 받아도 envelope 문제인지 admission인지 upstream/cache인지 구분할 수 없습니다. #3597(trusted encrypted route fallback)이 이미 암호화 경로 보존 불변식을 깔아 둔 뒤라, 이 층은 그 경로를 넓히지 않고 거절 이유만 요청-로컬로 노출합니다.

AgentTaskRecoveryFailureReason 다섯 개(unsupported_envelope / admission_denied / recovery_unavailable / caller_cancelled / input_changed)와 recoverEncryptedAgentTaskWithResult를 추가합니다. 기존 recoverEncryptedAgentTask는 WithResult의 .recovered를 감싸 boolean 계약을 유지합니다. admittedRecovery는 null 대신 {admitted:false, reason} / {admitted:true, recovery}로 바뀌어 discard/restore도 같은 판별을 씁니다. assignment 없음은 abort면 caller_cancelled, 아니면 recovery_unavailable(캐시/singleflight capacity·upstream 실패를 포함, “업스트림을 쳤다”는 증거가 아님). inject 실패는 input_changed + cache discard. core.ts의 combo/direct 경로는 WithResult를 호출해 unreadableEncryptedAgentTaskResponse(reason)recovery_reason을 선택적으로 넣습니다. throw catch 시 reason을 비워 필드를 생략합니다 — 분류 못 한 실패를 가짜 enum으로 안 채웁니다.

범위 밖을 명시적으로 남긴 점이 좋습니다. granular transport error, multipart reconstruction, chatgpt.com 5xx 창 중 retry(#3661 본문)는 안 켭니다. 테스트는 cache/security/recovery/server-auth 네 파일을 넓혀 shared failure 격리, cancellation locality, pre-abort cache hit 거부, input_changed discard, flight-slot capacity(fetch 0회), unsupported envelope(무자격·내용 비노출), admission_denied가 타인 캐시를 못 읽/폐기하는지, boolean/replay/discard 동등성, 실패 모드가 모두 coarse recovery_unavailable인지, HTTP 400에 recovery_reason이 실리는지를 고정합니다. #3792가 손댄 server-auth compact 스트림 픽스처도 highWaterMark 0 pull 대기로 다듬어 flake를 줄였습니다. types/config 대분리와 무관합니다.

경로 src/server/responses/agent-task-recovery.ts - boolean API 유지 + WithResult 추가. admission-before-cache 주석/순서는 그대로라 plaintext oracle이 안 생긴다. recovery_unavailable이 넓다는 주석이 코드·docs에 반복된다.

경로 src/server/responses/core.ts unreadableEncryptedAgentTaskResponse - recovery_reason은 reason이 있을 때만 spread. 기존 code/message/type/400은 동일. combo와 handleResponsesInner 둘 다 FailureReason을 스레드한다. catch 시 undefined → 필드 생략.

경로 tests cache - shared 503 waiters가 각자 다른 result 객체를 받고 다른 parent key는 성공 유지. cancel이 shared AbortSignal을 안 죽이는지 확인. capacity 32 occupied 시 fetch===0 + recovery_unavailable.

경로 tests security - 자격 없는 요청의unsupported envelope도 fetch 0·입력 불변. mismatched chatgpt-account-id는 admission_denied이며 원 소유자 캐시 restore===1 유지.

경로 docs - 다섯 reason과 “recovery_unavailable ≠ upstream attempted”, “retry/envelope 확대 없음”을 명시. #3661 기대와 맞되 수정이 아니라 관측 개선임을 분명히 한다.

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

  • recovery_unavailable을 cache-full / upstream-5xx / malformed 등으로 더 쪼갤지(지금 의도적 거침), 클라이언트/대시보드가 이 다섯 값만으로 #3661 간헐 실패를 나눌 수 있는지
  • catch로 reason을 비우는 대신 recovery_unavailable로 채울지(관측 vs 정직성)
  • Track1 최종 CI green 후 이 진단만 먼저 올리고 #3661 재시도/envelope 후속을 따로 둘지
  • 외부 API에 recovery_reason을 안정 필드로 공지할지, ocx 내부/디버그 전제로 둘지

너의 추천
스택 tip 진단 층으로 최종 CI와 함께 올리는 쪽을 추천합니다. 동작/보안 gate를 바꾸지 않으면서 #3661 현장 분류에 필요한 최소 필드를 넣었고, 테스트가 oracle·캐시·취소 경계를 잘 고정합니다. 이 PR만으로 간헐 실패가 사라지진 않으니, 랜딩 후 recovery_reason 분포를 보고 재시도/envelope 후속을 여는 편이 맞습니다. types/config 대분리로 닫을 PR은 아닙니다. DRAFT 해제는 CI·ancestry 기록 후 스택 단위로.

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

@lidge-jun
lidge-jun marked this pull request as ready for review September 6, 2026 18:10
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 6, 2026 18:10
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 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-06T18:13:33.122601Z a4edcc2 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.

@lidge-jun
lidge-jun force-pushed the codex/track1-04-recovery-592d branch from a4edcc2 to 642b1ab Compare September 6, 2026 18:50
@lidge-jun
lidge-jun force-pushed the codex/track1-03-websocket-592d branch from 954d0df to 37ed82e Compare September 6, 2026 18:50
t and others added 2 commits September 7, 2026 04:06
Carry #3740 after stream correlation checks; preserve only bounded permitted metadata and dispose the refused socket. Post-commit and 5xx events cannot authorize a transport resend. Local validation deferred to final hosted CI.

Co-authored-by: Fred Amartey <43480311+FredAmartey@users.noreply.github.com>
Add request-local diagnostics for #3661 while retaining boolean API, cache behavior and admission gates. Granular transport errors, multipart reconstruction and retries remain deferred. Local validation is prohibited; final hosted workflow_dispatch verifies the complete stack.
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