fix(complete): use type -P so the CLI-presence guard ignores shell functions

[JamBalaya56562]

Problem

The generated bash completion opens with a guard that is supposed to bail out when the usage CLI isn't installed:

if ! type -p usage &> /dev/null; then
    echo "Error: usage CLI not found. ..." >&2
    return 1
fi

But type -p returns exit status 0 for a shell function — it only suppresses the path output. So in any environment that defines a shell function named usage, the guard passes even with no CLI on $PATH, and the completion fails further down with an unrelated error.

Measured on GNU bash 5.2.21(1)-release and 5.3.15(1)-release:

state	type -p usage	type -P usage
usage shell function defined, no CLI on $PATH	exit 0 ← the bug	exit 1
neither function nor CLI	exit 1	exit 1

Repro:

usage() { echo "I am a function"; }
type -p usage; echo "exit=$?"   # => exit=0
type -P usage; echo "exit=$?"   # => exit=1

command -v is not an alternative — it finds functions too.

Sourcing the current cli/assets/completions/usage.bash with a usage function defined and no CLI:

$ _usage
usage.bash: line 11: _comp_initialize: command not found
exit=127

With this patch it does what it was meant to do:

$ _usage
Error: usage CLI not found. This is required for completions to work in usage.
See https://usage.jdx.dev for more information.
exit=1

Fix

type -p → type -P in the two bash guards:

• lib/src/complete/bash.rs — the leading guard in complete_bash()
• lib/src/complete/bash.rs — the guard inside complete_bash_init()'s usage-shebang branch. Same defect: a function makes the branch taken, and the command usage ... on the next line bypasses the function, so it fails on the missing external command.

Note that complete_bash_init() already used type -P for command path resolution:

cmdpath="$(type -P "$cmd" 2>/dev/null)"

So this is restoring consistency rather than introducing a new convention — the guards were simply missed.

I also changed the equivalent guard in lib/src/complete/fish.rs, since fish draws the same -p (prints nothing for functions and builtins) / -P, --force-path ($PATH only, implies -f) distinction. Caveat: I could not run fish to measure its exit status directly — this one is by analogy with the bash behaviour above, so please push back if fish differs.

Snapshots and the checked-in cli/assets/completions/{usage.bash,usage.fish} are regenerated; the generated files were verified byte-identical to fresh generator output.

Deliberately not changed

• zsh (lib/src/complete/zsh.rs) — zsh's type -p is whence -p, which already forces a $PATH search and ignores functions, aliases and builtins. It is correct as written; switching it to -P would break it.
• complete_fish_init()'s type -q — -q also matches functions, so it likely has the same class of issue, but the replacement (type -qP? command -q?) deserves someone with a fish setup to verify. Happy to follow up separately.
• nu (which) and powershell (Get-Command) use native lookups and are unaffected.

Downstream

mise's completions/mise.bash is generated by this code, and its line 3 is currently if ! type -p usage &> /dev/null; then. jdx/mise discussion #5358 reports exactly this symptom — an oh-my-bash usage shell function colliding with the completion — and is resolved by this fix once mise picks up the next usage release.

Verification

• cargo test -p usage-lib --all-features complete:: — 10 passed, no pending .snap.new
• cargo clippy --all --all-features -- -D warnings — clean
• cargo fmt --all -- --check — clean
• usage g completion {bash,fish,zsh} usage --usage-cmd "usage --usage-spec" diffed against the checked-in assets — identical (zsh unchanged)
• Sourced the generated bash completion with a usage shell function defined and no CLI on $PATH; the guard now errors and returns 1 (transcript above)

Summary by CodeRabbit

• Bug Fixes
  • Improved Bash and Fish shell completion checks to recognize only executable usage commands.
  • Prevented shell functions from incorrectly shadowing or replacing the usage executable.
  • Updated generated completion scripts for more reliable command detection and invocation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Completion scripts now require an executable usage command. They invoke usage through command and test behavior when shell functions shadow or replace the executable.

Changes

Completion executable lookup

