Skip to content

fix(commands): name the configured alias instead of "unrecognized subcommand" outside a repository - #3982

Open
yzx9 wants to merge 3 commits into
max-sixty:mainfrom
yzx9:fix-alias-error-outside-repo
Open

fix(commands): name the configured alias instead of "unrecognized subcommand" outside a repository#3982
yzx9 wants to merge 3 commits into
max-sixty:mainfrom
yzx9:fix-alias-error-outside-repo

Conversation

@yzx9

@yzx9 yzx9 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

wt co bar outside a git repository, with co a user-config alias, fell through alias dispatch (aliases resolve the current worktree) and PATH lookup to the synthesized InvalidSubcommand error — which mixes user-config alias names into its did-you-mean candidates, so the tip suggested the very name the user typed:

> cat ~/.config/worktrunk/config.toml
[aliases]
co = "wt switch {{ args }}"

> wt co
error: unrecognized subcommand 'co'

  tip: some similar subcommands exist: 'co', 'config'

Usage: wt [OPTIONS] [COMMAND]

For more information, try '--help'.

Now try_alias itself ends dispatch when no repository is present and the name is a user-config alias:

> wt co
✗ co is an alias, but there's no git repository here
↳ Run wt inside a repository, or to target one, run wt -C <path> co

Only a genuine "not a repository" earns that message: it gates on the structured signal (git rev-parse exiting 128), while any other discovery failure — a spawn error from a bad -C path, a git that errors for its own reasons — propagates as itself, the way it does for every other command:

> wt -C /tmp/does-not-exist-xyz co
✗ Failed to execute: git rev-parse --git-common-dir
  No such file or directory (os error 2)

The is_alias check stays first, so a non-alias name still falls through to its wt-<name> PATH binary whatever git did. Because the check lives in alias dispatch, the alias owns its name outside repositories too — a colliding wt-<name> PATH binary no longer shadows it there, matching the in-repo precedence ("user config wins over wt-<name> PATH binaries"). Inside a repository, dispatch is unchanged.

Also fixed on a sibling path: similar_subcommands never suggests the input itself anymore. Non-UTF-8 args skip alias dispatch wholesale, so wt co $'\xff' inside a repo still produced tip: … 'co' — suggesting the typed name. An exact match is now filtered before did_you_mean, which kills the self-suggestion everywhere it can occur; near-match dedupe is unaffected.

Tests: the outside-repo output is pinned by a snapshot (exit code, symbols, styling, hint attachment), with the mock git in set_git_only_path answering rev-parse the way real git does outside a repository (exit 128 with the fatal message) so the snapshot exercises "no repository" rather than "git failed"; a test proves the alias wins over a colliding PATH binary outside repos; the non-UTF-8 sibling path asserts a tip without 'co'.

Assisted-by: Claude-Code:GLM-5.3

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The diagnosis is right and the placement is good: putting the check after the PATH lookup keeps it off the wt-<name> dispatch path, and scoping it to UserConfig is correct since project config can't exist without a repo. Three things.

The hint doesn't style its commands (inline suggestion). .claude/skills/writing-user-outputs/SKILL.md — "In hints: Use <underline> for commands and data values (paths, branches)" and "Command suggestions in hints: When a hint includes a runnable command, use 'To X, run Y' pattern. End with the command for easy copying." The current hint ends with "to target one" and leaves wt and -C <path> unstyled. The suggestion routes the command through a runtime variable rather than putting -C <path> inside the cformat! literal — nowhere in src/ does an angle-bracket placeholder sit inside a cformat! format string, and the literal is what color-print's tag parser reads.

Nothing pins the rendered message. tests/integration_tests/custom.rs has no assert_cmd_snapshot! at all, while the adjacent step_alias.rs uses it 56 times, and the output skill says every command output must have snapshot tests. The contains() assertions here confirm the words but not the symbols, colors, or hint attachment — a snapshot is exactly what would have caught the styling gap above.

