mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
v0.69.2
2291 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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> |
||
|
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>
|
||
|
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
|
||
|
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_ |
||
|
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> |
||
|
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_ |
||
|
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_ |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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_ |
||
|
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>
|
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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>
|
||
|
Maximilian Roos
|
4121dc979c |
refactor(wt-perf): cache fixtures in the platform cache dir, not /tmp or target/ (#3542)
## What Move `wt-perf`'s benchmark/debug fixture repos out of `std::env::temp_dir()` and `target/bench-repos/` into the per-user cache dir, resolved by a new `wt_perf_cache_dir()`: - `$WT_PERF_CACHE_DIR` if set, else `<cache>/wt-perf` via `etcetera::choose_base_strategy().cache_dir()` — `~/.cache/wt-perf` (or `$XDG_CACHE_HOME/wt-perf`) on Linux **and** macOS. This matches worktrunk's own base-dir convention (`src/config/user/path.rs` resolves user config through the same XDG-on-macOS strategy). - `setup <config>` fixtures → `<cache>/<config>` (was `temp_dir()/wt-perf-<config>`). `--path` still overrides. - Real cloned fixtures (`prune-real`, the rust-lang/rust clone) → `<cache>/bench-repos/` (was `target/bench-repos/`). - In-process throwaway fixtures (`create_repo`) are unchanged — they keep using `tempfile::TempDir`. ## Why Both old homes were wrong for a *reused* fixture (these are given stable names, persist between runs, and are referenced from docs and `-C <path>` — that's a cache, not a temp file): - **A fixed name in a shared `/tmp` (Linux)** collides across users: `/tmp` is sticky (`1777`), so a second user's `setup` hits `remove_dir_all().unwrap()` on a dir they don't own → `EPERM` → panic. It's also a predictable-path/TOCTOU hazard, and `systemd-tmpfiles` reaps `/tmp` per-file at 10 days by `max(atime,mtime,ctime)` — on a `noatime` mount an actively-*read* fixture still loses cold `.git/objects/pack/*.pack` mid-use → silent git corruption. - **On macOS**, `temp_dir()` is already `/var/folders/.../T` (per-user, `0700`), so the docs' `/tmp/wt-perf-*` paths were simply wrong there. - **`target/`** is per-worktree (git worktrees don't share it) and wiped by `cargo clean`, so the ~15 GiB rust clone was re-cloned per worktree — worst for the most expensive fixture, in a worktree-heavy workflow. The cache dir is per-user (no collision, no TOCTOU), stable and discoverable (docs/`-C` references still work), shared across worktrees (clone once per machine), and survives `cargo clean` — matching sccache, cargo, rustup, Go, Bazel, and Hugging Face, which all place large reusable caches there rather than in `/tmp`. ## Migration notes - First bench/setup run after this rebuilds the cache under the new location (the old `target/bench-repos/` is no longer read). Existing fixtures can be dropped with `rm -rf target/bench-repos`. - `.github/workflows/benchmarks.yaml` sets `WT_PERF_CACHE_DIR` and caches `$WT_PERF_CACHE_DIR/bench-repos`, so the cached path and the path wt-perf writes stay pinned together (no drift if a runner sets `$XDG_CACHE_HOME`); key unchanged. - Docs (`benches/CLAUDE.md`, `src/commands/CLAUDE.md`) updated to the new paths, noting that `wt-perf setup` prints the exact path. ## Testing - `cargo build --workspace --all-targets`, `cargo clippy --workspace --all-targets --features shell-integration-tests -- -D warnings` — clean. - `cargo test -p wt-perf` — passes. - Smoke-tested default resolution (`~/.cache/wt-perf/…` on macOS), `$WT_PERF_CACHE_DIR` override, and the `prune-real --path` rejection (now names the resolved cache path). > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
0aca156ccd |
fix: use @ before path in -vv startup pointer message (#3543)
The \`-vv\` startup pointer (\`announce_trace_destination\` in \`src/logging.rs\`) printed "Verbose logging to \<dir\>/", using "to" before the path. The codebase convention (see the \`writing-user-outputs\` skill) is \`@\` before paths in user-facing output — #3521 already applied this to the \`-vv\` end-of-run block but left this start-of-run pointer as the one remaining inconsistency. Changes "Verbose logging to \<dir\>/" to "Verbose logging @ \<dir\>/" and updates the matching test assertions/doc comment in \`tests/integration_tests/diagnostic.rs\`. The historical wording quoted in \`CHANGELOG.md\` is left as-is since it documents a past release. > _This was written by Claude Code on behalf of max_ |
||
|
Worktrunk Bot
|
a660d859ee |
refactor: unify subdir preservation for wt step relocate (#3346)
Route `wt step relocate`'s shell `cd` through the shared `resolve_subdir_in_target` helper, so every path-switching command (`switch`, `remove`/`merge`, `relocate`) preserves the user's subdirectory position identically — resolving the follow-up from #3344. Gate `test_relocate_preserves_subdir` to Unix: subdir preservation only fires when the cwd is inside the moving worktree, which is exactly when `git worktree move` (a directory rename) fails on the legacy Windows console — a real limitation, not a test artifact, so the test documents the boundary rather than masking it. |
||
|
Maximilian Roos
|
6a85da8f46 |
refactor(eval): render -v variables through the shared formatter (#3536)
`wt step eval -v` was the last of the four `-v` template-variable listings to hand-roll its own formatting. The hook (foreground + background) and alias sites all render through `format_hook_variables` / `format_alias_variables`; eval instead sorted keys alphabetically, padded, and joined them by hand. #3495 labelled all four block *headers* with their owner, which made eval's differently-formatted *body* stand out. This adds a sibling `format_base_variables` for the bare `base_vars` scope (no hook operation/infrastructure vars, no alias `args`) and routes eval through it, so all four listings are one family. Consequences for the eval listing, now matching the other three: - curated help-table order instead of alphabetical - deprecated aliases (`main_worktree`, `repo_root`, `worktree`, `main_worktree_path`) dropped, as the other sites already omit them - absent optional vars show `(unset)` (e.g. `owner` with no PR context) - variable names render plain, not bold Behavior-only change to the `-v` diagnostic lane; the `eval_verbose` / `eval_format_json_verbose` snapshots capture the new output. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
56249f9122 | fix: clear error when new branch name collides with existing branch namespace (#3528) | ||
|
Worktrunk Bot
|
c6eb148e62 | docs(hook): correct pre-switch {{ branch }} resolution note (#3516) | ||
|
Maximilian Roos
|
17add1f6dd |
fix: drop companion log paths from the -vv end-of-run message (#3521)
The `-vv` end-of-run block printed `trace.jsonl` and `subprocess.log` as bare gutter lines under a headline that named only `diagnostic.md` — an unannounced list whose relationship to the headline lived in a code comment, and the third and fourth prints of the same directory in one run. The start-of-run pointer already names the log directory, and the report body already routes readers to the raw companions (`subprocess_log_path` in the template; the profile heading names `trace.jsonl` as its source), so the end block now announces only the one human-facing file: ``` ○ Diagnostics and performance profile saved @ ~/…/.git/wt/logs/diagnostic.md ↳ To report a bug, open an issue (https://github.com/max-sixty/worktrunk/issues/new) and attach a secret gist: gh gist create --web ~/…/diagnostic.md ``` The headline drops "Logs," (the raw logs were precisely what wasn't at that path), and the hint now ends with the copyable gist command, linking the plain `/issues/new` URL instead of the ~150-char prefilled-body URL. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
cda5080195 | test(list): exercise real wide-char branch names in width test (#3511) | ||
|
Rene Leonhardt
|
79be81a4bb | fix(merge): Accept merge-shaped fast-forward histories (#3509) | ||
|
Maximilian Roos
|
85c9241cb1 |
Name the template owner in the -v variables block (#3495)
## The question
When `wt` shows a `template variables:` block under `-v`, does it say
which template the variables belong to? Mostly no — and in one case it
genuinely can't be worked out from the header.
## What was there
Four sites print a variables listing, and none named its owner in the
header:
| Site | Header | How you'd tell what it's for |
|---|---|---|
| foreground hook (`command_executor.rs`) | `template variables:` | the
`hook_type` row at the bottom of the table, or the `Running …` line
below it |
| background hook (`hooks.rs`) | `template variables:` | same — but see
below |
| alias (`alias.rs`) | `template variables:` | only the `Running alias
greet` line below it |
| `wt step eval` (`eval.rs`) | `Available template variables` |
unambiguous (one template, named on the command line) |
The adjacent blocks in the same `-v` lane already label themselves — `○
eval source`, `○ user:noop result` — so the variables table was the odd
one out in its own family.
Background hooks turned that from inconvenient into ambiguous. `wt -v
remove <branch>` prints one table per hook type back to back, then a
*single* combined `Running post-remove: cleanup (user); post-switch:
notify (user)` line — two identical headers over two ~20-row tables that
differ only in their rows.
## The change
Prefix each header with the hook type, alias name, or `eval`.
```
- ○ template variables: ○ post-remove template variables:
- ○ template variables: ○ post-switch template variables:
```
`wt step eval -v` now reads as one labeled family:
```
○ eval template variables:
branch = feature/auth-oauth2
○ eval source
{{ branch }}
○ eval result
feature/auth-oauth2
```
`eval`'s header was reworded from `Available template variables` for the
same shape; it was never ambiguous, but it's the fourth member of the
family and now matches the `source` / `result` headers directly beneath
it.
Display-only — no logic changed. Snapshots and the generated `step.md`
pages regenerated; `cargo run -- hook pre-merge --yes` green (4461
tests).
> _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>
|
||
|
Maximilian Roos
|
568b6de85f |
docs: consolidate duplicated explanations and trim slop (#3494)
Sweep of the docs for repetition and filler, from an audit of the hand-authored pages, the command pages' source in `src/cli/mod.rs`, and the plugin skill. Net −574 lines; every cut either had a surviving canonical home or restated an adjacent sentence. **One home per mechanism** (other mentions now link to it): - `template-append` — the LLM commits guide owns the rendering mechanism; the user- and project-config sections keep the key, an example, and what is unique to them (the project approval gate and the only-`template-append`-from-project scoping). Was explained in full in three places. - `-vv` diagnostic files — `wt config state logs` owns the four file descriptions; the FAQ summarizes in one sentence and links. - fsmonitor/trash cleanup — the FAQ's two overlapping `wt remove` bullets merge into one that distinguishes own-daemon teardown from the orphan sweep; troubleshooting.md's restatement compresses to a pointer, keeping its unique wedged-daemon-on-live-worktree guidance. - copy-ignored built-in excludes — the `wt step copy-ignored` page owns the directory list (previously enumerated 4×); the config sections state the rule and link. - hook-types table — `wt hook` owns it; the extending guide replaces its verbatim copy with a sentence. - LLM tool commands — unchanged, deliberately: the apparent hand-synced duplication between llm-commits.md and the config example is already machine-pinned by `test_llm_docs_commands_match_config_example`, so it cannot drift. **Cut-over debt**: the deprecated `wt config state ci-status` section shrinks to a deprecation pointer — its status table, fetch order, and caching notes all duplicated the `wt list` CI-status section. **Structure**: `wt step copy-ignored`'s "Features" list dissolves into the sections that owned its facts (excludes → "What gets copied", reflink → "Performance"); the four trailing "Note: This command is experimental…" lines go (the `[experimental]` badge already appears twice per section); shell-integration.md described the directive-file mechanism twice and now describes it once. **SKILL.md** drops from 339 to 181 lines: the permission-model section duplicated the config-types bullets, the hook-type mapping appeared twice, and the "Determining Which Config to Use" / "Validation Before Adding Commands" scaffolding enumerated judgments an agent makes on its own. The approvals-escalation and agent-handoff sections are untouched. **Deliberately not addressed**: `wt list`'s schema 1 JSON tables (still the default schema; the wholesale deletion lands when the default flips) and corpus-wide em-dash density (a house-style decision, not a per-page fix). Generated mirrors (docs command pages, skill references, plugins mirror, `dev/*.example.toml`, help snapshots) regenerated via `test_docs_are_in_sync` and `cargo insta`. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
7735d115ce | fix(tests): restore hidden-subcommand completion coverage (#3492) | ||
|
Maximilian Roos
|
0a1cd19cc9 |
chore: drop the NEXTEST_NO_INPUT_HANDLER workaround (#3482)
nextest#2878 (the run suspending with SIGTTOU when a test spawns an interactive shell, as the `shell-integration-tests` feature does) was fixed in nextest 0.9.118, so the `NEXTEST_NO_INPUT_HANDLER=1` workaround threaded through the insta hook, CI, and the test docs is obsolete. Verified on nextest 0.9.132: the shell-wrapper tests run to completion under a real PTY without the variable. `.config/nextest.toml` now pins `nextest-version = "0.9.118"`, so an older nextest fails with a clear version error instead of suspending mid-run. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
612580b030 |
refactor(errors): finish routing command-failure bails through CommandError (#3483)
Follow-up to #3474: converts the remaining raw-output `bail!("{stderr}")` sites to typed `CommandError`, so every failed-command display goes through the one normalizing constructor (`\r` stripping, canonical `✗ <command> failed (exit N)` header + output gutter). **Sites converted:** - `step/shared.rs` (`git ls-files` in ignored-entry listing) and `step/commit.rs` (`git add` into the temp index) — the two commands-layer git sites. - The five claude/codex plugin CLI calls in `commands/config/{plugins,codex}.rs`, collapsed into one private `run_plugin_cli` helper in `commands/config/mod.rs`. Their failure snapshots now show the canonical header + stderr gutter instead of `✗ claude plugin marketplace add failed: <stderr>`. - `execute_llm_command` in `llm.rs` — its hand-rolled stderr/stdout/exit-code fallback ladder was `from_failed_output`'s job; deleted in favor of the constructor. The `LlmCommandFailed` wrappers read the detail via `ErrorExt::display_message()`, which extracts the captured output for command failures and matches the old text in the common cases; the picker summary pane guards on the error type (`format_summary_error`) — command failures surface their captured output, everything else keeps the full anyhow chain the pane always showed (folded in from review). The shell program is recorded by basename only (same rule as `render_llm_invocation`), so a no-output failure doesn't leak the Git Bash install path on Windows. **Deliberately left:** the two `gh pr list` / `glab mr list` bails in `picker/prs.rs`. Their consumer is the skim-footer warning sink where the current one-line text is the intended display, and picker text sanitization was explicitly dropped (#3177). **Behavior note** (intended, but visible): an LLM command that emits on both streams before failing now shows both in the error block (`combined_output()` joins stderr + stdout); the old ladder showed stderr only. **Testing:** three new error-path unit tests (`list_ignored_entries` / `stage_to_temp_index` via corrupt-config and bad-pathspec, `execute_llm_command` exit-code + basename assertion); the five plugin sites are covered by the existing failing-mock integration tests whose snapshots this updates; `test_generate_summary_llm_error` covers the summary-pane command-failure arm and `format_summary_error_keeps_chain_for_non_command_errors` the chain-preserving arm. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
9ed48fb32c | fix(config): redirect misplaced nested keys to the other config with a scope note (#3473) | ||
|
Worktrunk Bot
|
3b6630be4a | fix(config): show reflects the #3461 object-store project-config fallback (#3477) | ||
|
Worktrunk Bot
|
d80cc80ebd | feat(config): flag stale default-branch cache in wt config state (#3471) (#3478) | ||
|
Maximilian Roos
|
489fd0f501 |
refactor(errors): route git-plumbing failure bails through CommandError (#3474)
Follow-up to #3459. Eight git-plumbing sites turned a captured non-zero exit into `bail!("…{stderr}")`, bypassing `CommandError::from_failed_output` — the one constructor that normalizes `\r` and gives renderers the typed command/output split. This converts them (`resolve_git_common_dir`, the bulk `config --list` read, `config --unset`, `config --get-regexp`, `merge-tree`, `merge-base`, the untracked-diff numstat, and `llm.rs::run_git_capture`), so failed-command display routes through one constructor and the `main.rs` multiline-error tripwire has nothing left to catch on these paths. `from_failed_output` now takes `&[impl AsRef<str>]` so `Vec<String>` arg lists work. One structural fix rode along: `generate_commit_message` now wraps only the `execute_llm_command` failure in `GitError::LlmCommandFailed`, mirroring `generate_squash_message` — a git failure while building the prompt is not an LLM-command failure, and flattening it into `LlmCommandFailed.error` dropped the typed stderr. Rendering: context-wrapped failures render byte-identically (the outside-repo snapshots pass untouched); bare ones gain the standard `git … failed (exit N)` header + gutter. The only snapshot changes are two `wt list` task-failure lines where merge-base errors drop the hand-rolled `git merge-base failed for <sha> <sha>:` prefix and now match their sibling lines. Every converted error path has a direct test — five new ones using empirically-probed failure modes (invalid regexp pattern → exit 6, invalid key → exit 1, corrupt config → 128 for the lazily-populated bulk read and merge-tree, unborn HEAD for the numstat), plus the updated `run_git_capture` unit test; `resolve_git_common_dir` and `merge_base` were already covered by existing snapshots. Two same-pattern sites remain in the commands layer (`step/shared.rs` ls-files, `step/commit.rs` stage-to-temp-index) — deliberately left for a follow-up along with the non-git CLIs (gh/glab, claude/codex). > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
934aad13cb |
fix(list): stop repeating the failure count in the list footer (#3467)
Follow-up to #3442. A failed task printed the same count twice on adjacent lines — the summary footer's `N tasks failed` clause and the `▲ N tasks failed:` warning header directly below it that names each failure: ``` ○ Showing 5 worktrees, 3 ahead; 6 tasks failed ▲ 6 tasks failed: (detached): working-tree conflict check fatal: bad object HEAD ``` The footer clause was never a distinct signal: its count is exactly `non_timeout_errors.len()`, and that vector being non-empty is precisely the condition under which the warning renders. So the number was duplicated in 100% of cases. Rather than suppress the duplicate when a warning happens to follow, this splits the two by what each reports. The footer counts what's *missing* from the table — timed-out tasks, which leave an empty cell and have no message to print, so a count is the only signal they can carry (and the warning deliberately filters them out). Failures are named in the warning with git's full error text. `format_summary_message` drops its `error_count` parameter entirely; the footer now reads: ``` ○ Showing 5 worktrees, 3 ahead ▲ 6 tasks failed: ... ``` Also corrects a stale claim in the `status_symbols` module spec: it said the footer "lists which tasks did not finish per item," giving a mapping from a `·` cell back to the timed-out `TaskKind`. No such mapping ever existed — the footer only carried counts, and budget-based truncation produces no diagnostic at all by design. One narrow behavior note: in progressive TTY mode the footer is stdout (a repainted table row) while the warning is stderr, so `wt list 2>/dev/null` on a TTY no longer shows a bare failure count. That case was already discarding the error detail, and discarding stderr discards narration by definition; the ordinary piped case (both on stderr) is unchanged. ## Testing The `format_summary_message` unit test now covers the timeout-only footer variants; the two failure-warning integration snapshots are updated to show the deduplicated footer. Verified end-to-end against the `index.lock` scenario (dirty worktree + pre-existing lock file). > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
bb4abca109 |
fix: anchor relative WORKTRUNK_PROJECT_CONFIG_PATH to the worktree root (#3460)
`WORKTRUNK_PROJECT_CONFIG_PATH` was returned verbatim, so a relative value resolved against the process cwd while the default `.config/wt.toml` is anchored to the worktree root. Running `wt` from a subdirectory made a relative override silently no-op (or read an unintended file). This is the config-path footgun from #3454 (the RFC there is a separate discussion). A relative override now resolves against the same worktree root as the default (current worktree, or the primary worktree at a bare root), so it behaves identically from any subdirectory. Values that can't be anchored fail loudly instead of guessing: - relative with no worktree root to anchor to (bare repo with no linked worktrees) errors - Windows forms that are neither fully absolute nor relative (drive-relative `C:cfg`, rooted-but-driveless `\cfg` or `/tmp/x`) error rather than resolving against the process drive or cwd Absolute values are unchanged (returned verbatim, no git calls), which keeps the test-isolation use of the var working. An empty value still means no project config, as before. `wt config show` (human format) and `wt config update` previously folded a `project_config_path()` error into "not in a git repository" / a silent skip; they now propagate it, matching the JSON path. Merging main brought in #3462's committed default-branch config fallback for bare repos whose primary worktree is parked off the default branch. That fallback now stands down when `WORKTRUNK_PROJECT_CONFIG_PATH` is set: an explicit override names the config source outright (a missing file there means no project config), and the override exists for test isolation, where reading the repo's own committed config is exactly the leak being prevented. The human format of `wt config show` now reports "No project config" when no location exists (bare repo with no worktrees, empty override) instead of the inaccurate "Not in a git repository", which stays reserved for actually being outside a repo. Compatibility: invoked from the worktree root, cwd and root coincide, so old and new resolution agree. From a subdirectory the old resolution found nothing, which is the bug being fixed, so no deprecation shim is included. Thanks @indexzero for reporting in #3454. 🤖 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> |
||
|
Worktrunk Bot
|
5c6fbed809 |
fix: resolve default-branch project config by name, not cwd HEAD (#3466)
Follow-up to #3462 (merged). That PR's final commit changed the object-store fallback to read `git show HEAD:.config/wt.toml`, and this reintroduces #3461 in the common agent case. ## The bug `git show` resolves `HEAD` against the **invocation cwd's per-worktree HEAD**, and the read runs with `discovery_path` (the cwd `wt` was invoked from) as its working directory. When `wt` runs from *inside a linked worktree parked on another branch* — exactly the agent-driven workflow the #3461 fix targets — `HEAD` is that worktree's branch, not the default branch. So the fallback either reads the wrong branch's config or, when the invoking worktree's branch carries no committed config, drops the default branch's config and every project hook. The merged PR's two regression tests only invoke `wt` from the **bare root**, where `HEAD` does point at the default branch, so they can't catch this. ## Verified end-to-end Bare repo; default branch `main` has `.config/wt.toml` with a `post-start` hook; `main` checked out in no worktree; a second worktree `other` whose branch dropped the config on disk: | Invoked from | `git show HEAD:.config/wt.toml` | Hook fires? | |---|---|---| | bare root | reads `main`'s config | ✅ (what the merged tests cover) | | `other` worktree | reads `other`'s HEAD → no config | ❌ **default-branch config dropped** | The absolute ref `git show main:.config/wt.toml` reads the config correctly from any cwd — which is what the pre-regression commit (and @max-sixty's original "load it in with git show" request on #3462) did. ## The fix Resolve the default branch **by name** (`<default-branch>:.config/wt.toml`), which is cwd-independent. This is a one-line change back to an absolute ref, plus a doc-comment correction. The added regression test invokes `wt switch --create` from inside the `other` worktree and asserts the default branch's hook fires. It fails against the merged `HEAD:` behavior (marker never written) and passes with the fix; the two existing #3461 tests stay green. ## Coverage note `self.default_branch()?` restores an early-`None` return when the default branch can't be determined (bare repo with no resolvable default). That branch is a defensive guard and isn't exercised by the tests — the `HEAD` form avoided it, which is likely why it was introduced, but correctness outranks covering a one-line unreachable-in-practice guard. Flagging in case `codecov/patch` trips on it. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Worktrunk Bot
|
f2ecccbabe | docs(testing): correct TEST_EPOCH date to 2025-01-02 (#3463) | ||
|
Worktrunk Bot
|
f66b49e348 | fix: resolve project config from bare root when primary worktree is off default branch (#3462) | ||
|
Maximilian Roos
|
a6972e4be0 |
fix(shell): detect the current shell from the process tree, not $SHELL (#3455)
`wt switch` could tell a fish user "shell requires restart" because `$SHELL` (the login shell, say zsh) had the integration line while the shell actually in use didn't. `current_shell()` keyed everything on `$SHELL`, which names the login shell rather than the shell wt was invoked from. **Process-tree detection.** `shell::ancestor_shell()` (src/shell/utils.rs) walks up from wt's parent to the nearest enclosing shell: Linux reads `/proc/<pid>/stat`; macOS parses one `ps -Ao pid=,ppid=,comm=` snapshot. macOS cannot use per-pid `p_comm` (libproc/sysctl): `/bin/sh` re-execs bash via `/var/select/sh`, so an sh script's `p_comm` reads "bash", while `ps`'s comm preserves argv[0]. Verified live and pinned by `test_probe_reports_invoked_name_for_sh`. The walk passes through non-shell wrappers (`git`, `sudo`) and plain script interpreters (`sh`, `dash`); a known-but-unsupported interactive shell (tcsh, ksh) stops the walk so a shell further up can't claim the session. `current_shell()`/`current_shell_name()` consult the walk first and fall back to `$SHELL` / `PSModulePath` (Windows behavior unchanged; Git Bash sets `$SHELL` itself). **Hedged messaging.** The reason string is now "shell integration installed but not active", and the hint presents restart as the likely fix with an escape hatch: "A shell restart usually activates shell integration; if it doesn't, ask an agent to debug with the docs @ https://worktrunk.dev/llms.txt" (the LLM docs index works for any agent; the `/worktrunk` skill exists only for plugin users). Post-install hints are unchanged (restart advice there is unconditional by construction). `wt config show` now prints `Detected shell: <name> (process tree)` next to `$SHELL` in the diagnostic gutter. **Also fixed along the way:** `shell_from_name` requires a word boundary, so `fishd`/`bashtop`/`numactl` no longer classify as shells (`numactl` previously matched Nushell via the bare `nu` prefix); the post-install/uninstall restart hints compare `Shell` values instead of display strings, which never matched process-tree names like `pwsh` or `zsh-5.9`. **Testing.** Integration tests spawn wt under a test harness whose real ancestry (nextest, cargo, the developer's shell) would leak in, so `WORKTRUNK_TEST_PARENT_SHELL` is pinned empty in `STATIC_TEST_ENV_VARS` (and in the two PTY helpers that mirror it by hand, which CI round 1 caught: locally the walk found the dev zsh, on CI the runner's bash) and set per-test to simulate ancestors. New integration tests cover the three detection outcomes (mismatch suppresses restart advice, configured ancestor shows the hedged hint, unsupported ancestor overrides `$SHELL`); unit tests cover the classifier, the walk itself (injectable process-table lookup: wrapper pass-through, transparency, cycle/depth/init terminations), and both OS probes against live processes; `wt config show` is snapshot-tested with an override ancestor and run once unpinned so the production walk executes end-to-end. Known limits, accepted: the walk stops at the nearest supported shell even when it's a non-interactive script interpreter (`bash -c` wrapping wt reports bash), hence the hedged wording; the walk is bounded at 16 hops and any unreadable hop falls back to `$SHELL`. > _This was written by Claude Code on behalf of max_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
36ba57bdca |
fix(plugin): ship skills to Codex installs via a generated real-file mirror (#3440)
Codex installs of the worktrunk plugin carried no skills. `codex plugin add` copies the plugin into `$CODEX_HOME/plugins/cache/` via `copy_dir_recursive` (codex-rs core-plugins), which handles only regular files and directories — so the `skills -> ../../skills` symlink (and the nested `reference/README.md` link inside the tree) were silently dropped, and sessions load from that cache copy. Verified against codex-cli 0.144.1 both by reading the tagged sources and by installing this repo's plugin into a scratch `CODEX_HOME`: the installed root had no `skills/` at all and `codex debug prompt-input` showed an empty plugin skill inventory. This PR cuts the plugin's `skills` symlink over to a **generated real-file mirror** of the authored repo-root `skills/`, kept current by a new `sync_plugin_skills_mirror` stage in `test_docs_are_in_sync` (dereferences symlinks, deletes stale files, self-heals and fails on drift — same pattern as the other generated mirrors). Repo-root `skills/` stays the authored home: Gemini hard-probes it at the extension root, the docs sync writes into it, and Windows checkouts read it. Reversing the symlink direction instead would put a symlink at the repo root, breaking Gemini's install copy and every Windows checkout — which also surfaces a latent bug this fixes: symlinks materialize as plain text files on Windows clones, so installs from a Windows checkout shipped no skills to Claude or Codex either. It also drops the Codex manifest's `skills: "./skills/"` key: with no key, Codex scans `<plugin-root>/skills/` by convention (`default_skill_roots` in `codex-rs/core-plugins/src/loader.rs`), and the explicit path resolves to the same directory, so the key was redundant — symmetric with the Claude manifest cutover in #3431. **For the reviewer:** - The 20 files under `plugins/worktrunk/skills/` are the generated mirror (byte-identical to repo-root `skills/`; git stores shared blobs once). `plugins/worktrunk/CLAUDE.md` → "Plugin skills are a generated mirror" documents the rationale with codex-rs citations. - `sync_plugin_skills_mirror` in `tests/integration_tests/readme_sync.rs` is the sync stage (Step 3b of the pipeline); it already proved itself once in this branch — merging main regenerated `reference/{config,list}.md` in the mirror. - `test_plugin_layout_is_consolidated` now pins the mirror shape cross-platform (real directory, no symlinks anywhere under it), replacing the unix-only symlink assertion. **Verification:** fresh scratch-`CODEX_HOME` install now carries both skills into the cache and `codex debug prompt-input` lists `worktrunk` and `wt-switch-create`; `claude plugin validate` passes and `claude --plugin-dir … plugin details` discovers Skills (2) plus all 8 hooks; Gemini's repo-root path is untouched. A 2×2 probe matrix (manifest key present/absent × real dir/symlink) confirmed key-absence changes nothing and symlinks ship nothing. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
c310767216 |
refactor(bench): canonicalize prune cold to probe-cold via bench_wt (#3437)
Prune's Criterion "cold" had two structural problems: it over-invalidated (deleting worktree indexes models a state users never see, and needed the `restore_worktree_indexes` pairing just to keep prune's clean-worktree gate from silently dropping candidates), and its per-iteration stdout assertions kept it off the canonical `bench_wt` runner. This PR fixes both. **Probe-cold is now the canonical prune cold state.** `invalidate_probe_caches` clears only `.git/wt/cache/`, the exact state a repo is in right after fetching new commits — probes re-run at real cost, git's own caches (indexes with stat data, commit graph, `worktrunk.default-branch`) stay warm. `bench_wt` picks its invalidation via a new `CacheState` enum (`Warm` / `Cold` / `ProbeCold`), and every prune dry-run arm is now a `bench_wt` one-liner. `restore_worktree_indexes` is demoted to a private fixture-healing helper. **Verification moved out of the timed loops.** Timed iterations assert only exit status, like every other bench. Fixture correctness is pinned once after setup: `verify_candidates` runs one dry-run that must list exactly the expected candidates (8 synthetic, 24 rust-scale, 0 on the live fixture's backdrop). A live run that removes nothing trips the next iteration's candidate re-creation (branch-name collision) instead of being silently timed. **The rust-scale cold cell is now repeatable.** `prune_e2e/dry_run_cold` is renamed to `dry_run_probe_cold` (the metric changed meaning, so the old gist series ends deliberately), and a new `prune_real_repo/dry_run_probe_cold` measures at rust scale what was previously a hand-run one-shot — feasible because probe-cold iterations cost ~0.6 s where full-cold costs ~1 min. Measured on a quiet machine, all with tight CIs: synthetic probe-cold 159 ms / warm 96 ms / live 704 ms; rust-scale warm 247 ms / probe-cold 630 ms. ## Testing All five bench arms run end-to-end with the fixture checks passing (synthetic groups plus the feature-gated rust-scale group against a freshly rebuilt 13 GiB fixture). `cargo test -p wt-perf`, clippy on both feature paths, and the rustdoc `-Dwarnings` gate all pass. Benchmarks don't run on PR CI; the daily workflow picks up the renamed series. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
d12c55a320 |
feat(config): wt config update adopts json-schema = 2 instead of pinning 1 (#3436)
Flips the direction of the `[list] json-schema` pending-default handling that #3411 introduced: `wt config update` now writes `json-schema = 2` (adopting the upcoming default) instead of pinning `= 1` (preserving the current one). Running update is the migration; staying on schema 1 is the deliberate manual edit. The `wt list --format=json` nag flips to match, keeping the adopt action last for easy copying: ``` ▲ JSON output is schema 1; a future release switches the default to schema 2 ↳ To keep this format set [list] json-schema = 1; to adopt the new schema, run wt config update ``` Why: with pin-to-1, every `wt config update` run during the deprecation window entrenched users on the schema being retired, leaving a pinned cohort the default flip could never migrate. With adopt-2, update moves users forward as a reviewed config edit, and after the flip `= 2` is just a redundant default a future rule can strip. The trade: `wt config update --yes` in a script switches JSON output as a side effect of unrelated migrations (the interactive path shows the diff first). The system-config gate is unchanged — when the system layer defines the key, update leaves the user file alone; the test now covers the sharper direction (system `= 1` must not be overridden by a user-file `= 2`). All detection/warning invariants carry over; the diff is ~6 semantic lines plus pin→adopt wording and 65 one-line snapshot flips. 🤖 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> |
||
|
Maximilian Roos
|
e02453aa5f |
feat(list): demote branch-worktree mismatch from red flag to dim info (#3419)
Implements option 3 ("demote the presentation") from the design proposal
in #3415, for #3389: worktrees created by agent harnesses at their own
conventional paths (`.claude/worktrees/…`) made every `wt list` row
carry a red `⚑` at merge-conflict severity.
The list predicate, JSON fields, and Path-column behavior are unchanged;
only the presentation changes:
- `⚑` renders dim yellow (was red) in `wt list`, the picker, and the
`--help` legend: below the full-yellow actionable states (prunable,
locked), above plain dim.
- Priority within the status slot becomes `✘ > ⤴ > ⤵ > ⊟ > ⊞ > ⚑ > /`:
the yellow actionable states (prunable, locked) now outrank the
informational glyph instead of being masked by it. The JSON schema-1
`state` field follows the same priority; schema-2 booleans are
independent and unaffected.
- The inline notice on `wt switch` / `wt remove` / `wt merge` / `wt step
prune` is gone entirely: the `wt list` glyph is the state's one surface.
The `expected_path` plumbing is deleted end to end, so switching to an
existing worktree no longer expands the `worktree-path` template at all
(it only ran to feed the notice).
Before/after on a repo with agent worktrees:
```
+ claude/frosty-kilby-92c7d3 ⚑_ (red ⚑, reads as an error)
+ claude/frosty-kilby-92c7d3 ⚑_ (dim-yellow ⚑, reads as a note)
```
Option 1 from the design doc (narrowing the predicate so only genuine
collisions flag at all) can land separately on top of this.
Well-tested: the pre-merge gate passes (4390 tests), snapshots reviewed
line-by-line (glyph color and priority changes in list/help output; the
notice lines removed from switch/remove output), the removed-notice
tests renamed to pin the silence, and clippy is clean including
`--features shell-integration-tests`. Verified live: `wt list` shows the
dim-yellow flag; `wt switch`/`wt remove` print nothing about the
mismatch.
Thanks to @dmsmidt for the report in #3389.
> _This was written by Claude Code on behalf of max_
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|