completions: complete files for bun <path> and skip runtime flags before dispatch - #36629
completions: complete files for bun <path> and skip runtime flags before dispatch#36629robobun wants to merge 1 commit into
bun <path> and skip runtime flags before dispatch#36629Conversation
|
Warning Review limit reached
Next review available in: 36 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. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
|
Updated 12:40 PM PT - Aug 16th, 2026
✅ @robobun, your commit 6c595b03a7ec1037bd6727fe3b456363e7286c54 passed in 🧪 To try this PR locally: bunx bun-pr 36629That installs a local version of the PR into your bun-36629 --bun |
|
Status: reproduced in bash/fish/zsh by driving each completion non-interactively (bash via With fish and zsh installed: CI: the completions test passed on every lane for 631ed8d (#93801) and 4a001b4 (#93972); all red there was flaky-tagged and unrelated. Waiting on the run for dbef2b0. |
|
Found 4 issues this PR may fix:
🤖 Generated with Claude Code |
|
Of the four suggested: #6037 and #20216 are the same symptom and are now in the PR description. #24847 (the macOS |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Re #30387: that PR adds the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
completions/bun.bash:114-123— nit: the--backend)arm of thecase "${prev}"block just below still dispatches on${COMP_WORDS[1]}— the oneCOMP_WORDS[1]site thefirst_wordrewrite didn't cover.first_wordis already computed and in scope here, sobun --cwd /x add --backend <TAB>(or any global flag before the pm subcommand) offers nothing where it should offerclonefile copyfile hardlink …. One-token fix:case "${first_word}" in.Extended reasoning...
What the bug is
This PR replaces the main
case "${COMP_WORDS[1]}"dispatch (now line 154) withcase "${first_word}", computingfirst_wordvia a scanner (lines 114-123) that skips runtime flags and their values. But the--backend)arm of thecase "${prev}"block sitting between the scanner and the main dispatch was not updated:--backend) case "${COMP_WORDS[1]}" in a|add|remove|rm|install|i) COMPREPLY=( $(compgen -W "clonefile copyfile hardlink clonefile_each_dir symlink" -- "${cur_word}") ); ;; esac return ;;first_wordis declaredlocalat line 114 and populated by line 123, and thecase "${prev}"block starts at line 125 — sofirst_wordis already in scope and holds the correct subcommand at the point this arm runs.Step-by-step trace
For
bun --cwd /some/dir add --backend <TAB>(--cwdis a documented global flag forbun add/install/remove),COMP_WORDS=(bun --cwd /some/dir add --backend ""),COMP_CWORD=5,prev="--backend":step value first_wordscanner, i=1--cwd→ matches value-taking arm →((i++))skips/some/dirfirst_wordscanner, i=3add→first_word="add", breakcase "${prev}"matches --backend)inner case "${COMP_WORDS[1]}""--cwd"→ matches none of `areturnResult: no completions. With
case "${first_word}"the inner case would matchaddand offerclonefile copyfile hardlink clonefile_each_dir symlink. The same happens forbun --silent install --backend <TAB>,bun -b add --backend <TAB>, etc.Why nothing else prevents it
The
--backend)arm ends in an unconditionalreturn, so the correctedcase "${first_word}"dispatch at line 154 never gets a chance to run for this position. The only guard on offering the backend list is the innercase, and it is looking at the wrong word.Why this is in scope
grep -n 'COMP_WORDS\[1\]' completions/bun.bashafter this PR returns exactly one line — this one. It is the last remaining position-1 hard-code left behind by a rewrite whose entire purpose was to eliminate exactly this pattern. Per REVIEW.md → Fix the whole class in the same PR: "grep for every sibling site sharing the pattern: parallel switch arms …".Impact
Not a regression — pre-PR the whole file dispatched on
COMP_WORDS[1], sobun --cwd /x add --backend <TAB>completed nothing before either. Narrow: only affects--backendvalue completion when a global flag precedes the pm subcommand. Marked nit for that reason.Fix
One-token change on line ~131:
--backend) case "${first_word}" in a|add|remove|rm|install|i)
|
The |
There was a problem hiding this comment.
LGTM — completion-script-only change, all prior review rounds addressed.
What was reviewed:
- zsh
'*:: :->args'+words=($program …)shift: tracedbun --hot run src/,bun run --watch src/,bun add -d, and thehelp)inner$line[2]path — sub-completer positional numbering lines up in each. - bash:
replaced_script/_subcommand_comp_replyremovals are dead (no remaining refs);eval "$reset"is theshopt -poutput, safe. - fish
__bun_use_subcommandnow covers every prior__fish_use_subcommandgate;__bun_entrypoint's-Frule is scoped away from non-runsubcommands.
Extended reasoning...
Overview
Touches only completions/bun.{bash,fish,zsh} plus a new test/cli/completions.test.ts. The scripts are embedded via include_bytes! and drive shell tab-completion; no runtime, bundler, or install code paths. The change makes each shell locate the first non-option word before dispatching, so bun <path> and bun --hot run <path> complete files instead of nothing.
Security risks
None. Shell completion scripts run in the user's interactive shell on tab press and only call compgen/_files/complete -C and bun getcompletes. No new external input reaches native code; the embedded script bytes are unchanged in how they are loaded.
Level of scrutiny
Low-to-medium. Worst-case failure is degraded or wrong tab-completion suggestions, not a crash, hang, or data corruption. The scripts are non-load-bearing for any bun invocation. That said, they ship to every user, so I traced the final-commit changes carefully:
- zsh f44ee4a (the last fix):
'*:: :->args'rewriteswords/CURRENTto the positionals, thenwords=($program "${words[@]}"); ((CURRENT++))restores thebun <subcmd> …layout each sub-completer's_arguments '1: :->cmd' '2: :->script'was written for.$line[1](set by the top-level_arguments, independent of the::rewrite) still holds the subcommand for thecasedispatch, and thehelp)arm's inner_argumentsre-derives its own$line[2]from the shiftedwords, so that path is unaffected.-A '-*'leaves post-subcommand flags (e.g.bun add -d) for the sub-completer as intended. - bash: the removed
_subcommand_comp_replyandreplaced_scripthad no remaining callers after the*)/"")split._file_argumentsnow saves/restoresextglobviashopt -p/eval, appends instead of overwrites, and addscompgen -d; bothrun)and"")call_read_scripts_in_package_jsonfirst so its always-true filter can't strip later-appended dirs. - fish: every
__fish_use_subcommandgate (subcommand entries,$flag_applies, thenot …on--version) now uses__bun_use_subcommand, so value-taking flags before the subcommand no longer suppress any completion class.
Other factors
This PR went through six review iterations; every inline finding (bash COMP_WORDS[1] leftover, =/: word-break tokens, run) reorder, fish $flag_applies, zsh =- optional-arg suffix, *-prefix for repeatable flags, and the two-layer optspec drift) was addressed with a targeted commit and a matching test assertion. All threads are resolved and the bug-hunting pass on the current head found nothing. The new test file drives each shell non-interactively, skipIfs when the shell binary is absent, and asserts the negative case (bun add src/ does not get file completion). Given the low blast radius and the resolved review history, this does not need a further human pass.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
150c45e to
631ed8d
Compare
|
One thing noticed while folding #30387 in, pre-existing on main and carried through here since So |
There was a problem hiding this comment.
LGTM — every prior finding from the earlier review rounds is addressed, and this run found nothing new.
Checked the three additional commits since the last round (bash #30386 test, fish __bun_takes_files, and the alias exclusion list): the fish -F gate correctly excludes package-manager aliases (i/ci/a/rm/c/etc.) so bun add src/ still doesn't get file completion; the zsh words=($program …) shift keeps sub-completer '1: :->cmd' numbering intact for all dispatch arms including help. The pre-existing compgen -fG -X empty-word quirk noted in the thread is on main and only affects the no-prefix case — not a regression here.
Extended reasoning...
Overview
Touches only completions/bun.{bash,fish,zsh} and adds test/cli/completions.test.ts. No src/ changes — the scripts are embedded via include_bytes! from shell_completions.rs, so the only runtime effect is what bun completions writes when installing shell integration. The fix makes each shell locate the first non-option positional (skipping runtime flags and their values) before dispatching, adds file completion for bare bun <path> and for the arguments after an executed script, and declares the missing runtime flags (--hot/--watch/--smol/--inspect*/etc.) so they're both offered and skipped.
Security risks
None. Shell-completion scripts are sourced by the user's own interactive shell and only run compgen/_files/bun getcompletes against the current directory. No new external-input parsing, no network, no privilege boundary.
Level of scrutiny
Low blast radius: the failure mode is "wrong completions offered" in an interactive shell, not a crash or behavioural change in the runtime. That said, the _arguments optspec and COMP_WORDS walking are subtle, which is why this went through eight prior review rounds. Every issue raised there — the zsh -A '-*' regression on bun add -d, the bash =|: word-break tokens, dead _subcommand_comp_reply/replaced_script, the _read_scripts_in_package_json ordering that stripped subcommand names, extension-list drift between arms, fish value-taking flag skipping, zsh --inspect=- optional-arg form, repeatable *-prefixed flags, and the two-layer optspec drift fixed structurally by '*:: :->args' + the words shift — is in the current diff, and each has a targeted assertion in the new test.
Other factors
The new test/cli/completions.test.ts drives all three shells non-interactively (bash via COMP_WORDS+_bun_completions, fish via complete -C, zsh via -n + structural assertions), skipIfs on missing binaries, uses tempDir/bunEnv/test.concurrent, and drains stdout/stderr/exited together. CI for the head commit passed the new test on every lane per the build-status comment. The one open note in the thread — the pre-existing compgen -fG -X in _file_arguments' empty-word branch — is on main today, only affects bun run <TAB> with no prefix (directories are still offered via the new compgen -d), and is slated for #35443's generator; it doesn't block this fix.
|
Re the |
There was a problem hiding this comment.
LGTM — completion-script-only change with a benign failure mode; all prior review findings have been addressed.
What was reviewed:
- bash
first_wordscanner: value-taking flags,=/:word-break tokens, ordering vs._read_scripts_in_package_json's always-true filter,_file_argumentsextglob handling and the empty-word branch — all fixed across the iteration. - fish:
--hot -rdrop,__bun_use_subcommand/__bun_takes_filesgating, subcommand-alias exclusion list, fish < 3.4 range compat. - zsh:
-A '-*'so-d/-pafteraddaren't claimed as--define/--print;--inspect=-optional-arg form;*-prefixed repeatable flags;'*:: :->args'word-shift so sub-completers don't re-parse pre-subcommand flags.
Extended reasoning...
Overview
Touches completions/bun.{bash,fish,zsh} and adds test/cli/completions.test.ts. The fix makes each shell locate the first positional word past runtime flags (bun --hot run …, bun --watch src/…) and complete files for bare bun <path> and for arguments after an executed script. bash gets a first_word scanner and a split ""/* dispatch arm; fish gets __bun_first_positional/__bun_use_subcommand/__bun_takes_files and the missing runtime-flag declarations; zsh declares runtime flags on the top-level _arguments with -A '-*' and '*:: :->args', adds _files -g to the cmd state, and a *) → _files fallback in args.
Security risks
None. Shell completion scripts read COMP_WORDS/commandline and emit candidates; they don't execute user input, touch the network, or affect bun's own execution. Worst-case failure is wrong or missing tab-completion candidates.
Level of scrutiny
Medium diff size (~460 lines) but low-risk surface: nothing here ships in the runtime code path — the scripts are include_bytes!'d and only ever sourced by the user's shell for tab completion. The subtle parts (zsh _arguments optspec semantics, bash COMP_WORDBREAKS tokenisation, fish condition gating) went through ten prior review rounds on this PR; every finding I raised (top-level -d/-p regressing bun add -d, =/: word-break handling, dead _subcommand_comp_reply/replaced_script, extension-list drift between arms/shells, run) ordering vs. the script-name filter, fish --cwd <dir> gap, zsh --inspect optional-arg greediness, repeatable-flag * prefix, and the two-layer optspec drift) has a corresponding fix commit and, where testable, a covering assertion in completions.test.ts.
Other factors
The new test file drives the embedded scripts non-interactively per shell (bash via COMP_WORDS/_bun_completions, fish via complete -C, zsh via zsh -n + structural assertions on the top-level _arguments string), asserts stderr is empty, and includes negative checks (bun add/link/outdated do not get file completion; notes.txt is filtered). Tests skipIf on missing binaries and on macOS bash 3.2. The github-actions comment-cop noise on 2026-08-13 is on files this PR does not touch and is unrelated. No outstanding unresolved reviewer comments.
…efore dispatch bash, fish and zsh all located the subcommand by looking at the first word after `bun`, so any runtime flag there (`bun --hot run src/`, `bun --watch src/`) fell through the dispatch and completed nothing, and bare `bun <path>` (no `run`) never offered files. bash: find the first non-option word (skipping value-taking flags, their values, and the `=`/`:` tokens readline splits them into) and dispatch on that; complete subcommands, scripts, directories and runnable files at the entrypoint position and files after an executed script; one extension list shared by the run and entrypoint arms; `_file_arguments` enables extglob itself and no longer passes -fG (which made compgen read -X as its glob and list nothing for an empty word); dead `_subcommand_comp_reply` / `replaced_script` removed. fish: `--hot` takes no argument; declare the missing runtime flags; gate every subcommand/flag rule on a first-positional walk that skips flag values; add a file-completion rule that applies unless the first positional is a subcommand (or alias) whose arguments are never files. zsh: declare the runtime flags on the top-level `_arguments` (`-A '-*'`, the `=-` form for optional-argument flags, `*` for repeatable ones), shift `words` to the subcommand before calling the per-subcommand completers, offer files in the subcommand position, and fall back to `_files` after a script path. Rebased over #38333 (audit/dedupe/prune/update completions): its new bash arms are kept and its two new `COMP_WORDS[1]` lookups use `first_word`; its new fish subcommand entries use the same flag-skipping gate as the rest; `up`/`dedupe`/`prune` are excluded from fish file completion. Fixes #7805. Fixes #6037. Fixes #20216. Fixes #30386.
dbef2b0 to
6c595b0
Compare
There was a problem hiding this comment.
LGTM — completion-script-only change with test coverage; every earlier inline finding on this PR has been addressed and resolved.
What was reviewed:
- bash:
first_wordwalk skips value-taking flags and=/:word-break tokens;_file_argumentsnow appends and enables extglob locally;_read_scripts_in_package_jsonordering so atest/buildscript can't hide the subcommand; deadreplaced_script/_subcommand_comp_replydeleted. - fish:
--hotno longer-r;__bun_first_positionaltoken walk;__bun_takes_filesgate keepsadd/link/outdated/etc from getting file completion (asserted in the test). - zsh: top-level
_arguments -s -A '-*'with'*:: :->args'+words=($program …)prepend so sub-completers never re-parse the runtime flags;--inspect=-/--config=-optional-arg form; repeatable flags*-prefixed;*)fallback →_files. test/cli/completions.test.ts: hermetic (bun getcompletesstubbed in the fish case, temp $HOME),test.concurrent, per-shellskipIf, and-nsyntax checks for all three shells.
Extended reasoning...
Overview
Fixes #7805/#6037/#20216/#30386 by making all three shell completion scripts locate the first non-option word (skipping known value-taking runtime flags) before dispatching, and offering real file completion at the bun <path> position and after an executed script. The diff is confined to completions/bun.{bash,fish,zsh} plus a new test/cli/completions.test.ts; the scripts are embedded via include_bytes! in src/runtime/cli/shell_completions.rs, which is untouched.
Security risks
None. Shell completion scripts are user-invoked, read-only DX helpers that run compgen/_files/complete -C over the local filesystem. No auth, crypto, network, or privilege boundaries are involved, and no runtime code path changes.
Level of scrutiny
Low-to-medium. The failure surface is tab-completion quality: worst case a completion offers the wrong candidates or nothing at all. The zsh _arguments restructuring ('*:: :->args' + program-name prepend) is the most subtle piece and was the subject of the f44ee4a fix following my earlier inline note about two-layer optspec drift; the current shape eliminates that class structurally. -A '-*' correctly stops top-level option matching at the first positional so subcommand-specific short flags (bun add -d) aren't claimed as --define.
Other factors
This PR has been through many rounds of inline review from this bot on 2026-08-01, each addressed with a fix commit (2e4cd1e/2f7c4ab/4f5e509/f44ee4a/…/23e8008) and every thread is marked resolved. The mass of comment-cop github-actions comments dated 2026-08-13 target unrelated src/**/*.rs files that are not in this PR's diff and are all resolved — they appear to be a bot misfire during a rebase and don't bear on this change. No CODEOWNERS entry covers completions/. The new test is hermetic, concurrent, gated on shell availability (bash ≥ 4, fish/zsh on PATH), and drives bun completions piped output so it exercises the embedded bytes rather than the checked-out files.
2 participants
Fixes #7805. Fixes #6037. Fixes #20216. Fixes #30386.
Supersedes #30387 (closed): this includes its bash/zsh
*)fallback and adds the runtime-flag handling and fish coverage that PR did not have. Its bash test cases (bun myscript.ts foo<TAB>, and the same with another argument before the cursor) are folded intotest/cli/completions.test.tshere.Reproduction
In a project with
src/index.tsandsrc/other.ts, drive each shell's completion non-interactively:On
main:bun run src/src/index.ts src/other.ts¹index.ts other.tsbun src/src/index.ts src/other.ts²bun --hot run src/bun run --watch src/src/index.ts src/other.ts¹index.ts other.tsbun --hot src/bun --watch src/src/index.ts src/other.ts²¹ only when
extglobis already on (bash-completion loaded); otherwise nothing² accidental:
-Fon the__fish_use_subcommandinit/add/removeentries leaks throughCause
All three shells located the subcommand by looking at the first word after
bun, so any runtime flag there made the dispatch fall through and complete nothing.case "${COMP_WORDS[1]}" inhard-codes position 1.--hotat position 1 falls to the*)arm, which offers global flags but never files. Barebun <path>hits the same arm._file_argumentsalso relied on the caller havingextglobon, sobun run src/completed nothing when bash-completion wasn't loaded.--hotwas declared-r(requires argument), sobun --hot run …consumedrunas--hot's value and nothing matched therunsubcommand.__bun_complete_bins_scriptsemittedbun getcompletes joutput (cwd files only), sobun run src/never descended intosrc/.--watch/--smol/--bun/--inspect*weren't declared at all._arguments -s '1: :->cmd' '*: :->args'declares no options, so--hotbecomes$line[1]andcase $line[1]matches nothing. Thecmdstate offersbun getcompletes jviacompadd, which again is cwd-only, sobun src/never descended intosrc/.Fix
COMP_WORDSto find the first non-option word, skipping known value-taking flags and their values, and dispatch on that. The no-subcommand position now offers subcommands, scripts and files; the unknown-subcommand fallback offers global flags and files (matching whatmainoffered forbun build/test/etc while adding file completion)._file_argumentsnow enablesextgloblocally, offers directories, and appends instead of overwriting.-rfrom--hot, declare--watch/--smol/--bun/--silent/--no-clear-screen/--inspect*/--preload/--cwd/--env-file, and add acomplete -c bun -n __bun_takes_files -Frule: file completion unless the first positional (found by the same flag-skipping walk the subcommand gates use) is a subcommand whose arguments are never files (the package-manager family,link/unlink/outdated/publish/patch/info/audit/exec). That coversbun <file>,bun run <file>,bun test/bun build, and the arguments after an executed script (Tab key can't work for the 2nd argument in macOS shell #30386), including through runtime flags;bun add/link/... keep their existing behaviour._argumentscall so it skips them when locating the first positional (verified via zpty:bun --hot run src/now setsstate=args line[1]=run). Add_files -g '*.(js|mjs|cjs|ts|mts|cts|jsx|tsx|wasm|html)'to thecmdstate sobun src/<TAB>descends into directories, and a*)fallback in theargsdispatch that completes files when$line[1]is a script/file path.With the fix every cell in the table above is
src/index.ts src/other.ts.Relationship to other open PRs
#35443 regenerates the entire bash and fish scripts from
completions/bun-cli.jsonand fixes this for those two shells as part of a much larger change; it doesn't touch zsh. This PR is the minimal three-shell fix for #7805 specifically and can land independently.#26743 is complementary, not overlapping:
_read_scripts_in_package_json(untouched here) still tests=~ ${re_comp_word_script}against an undeclared variable, which is always true under GNU regex and an error under BSD regex, so package.json scripts are still filtered out of bash completions until that one-line fix lands. Both PRs merge cleanly with each other and with #27908 (bashbun testflags), #29364 (fish--frozen-lockfile), #31665 and #34062 (zsh one-liners).Verification
test/cli/completions.test.tsdrivesbun completions(piped stdout emits the embedded script) and then:COMP_WORDS/COMP_CWORDand calls_bun_completionsfor each line above plusbun --cwd <dir> run src/, asserting both files are offered, and thatbun --hot <TAB>still offersrun/install/--watch;bun myscript.ts fooandbun myscript.ts prev-arg foocomplete exactly the two matching files (Tab key can't work for the 2nd argument in macOS shell #30386). Alsobash -n.complete -C '<line>'for each line, plusbun src/index.ts src/andbun src/index.ts prev-arg src/(Tab key can't work for the 2nd argument in macOS shell #30386), and negative checks thatbun add/bun link/bun outdateddo not get file completion. Alsofish -n.zsh -n, plus a structural assertion that the top-level_argumentsdeclares--hot/--watch/etc and that thecmdstate offers_files.With fish 4.8 and zsh 5.8 installed (so nothing is skipped):
The 3 that pass either way are the
bash -n/fish -n/zsh -nsyntax checks.Note for the reviewer: the only diff is under
completions/andtest/; the scripts are compiled in viainclude_bytes!fromsrc/runtime/cli/shell_completions.rs, so the gate'ssrc/-stash step leaves the fixed scripts in place either side and cannot reproduce fail-before. The fail-before is demonstrated above against the released binary.no test proof · iteration 5 · Platform-specific test-only change; deferring to CI.