Skip to content

fix(compact): preserve progress and accepted request lifetime - #3792

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

fix(compact): preserve progress and accepted request lifetime#3792
lidge-jun merged 2 commits into
devfrom
codex/track1-02-compact-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 17b1899a6acfaf63720237be702eb7489625837b 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: Progress/noise, ciphertext, backpressure, partial request bodies, stalled native response bodies, progress rearming, cancellation ordering, size/byte/header preservation and account cleanup.
  • 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.

Co-authored-by: mashfromband matsumoto.yukuhashi@gmail.com
Co-authored-by: Hylouis233 88263959+Hylouis233@users.noreply.github.com

@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: 809e0ee0-3638-445f-adfd-ff7d5bad594e

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.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 68 / 80

설명

이 PR은 Track1 스택의 둘째 층입니다. base=codex/track1-01-quota-592d / head=codex/track1-02-compact-592d(6bb5d8da0), DRAFT입니다. 열려 있는 #3736(compaction progress + 600s stall)과 #3744(compact idle timeout opt-out)를 가져와, stall 기본값은 300초로 유지하고 encrypted compaction 출력은 그대로 두도록 줄인 판입니다. 지금 dev(24c761a05, 2.45.0)만 놓고 보면 compact 경로와 buffered progress 두 곳이 아직 빠져 있습니다. #3791 쿼터 층 위에 쌓이므로 이 PR만 단독으로 dev에 붙이면 base가 어긋납니다.

현재 dev/v1/responses는 이미 onRequestBodyRead → disableResponsesRequestTimeout을 쓰지만(src/server/index.ts), /v1/responses/compacthandleResponsesCompact(...)만 호출하고 idle timeout을 풀지 않습니다. 원격 compact는 응답을 오래 버퍼링할 수 있어서, 본문을 다 읽기 전에 listener idle에 끊기거나, 본문 수락 뒤에도 서버 idle에 잘릴 수 있습니다. 이 PR은 HandleResponsesCompactOptions.onRequestBodyRead를 추가하고, model 문자열이 검증된 뒤에만 콜백을 호출합니다. 부분/깨진 JSON·빈 model은 400을 주고 idle guard를 유지합니다. src/server/index.ts compact 분기만 그 콜백으로 timeout을 끕니다. 클라이언트 취소와 upstream operation deadline은 그대로입니다.

둘째 축은 buffered routed compaction progress입니다. createResponsesPassthroughAdapter(src/adapters/openai-responses.ts)가 response.output_text.delta / reasoning_summary_text.delta / reasoning_text.delta의 비어 있지 않은 delta를 heartbeat로만 yield합니다. 부분 요약 텍스트는 클라이언트에 안 나갑니다. 주석·빈 delta·gateway keepalive·unknown delta는 stall을 리셋하지 않습니다. completedSeen 뒤에는 heartbeat를 안 보내서 late delta가 progress로 위장하지 않습니다. 기본 stall 300초·encrypted_content 스냅샷 우선은 유지합니다. #3597(trusted encrypted route fallback)과 직접 충돌하지 않고, ciphertext를 새로 해석하거나 경로를 바꾸지 않습니다.

테스트는 새 tests/responses/compaction-progress.test.ts(205줄)가 heartbeat·stall noise·snapshot/usage/ciphertext·취소 시 reader cancel·failed/incomplete 후 성공 요약 미flush를 고정하고, tests/server/server-auth.test.ts가 compact idle-guard 타이밍(부분 body → accepted=0, 완성 후 accepted=1, 거절 body는 callback 없음)을 검증합니다. layout 등록도 있습니다. #3736의 600s 상향은 가져오지 않았으므로, 긴 compact가 300s에 걸리는 환경이면 별도 판단이 남습니다. types/config 대분리와 무관합니다.

경로 src/server/responses/compact.ts onRequestBodyRead - model 가드 통과 뒤에만 호출한다. "{}"/{"model":""} 등은 400 + callback 미호출. responses 경로와 대칭이지만 compact 전용 options로 묶었다.

