Skip to content

fix(diagnostics): distinguish inbound body size evidence - #3819

Merged
lidge-jun merged 3 commits into
devfrom
codex/platform-lane4-body
Sep 7, 2026
Merged

fix(diagnostics): distinguish inbound body size evidence#3819
lidge-jun merged 3 commits into
devfrom
codex/platform-lane4-body

Conversation

@lidge-jun

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

Copy link
Copy Markdown
Owner

Summary

Oversized request errors distinguish declared wire length, observed wire lower bounds, exact decoded buffers and decoder lower bounds. Fixed categories and numeric values travel through the existing HTTP 413 mapping without retaining rejected payloads or changing the 256 MiB cap and retry behavior.

Follow-up to #3573, with thanks to @nowhere1975 for the incident measurements. The issue remains open for the actual compact-attempt evidence. Layer 2, based on #3818.

Verification

  • Final candidate: 6f2ad1ef32c113ff5827c58959c85cefa923b15c. Cross-platform CI, lane=all passed (26 successful jobs). The verification ref points to the same commit as the final PR.
  • Independent Astra high implementation review completed; container lifecycle and workflow changes received explicit independent security/contract reviews.
  • Documentation build passed on isolated Linux at this exact source snapshot: 425 pages.
  • Local test suites, typecheck, and Docker execution were not run, as instructed by the maintainer. Pushes use --no-verify.
  • Lower-layer expensive CI is deliberately omitted; only failed final checks triggered narrowed diagnosis. No lower-layer green is fabricated.
  • Earlier combined runs passed all six Windows shards, four Linux shards, and two macOS shards. They exposed Docker harness assumptions, a TypeScript 7 CLI requirement, and then a real container shutdown/catalog persistence defect. These were corrected. One earlier macOS full-control run failed an unchanged Cursor watchdog test; its cause remains unproven, with no timeout/skip workaround. Superseded runs are not final-head success evidence.

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.

Maintainer integration

The repository owner explicitly authorized admin integration of this manual chain into dev. The final combined head 6f2ad1ef32c113ff5827c58959c85cefa923b15c passed all 26 CI jobs. One cancelled macOS shard was rerun with unchanged source, commands and limits; its original stall remains recorded rather than claimed fixed.

Independent integration review found no conflict with current dev a5f9c3497. A clean merge produced tree 2c51d5e8042011fe8362e267b224016c6061d684; the relevant OAuth/body/container tests passed there on isolated Linux (129 passed, zero failures). This focused result is distinct from the full CI proof on the combined PR head. The branch commits preserve original contributors through Co-authored-by trailers.

t and others added 3 commits September 7, 2026 07:35
Record scoped roadmap and independent plan audit. Local suites are excluded by owner instruction; final remote CI will verify the combined candidate.
Carry the remaining teardown fix from #3383 (51726d2) and verify cleanup ordering with a held asynchronous runner. Final combined CI will validate the stack; no local suite was run.

Co-authored-by: x3M3x <amroeid1999@gmail.com>
…ip ci]

Report declared wire length, observed lower bounds, and decoded sizes without reading rejected payloads further or changing admission and retry policy. Follow up on #3573; thanks to @nowhere1975 for the content-free incident measurements. The issue remains open for exact compact-attempt evidence. Independent source review passed; final remote CI pending.
@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: 0875c9c9-df44-4dde-95f7-07c8f1bfa0da

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

리뷰 · 우선순위 60 / 80

이 PR은 플랫폼 체인 2층입니다. 베이스는 #3818의 codex/platform-lane4-oauth 이고, 헤드가 codex/platform-lane4-body 입니다. 하는 일은 요청 본문이 너무 커서 거절될 때, 그 “얼마나 컸는지” 증거가 어디서 왔는지 종류를 나누는 것입니다.

지금 devDecompressedBodyTooLargeError 는 bytes 숫자 하나와 limit만 들고, 메시지에는 limit만 넣습니다. 그런데 그 bytes가 가끔은 보낸 사람이 선언한 Content-Length이고, 가끔은 읽다 만 와이어 하한이고, 가끔은 디코더가 준 정확한 길이이고, 가끔은 limit+1 하한입니다. 운영자가 로그를 보면 숫자가 같아 보여도 뜻이 다릅니다. #3573 후속으로 그 혼동을 줄이려는 PR입니다.

