shell(rm): exit 0 silently when -f is given without operands

[robobun]

Problem

• The shell's rm builtin prints usage: rm [-f | -i] [-dIPRrvWx] file ... and exits 1 when it is given -f but no file operands. rm -f, rm -rf and rm -f ${files} with an empty files array all fail, so a cleanup step such as await $`rm -f ${files}` throws whenever the list happens to be empty.
• POSIX specifies that -f shall "not write diagnostic messages or modify the exit status in the case of no file operands"; GNU and BSD rm -f exit 0 silently. Plain rm with no operands is a usage error everywhere (GNU: missing operand, BSD: usage), and stays one here.
• Cause: the ParseOpts arm of Rm::next (src/runtime/shell/builtin/rm.rs) writes the usage string and exits 1 as soon as it runs out of arguments, without looking at opts.force, even though parse_flag has already recorded the -f. This has been the behavior since the builtin was written.

Fix

• Running out of arguments while parsing flags now falls into the Done arm, the same arm an explicit operand reaches. That arm checks the operand count in one place, immediately before ExecState is built: with -f it finishes with exit 0 and no output, otherwise it prints usage and exits 1 as before.
• The check sits ahead of the "-i" is not supported yet rejection, so rm -i with no operands is still a usage error, as it is with GNU and BSD rm (and as it was before this change).
• Why this is correct: it is the POSIX rule for -f, so rm -f behaves the same whether the shell runs the builtin or an external rm. Nothing with operands changes: a missing operand under -f, illegal options (rm -f -x), and the prompting-flag rejection all take the same paths as before. Keeping the decision next to the only ExecState construction also keeps the invariant that Exec always has at least one operand in one place, so a later change that consumes an argument during parsing (shell: accept -- end-of-options delimiter in builtins #33995 adds --) has one check to extend rather than a second exit to add.
• Not changed here: -i/-I do not cancel an earlier -f, so rm -f -i with no operands exits 0 where GNU exits 1. That belongs with the prompting-flag handling (shell(rm): reject -i/-I/--interactive by name through the shared unsupported-option path #39233), which owns those parse_flag arms. rm --force is added separately in shell(rm): accept --force as the long form of -f #39224; once both land it takes this path too, since both spellings set opts.force.
• Verified with the "no operands" block in test/js/bun/shell/commands/rm.test.ts: rm -f, rm -f quiet, rm -f ${[]}, rm -rf, rm -fv and rm -f ${[]} && echo cleaned exit 0 with empty output; rm, rm -r, rm -rv quiet and rm -i still print usage and exit 1; rm -f -x still reports the illegal option. The six exit-0 cases fail on the current release (USE_SYSTEM_BUN=1); the whole file passes on the debug build.
• Also ran the rm cases of test/js/bun/shell/bunshell.test.ts on the debug build, and checked by hand that rm -f -i file, rm -i -f file, rm -f missing, rm -f -- and rm --bogus behave exactly as on main.

Background

• rm in the Bun shell is a builtin on every platform (src/runtime/shell/builtin/rm.rs), so this exit status is Bun's, not the system rm's.
• The builtin is a state machine. ParseOpts looks at one argument per iteration: a flag advances the index, and the first non-flag word returns RmParseFlag::Done, whose arm validates the options and builds ExecState with the words from that index on as operands. Before this change, exhausting the arguments was handled separately, before parse_flag was called; now it is just another way of reaching Done with nothing left.
• write_err_literal is the builtin's helper for "write this to stderr, then exit 1". Builtin::done(interp, cmd, code) finishes the command with code and writes nothing, which is what the -f case needs.
• The quiet and non-quiet variants in the tests exercise the two stderr setups of a builtin (captured buffer vs a real fd); the exit-0 path writes nothing, so the same assertion covers both.

Earlier revision of this PR

The first push put the opts.force check inside the existing "ran out of arguments" early return at the top of ParseOpts, and also made -i, -I, --interactive=once and --interactive=always clear force so that rm -f -i kept exiting 1. Review of that revision pointed out two things: #33995 (-- support) merges cleanly onto that placement and adds a second, force-blind operand check after --, so rm -f -- would have silently stayed a usage error; and the force clearing was an operand-bearing change too (rm -f -i --interactive=never missing would have started failing) on lines that #39233 rewrites. The current revision moves the check to the Done arm and drops the force clearing.

---

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: 36 seconds

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: 31707c53-c2f4-4db1-974b-9125edf200ac

📥 Commits

Reviewing files that changed from the base of the PR and between bcba472 and 20f18a4.

📒 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]

Status: ready for review (reworked once after review, see the PR description).

Reproduced on the current release (USE_SYSTEM_BUN=1): rm -f, rm -rf and $`rm -f ${[]}` in the Bun shell each exit 1 and print the usage: rm ... string, while GNU and BSD rm -f exit 0 silently. Plain rm with no operands still exits 1 with usage.

The six exit-0 cases under "no operands" in test/js/bun/shell/commands/rm.test.ts fail on the release build and the whole file passes on the debug build with this change.

CI: both runs (first revision and the current head) have the same single red lane, debian 13 x64-asan failing test/js/web/fetch/blob.test.ts ("streams only the slice", expected 5 bytes, got 100). That test fails the same way on main with a debug build and does not involve the shell; it has been reported separately. Every other lane is green, and the remaining annotation entries passed on retry.

[claude]

LGTM — small, well-scoped POSIX alignment for rm -f with no operands.

What was reviewed:

• The new opts.force early-return in Rm::next uses Builtin::done(interp, cmd, 0), the same completion path the state machine already routes through for Done/Err.
• Confirmed clearing force on -i/-I/--interactive={once,always} has no observable effect on the with-operands path (any prompting mode is rejected before force is read), and --interactive=never correctly does not clear it.
• Checked that rm -f --, rm -f -x, and empty-string operands are unaffected — they hit IllegalOption* / Done before the new branch.
• 15 test cases cover both exit-0 and exit-1 sides plus last-flag-wins ordering; quiet vs non-quiet exercises both stderr paths.

Extended reasoning...

Overview

This PR changes the shell rm builtin so that rm -f (and rm -rf, rm -f ${emptyArray}, etc.) with no file operands exits 0 silently instead of printing usage and exiting 1, matching POSIX, GNU coreutils, and BSD rm. The change is a 3-line early return in the ParseOpts arm of Rm::next in src/runtime/shell/builtin/rm.rs, plus four opts.force = false lines so that a prompting flag given after -f cancels it (last flag wins, as in GNU rm). A new 15-case describe("no operands") block is added to test/js/bun/shell/commands/rm.test.ts.

Security risks

None. The change only affects the exit status and diagnostic output when zero file operands are given — no filesystem operations run on the new path (Builtin::done is called before any Exec state is entered). No paths are parsed, no syscalls are made.

Level of scrutiny

Low-to-medium. This is a small behavioral fix in a self-contained builtin, aligning with a well-documented POSIX rule and the behavior of every mainstream rm. The new code path is a single early return using the existing Builtin::done helper (the same one the Tag::Done/Tag::Err arms already use), so there are no new state transitions, allocations, or I/O to reason about. The force = false additions to the prompting-flag arms were traced: with operands present, the prompting-mode rejection fires before force is ever read, so the only observable effect is on the no-operands path this PR is about — exactly as the description claims.

Other factors

• Test coverage is thorough: 8 exit-0 cases (including rm -f ${[]}, rm -rf, rm -fv, rm -i -f, an && chain, and quiet vs buffered stderr) and 7 exit-1 cases (bare rm, rm -r, and each prompting flag after -f). The description confirms the exit-0 cases fail under USE_SYSTEM_BUN=1 and the whole file passes on the debug build.
• I checked adjacent cases the new branch does not touch: rm -f -- still hits IllegalOptionWithFlag (short-flag loop on -), rm -f -x still hits IllegalOptionWithFlag, and an empty-string operand still returns RmParseFlag::Done and enters Exec — none reach the idx >= argc check.
• --interactive=never correctly does not clear force (it is not a prompting flag), and the test rm -f --interactive=never → exit 0 covers that.
• No prior human review comments to address; the only timeline entry is a coderabbit rate-limit notice.

[robobun]

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

❌ @robobun, your commit 20f18a4 has 1 failures in Build #98919 (All Failures):

• test/js/web/fetch/blob.test.ts - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 39225

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

bun-39225 --bun

[claude]

LGTM — the no-operands check now sits in one place in the Done arm and matches the POSIX -f rule.

What was reviewed:

• Traced rm -f, rm, rm -i, rm -f -x through the reworked ParseOpts/Done flow — the synthetic (Vec::new(), Done) only reaches the Done arm, so arg is never sliced on the empty vec.
• Confirmed the operand-bearing path is byte-for-byte unchanged after args_start >= argc fails, and Kind::Rm.usage_string() matches the test's usage literal.
• My earlier comment about -i clearing force was against the previous description; the rewritten description scopes that to #39233 and the 11 tests match what's claimed.

Extended reasoning...

Overview

Two files: src/runtime/shell/builtin/rm.rs (the ParseOpts arm of Rm::next) and test/js/bun/shell/commands/rm.test.ts (a new 11-case "no operands" describe block). The Rust change replaces the top-of-loop "ran out of arguments → usage + exit 1" early return with a fall-through into the existing RmParseFlag::Done arm via a synthetic (Vec::new(), Done); that arm then decides once, based on opts.force, whether to exit 0 silently or print usage and exit 1. The args_start binding moves up a few lines but is otherwise identical.

Security risks

None. The new code path returns before ExecState is constructed, so no filesystem operation, allocation of ShellRmTask, or worker dispatch happens. It only changes an exit code and suppresses a diagnostic. The synthetic empty arg vec is only read in the IllegalOptionWithFlag arm (&arg[1..]), which is unreachable from the synthetic branch since parse_flag isn't called there.

Level of scrutiny

Low-to-moderate. This is a small, well-scoped POSIX-conformance fix in a flag-parsing state machine, with the operand-bearing path left untouched (verified by reading the Done arm below the new check — everything after args_start >= argc is unchanged, just with args_start bound earlier). The behavior is pinned by 11 tests that cover both the exit-0 cases (-f, -rf, -fv, empty-array interpolation, && chain, quiet vs captured stderr) and the exit-1 cases (rm, rm -r, rm -i, rm -f -x), and the author confirmed the exit-0 cases fail on USE_SYSTEM_BUN=1.

Other factors

All prior bot feedback is resolved: the comment-cop long-comment complaints were addressed in 22dd330/20f18a4 (the Rust comment is now one line), and my earlier inline comment about -i/-I clearing force was based on the first revision's PR description — the author responded that this was deliberately dropped and deferred to #39233 (which owns the prompting-flag arms), and the description now explicitly calls out rm -f -i with no operands as a known GNU difference until then. Given -i isn't functional in Bun's rm anyway (it errors "-i is not supported yet" when operands are present), scoping that out is reasonable. The description, code, and tests now agree.