Layer / File(s)	Summary
Enforce executable lookup and invocation cli/assets/completions/*, lib/src/complete/*.rs, mise.toml	Generated and checked-in Bash and Fish scripts use type -P. Usage-spec generation uses command usage.
Cover shell shadowing cases cli/tests/shell_completions_integration.rs	Integration tests cover function-only matches, executable shadowing, fallback chaining, and Fish initialization.

Estimated code review effort: 2 (Simple) | ~15 minutes

Suggested reviewers: jdx

Poem

A rabbit checks the PATH with care,
For executable commands there.
Shell functions yield their place,
Completion hops at the proper pace.
Tests guard each lookup case.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: using type -P to ignore shell functions during CLI presence checks.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

---

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.}

[greptile-apps]

Greptile Summary

Updates generated Bash and Fish completions to distinguish external executables from shell functions.

• Uses path-only command checks in completion and init guards.
• Invokes the external usage command explicitly when generating dynamic specs.
• Regenerates checked-in completion assets and snapshots.
• Adds integration coverage for shell-function collisions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
lib/src/complete/bash.rs	Changes Bash executable-presence guards to path-only lookup, preventing shell functions from satisfying them.
lib/src/complete/fish.rs	Applies path-only lookup to Fish completion and init guards so only an external executable enables completion behavior.
cli/tests/shell_completions_integration.rs	Adds Bash and Fish integration tests covering function-only collisions, executable shadowing, and init guards.
mise.toml	Regenerates self-completions with an explicit external-command invocation to bypass shadowing shell functions.
cli/assets/completions/usage.bash	Updates the checked-in generated Bash completion with the corrected guard and explicit external command invocation.
cli/assets/completions/usage.fish	Updates the checked-in generated Fish completion with the corrected guard and explicit external command invocation.

_{Reviews (2): Last reviewed commit: "fix(complete): stop shell functions from..." | Re-trigger Greptile}

[jdx]

Thanks for digging into this. The Bash type -P change itself looks correct, including on macOS: I ran a one-off macos-15 Actions job against Apple’s actual /bin/bash (3.2.57(1)-release). The function-only case correctly failed, and a function shadowing a real PATH executable correctly resolved the executable: https://github.com/jdx/usage/actions/runs/30666839075

Local behavior testing on this PR found two remaining gaps:

1. complete_fish_init() still uses type -q usage. In Fish 3.7, a function named usage with no executable makes that return 0; type -P usage correctly returns 1. So the same false positive remains in the Fish init path.

2. When a function shadows a real executable, the generated usage.bash and usage.fish guards now pass correctly, but both later execute an unqualified usage --usage-spec. The function still intercepts that call. I tested the checked-in generated assets with a marker function and a real executable on PATH; both generated spec files contained the function’s marker output. The self-completion path should use command usage --usage-spec (without blindly changing arbitrary caller-provided usage_cmd strings).

The existing checks all pass locally—10 completion unit tests, 4 shell completion integration tests, and 3 completion-init integration tests—but none exercises these collision cases. I’d add function-only and function-shadowing-executable behavioral tests for the changed Bash/Fish paths.

For historical context, type -P was attempted in f65a7b4 and broadly reverted in dfdc67b because zsh rejects uppercase -P. This PR’s selective approach—leaving native zsh on type -p—avoids that old regression.

AI-assisted — Tool: Codex; model: unavailable; version: unavailable.

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

[JamBalaya56562]

Thanks — both gaps confirmed and fixed in 097e6fc.

① complete_fish_init()'s type -q — fixed to type -P. This is the one I flagged as a follow-up because I couldn't measure fish; thanks for running it.

② Function shadowing an executable — I reproduced this locally before fixing: with a real usage executable on $PATH and a usage() { echo FUNCTION_MARKER; } function, the spec file came out containing FUNCTION_MARKER. Fixed by generating the shipped completions with --usage-cmd "command usage --usage-spec" rather than rewriting caller-provided usage_cmd strings — nu.rs already prefixes those with ^, so a blanket rewrite would break it. I included _usage (zsh) as well: its guard was already correct, but line 30 had the same unqualified call.

③ Tests — six behavioral tests added to shell_completions_integration.rs, covering function-only and function-shadowing-executable for the bash/fish per-bin, self-completion, and init paths. The guard tests generate with --usage-bin usage_guard_probe so nothing on $PATH can satisfy the guard except the shell function. The bash init test pre-registers a complete -D handler, so fallthrough is a positive assertion rather than an absence of output.

Also useful context on that revert history: I verified f65a7b4 / dfdc67b — good to know the zsh constraint is what drove it. zsh's type -p is whence -p, which already forces a $PATH search, so leaving it alone is correct rather than merely safe.

Verified locally: completion unit tests, clippy, fmt. The bash integration tests don't run on my Windows box (pre-existing path-quoting issue that also breaks the existing test_bash_completion_integration), so I drove the same scripts directly under bash 5.3.15 — guard errors with exit 1 on function-only, real spec content under shadowing, and fallthrough on the init path. Relying on CI for fish.

[jdx]

can you actually test fish too?

[coderabbitai]

🧹 Nitpick comments (1)

cli/tests/shell_completions_integration.rs (1)

1403-1713: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider extracting shared boilerplate for the new collision tests.

All six new tests repeat the same pattern: create a labeled temp dir, write a shell test script, spawn the interpreter, capture stdout/stderr, then remove the temp dir. The two guard-rejection tests (bash at Lines 1415-1427, fish at Lines 1577-1589) also duplicate the same generate completion <shell> testcli --usage-bin usage_guard_probe --usage-cmd "command usage_guard_probe --usage-spec" invocation.

Extract a small helper, for example fn run_shell_test_script(shell: &str, temp_dir: &Path, script: &str) -> Output, to reduce duplication and make future collision tests easier to add.

This is not required before merge; the tests are correct as written.

🤖 Prompt for 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.

In `@cli/tests/shell_completions_integration.rs` around lines 1403 - 1713, Extract
shared test helpers for the repeated shell-test setup and completion-generation
flow in the six collision tests. Add a helper such as run_shell_test_script to
create the script, invoke the selected interpreter, and return its Output, and
reuse a second helper for the common guard completion generation used by the
bash and fish guard tests. Update the affected tests to use these helpers while
preserving their existing assertions and cleanup behavior.

🤖 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.

Nitpick comments:
In `@cli/tests/shell_completions_integration.rs`:
- Around line 1403-1713: Extract shared test helpers for the repeated shell-test
setup and completion-generation flow in the six collision tests. Add a helper
such as run_shell_test_script to create the script, invoke the selected
interpreter, and return its Output, and reuse a second helper for the common
guard completion generation used by the bash and fish guard tests. Update the
affected tests to use these helpers while preserving their existing assertions
and cleanup behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: b8b969a9-f0a3-45ac-9b84-7d7961cca567

📥 Commits

Reviewing files that changed from the base of the PR and between 3f39835 and 097e6fc.

⛔ Files ignored due to path filters (1)

• lib/src/complete/snapshots/usage__complete__fish__tests__complete_fish_init.snap is excluded by !**/*.snap

