fix(noema): bound external JSON oversize cancellation latency - #668
Conversation
|
Warning Review limit reachedNext included review available in 26 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
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. Comment |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review on efaaebae161a71b1274f5aa0f26e27376a3edb3a: verified the base→head diff remains confined to the external bounded OIDC/GitHub JSON reader plus hostile cancellation-liveness tests. The shared ignoreCancellationFailure sink preserves the existing timeout semantics while preventing an already-decided 65,536-byte oversize failure from awaiting hostile cancellation cleanup. The coverage follow-up exercises rejecting cleanup without weakening the 100% gate. Exact-head ci 34684572111, reviewer-ci 34684572034, Security Scan 34684572119, and patch-validator-image 34684572074 are terminal SUCCESS; unresolved review threads are zero. CodeRabbit is rate-limited on this head, so its absence is not treated as approval. COMMENT only; no self-approval.
…ancel-liveness-authority-20260912 docs(noema): converge #668 cancellation-liveness authority
Hosted RED
Protected
main@70cbe786113ad14ba8d61e2469f9b4725558c33aawaitedreader.cancel()after the external OIDC/GitHub JSON reader had already determined that a chunk crossed its 65,536-byte ceiling. A hostile peer could therefore stretch an otherwise immediate fail-closed oversize decision until the 10-second absolute deadline.Test-only exact
bc6f96fba17a717c49ed3484f154a7845d2a7cdeadded a hostile discovery-response case whose oversize chunk is available immediately while the underlying cancellation promise never settles. Hosted application CI34684042367passed exact checkout, toolchain/lock controls, install, and release typecheck, then failed at release tests withexpected "failsafe" to be "response"(4727 passed / 1 failed). On that RED exact, reviewer-ci, Security Scan, and patch-validator-image were independently successful.Minimal causal repair
The runtime repair starts oversize cancellation as best-effort cleanup instead of awaiting it before throwing the already-decided size-limit failure. Exact
cdbd970db8bb65c9a578ec818ec1a2c189fd1178then reused oneignoreCancellationFailuresink across timeout and oversize cleanup, preserving the existing timeout semantics while avoiding duplicate inline handlers.Exact
758fed469fb128de7f1ffe69eaf4dfd8871c0ce0had already proved all 4,728 behavioral tests, but the 100% coverage gate correctly rejected the then-unexecuted cancellation-rejection handler (src/index.ts:451:41, global statements 99.98% / functions 99.91%). Current exactefaaebae161a71b1274f5aa0f26e27376a3edb3aadds the hostile oversize-cancellation rejection coverage and preserves prompt 502 OIDC classification plus reader-lock release.Base→head changes only
src/index.tsandtest/oidc-external-json-read-deadline.test.ts. The 65,536-byte ceiling, fixed retained buffer, absolute 10-second deadline, fatal UTF-8/duplicate-key/JSON admission, OIDC/GitHub trust semantics, and GitHub App authority remain unchanged. No provider/model routing, destination/outbound authority, credential authority, quarantine/security authority, or foreign domain truth moves into Noema.Exact-head GREEN
Current exact
efaaebae161a71b1274f5aa0f26e27376a3edb3ais terminal GREEN on all required hosted lanes:34684572111346845720343468457211934684572074Reviewer-ci retains the repository 100% line/branch and docstring gates. Unresolved review threads are zero. CodeRabbit is rate-limited on this exact, so its absence is not treated as approval; a COMMENT-only exact-head review was recorded instead, with no self-approval.