네 가지 측정 카테고리를 씁니다. declared_wire(선언만 보고 읽기 전 거절), observed_wire_lower_bound(읽다 멈춰서 본 하한), decoded_exact(디코더/아이덴티티가 준 정확한 버퍼), decoded_lower_bound(팽창 중 cap에 걸려 limit+1). 메시지 끝에 [measurement=…; bytes=…] 접미사를 붙이되, 고정 카테고리와 유한 숫자만 허용합니다. 이상한 문자열이나 NaN/Infinity는 접미사를 아예 안 붙입니다. HTTP 413 매핑, 256 MiB 한도, 재시도 동작은 그대로입니다. 거절된 본문을 더 읽거나 보관하지 않습니다.

문서 docs-site/.../proxy-formats.md 도 inbound 256 MiB와 compact 응답 32 MiB를 분명히 갈랐습니다. Bun 리스너가 앱 진단보다 먼저 413을 줄 수 있다는 한계도 적어 두었습니다. 테스트는 레거시 1·2인자 생성자 호환, 비정상 카테고리 차단, gzip/zstd/deflate(래핑·raw·Bun) lower_bound, 선언 oversize 시 pull 0·cancel 1까지 확인합니다. 기존 decodeRequestErrorResponse 경로로 413/invalid_request_error가 유지되는지도 봅니다.

현재 dev 런타임(#3808)이나 새 프로바이더/인증 기본값(#3689)과는 무관합니다. types/config 분할에도 걸리지 않습니다. 이 층만의 가치는 “거절 로그를 사람이 읽을 수 있게 만드는 것”이고, 실제 과거 compact 페이로드 크기를 증명하지는 않습니다. 그래서 #3573을 닫으면 안 됩니다.

경로 src/server/request-decompress.ts assertBodySizeWithinLimit(raw, …, observed_wire_lower_bound) - 스트리밍 리더가 이미 cap 아래에서 성공적으로 읽은 뒤의 이중 검사에도 lower_bound 라벨을 씁니다. 실제로 여기가 터진다면 보유 버퍼 길이는 exact wire에 가깝습니다. 보수적 라벨이라 치명적이진 않지만, 성공 경로 가드에는 decoded_exact/observed_wire_exact 쪽이 더 정직할 수 있습니다.
라인 DecompressedBodyTooLargeError 생성자 - measurement 기본 null로 레거시 호환은 좋습니다. limit를 readonly 필드로 노출한 것도 테스트/진단에 도움됩니다.
경로 tests/usage/request-decompress.test.ts - 메시지 길이 <200 단언은 접미사 폭주를 막지만, MAX_VALUE 과학적 표기 케이스처럼 긴 숫자는 허용합니다. 의와 맞습니다.
경로 docs-site/.../proxy-formats.md - compact 32 MiB 표를 inbound 설명 뒤로 밀었고 문장이 자연스럽습니다.

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

  • #3573을 열린 채로 둘지(권장) , 진단 개선만으로 partial close/라벨만 조정할지. 라벨 변경은 이 리뷰 범위 밖이니 메인테이너 손으로.
  • 성공 읽기 후 assert에 observed_wire_lower_bound를 쓸지 더 좁은 이름을 추가할지.
  • 최종 CI 초록 전 이 층만 cherry-pick 머지할지, 체인 통째 검증 후 bottom-up할지(계획이 후자).

너의 추천
#3818 머지 다음에 #3819를 merge commit으로 올리세요. 413/256MiB/재시도 불변은 유지된 채라 운영 리스크가 작습니다. #3573은 실제 compact 거절 바이트 증거가 오기 전까지 열린 채로 두고, 커밋/PR 본문의 @nowhere1975 감사 문구만 남기면 됩니다. 상위 Docker 층(#3822/#3823)과 파일 충돌 없습니다.

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

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

chatgpt-codex-connector Bot commented Sep 7, 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 ⚠️ Failed 2026-09-07T00:48:42.176310Z cb8ac02 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 changed the base branch from codex/platform-lane4-oauth to dev September 7, 2026 00:48
@lidge-jun
lidge-jun merged commit da18da4 into dev Sep 7, 2026
25 of 28 checks passed
@lidge-jun
lidge-jun deleted the codex/platform-lane4-body branch September 7, 2026 00:48
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