경로 src/server/index.ts compact 분기 - handleResponsesCompact(..., { onRequestBodyRead: () => disableResponsesRequestTimeout(...) }). 다른 compact/fallback 호출부(compact.ts 내부 재귀 등)는 콜백 없이 두는지 한 번만 보면 좋다. 서버 listener에 묶인 HTTP 진입점만 timeout owner라는 전제면 맞다.

경로 src/adapters/openai-responses.ts parseStream - nonempty text/reasoning delta → yield { type: "heartbeat" }. bridge stall watchdog만 리셋하고 내용은 안 샌다. 고빈도 작은 delta면 heartbeat가 많아질 수 있으나, buffered compact 특성상 허용 범위로 보인다. completedSeen 가드가 late delta 오인식을 막는다.

경로 tests/responses/compaction-progress.test.ts - stallTimeoutSec=1 픽스처로 “진행 중 cleanupCalls===0, 완료 후 ===1”을 고정한다. PR 본문이 말한 “progress 중 zero cleanup / completion 후 exactly one” 회귀와 맞다. ciphertext-only + reasoning progress가 가짜 summary text를 만들지 않는 케이스도 있다.

경로 docs architecture / structure - idle 해제는 “완전한 JSON + valid model 이후”, progress는 “비어 있지 않은 text/reasoning만, keepalive 제외, 300s 유지, encrypted 불변”으로 적혀 구현과 같다.

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

  • #3736처럼 stall을 600s로 올릴지, Track1은 300s 고정으로 두고 관측 후 올릴지
  • compact 내부 fallback handleResponsesCompact 재호출에도 idle 콜백을 전달할지(HTTP listener가 이미 풀렸다면 불필요)
  • #3791과 통째 랜딩할지, compact만 cherry-pick할지(base가 track1-01이라 실무상 통째가 단순)
  • 최종 CI 전에 이 층만 따로 돌릴 필요가 있는지(본문은 lower-head 단독 검증을 의도적으로 안 함)

너의 추천
#3791 위에 쌓인 DRAFT 02로 두고, 최종 CI green이면 스택과 함께 올리는 쪽을 추천합니다. compact idle 미해제는 긴 원격 compact에서 사용자에게 끊김으로 보이고, progress heartbeat는 부분 요약을 안 새리면서 stall만 막는 깔끔한 절충입니다. 600s stall이 필요하면 후속 PR로 분리하는 편이 Track1 범위를 지키기에 낫습니다. types/config 대분리로 닫을 필요는 없습니다.

이 댓글은 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:14:18.817707Z 6bb5d8d 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6bb5d8da08

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/server/responses/compact.ts
Carry the quota-attribution slice of #3769. Keep canonical compact 404 fallback deferred pending identity and history preservation. Local checks deferred to final hosted CI by maintainer instruction.

Co-authored-by: Siddarth Reddy <221318067+ideabib@users.noreply.github.com>
@lidge-jun
lidge-jun force-pushed the codex/track1-01-quota-592d branch from a1fb1e2 to d17d323 Compare September 6, 2026 18:50
@lidge-jun
lidge-jun force-pushed the codex/track1-02-compact-592d branch from 6bb5d8d to b33dc64 Compare September 6, 2026 18:50
Carry the progress portion of #3736 and adapt #3744 to release listener idle protection only after a complete valid request body. Preserve the 300-second default and existing encrypted output.

Local validation deferred to the final hosted CI by maintainer instruction.

Co-authored-by: mashfromband <matsumoto.yukuhashi@gmail.com>
Co-authored-by: Hylouis233 <88263959+Hylouis233@users.noreply.github.com>
@lidge-jun
lidge-jun force-pushed the codex/track1-02-compact-592d branch from b33dc64 to 17b1899 Compare September 6, 2026 19:06
@lidge-jun
lidge-jun changed the base branch from codex/track1-01-quota-592d to dev September 6, 2026 19:29
@lidge-jun
lidge-jun merged commit 823ffeb into dev Sep 6, 2026
17 checks passed
@lidge-jun
lidge-jun deleted the codex/track1-02-compact-592d branch September 6, 2026 19:30
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