One path still reaches the old output. try_alias is skipped entirely when any argument isn't UTF-8 (the alias_args collect returns None), so it establishes nothing about whether the name is configured. Inside a repo, wt co $'\xff' with co configured falls through to the clap error and prints the original shape — unrecognized subcommand 'co' with tip: some similar subcommands exist: 'co', 'config'. That's pre-existing, not a regression, but it's the same bug on a sibling path, and it points at a complementary one-line fix: similar_subcommands in src/commands/mod.rs filters out help but never the input itself, so an exact match is always eligible as its own "did you mean". Filtering candidate != name there would kill the self-suggestion everywhere it can occur, independent of the repo check.

Minor, take or leave: the error's second clause states a general rule rather than the current state. "co is an alias, but there's no git repository here" reads closer to the actual/expected shape the output skill asks for.

Comment thread src/commands/custom.rs Outdated

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

Everything from the last pass is addressed: the hint styles its command, the rendered output is pinned by a snapshot, and the self-suggestion filter closes the non-UTF-8 sibling path. Moving the gate into try_alias reads well and the precedence argument for it holds. Three things on the new commit.

test (windows) is red on this commit, and it's from the diff. TestRepo is imported at the top of tests/integration_tests/custom.rs, but its only use is inside custom_subcommand_alias_with_non_utf8_arg_never_self_suggests, which is #[cfg(unix)]. On Windows that leaves the import unused, and the pre-merge hook runs with RUSTFLAGS='-D warnings', so it's a build failure rather than a warning:

error: unused import: `TestRepo`
 --> tests\integration_tests\custom.rs:4:5
  |
4 |     TestRepo,
  |     ^^^^^^^^
  = note: `-D unused-imports` implied by `-D warnings`
error: could not compile `worktrunk` (test "integration") due to 1 previous error

Two inline suggestions move it into the #[cfg(unix)] body, beside the OsString imports that test already scopes locally.

The new message asserts a cause the code didn't establish. try_alias discards the error from Repository::current() and the message names one specific reason, but Repository::at also fails when git rev-parse --git-common-dir can't be spawned or its output can't be canonicalized. The sharpest case is a typo'd -C, where the hint then recommends exactly what the user just ran:

$ wt -C /tmp/does-not-exist-xyz co
✗ co is an alias, but there's no git repository here
↳ Run wt inside a repository, or to target one, run wt -C <path> co

Every other command surfaces the real failure for that same input — wt -C /tmp/does-not-exist-xyz list gives ✗ Failed to execute: git rev-parse --git-common-dir / No such file or directory (os error 2), and with no repo at all it gives fatal: not a git repository …. So this path is now the one place that swallows the distinction. Not a regression (the else arm already dropped the error), but the PR is specifically about making this message accurate, and it's cheap to keep it honest: capture the error and only substitute the alias message when the failure really is "not a repository" — git rev-parse exits 128 there, which is the structured signal CLAUDE.md's "Structured Output Over Error-Message Parsing" points at — otherwise propagate what git said. Your call whether that's in scope for this fix; if it isn't, it's worth a line in the docstring saying the message is a best guess at the cause.

set_git_only_path is inert in the new PATH-collision test. prepend_path rebuilds PATH from the test process's environment (std::env::vars_os()), not from what's already on cmd, so the git-only value set one line earlier is discarded; the following cmd.env("WORKTRUNK_TEST_MOCK_CONFIG_DIR", …) overwrites the other half. The test runs against the host PATH and host git, which is fine here — the alias name is unique enough that nothing collides — but the _git_only_path binding reads as isolation that's in effect, and the next person to tighten the mock-git setup will trust it. Inline suggestion drops the line.

Minor, take or leave: alias_needs_repo_error's lookup is the same three lines as the no-repo branch of alias_names_for_suggestions (suppress_warnings(), UserConfig::load(), aliases(None)), and on the miss path both run — wt <not-an-alias> outside a repo now parses the system + user config twice for one invocation. A shared user_config_alias_names() would cover both.

Comment thread tests/integration_tests/custom.rs
Comment thread tests/integration_tests/custom.rs
Comment thread tests/integration_tests/custom.rs
@yzx9
yzx9 marked this pull request as ready for review September 1, 2026 14:11
@yzx9
yzx9 force-pushed the fix-alias-error-outside-repo branch from 7faf08a to 53065a1 Compare September 1, 2026 14:16

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The dispatch change reads well and the precedence argument for moving the gate into try_alias holds. I built the head and walked the paths: an alias outside a repository errors instead of falling through, a colliding wt-<name> no longer shadows it, a non-alias name still reaches its PATH binary, and the self-suggestion filter does kill the 'co' tip. One thing holds the approval.

