Skip to content

fix(completions): complete files after a script arg in bash & zsh - #30387

Closed
robobun wants to merge 2 commits into
mainfrom
farm/ff5731ab/complete-files-after-script-arg
Closed

fix(completions): complete files after a script arg in bash & zsh#30387
robobun wants to merge 2 commits into
mainfrom
farm/ff5731ab/complete-files-after-script-arg

Conversation

@robobun

@robobun robobun commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #30386.

Repro

$ touch myscript.ts foo-file.txt foo-bar.txt
$ bun myscript.ts foo<TAB>      # before: nothing; after: completes to `foo-`
$ bun run myscript.ts foo<TAB>  # always worked — run's own branch calls `_files`

Cause

Both completions/bun.zsh and completions/bun.bash dispatch on the first
word after bun. When it's a known subcommand (run, test, add, …) the
per-subcommand branch handles positions 2+ appropriately — the run branch,
for example, explicitly calls _files. When the first word is a script
path
(bun myscript.ts …), no branch matches and the case falls through
with no default, so TAB at positions 2+ silently produces no matches.

completions/bun.zsh, line 767

case $line[1] in
add|a)      _bun_add_completion ;;
run)        _bun_run_completion ;;
# … every known subcommand …
esac        # ← no default branch; fall-through = no completion

completions/bun.bash, line 164

*)
    _long_short_completion …
    _read_scripts_in_package_json
    _subcommand_comp_reply …
    # ← no _file_arguments for positional args after a script name
    ;;

Fix

Add a default branch to both scripts that completes files for positions 2+
when the first word isn't a recognised subcommand. Mirrors what _bun_run_completion's
other) state already does:

  • bun.zsh: new *) arm of the outer case $line[1] calls _files.
  • bun.bash: in the *) arm of case ${COMP_WORDS[1]}, when
    COMP_CWORD >= 2 also run compgen -f -- "${cur_word}".

Total diff: 15 lines of shell.

Verification

test/cli/completions.test.ts (new — there was no existing test file for
shell completions) covers both shells:

  • bash — spawn bash, source the completion, drive _bun_completions
    with a simulated COMP_WORDS / COMP_CWORD, inspect COMPREPLY:
    • bun myscript.ts foo<TAB>["foo-bar.txt", "foo-file.txt"] (was: [])
    • same with an extra positional arg in between (position 3)
    • first-arg completion still drives cleanly (no throw)
  • zsh — spawn zsh, use zpty to drive an interactive zsh that
    sources the completion, send <line><TAB>, read the PTY buffer:
    • bun myscript.ts foo<TAB> auto-completes to foo- (was: BEL bell,
      no change)
    • bun run myscript.ts foo<TAB> still auto-completes (regression guard)
    • skipped silently if zsh isn't installed on the runner
  • syntaxbash -n / zsh -n on both scripts to catch future
    typos at source-load time.

Without the patch (on bun bd): 3 tests fail — [bash] bun <script> <arg>,
[bash] bun <script> <arg1> <arg2>, [zsh] bun <script> <arg>. With the
patch: all 7 pass.

`bun myscript.ts foo<TAB>` did nothing in either shell: the dispatch
in both completion scripts only ran file completion for positions 2+
when the first word was a known subcommand (`run`, `test`, etc.).
When the first word is a script path, the case fell through with no
default branch, so TAB silently produced no matches.

Add a `*)` fallback to both scripts that calls `_files` / `compgen -f`
for positional args after the script, mirroring `bun run`'s behaviour.

Fixes #30386.
@robobun

robobun commented May 8, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 1:11 AM PT - May 8th, 2026

@robobun, your commit 65f5ec3 has 3 failures in Build #52738 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 30387

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

bun-30387 --bun

@coderabbitai

coderabbitai Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack
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: 63b39926-fa3c-4fd6-9fe3-1f0d4553ad85

📥 Commits

