Nightly sweep finding. The `wt switch` first-run shell-integration offer
could delete a deprecated wrapper file it never named in the
confirmation prompt — the same unpreviewed-deletion defect
[#3644](https://github.com/max-sixty/worktrunk/issues/3644) fixed for
`wt config shell install`, left on the sibling first-run path.
## The gap
When `wt switch` shows *"Install shell integration?"* on first run,
[`prompt_shell_integration`](https://github.com/max-sixty/worktrunk/blob/c4e439409445b300c052a7761450511fa1ada0a6/src/output/shell_integration.rs#L474)
passed an **empty** legacy-cleanups list to the prompt, then called
`handle_configure_shell(None, /* skip_confirmation */ true, …)`. That
call removes any deprecated wrapper file as a side effect — the fish
`conf.d/wt.fish` superseded by `functions/wt.fish` (#566), or a nushell
wrapper stranded at a legacy autoload path (#2878). So a user on an
unconfigured bash who also had a stale, worktrunk-managed
`~/.config/fish/conf.d/wt.fish` would accept *"Install shell
integration?"* and have that fish file deleted — reported only after the
fact.
This is exactly what `wt config shell install` now previews (#3644,
merged in #3648); the first-run offer was the one path still deleting
without naming the file first. It runs against [CLAUDE.md's data-safety
rule](https://github.com/max-sixty/worktrunk/blob/c4e439409445b300c052a7761450511fa1ada0a6/CLAUDE.md):
*"No implicit destructive side effects — never silently delete/overwrite
as a side effect of an unrelated operation."*
## The fix
Compute the same dry-run legacy-cleanup list `handle_configure_shell`
re-derives internally (both come from the same `scan_shell_configs(None,
true, …)` scan, so the lists are identical) and pass it to the offer's
prompt. The deletion is now named before consent. The change is **purely
additive to the prompt** — it does not change what gets removed, only
what the prompt discloses.
## Test
A PTY regression test drives the first-run offer with bash unconfigured
(so the offer fires) and a deprecated fish `conf.d/wt.fish` present,
requests the preview (`?`), and asserts it names the removal. Verified
it **fails without the fix** (the preview shows only bash's *"Will add"*
line) and **passes with it**.
## Note for review
The prior code carried a comment documenting the empty-list behavior as
deliberate (*"resolves no legacy cleanups of its own … reports removal
after the fact, as before"*). Reading it as *"#3644's fix wasn't
extended here"* rather than *"the first-run offer should delete without
previewing"*, this completes that fix — but flagging it so the call is
explicit. If the boundary was intentional, this is a safe no-op to
close.
Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
This consolidates cross-cutting models that had accumulated parallel
representations, while preserving the CLI and config interfaces.
## What changed
- Forge identity now flows through one `ForgeKind`, with boundary-safe
network-host classification shared by CI, remote references, and
structured repository metadata. Branded SSH aliases such as
`github-personal` remain outside provider dispatch and receive an
actionable `forge.platform` diagnostic when forge data is requested.
- Worktree removal now uses one owned target type and rechecks uncached
worktree topology immediately before compare-and-swap branch deletion,
retaining branches that gained a live or locked checkout.
- Shell integration no longer implements the retired single-file
directive writer. Stale wrappers receive repair guidance, execution
fails closed, and child processes cannot inherit the retired
sourceable-file capability.
- Zsh and Git-version probes are shared, while dead mocks, redundant
dependency edges, serializer detours, pass-through types, and duplicate
tests are removed.
The removal guard deliberately distinguishes live, stale-prunable, and
locked registrations. The detached cleanup path performs the same
record-aware check before deleting a branch ref.
## Testing
`cargo run -- hook pre-merge --yes` passed after merging current
`origin/main`: 4,489 tests, Clippy, formatting, lockfile checks, docs,
doctests, and snapshot review.
> _This was written by Claude Code on behalf of max_.
`wt switch` could tell a fish user "shell requires restart" because
`$SHELL` (the login shell, say zsh) had the integration line while the
shell actually in use didn't. `current_shell()` keyed everything on
`$SHELL`, which names the login shell rather than the shell wt was
invoked from.
**Process-tree detection.** `shell::ancestor_shell()`
(src/shell/utils.rs) walks up from wt's parent to the nearest enclosing
shell: Linux reads `/proc/<pid>/stat`; macOS parses one `ps -Ao
pid=,ppid=,comm=` snapshot. macOS cannot use per-pid `p_comm`
(libproc/sysctl): `/bin/sh` re-execs bash via `/var/select/sh`, so an sh
script's `p_comm` reads "bash", while `ps`'s comm preserves argv[0].
Verified live and pinned by `test_probe_reports_invoked_name_for_sh`.
The walk passes through non-shell wrappers (`git`, `sudo`) and plain
script interpreters (`sh`, `dash`); a known-but-unsupported interactive
shell (tcsh, ksh) stops the walk so a shell further up can't claim the
session. `current_shell()`/`current_shell_name()` consult the walk first
and fall back to `$SHELL` / `PSModulePath` (Windows behavior unchanged;
Git Bash sets `$SHELL` itself).
**Hedged messaging.** The reason string is now "shell integration
installed but not active", and the hint presents restart as the likely
fix with an escape hatch: "A shell restart usually activates shell
integration; if it doesn't, ask an agent to debug with the docs @
https://worktrunk.dev/llms.txt" (the LLM docs index works for any agent;
the `/worktrunk` skill exists only for plugin users). Post-install hints
are unchanged (restart advice there is unconditional by construction).
`wt config show` now prints `Detected shell: <name> (process tree)` next
to `$SHELL` in the diagnostic gutter.
**Also fixed along the way:** `shell_from_name` requires a word
boundary, so `fishd`/`bashtop`/`numactl` no longer classify as shells
(`numactl` previously matched Nushell via the bare `nu` prefix); the
post-install/uninstall restart hints compare `Shell` values instead of
display strings, which never matched process-tree names like `pwsh` or
`zsh-5.9`.
**Testing.** Integration tests spawn wt under a test harness whose real
ancestry (nextest, cargo, the developer's shell) would leak in, so
`WORKTRUNK_TEST_PARENT_SHELL` is pinned empty in `STATIC_TEST_ENV_VARS`
(and in the two PTY helpers that mirror it by hand, which CI round 1
caught: locally the walk found the dev zsh, on CI the runner's bash) and
set per-test to simulate ancestors. New integration tests cover the
three detection outcomes (mismatch suppresses restart advice, configured
ancestor shows the hedged hint, unsupported ancestor overrides
`$SHELL`); unit tests cover the classifier, the walk itself (injectable
process-table lookup: wrapper pass-through, transparency,
cycle/depth/init terminations), and both OS probes against live
processes; `wt config show` is snapshot-tested with an override ancestor
and run once unpinned so the production walk executes end-to-end.
Known limits, accepted: the walk stops at the nearest supported shell
even when it's a non-interactive script interpreter (`bash -c` wrapping
wt reports bash), hence the hedged wording; the walk is bounded at 16
hops and any unreadable hop falls back to `$SHELL`.
> _This was written by Claude Code on behalf of max_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The first-time commit-generation setup prompt (`wt step commit` / `wt
merge` / `wt step squash` with no `[commit.generation]` configured)
hardcoded `~/.config/worktrunk/config.toml` in two user-facing hints —
the `?`-preview ("Would add to …") and the save-failure hint ("add
manually to …") — while the actual save resolves the path via
`require_config_path()`. So the displayed path was wrong whenever
`--config`, `WORKTRUNK_CONFIG_PATH`, or `$XDG_CONFIG_HOME` redirected
the real config location. Same bug class as #3290 (the picker's
disabled-summary hint).
Rather than add a third inline copy of the `config_path() →
format_path_for_display → fallback-literal` dance — the picker and the
bare-repo `resolve.rs` warning each already carried one — this extracts
a single helper, `config_path_for_display()`, the display-only
counterpart of `config_path()`. The `--config` / `WORKTRUNK_CONFIG_PATH`
/ `$XDG_CONFIG_HOME` resolution and the one canonical fallback literal
now live in one place, and all three sites route through it (the inline
at `resolve.rs` also dropped a redundant `.to_string()`). The helper's
doc points callers at `require_config_path()` for the actual mutation.
## Testing
The helper is covered three ways over by existing integration tests (the
picker summary-preview PTY snapshot, the bare-repo prompt snapshot, and
the commit-generation prompt PTY tests). The commit-gen prompt's preview
line was already covered by `test_user_requests_preview`; this PR adds
`test_user_accepts_but_save_fails_shows_manual_hint` to cover the
save-failure hint — it points `WORKTRUNK_CONFIG_PATH` at a path whose
parent is a regular file, so the write fails with `ENOTDIR` (root-proof,
unlike a chmod). No snapshot drift: the picker/resolve conversions emit
byte-identical output.
> _This was written by Claude Code on behalf of max_
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The shell wrapper previously used a single `WORKTRUNK_DIRECTIVE_FILE`
where wt wrote shell commands (`cd '/path'`, arbitrary `--execute`
payloads). This meant the cd path went through shell parsing — any
content wt wrote was sourced as shell.
This splits the protocol into two files with different trust levels:
- **`WORKTRUNK_DIRECTIVE_CD_FILE`** — raw path, read with `cd -- "$(<
file)"`. No shell parsing, no escaping, no injection surface. Safe to
pass through to alias/hook child processes.
- **`WORKTRUNK_DIRECTIVE_EXEC_FILE`** — arbitrary shell (from
`--execute`), sourced by the wrapper. Scrubbed from alias/hook child
environments so hook bodies cannot inject shell into the parent session.
When a nested `wt` inside an alias body tries `--execute` without the
EXEC file, the command is dropped with a warning linking to #2101 for
user feedback.
The old `WORKTRUNK_DIRECTIVE_FILE` is silently honored for one release
(users who upgrade wt without restarting their shell). Bash, zsh, fish,
and PowerShell self-update on restart; nushell requires `wt config shell
install`.
Closes#2101
> _This was written by Claude Code on behalf of @max-sixty_
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix(shell): harden nushell wrapper and improve diagnostics
- Move LAST_EXIT_CODE capture inside do{} block so it reflects the
actual command exit code, not a subsequent operation
- Wrap directive processing in try/catch to ensure temp file cleanup
on error
- Include nushell vendor autoload paths in scan_for_detection_details
so `wt config show` reports nushell integration status
- Add "nu" to the supported shells hint shown on unsupported shells
- Fix detection tests to use actual nushell config line patterns
- Document why non-cd directives delegate to sh -c
Co-Authored-By: Claude <noreply@anthropic.com>
* refactor(shell): remove try/catch from nushell directive cleanup
Drop error-path cleanup for the temp directive file. On error, the file
persists in /tmp as a useful debugging artifact (the OS cleans it up).
This matches bash and fish which already use a single rm on the happy
path with no error wrapping.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Remove blank lines after prompts; add prompt-aware PTY tests
Remove redundant eprintln!() after read_line() in prompt functions —
the Enter key echo already provides the line break, so the extra blank
line violated "blank before prompts, not after."
Add prompt-aware PTY test helpers that wait for the prompt marker before
sending input, so captured output matches real terminal behavior. The
PTY module now exposes 3 composable functions (build_pty_command +
exec_cmd_in_pty / exec_cmd_in_pty_prompted) instead of 8 wrappers.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(test): eliminate PTY echo artifact causing blank lines in snapshots
portable_pty's UnixMasterWriter::drop() sends \n + EOT to the PTY master.
When drop(writer) happened before child.wait(), the terminal echoed this
\n as \r\n — producing a spurious blank line in captured output. Fix by
reordering: wait for the child first, then drop the writer (echo goes to
a dead PTY). Also remove the "collapse consecutive newlines" filter in
configure_shell tests that was masking this bug.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix(ci): build wt binary before running nextest
## Problem
The test `test_switch_with_execute_through_wrapper` was failing on macOS CI with:
```
bash: /Users/runner/work/worktrunk/worktrunk/target/debug/wt: No such file or directory
```
Root cause: `cargo nextest` only builds test binaries, not the main binary. The
shell_wrapper tests use `insta_cmd::get_cargo_bin("wt")` which expects the main
binary to exist at `target/debug/wt`. When running via nextest without building
the binary first, this path doesn't exist, causing the test to fail.
The issue surfaced after commit 0679e6d8 but wasn't caused by that change - it
exposed a pre-existing race condition where tests sometimes ran before the binary
was built.
## Solution
Add `cargo build --bin wt` before the nextest command in the pre-merge hook.
This ensures the main binary is built before tests that depend on it run.
The fix is at the right level because:
- Tests legitimately need the binary to exist (they're testing CLI behavior)
- Building the binary once before all tests is more efficient than per-test builds
- The pre-merge hook is the natural place to ensure test prerequisites are met
## Testing
Verified locally:
```
cargo build --bin wt && NEXTEST_NO_INPUT_HANDLER=1 cargo nextest run \
--test integration --features shell-integration-tests \
test_switch_with_execute_through_wrapper
```
Test passes with the binary built first.
---
🤖 Automated fix for [failed run](https://github.com/max-sixty/worktrunk/actions/runs/21382473224)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* fix(ci): use compile-time binary path instead of runtime workaround
Replace `get_cargo_bin("wt")` with `env!("CARGO_BIN_EXE_wt")` via a new
`wt_bin()` helper. The compile-time macro tells Cargo to build the binary
when compiling tests, fixing nextest compatibility without needing an
explicit `cargo build` step.
This supersedes the previous commit's workaround (adding `cargo build --bin wt`
to the pre-merge hook).
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: move wt_bin imports into cfg(unix) blocks
Fix unused import warnings on Windows where some tests are Unix-only.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Maximilian Roos <m@maxroos.com>
* Add LLM setup prompt for first-time commit configuration
Implement one-time interactive prompt when users attempt commit/merge/squash without LLM configuration. Detects available tools (claude, codex) and offers auto-configuration with preview on `?`. Adds `skip-commit-generation-prompt` flag to suppress re-prompting, `set_commit_generation_command()` config method, and reusable `prompt_yes_no_preview()` utility. Updates commit message templates to clarify format requirements, fixes Claude Code command syntax (spaces to equals in flags), and adds visual styling to section headers in documentation.
* Update snapshots for template format changes
Co-Authored-By: Claude <noreply@anthropic.com>
* Simplify commit template format guidance
Remove redundant trivial-changes line from template format section.
Fix duplicate "# Other" header from merge. Document skip-commit-generation-prompt
in first-run prompts section alongside skip-shell-integration-prompt.
Co-Authored-By: Claude <noreply@anthropic.com>
* Add unit tests for command detection functions
Cover command_exists() and detect_llm_tool() with basic unit tests
to improve test coverage on the commit generation module.
Co-Authored-By: Claude <noreply@anthropic.com>
* Add PTY tests for commit generation prompt
Test the interactive prompt flow for LLM commit configuration:
- No tool found → sets skip flag
- User declines → sets skip flag
- User accepts → saves config
- User requests preview → shows preview
Uses fake claude script to test the prompt path that requires
a detected LLM tool.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Clarifies that this struct represents user-level configuration
(~/.config/worktrunk/config.toml), distinct from ProjectConfig
which is checked into the repository.
Co-authored-by: Claude <noreply@anthropic.com>
* refactor(tests): unify PTY execution into common/pty module
Consolidates 6 duplicate PTY execution functions from test files into a
shared module at tests/common/pty.rs:
- exec_in_pty() - simple PTY execution
- exec_in_pty_with_home() - with HOME override for shell config tests
- exec_cmd_in_pty() - for pre-configured CommandBuilder
- exec_in_pty_multi_input() - for tests requiring multiple inputs
Also:
- Migrates shell_integration_prompt.rs from manual normalize_output() to
insta filters, consistent with other PTY tests
- Removes duplicate get_shell_binary() from shell_wrapper.rs
Net reduction of ~183 lines of duplicated code.
Co-Authored-By: Claude <noreply@anthropic.com>
* style: apply cargo fmt formatting
* fix: move get_shell_binary import to shared section for Windows
---------
Co-authored-by: Claude <noreply@anthropic.com>
* refactor: simplify test normalization by using insta filters
- Remove redundant `normalize_newlines()` from shell_wrapper.rs
(already handled by `add_pty_filters()`)
- Remove dead `add_pty_tmpdir_filters()` from common/mod.rs
(no snapshots used [TMPDIR] placeholder)
- Convert config_state.rs `normalize_log_path()` to insta filter
- Convert configure_shell.rs `normalize_output()` to insta filter
via `install_pty_settings()` function
Analyzed but kept as-is (appropriate for their use cases):
- shell_integration_prompt.rs: Uses `contains()` assertions, not snapshots
- select.rs: Requires line-specific manipulation for TUI timing
- diagnostic.rs: Complex domain-specific normalization with ordering
Net reduction: 46 lines of test code.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: restore CRLF normalization in PTY exec functions
The insta filter for \r\n wasn't fully equivalent to the
String::replace() call. Restore eager normalization to avoid
subtle edge cases with ANSI code matching.
Also removes redundant set_snapshot_path() since it's inherited
from TestRepo's bound settings.
Co-Authored-By: Claude <noreply@anthropic.com>
* refactor: consolidate CRLF normalization to PTY source
Move CRLF normalization from insta filter to PTY exec functions.
This follows the principle: normalize once, at the source.
Changes:
- Add CRLF normalization to all PTY exec functions
- Remove CRLF filter from add_pty_filters()
- Update snapshots (removes redundant trailing [0m codes)
This eliminates subtle ordering issues between filters and ensures
consistent data for all downstream processing.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Simplify assert messages by removing redundant descriptions
Remove verbose assertion messages that repeat the condition being tested.
Messages like "Should contain indicator" when asserting `contains("●")` are
redundant since the code is self-documenting. This reduces noise in tests
while maintaining clarity through meaningful assertion failures.
* test: remove verbose assertion messages and doc comments
Removed facile test verbosity across three categories:
1. **Doc comments** (~236 lines): Removed `/// Test X` comments that just
restated test function names. Kept multi-line explanations that add context.
2. **Test names** (9 functions): Shortened overly verbose names by removing
filler words while maintaining clarity:
- test_merge_when_primary_not_on_default_but_default_has_worktree →
test_merge_primary_not_on_default_with_default_worktree
- test_complete_switch_option_prefix_shows_options_not_branches →
test_complete_switch_option_prefix_shows_options
3. **Assertion messages** (19 instances): Removed messages that just restate
the check:
- assert!(path.exists(), "Path should exist") → assert!(path.exists())
- assert!(x.is_file(), "Should be a file") → assert!(x.is_file())
Kept messages that add semantic meaning (e.g., "hook should have run") or
include debugging variables.
Total: 279 lines removed across 31 files. All 723 integration + 401 unit tests
passing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* test: remove unreferenced snapshot files from renamed tests
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove verbose assertion messages that repeat the condition being tested.
Messages like "Should contain indicator" when asserting `contains("●")` are
redundant since the code is self-documenting. This reduces noise in tests
while maintaining clarity through meaningful assertion failures.
Update switch messages to explicitly state what was created:
- `--create`: "Created branch X and worktree from Y @ path"
- DWIM from remote: "Created branch X (tracking origin/X) and worktree @ path"
- Worktree only: "Created worktree for X @ path"
- Switch to existing: "Switched to worktree for X @ path"
Previously, creating a worktree for an existing local branch showed
"Switched to worktree" which was misleading since a worktree was actually
created. Now messages accurately reflect whether a branch, worktree, or
both were created.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude <noreply@anthropic.com>
When running wt via an explicit path (cargo run, ./wt, /usr/bin/wt),
shell integration can't intercept the command. Previously this showed
a misleading "restart shell" message.
Now shows: "ran target/debug/wt; shell integration wraps wt" - clearly
explaining what was run vs what the shell wrapper intercepts.
Detection uses argv[0]: PATH lookup sets it to just "wt", while explicit
paths include the path separator. This catches cargo run, symlinks to
dev builds, and any direct binary invocation.
🤖 Generated with [Claude Code](https://claude.ai/claude-code)
Co-authored-by: Claude <noreply@anthropic.com>
* Add shell integration prompt after wt switch
When shell integration is not active after `wt switch`, show a one-time
interactive prompt asking users if they want to install it. This helps
new users discover shell integration without requiring them to read docs.
## Features
- Interactive prompt with preview option (?) to see what will be installed
- Remembers user choice via `skip-shell-integration-prompt` config flag
- Falls back to hint message for non-TTY or after declining
## Decision flow
1. Unsupported shell → hint with supported shells
2. $SHELL not set → hint to run install command
3. Already installed → hint to restart shell
4. Already prompted → hint to run install command
5. TTY available → show interactive prompt
6. Non-TTY → hint to run install command
## Code improvements
- Consolidated shell install output into `print_shell_install_result()`
- Extracted `print_skipped_shells()` helper to eliminate duplication
- Improved hint styling: commands/paths use `<bright-black>` per guidelines
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: install for all shells on prompt accept, skip flag only on decline
- Prompt now installs for ALL shells with config files, matching
`wt config shell install` behavior
- Skip flag only set on explicit decline, not before prompt (Ctrl+C
doesn't permanently disable prompts)
- Shell detection uses starts_with for versioned binaries (zsh-5.9)
- If no config files exist anywhere, show install hint
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: update prompt_accept snapshot for all-shells install
The prompt now installs for all shells (same as `wt config shell install`),
so the snapshot includes skip messages for shells without config files.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Worktrunk Bot