diff --git a/src/commands/alias.rs b/src/commands/alias.rs index cbd374f02..78d7c6485 100644 --- a/src/commands/alias.rs +++ b/src/commands/alias.rs @@ -51,10 +51,10 @@ use worktrunk::config::{ ALIAS_ARGS_KEY, CommandConfig, ProjectConfig, UserConfig, VarScope, alias_context_filter, format_alias_variables, referenced_vars_for_config, }; -use worktrunk::git::Repository; +use worktrunk::git::{CommandError, Repository, WorktrunkError}; use worktrunk::styling::{ - eprintln, format_with_gutter, hint_message, info_message, progress_message, verbosity, - warning_message, + eprintln, error_message, format_with_gutter, hint_message, info_message, progress_message, + verbosity, warning_message, }; use worktrunk::trace::Span; @@ -433,15 +433,21 @@ fn referenced_vars_for_entry(name: &str, entry: &AliasEntry) -> anyhow::Result` PATH lookup for it. Discovery failures git +/// didn't answer for (a spawn error) propagate, and config load errors +/// propagate — /// a broken `wt.toml` should fail loudly here just as it does for `wt list`, /// rather than silently turning into an "unrecognized subcommand" once we /// fall through to PATH lookup. pub fn try_alias(name: String, rest: Vec, global_yes: bool) -> anyhow::Result> { let _span = Span::new(format!("try_alias:{}", name)); - let Ok(repo) = Repository::current() else { - return Ok(None); + let repo = match Repository::current() { + Ok(repo) => repo, + Err(err) => return alias_needs_repo_error(&name, err), }; let user_config = repo.user_config(); let project_config = repo.project_config()?; @@ -556,6 +562,64 @@ pub fn alias_names_for_suggestions() -> Vec { .collect() } +/// Outside a git repository an alias cannot run (execution resolves the +/// current worktree), yet `wt --help` still lists user-config aliases and +/// the typo suggestions still include them. When `name` is one of those and +/// repository discovery died in git, print the alias-needs-repository error +/// and return it — a configured alias isn't "unrecognized", and the caller's +/// clap-style fallback would suggest the very name the user typed as its own +/// "did you mean". Otherwise `Ok(None)`: not an alias here, the caller falls +/// through to the `wt-` PATH lookup — whatever git did. Best-effort — +/// a config that fails to load reads as no aliases. +/// +/// A 128 exit is the closest structured signal git offers, not a dedicated +/// "not a repository" code: `git rev-parse` dies with 128 for any fatal +/// error, so a `safe.directory` violation or an unsupported +/// `core.repositoryformatversion` reaches this message too, inside a real +/// repository. That is why the message states what aliases require and +/// quotes git's own line for what went wrong, instead of asserting a cause +/// the exit code doesn't carry. Failures that exit some other way (a spawn +/// error from a bad `-C` path) propagate as themselves, the way they do for +/// every other command. +fn alias_needs_repo_error(name: &str, err: anyhow::Error) -> anyhow::Result> { + worktrunk::config::suppress_warnings(); + let is_alias = UserConfig::load() + .map(|uc| uc.aliases(None).contains_key(name)) + .unwrap_or(false); + if !is_alias { + return Ok(None); + } + // `git rev-parse` exits 128 when discovery fails fatally — the structured + // channel, per "Structured Output Over Error-Message Parsing", though not + // one specific to "not a repository"; see the docstring. + let Some(git_err) = CommandError::find_in(&err).filter(|e| e.exit_code == Some(128)) else { + return Err(err); + }; + eprintln!( + "{}", + error_message(cformat!( + "{name} is an alias, but aliases only run inside a git repository" + )) + ); + // git's own line says which fatal error this was — "not a git repository" + // in the common case, dubious ownership or an unreadable config in the + // others, where the alias message alone would misreport the cause. + let git_output = git_err.combined_output(); + if !git_output.is_empty() { + eprintln!("{}", format_with_gutter(&git_output, None)); + } + // Built with `format!` so `-C ` stays out of the `cformat!` + // literal, which is what color-print's tag parser reads. + let targeted = format!("wt -C {name}"); + eprintln!( + "{}", + hint_message(cformat!( + "Run wt inside a repository, or to target one, run {targeted}" + )) + ); + Err(WorktrunkError::AlreadyDisplayed { exit_code: 1 }.into()) +} + /// Execute the alias body for `opts.name`. Caller must have already verified /// the entry exists. /// diff --git a/src/commands/custom.rs b/src/commands/custom.rs index be7f5024d..943586b42 100644 --- a/src/commands/custom.rs +++ b/src/commands/custom.rs @@ -7,7 +7,10 @@ //! 1. **Alias**: if `foo` is configured as an alias in user/project config, //! run it via the same path as `wt step foo`. User config wins over //! `wt-` PATH binaries — aliases are how users customize wt, so the -//! user's intent should take precedence. +//! user's intent should take precedence. Outside a repository an alias +//! cannot run (execution resolves the current worktree), so dispatch ends +//! there with the alias-needs-repository error and the PATH binary never +//! sees the name. //! 2. **PATH binary**: resolve `wt-` via `which`. If found, run it with //! the remaining args, inheriting stdio, and propagate the exit code. //! Mirrors how `git foo` finds `git-foo`. @@ -220,12 +223,22 @@ mod tests { } #[test] - fn similar_subcommands_dedupes_alias_matching_builtin() { - // An alias whose name shadows a built-in (e.g. `list`) should appear - // only once in suggestions, not duplicated. + fn similar_subcommands_never_suggests_the_input_itself() { + // An exact match — e.g. a user-config alias named like the input — is + // never a "similar" suggestion: dispatch paths that reach the error + // with the input in the candidate pool (alias outside a repository, + // alias skipped as non-UTF-8) must not tip the typed name back. let cmd = build_command(); let aliases = vec!["list".to_string()]; let suggestions = similar_subcommands("list", &cmd, &aliases); + assert!( + !suggestions.contains(&"list".to_string()), + "got: {suggestions:?}" + ); + + // A near match reachable from both pools (built-in + alias of the same + // name) still appears once — the dedupe set survives the filter. + let suggestions = similar_subcommands("lst", &cmd, &aliases); let count = suggestions.iter().filter(|n| *n == "list").count(); assert_eq!(count, 1, "got: {suggestions:?}"); } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 2a4fb8784..7dd4c0003 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -117,9 +117,13 @@ pub(crate) fn did_you_mean( /// Return visible subcommand names of `parent` plus `alias_names`, filtered to /// those similar to `name`. Hidden subcommands (e.g., deprecated aliases) and -/// clap's implicit `help` are excluded. Shared by the top-level (`wt `) -/// and `wt step ` suggestion paths — both surfaces want "visible -/// built-ins + configured aliases" as the candidate pool. +/// clap's implicit `help` are excluded, and `name` itself never appears: some +/// dispatch paths reach this error with the input present in the candidate +/// pool (a user-config alias outside a repository, or one whose args skipped +/// alias dispatch as non-UTF-8), and "did you mean the thing you typed" is +/// never a useful tip. Shared by the top-level (`wt `) and +/// `wt step ` suggestion paths — both surfaces want "visible built-ins + +/// configured aliases" as the candidate pool. pub(crate) fn similar_subcommands( name: &str, parent: &clap::Command, @@ -130,7 +134,12 @@ pub(crate) fn similar_subcommands( .filter(|c| !c.is_hide_set()) .map(|c| c.get_name().to_string()) .filter(|candidate| candidate != "help"); - did_you_mean(name, builtins.chain(alias_names.iter().cloned())) + did_you_mean( + name, + builtins + .chain(alias_names.iter().cloned()) + .filter(|candidate| candidate != name), + ) } /// Build a clap `InvalidSubcommand` error anchored on `anchor`, populating the diff --git a/tests/integration_tests/custom.rs b/tests/integration_tests/custom.rs index 2f724b2b5..abd0f8c0a 100644 --- a/tests/integration_tests/custom.rs +++ b/tests/integration_tests/custom.rs @@ -2,9 +2,14 @@ use crate::common::{ mock_commands::{MockConfig, MockResponse}, - wt_command, + setup_home_snapshot_settings, wt_command, }; +// Used only by the `#[cfg(unix)]` non-UTF-8 test below; gate the import to +// match, or it reads as unused on Windows under `-D warnings`. +#[cfg(unix)] +use crate::common::TestRepo; use ansi_str::AnsiStr; +use insta_cmd::assert_cmd_snapshot; use std::path::{Path, PathBuf}; use std::process::Command; use tempfile::TempDir; @@ -35,11 +40,22 @@ fn mock_bin_dir(name: &str, response: MockResponse) -> TempDir { } /// Limit PATH to a supported Git executable, excluding every `wt-*` custom -/// subcommand without bypassing wt's startup requirements. +/// subcommand without bypassing wt's startup requirements. `rev-parse` +/// answers the way real git does outside a repository — exit 128 with the +/// fatal message — so repo-discovery tests exercise "no repository" rather +/// than "git failed" (mock_stub's fallback for an unmatched command is +/// exit 1 with no output), and that fatal line is the one the alias error +/// quotes back. fn set_git_only_path(cmd: &mut Command) -> TempDir { let dir = TempDir::new().unwrap(); MockConfig::new("git") .version("git version 2.43.0") + .command( + "rev-parse", + MockResponse::exit(128).with_stderr( + "fatal: not a git repository (or any of the parent directories): .git\n", + ), + ) .write(dir.path()); cmd.env("PATH", dir.path()) .env("WORKTRUNK_TEST_MOCK_CONFIG_DIR", dir.path()); @@ -197,6 +213,193 @@ fn custom_subcommand_typo_suggests_closest_builtin() { ); } +#[test] +fn custom_subcommand_alias_outside_repo_names_the_alias() { + // `wt co` outside a git repository, with `co` a user-config alias: alias + // dispatch ends inside try_alias with the alias-needs-repository error — + // a configured alias isn't "unrecognized", and the clap-style fallback + // would suggest the typed name itself. + let config_dir = tempfile::tempdir().unwrap(); + let config_path = config_dir.path().join("config.toml"); + std::fs::write(&config_path, "[aliases]\nco = \"echo hi\"\n").unwrap(); + + let mut cmd = wt_command(); + let _git_only_path = set_git_only_path(&mut cmd); + cmd.env("WORKTRUNK_CONFIG_PATH", &config_path); + cmd.arg("co"); + + let settings = setup_home_snapshot_settings(&config_dir); + let _guard = settings.bind_to_scope(); + + assert_cmd_snapshot!(cmd); +} + +#[test] +fn custom_subcommand_alias_outside_repo_omits_gutter_when_git_says_nothing() { + // The gutter quotes git's explanation, so a fatal exit that carries no + // output leaves the alias error and its hint alone — not a blank gutter + // line where the explanation would have gone. + let config_dir = tempfile::tempdir().unwrap(); + let config_path = config_dir.path().join("config.toml"); + std::fs::write(&config_path, "[aliases]\nco = \"echo hi\"\n").unwrap(); + + let git_dir = TempDir::new().unwrap(); + MockConfig::new("git") + .version("git version 2.43.0") + .command("rev-parse", MockResponse::exit(128)) + .write(git_dir.path()); + + let mut cmd = wt_command(); + cmd.env("PATH", git_dir.path()) + .env("WORKTRUNK_TEST_MOCK_CONFIG_DIR", git_dir.path()) + .env("WORKTRUNK_CONFIG_PATH", &config_path) + .arg("co"); + + let output = cmd.output().expect("failed to run wt"); + assert_eq!(output.status.code(), Some(1)); + let stderr = String::from_utf8_lossy(&output.stderr) + .ansi_strip() + .into_owned(); + let lines: Vec<&str> = stderr.lines().collect(); + assert_eq!( + lines, + vec![ + "✗ co is an alias, but aliases only run inside a git repository", + "↳ Run wt inside a repository, or to target one, run wt -C co", + ], + "got: {stderr}" + ); +} + +#[test] +fn custom_subcommand_alias_propagates_non_fatal_git_failure() { + // Only a fatal git exit (128) reaches the alias message. A git that fails + // any other way surfaces as itself, the way it does for every other + // command, rather than being reported as a repository verdict git never + // gave. + let config_dir = tempfile::tempdir().unwrap(); + let config_path = config_dir.path().join("config.toml"); + std::fs::write(&config_path, "[aliases]\nco = \"echo hi\"\n").unwrap(); + + // No `rev-parse` response, so it falls through to mock_stub's default: + // exit 1 with no output, unlike `set_git_only_path`'s fatal 128. + let git_dir = TempDir::new().unwrap(); + MockConfig::new("git") + .version("git version 2.43.0") + .write(git_dir.path()); + + let mut cmd = wt_command(); + cmd.env("PATH", git_dir.path()) + .env("WORKTRUNK_TEST_MOCK_CONFIG_DIR", git_dir.path()) + .env("WORKTRUNK_CONFIG_PATH", &config_path) + .arg("co"); + + let output = cmd.output().expect("failed to run wt"); + assert!(!output.status.success(), "expected failure"); + let stderr = String::from_utf8_lossy(&output.stderr) + .ansi_strip() + .into_owned(); + assert!( + stderr.contains("git rev-parse --git-common-dir failed (exit 1)"), + "git's own failure should surface: {stderr}" + ); + assert!( + !stderr.contains("is an alias"), + "a non-fatal git failure must not be reported as a missing repository: {stderr}" + ); +} + +#[test] +fn custom_subcommand_alias_outside_repo_wins_over_path_binary() { + // Outside a repository the user-config alias still owns its name: dispatch + // ends in try_alias with the needs-repository error instead of falling + // through to a colliding `wt-` PATH binary — the same "user config + // wins over wt- PATH binaries" precedence as in-repo dispatch. + let config_dir = tempfile::tempdir().unwrap(); + let config_path = config_dir.path().join("config.toml"); + std::fs::write( + &config_path, + "[aliases]\nwt-test-extcmd-alias-shadow = \"echo hi\"\n", + ) + .unwrap(); + let bin_dir = mock_bin_dir( + "wt-wt-test-extcmd-alias-shadow", + MockResponse::output("path binary ran\n"), + ); + + let mut cmd = wt_command(); + // No `set_git_only_path` here: `prepend_path` rebuilds PATH from this + // process's environment, so a git-only PATH set above it would be + // discarded. The unique alias name is what keeps host `wt-*` binaries + // out of the way. + prepend_path(&mut cmd, bin_dir.path()); + cmd.env("WORKTRUNK_TEST_MOCK_CONFIG_DIR", bin_dir.path()); + cmd.env("WORKTRUNK_CONFIG_PATH", &config_path); + cmd.arg("wt-test-extcmd-alias-shadow"); + + let output = cmd.output().expect("failed to run wt"); + assert!(!output.status.success(), "expected failure"); + assert_eq!(output.status.code(), Some(1)); + let stderr = String::from_utf8_lossy(&output.stderr) + .ansi_strip() + .into_owned(); + assert!( + stderr.contains("wt-test-extcmd-alias-shadow is an alias"), + "error should name the configured alias: {stderr}" + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + !stdout.contains("path binary ran"), + "the colliding PATH binary must not run: {stdout}" + ); +} + +#[cfg(unix)] +#[test] +fn custom_subcommand_alias_with_non_utf8_arg_never_self_suggests() { + // Non-UTF-8 args skip alias dispatch wholesale (the alias arg parser + // requires UTF-8), so inside a repo `wt co ` with `co` + // configured still falls through to the clap-style error. The + // self-suggestion filter in similar_subcommands keeps that error's tip + // from suggesting 'co' — the name the user typed. + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + + let repo = TestRepo::new(); + repo.write_project_config( + r#"[aliases] +co = "echo hi" +"#, + ); + + let mut cmd = repo.wt_command(); + cmd.arg("co").arg(OsString::from_vec(vec![0xff])); + + let output = cmd.output().expect("failed to run wt"); + assert!(!output.status.success()); + assert_eq!(output.status.code(), Some(2)); + let stderr = String::from_utf8_lossy(&output.stderr) + .ansi_strip() + .into_owned(); + assert!( + stderr.contains("unrecognized subcommand 'co'"), + "dispatch falls through to the clap-style error: {stderr}" + ); + // The error line legitimately quotes 'co'; only the tip must not. + let tip = stderr + .lines() + .find(|line| line.contains("tip:")) + .unwrap_or_default(); + assert!( + tip.contains("'config'"), + "tip should suggest the built-in it is similar to: {stderr}" + ); + assert!( + !tip.contains("'co'"), + "the tip must not suggest the typed name itself: {tip}" + ); +} + #[test] fn custom_subcommand_nested_suggestion_wins_over_path_lookup() { // `wt squash` should suggest `wt step squash` even though `wt-squash` is diff --git a/tests/snapshots/integration__integration_tests__custom__custom_subcommand_alias_outside_repo_names_the_alias.snap b/tests/snapshots/integration__integration_tests__custom__custom_subcommand_alias_outside_repo_names_the_alias.snap new file mode 100644 index 000000000..63b3b2397 --- /dev/null +++ b/tests/snapshots/integration__integration_tests__custom__custom_subcommand_alias_outside_repo_names_the_alias.snap @@ -0,0 +1,50 @@ +--- +source: tests/integration_tests/custom.rs +info: + program: wt + args: + - co + env: + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + PATH: "[PATH]" + TERM: alacritty + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" +--- +success: false +exit_code: 1 +----- stdout ----- + +----- stderr ----- +✗ co is an alias, but aliases only run inside a git repository +  fatal: not a git repository [GIT_DISCOVERY_MSG] +↳ Run wt inside a repository, or to target one, run wt -C co