Reviewing files that changed from the base of the PR and between 692f2ba and 65f5ec3.

📒 Files selected for processing (1)
  • test/cli/completions.test.ts

Walkthrough

This PR adds fallback file completion to bash and zsh shell completion scripts when a user types an unrecognized first argument after bun. Previously, tab completion for positional arguments (2nd, 3rd, etc.) did not work when the script name was not a known subcommand. The changes introduce completion logic in both shells and comprehensive end-to-end tests.

Changes

Shell Completion Fallback for Positional Arguments

Layer / File(s) Summary
Bash completion fallback
completions/bun.bash
When at position 2 or later and no subcommand matched, _bun_completions now calls compgen -f to complete the current word as a file path.
Zsh completion fallback
completions/bun.zsh
The default case in the args state now invokes _files to complete positional arguments when the first word is not a recognized subcommand.
Bash completion driver
test/cli/completions.test.ts
Adds a harness that sources the bash completion script, injects COMP_WORDS/COMP_CWORD/COMP_LINE, and parses COMPREPLY results.
Zsh completion driver
test/cli/completions.test.ts
Adds a harness using zpty to spawn an interactive zsh session, inject commands with TAB, and extract completion results from terminal output.
Zsh availability helper
test/cli/completions.test.ts
Helper function probes whether zsh is available on the system.
Bash completion tests
test/cli/completions.test.ts
Validates file completion for bun <script> <arg><TAB> at various depths, including partial first-argument input.
Zsh completion tests
test/cli/completions.test.ts
Validates file completion for bun <script> <arg><TAB> with common-prefix expansion and confirms bun run paths still work correctly.
Syntax validation
test/cli/completions.test.ts
Verifies both completion scripts pass bash -n and zsh -n syntax checks.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding file completion for positional arguments after a script in bash and zsh shells.
Description check ✅ Passed The description comprehensively covers what the PR does, includes a detailed repro section, explains the root cause with code examples, describes the fix, and documents verification through new tests.
Linked Issues check ✅ Passed The PR fully addresses issue #30386 by implementing file-path completion for positional arguments 2+ after a script name in both bash and zsh, with comprehensive test coverage validating the fix.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing shell completion for script arguments: two completion script updates and a new test file with no unrelated modifications.

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


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/cli/completions.test.ts`:
- Around line 125-128: The thrown error in zshCompleteLine currently omits
stderr making failures hard to debug; update zshCompleteLine to capture both
stdout and stderr like bashComplete does: when awaiting
Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]) and exitCode
!== 0, include the captured stderr (and optionally stdout) in the thrown Error
message so the error contains the zsh driver's stderr output for diagnosis.
🪄 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: 7abd9a34-48cb-4078-b805-59d583fa5837

📥 Commits

Reviewing files that changed from the base of the PR and between 9ed6e89 and 692f2ba.

📒 Files selected for processing (3)
  • completions/bun.bash
  • completions/bun.zsh
  • test/cli/completions.test.ts

Comment thread test/cli/completions.test.ts Outdated
Comment thread test/cli/completions.test.ts Outdated
Comment thread test/cli/completions.test.ts
Comment thread test/cli/completions.test.ts
- Replace post-TAB `sleep 0.5` + drain with a poll-until-seen loop
  that breaks on the expected substring or a BEL byte, with a 4s
  upper bound. Faster in the common case and robust under CI load.
- Drop bogus pattern argument (`0.05` / `0.2`) from `zpty -r -t`
  calls — those aren't per-read timeouts. Add `sleep 0.02` to the
  wait_for else-branch so it doesn't busy-spin.
- Switch `describe` to `describe.concurrent` for all three blocks.
  Every test spawns its own shell into its own tempDir — no shared
  state, and the concurrency roughly halves total test time.
- Use `compinit -D` (skip writing `~/.zcompdump`) so concurrent
  inner zsh processes don't race on the dump file.
- Strip ANSI colour escape codes from the accumulated pty buffer
  BEFORE matching — zsh's list-colors can interleave `\x1b[…m`
  between completion chars, defeating a naive substring match.
- Include stderr in the zsh-driver error message for easier
  diagnosis on failure (matches the bash driver).

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the completion-script change is a minimal, low-risk fallback, and all the test-harness feedback from my earlier review (concurrent describes, compinit -D, poll-until-seen instead of sleep, corrected zpty -r args, stderr in errors) is addressed in 65f5ec3.

Extended reasoning...

Overview

This PR fixes #30386 by adding a default fallback branch to both completions/bun.bash and completions/bun.zsh so that bun <script> <arg><TAB> completes files. The production change is ~15 lines of shell across two files: a (( COMP_CWORD >= 2 )) && compgen -f block in the bash *) arm, and a new *) arm calling _files in the zsh case $line[1] dispatch — both mirroring what the existing run branch already does. The bulk of the diff (~265 lines) is a new test file, test/cli/completions.test.ts, that drives bash via COMP_WORDS/COMPREPLY and zsh via zpty, plus bash -n/zsh -n syntax checks.

Security risks

None. Shell tab-completion scripts are sourced into the user's interactive shell and only affect what candidates appear when the user presses TAB; they do not execute the completed command. The added code calls standard completion builtins (compgen -f, _files) with the user's own current word — no new external input, no eval, no privilege boundary. The test file spawns local bash/zsh subprocesses in temp dirs with no network or credential access.

Level of scrutiny

Low for the production change: completion scripts are pure UX, the worst-case failure mode is "TAB suggests the wrong things" or "TAB still does nothing," and the new branches follow the exact pattern already used by _bun_run_completion's other) state. Moderate for the new zpty-based test harness, which is the most intricate part — but it has already been through one review round and the author addressed every point.

Other factors

I previously left three inline comments on this PR. Reviewing the post-65f5ec3 diff against each:

  • 🔴 sleep 0.5 + blind drain → replaced with a poll-until-seen loop that breaks on the expected substring or BEL, with ANSI stripping and a 4s ceiling. Addressed (thread marked resolved).
  • 🟡 bogus 0.05/0.2 positional args to zpty -r -t → removed; wait_for() and the post-TAB loop now use else sleep 0.02 for the poll interval, and the drain loop drops the trailing arg. Addressed (thread not marked resolved on GitHub, but the code change is in).
  • 🟡 describedescribe.concurrent + compinit -D → all three describe blocks are now .concurrent, and the inner zsh uses compinit -D to avoid ~/.zcompdump races. Addressed (thread not marked resolved on GitHub, but the code change is in).

CodeRabbit's stderr-in-error nit is also addressed. The bug-hunting system found no issues. No CODEOWNERS cover completions/ or test/cli/. Given the trivial production change and the fully-addressed test feedback, this is safe to approve without further human review.

robobun added a commit that referenced this pull request Aug 13, 2026
Folds the bash cases from #30387 into this test file: `bun myscript.ts foo<TAB>`
and the same line with an extra argument before the cursor both complete
files via the fallback arm (#30386).
@robobun

robobun commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #36629, which carries the same bash and zsh *) fallbacks and also handles runtime flags before the subcommand (bun --hot run …) and fish. The bash cases from this PR's test (bun myscript.ts foo<TAB>, and the same line with another argument before the cursor) are now part of #36629's test/cli/completions.test.ts (150c45e); the zsh fallback is asserted structurally there rather than through zpty. Closing this one; #30386 stays linked from #36629.

@robobun robobun closed this Aug 13, 2026
robobun added a commit that referenced this pull request Aug 13, 2026
Folds the bash cases from #30387 into this test file: `bun myscript.ts foo<TAB>`
and the same line with an extra argument before the cursor both complete
files via the fallback arm (#30386).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tab key can't work for the 2nd argument in macOS shell

1 participant