jsc: throw RangeError from JSON.parse when a string value cannot be allocated - #35194
Open
robobun wants to merge 6 commits into
Open
jsc: throw RangeError from JSON.parse when a string value cannot be allocated#35194robobun wants to merge 6 commits into
robobun wants to merge 6 commits into
Claude / Claude Code Review
completed
Jul 22, 2026 in 8m 55s
Code review found 1 important issue
Found 3 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | scripts/build/deps/webkit.ts:11 |
WEBKIT_VERSION pinned to ephemeral preview tag |
| 🟡 Nit | test/js/bun/util/json-parse-oom.test.ts:67-68 |
Explicit 30s per-test timeout violates test/CLAUDE.md |
Annotations
Check failure on line 11 in scripts/build/deps/webkit.ts
claude / Claude Code Review
WEBKIT_VERSION pinned to ephemeral preview tag
`WEBKIT_VERSION` is pinned to `autobuild-preview-pr-317-d9b06a28`, a PR-preview release tag rather than a merged `oven-sh/WebKit` main sha (contradicting the comment directly above). Preview-PR release artifacts can be garbage-collected once WebKit#317 merges/closes, at which point `prebuiltUrl()` will 404 and fresh clones of Bun stop building. The PR description already notes this needs updating — flagging so it isn't merged before the pin moves to the merged main sha.
Check warning on line 68 in test/js/bun/util/json-parse-oom.test.ts
claude / Claude Code Review
Explicit 30s per-test timeout violates test/CLAUDE.md
nit: `test/CLAUDE.md:120` says "**CRITICAL**: Do not set a timeout on tests. Bun already has timeouts", and REVIEW.md says to shrink the workload rather than raise per-test timeouts. Consider dropping the explicit `30_000` and reducing the sweep — e.g. two sizes instead of four, and/or a lower `limitKiB` (2–3 GiB) so each fixture has less address space to fill before the halving loop bottoms out.
Loading