fix(logs): bound JSON and error response inspection

[luvs01]

Summary

• Replace the eager response.text() / re-encoding path in responseWithDeferredRequestLog with one pull-driven relay that forwards the original bytes immediately. Slow downstream readers do not cause a separate inspection branch to drain ahead. Preserve the response status, status text, headers, and existing log parser/accounting ownership.
• Bound the retained inspection copy to 32 MiB for JSON and an 8 KiB byte prefix for other error bodies. JSON is inspected only at clean EOF when the entire document fits. On overflow, discard its inspection copy and continue forwarding all bytes; on cancellation/read failure, never parse a partial JSON document. These are per-response inspection-copy bounds, not a process-memory ceiling or an output limit.
• Finalize EOF/read-error/client-cancel exactly once, using the original status / 502 / 499 respectively. Propagate the cancellation reason, release the source reader, avoid awaiting tee cancellation, and isolate optional logging failures from delivery. Add 25 shared body-lifecycle cases, six deferred-log integration cases, and a late SSE terminal regression; document the byte-accounting contract.

This is a fresh implementation of the remaining non-streaming issue described by luvs01/opencodex#177, incorporating the earlier non-JSON work in luvs01/opencodex#106. It is based on dev 3070d64 rather than carrying the old consolidated branch wholesale. The three existing modified files were confirmed unchanged between the initial inspected base and this refreshed base before carrying the five-file change set.

SSE scope is deliberately unchanged. The old proposal's total-stream inspection cutoff is not carried: detaching a healthy long SSE observer can lose its terminal, usage, and completed-response callback. Current per-frame/output-item bounds and bounded post-disconnect drain ownership remain intact. A regression explicitly places an SSE terminal after more than the JSON inspection budget. The source fork PR remains unchanged.

Verification

Published implementation commit: 02be039.

Executed locally

• 25 passed / 0 failed on Node v22.16.0, executing the exact shared cases in tests/helpers/response-log-body-cases.ts against the new production helper. A temporary node:test runner and extensionless-TypeScript resolver were used with --experimental-strip-types; these local harness files are not part of the PR. This is WHATWG-stream helper evidence, not a Bun integration or full-suite result.
• Isolated strict TypeScript checks passed for src/server/response-log-body.ts and the shared case module, including --noUncheckedIndexedAccess. The check used ES2022, DOM and Node test types with bundler module resolution. This is not a repository-wide typecheck claim.
• Verified the original source and test files against Git blob identities, inspected the focused relay diff, and verified the uploaded source/helper/test blobs against the tested local copies. git diff --no-index --check reported no whitespace findings for the two existing TypeScript files.
• Cases cover early delivery, backpressure, exact/overflow byte limits, an actual body over 32 MiB, invalid/split UTF-8 preservation, JSON truncation, same-reason cancellation, EOF/error/cancel races, reader release, logging failures, cancellation rejection, copied inspection storage, and a tee sibling that continues after the inspected branch is cancelled.

Not yet executed / required before review readiness

• Bun is unavailable in this sandbox and package-download/network access is unavailable. The six new deferred-log integration cases and the long-SSE case have not run, nor have the existing Bun tests.
• Run bun test tests/server/consume-for-inspection-cancel.test.ts, the repository typecheck, bun run test:changed, and the required full-suite gate before marking review-ready.
• Repository privacy/security checks, bun run structure:check, the remaining owner-document/public-documentation synchronization audit, and hosted CI are pending. The specific byte-accounting document is updated; a complete documentation-gate pass is not claimed.
• No live provider requests, service/configuration changes, workflow/dependency changes, merge, or approval were performed. Maintainer review and CI remain separate from the limited local evidence above.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed. The byte-accounting contract is updated; the complete owner-document/public-documentation audit remains pending.
• 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]

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

⏳ DRAFT

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

What to do

• Tick all four boxes in the PR description once you're done (currently 1/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.

1/4 boxes ticked.

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

[lidge-jun]

리뷰 · 우선순위 68 / 80

이 PR은 비-SSE JSON/에러 응답 로그 검사에서 response.text()로 전체를 읽던 경로를 풀 기반 relayResponseLogBody로 바꾸고, JSON 32 MiB·기타 에러 8 KiB로 진단 복사본만 남깁니다. 지금 dev의 responseWithDeferredRequestLog가 실제로 그 text() 패턴을 쓰고 있어 문제 인식은 맞습니다. 다만 같은 주제·같은 날의 #4775가 이미 (a) 동일한 비스트림 바운딩, (b) SSE teeWithBoundedInspection, (c) passthrough-delivery 연결, (d) 더 큰 검사 테스트 스위트를 포함한 상위 집합으로 열려 있고, 심지어 현재 dev b3035fe까지 머지해 둔 상태입니다. 공통 파일은 relay.ts/response-log-body.ts/byte-accounting.md이고 API 이름도 relayResponseLogBody vs createBoundedResponseLogBody로 갈라져 두 번 랜딩하면 충돌만 납니다.

#4742만의 자산은 tests/helpers/response-log-body-cases.ts와 consume-for-inspection-cancel.test.ts 갱신 정도입니다. #4775의 response-log-inspection.test.ts가 같은 수명(EOF/error/cancel)을 더 넓게 덮습니다. 따라서 현재 dev 기준으로 이 PR을 따로 살릴 이유는 거의 없고, #4775에 흡수·대체하는 편이 맞습니다. draft입니다.

response-log-body.ts relayResponseLogBody - 기하 증가 버퍼. JSON overflow 시 inspectionUnavailable. cancel 시 await 없이 reader.cancel. 방향은 건전.
relay.ts - #4775와 같은 함수를 다른 심볼로 패치. 동시 머지 불가에 가깝음.
테스트 helpers - #4775에 비슷한 케이스가 없으면 일부 assertion만 #4775로 이식할지 검토.
SSE - 이 PR은 tee 페이싱이 없음. “로그 검사 바운딩” 이슈를 이 PR만으로 닫으면 SSE 쪽이 남음.

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

• fix(logs): bound response inspection without losing SSE finality #4775 머지 전 지금 바로 superseded 클로즈할지, fix(logs): bound response inspection without losing SSE finality #4775 CI 그린 후 닫을지.
• fix(logs): bound JSON and error response inspection #4742 전용 테스트 helpers 중 #4775에 없는 케이스가 있는지 5분 대조.

너의 추천
CLOSE(superseded by #4775). 따로 리베이스·머지하지 마세요. #4775가 먼저 랜딩되면 이 PR에 Landed via #4775/superseded 코멘트로 닫으면 됩니다. #4775가 폐기될 때에만 이 PR을 부활시키세요.

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

[lidge-jun]

Closing as superseded by #4775, following the owner review above and the current dev backlog consolidation. #4775 retains the bounded non-stream inspection work and also covers SSE inspection pacing and terminal ownership; maintaining two competing implementations of relay.ts/response-log-body.ts is not a separate delivery plan.

This is a duplicate/supersession close, NOT a claim that #4775 has merged, passed its remaining gates, or shipped in a release. #4775 remains the active implementation and review target. Any additional useful assertions from this PR can be retained there; the commits and discussion here remain available if the successor is withdrawn. No branch is deleted.