shell: widen brace-rollback counter to u32 to avoid debug overflow panic

[ldkhang1201]

What does this PR do?

Fix a u8 overflow in NewLexer::rollback_braces (src/shell_parser/braces.rs) that aborts the debug build of Bun.$.braces(...) when the input has an unclosed outer { wrapping ≥256 levels of balanced {...}.

Same shape as #31010 (semver): direct port of var braces: u8 = 0 from the Zig original. Cargo's dev profile enables overflow-checks (release does not), so debug panics on the 256th += 1 while release wraps silently (256 → 0), matching Zig's ReleaseFast.

Reproducer:

import { $ } from "bun";
$.braces("{" + "{".repeat(300) + "x" + "}".repeat(300));
// debug: panic: attempt to add with overflow at src/shell_parser/braces.rs:1381
// release: exits cleanly, but the silent wrap kicks the rollback walk out of
//          "skip nested" state mid-span and converts inner `}` tokens to literal
//          `}` text — a latent Zig bug that this widening also fixes.

Why nothing upstream stops it: there is no depth cap on the input. MAX_NESTED_BRACES = 10 only sets SmallVec inline capacity (heap-spills on overflow rather than rejecting), and BunObject::braces caps expansion_count after tokenization, which is too late.

The fix: widen braces to u32 to match the surrounding index type i: u32. braces is only compared > 0 / == 0, never stored or cast, so a wider type is strictly more correct.

- let mut braces: u8 = 0;
+ let mut braces: u32 = 0;

How did you verify your code works?

Added a regression test in test/js/bun/shell/brace.test.ts that runs $.braces with 300 nested levels inside an unclosed outer brace in a subprocess (the failure is an uncatchable abort, same pattern as the #31010 test).

$ bun run build --asan=off test test/js/bun/shell/brace.test.ts
 13 pass
 0 fail
 16 expect() calls

With the u32 change reverted, the new test fails on debug with the expected stack:

panic_const_add_overflow
  rollback_braces (src/shell_parser/braces.rs)
  tokenize_impl
  tokenize
  bun_object::braces

The pre-existing 12 brace tests continue to pass.

Note on --asan=off: the default ASan-enabled debug build crashes at startup on Darwin 25 (AddressSanitizer: CHECK failed: sanitizer_procmaps_mac.cpp:214) — an environmental ASan/macOS issue unrelated to this change.

🤖 Generated with Claude Code

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9828f2e9-7b47-4f83-91bc-efdace177b2e

📥 Commits

Reviewing files that changed from the base of the PR and between 7d44b5b and 70d461e.

📒 Files selected for processing (1)

• test/js/bun/shell/brace.test.ts

---

Walkthrough

The PR widens the internal brace nesting counter in rollback_braces from u8 to u32 to prevent debug overflow-check panics when handling deeply nested unclosed brace expansions. A regression test validates that deeply nested (300 pairs) unclosed braces no longer cause crashes.

Changes

Brace nesting overflow fix

Layer / File(s)	Summary
Brace counter overflow fix src/shell_parser/braces.rs	rollback_braces counter widened from u8 to u32 to prevent overflow panics in debug builds when brace nesting exceeds 255 levels.
Regression test for deeply-nested braces test/js/bun/shell/brace.test.ts	Test imports bunEnv and bunExe from harness and adds async regression test that spawns a subprocess running $.braces on input with 300 nested balanced pairs inside an unmatched opening brace, asserting exit code 0 and no stdout/stderr.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: widening the brace-rollback counter from u8 to u32 to avoid debug overflow panic.
Description check	✅ Passed	The description fully addresses both required template sections with detailed context, reproducer code, explanation of the fix, verification steps, and test results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/shell/brace.test.ts`:
- Line 97: Remove the explicit per-test timeout argument (30_000) from the test
in brace.test.ts: locate the test(...) or it(...) invocation that currently ends
with "}, 30_000);" and change it to call the test function without the timeout
parameter so Bun's default timeout is used; ensure only the numeric timeout
argument is removed and the test callback (the function referenced by the
closing "}") remains unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 828a34ef-0a07-410c-8d82-0c4490a026ef

📥 Commits

Reviewing files that changed from the base of the PR and between fba43af and 0854936.

📒 Files selected for processing (2)

• src/shell_parser/braces.rs
• test/js/bun/shell/brace.test.ts

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/shell/brace.test.ts`:
- Around line 88-96: The test currently awaits proc.stderr.text() and
proc.exited together and asserts stderr and exitCode; change it to capture and
assert stdout (proc.stdout.text()) and stderr first, then await proc.exited as
the final assertion to improve subprocess failure diagnostics—locate the
Bun.spawn call and the Promise.all usage with proc, replace or reorder to call
proc.stdout.text() and proc.stderr.text(), assert stdout/stderr, then await
proc.exited and assert exit code.
- Around line 86-87: Replace the use of String.prototype.repeat when building
the large test string: instead of using "{" + "{".repeat(depth) + "x" +
"}".repeat(depth) to construct input, allocate a Buffer with the repeat count
and fill character and call toString() for the repeated segments, preserving the
same concatenation and the variables depth and input so the test behavior is
unchanged (i.e., use Buffer.alloc(depth, "{").toString() for the left braces and
Buffer.alloc(depth, "}").toString() for the right braces while keeping the "x"
in the middle).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bfb1cf19-4384-435c-a426-2b72f94dab3b

📥 Commits

Reviewing files that changed from the base of the PR and between 0854936 and 7d44b5b.

📒 Files selected for processing (1)

• test/js/bun/shell/brace.test.ts