📒 Files selected for processing (6)

• cli/assets/completions/_usage
• cli/assets/completions/usage.bash
• cli/assets/completions/usage.fish
• cli/tests/shell_completions_integration.rs
• lib/src/complete/fish.rs
• mise.toml

🚧 Files skipped from review as they are similar to previous changes (3)

• cli/assets/completions/usage.fish
• lib/src/complete/fish.rs
• cli/assets/completions/usage.bash

[JamBalaya56562]

Yes — ran it for real this time, in a Linux container with fish 3.6.0 installed. Full shell_completions_integration suite: 19 passed, 0 failed, nothing skipped.

Raw flag semantics on fish 3.6.0, function named usage defined and no executable on $PATH:

	type -q	type -p	type -P
function only	0	0	1
neither function nor executable	1	1	1

Same shape as bash, so both call sites were affected — complete_fish()'s -p and complete_fish_init()'s -q.

To confirm the new tests actually discriminate, I reverted each fix in isolation and re-ran under fish:

per-bin guard (function only, no CLI)

[type -p] fish: Unknown command: usage_guard_probe   <- guard passed, failed downstream
          …and the completion still got registered, pointing at a bogus spec file
[type -P] Error: usage_guard_probe CLI not found. …  <- clean error, nothing registered

init guard

[type -q] complete --exclusive ex -a '(command usage complete-word …)'   <- registered anyway
[type -P] (nothing registered)

self-completion, function shadowing a real executable

[usage --usage-spec]          spec file contains: FUNCTION_MARKER
[command usage --usage-spec]  spec file contains: // @generated by usage-cli from clap metadata
                                                  min_usage_version "4.0"
                                                  name usage-cli

So all three fixes are load-bearing and each new test fails without its fix.