shell(rm): reject -i/-I/--interactive by name through the shared unsupported-option path

[robobun]

Problem

• rm -i file in the Bun shell fails with rm: "-i" is not supported yet and no trailing newline, so whatever is written to stderr next is glued onto the same line (rm: "-i" is not supported yetnext line). Every other rm diagnostic (usage, illegal option -- x, No such file or directory, may not be removed) ends in a newline.
• The message says -i whichever option was given (-I, --interactive=once and --interactive=always all print it), and it is a one-off string: the other builtins report options they do not implement through Builtin::fail_parse, as <cmd>: unsupported option, please open a GitHub issue -- <option as given>\n.
• Cause: the RmParseFlag::Done arm of Rm::next (src/runtime/shell/builtin/rm.rs) writes that literal whenever opts.prompt_behaviour is not Never. The PromptBehaviour enum it checks had Once { removed_count } / Always variants that nothing else read, and the short-flag arms of parse_flag set them the opposite way round from their doc comments and from the --interactive=once / --interactive=always arms (-i selected Once, -I selected Always; GNU rm's -i prompts before every removal and -I prompts once). Both the literal and the inverted arms date from the original implementation of the builtin.

Fix

• PromptBehaviour is now Never or Prompt { flag }, where flag is the option as the user spelled it (-i, -I, --interactive=once, --interactive=always); -f and --interactive=never still set Never.
• The Done arm rejects a remaining Prompt through Builtin::fail_parse with ParseError::Unsupported, so rm --interactive=once file prints rm: unsupported option, please open a GitHub issue -- --interactive=once plus a newline, exits 1 and leaves the operand alone. The message text and the newline come from the shared formatter; rm's private literal is deleted.
• Why this shape: naming the option as given is what the other builtins do (touch --date reports --date, an unknown short flag reports the letter), so the user sees the option they typed. Tracking the spelling is all the builtin needs today; the once/always distinction and removed_count had no consumer, which is how the inverted arms went unnoticed, so they are removed rather than flipped. Whoever implements prompting adds the modes back together with the code that reads them. The check stays after flag parsing (not inside parse_flag) so that a later -f or --interactive=never still cancels an earlier prompting option, as in GNU rm; rm -if file and rm -i -f file keep removing the file.
• Verified with test/js/bun/shell/commands/rm.test.ts ("interactive flags"): 13 rejected combinations (each spelling, clustered -ri / -rfI / -fi / -iI, and pairs where the option given last is the one named), each run with stderr as a real fd and with .quiet() (the two branches of write_failing_error); 8 cancelled combinations that must remove the file silently; rm -i alone still prints usage. The 26 rejection cases fail on the current release (USE_SYSTEM_BUN=1: old text, no newline); the whole file and test/js/bun/shell/bunshell.test.ts pass on the debug build.
• Overlap with open rm PRs: shell(rm): exit 0 silently when -f is given without operands #39225 also edits the four prompting arms (it adds opts.force = false; to each) and shell: name the rejected flag in the builtins' illegal option errors #39230 edits the _ => arm directly below them, so whichever of these lands later gets a conflict confined to those lines; the resolution is to keep both sides (this PR's Prompt { flag } assignments plus their added statements). shell(rm): accept --force as the long form of -f #39224's new --force arm assigns PromptBehaviour::Never, which still exists, and merges cleanly.

Background

• rm is a builtin of the Bun shell on every platform (src/runtime/shell/builtin/rm.rs), so this message is Bun's own, not the system rm's.
• The builtin walks argv one word at a time in its ParseOpts state, calling parse_flag on each; the first non-flag word returns RmParseFlag::Done, which is where the collected options are validated before any operand is removed. "Last flag wins" for -f vs -i falls out of each arm overwriting prompt_behaviour, which is why the rejection has to happen at Done rather than when -i is seen.
• Builtin::fail_parse (src/runtime/shell/Builtin.rs) is the shared failure path for option parsing: given a ParseError it formats the illegal option / usage / unsupported option message, lets the builtin mark its state as waiting for the stderr write, then writes the message and exits 1. cat, cp, mkdir and touch report their unimplemented options through it; rm now does too.
• A builtin's stderr is either a real fd (the message is queued on the IOWriter and the builtin finishes from the write callback; what a non-quiet $ uses) or a capture buffer (written synchronously; .quiet()). The tests run every rejection through both.

Earlier revision

The first push kept the Once / Always variants, swapped the -i / -I arms, and named the mode's short flag in the message (--interactive=once was reported as -I). Self-review pointed out that every other builtin names the option as typed and that the once/always model had no reader, so this revision records the spelling instead and drops the unused variants.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/shell/commands/rm.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

Limit details: You’ve used all 5 included reviews currently available under your plan.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a666708e-3d88-4f82-ad1c-e1a152c0b908

📥 Commits

Reviewing files that changed from the base of the PR and between a42889a and 758ffab.

📒 Files selected for processing (2)

• src/runtime/shell/builtin/rm.rs
• test/js/bun/shell/commands/rm.test.ts

---

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

[robobun]

^{Updated 6:52 PM PT - Aug 15th, 2026}

✅ @robobun, your commit 758ffabfba483c3714bfe09457f3d9ad9dd60abd passed in Build #98936! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 39233

That installs a local version of the PR into your bun-39233 executable, so you can run:

bun-39233 --bun

[robobun]

Status: fix is up in this PR (current revision: 758ffab).

Reproduced on bun 1.4.0 (and a debug build of main at bcba472):

bun -e 'import {$} from "bun"; $.nothrow(); const r = await $`rm -i x`.quiet(); console.log(JSON.stringify(r.stderr.toString()), r.exitCode)'
"rm: \"-i\" is not supported yet" 1        # same text for -I, --interactive=once, --interactive=always

With this branch each of those prints rm: unsupported option, please open a GitHub issue -- <the option as given> followed by a newline, exits 1 and leaves the operand alone; rm -if x, rm -i -f x and rm -i --interactive=never x still remove the file.

Tests: test/js/bun/shell/commands/rm.test.ts ("interactive flags"); 26 of the 35 new cases fail on the release binary, all pass on the debug build.

CI (build 98754, first revision): rm.test.ts passed on every lane; the one red job (debian 13 x64-asan) was test/js/web/fetch/blob.test.ts, which fails the same way on main and is unrelated. The later revisions only change rm.rs comments/naming and the expected strings in the same test block.

[claude]

LGTM — routes rm's unsupported-prompting rejection through the shared Builtin::fail_parse path and fixes the swapped -i/-I → PromptBehaviour mapping.

What was reviewed:

• fail_parse + ParseError::Unsupported(unsupported_flag(...)) matches the existing pattern in cat/cp/mkdir/touch; the set_wait_err closure sets RmState::ParseOpts { wait_write_err: true } exactly as write_err_literal did, and on_io_writer_chunk's non-Exec arm already resolves that to exit 1.
• PromptBehaviour::flag() and the swapped b'i'/b'I' arms now agree with the enum's doc comments and the --interactive=once/always long-form arms.
• Test matrix covers each spelling, clustered short flags, last-wins ordering, both stderr paths (quiet/non-quiet), the -f / --interactive=never cancellation cases, and the no-operand usage path.

Extended reasoning...

Overview

Two files: src/runtime/shell/builtin/rm.rs replaces the bespoke b"rm: \"-i\" is not supported yet" literal (no trailing newline, always said -i) with a call to the shared Builtin::fail_parse → write_failing_error path, adds a PromptBehaviour::flag() helper that maps Once→-I / Always→-i, and swaps the b'i'/b'I' short-flag arms so they match the enum doc comments, the long-form --interactive=... arms, and GNU rm. test/js/bun/shell/commands/rm.test.ts adds a 35-case describe("interactive flags") block.

Security risks

None. This is error-message formatting in the shell builtin's option parser; no user input reaches new syscalls or allocations, and the unsupported_flag payload is a 'static byte literal.

Level of scrutiny

Low. The change is small and mechanical: it replaces a one-off error path with the exact idiom already used by four sibling builtins (cat.rs:58, touch.rs:56, mkdir.rs:62, cp.rs). I verified fail_parse calls set_wait_err() before write_failing_error, which branches on needs_io() the same way rm's own write_err_literal does, and that rm's on_io_writer_chunk non-Exec arm already returns Builtin::done(.., 1) for the ParseOpts { wait_write_err: true } state — so the async-stderr completion path is unchanged. The -i↔Always / -I↔Once swap only affects which flag name appears in the error (the sole consumer was previously an is not Never check), and now matches GNU semantics.

Other factors

Test coverage is thorough: 13 rejection combos × 2 stderr modes assert the exact message (with newline), exit 1, and that the operand file is left untouched; 8 cancellation combos assert the file is removed silently; the no-operand case pins usage-first ordering. Array interpolation in TestBuilder.command expands to separate argv words (per bunshell.test.ts:117), so multi-flag cases like ["-I", "-i"] are passed as distinct arguments. The PR description confirms USE_SYSTEM_BUN=1 fails the 26 rejection cases and the debug build passes the full rm.test.ts and bunshell.test.ts suites. The noted overlap with #39225 is a trivial two-line merge.

[robobun]

Related: #39225 makes rm -f with no operands exit 0 (POSIX). It deliberately leaves the -f/-i interplay alone, so after it lands rm -f -i with no operands also exits 0, where GNU rm reports a missing operand because its prompting options reset ignore_missing_files. Since this PR is rewriting the prompting arms of parse_flag and already tests the cancellation orderings, clearing opts.force in those arms (and pinning rm -f -i -> usage) would fit here if you want GNU's behavior for that case. Note that it is also observable with operands: rm -f -i --interactive=never missing would then fail instead of being ignored, which is what GNU does as well.

[claude]

LGTM — routes rm's "-i not supported" through the shared fail_parse/ParseError::Unsupported path and drops the dead Once/Always variants.

What was reviewed:

• Verified Builtin::fail_parse + write_failing_error produce rm: unsupported option, please open a GitHub issue -- <flag>\n and that on_io_writer_chunk in the ParseOpts state still exits 1 after the async write, matching the old write_err_literal path.
• Checked unsupported_flag(&'static [u8]) → ParseError::Unsupported(*const [u8]) is sound: the stored flag is a 'static literal, so opt()'s deref is always valid.
• Confirmed the -f / --interactive=never cancellation still works because the rejection stays at RmParseFlag::Done; grepped for other users of PromptBehaviour::Once|Always — none.
• Tests cover every spelling, clustered short flags, last-wins ordering, both stderr modes (fd + .quiet()), the cancelled cases, and the no-operand usage path.

Extended reasoning...

Overview

Two files: src/runtime/shell/builtin/rm.rs collapses PromptBehaviour::{Once{removed_count}, Always} into a single Prompt { flag: &'static [u8] } recording the option as spelled, and replaces the hand-written b"rm: \"-i\" is not supported yet" (no newline, always says -i) with a call to Builtin::fail_parse(…, ParseError::Unsupported(unsupported_flag(flag)), …). test/js/bun/shell/commands/rm.test.ts adds a 35-case describe("interactive flags") block.

Security risks

None. This is error-message formatting for an already-rejected option in the shell builtin; no new parsing of untrusted input, no filesystem operations added, and the raw *const [u8] handed to ParseError::Unsupported points at a 'static byte literal so the unsafe deref in ParseError::opt() stays within its documented contract.

Level of scrutiny

Low-to-medium. The change is mechanical: it deletes a bespoke error path in favour of the shared one already used by cat/cp/mkdir/touch, and removes enum variants that had no reader. I traced the async-stderr completion path (fail_parse → write_failing_error → .stderr.enqueue → on_io_writer_chunk non-Exec arm → Builtin::done(…, 1)) to confirm it behaves identically to the removed write_err_literal call, and confirmed the no-io / .quiet() branch goes straight to Builtin::done(…, 1) as the other builtins do. The rejection stays at RmParseFlag::Done so -i -f / -if still cancel, which the tests pin.

Other factors

Test coverage is thorough (each spelling × both stderr modes, clustered flags, last-prompting-option-wins, 8 cancellation orderings, no-operand usage) and the PR notes 26 of the 35 new cases fail on the release binary and pass on the debug build, with an earlier CI run green on rm.test.ts across all lanes. The two comment-cop threads were addressed in 758ffab (comments cut to one line each) and are resolved. The robobun note about #39225 / clearing opts.force in the prompting arms is an optional GNU-compat follow-up already tracked by that PR, not a defect here. No other references to the removed Once/Always variants exist in the tree.