2319 Commits

Author	SHA1	Message	Date
Maximilian Roos	0e7454890e	fix(hooks): an empty command list no longer leaves a bare heading (#3641) ... `wt hook show` decided whether a section had printed anything from whether the config held an entry for each hook type, not from whether any command was rendered. A hook type declared with an empty command list has an entry but no commands, so the section printed its heading and then stopped: ```console $ cat .config/wt.toml post-switch = [] $ wt hook show PROJECT HOOKS @ /path/.config/wt.toml ``` Both sections carried it, since the loop and the `(none configured)` fallback were duplicated in `render_user_hooks` and `render_project_hooks`. `render_hook_commands` now reports whether it wrote any rows, and the loop plus fallback live once in `render_hook_section`, which both callers delegate to. The flag means "something was printed", which is what the `(none configured)` line claims. The execution path was already correct: an empty list announces nothing, and the JSON output omits it. Verified against the built binary across four cases (project-only empty, user-only empty, an empty list alongside a real hook, and a filter naming the empty type), and pinned by `test_hook_show_empty_command_lists`, which covers both the user and project halves. Reverting the flag to its old meaning fails the test on both. Also corrects a docstring in the same file that still described the preview's pre-#3638 behavior ("shows a `vars.*` template raw"), the same drift #3639 fixed for `wt config alias dry-run`. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 18:13:27 -07:00
Maximilian Roos	9e7ad0144d	fix(hooks): expand everything but vars.* in a preview (#3638) ... Follow-up to #3635, from two reviews that landed after it merged. ## The `vars.*` preview was worse than #3635 claimed `render_template_preview` short-circuits on `template_references_var(template, "vars")`, returning the raw template. So one `vars.` token disabled expansion for the entire command: ``` pre-commit = "deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }}" before: deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }} after: deploy --branch=main --repo=repo --env={{ vars.env }} ``` #3635 described this as "a `vars.*` template renders raw", which is true but understates it: `{{ branch }}` and `{{ repo }}` stopped expanding too, in a command whose whole job is to show the expansion. That short-circuit predates #3635 and has been degrading `wt hook <type> --dry-run` the same way; #3635 only extended it to `wt hook show --expanded`. The fix is at the source rather than at either caller. A preview now injects a stand-in object for `vars` that renders each reference back as itself, nested access included (`{{ vars.config.port }}` round-trips), while every other variable expands normally. `VarsMode::Resolve` keeps execution reading real values from git config; only previews pass `VarsMode::Literal`. A preview also no longer spawns the git read that resolving `vars` required. `vars.*` stays literal on purpose: those values are read when the step runs, after an earlier step in the pipeline may have written them, so a value resolved at preview time can differ from the one the run uses. Nothing covered this, which is why the suite stayed green through the regression. `test_hook_show_expanded_matches_dry_run` now sets a var and asserts the listing and the dry-run both leave it alone while expanding `{{ branch }}` beside it. ## The syntax gate is a type error now #3635 moved the template syntax check out of `prepare_steps` into a free `validate_pipeline_syntax` that both execution funnels had to remember to call. `prepare_steps` now returns a `PreparedPipeline` the caller must resolve: `.validated()` for the paths that run hooks, `.into_unvalidated()` for the listing, which annotates a broken template in place rather than blanking itself. Forgetting is a compile error, the same property `ApprovedHookPlan` gives hook approval. ## Smaller items Four cross-references went stale when the syntax check moved: `PreparedCommand.template`, `validate_template_syntax`, the `switch.rs` skip comment, and `HOOK_INFRASTRUCTURE_VARS` (which still named two deleted functions). The `--expanded` behavior is now documented in the sentence that already owns `{{ vars.<key> }}` semantics, with its three generated mirrors regenerated. `PreparedStep::commands()` replaces two hand-rolled matches in `hooks.rs`. `default_branch` moves inside `build_manual_hook_template_vars` — only the commit-hook arm reads it, and resolving it can cost a `git ls-remote` on a fresh clone, so the other eight hook types no longer pay for it. The listing carries its expansion state in an `Option<String>` instead of re-deriving "was this expanded?" from whether a context exists. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 16:44:29 -07:00
Maximilian Roos	c90c4b1b77	refactor(remove): report a removal's branch fate, not its plan's intent (#3637) ... Follow-up to #3633, which patched the most visible symptom (prune's summary counting candidate kinds). This lands the structural fix behind that class: the removal chain returned nothing from execution, so everything downstream reported the plan's intent as if it were the outcome. The type even said so — `RemoveResult` was produced by `prepare_worktree_removal` before anything ran, and the codebase couldn't agree on what it was (`RemovePlans` in remove.rs, `plan` in prune, `result` in the handler). ## The plan/outcome split `RemoveResult` → `RemovalPlan` (variant `RemovedWorktree` → `Worktree`), and `handle_remove_output` now returns a `BranchFate`: `Deleted` / `Retained` / `NotAttempted` observed synchronously, `Deferred` only for the detached legacy fallback whose outcome this process never sees. Consumers report the fate: - `wt remove --format=json` said `"branch_deleted": true` while stderr said `▲ Removed worktree but kept branch feature (not integrated)` — a pre-remove hook had advanced the branch, the SafeDelete re-check declined, and stdout reported the plan anyway. Now `false`. - `wt step prune`'s summary counts executed outcomes, so a deletion declined mid-run reads `✓ Pruned 1 worktree`, matching the per-item line above it. A declined *orphan* deletion (no worktree entry, nothing removed at all) now drops out of the removed list entirely instead of being counted as either lie. - The detached fallback emits the "kept branch" correction in the one case it can know (no CAS tail to append), instead of letting the progress message's "worktree & branch" stand uncorrected. The picker's `foreground: true, silent: true` contradiction collapses into one `RemovalExecution::{Foreground, Background(fallback), Silent}` axis, and the four hand-rolled capture-refs-then-CAS-delete sites share one `worktrunk::git::execute_branch_deletion` — the single spelling of "the plan said delete; do it now, against fresh refs". ## For the reviewer `src/output/handlers.rs` carries the bulk: each execution path constructs its fate where it observes the result, and rendering decisions (foreground propagates deletion errors, background warns, silent does neither) are unchanged. The merge with #3631's concurrent prune rework is the diff's riskiest region — the worker result channel widened from `Result<bool>` to `Result<Option<bool>>` so fates flow back through the done-channel and the collector stamps `candidate.deletes_branch` before the summary and JSON read it. Deliberately *not* done: reusing the planner from `wt merge`'s finish path (its divergences — clean-check strictness, preserve-vs-error on primary, merge-target — are intentional), and threading the fallback mode structurally (it just relocates an unreachable arm). Zero snapshot churn: the change is provably invisible wherever intent matched outcome. The divergence cases are pinned end-to-end by tests whose hook must be the branch tip to pass. The one deterministically unreachable arm is the detached-fallback warning (needs a cross-filesystem rename failure); expect `codecov/patch` to flag those few lines. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-28 15:49:33 -07:00
Maximilian Roos	4888d5246f	refactor(hooks): derive the hook preview from the execution path (#3635) ... `wt hook show --expanded` rebuilt the hook pipeline's template context by hand in `expand_command_template`, duplicating what `prepare_steps` already does for what actually runs: the same `build_hook_context` call, the same `hook_type` and `hook_name` inserts, the same `args` default, the same POSIX escape mode. Two copies of one rule, so the preview drifted from the executed command whenever the execution path gained a context key, with nothing to catch it. The listing now prepares its commands through `prepare_steps` itself (`hook_command_rows` in `hook_commands.rs`) and renders them through `render_template_preview`, the renderer `wt hook <type> --dry-run` already used. `prepare_steps` is the sole producer of hook command contexts, so a key added there reaches both with no second edit. ## The `args` divergence The old preview inserted `args = "[]"` unconditionally; `prepare_steps` defaults it only when unset, because manual `wt hook <type>` supplies real args upstream via `extra_vars`. The shared path keeps the conditional default and the listing simply does not supply `args`. The values coincide (a listing has no CLI args to forward, which is exactly what the default encodes), and keeping the conditional form means the rule stays written once, in the place that has a caller who needs the other branch. ## Where the syntax check went `prepare_steps` used to reject an unparsable template, so a pipeline that could not render in full never started. A listing wants the opposite: `wt hook show` is what you run when your hooks are broken, so it annotates the bad template in place and shows the rest. That check is an execution policy rather than part of building a command, so it moved out of `prepare_steps` into `validate_pipeline_syntax`, called by the two funnels every hook-running path goes through: `prepare_and_check` (foreground, background, dry-run, filtered) and `render_planned` (the plan-backed hooks behind `execute_planned_hook` and `register_planned`). Both are mutation-verified: removing either call fails a test. A newtype that made forgetting the gate a compile error would be stronger, but it threads a wrapper through `SourcedStep`, `ForegroundStep`, and the background pipeline spec for a guard whose failure mode is degraded fail-fast rather than incorrectness (a syntax error still surfaces when its step renders). ## User-visible changes Preview expansion errors now name the hook (`Failed to expand project:lint: ...`) instead of the generic `hook preview`. A template referencing `vars.*` renders raw in `--expanded`, matching `--dry-run`, where before it resolved against git config at preview time. Raw is the honest preview: those values resolve when the step runs, and an earlier step in the pipeline may write them. ## Tests `test_hook_show_expanded_matches_dry_run` pins the listing and the dry-run to the same rendering of `hook_type`, `hook_name`, and `args`. `test_foreground_pipeline_syntax_error_aborts_before_first_step` pins the relocated gate; it runs through `wt merge`'s pre-commit hooks because `wt hook <type>` cannot reach it (the CLI pre-parses every template for shorthand-argument routing and errors first). Also folded in: adding `source: HookSource` to the listing renderer left `approval_context: Option<(&Approvals, Option<&str>)>` encoding the same user-vs-project discriminator, so both sites now call one `needs_approval`. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 15:25:17 -07:00
Maximilian Roos	e91b5fa885	perf(prune): parallelize removals on the scan lock's read side (#3631) ... `wt step prune` executed removals one at a time under the write side of `check_lock` while the candidate scan fanned out on the read side. On the rust-scale fixture that serialized ~2.6 s of a ~2.9 s live prune. Removals now run concurrently: the main thread streams scan results into a FIFO queue of per-candidate jobs, and a worker pool sized like the rayon pool executes them under the read side of the lock. **Why the read side is safe.** The lock exists for the Windows `.git/config` race (#2801): `git branch -D` rewrites config via lockfile plus atomic rename, and a concurrent reader fails on the rename. The removal chain has since moved to the CAS `git update-ref -d`, and (verified empirically, by inode-watching `.git/config` across each command) neither it nor `git worktree remove` / `git worktree prune` touches config; `branch -D` remains reachable only through arms prune never takes. Concurrent `git worktree prune` runs are idempotent and exit 0, and `update-ref -d` does not consult worktree checkout state, so nothing in the hook-free chain needs exclusivity. The write side remains for the removals that do: hook-bearing candidates (a foreground `pre-remove` body is an arbitrary command with streamed output), `--foreground` candidates (TTY spinner), the fail-hard metadata-pruning candidates (`StaleDetached`, plan-less `Prunable`), and the deferred current worktree. `removal_needs_write` in `src/commands/step/prune.rs` is the decision table; the `check_lock` field doc carries the full analysis. **Output and determinism.** Skip lines ride the same FIFO queue as removals, so `RAYON_NUM_THREADS=1` (one worker) reproduces the exact serial output order the snapshot tests pin; with more workers, whole lines interleave (one `anstream` `eprintln!` writes a line under a single stderr lock). Live `--format=json` output is now sorted by scan index, matching dry-run, with the current worktree still last. The first failing removal flips an abort flag that drains the rest of the queue unexecuted, matching the serial loop's abort-on-first-error. **Measured.** `prune_e2e/live` criterion: ~615 ms to ~398 ms. Rust-scale live one-shot (36 worktrees, 24 candidates, warm): 2.9 s to 0.56 s wall, all 24 removals overlapping in the trace (6.5 s of subprocess time). **Abort semantics.** The first failing removal flips an abort flag: in-flight removals complete (their per-candidate lines already printed), queued ones drain unexecuted, no summary prints, and the first error propagates — the same abort-on-first-error the serial loop had, where an error mid-loop likewise left earlier removals done and later candidates untouched. A mid-scan check error takes the same abort path. **Testing.** New `test_prune_removals_run_concurrently` proves the overlap causally: two orphan-branch removals barrier on each other inside a git shim, and serialized removals trip the shim's timeout sentinel (verified: the test fails under forced `RAYON_NUM_THREADS=1`). `test_prune_removal_failure_aborts_remaining_queue` and `test_prune_concurrent_removal_failures_report_first` pin the abort semantics deterministically (single-worker FIFO, and a barrier forcing two concurrent failures). The config-race canary keeps its role with its premise rewritten: a regression to a config-rewriting deletion is what it would now catch. The deterministic-output snapshot tests pass unchanged. The one patch line no test reaches deterministically is the worker's early `return` when the result receiver is gone (a scan error racing an in-flight removal). Accepted residuals, documented in code: a `packed-refs.lock` contended past git's ~1 s retry budget reads as `RetainedRaced` (fail-closed; unobserved at 24-way concurrency on local disks), and concurrent fail-soft `git worktree prune` races leave at worst stale metadata for the next prune to collect. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-28 12:49:59 -07:00
Maximilian Roos	4c845597b2	fix(prune, remove, ci): count what a removal took, not what it selected (#3633) ... Three follow-ups from the review of #3533, which landed shared-branch retention across the removal surface. Each is small and independent; they share a branch because they came out of the same pass. ## `wt step prune` counted kinds, not outcomes A candidate's kind is what the scan selected, not what the removal took. A branch a sibling worktree still has checked out is retained, so a worktree candidate can take the worktree and leave the branch standing. The summary counted the kind anyway: ```console ○ Worktree directory missing for feature; pruned ↳ branch checked out at ~/code/repo.feature ✓ Pruned 1 branch ← the branch is right there ``` `prune_summary` now counts the planned outcome, so that reads `✓ Pruned 1 worktree` — the stale entry, which is all that went. Both `--format=json` payloads gained `branch_deleted` for the same reason: a consumer reading `{"branch": "feature", "kind": "branch_only"}` would reasonably conclude the branch is gone. The predicate has one home now. `RemoveResult::to_json` already computed `branch_deleted` inline; it moved to `RemoveResult::deletes_branch()` and both callers share it. That incidentally fixes a detached worktree reporting `"branch_deleted": true` beside a null branch — it has no branch to delete. `wt step prune --dry-run` had the same defect and no per-item line to contradict it, so it now predicts retention too. A `Prunable` item has no plan until `try_remove` prunes its stale entry, so the dry run asks `live_sibling_checkout` — the same predicate the plan would. ## `RemoveTarget::Branch` now means "a branch with no worktree" All three callers resolve first and pass `Path` for anything with a worktree, so the arm handling "the branch turned out to have a worktree" was unreachable at selection time — codecov confirmed it never executed. Deleting it would have been enough, but the arm was a live hazard rather than dead weight: the picker builds a fresh `Repository` and re-lists worktrees between selecting a row and removing it, so a `wt switch` in another terminal can give a branch-only row a worktree mid-flight. The old code would then have removed that worktree, from a row that said "delete this branch" — the same defect class #3533 fixed. ```console ✗ Branch feature gained a worktree @ ~/code/repo.feature since it was selected; to remove that worktree, run wt remove ~/code/repo.feature ``` `test_prepare_removal_refuses_branch_that_gained_a_worktree` covers the picker path. I checked it fails when the guard is neutralized rather than passing trivially. ## `affected tests (windows, advisory)` was permanently red git calls a file binary only when it finds a NUL byte in the first 8000, and two of the committed loose objects under `tests/fixtures/*/_git/` are zlib streams small enough to have none. Those diff as text, so `git diff` writes raw deflate into its output, and `cargo affected` aborts decoding it as a string before running a test. Reproduced with `git show <commit> -- <object>` (invalid UTF-8, byte `0x95` at position 5403) and confirmed fixed — the same command now reports `Binary files … differ`. The rest of `_git/` stays text worth reading. ## Testing `prune_summary_counts_a_retained_branch_as_worktree_only` covers the counting; `test_prune_retains_branch_checked_out_in_another_worktree` gained an assertion on the summary line, and I verified it fails when the guard is removed. The six `.snap` changes are all the same added `branch_deleted` key. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 11:05:22 -07:00
Worktrunk Bot	4af04c2260	fix(remove, prune, merge): target the named worktree, retain shared branches (#3533) ... Follow-up to the discussion on #3480: [@max-sixty asked](https://github.com/max-sixty/worktrunk/pull/3480#issuecomment-5039137116) to have `wt remove` honor an explicit path generally. #3607 has since landed the resolution half, `Repository::resolve_worktree`, which turns a path into a worktree. This is the removal half, and it is the part resolution cannot decide: which worktree to act on is a naming question, whether the branch may be deleted is not. ## Problem `wt remove` threw the resolved answer away. For any non-current worktree it re-targeted by branch name, and `prepare_worktree_removal` mapped that branch back to git's *first-listed* worktree. Two failures followed, both silent. **The wrong worktree is removed.** With `feature` checked out twice: ```console $ git worktree list …/dup [feature] …/first [feature] $ wt remove …/first ◎ Removing feature worktree & branch in background (same commit as main, _) ``` `…/dup` is gone. `…/first`, the one named, is still there. **The survivor is left broken.** The shared branch is deleted with it. Worktrunk deletes branches with `git update-ref -d`, git's compare-and-swap primitive, which unlike `git branch -d` does not refuse a ref that is checked out somewhere: ```console $ git worktree list …/first 0000000 [feature] $ git -C …/first rev-parse HEAD fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree. ``` `wt step prune` reached the same deletion unattended, and `wt merge` reached it with a freshly integrated branch, so nothing else declined. A branch gets a second worktree only through `git worktree add --force`; worktrunk never does it itself. ## Fix **Remove the worktree that was named.** `wt remove` drops non-current worktrees via `RemoveTarget::Path`. `wt step prune` does the same: its candidates already carry a path, and targeting a *stale* entry by branch name resolved to a live worktree that the same prune had just skipped as too young, then removed it. **Retain a branch another worktree holds.** One predicate, `live_sibling_checkout`, answers "would deleting this ref orphan a checkout?", and every path that can delete a branch asks it: `prepare_worktree_removal`'s worktree and pruned-branch-only arms (covering `wt remove`, `wt step prune`, and the picker, which already targeted by path) and `wt merge`'s finish. A hit forces `BranchDeletionMode::Keep`, the single chokepoint every deletion path honors, and names the surviving checkout: ```console $ wt remove …/dup ◎ Removing feature worktree in background ○ Branch feature retained; still checked out @ …/first ``` A sibling whose *directory* is already gone is stale metadata, not a checkout with anything to lose, so it does not retain: removing the last live checkout still deletes the branch. **`-D` is refused out loud.** Everywhere else `-D` is the override that wins, so one that cannot be honored warns rather than passing quietly: ```console $ wt remove …/dup -D ◎ Removing feature worktree in background ▲ Branch feature retained despite -D; still checked out @ …/first ``` The ordinary single-checkout case is unchanged, and a retained branch skips the integration check entirely rather than computing a verdict it would discard. #3480's duplicate-checkout hint now points at `wt remove <path>`, which this makes the safe answer. ## Testing Full gate green. New coverage, each case asserting the survivor still resolves `HEAD`, which is the corruption in question: - `remove`: by path, by name, refused `-D`, the pruned-directory fallback, and the mirror case where a stale sibling must *not* retain. - `step prune`: a stale entry whose branch is live in an age-skipped worktree. This test is what surfaced the wrong-worktree bug in prune. - `merge`: merging a branch that a `--force` duplicate also holds. ## Not addressed `wt step prune`'s summary counts candidates rather than outcomes, so a retained branch still reports `✓ Pruned 1 branch`. The per-item line above it already says the branch was retained. Fixing the count means threading removal outcomes back through prune's accounting, which is a separate change. > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 09:57:34 -07:00
Maximilian Roos	835d5f0cc1	perf(switch): compute only the template vars --execute names, and give the context a type (#3628) ... `wt switch --execute` built its template context with `referenced: None`, so every invocation resolved every standard variable whether or not the command named one. That context feeds `expand_template` and nothing else: the payload reaches the child as a shell string through the EXEC directive file (or `sh -c`), never as JSON on stdin, and `--execute` renders no `-v` variables table. Every unnamed variable was a git subprocess whose result was discarded. `var_default_branch` is the one with teeth. On a clone with no `refs/remotes/origin/HEAD` and no cached `worktrunk.default-branch`, it falls through to `git ls-remote`, so `wt switch feature -x 'echo hi'` reached the network for a variable the command never mentions. Traced on a scratch repo in that state: | `--execute` | before | after | | --- | --- | --- | | `echo hi` | 13 subprocesses, 1 `ls-remote` | 8, none | | `echo {{ commit }}` | 13, 1 | 9, none | | `echo {{ default_branch }}` | 13, 1 | 11, 1 (correctly still fetches) | `referenced_vars_for_templates` unions minijinja's `undeclared_variables` across the command and its trailing args, which are two expansion positions over one context map. It does not error on an unparsable template: `validate_switch_templates` has already parsed both positions against `ValidationScope::SwitchExecute` before the switch runs, and reports a syntax error against the position that carried it. ## The second commit: why `None` was reachable at all `build_hook_context` returned a bare `HashMap<String, String>`, so each of its seven callers had to know four separate things about the result: how to reborrow it as `HashMap<&str, &str>` for `expand_template` (five copies), how to serialize it for a child's stdin (four copies, two carrying the same `expect` string), what to insert into it afterwards, and whether it had been filtered. The doc comment on that last point said "hooks pass `None`", which described the callers rather than the rule, so `--execute` inherited `None` by sitting next to code that needed it. `TemplateContext` (`src/config/expansion.rs`) owns all of it, with `expand` / `to_json` / `insert` / `get` / `remove` as the interface and the map private. It serializes transparently, so the JSON a hook child reads on stdin and the `PipelineSpec` the background runner deserializes are the same flat object as before. The filter argument becomes `VarScope` rather than `Option<&BTreeSet<String>>`. `VarScope::All` has to be typed out, and its doc states the discriminator: `Referenced` when only the templates read the context, `All` when a child reads the JSON or the command's output is the variable listing. The same enum replaces the second `Option<&BTreeSet<String>>` on `format_alias_variables`, which meant the same thing (which vars render as dim `(unused)`). Two changes fell out of that rather than being planned. `src/llm.rs` had its own private `TemplateContext` for LLM prompt data, which would have made two types of that name in one crate; it is now `PromptContext`. And `expand_shell_template` shrank to a one-line wrapper but stays, because it still fixes POSIX for its four callers and converts the error type. Not included: `render_hook_commands`'s preview path reimplements what `prepare_steps` does (build context, insert `hook_type`, insert `hook_name`, default `args`, expand POSIX), so the preview drifts from what actually runs whenever the execution path gains a context key. That touches `wt hook` output and wants its own change. ## Testing `test_switch_execute_computes_only_referenced_vars` reads `var_*` spans out of the `-vv` `trace.jsonl` and pairs the variable-free case with a `{{ commit }}` control, so an empty span set cannot pass by the trace simply missing them. Reverted to `None` it fails with `got ["var_commit", "var_default_branch", "var_primary_worktree", "var_remote"]`. Two unit tests cover `referenced_vars_for_templates` directly (union across positions, unparsable template contributes nothing). The refactor is covered by the existing suite rather than new tests, which is the point: 4606 tests pass with no snapshot movement, so no user-visible output changed. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 22:17:39 -07:00
Maximilian Roos	a285ed308d	refactor(tests): bring the mock-stub env vars under the WORKTRUNK_TEST_ prefix (#3621) ... `MOCK_CONFIG_DIR` and `MOCK_CALL_LOG_DIR` were the only two worktrunk-invented environment variables without the `WORKTRUNK_` prefix. That is not just a naming inconsistency: `isolate_subprocess_env` scrubs the parent environment by prefix — `GIT_*` and `WORKTRUNK_*` — so an unprefixed name is the one thing a test child inherits from whoever ran the suite. Renaming them brings them under that scrub. - `MOCK_CONFIG_DIR` → `WORKTRUNK_TEST_MOCK_CONFIG_DIR` - `MOCK_CALL_LOG_DIR` → `WORKTRUNK_TEST_MOCK_CALL_LOG_DIR` `TEST` rather than a bare `WORKTRUNK_` because both are read only by `tests/helpers/mock-stub` — they are the protocol between the harness and its helper binary, and `wt` itself never reads either one. That matches the ~15 existing `WORKTRUNK_TEST_*` knobs. ## The snapshot half Mechanical but not a substitution: the `env:` block is byte-sorted by key, so the renamed entry moves position within it and a `sed` in place would leave it where the old name sorted. All 971 affected blocks were rewritten by dropping the old line, inserting the new one, and re-sorting — the aggregate diff is exactly one removed and one added line per file: ``` 971 files changed, 971 insertions(+), 971 deletions(-) - MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" ``` The redaction placeholder follows its neighbours' convention in `add_standard_env_redactions` (`WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR` → `[TEST_NU_VENDOR_AUTOLOAD]`), so it now reads `[TEST_MOCK_CONFIG]` rather than repeating the full key. `MOCK_CALL_LOG_DIR` appears in no snapshot — its two call sites are `.output()` assertion tests — so it needs no redaction. ## Keeping it from regressing `tests/CLAUDE.md` gains the rule under "Where a new environment variable goes": name it `WORKTRUNK_TEST_*`, and the rule covers the harness↔helper-binary protocol, not just knobs `wt` itself reads. Without it the next helper-binary variable gets named `MOCK_*` again and the hermeticity hole reopens. ## Verification `cargo run -- hook pre-merge --yes` passes (exit 0) on the merged tree: 4607 tests including `--features shell-integration-tests`, `pre-commit run --all-files`, clippy, doctests. No pending snapshots. A repo-wide sweep finds no remaining unprefixed spelling. ## Merged main #3620 landed while this was in flight and regenerated several `for_each` snapshots that still carried the old key, so main is merged in here. It resolved with no conflicts, and the result is what you'd want rather than what git happened to produce: those blocks now carry #3620's new keys (`GIT_ALLOW_PROTOCOL`, `CLAUDE_CONFIG_DIR`, `WORKTRUNK_TEST_PARENT_SHELL`) *and* the renamed key, each in sorted position. The sweep and the gate above both ran after the merge. #3620 also rewrote the "Where a new environment variable goes" section this branch adds to — three layers became four. Both edits survived; the new naming paragraph follows the updated layer list. The two advisory `affected tests` checks are red for the same reason, and merging clears them: `cargo affected` errors on `git diff stdout was not valid UTF-8`, because the diff from this PR's base contains 16 binary files — the `tests/fixtures/standard/` git objects and index files that #3620 deleted. This branch's own commit contributes none. A PR based after #3620 won't see them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-07-26 21:44:57 -07:00
Maximilian Roos	4202cffe61	fix(ci): split ci by cadence so coverage uploads on every main commit (#3608) ... ## What prompted this Getting #3605 green ran into codecov reporting a `base_commit` three commits older than the real merge-base. This audits whether our config causes that. ## The cause Codecov picks a PR's base by walking back to the newest ancestor that has a coverage report. It used the real merge-base for PRs #3480, #3532 and #3602, and a stale one for #3603 and #3605. The difference is whether the merge-base uploaded a report. **29 of the last 40 main commits did not.** `ci` had one concurrency group for main pushes, and GitHub cancels the *pending* run in a group whenever a newer one joins, even with `cancel-in-progress: false`. So the question is how long a run holds the group, and a run isn't done until its slowest job is: | job | duration on main | |-----|------------------| | `fast-checks` | 2 min | | `code-coverage` | 3-4 min | | `test (windows)` | 11 min | | `collect affected coverage (windows)` | 110-129 min | Each main run held the group for ~2 hours, so nearly every subsequent main push was cancelled while queued, taking the 4-minute coverage job with it. Every cancelled main run's `updated_at` lands within a second of the next push's `created_at`. The 2 hours is real work, not queue: 2-5s from `created_at` to `started_at`, then 108 minutes inside `cargo affected collect` — 4181 tests under `-C instrument-coverage` with a per-test LLVM profile, ~5 GB of profraw. ## The fix: one workflow per cadence The three groups of jobs have incompatible needs, and one group was serving all of them. | workflow | cadence on main | why | |----------|-----------------|-----| | `ci` | every commit, ~11 min | required gate + fast checks | | `coverage` | every commit, keyed per-sha | a skipped upload leaves later PRs on a stale base | | `affected` | sampled, ~2 h | a DB a few commits old still anchors a correct superset | `affected` keeps exactly the grouping it has today, so its sampling is unchanged and deliberate. It just no longer drags the other two along. ### Scope of the impact The posted `codecov/patch` check scopes to the PR's own GitHub diff, so a stale base did **not** score PRs against other people's lines. On #3605 the posted 91.66% is exactly `github.rs`'s 11/12, while the stale-base compare object reported 64/65 across 13 files. What a stale base costs: - `codecov/project` reports "compared to \<stale sha\>" - the patch `auto` target is the stale base's project coverage (0.02pp here) - the compare API object widens to `base..head`, which is what made the investigation look like silence Separately, `test`/`lint`/`fast-checks` also stopped completing on main. Nothing load-bearing rode on that (they already ran on the PR), but it left `tend-ci-fix` with nothing to watch, since it doesn't fire on cancelled runs. ## Two smaller fixes - `ignore: "**/tests/**"` compiles to `.*/tests/.*` (confirmed against codecov's validator), which needs a leading directory and so never matched `tests/` itself. Inert today since `cargo llvm-cov` reports only `src/` (verified against a downloaded `cobertura.xml`), but now correct if that changes. Now `tests/**`. - `fail_ci_if_error` gated on `github.repository_owner`, which is the *base* repo's owner on a fork PR too, so the soft-fail its comment describes never applied. It keys off the head repo now. ## Docs The API behaviour was ours to misuse, not codecov's to explain. Three traps, all confirmed against the live API: - `file_report/<path>/` 404s with `coverage info not found` because the route swallows the trailing slash into the path. Without it the endpoint returns `line_coverage`. - `?pullid=N` always compares the PR's **current** head. `?base=&head=` asks about an earlier commit. - the compare response has no `patch_totals` key, and `.name` is `{base, head}` rather than a string, so a filename lookup silently matches nothing. A working recipe already existed in `running-tend`, but that skill is scoped to CI. `tests/CLAUDE.md` owns coverage investigation, so the queries go there and `running-tend` points at them instead of keeping a second copy. Re-running the corrected query against #3605's failing commit reproduces the miss exactly: `src/git/remote_ref/github.rs:164`, the `gh repo set-default` hint, matching what the session eventually found by hand. ## This PR demonstrates it It changes no Rust at all, only YAML and markdown. Codecov still reported a **10-file, 111-line patch** on its first commit, because it based the comparison on `203603909` rather than the real merge-base `32f380a27`. Every main commit in between has no report: | commit | ci run | report | |--------|--------|--------| | `32f380a27` | queued | no | | `9645e3e13` | cancelled | no | | `bcd1ffdfd` | cancelled | no | | `8865f20ab` | cancelled | no | Every one of those 111 patch lines belongs to somebody else's merged commit. It passed at 100% only because those commits are well covered. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-26 20:19:15 -07:00
Maximilian Roos	9cb128328f	refactor(tests): deny network transports for in-process git, consolidate fixtures (#3620) ... An in-process `Repository::run_command()` spawns git with the test process's own environment, so none of the harness env isolation reaches it — no `GIT_CONFIG_GLOBAL`, and crucially no `GIT_ALLOW_PROTOCOL`, meaning a unit test driving the library directly could make a real network connect (reproduced: `ls-remote` against a fixture-style `https://` URL attempted the wire). The repo's own config is the one layer such a spawn still reads, so every fixture repo now carries `LOCAL_TEST_CONFIG` (identity, `commit.gpgsign`, `protocol.allow = never` with a `file` exception), written by a single `TestRepo::assemble` that every constructor routes through. The consolidation on top of that fix: - **`git_test_env()`** is the one home for the nine git-isolation settings; `configure_git_cmd` (Command), `configure_git_env` (`Cmd`), and `pty_env_vars` (PTY) all consume it. Composes with #3618's PTY layering — the env doc in `tests/CLAUDE.md` now reads four layers. - **The test gitconfig is one process-static file** (`test_gitconfig_path()`); the per-fixture copies (including two hand-rolled, drifted ones in `bare_repository.rs`) are gone. - **`BareRepoTest` and `NestedBareRepoTest` compose over `TestRepo`** (new `TestRepo::bare_at()`), so they inherit `assemble`'s guarantees structurally. Their public APIs are unchanged, deliberately — ~220 call-site references and the bare-repo snapshots are written against them. - **The committed `tests/fixtures/standard/` is deleted** (83 files), along with its Taskfile generator and the `_git` rename/sed dance. `build_standard_fixture` constructs the template once per `target/<profile>` (private-dir build + atomic rename, so parallel nextest processes race benignly) and `copy_standard_fixture` keeps only copy + per-copy gitdir rewrite. Two review-worthy subtleties in the fixture rebuild: - The old fixture's commits were stamped `-08:00` (the generator machine's timezone) and **~170 snapshots embed the resulting commit SHAs**, so the builder pins that timestamp as an explicit constant and `test_standard_fixture_template_reproduces_pinned_shas` asserts the four exact SHAs — the template reproduces the committed fixture bit-for-bit. - The old fixture's index files carried an untracked-cache extension from the generator machine's config, which made `git status` print `warning: untracked cache is disabled…` into exactly two `for_each` snapshots. The clean-room template drops it; those two snapshots are regenerated here (that's the whole content diff, plus documented-cosmetic env-block lines). Tested by the full pre-merge gate including `shell-integration-tests` (4583 tests) plus a post-merge-with-main revalidation. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 19:41:38 -07:00
Maximilian Roos	5ecdf2bb78	perf(remove, prune): one check per guarantee in the removal chain (#3617) ... `wt step prune` scans in parallel, but each removal ran a serial chain of ~17 git subprocesses under the scan write lock: a re-run of `prepare_worktree_removal` (the scan had already run it and kept only `is_ok()`), a post-hook "safety refresh" (status + fresh snapshot + integration re-compute) that ran even when no pre-remove hooks executed, and a final pre-rename clean check that ran right after the fsmonitor daemon stop, paying a full un-fsmonitored re-stat. At rust-lang/rust scale that post-stop status is the 0.5-1.7 s per-removal driver. This trims the chain to one mechanism per guarantee, with no behavioral additions: - **Prune passes the scan-time plan through** (`CheckOutcome.plan`) instead of re-preparing under the write lock. `Prunable` candidates still prepare in `try_remove` because preparing them prunes stale metadata, which must stay serialized. - **`refresh_removal_safety_after_pre_remove` is deleted.** `prepare_worktree_removal` now computes the integration verdict and effective target for display (carried on `RemoveResult::RemovedWorktree`), the pre-rename clean check catches hook-dirtied worktrees, and `delete_branch_if_safe`'s CAS re-decides the branch deletion against fresh refs. The clean check also moves into `remove_worktree_with_cleanup`, so the foreground and picker paths get their final gate adjacent to the rename rather than relying on the deleted refresh. - **The clean check runs before the fsmonitor daemon stop.** The stop must stay before the rename: on Windows the daemon holds a handle on the worktree that would fail the rename, and git's graceful stop resolves the daemon by worktree path, so it cannot reach it after the move (verified: a healthy daemon self-exits and unlinks its socket within ~500 ms of the rename; the client from the renamed path reports "not running"). - **`prune-gather` stops shelling out**: the main worktree comes from `git worktree list` ordering (documented: main lists first; `list_worktrees()` filters bare entries) instead of a `git rev-parse --git-dir` per worktree. Criterion on the `prune-4-8` fixture: `dry_run_probe_cold` 197 ms to 147 ms (-25%), `dry_run_warm` 110 ms to 62 ms (-43%), `live` 783 ms to 615 ms (-21%). A `prune-12-8` one-shot timeline (24 candidates, live): wall 3.14 s to 2.09 s; per-worktree removals 170-210 ms to 112-134 ms, now dominated by the ~60 ms daemon stop. The rust-scale numbers in `benches/CLAUDE.md` are marked for re-measurement on the next fixture rebuild. **Consistency corner, unchanged in kind but wider in trigger:** a worktree that becomes dirty after `handle_removed_worktree_output` starts (in practice: a pre-remove hook dirtying it) now fails at the pre-rename check, which is after the cd directive is written (and, in background mode, after the "Removing … in background" message and retention hint print), and the shell wrapper applies the cd file regardless of exit code. So in that corner the shell moves to the primary worktree while the dirty worktree is preserved with an error. The same outcome already existed for a file appearing between the refresh and the final check; nothing is removed in either case. Testing: full pre-merge gate green (4,578 tests, no snapshot changes), plus a new shim-based test covering the skip path when a stale (`Prunable`) candidate's preparation fails at removal time. The hook-dirtied corner has no dedicated test; existing dirty-worktree removal tests cover the command-entry gate. The one remaining uncovered added line is the `?` propagation closing the live scan closure (`prune.rs:1023`), which has no deterministic trigger. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-26 17:34:14 -07:00
Maximilian Roos	951dac773f	fix(tests): give the picker's directive files a temp directory of their own, and make a PTY failure explain itself (#3619) ... Chasing the Windows-only failure of `test_switch_picker_runs_execute_command` that @max-sixty flagged after #3607 merged: it failed the advisory `affected tests (windows)` leg while the required `test (windows)` leg **passed the same test on the same SHA** in the same run. ## The failure message was the whole problem ``` assertion `left == right` failed: Expected exit code 0 for picker switch with --execute left: 1 right: 0 ``` That is everything CI had. wt prints an error before exiting non-zero, so there was a message — it just never reached the captured frame, because of two harness gaps: - **The post-exit drain was a single non-blocking sweep**, taken the instant `try_wait` reported the child reaped. The child's last writes can still be in the PTY at that moment, so the tail — wt's `✗` line — was dropped. This is also why #3427 concluded a picker-create "self-exited 1 after fully-correct work with no error on screen": the error was there, just not drained. - **`child.kill()` ran unconditionally after the exit poll.** On Windows that is `TerminateProcess(proc, 1)`, so a hung picker and a picker that failed on its own both report exit 1 — the ambiguity that made #3427 reason from wall-clock time to rule out a kill. Both are fixed here: drain until the output goes quiet, and panic on a hang rather than killing silently, so every exit code a caller asserts on is one the child chose. The four directive tests now also dump the screen (and the EXEC file) in their exit-code assertions. ## What reproducing it turned up Loop-based reproduction of the reported symptom is hopeless by the numbers: across the last 38 PR runs, exactly one advisory job failed — this one — i.e. ~1 failure per 160,000 test executions. What *did* reproduce was a different Windows failure in the same five tests, at a rate 1000× higher, once the loop ran with realistic neighbours (the full suite running alongside, as the failing leg had at test 3111/4204): ``` thread '…switch_picker::test_switch_picker_emits_cd_directive_by_default' panicked at tests\integration_tests\switch_picker.rs:2531:45: failed to create cd temp file: … PathError { path: "D:\\tmp\\.tmp3XI9wb", err: Os { code: 5, kind: PermissionDenied, message: "Access is denied." } } ``` **4 of 80 runs of the five directive tests.** `directive_files()` called `NamedTempFile::new()` in the shared temp directory; `tempfile` retries a name collision only when it surfaces as `AlreadyExists`, and on Windows `create_new` against a name held by a *directory* — or by a file in delete-pending state — returns `PermissionDenied`, which it hands back to the caller. A full suite run fills that directory with `.tmpXXXXXX` entries (every `TestRepo` makes one), so the collision is routine rather than astronomical. The helpers now take a `TempDir` and put fixed-named files inside it. `TempDir::new` isn't exposed to the same problem — a directory collision surfaces as `AlreadyExists`, which tempfile retries — and fixed names inside a private directory can't collide with anything. That covers all 75 call sites of `directive_files()` / `legacy_directive_file()`, not just the picker's five, and the switch_picker copy of the helper is gone. ## The reported exit-1 is not proven fixed Stated plainly: the panic above is a *different symptom* from the reported exit 1, so this PR does not prove that failure is gone. What the evidence does say, and what a reader should check first if it recurs: - The failing run took 3.34s where the same test passing on the required leg took 3.11s and the same switch *without* `--execute` takes 2.42s. The `--execute` tail — `build_hook_context`'s git lookups, ~0.7s of the difference — had therefore run, so the failure was at the end of the pipeline, not in the picker or the switch. - After that tail, the only fallible step left is the EXEC directive append. Every other step is infallible or `.ok()`-swallowed. - That write's error was context-free, which is why nobody could place it. `wt` rendered `✗ No such file or directory (os error 2)` — no file, no operation, and positioned right under `◎ Executing (--execute):`, where it reads as if the *command* were missing. It now reads: ``` ✗ Failed to write the command to the directive file /tmp/tmp.aBc No such file or directory (os error 2) ``` The message lives in `append_line` / `write_cd_path` rather than at their four call sites, so every directive write says which file and what didn't get written — including the legacy-protocol arms, which no test can drive to failure. So the next occurrence prints its own diagnosis, in one look, instead of `left: 1, right: 0`. ## Also here **The four directive tests select their row by cursor, not by a typed query.** They typed `target` and waited for `target-branch`'s preview pane before Enter. That gate can only assert what skim *painted*: the filtered item list is swapped into `ItemList` during a render, while `Accept` reads `item_list.selected()` — the cursor's slot in whatever list is current at that moment. The list-pane `>` pointer comes from the same render state the accept reads, which is why this file's own `wait_for_cursor_on_row` calls it the race-free signal. One Down reaches `target-branch` (the picker sorts the current worktree first) and the arrow is re-issued until the pointer settles, so no query — and no matcher — is in the accept path at all. It also drops the wait on the preview pane rendering. ## Testing - Reproducer and confirmation both on Windows CI, via a temporary workflow (deleted in the last commit): 80 runs of the five directive tests with the full suite running alongside. **4 failures before the fix, 0 after.** The concurrent full suite passed 4208/4208 in both. - Local: `switch_picker` (68), plus every suite that calls these helpers — `directives`, `switch`, `remove`, `security`, `bare_repository`, `step_alias`, `user_hooks`, `step_relocate` (861 tests) — green, plus clippy `-Dwarnings` on all targets with `--features shell-integration-tests`. - The two directive-write messages are pinned by tests of their own (`test_exec_directive_write_failure_names_the_file`, `test_cd_directive_write_failure_names_the_file`): an unwritable directive path fails the same way on every platform, so the paths that produce these messages are covered rather than checked by hand. ## Follow-ups, not in here - `build_hook_context` is called with `referenced: None` on the `--execute` path, so a `-x 'echo hi'` with no template variables still runs the `default_branch`, `primary_worktree`, `commit`, `short_commit`, `remote`, and `upstream` lookups — roughly doubling the command's subprocess count (30ms → 60ms locally, 2.42s → 3.11s in the Windows test timings). The alias path already passes its referenced set; a `--execute` command receives no JSON context, so the reason hooks pass `None` doesn't apply to it. - `cancel_background_commands()` is a no-op on Windows (`signal_background_pid`), so the picker's "the picker's exit cancels the unfinished request" promise isn't kept there: preview git children keep reading `.git/config` while the accept path writes `worktrunk.history` and `worktrunk.default-branch` to it. Issue #2564 is the same shape of concurrent-config-access failure on Windows. - `assert_valid_create_exit_code`'s Windows tolerance (#3427) and `assert_valid_abort_exit_code`'s acceptance of `1` both rest on the kill-vs-self-exit ambiguity this PR removes; they can be tightened once a Windows run confirms nothing else produces a stray 1. > _This was written by Claude Code on behalf of Maximilian Roos_	2026-07-26 15:41:35 -07:00
Maximilian Roos	9032308400	refactor(tests): give the PTY test environment one home (#3618) ... Follow-up to #3616, which fixed a PTY snapshot flake by adding one env knob — and to add it I had to touch three separate env builders, none of which knew about the others. This consolidates that surface. ## What was there The environment a test subprocess runs in was assembled at nine sites: five copies of the PTY prologue (`env_clear`, HOME, PATH, the Windows block, coverage passthrough) and four partial restatements of the determinism knobs. There was no rule for which belonged where, so adding a knob meant finding every copy, and a missed copy surfaced later as a flake somewhere unrelated. ## What's there now Three named layers, each with one home in `src/testing/mod.rs`: | Layer | Home | Contents | |---|---|---| | Baseline | `STATIC_TEST_ENV_VARS` | knobs every child needs, whatever it's attached to | | Terminal | `PTY_TEST_ENV_VARS` (new) | knobs only a TTY triggers — `WORKTRUNK_TEST_SPINNERS=0` | | Fixture | `pty_env_vars(TestEnvPaths { … })` (new) | the paths that vary per fixture | `configure_cli_command` and `configure_pty_command` apply them by transport. That turns the standing `// NOTE: TERM is intentionally NOT in STATIC_TEST_ENV_VARS` comment into a consequence rather than an exception: `TERM` is transport-level, so it can't sit in a baseline both transports share. `WORKTRUNK_TEST_SPINNERS` stays out of the shared baseline deliberately. It's inert on a pipe, and insta-cmd records the whole environment into every snapshot it writes (`Info::from_std_command` builds it unconditionally from `cmd.get_envs()` — there's no hook to suppress it), so putting it there would add a no-op line to 1043 snapshot files. `configure_pty_command` is now the only place a PTY child's isolation is set up. `shell_command`, `execute_shell_script`, `configure_pty_environment`, `exec_in_pty_shell`, `exec_bash_truly_interactive` and two `wt switch` spawns all delegate to it. `shell_wrapper`'s `STANDARD_TEST_ENV` and `bare_repository`'s hand-rolled `test_env_vars` are gone, as are `configure_shell`'s hand-copied knobs and four redundant `CLICOLOR_FORCE` lines in `switch_picker`. Net −149 lines. One spawn stays outside: the Windows ConPTY smoke test, which runs PowerShell against a deliberately bare environment and isn't a wt child at all. ## Reviewing Start at `src/testing/mod.rs` — the three layers and their doc comments are the whole design. Everything under `tests/` is deletion plus a delegation call. Two snapshots change: `install_preview_with_gutter` and `install_preview_declined` now carry ANSI, because those two tests previously ran without `CLICOLOR_FORCE`. Text is identical. Arguably a fix — the test named "with_gutter" couldn't see the gutter (a background-color block), while its own prompt line was already colored, so the file was internally inconsistent. ## Testing Full `wt hook pre-merge --yes` green: 4601 tests, `--features shell-integration-tests`, `RUSTFLAGS='-D warnings'`, `insta --check`. The knob's delivery path was verified by probe rather than by inspection. With `sleep 6` in the mock `llm`, `test_readme_example_hooks_pre_merge` passes; flipping `PTY_TEST_ENV_VARS` to `"1"` reproduces the original failure byte for byte: ``` +␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (4s)␛[22m␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (5s)␛[22m ``` So the knob reaches the shell-wrapper PTY child through the shared setup, not through a surviving copy. Both probe edits are reverted. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 14:59:48 -07:00
Maximilian Roos	d6ef23d573	test(pty): suppress the TTY spinners in PTY captures (#3616) ... `test_readme_example_hooks_pre_merge` flakes when the whole suite runs together. The mock `commit.generation` command gets starved past the watchdog's 4s threshold, and the PTY capture records the redraw frames a terminal would have erased: ``` ↳ Waiting for the commit generation command (4s) ``` The elapsed seconds vary run to run, so accepting the snapshot just moves the flake. ## The fix Both spinners in `src/progress.rs` now check `WORKTRUNK_TEST_SPINNERS` alongside the existing TTY (and, for `Watchdog`, verbosity) gate, and the PTY env builders set it to `0`. This is the shape the repo already uses for `WORKTRUNK_TEST_DELAYED_STREAM_MS=-1`, whose comment names the same failure mode: "slow CI triggers progress messages that don't appear on faster systems". Only the render is gated, so the counters behind the summary lines the snapshots do assert are untouched. Gating both spinner types rather than the one call site closes the latent exposure elsewhere: `Progress` renders after 300ms in any PTY test that copies or removes files, and `Watchdog` also wraps `wt switch` and the version check. ## Why not filter the frames out of the capture A line-scoped filter in `add_pty_filters` would be a one-liner, but it only covers the single-row block. Past 10s the watchdog escalates to two rows and redraws with a cursor-up, which spans a newline the filter cannot follow, and the revealed command line carries a tempdir path. Closing that would mean modeling cursor arithmetic in a regex that silently deletes bytes from captured output. ## Evidence Reproduced first, by making the mock command slow directly rather than waiting for ambient CPU starvation to do it. The resulting diff is byte-identical to the reported flake, including the `(4s)` / `(5s)` counters: - `sleep 6` in the mock, before the fix: FAIL, same diff as reported - `sleep 6`, after the fix: pass - `sleep 12` (past the 10s escalation tier, where a filter would not have helped): pass - negative control, `sleep 6` with only `WORKTRUNK_TEST_SPINNERS` flipped from `0` to `1`: FAIL again, so the knob is what engages Full gate green: 4601 tests, no pending snapshots. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 14:02:40 -07:00
Maximilian Roos	63071709af	feat(config): deprecate list.task-timeout-ms (#3615) ... Removes the `[list] task-timeout-ms` per-task command timeout and the thread-local machinery behind it. The key stops having any effect immediately; a config that still carries it loads, warns, and is stripped by `wt config update`. The timeout killed any git command that outlived its budget, on every collect worker, through a thread-local that `Cmd::run` consulted on each invocation and clamped an explicit `.timeout()` against. Progressive rendering removed the reason for it: `wt list` and the picker paint from local data and stream results in behind the frame, so no single git command can hold up the first paint. What it bounded instead was completion, which `[list] timeout-ms` already bounds directly, and the drain falls back to a hardcoded 120s `DRAIN_TIMEOUT` whenever that is unset (`collect/mod.rs` `drain_deadline`), so removing this cannot introduce an unbounded wait. Both keys default to unset, so the default path never had a per-task timeout at all. `[list] timeout-ms`, the wall-clock budget for the whole collect phase, stays. It is the surviving knob and the more direct expression of the same goal. With the thread-local gone, the two-source `min()` in `Cmd::run` collapses to the command's own `self.timeout`, so every explicit `.timeout()` caller keeps its bound unclamped: `PROBE_TIMEOUT` in `git/reap.rs`, the fsmonitor stop/lsof bounds in `git/remove.rs`, the version check in `config/show.rs`, and `REMOTE_DETECTION_TIMEOUT`. ## Deprecation A `Structural` row in `DEPRECATION_RULES` strips the key from `[list]` in both the section and inline forms, top-level and per-project, following the `[switch.picker] timeout-ms` precedent (also a strip with no equivalent key to migrate into): ``` ▲ User config: list.task-timeout-ms is no longer used — list.timeout-ms bounds the collect phase ``` The env overlay (`WORKTRUNK__LIST__TASK_TIMEOUT_MS`) and `--config-set` route through the same rule and migrate silently, since neither layer has a file for `wt config update` to materialize. Neither errors. ## Testing New unit tests cover detection and migration for the section, inline, and per-project forms plus the warning text, and two cases join the `test_warning_fires_iff_update_changes` battery that pins the warn-iff-update-changes invariant. The three integration tests that exercised the feature are gone; `Cmd::timeout` keeps its own coverage in `shell_exec.rs`, so dropping the four thread-local unit tests loses nothing for the surviving path. Verified end to end that a config carrying the key loads, warns, and has it stripped by `wt config update` with sibling keys intact. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 13:36:41 -07:00
Maximilian Roos	14580de79c	feat(worktree): accept a worktree path wherever a branch is accepted (#3607) ... Follow-up to the [`wt remove <path>` discussion on #3480](https://github.com/max-sixty/worktrunk/pull/3480#issuecomment-5039137116), widened from that one command to the whole surface. #3480 has since landed and is merged in here — its duplicate-checkout warning composes with this: the warning names the shadowed worktrees, and a path is how you then address one. ## Audit Verified against the built binary. wt had three answers to "what does this token mean?": | Route | `@` `-` `^` | worktree path | `pr:N` | |---|---|---|---| | `wt switch` (`resolve_switch_target`) | yes | only if absolute or ≥2 components, and not `--create` | yes | | `wt remove` (`resolve_worktree_arg`) | yes | any token | no | | everything else (raw `worktree_for_branch`) | **no** | **no** | no | Same token, same cwd, two answers: ```console $ wt remove inner # ✓ Removed innerbranch worktree & branch $ wt switch inner # ✗ No branch named inner ``` And outside switch/remove the shortcuts didn't work at all — `wt step diff --branch @` was `✗ Branch @ has no worktree`, while `wt config state marker set --branch @` silently wrote state under the literal key `@`. Separately, wt prints paths as `~/…` but wouldn't accept that form back. ## Change One canonicalizer in the lib, `Repository::resolve_worktree`, absorbing the path fallback that lived in the bin crate's `resolve_worktree_arg` (now deleted). Resolution order is documented once, on that function: `@`, then `-`/`^`, then a branch with a worktree, then a path naming a registered worktree, then the branch alone. **Branch-first, everywhere.** A directory never shadows a branch that shares its name; a path answers only what a branch cannot — a detached worktree, or one of two checkouts of the same branch (#3480's case). The `looks_like_path` shape gate is gone, so a single-component path resolves like any other. Two shapes cover what callers need: `require_worktree` for commands that need a worktree to operate in, `require_selected_branch` for arguments that key by branch. The merge/rebase target validators fall through to the same path lookup, so a target can be named by the worktree it's checked out in. Routed through it: `switch` (including `--base`), `remove`, `step commit --branch`, `step diff --branch` and its target, `step copy-ignored --from`/`--to`, `step promote`, `step relocate`, `config state --branch` (9 sites), and `merge` / `step rebase` / `step squash` / `step push` targets. `resolve_input_path` — already documented as the one resolution point for user-supplied paths — now expands a leading `~`, so the tilde form worktrunk prints is a form it reads back. `~user` stays literal; wt doesn't reimplement that shell feature. ## Documentation A path is an alias, not a second addressing scheme, so it is stated once rather than on every argument: one paragraph in `wt switch`'s help and one sentence on the addressing line in `worktrunk.md`. Argument descriptions still read as branches. The two exceptions are the arguments whose descriptions are already catalogues of accepted forms — `wt switch`'s (`Branch, worktree path, shortcut, or PR/MR URL`) and `wt remove`'s, which has named the path since before this branch. The Worktree Model section of `CLAUDE.md` records which way to document it, so the next argument doesn't grow its own copy. ## Two silent no-ops fixed along the way - `wt step relocate <unmatched>` matched arguments against branch names by string equality, so a typo filtered everything out and the empty result rendered as `○ All worktrees are at expected paths` — a success message for work that never happened. Every way an argument can fail to land on a relocatable worktree now errors, including the detached and prunable cases the new path route makes reachable. - A selector matching nothing was reported as a branch without a worktree, hinting `wt switch <token>` — which creates a worktree only when the branch exists, so for a mistyped path it would just fail again. `WorktreeSelectorNotFound` now says `No branch or worktree named X`; a branch that genuinely exists without a checkout keeps the create hint. ## Testing Full gate green: 4596 tests, lints, docs sync, `--features shell-integration-tests` clippy. `codecov/patch` is 99.25% of diff hit against a 97.93% target. New coverage: - Unit: branch-and-path equivalence, branch-beats-same-named-directory, detached-by-path (and its `require_selected_branch` refusal), shortcuts never treated as paths, branch-only fallthrough, and the two distinct not-found errors. Plus `expand_tilde` round-tripping `format_path_for_display`. - Integration: `switch` by relative/single-component/absolute/tilde path, `--base` by path, `step diff --branch` by path and `@` (asserted equal to the by-branch output), `config state --branch` set via `@` and read via the worktree path, and both new relocate errors. `wt remove`'s resolution is unchanged — it already had this rule; it now shares the implementation. The 106-test `remove::` suite is untouched and green. - Integration: `wt step push <worktree-path>` (the `require_target_branch` half of the target fallback), and `wt step relocate` against a prunable worktree. One diff line is unhit: `expand_tilde`'s fallback when `home_dir()` returns `None`, which has no deterministic trigger. The `@`-resolution backstop in `resolve_worktree` is untested for the same reason — no CLI route reaches it — so it kept its original `match` arm rather than being re-indented into the diff. ## Left out `wt config state default-branch set` and `previous-branch set` take a branch name as a *value to store* rather than a selector, so they still take it literally. > _This was written by Claude Code on behalf of Maximilian Roos_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 09:10:46 -07:00
Maximilian Roos	04d8d587f2	feat(list): flag a branch checked out in more than one worktree (#3606) ... ## Problem Follow-up to #3480. That PR made a duplicated branch checkout (`git worktree add --force <path> <branch>`) visible at *resolution* time: `worktree_for_branch` warns once per branch, then resolves to whichever worktree git lists first. `wt list` said nothing about it. Two rows named `feature`, and no column that explains why. The nearest thing to a signal was accidental. A force-added duplicate usually lands off-template, since the original holds the template path, so it picks up `⚑` for the location mismatch — while the worktree *at* the template path, the one `wt` actually resolves to, carried no flag at all. Exactly backwards from what's useful. The framing from the request: a worktree in the wrong location gets a status flag; a worktree sharing its branch should get one too. ## Solution `⚑` now covers both, on every worktree of the duplicated branch, resolved one included. Which worktree `wt` picks is git's listing order, so singling out the shadowed rows would imply a legitimacy the ordering doesn't carry. ``` @ main ^| | . 05a4a45d 16h Initial commit + feature ⚑_ ../repo.feature 05a4a45d 16h Initial commit + feature ⚑_ ../repo.feature-dup 05a4a45d 16h Initial commit ``` This started as a seventh glyph (`⧉`) and collapsed onto `⚑` in the second commit. The Status column is a dense alphabet the reader has to learn, and the two states say one thing: this worktree's place in the branch ⇔ worktree map is irregular. Off-template path and branch-claimed-twice are both instances. The table already distinguishes them without a glyph — a repeated Branch cell is the duplicate, an odd Path cell the mismatch — so the flag only has to say "not a rendering glitch, look at the Path column". Sharing the glyph means sharing its dim-yellow styling, since the codebase treats a symbol's color as part of its identity; #3480's warning remains the loud channel, firing the moment any command resolves the branch. **The Path column comes along.** It previously appeared only for a location mismatch, on the reasoning that the path is otherwise redundant with the branch. A duplicate inverts that: the branch name no longer identifies the row, and the path is the only thing telling the two apart. The layout flag is renamed from `has_branch_worktree_mismatch` to `path_is_informative` to say what it now means. **The data model keeps the distinction.** JSON has no cardinality budget, and reporting a duplicate that sits at the template path as `branch_worktree_mismatch` would be false — its path does match. Schema 1's `worktree.state` names the cause (`"duplicate_branch"`), schema 2 gets its own `worktree.duplicate_branch` bool beside `branch_mismatch`, matching that schema's one-fact-per-field shape. The priority between the two `⚑` states now decides only which cause JSON reports. Detection is one pass over the worktree list (`duplicated_branches`, next to #3480's `worktree_paths_for_branch`), in memory, pre-skeleton, no git calls. ## Testing - `test_worktree_paths_for_branch_detects_duplicates` gains the set form and a detached-HEAD worktree, which has no branch to duplicate. - `test_metadata_worktree_state_priority` covers the two `⚑` states' ordering and both yielding to `⊟`/`⊞`. - `test_list_duplicate_branch` snapshots the table above, showing both flagged rows and the Path column earning its place. - `test_list_duplicate_branch_json` asserts both schemas flag exactly the two duplicated rows. The flag only fires on a state no prior test sets up, so the only snapshot churn is the help pages and the schema-2 envelope's new field. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 07:16:22 -07:00
Worktrunk Bot	cad89eaebb	fix(relocate): skip worktree blocked by an immovable occupant (#3530) ... ## Problem `wt step relocate` could strand a worktree in the internal staging directory and then error out. When worktree **A**'s target is occupied by worktree **B**, A depends on B vacating. But if B is itself *blocked* — its own target is a non-worktree path and `--clobber` wasn't passed — B never moves. The dependency-resolution loop's no-progress branch assumes "no progress ⟹ a cycle to break," so it temp-moved A into `.git/wt/staging/relocate/`. `finalize_temp_relocations` then ran `git worktree move <temp> <A's target>` into the still-occupied path, which fails and propagates via `?` — leaving A relocated into the staging dir rather than at either its original or expected path. ### Reproduction - A at `/w/a` wants `/w/b`; B at `/w/b` wants `/w/c`; `/w/c` is a plain non-worktree file, no `--clobber`. - B is classified `target_blocked` and never moves. - A is temp-moved by `break_cycle`, then `finalize` fails with "Failed to move worktree from temp to final location", stranding A in staging. ## Fix `is_target_empty` only distinguished "occupant moved" from "occupant present"; it couldn't tell a still-pending occupant from a permanently-blocked one. A new `blocked_occupant` helper detects when a dependent's target is held by a worktree already in the `blocked` set, and the loop propagates the block to the dependent (skipping it cleanly) instead of handing it to `break_cycle`. Setting `made_progress` re-drives the loop so a chain of dependents (A → B → C-blocked) is blocked transitively. After the fix both worktrees are skipped and the command succeeds: ```console ▲ Skipping beta (target blocked: repo.beta) ↳ To backup blocking paths, use --clobber ▲ Skipping alpha (blocked by beta, which can't be relocated) ○ Relocated 0 worktrees, skipped 2 worktrees ``` ## Test `test_relocate_blocked_occupant_skips_dependent` sets up exactly this shape and asserts the command succeeds, both worktrees stay put, and nothing is stranded in `.git/wt/staging/relocate/`. Before the fix the command exited non-zero and `alpha` was left in the staging dir, so both the snapshot (`success: true`) and the location assertions would have failed. Found during the nightly code-quality survey of `src/commands/relocate.rs`. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-26 05:34:12 -07:00
Maximilian Roos	32f380a27b	refactor(switch): forward the forge CLI's own error text (#3605) ... Follow-up to #3600, independent of it (no shared files). GitHub and GitLab rewrote forge API failures into messages of our own, discarding what the CLI said. GitLab picked its three by prose — `error_text.starts_with("404")` — the pattern #3595/#3597 removed elsewhere. ## Before / after A GitLab MR that doesn't exist: ``` - ✗ MR !9999 not found + ✗ glab api failed for MR !9999 + ▎ glab: 404 Not found (HTTP 404) ``` A GitHub PR that doesn't exist — the one message kept, now with gh's verdict under it rather than instead of it: ``` ✗ PR #999999 not found on max-sixty/worktrunk (gh default). Check that `gh repo set-default` points to the correct repository. + ▎ gh: Not Found (HTTP 404) ``` ## The rule, now written down `src/git/remote_ref/mod.rs` gets an **Error Messages** section: forward the CLI's own line, and write our own only where the CLI *structurally cannot* report the condition. Two cases qualify — GitHub's 404 (it answers about an owner/repo *we* chose, from `gh repo set-default` or a remote; gh can't know that) and Azure's missing `azure-devops` extension (a precondition `az extension list` answers, which `az` never names). Reading more nicely than the CLI doesn't qualify. `cli_api_error` and `cli_api_error_details` gained docstrings for the mechanism: our context as the headline, the CLI's words in the gutter, so a provider with something to add passes it as `message` rather than bailing. ## Evidence Measured, not assumed — both CLIs render an API failure the same way: | | stdout | stderr | |---|---|---| | `gh api …/pulls/999999` | `{"message":"Not Found",…,"status":"404"}` | `gh: Not Found (HTTP 404)` | | `glab api …/merge_requests/…` | `{"message":"401 Unauthorized"}` | `glab: 401 Unauthorized (HTTP 401)` | `cli_api_error_details` prefers stderr, so the gutter gets the human line, not the JSON. That also surfaced a test bug: **the glab mocks never set stderr**, so they'd have enshrined output real glab doesn't produce. Fixed to match the probe. `gh` puts the status in a structured field, so its 404 arm keys on `status`. `glab` puts it only inside the message text — nothing to key on, which is why prose-matching was the only way to keep those arms. ## What this gives up A bad or expired token no longer gets "run gh auth login" / "run glab auth login"; it gets `gh: Bad credentials (HTTP 401)`. The far more common case — no auth configured at all — is unaffected: gh prints its own "please run: gh auth login" and always did fall through to forwarding. Happy to restore the GitHub 401 arm (it's structurally keyable); GitLab's can't come back without prose-matching. ## Testing `cargo run -- hook pre-merge --yes` → 4572 passed. Six snapshots reviewed individually before accepting (five updated, one new). Both paths also driven through the **real** `gh` binary (a live 404 and a live 401), not only mocks. Clippy clean with `--features shell-integration-tests`. Coverage: the GitHub 404 arm and the forwarding fallthrough each have a test, on both forges. `codecov/patch` also flagged one line — re-indenting the 404 block pulled a pre-existing gap into the patch, and it turned out to be real: the `gh repo set-default` half of the hint, the fork workflow the message exists for, had never been tested. `test_switch_pr_not_found_gh_default` closes it. > _This was written by Claude Code on behalf of Maximilian Roos_	2026-07-25 15:02:07 -07:00
Maximilian Roos	9645e3e132	fix(shell): reclaim the legacy wrapper paths instead of inspecting them (#3602) ... `wt config shell install` decided whether the file at a legacy location was worktrunk's by reading it — a substring test for the wrapper header, falling back to "every code line looks like an integration line". Anything that didn't match was left in place. Fish sources `conf.d` at startup, so whatever `conf.d/{cmd}.fish` defines is already loaded by the time fish would autoload the `functions/{cmd}.fish` the install just wrote — the stale definition wins and the new wrapper may never load. Ownership is now the path. `conf.d/{cmd}.fish` and the stranded nushell `{cmd}.nu` candidates are paths worktrunk itself computes for the command name being installed, so install takes them back whole without reading them. Only that exact filename is touched: a neighbour under another name is not worktrunk's, and each removal is still reported. `wt config shell uninstall` still reads the header, because it takes no `--cmd` and so genuinely doesn't know the name — it lists the shell-owned directories and has to tell our `{cmd}.fish` from the user's own files beside it. That is the one place ownership can't come from the path, and it prompts and previews every file before removing it, which install does neither of. `is_worktrunk_managed_content`'s docstring now says so; `is_worktrunk_managed_nushell` existed only to gate the install-time deletion and is gone. Audited the rest of the surface for the same principle: the OpenCode plugin (`~/.config/opencode/plugins/worktrunk.ts`) is already removed by path with no inspection, the Claude plugin delegates to `claude plugin uninstall`, Gemini installs through `gemini extensions install` and leaves nothing of ours on disk, and fish is the only shell with a separate completion file — which uninstall already covers. No other gaps. ## Data safety This deliberately widens what install deletes, so it's worth naming: two tests asserted the old conservatism and now assert the new boundary — `test_configure_shell_fish_reclaims_conf_d_path` (was `..._preserves_user_conf_d_file`, from #3589) and `test_nushell_install_reclaims_only_the_command_name` (was `..._keeps_unmanaged_legacy_file`, from #2992). Both put a neighbour file in the same directory to pin that only `{cmd}.{ext}` is taken. The FAQ's "What can Worktrunk delete?" inventory is updated to describe path-based ownership rather than the old content-marker rule. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 14:10:00 -07:00
Worktrunk Bot	bcd1ffdfde	feat(worktree): warn when a branch is checked out in multiple worktrees (#3480) ... ## Problem `git worktree add --force <path> <branch>` bypasses git's "already used by worktree" guard, so the same branch can be live in two worktrees at once — breaking worktrunk's branch ⇔ worktree bijection. worktrunk never creates that state itself, but once it exists [`worktree_for_branch`](https://github.com/max-sixty/worktrunk/blob/cfd7fe469/src/git/repository/worktrees.rs#L64) silently resolved to whichever worktree git listed first (roughly creation order), and that choice flowed into every resolution path — `wt switch`, `wt push`, `wt step diff`, `wt context`, the picker — with no warning, error, or way to name the shadowed worktree. This implements the **detect-and-warn** option from #3392: make the invisible choice visible without changing resolution semantics. ## Solution - `worktree_for_branch` now collects **all** worktrees on the branch via a pure `worktree_paths_for_branch` helper; when more than one exists it warns once (per branch, deduplicated per process) and still resolves to the first — semantics are unchanged. - The warning names every path and points at a concrete fix: ``` ▲ Branch feature is checked out in 2 worktrees; wt uses the first: ┃ ~/repo.feature ┃ ~/repo.feature-dup ↳ To drop a duplicate, run git worktree remove ~/repo.feature-dup ``` Detection is separated from emission so the pure list logic is unit-testable and the ambiguity check stays cheap — `list_worktrees()` already holds the full list. ## Testing - Unit test `test_worktree_paths_for_branch_detects_duplicates` — the pure helper returns both paths in git's listing order for a duplicated branch, one path for a unique branch, and none for an absent one. - Integration test `test_step_diff_duplicate_branch_warns` — creates the duplicate with `git worktree add --force`, runs `wt step diff --branch=feature`, and snapshots the rendered warning. The warning only fires when a duplicate exists (which no prior test sets up), so existing snapshots are unaffected. --- Closes #3392 — automated triage --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-25 13:47:19 -07:00
Maximilian Roos	8865f20ab8	fix(git): bound the default-branch remote query, and make the bound bound (#3603) ... Follow-up to #3596, which walled the test suite off from the network. That PR left the same unbounded call live for real users: `Repository::default_branch()` is the one detection helper allowed to fall through to `git ls-remote`, and nothing limited how long it could take *not* to answer — an unanswered SYN costs ~127 s per address on Linux (`tcp_syn_retries=6`) and git tries each of a host's addresses in turn, so a remote behind a dropped VPN or a dead host stalled `wt list --full` and `wt switch` for minutes. ## `Cmd::timeout` didn't bound wall-clock Adding a bound alone would have been decorative, which is the substance of this PR. `run_with_timeout_impl` killed only the direct child, and a grandchild inherits the child's stderr pipe — so a surviving one held the write end open and `read_to_end` blocked until it exited. Measured on a 3 s timeout over an `ls-remote` whose upload-pack sleeps 120 s: | | elapsed | |---|---| | before | 120.03 s | | after | 3.20 s | `git-remote-https` sitting in `connect()` is exactly that shape: it doesn't notice git died. So a timed child spawns into its own process group and expiry tears down the group — `killpg` with TERM → KILL escalation on Unix, `taskkill /T /F` on Windows, matching `wt step tether`. Every existing `.timeout()` caller (the fsmonitor stop/lsof probes, `reap.rs`, the shell probe) was latently unbounded the same way and is fixed with it. The isolation costs the kernel's tty broadcast: a Ctrl-C no longer reaches a timed child, so the user waits out the remaining bound. That's seconds, against an orphan holding a pipe for as long as its own operation takes, once per spawn. Recorded at `run_with_timeout_impl` and in CLAUDE.md's Signal Handling section, whose "only the current child does" claim was no longer true. ## The cache is the other half A timed-out query takes the local-inference fallback but is *not* persisted to `worktrunk.default-branch`. That cache is what stops later calls from re-detecting, so a guess made while the network was down would otherwise become the repo's permanent answer. `detect_from_remote` now returns a `RemoteDetection` enum so the cacheability decision is explicit and exhaustive rather than an `Option` that loses the distinction. Only a timeout separates cleanly, via `ErrorKind::TimedOut`. `ls-remote` exits 128 whether the network is down or the remote simply has no HEAD, so telling those apart would mean reading git's error text — and re-querying on every command is the cost the cache exists to avoid. Those stay cached, as before. ## Reviewing - `src/shell_exec.rs` — the process-group teardown; its docstring carries the rationale - `src/git/repository/config.rs` — `RemoteDetection`, the 10 s bound, and the no-persist path - `src/git/repository/mod.rs` — `run_command` now delegates to a `run_command_bounded` that takes the bound Also routes the four remaining hand-rolled git test envs (`src/git/remove.rs`, `src/git/repository/tests.rs`, `tests/integration_tests/bare_repository.rs`) through `configure_git_env`, so they carry #3596's `GIT_ALLOW_PROTOCOL` deny rather than re-deriving a subset of it. All four run local-only git commands today, so this is completeness, not a live hole — and a full suite run under `GIT_TRACE` confirms zero `git-remote-*` transport-helper spawns across 4374 tests. ## Testing Three tests, none of which touch the network: the grandchild case via `sh -c 'sleep 30; :'` (which stops the shell `exec`ing sleep, so there really is a grandchild), the end-to-end no-persist behavior via `remote.origin.uploadpack` pointed at a `sleep`, and the fail-fast unresolvable remote, which had no coverage before. The second waits out the real 10 s bound; both hanging-remote tests are `#[cfg(unix)]` since the vehicle needs a POSIX `sleep`, so `test (windows)` doesn't cover the no-persist path. > _This was written by Claude Code on behalf of max_	2026-07-25 13:47:09 -07:00
Worktrunk Bot	203603909d	test(picker): release --prs loading marker on parent exit, not a timer (#3532) ... ## Problem `test_switch_picker_prs_shows_loading_marker` intermittently times out on `test (windows)` — it flaked on #3531 (a docs-only change), which is what prompted this fix. The test asserts a *transient* frame: while a mocked, delayed `gh pr list` is in flight, the picker header shows `↳ Loading open PRs…` and the `#42` row has not yet streamed in. The mock held that loading state for a fixed time (originally 3s). On a loaded Windows runner, the PTY boot sequence — `boot_picker_pty`'s skim-ready wait plus its initial `wait_for_stable`, together with the async worktree-column churn (`·` placeholders resolving) — can consume that whole window. By the time the stabilization helper first polls for `Loading open PRs`, the mocked fetch has already returned and the rows have streamed in, so the marker is gone and the wait times out at `STABILIZE_TIMEOUT` (30s). ## Fix Per review feedback ([comment](https://github.com/max-sixty/worktrunk/pull/3532#issuecomment-5039146240)), tie the marker's lifetime to the picker's, with **no timing number at all** — the file's "poll, don't pick a time" rule applied to the producer side. `mock-stub` gains a `hold_until_parent_exit` response mode: the mocked `gh pr list` holds until `wt` exits instead of sleeping a fixed `delay_ms`. Detection needs no platform primitive (`getppid` / `OpenProcess`): - While `wt` lives, its `--prs` fetch thread runs the mock with a piped stdout and blocks reading that pipe to EOF, so as long as the mock neither writes a full response nor exits, the loading marker stays. - When the picker aborts, `wt` detaches the fetch thread (`drop(prs_handle)`) and exits, which closes the read end of the mock's stdout pipe. The mock's next write then fails with `BrokenPipe` (Rust ignores `SIGPIPE`, so the broken write surfaces as an `Err` rather than killing the process), and the mock exits. Polling for that write error is a **causal** parent-death signal — release is tied to `wt` exiting, not to a timer — and it behaves identically on Unix and Windows, so there's no platform-specific FFI to get right. The one-byte probes written into stdout are never observed (this mode's contract is that the parent aborts without parsing the response, and the fetch thread drains the pipe until process exit). A 60s poll cap bounds a stuck orphan if the pipe somehow never breaks. This removes the race the earlier 30s bump only widened: the marker is present for the entire window the poll could observe it, on any boot latency, and no orphaned sleeper is left behind — the mock exits the moment the picker does. ## Testing `cargo test --test integration --features shell-integration-tests test_switch_picker_prs_shows_loading_marker` passes in ~1.3s, confirming the capture-and-abort path stays fast and the mock releases promptly on `wt` exit. The sibling `--prs` tests (`test_switch_picker_prs_{github,gitlab}_list`, `..._rows_survive_alt_x_removal`) still pass through the refactored `mock_forge_env` helper. Reasoning is documented inline at the mock-stub call site and in `wait_for_parent_exit`. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-25 13:42:06 -07:00
Worktrunk Bot	8fd289ac48	test(alignment): drop dead no-op cross-row alignment check (#3575) ... Nightly survey finding in `tests/integration_tests/column_alignment_verification.rs`. The block commented "Additional check: verify that ALL rows have the same column start positions" computed a `matching_boundary` for each header boundary and then did nothing with it — the only branch was `if matching_boundary.is_none() { continue; }`, and the match case had no body. It never asserted, recorded an error, or affected the test result, so a genuine cross-row misalignment would have passed it silently. The comment implied a verification that wasn't happening, which is worse than no check — it reads as coverage that doesn't exist. Removed the dead block along with the now-unused `all_row_boundaries` accumulator and its per-row `push`. The remaining per-row check (each column measured against the header position, lines 205–260) still runs; since every row is verified against the same header, cross-row consistency is already covered transitively — the deleted block was redundant even had it worked. No behavior change to the assertions: the five `column_alignment_verification` tests plus the other alignment suites still pass (`cargo test --test integration column_alignment` → 10 passed). No new regression test — this removes dead test code with no production-behavior change to guard; the existing alignment tests remain the coverage. _Note, not changed here:_ the per-column check for non-`Path` columns (lines ~240–258) only `println!`s a warning rather than pushing to `errors`, so today only a `Path`-column misalignment can actually fail this test. That looks deliberate (Path was the historical alignment bug), so I left it alone rather than tightening it and risking false failures on intentionally-sparse columns — flagging it in case a maintainer wants it made assertive. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-25 13:01:33 -07:00
Maximilian Roos	ae363c2b4c	test: stop the suite reaching the network (#3596) ... `ci_status::test_list_full_with_configured_platform_github` hit nextest's 180s timeout in an advisory CI job while the same 2586-test run passed everything else. Its unloaded runtime is 2.2s, and a `-vv` trace of the same fixture shows exactly one non-local subprocess: `git ls-remote --symref origin HEAD` against `https://bitbucket.org/test-owner/test-repo.git`, the URL the test sets on itself. `Repository::default_branch()` falls through to the wire when neither the worktrunk cache nor `origin/HEAD` resolves, which is sanctioned behavior, but nothing bounds it, so the test blocks for as long as the connect takes. Reproduced by pointing that remote at an unroutable address: 11s on macOS, and on Linux an unanswered SYN costs ~127s per address (`tcp_syn_retries=6`), with `bitbucket.org` publishing six. It isn't one test. Setting `GIT_TRACE` across the harness shows a single full-suite run spawning 180 `git-remote-https` processes: 63 to github.com, 42 to gitlab.com, 27 to dev.azure.com, 3 to bitbucket.org, and the rest to names that don't resolve. Every one is unbounded, on three OSes, on every PR. So the fix goes in the harness rather than in the one test. `GIT_ALLOW_PROTOCOL=file` joins `STATIC_TEST_ENV_VARS`, which covers every `wt` subprocess and every PTY env builder including the hand-rolled ones, and the two git-only builders set it as well for direct `git_command()` spawns that don't consult that table. Local paths and `file://` keep working; anything else fails in milliseconds with `transport 'https' not allowed`, and default-branch detection falls back to local inference, so output is unchanged. `GIT_TERMINAL_PROMPT` was never enough on its own: it only suppresses the credential prompt that the host's 401 triggers, by which point the request has already gone out. The rust-lang/rust benchmark fixture clone is the one git call that *should* reach upstream. It opts back in through `allow_network_transports`, which lives next to the constant so the deny and the opt-out can't drift apart. `switch_pr_self_hosted_tea_authed` recorded the resolver's `Could not resolve host` in its snapshot, which is the same defect class (asserted output depending on DNS). It now records the transport refusal. Also folded in: `build_worktree_config_bare_layout` in `src/git/repository/tests.rs` carried a nine-line hand-rolled copy of `configure_git_env`, so it now calls it. Not addressed here: nothing bounds that `ls-remote` for real users either, so an unreachable remote blocks `wt list --full` and `wt switch` for minutes on the first call per repo. That needs a decision about whether a timed-out detection should write the `worktrunk.default-branch` cache. > _This was written by Claude Code on behalf of max_	2026-07-25 12:15:42 -07:00
Maximilian Roos	58444440d7	fix(ci-status): classify tea failures by shape; report azure-devops extension (#3597) ... Two follow-ups from #3595, both in code that talks to a forge CLI. ## Gitea CI status had the same exit-0 blind spot as the switch path `tea api` never reads the HTTP status — it copies the response body to stdout and exits 0 — so the `!output.status.success()` gate in `src/commands/list/ci_status/gitea.rs` never fired for a 4xx/5xx. #3595 fixed this for `wt switch pr:<n>`; the CI-status backend is the second instance of the same bug. The commit-status path was the quieter one: every field of `GiteaCombinedStatus` carries `#[serde(default)]`, so Gitea's `APIError` body deserialized as `{state: "", total_count: 0}` — an error indistinguishable from "this commit has no statuses". The PR-list path was noisier but less harmful: the error body failed to parse and logged `Failed to parse tea api pulls JSON`, blaming an API change for what was an API error. `tea_api_error` now reads the response shape — the `APIError` body, whose message feeds the existing retriable sniff — and a non-zero exit keeps meaning what it always meant, `tea` itself failing (transport error, no login). A proxy's HTML error page stays unclassified, as before: sniffing arbitrary bodies is the string-heuristic pile this removes. ## `wt config show --full` reports the azure-devops extension The whole `az repos` command group ships in that extension, so without it CI status stays blank however well `az` is authenticated — a persistent, user-fixable condition, which is what this surface is for. `azure_devops_extension_installed` (added in #3595) answers it in JSON: ```console ✓ az installed & authenticated ▲ azure-devops extension not installed; run az extension add --name azure-devops ``` An `az` that can't answer counts as installed, so an unanswered check never becomes an accusation. The `wt switch pr:<n>` bail now uses the same sentence — one wording for one condition, which is the one-line change to `switch_pr_azure_extension_not_installed.snap`. ## Testing The two new Gitea tests were checked against the unfixed source: reverting `gitea.rs` alone fails both, so they pin the fix rather than describe it. The two pre-existing retriable tests kept their exit-1 shape but lost their fabricated payload — `exit 1` alongside `429 Too Many Requests` is the combination `tea` can't produce — and now carry a real transport error, which is the case that does exit non-zero. `config show --full` gets a case per extension state. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 11:54:15 -07:00
Maximilian Roos	c136cbcd6a	fix(switch): classify forge-CLI failures by structure, not tool prose (#3595) ... Follow-ups from #3589's audit of brittle string matching, plus one live bug the audit turned up. ## Gitea: the error path was unreachable From tea's own `cmd/api.go`: `runApi` never inspects `resp.StatusCode` — it copies the response body to stdout and returns nil. So `tea api` **exits 0 for 404, 401, and 403**, which put the whole 404/401/403 classification behind an `if !output.status.success()` that those responses never reach. A missing PR surfaced as `Failed to parse Gitea API response for PR #N. This may indicate a Gitea API change.` The fabricated `{"message":"403 Forbidden"}` fixtures hid this by mocking an exit code tea does not produce. The response *shape* now discriminates — the PR object, or Gitea's `APIError` body whose message is surfaced verbatim. A non-zero exit means `tea` itself failed (no login configured, transport error) and keeps the existing `cli_api_error` path, which shows tea's own stderr. ## Azure: the install command, from JSON #3589 dropped the appended `az extension add --name azure-devops` because detecting the case needed prose matching. `az extension list --output json` answers it structurally instead, and the whole `az repos` command group ships in that extension — so it's a precondition of the call, not one guess among many. The check runs only after a failure, so the happy path pays nothing, and an `az` that can't answer counts as installed: an unanswered check never becomes an accusation the user would act on. ## Statusline detection `is_statusline_configured` matched the substring `"wt "`, which `newt status` also satisfies. That is not only a wrong diagnostic line — `install-statusline` early-returns on it, so a foreign statusline made install refuse. It now asks for the adjacent token pair `list statusline`, which is the actual question and stays agnostic to `wt` / `git-wt` / an absolute path. ## Worktree removal: comment only The remaining `error.contains("not empty")` stays. There is no structured form to recover: the text is `git worktree remove`'s own stderr, so the failure never passes through a Rust `io::Error`, and `remaining_entries` can't substitute because permission-denied also leaves entries behind. The comment claiming an `io::ErrorKind` was lost upstream is corrected. ## Testing Every changed path is covered by an integration test with realistic mocks: the four Gitea tests now mock exit 0 with `APIError` bodies (`gitea_server_error` became `gitea_request_failed`, since a 5xx no longer exits non-zero), the Azure tests mock `az extension list` on both sides — present, and failing so the check comes back unanswered — and a new `install_statusline_foreign_command` test asserts a foreign statusline gets replaced rather than mistaken for worktrunk's. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 03:44:53 -07:00
Maximilian Roos	321ffe838c	fix(step): gate every staging path on an unmerged index (#3588) ... `wt step relocate --commit` committed conflict markers, the bug #3579 fixed everywhere else. It stages with `git add -A` and calls `CommitGenerator::commit_staged_changes` directly, so it reached neither of that PR's two gates: ```console $ git merge side # CONFLICT (content): Merge conflict in f $ wt step relocate --commit ◎ Committing changes in feature... ✓ Committed changes @ eb4cf19 $ git show HEAD:f <<<<<<< HEAD feature ======= side >>>>>>> side ``` `MERGE_HEAD` is gone and the working tree is clean afterwards, so the broken merge reads as finished. ## The gate belongs at the staging step, not the commit The obvious funnel is `commit_staged_changes`, the one call all three staging paths share. It cannot work, and the reason rules the shape out rather than merely disfavoring it: every caller stages *before* that call, and `git add -A` collapses an unmerged path's three index stages into one entry. After it, `git diff --diff-filter=U` returns nothing and git itself stops refusing. ```console $ git ls-files -u 100644 df967b9… 1 f 100644 ba2906d… 2 f 100644 2299c37… 3 f $ git add -A $ git diff --name-only --diff-filter=U # empty $ git commit -m x # succeeds ``` A check there passes vacuously in exactly the case it exists for. So the funnel sits one step earlier. `WorkingTree::stage(StageMode, action)` refuses, then stages, with nothing in between. It is now the only path to `git add` against a worktree's real index (every other `git add` writes a throwaway index via `temp_index`), so a fifth entrypoint can neither forget the gate nor order it after the staging that destroys the evidence. It also replaces three hand-rolled copies of the `StageMode` to `git add` match, so the callers come out smaller than they went in. ## The second gate is not belt-and-suspenders The callers keep their early refusals, which guard a different window: those run before hooks, prompts, and the LLM call, so a doomed commit runs no project commands. That placement leaves a hole the early refusal structurally cannot see, because a `pre-commit` hook is arbitrary project code, free to conflict the index after the gate has passed. ```console $ cat .config/wt.toml pre-commit = "git merge side" $ wt step commit --yes ◎ Running pre-commit project hook ✓ Committed changes @ 9e89c76 # markers and all ``` So `wt step commit` carried the same bug, reachable through any project config. That path isn't experimental, which may make it the more consequential half of this PR. `test_step_commit_refuses_hook_created_conflict` pins it. ## relocate skips rather than aborting An unresolved conflict is a per-worktree blocker the user has to fix by hand, like a locked worktree. relocate is a bulk command whose every other blocker (locked, uncommitted, template_error, occupied target) skips one worktree and carries on. Aborting the run would leave the other N-1 unrelocated, and would make passing `--commit` strictly less capable than omitting it. ```console $ wt step relocate --commit ▲ Skipping feature (1 path with unresolved conflicts) conflict.txt ✓ Relocated other: ~/code/wrong-clean → ~/code/myproject.other ``` `--format=json` records it as `reason: "unmerged"`, alongside the existing codes. Relocating a conflicted worktree *without* `--commit` is unchanged. Confirmed by hand that `git worktree move` carries the unmerged index, `MERGE_HEAD`, and the markers across, and that git still refuses to commit at the new path. Nothing stages there, so nothing is lost. ## Detection stays git's Nothing greps file contents for `<<<<<<<`. The whole chain is one structured question, `git diff --name-only --diff-filter=U -z`, which asks git which paths its index records as unmerged: the same question `git commit` asks itself. A content grep would false-positive on this very repo, whose docstrings contain the markers. The only marker-content checks in the diff are two test assertions, that the markers stay on disk for the user and never reach a commit. ## Merged with #3587 #3587 landed while this was in flight (as did #3578, whose `wt step push` refusal test shares the same spot in `merge.rs`) and hoisted `ensure_no_unmerged_paths("merge")` to `wt merge`'s entry, so merge refuses in its own name. The two are complementary: that PR gives each entry point a gate in the name the user typed, and this one guarantees the staging step itself is gated no matter who reaches it. Both sides edited the same docstring and added a test at the same spot; the merge keeps both paragraphs, lists all four entry points, and keeps both tests. The interaction leaves one narrow seam. `wt merge --no-squash`, with a `pre-commit` hook that conflicts the index, now refuses with `Cannot commit` rather than `Cannot merge`, because the entry gate already passed and the funnel names the step it fired in. Fixing the wording would mean threading a caller-supplied noun into `CommitOptions`, which #3587 considered and rejected as the wrong shape, so it stays. The default squash path isn't affected: squash stages before its hooks, so the funnel fires ahead of them. ## Verification Three integration tests, each asserting HEAD is unmoved rather than only matching the message: - `test_relocate_refuses_unmerged_paths`, the reported bug. - `test_relocate_unmerged_skips_only_that_worktree`: one conflicted worktree, one merely dirty. The sibling still relocates, and JSON carries `reason: "unmerged"`. - `test_step_commit_refuses_hook_created_conflict`, driven from a `pre-commit` hook, so it can only pass through the check inside `stage`. Gate green on the merged head (4557/4557, lints, fmt, doc sync), plus `cargo nextest run --features shell-integration-tests` (4546/4546 pre-merge) and `cargo clippy --all-targets --features shell-integration-tests`, which the gate doesn't compile. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-24 19:34:33 -07:00
Maximilian Roos	b623d7f951	fix(shell): don't delete user data that only mentions the init command ... Three places decided whether text was worktrunk's by testing a blob for substrings. Two of them deleted the user's data when they guessed wrong. `wt config shell uninstall` asked two unlinked questions of an rc line — is an exec keyword anywhere in it, and is `<name> config shell init` anywhere in it — so `alias setup='echo "run: wt config shell init fish | source"'` classified as worktrunk's and was removed. One question replaces both: does the line invoke the init command, with the name in command position? The exec-keyword scan is gone rather than kept alongside; it read English words out of the whole line and rejected nothing the positional rule doesn't. `wt config shell install` had the same defect one file over and one step worse: `is_worktrunk_managed_content` tested the whole file for `config shell init` and `| source`, and `cleanup_legacy_fish_conf_d` deleted a user's own `conf.d/wt.fish` outright — during an install, which never offered to remove anything. The cmd-agnostic twin beside it already answered correctly, so the substring version is gone. Uninstall now prints every line it takes, in the confirmation and again after removal, since shell quoting stays undecidable without running the line. Sweeping for the same shape found it in `az` failure classification, where `stderr.contains("login")` alone meant "not authenticated" and `bail!` discarded the real error; `az` states the remedy itself, so its text now reaches the user. Provider host matching moved from `contains("github")` to label-boundary comparison, so `dev.azure.com.attacker.example` and `mygithubmirror.com` no longer resolve to Azure DevOps and GitHub. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-24 19:25:31 -07:00
Maximilian Roos	b17a1364f1	docs(step): render wt step rebase & wt step push on the docs site (#3578) ... Follow-up from #3568, which named these two as the one plain oversight in its audit of `after_long_help` bodies that never reach a docs page. `wt step rebase` and `wt step push` were the only two of twelve step operations with no `<!-- subdoc: -->` marker, so their help was terminal-only, and the only two bullets in `## Operations` left unlinked. Both markers are added between `squash` and `diff`, per the ordering invariant in `src/cli/step.rs`. Both openers restated their `about` line, which reads as immediate self-repetition on the web where `combine_command_docs()` concatenates the two. Both bodies are rewritten. ## What reviewing the text against the code turned up The old rebase body said "Conflicts abort immediately; use `git rebase --abort` to recover", which is self-contradictory and wrong. Nothing aborts: the worktree is left mid-rebase with git's conflict markers, and git's output names `--continue`, `--skip`, and `--abort`. `wt merge`'s pipeline step 3 carried the same sentence. Four review passes then falsified nine more claims, every one reproduced against a scratch repo before editing: - **`--no-ff` runs no `git push` at all.** It builds the merge with `commit-tree` + `update-ref` and syncs the worktree with `read-tree`; a `-vv` trace shows zero `git push` invocations. The opener attributed the whole command to `git push` with `--no-ff` as an example four lines below. Its worktree sync is also best-effort, so the ref can move while the worktree stays behind. - **The Outcomes table was falsified by the upstream-swap topology.** With local `main` diverged and behind `origin/main`, `wt step rebase main` prints `Already up to date with origin/main` — `main` is not an ancestor of the branch, and the result names a ref never typed. - **"Nothing here reaches a remote" was false on a fresh clone.** With no target argument and no cached `worktrunk.default-branch`, `default_branch()` falls through to `git ls-remote`. The claim is now "no commits leave the repository", which holds unconditionally. - **The generated Arguments table contradicted the prose.** `[TARGET] Target branch` sat two paragraphs below "the target is any commit-ish"; a blind reader nearly concluded rebase needs a branch name. - **The table implied mutually exclusive rows.** `is_rebased_onto` is checked first, and a branch at the target's tip satisfies both of the first two. - Plus: an orphan branch matched no row; "git's own output names the ways out" is false under `advice.mergeConflict false`; "refused rather than forced" raised the force question without closing it. Adjacent, in files this change already touched: `wt merge`'s step 3 contradicted the new table and step 5 restated `wt step push`'s rule with no link, so half the pipeline deferred and half repeated. Step 2 promised a backup ref unconditionally, but `create_safety_backup` runs only when working-tree changes were staged — a clean-tree squash rewrites commits and writes no `refs/wt-backup/` ref. That is a data-safety claim, so it is corrected rather than left. `docs/CLAUDE.md`'s subdoc "Use cases" cited `wt config create`, which has no marker and no section. ## The code changes **An annotated-tag target was never peeled.** Adding a tag example to the rebase help is what made it worth running, and it did not work. `is_rebased_onto` compared `git merge-base`, which peels an annotated tag to its commit, against a bare `rev-parse`, which returns the tag object's SHA. Those never match, so an annotated-tag target always looked like it needed a rebase: ``` annotated v1.2.0 → "outcome": "rebased" (HEAD unchanged) lightweight v1.2.0-lw → "outcome": "up_to_date" (same graph, same commit) ``` Fixed at the root by peeling with `^{commit}`, rather than documenting the quirk. The test pairs the two tag types on one commit so the control is a single factor away; it fails without the fix. The reviewer caught that this test had gone missing, and it was right about the consequence — without it, removing the `^{commit}` peel passes CI. The cause was a merge resolution on this branch: `checkout --theirs` on `tests/integration_tests/merge.rs` took main's whole file, dropping `test_step_rebase_annotated_tag_is_peeled` along with the duplicate squash test it was meant to drop. Restored, and re-verified in both directions. **`wt step push` ran out of a half-finished operation.** Mid-rebase, the detached HEAD is a linear extension of the target, so the fast-forward check passed and `wt step push main` printed `✓ Pushed to main (1 commit)` — moving the target branch onto a half-replayed history while the worktree kept its conflict markers and the rebase stayed open. It now runs the `ensure_no_operation_in_progress` gate `wt step rebase` and `wt merge` have used since 84cbb0489. #3579 landed while this was open and closed the same class for the staging commands, with a better predicate for them: `git add -A` collapses an unmerged path's stages, so the index is what knows, and an index read also catches a conflicted `git stash pop` that writes no state file. `wt step push` was scoped out of that, correctly — it stages nothing, so an index read cannot speak for it. Its hazard is HEAD itself, which is what the operation gate answers. The merge takes main's `squash.rs` and its `test_step_squash_refuses_mid_merge` wholesale and drops this branch's version of both; the gate's docstring now hands the unresolved-conflict question to `WorkingTree::ensure_no_unmerged_paths` instead of claiming it, and the rebase help enumerates all four gated commands. **A target worktree registered after its directory is gone got two different answers.** The fast-forward died inside receive-pack — `fatal: exec 'update-index': cd to '…' failed`, `! [remote rejected] HEAD -> main (Up-to-date check failed)` — with the ref untouched, while `--no-ff` moved the ref with plumbing of its own and skipped the sync, so it succeeded over the same broken registration. Both refuse now with the branch named and `git worktree prune` as the remedy, which is what retires the two `.exists()` checks that papered over the state downstream. `--no-ff` succeeding here is the one behavior this takes away; a stale registration is worth one error rather than half a success. ## Found and not fixed here **Ignored files in a target worktree are silently overwritten, and `git worktree lock` does not stop it.** `git status --porcelain` omits ignored files, so the overlap check cannot see them and the stash does not take them. Reproduced against the FAQ's own example (`docs/content/faq.md:180` recommends the lock for "precious ignored data"): a locked target worktree holding `db.sqlite` had it replaced by the branch's tracked file, exit 0, no warning. The lock scopes to removal only. A pathspec-limited probe of the push range detects it without enumerating large ignored trees — `git -C <target-wt> ls-files -o --ignored --exclude-standard -z -- <push_files>` names the colliding file and stays silent about a 200-file ignored `target/` — but whether a collision should refuse is a policy call, so it is left out. ## Verification `wt hook pre-merge --yes` on the merged tree → exit 0, 4554 tests, no pending snapshots. `zola build` clean, both anchors resolving and merge.md's cross-page link landing. Every factual claim checked by running `wt` against a purpose-built repo, and each of the three code fixes has a test that fails without it. > _This was written by Claude Code on behalf of Maximilian Roos_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-24 19:16:20 -07:00
Maximilian Roos	0ad5cc8ce5	fix(shell): rewrite rc files atomically on uninstall (#3585) ... `wt config shell uninstall` ended its rc rewrite with `fs::write`, which truncates in place. A crash, a full disk, or a lost power cable between the truncate and the write left `~/.bashrc`, `~/.zshrc`, or a PowerShell profile empty or half-written, taking every line the user ever added to it. That is the silent loss CLAUDE.md's Data Safety section rules out. The new content now goes to a temp file created beside the target and renamed over it, the same shape `write_approvals_file` already uses. The rc file ends up either entirely the old version or entirely the new one. Two details a bare `NamedTempFile::new()` plus `persist()` gets wrong: - **Symlinks.** Dotfile managers (Mackup, chezmoi, stow) symlink `~/.zshrc` into a repo or a synced folder. `fs::write` followed the link and updated the real file; a rename would replace the link itself with a regular file and silently detach the user's setup. The target is resolved first, so the link survives. - **Mode.** A fresh temp file is `0600`, an rc file is typically `0644`. The target's mode is copied onto the temp file before the rename. Ownership is not preserved: the replacement belongs to the running user, so a root-run uninstall against another user's rc file would change its owner. Restoring it needs a `chown`, which no current dependency exposes. ## Why only uninstall The other user-file writes in this module do not have the same exposure: - `configure_shell_file` appends to an existing rc with `OpenOptions::append`. A crash there can leave a partial integration line, but it never touches what was already in the file. Converting it to a full-file rewrite would introduce the truncate window this PR is removing. - That function's create branch only runs when the file does not exist. - `configure_wrapper_file` and the fish completion write replace files whose entire content worktrunk generates, and `wt config shell install` regenerates them. Uninstall is the only place worktrunk truncates a file whose other contents belong to the user. ## Behavior change Creating the temp file needs write access to the target's directory, which a truncating write did not. Uninstall now fails where a read-only directory holds a writable rc file, leaving the rc file as it was. ## Testing Three unit tests, Unix-gated since mode and symlink semantics differ on Windows: mode preserved with no temp file left behind, the rewrite following a symlink into a different directory, and the rc file intact when the write fails. Each was checked against the alternative it is meant to catch. The failure test fails under the old `fs::write`; the mode and symlink tests fail under a naive `NamedTempFile::new()` plus `persist()`. I also ran the real CLI against a fake home whose `.bashrc` is a 0644 file behind a symlink, and confirmed the link, the mode, and the surrounding lines all survived. Their shared helper and expected-output const are `#[cfg(unix)]` for the same reason, which the first push got wrong: used only behind `#[cfg(unix)]`, they read as live to the local gate and as dead code to `test (windows)` under `-D warnings`. `tests/CLAUDE.md` now records that blind spot, since the gate runs one platform and structurally cannot catch this class. ## Same bug class elsewhere, not fixed here Two writes outside this module read a user-authored file, modify it, and write it back with a truncating `fs::write`: - `src/commands/config/update.rs:114` rewrites the user's `wt.toml` or user config with migrated content. - `src/commands/config/plugins.rs:144` merges `statusLine` into the user's Claude `settings.json`. Fixing those wants a prior decision on whether every site, `write_approvals_file` included, should go through one shared writer. That is worth its own review rather than a rider on this one. > _This was written by Claude Code on behalf of max_	2026-07-24 18:43:29 -07:00
Maximilian Roos	b6b74574a8	fix(merge): refuse unresolved conflicts in merge's own name (#3587) ... Follow-up to #3579, which added the index gate that keeps conflict markers out of a commit. The gate works on `wt merge`; it just doesn't refuse in merge's name. ```console $ git stash pop CONFLICT (content): Merge conflict in f $ wt merge --yes ✗ Cannot squash: 1 path with unresolved conflicts f $ wt merge --yes --no-squash ✗ Cannot commit: 1 path with unresolved conflicts f ``` `wt merge` stages through two different steps, so it borrowed whichever noun it happened to delegate to. Its sibling gate on the same command says `Cannot merge`, so one command named itself three ways depending on flags. Threading a caller-supplied noun into `handle_squash` and `CommitOptions` would fix the wording, and it's the wrong shape: `wt merge` already runs `ensure_no_operation_in_progress("merge")` at its entry, in its own name. Each command gates for itself; the index gate is the one that hadn't been hoisted yet. So it moves up next to its sibling: ```rust repo.ensure_no_operation_in_progress("merge")?; current_wt.ensure_no_unmerged_paths("merge")?; ``` The inner guards stay where they are — `wt step commit` and `wt step squash` are entry points too, and they keep answering in their own names. Refusing at the entry also puts it ahead of the approval prompts and the hook plan, rather than partway through the flow. `--no-commit` changes which error it hits. It used to reach the dirty-tree check first: ```console $ wt merge --yes --no-commit ✗ Cannot merge with --no-commit: feature has uncommitted changes UU f ↳ Commit or stash changes first ``` Committing is the one thing that must not happen here, and stashing a conflicted index isn't something git will do either. ## Verification Every entry point, against a real repo stopped on a conflicted `git stash pop`, with HEAD unmoved in each: | | before | after | |---|---|---| | `wt merge --yes` | `Cannot squash` | `Cannot merge` | | `wt merge --yes --no-squash` | `Cannot commit` | `Cannot merge` | | `wt merge --yes --no-commit` | `Cannot merge with --no-commit: … uncommitted changes` | `Cannot merge` | | `wt step squash --yes` | `Cannot squash` | unchanged | | `wt step commit --yes` | `Cannot commit` | unchanged | `test_merge_refuses_unmerged_paths` pins the first row. It's driven from the conflicted-stash-pop helper, which writes no state file, so it can only pass through the index read — and it asserts HEAD is unmoved, not just the message. Full gate green (4548/4548, lints, fmt, doc sync) on the rebased head, plus `cargo clippy --all-targets --features shell-integration-tests`. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-07-24 18:41:35 -07:00
Maximilian Roos	6c1fc91a12	refactor(statusline): share the worktree lookup and collection plan (#3582) ... The two statusline surfaces — `run_json` (`--format=json`) and `git_status_segments` (the rendered line) — each carried their own copy of "find my worktree, decide if it's home, build the item", and their own literal `CollectOptions`, kept in sync by a comment reading "Same plan as the other statusline path". Both now come from one place: `statusline_options()` holds the shared plan (all columns, full gates, CI on, summary off, untracked counted in the working diff), and `current_worktree_item()` holds the lookup, returning `Option<ListItem>` so it reports "no worktree here" rather than deciding what to print — the callers render that case differently (an empty JSON result vs. a bare branch segment). Behavior-preserving. Verified by building the pre- and post-change binaries side by side and diffing their output across four directories (a dirty worktree, the primary worktree, `$HOME`, `/tmp`) × default/`--format=json`, plus the `--format=claude-code` stdin path: byte-identical output and matching exit codes in every case. One ordering change is not a pure textual move and is where review attention is best spent: `git_status_segments`'s "no default branch → just show the branch" early-out now runs after `primary_worktree()` instead of before it. Both resolve through the same cached `default_branch()`, so there's no extra work and no new path to the wire. Two of the moved lines were the branch-name fallbacks at the end of `git_status_segments`, both uncovered on main and now counted as patch lines. Rather than leave them, the second one is gone — `format_statusline_segments` pushes the branch segment unconditionally, so the `segments.is_empty()` fallback below it was unreachable — and the first, "no default branch to compare against", gets the test it never had: a repo with two branches, neither named main/master/develop/trunk and no remote, leaves `default_branch()` nothing to infer from, and the statusline is the branch name alone. Otherwise no new tests: the refactor runs under existing coverage (107 statusline tests: 57 unit, 50 integration), with the A/B above as the end-to-end evidence, since the tests alone only pin what each surface does today and wouldn't catch the two plans silently diverging. > _This was written by Claude Code on behalf of max_	2026-07-24 17:51:49 -07:00
Maximilian Roos	a5ca8a169f	refactor(perf): build every wt-perf fixture branch through one primitive (#3584) ... Follow-up to #3548, which consolidated the `wt-perf` prune fixtures but left the branch *builders* diverged. ## The refactor Three generators had each grown their own way to create a fixture branch. `add_history_spread_branches` pointed a ref at an old commit, `add_diverged_backdrop` hand-rolled ~45 lines of index plumbing, and `create_mixed_repo_at` used `checkout -b` plus a return-to-main for two of its four states. They are the same operation. A branch's state is entirely the pair **(fork point, own-commit count)**: zero commits at an old fork is "behind" and at the tip is "identical"; a positive count from the tip is "ahead" and from anywhere else is two-sided "diverged". `add_branch_with_commits` takes that pair, and the mixed fixture's four-state rotation collapses to a table: ```rust let (fork, commits) = match i % 4 { 0 => (checkpoint, 0), // behind 1 => (base_tip.as_str(), 1 + i % 3), // ahead 2 => (checkpoint, 1 + i % 3), // diverged _ => (base_tip.as_str(), 0), // identical to the tip }; ``` The checkout path is what actually mattered. `git checkout` of an old fork point on rust-lang/rust rewrites the whole tree (minutes per branch), which is why `add_diverged_backdrop` grew plumbing in the first place. Routing the mixed fixture through the same primitive removes the last checkout from fixture construction, so the main worktree is never touched and the fixture is no longer structurally barred from a rust-scale repo. The win is structural. Fixture build time is unchanged: `mixed-8-8` runs 3.25/3.28/3.71s after against 3.33/3.55/3.91s before, and `mixed-24-120` 14.9/15.7s against 15.5/23.5s. That is parity within noise, and the before-side spread swamps the difference. ## The guards These generators had nothing pinning their output. The benchmarks measure one wall-clock number, so a generator change that collapsed (say) "diverged" into "ahead" would keep every bench green while silently measuring a different repo. Three tests now assert the shapes directly, via `merge-base --is-ancestor` exit codes and porcelain status: - the `full` fixture's documented `index % 4` rotation of branch and worktree states - the same fixture's zero-dimension contract (`mixed-W-0` / `mixed-0-B`), which holds only because `0..0` never enters the loop that divides by `branches` - `add_diverged_backdrop`'s own wiring: every member unintegrated, fork depths fanning across history, worktrees dirty only via untracked scratch, and populations that don't follow each other Each was mutation-tested rather than trusted for being green. A clamped divisor gives `left: 1, right: 0`, and a fork spread collapsed onto the tip gives `fork depths must fan out across history: [0, 0, 0, 0]`. ## One thing to look at Writing the guards turned up a docstring overclaim in two places. `history_spread_shas` samples newest-first, so index 0 is the default branch's own tip. That makes `add_diverged_backdrop`'s "the default branch has advanced past every fork" and `add_history_spread_branches`'s "behind-only" both false as written. The first is true in its only caller, where `add_squash_merged` advances the default branch on the very next line. I corrected the docstrings rather than the sampling, and pinned the behavior with `assert_eq!(depths[0], 0, "the newest sample is main's own tip")`. Changing the sampling would shift the workload of every prune and `list` benchmark that uses the spread, moving Criterion's historical series with no rename to signal it, in exchange for one branch out of 24 to 120 gaining fork depth. Happy to flip it if you'd rather; it is documented and asserted either way. ## Verification `hook pre-merge` green (4542 tests), and `cargo bench --bench list full` runs end to end against the rebuilt fixture. Shape verified at benchmark scale rather than only at the test's `N = 8`: a real `mixed-24-120` holds the rotation through `br-0118`. The refactor is content-identical for the backdrop (same blob content, filenames, commit messages, plumbing sequence) and structurally identical for the mixed fixture (same fork points and commit counts, since `0..=(i%3)` and `1 + i%3` agree). > _This was written by Claude Code on behalf of max_	2026-07-24 17:46:22 -07:00
Maximilian Roos	a0c1b662d2	perf(fsmonitor): resolve all daemons in one lsof spawn (#3581) ... ## Why `wt remove`'s end-of-command fsmonitor sweep forked one `lsof` per *machine-wide* `git fsmonitor--daemon` — and with `core.fsmonitor` enabled globally, a machine accumulates one daemon per repo ever touched (100+ is routine). So every `wt remove` paid ~100 process spawns. The cost is superlinear under load, not linear: a fork storm makes macOS Gatekeeper/XProtect assess each new image under contention, inflating per-spawn cost for everything else on the box, sweeps included. The `~50ms each` figure in the old docstring was measured *under that load* and read as a fixed per-call cost — so the sweep looked merely slow rather than self-amplifying. Two concurrent test suites driving hundreds of `wt remove` calls pinned all 18 cores (load average 142) with nothing hot in `htop`. This was diagnosed and measured before (#2814 added the sweep; #3401 wrote the cost into the docstring and explicitly changed no behavior) but never fixed. ## The fix Resolve every daemon in a single `lsof -a -p <pid1,pid2,…> -U -F pn` call and split its output on the `p<pid>` record boundary, handing each PID exactly the output the per-PID call produced — so classification and the whole data-safety contract in the module docs are unchanged. Verified end-to-end against a live 108-daemon machine with a PATH shim counting invocations: **108 spawns → 1**. Two details the batched path must preserve, each covered by a new test: - **Don't gate on `lsof`'s exit status.** It exits non-zero when *any* requested PID vanished mid-scan, while still printing full records for every survivor. Gating would drop the whole sweep whenever one daemon happened to exit — the common case on a busy machine. - **A PID with no record is dropped**, never synthesized as a socket-less entry. A socket-less entry reads as orphan class 1 and gets SIGTERM/SIGKILL — on a possibly-recycled PID. ## Also here - **Test consolidation.** Three integration fixtures built their branch set by spawning git per loop iteration. A new `TestRepo::create_branches` creates them in one `git update-ref --stdin`. The over-threshold completion test drops **10.5s → 1.4s** (~230 git spawns → 5); `switch_picker` and `statusline` branch loops get the same treatment. `mock-stub` gains an opt-in `MOCK_CALL_LOG_DIR` call log so a test can assert *spawn count*, not just response — deliberately outside the repo under test, since a log written inside the tree would dirty it mid-`wt merge`. - **Docs.** Dropped the "disable `core.fsmonitor` globally" workaround from the troubleshooting docs (canonical `skills/`, mirror regenerated). It stays enabled by choice, and the daemons were never the actual fseventsd CPU driver. - **CI hardening.** pre-commit.ci's `typos` hook maps the token `pn`→`on` and had auto-rewritten `lsof -F pn` to the broken `lsof -F on` (offset+name — no `p<pid>` records, so the parser reaps nothing). Pinned `pn` in `.typos.toml` next to the existing `PN`-from-`PNGs` entry that documents the same mapping, and added a test covering the empty-PID-set early return (flagged by `codecov/patch`). ## Testing New unit tests cover `daemons_from_batched_lsof` against real captured `lsof -F pn` output, a vanished-PID gap, and empty output; the rewritten integration test asserts exactly one `lsof` spawn with a comma-joined PID list. Full integration suite green (1960 passed), clippy and fmt clean. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-07-24 17:27:36 -07:00
Maximilian Roos	bb43011c8d	fix(picker): cancel background commands when the picker exits (#3560) ... ## What `shell_exec` gains `cancel_background_commands()`: it latches a flag that stops background `Cmd::run` / `Cmd::pipe_into` before they spawn, and SIGTERMs the PIDs of those already running (tracked in a registry each capture-mode command joins for its lifetime). The picker calls it once, after skim releases the terminal and after the stashed-warning drain. Preview diffs pick up `--no-optional-locks`, matching what `wt list`'s `git status` already does: a preview reads a worktree the user may be working in, and can now be signalled mid-run. Cancellation covers both accept and abort, and only ever targets background threads — the foreground thread is the one cancelling, and goes on to run the switch itself. ## Why Accepting the picker abandoned its preview work rather than stopping it. `wt`'s exit ends the pool's threads but not the `git` children they spawned, so those children kept running, orphaned, computing diffs into an in-memory cache that no longer existed — churning disk on a repo the user had already left. Measured on a 5-worktree fixture whose `git diff` sleeps 15s, with a shell (not `wt`) as the PTY session leader, since `wt` as session leader SIGHUPs its own children on exit and hides the effect entirely: | | children alive at t+0 / +1 / +3 / +6s after `wt` exits | | --- | --- | | without cancellation | 12 / 12 / 12 / 12 | | with cancellation | 0 / 0 / 0 / 0 | Single-variable control: same build, same fixture, cancellation gated off. Accept latency is unchanged (0.13s). ## Why a latch, not just a sweep A one-shot sweep over live PIDs isn't enough on its own. A task that has already cleared its caller's supersede check and parked on `CMD_SEMAPHORE` holds no PID for the sweep to find, and spawns the moment a permit frees — the very permits the sweep just freed by signalling everything holding one. Capping concurrency at 2 to force a queue reproduces it: sweep-only still leaves survivors, the latch reaches 0. For the same reason a freshly spawned child re-reads the flag as it registers, so a child spawned into the sweep's window escapes neither. This also replaced a first attempt that bumped the preview orchestrator's spawn generation. That stops *queued* tasks, but those tasks had already passed the generation check before parking on the semaphore, so it measured no better than the sweep alone. One mechanism in `shell_exec` covers all three states — queued, parked, running — so the orchestrator is untouched. ## Notes - **`alt-x` removals are exempt.** A removal is dispatched to its own thread so the picker stays live, so pressing Enter straight after can leave a `git worktree remove` in flight when the sweep fires. Its *result* is discardable; its *effects* are not, and a signal between the worktree remove and the branch delete would strand the user half-removed. `shell_exec::uninterruptible` exempts a thread from both halves — the sweep skips its PIDs, and the latch doesn't refuse the calls it makes afterwards, since a removal is several git calls rather than one. The picker routes every removal dispatch through one `spawn_removal` helper that applies the exemption, so it's carried by the spawn path itself rather than remembered at each call site. - **SIGTERM, not SIGKILL**, so git's lockfile handlers run rather than stranding an `index.lock` in a worktree the user is about to work in. - **Cancel is ordered after `drain_stashed_warnings`.** A `--prs` forge call killed mid-flight fails like any other and stashes that failure as a warning; cancelling first would print a spurious "couldn't fetch PRs" to the user. - **`Cmd::run`'s plain branch now spawns explicitly** instead of calling `cmd.output()`, whose stdio defaults it reproduces exactly — `output()` hands back only the finished result, never the running child. - **Windows** gets the latch but not the signal; a command already running there still finishes. Bounding those would take a job object. - **Tests.** The cancelled paths (both refusals and the mid-flight signal) are covered by `tests/cancel_background.rs`, deliberately its own test binary: cancellation latches process-wide state and signals every background PID in the process, so beside other tests it takes them out — `TestRepo`'s helpers drive git through the same `Cmd` path, and this is a real failure, not a theoretical one. Alone in its process it has nothing to collide with, and the test is deterministic (a file marker for readiness, not a sleep). An earlier revision of this description claimed `codecov/patch` contradicted its own line data. That was wrong — a mis-read of the API on my part, and the check was right. Local `cargo llvm-cov` had been failing to build `skim` (E0554), so I'd taken the API as a substitute for measuring. The build failure was a stale local tool: cargo-llvm-cov ≥ 0.7.0 instruments only workspace crates instead of the whole dependency graph, so on CI's pinned 0.8.7 `skim` never sees the `cfg(coverage)` that trips its nightly feature gate. With that upgraded, local coverage reproduces codecov's four missed lines exactly. Three were in `test_background_command_registers_while_running`, now removed — its poll loop's body only ran when the spawned thread hadn't registered yet. Coverage is the lesser reason: the test polled a process-global registry for non-emptiness, so under the shared-process runner the coverage job uses it could be satisfied by another test's PID and pass without its own command ever registering. `tests/cancel_background.rs` pins that property correctly, and more strongly — the sweep reaches a running command only if it registered. Three lines are left uncovered, knowingly. One is the spawn-race re-check, which by construction only fires on a race that can't be scheduled deterministically — covering it would need a scheduling hook in library code, which the repo's "no test code in library code" rule rules out. The other two are pre-existing: the `spawn_removal` refactor re-indents the dispatch closures, which codecov counts as new patch lines, pulling in the morph-failure backstop (the `warn` and `revert_morph` calls). That backstop only fires when a file appears in the TOCTOU window between the removal's safety check and the rename, so it can't be deterministically tested either. If `codecov/patch` reads red, this is why — the structural exemption was judged worth more than the metric. - **PID reuse.** Between the kernel reaping a child and the guard's `Drop` removing its PID, a freed PID is briefly still listed. Inherent to signalling by PID — the reap and the deregistration aren't one operation — and deregistering before the wait isn't a fix but a removal, since the wait *is* the command's lifetime. Documented on `BackgroundPid` rather than mitigated. > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-24 16:36:25 -07:00
Maximilian Roos	f5d99d4fe5	fix(step): refuse to commit unresolved conflicts; one gutter symbol for every operation (#3579) ... Two follow-ups from #3558, which stopped `wt merge` and `wt step rebase` from running out of a half-finished operation. Probing the sibling commands for the same root cause found one that was worse, and the symbol work is what that PR's detection change made possible. ## `wt step squash` committed conflict markers Mid-merge, invoked directly, it didn't refuse. It generated a commit message for the unresolved markers and committed them: ```console $ git merge side CONFLICT (content): Merge conflict in f $ wt step squash --yes ◎ Generating commit message and committing changes... (1 file, +6, no squashing needed) Merge branch 'side' This resolves a merge conflict between main and side branches. ✓ Committed changes @ e6262c6 $ git show HEAD:f <<<<<<< HEAD main ||||||| 56e6eb3 base ======= side >>>>>>> side ``` `MERGE_HEAD` is gone and the working tree is clean, so the broken merge reads as complete. `wt merge` was never exposed — its own gate stops it before it reaches squash — so this was the direct-invocation path only. ## The index is what knows The obvious fix is the gate #3558 added, and it is not sufficient. The hazard is narrower than "an operation is open", and also wider. `git add -A` collapses an unmerged path's three index stages into one entry. That resolves the conflict as far as the index is concerned, while `<<<<<<<` is still on disk — and it takes with it git's own refusal to commit an unmerged index, which is what would otherwise have stopped this. So the exposure is exactly the commands that stage on the user's behalf, and it does not need an operation to be open at all: ```console $ git stash pop CONFLICT (content): Merge conflict in f $ ls .git/MERGE_HEAD ls: .git/MERGE_HEAD: No such file or directory $ wt step commit --yes ✓ Committed changes @ 1f5387c # markers and all ``` A conflicted `git stash pop` leaves unmerged paths with no state file written, so no reading of `.git/` can see it. `WorkingTree::ensure_no_unmerged_paths` reads the index instead — `git diff --diff-filter=U`, the same question `git commit` asks — and both staging paths call it before staging: ```console $ wt step commit ✗ Cannot commit: 1 path with unresolved conflicts ┃ f ``` The paths are worth carrying where the operation refusal carried nothing: they are the one thing the user needs and git isn't being asked. `wt step squash` additionally takes the operation gate, ahead of its branch check, so mid-rebase it names the open rebase rather than blaming the detached HEAD and offering `git switch <branch>` — the one command that throws the rebase away, which is the same wrong remedy #3558 removed from `wt merge`. Both guards run before the pre-commit hooks and the LLM call. Neither is worth running for a commit that can't happen, and a refused commit runs no project commands — checked with a `pre-commit = "touch HOOK_RAN"` project config that never fires. `wt step commit --dry-run` is deliberately not gated. It mutates nothing, and guarding it displaced `test_step_commit_dry_run_propagates_git_add_failure`, which pins a distinct error path; a tested behavior is worth more than cosmetic parity. ## One symbol for every operation The Status column had `⤴` for rebase and `⤵` for merge, and nothing for the other three states git can leave open. A worktree stopped mid-cherry-pick, mid-revert, or mid-bisect rendered as idle — including the case #3558 closed, a multi-commit cherry-pick whose stop was resolved with `git commit`, which leaves a clean tree, no `CHERRY_PICK_HEAD`, and only the queued sequencer to say anything is wrong. Three more glyphs was the obvious fix and the wrong one. What the reader does about any of the five is identical: run `git status`, then finish or abort it. Splitting the column across a glyph per operation asked them to distinguish states that lead to the same next step, and the split is what left the other three invisible. So they collapse to one: ```console $ wt list Branch Status … Message @ main ↻^ … resolved by hand ``` `git status` names which operation it is, in git's own words — the same division of labor as the refusal message. `--format=json` keeps the identity the symbol drops, so a consumer that needs it still has it: `operation_state` gains `cherry_pick`, `revert`, and `bisect` alongside `rebase` and `merge`. That retires `ActiveGitOperation`, which existed only to re-encode `InProgressOperation` down to the two states the gutter knew. `WorktreeData.git_operation` is now `Option<Option<InProgressOperation>>` — outer `None` is "not loaded" — matching its neighbour `has_working_tree_conflicts`. `GitOperationTask` also stops swallowing errors: it reports a failed probe through the same `ctx.error` channel every other task uses, rather than reporting "no operation" for a probe that didn't answer. ## Verification Three integration tests, each asserting HEAD is unmoved rather than only matching the message: - `test_step_squash_refuses_mid_merge` — the case that committed markers. - `test_step_commit_refuses_unmerged_paths` — driven from a conflicted `git stash pop`, so it can only pass through the index read; the operation check cannot see that state. - `test_list_shows_symbol_for_bisect` — bisect because it is the only operation that leaves HEAD on the branch and the tree clean, so nothing else in the Status cell stands in for it. Snapshots pin both the symbol and `"operation_state": "bisect"`. Confirmed by hand against real repos: mid-merge, mid-rebase, and the conflicted-stash-pop state all refuse; a mid-merge commit whose conflicts *are* resolved still succeeds, as does a clean squash; a queued cherry-pick and a stopped revert both render `↻` and name themselves in JSON. Full gate green (4500/4500 tests, lints, fmt, doc sync), plus `cargo test --features shell-integration-tests` (2148/2148) and `cargo clippy --all-targets --features shell-integration-tests`, which the gate doesn't compile. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-07-24 16:33:49 -07:00
Maximilian Roos	a07cf60be9	docs(claude): stop coverage from vetoing a better design (#3580) ... ## The failure this fixes A session working on the picker found a structural improvement: routing both removal dispatches through a `spawn_removal` helper, so the uninterruptible-cancellation exemption lives in one place instead of being remembered at each spawn site. It declined to make the change, because the helper re-indents the morph closure's body and pulls two pre-existing uncovered lines (a failure-path warn and a `revert_morph` call) into the patch. Projected `codecov/patch`: ~97.3% against an `auto` target of ~97.8%. Red. The improvement was shelved. The section it was reading said "NEVER merge a PR with failing `codecov/patch` without explicit user approval," then "on failure, investigate and fix the gap," then "if you believe it's a false positive, ask." Every route treats a red patch as a defect in the change. The cheapest way to satisfy all three is to not make the change. ## What changed The Coverage section now separates the two decisions it was conflating. The check gates the merge; it never gates the design. A predicted red patch is not a reason to shelve or water down an improvement, and neither the code nor the tests get contorted to move the number. The merge rule is unchanged: still never merge red without approval. What is new is the route through it when the gap isn't real (misses that predate the change, or a path with no deterministic trigger): push the change and ask, handing over which lines, why they can't be covered, and what the patch percentage comes to. Two smaller corrections ride along: - "Remove unused code" to green the number now excludes a rarely-reached backstop that's load-bearing. The picker lines in question guard a TOCTOU window during rename-based removal, and the repo's Data Safety rules would rather keep them. - `tests/CLAUDE.md`'s note on rename false positives now covers the mechanism that caused this. Codecov counts any line the diff relocates as a patch line, re-indentation included, not just a `git mv`. The `spawn_removal` refactor itself is not in this PR: it touches picker code mid-flight on #3560. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-07-24 16:18:37 -07:00
Maximilian Roos	edcfe5fe0c	fix: honour -C in the statusline and in every supplied path (#3567) ... `-C` sets the repository discovery path but never chdir's the process, so anything reading `std::env::current_dir()` silently ignores it. Two places did. ## `wt list statusline` described the wrong worktree `wt -C /path/to/other/worktree list statusline` printed the statusline for the *cwd's* worktree, and from a directory outside any repo it printed nothing at all. Both formats now resolve their worktree through `Repository::current_worktree()`, which anchors to the discovery path. That accessor already exists for this bug class (its docstring cites #2625), so this is routing the statusline onto the canonical path rather than adding a new one. Details worth knowing while reading the diff: - `--format=json` was mostly right already. It picked its worktree via `current_worktree()` and only its "are we in a worktree" gate read the cwd, so it misbehaved just when the process cwd sat outside a repo (printed `[]`). That gate moves to `current_worktree()` and the `cwd` binding is gone. - `git_status_segments` now takes a `WorkingTree` instead of `(&Repository, &Path)`. That also settles a split inside it: the worktree lookup used the cwd while the no-worktree fallback used `repo.current_worktree()`. - `--format=claude-code` is unchanged where it matters. The directory Claude Code sends on stdin still wins, since that names the session's directory rather than wt's. With no stdin it falls back to the same worktree as the other formats, so it honours `-C` too. Four integration tests. Three run from the main worktree with `-C` pointed at a `feature` worktree, one per format; the existing statusline tests all set the child's cwd, which is the route that already worked. The fourth pins the claude-code split: when stdin names a directory in a *different* repository, the worktree comes from stdin while `list_worktrees` comes from wt's discovery path, so nothing matches and only the branch is reported. That fallback previously had no test, which is what `codecov/patch` caught on the first push. Discovering the `Repository` from the stdin directory instead would remove that split, but it would miss the `prewarm` cache (keyed on the base path) that this prompt-redraw hot path depends on, so the split stays and is now asserted rather than incidental. ## Supplied paths resolved from the shell, not from `-C` `wt switch ../repo.feature` and `wt remove ../repo.feature` joined the argument onto the process cwd. With `-C` in play that split the invocation in two — wt chose the repository, the shell chose the path — and the combination resolved to nothing, so `switch` reported "No branch named ../repo.feature". `--config`, `WORKTRUNK_CONFIG_PATH`, `WORKTRUNK_SYSTEM_CONFIG_PATH`, and the trace file of `wt config state logs profile` name a file the same way and had the same gap: `wt -C repo --config mycfg.toml config show` reported "Config file not found" for a file sitting in `repo/`. Git resolves the paths in its command line against its own `-C`, because it chdir's there before doing anything else. wt's `-C` sets a discovery path instead, so `resolve_input_path` joins onto that — one resolution point for all five, so they cannot drift apart. It returns the path untouched when `-C` was not given, rather than joining `.` onto it: the process cwd already resolves those, and the join would surface as a stray `./` in output. `Path::join` leaves an absolute path alone, which drops an `is_absolute` branch and, in `resolve.rs`, a fallible `current_dir()` call. Three neighbours are deliberately outside the rule, and the docstring says why: - Paths wt derives itself — a worktree root from `git worktree list`, an XDG config directory — arrive absolute. - `WORKTRUNK_PROJECT_CONFIG_PATH` resolves from the worktree root, its documented base, because the project config is per-worktree. - `WORKTRUNK_BIN` and `XDG_CONFIG_DIRS` are never opened by wt: the first is expanded later by the generated shell wrapper, and the XDG spec already requires the second to be absolute. Four integration tests, one per surface, each running from *outside* the repo with `-C` pointed into it — the neighbouring tests run from a cwd inside the repo, which is the route that works under either rule. All four fail against the old code: `switch` with "No branch named ../repo.feature-detached", `remove` by leaving the worktree standing, and the two config paths by not finding their file. The `set_base_path` docstring picks up the rule, so the next reader has one place to find it: `-C` is the base for the two things git takes from its chdir — the repository discovered, and every path the user names — while the process cwd stays the answer to where the user's shell physically is. ## Noticed, not touched `run_json` and `git_status_segments` duplicate the worktree-lookup-and-populate sequence (`list_worktrees` → find by root → `is_home` → `build_worktree_item` → `populate_item` with identical `CollectOptions`). Worth collapsing separately. > _This was written by Claude Code on behalf of max_	2026-07-24 15:57:23 -07:00
Maximilian Roos	f9cd38eb88	fix(shell): validate command names and tighten install/uninstall detection (#2864) ... Two shell-integration correctness fixes plus an uninstall-scope redesign. **Command-name validation.** The shell-integration command name comes from the user-typed `--cmd` flag, or `argv[0]` when `--cmd` is omitted. A malformed value (an empty string, a leading `-`, or characters like `;` or whitespace) renders into a broken shell rc line that the user then has to find and fix. `validate_shell_command_name()` rejects empty, leading-`-`, and non-`[A-Za-z0-9._-]` names at the CLI entry points, so a bad value produces a clear error up front instead of a silently broken config file. This is input hygiene on a value the user already controls, not a security boundary. **Noncanonical-line detection on install.** Install recognized only its exact current canonical line, so a manually-added or older-form integration line (e.g. `eval "$(wt config shell init zsh)"`) was not seen as already-present and a duplicate was appended on the next `install`. It now detects those forms and reports already-configured. **Uninstall scans every worktrunk-managed file, regardless of cmd.** Previously `uninstall` located Fish/Nushell wrapper files by name (`{cmd}.fish` / `{cmd}.nu`), so removing integration installed under an alternate binary name needed a matching `--cmd`. That symmetry leaked an implementation detail: uninstall already has a content marker to recognize worktrunk wrappers regardless of binary name. It now scans the wrapper directories (`~/.config/fish/functions`, `conf.d`; the nushell candidates' `vendor/autoload`) and admits every file whose content matches the cmd-agnostic marker `# worktrunk shell integration for <shell>` (always emitted by the wrapper templates). A headerless file (the legacy `conf.d/{cmd}.fish`, which held nothing but the init line) qualifies only when every non-blank, non-comment line is an integration line, so a user's own file that runs or merely mentions `wt config shell init` amid other code survives the scan. Fish completion files carry their own `# worktrunk completions for` header and are scanned the same way, so a completion whose wrapper is already gone is still cleaned up. For Bash/Zsh/PowerShell (line-based), `is_shell_integration_line_for_uninstall_any_cmd` reads the binary name off the line rather than pattern-matching it. `config shell init` is a fixed literal, so the name is the run of command-name characters ending at it, held to the same `validate_shell_command_name` rule that governs what worktrunk writes into shell code. The execution-context check that pairs with it is a line-level property, so it lifts out of the per-position loop it used to sit in, which is what let both cmd-agnostic detectors drop their regexes. Since the result asks only whether a line is worktrunk's, not which install owns it, uninstall also removes hand-written `git wt config shell init` lines, and it cleans every matching profile file rather than the first (both PowerShell profiles on Windows). All detection reads only the code portion of a line: a trailing `#` comment can mention an integration line without running it, so uninstall does not delete an unrelated `eval`/`source` line over a comment, and install does not treat such a mention as already-configured. The cmd-specific permissive detector lost its last caller in this cutover and is deleted. The `--cmd` flag is removed from `wt config shell uninstall`; `install` keeps it (installing under an alternate name is still a deliberate per-cmd act). The FAQ's "What can Worktrunk delete?" inventory documents the widened uninstall surface. Covered by new integration tests: custom-cmd install + cmd-less uninstall round-trips for zsh/fish/nushell (verifying scan-all removes the custom-cmd integration, a hand-written `git wt` line, a stale second wrapper, and an orphaned completion), a PowerShell uninstall round-trip on the old pre-`Out-String` line, malformed-name rejection at the CLI edge (`--cmd` and `argv[0]`), and the older-line dedup case. Unit tests pin the deletion criterion: worktrunk's own wrappers and the headerless legacy init file match; a file that mentions the integration, or runs it amid other code, does not. > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Worktrunk Bot <w@worktrunk.dev> Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-24 09:32:17 -07:00
Maximilian Roos	11a1498fa1	docs(list): render wt list statusline's help on the docs site (#3568) ... `wt list statusline`'s `after_long_help` documents the three output formats, the Claude Code stdin JSON contract, and the pace segment, but `wt list`'s help had no `<!-- subdoc: statusline -->` placeholder, so none of it reached `docs/content/list.md` or the skill reference. It was terminal-only. Adding the placeholder pulls it in as a `wt list statusline` section under `# Subcommands`, matching how `wt step` and `wt config` expose theirs. ## What reading it on the web surfaced The help text needed edits once it rendered as a page rather than a block below an Options list. A context-blind agent was given the two rendered pages and asked to answer setup questions from them alone; three of its snags were real. **The format lists were stale and used private names.** They read `branch status ±working commits upstream ci url`, while the Columns table higher up the same page calls those `HEAD±`, `main↕`, `Remote⇅`. They also omitted `main…±`, which `format_statusline_segments` has emitted since that column became a default. The lists now use the column names and include it, and `claude-code` is stated as a delta on `table` rather than repeating it. **The formats read as a fixed layout.** The example line on the Claude Code page has nine segments against an eleven-name format string, with no explanation. Three rules were in the code and in no doc: empty cells are omitted, `claude-code` drops `branch` when `dir` already ends in `.<branch>` (`filter_redundant_branch`), and an overlong line drops whole cells worst-priority-first (`fit_to_width`). All three are now stated. **The latency caveat lived only on the Claude Code page.** A reader of the command's own docs had no way to learn it reaches the network. It moves to the reference. That exposed a contradiction with the definition, "Single-line status for shell prompts", against a caveat saying it is too slow for a synchronous prompt — so the definition becomes "Single-line status for the current worktree". This is the one user-visible string change here; it lands in `wt list --help` and the three help snapshots. ## Deduplication with the Claude Code page The pace paragraph and the OSC 8 paragraph were near-verbatim on both pages, and would have rendered twice on the site. `claude-code.md` keeps what is Claude Code-side (install, demo, the example line, and a plain note that the links degrade to unclickable text where the terminal lacks support) and links to the reference for the rest. ## Follow-ups folded in The module docstring at `src/commands/statusline.rs:4` carried `±working commits upstream`, the last occurrence in the tree of the labels this branch retired, and opened "Statusline output for shell prompts" — the framing the definition dropped. Both now match the help text. ## Not done An audit of every `after_long_help` in `src/cli/` found 46 that never reach a docs page. Most are editorial calls rather than oversights: `wt config create` alone would embed ~450 lines of example TOML into a page that already covers that ground by hand. The one that looks like a plain oversight is `wt step rebase` / `wt step push`, the only two of twelve step operations without a marker, and the only two the `## Operations` list leaves unlinked. Covering them needs their openers rewritten first, since both currently restate their definition. Left for a follow-up. Verified with `wt hook pre-merge --yes` (4499 tests) and a `zola build`, which checks internal anchors. > _This was written by Claude Code on behalf of Maximilian Roos_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-24 07:45:25 -07:00
Maximilian Roos	84cbb0489e	fix(step): refuse to rebase or merge from a worktree mid-operation (#3558) ... ## The bug With a rebase in progress (a conflicted stop, or a killed `git`), `wt step rebase main` reported success over a conflicted tree: ```console $ git rebase main # stops on a conflict; HEAD detached mid-replay $ wt step rebase main ○ Already up to date with main $ echo $? 0 ``` `handle_rebase` asked `is_rebased_onto` first. Mid-rebase, HEAD sits on a linear extension of the target, so that returns true and the function returns `UpToDate` before anything consults `worktree_state`. `wt merge` from the same state failed differently but no better: ```console ✗ Cannot merge: not on a branch (detached HEAD) ↳ To switch to a branch, run git switch <branch> ``` The hint names the one command that throws the rebase away instead of finishing it. ## The fix Both commit-replaying commands now check for an open operation before doing anything else. `wt merge` runs the check ahead of its branch check, so the open operation is what gets named rather than the detached HEAD it causes. ```console $ wt step rebase main ✗ Cannot rebase: a git operation is already in progress ``` The gate covers every state detection reports, not just rebase. Measured on the others beforehand: a rebase started mid-bisect ran and stacked a second operation on the first, and one started mid-merge or mid-cherry-pick died inside git's autostash (`Cannot save the current index state`, `f: needs merge`). None told the user what to do next. ## Why it prescribes no remedy An earlier draft of this branch carried a per-operation table of `--continue`/`--abort` hints. It was wrong on first contact with the long tail: `git am` borrows the `rebase-apply` directory, so an open am session was reported as a rebase and pointed at `git rebase --continue`, which git refuses outright: ```console $ git rebase --continue fatal: It looks like 'git am' is in progress. Cannot rebase. ``` Adding an `Am` arm would have treated the enumeration as the fixable part. `git status` already answers this better than the table did, in git's own words and translations, for states worktrunk has no variant for: ```console $ git status interactive rebase in progress; onto b162044 You are currently rebasing branch 'feat' on 'b162044'. (fix conflicts and then run "git rebase --continue") (use "git rebase --skip" to skip this patch) (use "git rebase --abort" to check out the original branch) ``` It names the operation, the branch, the progress, and every way out including `--skip`, which the table never offered. So the table is gone rather than extended, and the refusal is a single line: it says what is blocked and why, and leaves the rest to the command every git user reaches for next. Detection still returns a structured `InProgressOperation` rather than a display string, but now purely as detection: the only caller that reads a variant is the one classifying a rebase worktrunk itself just started. The neighbouring `RebaseConflict` error took the same treatment. It forwards git's stderr in the gutter when it has it, and otherwise substituted two hint lines of its own: ```console ↳ To continue after resolving conflicts, run git rebase --continue ↳ To abort, run git rebase --abort ``` That is the operation table again in miniature — one operation's remedies, minus `--skip` — and the one path that reaches it is the safety check for a rebase git *reported as finished*, where worktrunk knows least about what is actually open. It now prints its title, plus git's own words when git said any. ## One detector Two places probed the same files for the same answer: `Repository::worktree_state` for the gate, and `WorkingTree::is_rebasing`/`is_merging` for the `wt list` gutter symbols. They are now one method, `WorkingTree::operation_in_progress`, which `Repository::operation_in_progress` delegates to — the rename also retires a name that collided with `wt list`'s unrelated `WorktreeState`. Detection now runs in the order `git status` consults the state files, which is a change from the old `wt list` path: it checked rebase before merge. Nothing can reach the difference — no rebase backend writes `MERGE_HEAD`, checked against all three — but matching git's order is one less thing to reason about. `wt list` maps the operations without a gutter symbol (cherry-pick, revert, bisect) to `None`, so its output is unchanged. ## The queued sequencer, now closed Having one detector is what made the last gap worth closing. `git cherry-pick A B` that stops on A writes `CHERRY_PICK_HEAD`; resolving that stop with `git commit` instead of `git cherry-pick --continue` removes the file and leaves B queued: ```console $ git status On branch main Cherry-pick currently in progress. (run "git cherry-pick --continue" to continue) $ ls .git/CHERRY_PICK_HEAD ls: .git/CHERRY_PICK_HEAD: No such file or directory $ cat .git/sequencer/todo pick 1f07533 side1 pick 6b32540 side2 ``` Git reads that same file to keep calling the sequence in progress, so detection reads it too: the first instruction word, `pick` or `revert`, exactly as git's own `sequencer_get_last_command` does. The reason this waited for its own change is the risk of refusing a `wt merge` the user can't un-refuse, so the lifecycle is measured rather than assumed. The directory is gone once the sequence finishes, and gone after `--quit` or `--abort`; a single-commit cherry-pick never creates one; a rebase keeps its todo elsewhere, under `rebase-merge/`. An empty or unrecognized todo reports nothing, so the failure direction is the old behavior rather than a refusal with no way out. ## Verification The repro above, plus the merge, am, cherry-pick, revert, and bisect states, checked by hand against real repos: all five refuse, including am, which is now correct precisely because nothing here claims to know what am needs. Three integration tests drive the real binary from a genuinely stopped operation (`test_step_rebase_refuses_mid_rebase`, `test_merge_refuses_mid_rebase`, `test_merge_refuses_mid_merge`). `test_merge_refuses_mid_merge` is what holds the broadened scope: a conflicted merge keeps HEAD on the branch, so it is the case the detached-HEAD check waves through. Narrowing the gate back to rebase-only turns it red, which I verified by making that edit and re-running. The sequencer case is driven end to end from a real stopped pick — `test_operation_in_progress_reads_the_queued_sequencer` commits the resolution by hand, asserts `CHERRY_PICK_HEAD` is gone, and still expects a cherry-pick, so it can only pass through the sequencer read. It then runs `git cherry-pick --quit` and expects the state to clear, which is the false-refusal half. Running the same sequence against the built binary refuses where it previously reported `○ Already up to date with main`. Confirmed unchanged: already-up-to-date, fast-forward, true-rebase, and rebase-conflict output; a plain detached HEAD still gets the `git switch` hint, which is the right remedy when no operation is open; a sibling worktree mid-rebase does not block a clean one; `wt list`'s gutter symbols. Full gate green (4494/4494 tests, lints, fmt), plus `cargo clippy --all-targets --features shell-integration-tests`, which the gate doesn't compile. ## Merged with main `main` gained #3539, which extracted the failed-rebase classification into `classify_rebase_failure`, and #3549, which adds `span_upstream` — both inside `handle_rebase`. The conflict was semantic as well as textual: #3539's `is_rebasing` check still read the old string API (`worktree_state()?.is_some_and(|s| s.starts_with("REBASING"))`), which this branch removes, so resolving the markers alone left a type error. The resolution keeps #3539's classification and reads the structured enum instead: ```rust let state = repo.operation_in_progress()?; let is_rebasing = matches!(state, Some(InProgressOperation::Rebase)); return Err(classify_rebase_failure(e, is_rebasing, &integration_target)); ``` All five of #3539's classification tests pass alongside the gate. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-24 07:39:28 -07:00
Maximilian Roos	166e1190ba	refactor(perf): canonicalize wt-perf prune-fixture layering + completion config (#3548) ... ## What Three behavior-preserving canonicalizations of the `wt-perf` benchmark fixtures, from a design pass on "why does prune need its own generator, and can the fixtures consolidate?" 1. **Share the prune-population layer.** `create_prune_repo_at` (synthetic) and `create_prune_real_repo_at` (rust-scale) both layered the same two populations onto their base repo — `add_diverged_backdrop` (the scanned-but-never-removed backdrop) + `add_squash_merged` (the removable candidates). Extracted into `add_prune_populations`, so the base repo (synthetic `create_repo_at` vs. rust clone) is now the *only* thing the two fixtures differ in, spelled out once instead of twice. 2. **Canonicalize `completion.rs` onto `RepoConfig::branches`.** Its two inline `RepoConfig` literals were exactly `RepoConfig::branches(50, 0)` (one with `worktrees` overridden), so they now use the canonical preset via struct-update instead of respelling all seven fields. 3. **Document the fixture → bench mapping.** `benches/CLAUDE.md` listed the `wt-perf setup` handles in one section and the bench groups in another with nothing connecting them, so a bench and its fixture (`full` and `mixed`) read as overlapping names for the same thing. Added a mapping table and the rule behind it. ## Why this is the whole safe consolidation The design question was whether prune could drop its bespoke generator for a "standard one with different params" — ideally one diverse repo serving prune *and* the other benches. The answer is that prune's generator is **already** a thin composition (`create_repo_at` + two additive layers), and it can't fold into the shared `mixed`/`full` fixture without breaking three prune-specific invariants: a known/cleanly-separated candidate count (the destructive `live` path re-creates exactly the consumed candidates), untracked-only dirt (the `git reset --mixed` index-heal depends on it — `mixed`'s staged-dirt states violate it), and a provably-unintegrated backdrop. And benchmark fixtures optimize for *attribution*, not realism: the uniform single-axis fixtures (`worktree_scaling`, etc.) exist precisely because the diverse `full` fixture can't localize a regression, so diversifying them would lose that. On the `full`/`mixed` naming specifically: they can't be canonicalized to one name, because fixtures and benches are **many-to-one** — `RepoConfig::typical` alone backs five bench groups (`skeleton`, `worktree_scaling`, `first_output`, `picker_preview`, `remove_e2e`), two of which sit in the same file measuring different things. `full`/`mixed` only looks 1:1 by coincidence. Renaming also isn't free: `.github/scripts/criterion-to-jsonl.py` keys each time-series row by the criterion output path (`<group>/<id>`) for the daily gist append, so a rename orphans that series. Documented rather than renamed. ## Proposed follow-up (not in this PR) The one remaining real duplication is two *implementations* of "two-sided-diverged population forked across history depth": `add_diverged_backdrop` (plumbing-based, scales to rust) vs. the diverged branch states inside `create_mixed_repo` (checkout-based, synthetic-only). They can't naively merge — the checkout path would make `prune-real` take minutes per branch — but a shared plumbing primitive could back both. That rewrites a carefully-tuned fixture feeding the `full` bench with **no shape-assertion test to catch drift**, so I'd land a fixture-shape test first (count refs by state + assert integration status). Happy to do it if wanted. ## Verification fmt `--check` clean; `clippy -p wt-perf --all-targets` and `clippy --bench completion` clean; `pre-commit` clean on the doc; `cargo test -p wt-perf` passes (incl. the prune-fixture tests `squash_merged_fixture_is_content_integrated` / `prune_fixture_state_classifies_lifecycle`, which exercise `create_prune_repo_at` → `add_prune_populations`); all benches build. Table contents verified against the code by grep. > _This was written by Claude Code on behalf of Maximilian Roos_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-23 18:40:00 -07:00
Maximilian Roos	f3e5c82604	feat(statusline): dim the dev-server URL until its port answers (#3561) ... Follow-ups to #3550. The dev-server URL in the statusline dims until something answers on its port, matching the `wt list` cell it was already copying in every other respect. ``` ~/w/worktrunk.feature ↕|🤖 ↑2 ↓1 #3550 :17913 server up ~/w/worktrunk ⊂| ↑2 ↓1 #3550 :12107 server down (dim) ``` `url_active` was already collected on this path (the health check runs under the statusline's full-column plan), so the dim consumes something already paid for rather than adding a probe. ## The other half, and why it isn't here The follow-up list also carried "detect OSC 8 support and render accordingly", which is how `wt list` decides its URL cell: linked `:3000` when the terminal supports hyperlinks, the URL in full when it doesn't. I built that, and three review passes took it apart. Recording the path, because the conclusion is the interesting part: 1. The first version probed `stderr`, since a shell prompt captures stdout through command substitution, and forced links on for `--format=claude-code`. An adversarial pass showed the exception reintroduced the bug it was meant to fix: Claude Code re-emits OSC 8 to the terminal rather than drawing it, so Claude Code inside Apple Terminal got the link, and the URL collapsed to a bare `:3000` with its host inside an escape the terminal discarded. 2. Reading the environment alone fixed that and deleted the exception. Then a bug sweep measured what the plain-text fallback actually costs: unlinked, the cell grows from 5 columns to the whole URL, and the URL is the worst-priority segment, so `fit_to_width` drops it first. Against a long dev hostname the linked line kept the URL down to 39 columns and the unlinked one lost it below 82. At 80 columns in tmux the "fallback" showed nothing where the old code showed a clickable port. 3. Separating the two decisions fixed that (collapse to `:port` always, link only when the terminal renders it) but left the probe controlling nothing but escape bytes. At which point an outer-loop pass asked whether it should exist, and it shouldn't: - The [OSC 8 spec](https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda) guarantees a terminal implementing OSC parsing per ECMA-48 "is guaranteed not to suffer from compatibility issues … the target URI is silently ignored and the supposed-to-be-visible text is displayed, without artifacts." The named buggy set is VTE up to 0.48 (2018), Windows Terminal up to 0.9, Emacs term-mode, and screen with 700+ character URLs. The statusline has emitted unconditional OSC 8 to shell prompts since #199 in January, on that same reasoning. - `supports-hyperlinks` is an allowlist, so its dominant error is the false negative. xterm, foot, mintty, rio, contour and a configured tmux all render OSC 8 and none are named. The probe's certain cost is withdrawing a working link from those users, against a speculative and shrinking risk. - The escape-hatch argument I had for it was inert. Once the probe no longer changed any text, `FORCE_HYPERLINK=0` could not reproduce what the old hardcoded Claude Code gate did, which was print the URL in full and copyable during that regression window. So the answer to "shouldn't we detect?" is that `wt list` detects because there the answer changes what the reader gets: `estimate_url_width` reserves a column wide enough for the full URL, so an unlinked cell can print it. The statusline reserves nothing, so it has no second rendering to switch to. `format_url_cell` now says that, in place of a stale note claiming the statusline's stdout was a pipe "to its editor". ## Also here - `isolate_subprocess_env` scrubs `FORCE_HYPERLINK`. It stands alone: `wt list` probes with `on(stream)`, which is `(FORCE_HYPERLINK set || tty) && allowlist`, and tests pin `TERM=alacritty`, so a developer with `FORCE_HYPERLINK=1` exported flips the `wt list` table snapshots from full URL to linked `:port`. - The `table` and `claude-code` format lists in `wt list statusline --help` name the `url` segment, which they emit and never listed. - A `--format=claude-code` test pins where the URL segment lands among the directory, CI and model segments. ## Verification Full gate green (`cargo run -- hook pre-merge --yes`, 4486 tests). Beyond the suite, the built binary rendering a live and a dead port, and the drop threshold measured at six widths to confirm the segment behaves as before. ## Known, not addressed A `[list] url` with no port is permanently dim on both surfaces: `UrlStatusTask` only connects when a port parses, so `url_active` stays `None`, and the `== Some(true)` test reads "never checked" as "nothing listening". Pre-existing in `render.rs`, which this change doesn't touch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-23 18:38:10 -07:00
Maximilian Roos	9ecd7945e0	fix(merge): measure squash/rebase against the target's upstream, not a stale local ref (#3549) ... Fixes #3519. ## The bug `wt merge` resolves its target to the **local** default-branch ref and squashes/rebases the span `merge-base(local-target, HEAD)..HEAD`. When the primary checkout's local `main` is behind `origin/main` **and** the branch descends from the newer `origin/main` tip (e.g. created with `--base origin/main`), that merge-base falls back at the stale local tip, so the span sweeps in every commit already on `origin/main`. The squash folds them into a single commit and the local fast-forward lands it — corrupting the primary checkout's `main` and, if that ref is later pushed, duplicating already-upstream content under new SHAs. Reproduced deterministically before fixing: a 1-commit feature off `origin/main`, with local `main` two commits behind, squashed "3 commits" into a new SHA on local `main` — matching the report. The same mis-measured span reaches every rewrite entry point: a standalone `wt step squash` folds the upstream commits into the *branch* (the reporter's 1-commit PR becoming 13), and `wt step rebase` replays them onto the stale ref when it fires. ## The fix: measure against the upstream; let the merge carry One local-only predicate (no fetch — worktrunk is local-first), `Repository::span_upstream`: the target's upstream, returned exactly when the branch's history extends past the local target ref into it — i.e. `merge-base(HEAD, upstream)` is not an ancestor of the local target. The rewrite pipeline responds by *measuring* there instead of guessing from the stale ref: - **`wt step squash`** (and the squash inside `wt merge`) computes its span, commit count, message, and `reset --soft` base against the upstream — only the branch's own commits fold, and the target ref is never touched. The `--dry-run`/`--show-prompt` previews use the same base. - **`wt step rebase`** (and the rebase inside `wt merge`) rebases *onto* the upstream, so a non-linear branch replays only its own commits instead of duplicating upstream ones onto the stale ref. In the plain stale topology it no-ops, as before. - **`wt merge`** announces the situation up front, then completes: the fast-forward carries local `main` through the already-fetched upstream commits by their real SHAs to the result — the same graph a fresh local `main` would have produced: ``` ○ Local main is 2 commits behind origin/main; the merge fast-forwards through them ◎ Squashing 2 commits into a single commit (2 files, +2)... ✓ Squashed @ 3f2a91c ✓ Merged to main (3 commits, 3 files, +3) ``` The one refusal left is forced by topology rather than chosen by policy: a target that has **diverged** from its upstream (its own commits *and* behind, with the branch based past it) can never fast-forward in any mode — reconciling the target's own commits is the user's call, so the merge stops before any rewrite or approval prompt: ``` ✗ Local main has diverged from origin/main — can't fast-forward to a branch based on origin/main ↳ Reconcile main with origin/main (rebase or merge its local commits), or specify a different target ``` This is the reporter's suggested direction ("use `origin/<default>` as the merge/rebase target"), minus the network: the already-fetched remote-tracking ref is the measurement base, and `wt merge` still never touches the wire. It also extends the rule the rest of worktrunk already applies — `IntegrationTargets` / the preview panes' comparison base treat the upstream as the truth when the local default lags it; the merge pipeline was the one mutating surface that didn't. Quiet on everything that must keep working: a legitimately-diverged local target with the branch forked from the shared base (the supported offline workflow — existing regression test unchanged), targets with no upstream, orphan branches, and explicit non-branch targets like `wt step squash origin/main`. ## Tests - `test_merge_carries_stale_local_main_through_upstream` — the #3519 topology end-to-end: merge succeeds, the carry is announced, the squash folds only the branch's own commits and sits directly on the `origin/main` tip, and `origin/main` is an ancestor of the merged `main` (real SHAs, no duplication). - `test_step_squash_measures_against_upstream_when_local_main_stale` — the standalone squash folds only the branch's own commits and never moves the target ref (this scenario previously corrupted `main` through a squash-then-merge sequence). - `test_step_rebase_targets_upstream_when_local_main_stale` — a branch that merged `origin/main` in rebases onto the upstream: linearized, real upstream SHAs retained, target ref untouched. - `test_merge_refuses_diverged_target_when_branch_based_on_upstream` — the forced refusal: nothing mutated, branch survives. - `test_merge_removes_branch_when_local_main_diverged_from_upstream` (legitimate divergence, branch from shared base) still passes — no regression. Docs: the merge help page (primary source in `src/cli/mod.rs`) describes the upstream-aware measurement, the carry, and the diverged refusal; generated mirrors and help snapshots regenerated. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-23 16:34:16 -07:00
Maximilian Roos	14d4d92532	feat(statusline): make the CI and URL segments clickable in Claude Code (#3550) ... The statusline suppressed OSC 8 hyperlinks in Claude Code mode, so its CI segment was colored but not clickable and its dev-server URL printed in full. Claude Code renders OSC 8, so both segments now link the way they already do in `wt list`. ``` ~/w/worktrunk.statusline-osc8-hyperlinks ↕|🤖 ↑2 ↓1 ^+93 -24 #3550 :17913 Opus 4.8 └link┘ └link┘ ``` ## Why it was off The gate dated from 2026-01-02, inside Claude Code's OSC 8 regression window — links worked through 2.0.76, broke around 2.1.3, and got a partial IDE-terminal-only fix in 2.1.42 ([anthropics/claude-code#26356](https://github.com/anthropics/claude-code/issues/26356)). The premise was correct when written and has since expired; that issue was auto-closed for inactivity rather than on a fix, so the tracker understates current support. ## Verification PTY-captured the raw byte stream from Claude Code 2.1.218, with `TERM_PROGRAM`/`VSCODE_*` stripped so it exercises the standalone-terminal path that was reported broken. Claude Code doesn't strip or blindly pass through — it parses OSC 8 into its frame model, assigns a hyperlink id, and re-emits canonically (normalizing an ST terminator to BEL): ``` PRE ESC]8;id=1l7bqdh;https://example.com/ST-PROBE BEL STLINKTEXT ESC]8;; BEL MID … ``` It holds in both the normal and alt-screen (`CLAUDE_CODE_NO_FLICKER=1`) render paths, with `FORCE_HYPERLINK` unset. Driving the real `wt` binary through Claude Code end to end yields both links live: ``` LINK: https://github.com/max-sixty/worktrunk/pull/3550 LINK: http://127.0.0.1:17913 ``` Degradation is graceful: a terminal or multiplexer that drops OSC 8 shows the same text, just not clickable (tmux only gained OSC 8 in 3.4; zellij and Alacritty support it). ## Shape of the change With links unconditional for the statusline, the plumbed `include_links` flag had one value, so it collapses into the segment builder. `format_url_cell` likewise takes the link decision from its caller rather than probing the terminal, matching `PrStatus::format_cell` — the statusline's stdout is a pipe, so `supports_hyperlinks` reports false there even though the consumer renders OSC 8. That left `hyperlink_stdout` with no callers, so it goes. `format_cell` keeps its `include_link` parameter: `wt list` passes the terminal probe, and the picker passes `false` because a `--prs` row never reaches the strip path. `format_url_cell` moved next to `estimate_url_width`, which budgets the column against it — the two have to agree on when a cell collapses to `:port` and were in separate files. The URL segment also gets shorter: the URL rides inside the escape sequence, so `http://127.0.0.1:17913` becomes `:17913`, returning 16 columns on a line that budgets by width. ## Safety of the truncation interaction `truncate_visible` ends its cut with `\e[0m`, which resets colour but leaves an OSC 8 link *open* — a severed link would make the rest of the terminal line clickable, and `ansi_cut` really will sever one if reached. It can't be reached, because the two cuts never meet: `fit_to_width` drops whole segments worst-priority-first and stops at one, so character truncation only ever lands on a best-priority survivor — Directory (0), Branch or Model (1) — none of which carry escapes beyond SGR. Every link-bearing segment is strictly worse (CI 5, URL 9), so each is dropped entire first. Reviewing the branch turned up that the numbered comments in `format_statusline_segments` had drifted from `COLUMN_SPECS` — CI was labelled 9 (it is 5) and the URL 8 (it is 9), with branch-diff and upstream also off — and the first version of the test had taken those stale numbers as its specification. The comments are corrected and the test now rests on the invariant above, which doesn't depend on where CI sits. It sweeps widths 1–90 over both links, asserts it spans every drop stage, and pins that the URL goes before CI. A second test pins that the hidden URL costs no visible width (`ansi_strip` drops OSC 8 for both terminators), so priority budgeting isn't inflated. ## Docs The Claude Code statusline page now says the segments are clickable — it's the feature's own page and said nothing about it. The `wt list --help` JSON field description gains the links but stays short: an earlier, longer wording shrank the help table's Field column and wrapped two dozen unrelated rows. ## One judgement call worth flagging `format_statusline_segments` also feeds plain `wt list statusline` (shell prompts) and the JSON `statusline` field. The CI link was already unconditional on both before this change; what's new is that the URL cell renders as a linked `:3000` rather than the full URL, so a consumer that strips OSC 8 sees only `:3000`. The structured `url` / `dev_server.url` field still carries the full URL, and `:3000` still answers "which port", so this reads as the right trade — but it is the one place the collapse to a constant reaches a renderer that isn't Claude Code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-23 14:20:01 -07:00
Maximilian Roos	a7c4a7a120	refactor(wt-perf): place fixtures under target/wt-perf, not the cache dir (#3547) ... ## What Move every `wt-perf` on-disk fixture out of the per-user cache dir and under the cargo target dir at `<target>/wt-perf/`, resolved by a renamed `wt_perf_fixture_dir()`: - `setup <config>` fixtures → `<target>/wt-perf/<config>` (was `~/.cache/wt-perf/<config>`). - The rust-lang/rust clone and `prune-real` fixtures → `<target>/wt-perf/bench-repos/` (was `~/.cache/wt-perf/bench-repos/`). - `wt_perf_cache_dir()` → `wt_perf_fixture_dir()`. The target dir is derived from the **running executable's own path** (it lives inside whichever dir cargo built into — `<target>/debug/wt-perf`, `<target>/release/deps/<bench>`), so it honors `CARGO_TARGET_DIR`, a config-file `build.target-dir`, and cargo-llvm-cov's `target/llvm-cov-target/` — none of which a bare `CARGO_TARGET_DIR` env read covers. Falls back to `<workspace>/target` if the binary isn't under a recognizable profile dir. The `etcetera` dependency is dropped. - The `WT_PERF_CACHE_DIR` env override (and the empty-value guard it required) is removed; the benchmarks workflow now caches the deterministic `target/wt-perf/bench-repos` path directly. - In-process throwaway fixtures (`create_repo`) are unchanged — they keep using `tempfile::TempDir`. This reverses the location decision from #3542, which had moved these into `~/.cache/wt-perf`. ## Why `target/` is the conventional home for build/test-generated artifacts — gitignored, reaped by `cargo clean`, and already where criterion writes (`target/criterion`). Keeping every wt-perf fixture there gives one predictable location under the repo that `cargo clean` fully resets. Deriving the target dir from the running binary (rather than assuming `<workspace>/target`) keeps that property intact even when the target dir is relocated. ## Tradeoff (accepted) `target/` is **not** shared across git worktrees (worktrees don't share it) and is wiped by `cargo clean`. So the ~15 GiB `prune-real` rust clone re-clones per worktree and after every `cargo clean` — the cost #3542 avoided by using the cache dir. This is deliberate and documented on `wt_perf_fixture_dir`; it's cheap for the synthetic `setup` fixtures, which rebuild in seconds. ## Testing - `cargo build -p wt-perf --all-targets`, `cargo test -p wt-perf` — clean. - New unit test `target_dir_from_exe_finds_cargo_target` covers the resolution logic (relocated `CARGO_TARGET_DIR`, bench binary under `release/deps/`, cargo-llvm-cov's nested target, closest-profile-wins, and the outside-any-target fallback). - `pre-commit run --all-files` (fmt, clippy, yaml, typos, cargo-lock, custom hooks) — clean. - Smoke-tested end-to-end: `setup branches-2` lands at `<worktree>/target/wt-perf/branches-2` and writes nothing to `~/.cache/`; a copy of the binary run from a fake `<dir>/debug/wt-perf` correctly resolves fixtures to `<dir>/wt-perf/`, proving a relocated target dir is honored. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-22 13:44:50 -07:00
Maximilian Roos	f14fa69c0e	feat(list): run merge analysis in a read-only object database (#3535) ... When the Git object database is read-only, `wt list` and `wt list statusline` redirect their object-writing merge/conflict probes (write-tree, commit-tree, merge-tree --write-tree) into a temporary object database with the real one as a read-only alternate, so the full analysis runs in a managed read-only sandbox. Detection is a one-time writability probe; the writable path is unchanged. Scoped to the two observational list entry points (collect and populate_item); mutating commands keep the persistent database and fail loudly on a read-only store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 11:14:20 -07:00
Worktrunk Bot	dbefb98ed4	fix(plugin): add pipefail to WorktreeCreate hook so wt failures surface (#3546) ... ## Problem The Claude plugin's `WorktreeCreate` hook (`plugins/worktrunk/hooks/hooks.json`) pipes the `wt` result straight into `jq`: ``` bash -c 'name=$(jq -er .name) || exit 1; cd "${CLAUDE_PROJECT_DIR:-.}" || exit 1; bash "$CLAUDE_PLUGIN_ROOT/hooks/wt.sh" switch --create "$name" --no-cd --format=json | jq -er .path' ``` Without `set -o pipefail`, the pipeline's exit status is `jq`'s, not `wt`'s. On jq 1.6, `jq -er .path` exits **0** on the empty stdout of a failed `wt`: ```console $ printf '' | jq -er .path; echo $? # jq 1.6 0 ``` So when `wt switch --create` dies (e.g. an existing-branch collision after the branch/worktree were already created by an earlier partial run), the hook "succeeds" while printing nothing. Claude Code then reports the misleading `WorktreeCreate hook failed: hook succeeded but returned no worktree path` instead of `wt`'s real stderr — as reported in #3545. The project's own `skills/wt-switch-create/rationale.md` already documents the `bash -c 'set -o pipefail; …'` wrapper as the intended design ("The hooks.json pipefail wrapper"), but the command in `hooks.json` never actually carried it. This PR brings the code in line with its own documentation. ## Solution Prefix the existing `bash -c` command with `set -o pipefail;`. The wrapper is already `bash -c`, so this is safe — dash rejects `set -o pipefail` fatally, which is precisely why the `bash -c` wrapper (rather than the login shell) exists per the rationale. With pipefail, the pipeline surfaces `wt`'s nonzero exit regardless of jq version. ## Testing Added a reproduction assertion to `test_plugin_layout_is_consolidated` (`tests/integration_tests/config_show.rs`) that fails when the `WorktreeCreate` hook command lacks `set -o pipefail` — it fails on the pre-fix `hooks.json` and passes after. Also verified the pipeline shape directly: ```console $ bash -c 'set -o pipefail; (echo "✗ Branch foo already exists" >&2; exit 1) | jq -er .path'; echo $? ✗ Branch foo already exists 4 # nonzero -> hook correctly fails, wt's stderr shown $ bash -c 'set -o pipefail; echo "{\"path\":\"/tmp/wt/foo\"}" | jq -er .path'; echo $? /tmp/wt/foo 0 # success path unchanged ``` The `bash -c '…'` body stays single-quoted, so `test_claude_hook_commands_parse_in_all_shells` (login-shell parse check) is unaffected — the outer shells never look inside the quotes. --- Closes #3545 — automated triage Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-22 11:07:12 -07:00