The message asserts a cause the code never established, and the new snapshot pins that. try_alias discards the error from Repository::current(), but Repository::at fails three ways: git rev-parse --git-common-dir exiting 128 (genuinely not a repository), the spawn itself failing (a -C path that doesn't exist), and the canonicalize failing. All three now print "there's no git repository here". Built at 53065a1:

$ wt -C /tmp/does-not-exist-xyz co
✗ co is an alias, but there's no git repository here
↳ Run wt inside a repository, or to target one, run wt -C <path> co

$ wt -C /tmp/does-not-exist-xyz list      # every other command
✗ Failed to execute: git rev-parse --git-common-dir
  No such file or directory (os error 2)

The hint there recommends exactly what the user just ran. wt step co on the same input reports the true cause as well, so top-level alias dispatch is now the one place in wt that swallows the distinction — in the PR that exists to make this message accurate.

The snapshot doesn't catch it because it isn't testing "outside a repository". set_git_only_path installs a mock git with only version configured, and mock_stub's fallback for an unmatched command is exit_code: 1 with no output — so custom_subcommand_alias_outside_repo_names_the_alias exercises "git failed", not "no repository", and would pass identically with a repository present. Same shape, run from inside a real checkout with a git that exits 1 for everything but --version:

$ git rev-parse --is-inside-work-tree   # real git, real repo
true
$ PATH=/tmp/fakegit wt co
✗ co is an alias, but there's no git repository here

The two inline suggestions gate the message on the structured signal — CLAUDE.mdStructured Output Over Error-Message Parsing, where git rev-parse exiting 128 is the "not a repository" channel — and keep the is_alias check first, so a non-alias name still falls through to its wt-<name> binary whatever git did. I compiled that patch and ran the six cases:

invocation before after
alias, no repo alias message alias message
alias, -C bad path alias message Failed to execute: git rev-parse …
alias, broken git inside a repo alias message git rev-parse … failed (exit 1)
alias, colliding wt-<name>, no repo alias message alias message
non-alias + wt-<name>, no repo binary runs binary runs
non-alias + wt-<name>, broken git binary runs binary runs

That patch turns the snapshot test red, which is the point: to keep asserting what its name says, the mock git needs to fail the way git actually fails — .command("rev-parse", MockResponse::exit(128).with_stderr("fatal: not a git repository (or any of the parent directories): .git\n")) on the MockConfig in set_git_only_path. Worth doing either way; as it stands the test passes for the wrong reason.

Happy to push this as a commit if you'd prefer that to applying the suggestions.

Verification notes

Failure modes of Repository::at, from resolve_git_common_dir in src/git/repository/mod.rs: Cmd::run().context("Failed to execute: git rev-parse --git-common-dir") (spawn), CommandError::from_failed_output (non-zero exit, carries exit_code), canonicalize(...).context("Failed to resolve git common directory"). Only the middle one with code 128 means "not a repository"; CommandError::exit_code is the field the suggestion reads.

mock_stub's default_response is CommandResponse { file: None, output: None, stderr: None, exit_code: 1, wait_for_file: None }, reached when no _default and no triple/compound/single key matches — which is the case for rev-parse --git-common-dir against MockConfig::new("git").version("git version 2.43.0").

Everything above was run against cargo build --bin wt at the PR head, and the patched variant compiled clean before the six cases were re-run.

Comment thread src/commands/alias.rs Outdated
Comment thread src/commands/alias.rs Outdated
…command" outside a repository

`wt co` outside a git repository, with `co` a user-config alias, fell
through alias dispatch and PATH lookup to the synthesized
InvalidSubcommand error — which mixes user-config alias names into its
did-you-mean candidates, so the tip suggested the very name the user
typed:

    error: unrecognized subcommand 'co'
      tip: some similar subcommands exist: 'co', 'config'

The alias also appears in `wt --help`'s Aliases block, so "unrecognized"
contradicted wt's own help.

try_alias now ends dispatch itself when no repository is present and the
name is a user-config alias:

    ✗ co is an alias, but there's no git repository here
    ↳ Run wt inside a repository, or to target one, run wt -C <path> co

The error states the situation; the hint ends with a styled, copyable
command (built with format! so the <path> placeholder stays out of
cformat!'s tag parser). Because the check lives in alias dispatch, the
alias owns its name outside repositories too — a colliding wt-<name>
PATH binary no longer shadows it there, matching the in-repo precedence
("user config wins over wt-<name> PATH binaries"). Inside a repository,
dispatch is unchanged.

Only a genuine "not a repository" earns the alias message: the message
gates on the structured signal — `git rev-parse` exiting 128 — while any
other discovery failure (a spawn error from a bad `-C` path, a git that
errors for its own reasons) propagates as itself, the way it does for
every other command. The `is_alias` check stays first, so a non-alias
name still falls through to its `wt-<name>` binary whatever git did.

similar_subcommands no longer suggests the input itself. Some dispatch
paths reach the error with the input in the candidate pool — non-UTF-8
args skip alias dispatch wholesale, so `wt co $'\xff'` inside a repo
still produced a tip suggesting 'co'. An exact match is filtered before
did_you_mean, killing the self-suggestion everywhere it can occur;
near-match dedupe is unaffected.

The outside-repo output is pinned by a snapshot (exit code, symbols,
styling, hint attachment); the mock git in `set_git_only_path` answers
`rev-parse` the way real git does outside a repository (exit 128 with
the fatal message), so the snapshot exercises "no repository" rather
than "git failed"; a test proves the alias wins over a colliding PATH
binary outside repos; the non-UTF-8 sibling path asserts a tip without
'co'; a unit test asserts exact matches are never suggested.

Assisted-by: Claude-Code:GLM-5.3
@yzx9
yzx9 force-pushed the fix-alias-error-outside-repo branch from 53065a1 to bcf4264 Compare September 3, 2026 14:27

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The dispatch change holds up. I built the head and walked it: an alias outside a repository names itself instead of falling through, a colliding wt-<name> no longer shadows it, a non-alias name still reaches its PATH binary, the self-suggestion filter does kill the 'co' tip, and set_git_only_path's mock now answers rev-parse the way real git does — so the snapshot exercises "no repository" rather than "git failed". One thing left, and it's the same accuracy question one level down.

Exit 128 isn't git's "not a repository" channel — it's its generic die(). git rev-parse exits 128 for every fatal error, so the gate admits a whole class of failures the docstring and the message say it excludes. Built at bcf4264, inside a real repository whose only problem is an unsupported repo-format version:

$ git config core.repositoryformatversion 999   # still a repository, just unreadable

$ wt co
✗ co is an alias, but there's no git repository here
↳ Run wt inside a repository, or to target one, run wt -C <path> co

$ wt list                                       # every other command
✗ git rev-parse --git-common-dir failed (exit 128)
  fatal: Expected git repo version <= 1, found 999

There is a repository here, the hint's remedy won't help, and git's own actionable line is exactly what's being swallowed. The everyday instance is safe.directory: a checkout owned by another user — Docker bind mount, a sudo clone, a shared drive — dies with fatal: detected dubious ownership in repository at '…' and exit 128, which makes wt <alias> the one place in wt that doesn't tell the user to add safe.directory.

Worktrunk already writes this down about 128 elsewhere — RemoteDetection::Unavailable in src/git/repository/config.rs: "ls-remote exits 128 for all of those alike, so telling a down network from a remote that simply has no HEAD would mean reading git's error text."

Not a regression — on main the same input gives unrecognized subcommand 'co', which is worse — so this needn't hold the fix. Two ways to close it:

  • Scope the claim to what the check establishes. The two inline suggestions do that and change no behavior.
  • Or stop dropping git's words. CommandError::stderr is already in hand; rendering it under the error with format_with_gutter leaves the alias line naming the alias and git's line explaining why, in every 128 case. It would change the snapshot — which reads like a gain, since the added fatal: not a git repository … is the real git text the mock now emits.

Happy to push either as a commit if that's easier than applying the suggestions.

Comment thread src/commands/alias.rs Outdated
Comment thread src/commands/alias.rs
@max-sixty

Copy link
Copy Markdown
Owner

@worktrunk-bot do you want to finish off these changes, and @yzx9 can object if they dislike them?

`git rev-parse` exits 128 for every fatal error, not only "not a
repository", so the message the 128 gate printed asserted a cause the
exit code doesn't carry: a `safe.directory` violation or an unsupported
`core.repositoryformatversion` reached "there's no git repository here"
inside a real repository, with git's actionable line dropped.

The alias line now states what aliases require, and git's own line
carries what went wrong, in a gutter like every other command that
surfaces a `CommandError`:

    ✗ co is an alias, but aliases only run inside a git repository
      fatal: not a git repository (or any of the parent directories): .git
    ↳ Run wt inside a repository, or to target one, run wt -C <path> co

    ✗ co is an alias, but aliases only run inside a git repository
      fatal: Expected git repo version <= 1, found 999
    ↳ Run wt inside a repository, or to target one, run wt -C <path> co

Failures that exit some other way still propagate as themselves — a bad
`-C` path gives `Failed to execute: git rev-parse --git-common-dir` — and
that path is now pinned by a test, which was the one uncovered patch line.
`CommandError::find_in` replaces the top-level downcast so a context-wrapped
error is still recognized. The docstrings say what 128 establishes rather
than what the message used to claim.
@worktrunk-bot

Copy link
Copy Markdown
Collaborator

Pushed 5d0f7336e@yzx9, revert or push over it if you'd rather it read differently.

The one thing left from the last review was that exit 128 is git's generic die(), not a "not a repository" code, so the message asserted a cause the check never established. I took the second of the two options offered there — stop dropping git's words — rather than only narrowing the docstring, because that's what makes the message right in the cases the gate can't tell apart. The alias line now states what aliases require, and git's own line says what went wrong, in a gutter like every other command that surfaces a CommandError:

$ wt co                                 # no repository
✗ co is an alias, but aliases only run inside a git repository
  fatal: not a git repository (or any of the parent directories): .git
↳ Run wt inside a repository, or to target one, run wt -C <path> co

$ wt co                                 # inside a repo git refuses to read
✗ co is an alias, but aliases only run inside a git repository
  fatal: Expected git repo version <= 1, found 999
↳ Run wt inside a repository, or to target one, run wt -C <path> co

The general-rule phrasing is deliberate here even though the earlier round preferred the actual/expected shape: git's line is now the "actual", so the error line carries the expectation and nothing repeats. Dispatch is otherwise unchanged — is_alias still comes first, non-128 failures still propagate as themselves (wt -C /tmp/does-not-exist-xyz co gives Failed to execute: git rev-parse --git-common-dir), and inside a repository nothing moved.

Two smaller things in the same commit: CommandError::find_in replaces the top-level downcast_ref, so a context-wrapped error is still recognized; and the non-128 propagation path now has a test, which was the single uncovered patch line codecov was failing on (src/commands/alias.rs, 24/25 hits).

Left alone: the user_config_alias_names() dedupe from the second review — it's cosmetic (a shared helper wouldn't avoid the second config parse without memoizing), and it's your call.

Verification

Built at the new head and walked the cases by hand: alias with no repository, alias inside a repo with core.repositoryformatversion 999 (git's line comes through, and matches what wt list prints for the same input), alias with a bad -C path (spawn error propagates), non-alias name still reaching its wt-<name> binary.

Tests: cargo nextest run --lib --bins (2521 passed) and cargo nextest run --test integration --no-fail-fast (2034/2035). The one failure is step_copy_ignored::test_copy_ignored_preserves_file_executable_permissions, which asserts 0644 and gets 0664 — this sandbox runs with umask 002; it doesn't touch alias dispatch and it passed on CI for the previous head. cargo clippy --all-targets --all-features under -D warnings is clean, cargo fmt --check clean, and the snapshot was regenerated with cargo insta test --accept.

…output

The gutter quotes git's explanation, so a 128 with nothing on stderr must
leave the error and its hint alone rather than print a blank gutter line.
That was the one patch line codecov still counted as missed.
@worktrunk-bot

Copy link
Copy Markdown
Collaborator

Follow-up: codecov came back still one line short — the empty-combined_output path, not the propagation path I named above — so de4de5e01 adds a test for a fatal git exit that carries no output (alias error plus hint, no blank gutter line). codecov/patch is now 100% on all three files, and fast-checks / test (linux|macos|windows) are green at that head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants