Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- **`-vv` diagnostics surface pager and terminal environment**: The diagnostic report (`.git/wt/logs/diagnostic.md`, written on every `-vv` run) gained an "Environment variables" section listing a curated, non-secret allowlist of the pager / terminal / locale knobs (`PAGER`, `GIT_PAGER`, `TERM`, `COLUMNS`, `NO_COLOR`, `LANG`, …) plus git's resolved `core.pager`. These are the inputs that most often explain a rendering bug — like a pager interaction suspending `wt config show` ([#3322](https://github.com/max-sixty/worktrunk/issues/3322)) — and they were previously invisible in the report. The list is a strict allowlist, never a blanket `env` dump, so no credential-bearing variable can leak into an uploaded report.

### Fixed

- **`wt step for-each` and the `--execute` fallback no longer inherit `wt`'s `GIT_*` discovery vars**: The command that for-each runs in each worktree — and the `--execute` payload, when `wt` executes it directly because shell integration isn't active — now discovers its repository from the worktree `wt` placed it in, rather than an inherited `GIT_DIR`/`GIT_WORK_TREE`. Previously such a command's `git` calls resolved against the one inherited repo (e.g. the invoking worktree, when `wt` runs as a `!wt` git alias from a linked worktree) while the per-worktree headers claimed otherwise. This extends the hook-spawn scrub from [#3374](https://github.com/max-sixty/worktrunk/pull/3374) to the remaining spawn sites that relocate a user command into a `wt`-chosen worktree; aliases and `commit.generation` commands run in the user's own context and keep the inherited environment. ([#3373](https://github.com/max-sixty/worktrunk/issues/3373))

## 0.66.0

### Improved
Expand Down
4 changes: 4 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ Cmd::new("gh").args(["pr", "list"]).run()?; // no context for standalone tools

**The `[wt-trace]` command record has one emitter: `CommandTrace` in `src/trace/emit.rs`.** The grammar lives there too (don't hand-write `log::debug!("[wt-trace] …")`). `CommandTrace::{complete,fail}` are the only callers of the private `command_completed`/`command_errored` writers, so a subprocess is either traced through the guard or produces no command record. Most spawns get this for free via `Cmd`. A few spawn sites have I/O shapes `Cmd` can't model and construct a `CommandTrace` directly: the concurrent-command runner (`output/concurrent.rs`), pipeline steps (`commands/run_pipeline.rs`), `wt step tether`, and the fsmonitor daemon launch. **Any new spawn site that runs an in-process command must construct a `CommandTrace` (start it just before spawn; `complete(success)` after wait, `fail(err)` on spawn/wait error)** — otherwise the command shows up as an unattributed gap in `wt-perf timeline`. The guard is `#[must_use]` and trips a debug-build assertion if dropped unresolved, so a forgotten `complete`/`fail` fails tests rather than silently going untraced. Detached background children (`commands/process.rs`) and interactive helpers (pagers, shell probes) are intentionally untraced — they outlive the invocation or aren't part of its timeline.

### Git-Discovery Env Vars Follow Who Chose the Cwd

Git resolves `GIT_DIR`/`GIT_WORK_TREE` (and the rest of `INHERITED_GIT_PATH_VARS`) before walking up from the cwd, so an inherited value silently overrides a child's working directory. **Any spawn site that relocates a user command into a `wt`-chosen worktree — hooks, `wt step for-each`, the `--execute` no-integration fallback — must scrub these vars** (`Cmd::scrub_git_discovery_env` or `scrub_git_discovery_env_vars`); children running in the user's own context (aliases, `commit.generation`) and `wt`'s internal git plumbing keep the inherited context (absolutized). Full site classification and rationale: `scrub_git_discovery_env_vars` in `src/shell_exec.rs`.

### Real-time Output Streaming

Stream command output line-by-line rather than buffering. Responsiveness is a priority.
Expand Down
5 changes: 5 additions & 0 deletions src/commands/for_each.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,10 @@ pub fn step_for_each(args: Vec<String>, format: crate::cli::SwitchFormat) -> any
/// program is exec'd directly without `sh -c` interposition. Directive env
/// vars are scrubbed by `Cmd` for every spawn, so child commands run in
/// other worktrees can't perturb the parent shell's CD/exec state.
/// Git-discovery vars are scrubbed too: for-each relocates the user's
/// command into each worktree, so its `git` calls must discover that
/// worktree from the cwd, not resolve an inherited `GIT_DIR` pinned to
/// wherever `wt` was invoked (see `scrub_git_discovery_env_vars`).
///
/// Child stdout is merged onto stderr so it interleaves cleanly with
/// for-each's decorated per-worktree headers and footers; the structured
Expand All @@ -240,6 +244,7 @@ fn run_argv(
Cmd::new(program)
.args(iter)
.current_dir(working_dir)
.scrub_git_discovery_env()
.stdout(Stdio::from(std::io::stderr()))
.forward_signals()
.stdin_bytes(stdin_json.as_bytes().to_vec())
Expand Down
24 changes: 19 additions & 5 deletions src/output/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ use worktrunk::git::WorktrunkError;
use worktrunk::shell_exec::Cmd;
#[cfg(unix)]
use worktrunk::shell_exec::ShellConfig;
// The std-`Command` scrub helper is only needed by the unix `exec` variant of
// `execute_command`; the non-unix variant scrubs via `Cmd`'s builder method.
#[cfg(unix)]
use worktrunk::shell_exec::scrub_git_discovery_env_vars;
use worktrunk::shell_exec::{
DIRECTIVE_CD_FILE_ENV_VAR, DIRECTIVE_EXEC_FILE_ENV_VAR, DIRECTIVE_FILE_ENV_VAR,
ShellEscapeMode, directive_shell_escape_mode,
Expand Down Expand Up @@ -488,12 +492,19 @@ fn execute_command(command: String, target_dir: Option<&Path>) -> anyhow::Result
// This gives the command full TTY access (stdin, stdout, stderr all inherited),
// enabling interactive programs like `claude` to work properly.
let mut cmd = shell.command(&command);
let err = cmd
.current_dir(exec_dir)
cmd.current_dir(exec_dir)
.stdin(Stdio::inherit())
.stdout(Stdio::inherit())
.stderr(Stdio::inherit())
.exec();
.stderr(Stdio::inherit());
// A buffered target means wt relocated the payload into a worktree it
// selected, so the payload's `git` calls must discover that worktree from
// the cwd, not an inherited `GIT_DIR` (issue #3373; see
// `scrub_git_discovery_env_vars`). With no target the payload runs where
// the user invoked wt, and keeps their context.
if target_dir.is_some() {
scrub_git_discovery_env_vars(&mut cmd);
}
let err = cmd.exec();

// exec() only returns on error
Err(anyhow::anyhow!(cformat!(
Expand All @@ -509,7 +520,10 @@ fn execute_command(command: String, target_dir: Option<&Path>) -> anyhow::Result
fn execute_command(command: String, target_dir: Option<&Path>) -> anyhow::Result<()> {
let mut cmd = Cmd::shell(&command).stdin(Stdio::inherit());
if let Some(dir) = target_dir {
cmd = cmd.current_dir(dir);
// wt relocated the payload into a worktree it selected; its `git`
// calls must discover that worktree from the cwd, not an inherited
// `GIT_DIR` (issue #3373; see `scrub_git_discovery_env_vars`).
cmd = cmd.current_dir(dir).scrub_git_discovery_env();
}

if let Err(err) = cmd.stream() {
Expand Down
8 changes: 4 additions & 4 deletions src/output/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1893,10 +1893,10 @@ fn remove_removed_worktree_silently(

/// Run a shell command with streaming output, signal forwarding, and ANSI reset.
///
/// Unified entry point for all foreground command execution — hooks, aliases,
/// and `for-each` all call this. The background pipeline runner
/// (`run_pipeline.rs`) has its own spawning logic since it redirects to log
/// files and runs detached.
/// Entry point for foreground hook and alias execution. `wt step for-each`
/// (`for_each.rs`, direct argv exec with no shell) and the background
/// pipeline runner (`run_pipeline.rs`, redirects to log files and runs
/// detached) have their own spawning logic.
///
/// Capabilities: optional stdout→stderr redirect for deterministic ordering,
/// SIGINT/SIGTERM forwarding to child process group, ANSI reset before child
Expand Down
56 changes: 42 additions & 14 deletions src/shell_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,18 +365,45 @@ pub fn scrub_directive_env_vars(cmd: &mut std::process::Command) {
/// `Command`, so the spawned process discovers its repository from its working
/// directory rather than a `GIT_DIR`/`GIT_WORK_TREE` that `wt` inherited.
///
/// Applied when spawning **user hooks**: `wt` sets a hook's working directory to
/// the worktree it is operating on, so the hook's own `git` commands must
/// resolve against that worktree — not a git-discovery context `wt` happened to
/// inherit (e.g. `wt` run as a `!wt` git alias, or nested under another tool's
/// git hook). Forwarding the inherited context into a hook is a footgun: with
/// both `GIT_DIR` and `GIT_WORK_TREE` present, a hook that runs `git init`
/// writes `core.worktree` into the *inherited* repo's config, silently
/// redirecting every later plain git command there. See issue #3373.
/// Git resolves these vars **before** walking up from the cwd, so an inherited
/// value silently overrides whatever working directory the child was given.
/// Whether a child keeps them is decided by who chose its cwd:
///
/// `wt`'s *own* internal git plumbing ([`Cmd`] via `Repository::run_command`)
/// keeps the inherited context on purpose (relative values absolutized, see
/// issue #1914), so this scrub is applied only at the hook spawn sites.
/// - **`wt` relocated a user command into a worktree it selected** — hooks
/// (run in the operation's worktree), `wt step for-each` (run in each
/// worktree in turn), and the `--execute` no-integration fallback (run in
/// the switch target when `wt` itself executes the payload) — the cwd
/// carries `wt`'s intent, so the inherited context is scrubbed and the
/// command's `git` calls discover the worktree from the cwd. The inherited
/// context is common, not exotic: `git` exports an absolute `GIT_DIR`
/// pinned to the invoking worktree's private gitdir when `wt` runs as a
/// `!wt` alias from a **linked worktree**, and git itself exports discovery
/// vars (e.g. `GIT_INDEX_FILE`) to the hooks it spawns. Forwarding those
/// into a relocated command misdirects every `git` call in it; with both
/// `GIT_DIR` and `GIT_WORK_TREE` present, a `git init` even writes
/// `core.worktree` into the *inherited* repo's config, silently redirecting
/// every later plain git command there. See issue #3373.
///
/// - **The child runs where the user already was** — aliases (the user's own
/// top-level command, run from the invoking worktree) and `commit.generation`
/// commands (spawned with no `current_dir`) — the inherited context *is* the
/// user's context, so it is forwarded untouched. Under shell integration the
/// `--execute` payload also lands here: the wrapper shell evaluates it, so
/// it sees that shell's own environment (which never contains the vars a
/// `!wt` git alias exports — git's exports die with `wt`'s process tree).
/// Scrubbing the fallback therefore *converges* the two `--execute` paths
/// for the alias case; only a `GIT_DIR` the user globally exported in their
/// interactive shell still differs, and such an env misdirects every plain
/// `git` command they run anyway.
///
/// - **`wt`'s own git plumbing** ([`Cmd`] via `Repository::run_command`) keeps
/// the inherited context on purpose (relative values absolutized, see issue
/// #1914): `wt` honoring the context it was handed is the point of running
/// `wt` under `git`.
///
/// Any new spawn site that relocates a user command into a `wt`-chosen
/// worktree must apply this scrub, via this helper or
/// [`Cmd::scrub_git_discovery_env`].
pub fn scrub_git_discovery_env_vars(cmd: &mut std::process::Command) {
for var in INHERITED_GIT_PATH_VARS {
cmd.env_remove(var);
Expand Down Expand Up @@ -1104,10 +1131,11 @@ impl Cmd {
}

/// Scrub inherited git-discovery vars ([`INHERITED_GIT_PATH_VARS`]) from the
/// child environment. Applied by user-hook spawn sites so a hook's `git`
/// commands discover the repository from the working directory `wt` sets,
/// child environment. Applied by spawn sites that relocate a user command
/// into a `wt`-chosen worktree (hooks, `wt step for-each`) so the command's
/// `git` calls discover the repository from the working directory `wt` sets,
/// not a `GIT_DIR`/`GIT_WORK_TREE` `wt` inherited. See
/// [`scrub_git_discovery_env_vars`] for the rationale (issue #3373).
/// [`scrub_git_discovery_env_vars`] for the site classification (issue #3373).
///
/// Applied after the inherited-`GIT_*` absolutization in
/// `apply_common_settings` (env-removes run last), so it also overrides the
Expand Down
51 changes: 51 additions & 0 deletions tests/integration_tests/for_each.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,57 @@ fn test_for_each_aborts_on_signal_exit(repo: TestRepo) {
);
}

/// for-each relocates the user's command into each worktree, so inherited
/// git-discovery vars (`GIT_DIR`/`GIT_WORK_TREE`) must be scrubbed — git
/// resolves them before walking up from the cwd, so a forwarded value would
/// misdirect every iteration's `git` calls to the one inherited repo
/// (issue #3373; same contract as the hook tests in `user_hooks.rs`).
///
/// Sets a discovery context consistent with the repo wt operates on (as a
/// `!wt` git alias would), so wt itself runs normally; each visited worktree
/// records the `GIT_DIR`/`GIT_WORK_TREE` it sees — `[][]` once scrubbed.
#[rstest]
#[cfg(unix)]
fn test_for_each_does_not_inherit_git_discovery_vars(mut repo: TestRepo) {
let feature_path = repo.add_worktree("feature");

let marker_dir = tempfile::tempdir().expect("create marker tmpdir");
let marker_path = marker_dir.path().to_string_lossy().to_string();
let shell_cmd = format!(
r#"printf '[%s][%s]' "$GIT_DIR" "$GIT_WORK_TREE" > {marker_path}/$(basename "$(pwd)")"#
);

let output = repo
.wt_command()
.args(["step", "for-each", "--", "sh", "-c", &shell_cmd])
.env("GIT_DIR", repo.root_path().join(".git"))
.env("GIT_WORK_TREE", repo.root_path())
.output()
.expect("run wt step for-each");
assert!(
output.status.success(),
"for-each run failed: {}",
String::from_utf8_lossy(&output.stderr)
);

// Both the primary worktree and the added one must have been visited and
// seen a scrubbed environment; misdirection is only observable when the
// command runs in a worktree other than the inherited context's.
for dir in [repo.root_path(), feature_path.as_path()] {
let marker = marker_dir
.path()
.join(dir.file_name().expect("worktree dir has a name"));
let seen = std::fs::read_to_string(&marker)
.unwrap_or_else(|e| panic!("marker for {} missing: {e}", dir.display()));
assert_eq!(
seen,
"[][]",
"for-each child in {} inherited git-discovery vars ([GIT_DIR][GIT_WORK_TREE]): {seen}",
dir.display()
);
}
}

#[rstest]
fn test_for_each_skips_prunable_worktrees(mut repo: TestRepo) {
let worktree_path = repo.add_worktree("feature");
Expand Down
40 changes: 40 additions & 0 deletions tests/integration_tests/step_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2526,3 +2526,43 @@ greet = "echo hello {{ args }}"
assert_cmd_snapshot!("alias_verbose_args_shell_escaped", cmd);
});
}

/// Aliases keep `wt`'s inherited git-discovery context: `wt <alias>` is the
/// user's own top-level command, run where they invoked it, so the inherited
/// `GIT_DIR`/`GIT_WORK_TREE` pass through — unlike hooks and `wt step
/// for-each`, which `wt` relocates into worktrees it selects and therefore
/// scrubs (the keep side of the classification in
/// `scrub_git_discovery_env_vars`; issue #3373).
#[rstest]
#[cfg(unix)]
fn test_alias_keeps_inherited_git_discovery_vars(repo: TestRepo) {
repo.write_project_config(
r#"
[aliases]
record-git-env = "printf '[%s][%s]' \"$GIT_DIR\" \"$GIT_WORK_TREE\" > env_seen.txt"
"#,
);
repo.commit("Add alias config");

let git_dir = repo.root_path().join(".git");
let output = repo
.wt_command()
.args(["-y", "record-git-env"])
.env("GIT_DIR", &git_dir)
.env("GIT_WORK_TREE", repo.root_path())
.output()
.unwrap();
assert!(
output.status.success(),
"alias failed:\nstdout: {}\nstderr: {}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);

let seen = std::fs::read_to_string(repo.root_path().join("env_seen.txt")).unwrap();
let expected = format!("[{}][{}]", git_dir.display(), repo.root_path().display());
assert_eq!(
seen, expected,
"alias should see wt's inherited git-discovery vars unchanged"
);
}
43 changes: 43 additions & 0 deletions tests/integration_tests/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,49 @@ fn test_switch_internal_with_execute(repo: TestRepo) {
);
}

/// The `--execute` no-integration fallback relocates the payload into the
/// switch target, so inherited git-discovery vars (`GIT_DIR`/`GIT_WORK_TREE`)
/// must be scrubbed there like at the hook and for-each spawn sites — git
/// resolves them before the cwd, so a forwarded value would misdirect the
/// payload's `git` calls to the inherited repo while the "Executing @ …"
/// header names the target (issue #3373; classification in
/// `scrub_git_discovery_env_vars`).
///
/// `repo.wt_command()` configures no directive files, so wt executes the
/// payload itself (unix: exec, non-unix: spawn — each platform's CI drives
/// its own variant). The relative marker path resolves in the payload's cwd,
/// pinning the relocation at the same time.
#[rstest]
fn test_switch_execute_fallback_does_not_inherit_git_discovery_vars(mut repo: TestRepo) {
let feature_path = repo.add_worktree("feature");

let output = repo
.wt_command()
.args([
"switch",
"feature",
"--execute",
r#"printf '[%s][%s]' "$GIT_DIR" "$GIT_WORK_TREE" > git_env_seen.txt"#,
])
.env("GIT_DIR", repo.root_path().join(".git"))
.env("GIT_WORK_TREE", repo.root_path())
.output()
.unwrap();
assert!(
output.status.success(),
"switch --execute failed:\nstdout: {}\nstderr: {}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);

let seen = fs::read_to_string(feature_path.join("git_env_seen.txt"))
.expect("payload should have run in the target worktree");
assert_eq!(
seen, "[][]",
"--execute payload inherited git-discovery vars ([GIT_DIR][GIT_WORK_TREE]): {seen}"
);
}

/// `--execute` with trailing `-- args` containing shell metacharacters: the
/// constructed command appended to the exec directive file must POSIX-escape
/// each trailing arg so the user's shell wrapper (`sh -c`, `bash -c`, …)
Expand Down
Loading