Repository navigation
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
| Layer / File(s) | Summary |
|---|---|
Normalized JSON byte accounting src/server/request-decompress.ts, structure/transports/byte-accounting.md, tests/server/inbound-body-admission.test.ts |
At line 337, parsedBytes now uses the UTF-8 size of serialized parsed JSON. The documentation at line 108 and the test name and expectation at lines 593 and 604 reflect normalized JSON-byte retention without requiring a translator budget. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: lidge-jun, oocheol
Merge Risk: 
1f55b
-
The normalized JSON accounting change is consistent with the documented contract and introduces no established merge-blocking risk.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: accounting for normalized inbound JSON when measuring inbound-body usage. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
✅ Deterministic PR hygiene checks passed. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
No description provided. |
|
Taking ownership of the requested review/test follow-up at Current remote head, reviews, CI, and visible activity are being checked in isolated workspaces. The old task's unpublished workspace is not accessible from this session, so I am not claiming that it is idle or empty. Any publication will preserve the current commit ancestry and re-check the remote head before a normal fast-forward update. These PRs stay separate; this ACK does not authorize a merge, force push, installed-app replacement, or security/permission setting change. |
|
Follow-up review completed at unchanged HEAD Existing exact-head CI 40988545901 and Catalog contracts 40988545881 completed successfully. This is normalized serialized-size accounting, not a measurement of exact process peak RSS. No new commit, hosted rerun, Windows execution or review approval is implied. The original Codex Task remains a handoff source; its unpublished workspace was not accessible, as recorded in the ownership ACK. |
…ly suite (lidge-jun#8016) * test(claude): report the hardened local-CA ACL in the first-party apply suite The local-CA ACL inspection spawns PowerShell under the shared 30 s principal budget, which a loaded hosted Windows runner exhausted (windows 1/11 of lane=all run 41073826161: local_ca_path_unsafe after 30095 ms). This suite covers first-party apply; inject the private ACL for the real current SID through the existing seam. Native inspection stays covered by claude-intercept-local-ca-windows.test.ts. * test(claude): read the cached SID inside the injected ACL runner and reset it in finally Review: no separate principal lookup before the environment snapshot; reset the seam even if the hardening drain throws. --------- Co-authored-by: JUN <243035832+lidge-jun@users.noreply.github.com>
* fix: compare file identity with bigint stat
NTFS file IDs can exceed 2^53, where adjacent IDs are equal as JS numbers. Take dev/ino from stat calls made with { bigint: true } wherever a module compares file identity, and add a shared FileIdentity helper.
* test: register file-identity tests in a layout shard and scan stat calls with balanced parentheses
* test: resolve guard scopes by declaration and ignore strings and comments
* test: mask template text and regex literals, require same-line method bodies and an exact bigint options argument
* test: recognize regex literals after keywords in the identity tripwire
* fix(oauth): name the best-effort catches on the rewritten lock line
* fix(oauth): name the best-effort metadata parse catch beside the lock line
---------
Co-authored-by: JUN <243035832+lidge-jun@users.noreply.github.com>
Current integration at
a943d91ea56204605b15c47c67a533bd9ead1994Integrated upstream dev
86b72a689c21b97b0a425303e64b54ccceeb530awhile preserving the original three-file fix. The bounded JSON reader always measures normalized serialized bytes before reducing its admission reservation, including routes without a translator budget. The retained charge remains the maximum of wire, decoded and normalized size and is still capped by the original allowance.Native Windows, pinned Bun1.4.4 with isolated test homes:
inbound-body-admission.test.tsandrequest-decompress.test.tspass 130 cases / 365 assertions, including normalized numeric expansion with and without a translator budget. Typecheck covers 2,253 source files, and structure/privacy/file-size gates plus diff whitespace checks pass. Independent read-only review confirmed the fix is absent from current dev and found no conflicting upstream fix. No new production edit was required after integration. Exact-head Cross-platform CI41131520818 and Catalog41131520923 succeeded; all required checks pass and no unresolved review thread remains. Ready is verified separately; upstream publication remains with Duri. Historical validation below is not carried forward as this head's CI. Maintainer/security acceptance remains separate.Motivation
TranslatorBudgetreclaimed capacity based on decoded wire length instead of the normalized JSON serialization, enabling aggregate memory amplification.JSON.stringifyresult) can be substantially larger than the incoming bytes.Description
Buffer.byteLength(JSON.stringify(parsed), "utf8")inreadBoundedJsonRequestBodybefore observing or shrinking the inbound-body reservation.updateInboundBodyReservation(req, Math.max(raw.byteLength, decoded.byteLength, parsedBytes))so the largest observed representation is charged.tests/server/inbound-body-admission.test.tsto assert the normalized JSON size is retained whether or not a translator budget is supplied.structure/transports/byte-accounting.mdto state that normalized JSON bytes are measured for every affected route, not only when a translator budget exists.Testing
tests/server/inbound-body-admission.test.tsandtests/usage/request-decompress.test.tswith Bun; both passed (all focused tests green).bun run typecheck— no type errors were reported.bun run structure:check— checks passed.bun run test:changedwas not runnable in this checkout due to a missing comparison ref; full change-set CI remains the final gate.Codex Task
Summary by CodeRabbit