mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
v0.69.1
2283 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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_ |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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_ |
||
|
|
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. |
||
|
|
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> |
||
|
|
56249f9122 | fix: clear error when new branch name collides with existing branch namespace (#3528) | ||
|
|
c6eb148e62 | docs(hook): correct pre-switch {{ branch }} resolution note (#3516) | ||
|
|
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> |
||
|
|
cda5080195 | test(list): exercise real wide-char branch names in width test (#3511) | ||
|
|
79be81a4bb | fix(merge): Accept merge-shaped fast-forward histories (#3509) | ||
|
|
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>
|
||
|
|
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> |
||
|
|
7735d115ce | fix(tests): restore hidden-subcommand completion coverage (#3492) | ||
|
|
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> |
||
|
|
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> |
||
|
|
9ed48fb32c | fix(config): redirect misplaced nested keys to the other config with a scope note (#3473) | ||
|
|
3b6630be4a | fix(config): show reflects the #3461 object-store project-config fallback (#3477) | ||
|
|
d80cc80ebd | feat(config): flag stale default-branch cache in wt config state (#3471) (#3478) | ||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
f2ecccbabe | docs(testing): correct TEST_EPOCH date to 2025-01-02 (#3463) | ||
|
|
f66b49e348 | fix: resolve project config from bare root when primary worktree is off default branch (#3462) | ||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
b8134563f1 |
fix(list): unify task-failure output vocabulary (#3435)
Reviewing this real `wt list` output against the `writing-user-outputs`
guidelines surfaced several inconsistencies:
```
○ Showing 15 worktrees, 6 branches, 2 with changes, 8 ahead, 2 columns hidden. 1 task failed
▲ Some git operations failed:
plugins: working-tree-conflicts (fatal: Unable to create
'/Users/…/.git/worktrees/worktrunk.plugins/index.lock': File exists.)
↳ To create a diagnostic file, run with -vv
```
The footer and the warning used different vocabulary for the same
failure ("task" vs "git operations", the latter inaccurate since
CI/URL/summary tasks aren't git operations), the warning said "Some"
when the exact count was known, and the per-failure lines leaked
internal kebab-case `TaskKind` discriminants (`working-tree-conflicts`)
that appear nowhere in docs or help.
The same output now reads:
```
○ Showing 15 worktrees, 6 branches, 2 with changes, 8 ahead, 2 columns hidden; 1 task failed
▲ 1 task failed:
plugins: working-tree conflict check (fatal: Unable to create '…/index.lock': File exists.)
↳ To create a diagnostic file, run with -vv
```
## Changes
- **One term, matching counts.** Footer and warning both say "N task(s)
failed" ("task" is the documented term — the JSON docs already use it).
The mixed form becomes disjoint (`2 tasks failed, 3 timed out` instead
of `5 tasks failed (3 timed out)`), so the footer's "failed" count
always equals the number of lines in the warning below it.
- **Human task names.** `TaskKind::display_name()` maps each task to
user vocabulary (`working-tree conflict check`, `ahead/behind counts`,
`CI status`, …) in the failure warning, the stall footer ("waiting on
…"), and the drain-timeout diagnostic. The strum kebab-case form remains
for tracing and `-vv` diagnostics.
- **Footer clause join.** Semicolon instead of a period splice, per
house style; also fixes always-plural "1 worktrees"/"1 branches" in
`--branches` mode.
- **Buffered summary to stderr.** The `○ Showing …` line narrates the
table rather than being part of the answer (`--format=json` omits it),
so in buffered mode it moves to stderr and piped stdout ends cleanly
after the last row. The progressive path keeps it on stdout, where the
summary is a repainted row of the table region. This is most of the
198-file snapshot churn (the line moves from the stdout section to the
stderr section).
- **Docs generator reads both streams.** `parse_snapshot_raw` in
readme_sync previously embedded stderr *or* stdout; it now concatenates
both in terminal order. The generated docs are byte-identical before and
after the stream move, which is the check that the console blocks still
read like the terminal.
Two negative test assertions that guarded against surfacing these
failures (`working-tree-conflicts` in `tests/integration_tests/list.rs`)
were retargeted to the new display names so they don't pass vacuously.
## Testing
Pre-merge gate (all tests + lints) green; feature-gated clippy and the
PTY progressive-list tests (`shell-integration-tests`) run separately
and green, since the local gate skips them.
> _This was written by Claude Code on behalf of max_
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
350ea1f6a8 |
refactor(perf): make wt-perf setup persist-only and trim the CLI/lib surface (#3433)
Follow-up to #3403, applying the deferred `--persist` decision and two sibling subtractions found sweeping the perf tooling for the same shape (a vestigial option or dual path guarding nothing). - **`wt-perf setup` is persist-only.** The `--persist` flag and the interactive "Press Enter to clean up (or Ctrl+C to keep)" path are gone — it was the one blocking, stdin-reading path in an otherwise scriptable tool, and it guarded little: default-path fixtures land in `/tmp/wt-perf-<config>`, which the next `setup <config>` run already wipes and rebuilds. `prune-real` never offered cleanup, so setup's exit behavior is now uniform. - **`timeline --repo` deleted.** It only restated the traced repo: `run_timeline` resolves the repo from `-C`/cwd, `--repo` defaulted to exactly that, and the only documented use passed the identical path to both flags. `--cold` now invalidates the repo being traced — the only coherent target. - **`wt_perf` lib surface tightened.** Seven helpers plus `PruneFixtureState` had no callers outside `lib.rs` and are now private (`ensure_rust_repo`, `clone_rust_repo_at`, `history_spread_shas`, `add_diverged_backdrop`, `create_mixed_repo_at`, `create_prune_real_repo_at`, `prune_fixture_state`); the public surface now matches what the benches and CLI actually use. Doc references to them are plain code spans rather than intra-doc links — rustdoc's `private-intra-doc-links` lint rejects public docs linking private items even under `--document-private-items`. Checked and deliberately kept: `wt-perf trace` and `wt-perf invalidate` (distinct documented workflows), the `TraceResult`/`CacheReport` re-exports (public field types of `TraceEntry`/`Profile`), and the `create_repo`/`create_repo_at` two-layer fixture API (both layers used). Gate green: 4387 tests, clippy/fmt/doctests/rustdoc (`-Dwarnings`) clean. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
6c122a5d08 |
refactor(plugin): adopt Claude's canonical manifest layout, convention-only discovery (#3431)
## What this does Moves the Claude plugin manifest to the canonical `plugins/worktrunk/.claude-plugin/plugin.json` and strips it to metadata only, dropping the `hooks` and `skills` keys so that both component types load purely by Claude Code's convention discovery. Rewrites the layout docs and inverts `test_plugin_layout_is_consolidated` to pin the new invariants. ## Background The plugin layout has been in flux: [#3382](https://github.com/max-sixty/worktrunk/pull/3382) Claude-scoped the hooks filename, which silently broke hook loading ([#3417](https://github.com/max-sixty/worktrunk/issues/3417)), and [#3418](https://github.com/max-sixty/worktrunk/pull/3418) reverted it. That revert restored working hooks but left two claims baked into the layout, the docs, and the test that this PR found to be stale. ## Research method Everything below was verified empirically against claude-cli 2.1.207 (the current release), not from docs alone: - **Scratch marketplace probe**: a throwaway marketplace mirroring worktrunk's exact shape (repo-root `.claude-plugin/marketplace.json`, plugin in a `./plugins/<name>` subdir source, `skills` as a symlink pointing outside the plugin dir, hooks at the conventional path) with two plugin variants — manifest in `.claude-plugin/plugin.json` vs. bare `plugin.json` at the plugin root — installed through the real `claude plugin marketplace add` + `claude plugin install` flow. - **Component inventory**: `claude plugin details` and `claude plugin validate` on both variants. - **Live sessions**: headless `claude -p` runs confirming skills appear in-session and hook commands actually execute (marker files, and reading the 🤖 activity marker from inside a live session). - **Loader inspection**: the manifest resolution order read directly out of the claude-code binary's bundled loader code. ## Findings 1. **The "no `.claude-plugin/` wrapper" claim no longer holds.** The repo documented — and the test pinned — that Claude wants `plugin.json` bare at the plugin root, and that a `.claude-plugin/` wrapper in a subdir-source plugin fails with "Plugin not found". On 2.1.207 the wrapper form installs, loads skills, and fires hooks end-to-end. The loader tries `.claude-plugin/plugin.json` first with root `plugin.json` only as a runtime fallback, several marketplace-resolution paths read *only* the wrapper, and `claude plugin validate` rejects the root-level form outright ("No manifest found in directory. Expected .claude-plugin/marketplace.json or .claude-plugin/plugin.json"). 2. **"Claude has no skill auto-discovery" is wrong.** Skills are auto-discovered by scanning `skills/*/SKILL.md`; a `skills` manifest array only *adds* directories to that scan (entries equal to the default dir are filtered). An unlisted skill behind the out-of-plugin symlink loaded fine — the installer dereferences the symlink into a real directory in the install cache. Dropping the array also makes the old test guard ("every skill must be listed or it's silently invisible") structurally unnecessary: a new repo-root skill now ships to Claude, Codex, and Gemini with no manifest change anywhere. 3. **The `hooks` manifest key was dead config.** #3417 established the string-path override isn't honored for plugin loads; convention discovery of `hooks/hooks.json` is the mechanism that works. A key pointing at the conventional path adds nothing and could mask a future mislocated file, so it's gone (one mechanism per guarantee). ## Changes - `plugins/worktrunk/plugin.json` → `plugins/worktrunk/.claude-plugin/plugin.json`; content reduced to `name`/`description`/`author`. - `plugins/worktrunk/CLAUDE.md`: layout diagram and Claude path-resolution bullet rewritten to the convention-only model; the refuted wrapper claim and the stale no-auto-discovery claim removed. - Root `CLAUDE.md`: the Plugin Layout pointer updated (dropped the stale "Codex no-hooks re-enablement" phrase — Codex hooks shipped inline in #3364). - `test_plugin_layout_is_consolidated`: now asserts the wrapper manifest exists, a root-level `plugin.json` does not, the manifest carries no `hooks`/`skills` keys, the plugin `skills` symlink resolves to the repo-root `skills/` (unix only — Windows checkouts without `core.symlinks` materialize it as a file), and every repo-root skill dir carries a `SKILL.md` (the scan silently ignores a dir without one). Codex and Gemini are untouched: Codex reads only its `.codex-plugin/` wrapper, and its inline `hooks` object still overrides Codex's own convention discovery, so the #3362 collision stays closed independent of any of this. ## Verification - `claude plugin validate plugins/worktrunk` passes (it failed on the old layout). Remaining warnings are deliberate: no `version` field (installs pin the git SHA as the version; a hand-maintained semver would go stale) and the plugin-root `CLAUDE.md` (developer docs, not plugin context). - A live `claude --plugin-dir` session on this exact tree loads both skills and the `UserPromptSubmit` hook sets the 🤖 marker, observed from inside the session. - The scratch-marketplace probe covered the same shape through the full marketplace install flow, including the symlinked skills dir. - Full pre-merge gate: 4389 tests passed plus lints, doctests, and docs-sync. ## Risk The one scenario this could regress is a Claude Code version old enough to read *only* the root-level manifest. That couldn't be tested directly; the mitigations are that the wrapper is the documented convention, the loader prefers it today, and Claude Code self-updates aggressively. The old layout's "verified end-to-end" claim most likely dates from an earlier 2.1.x behavior or a misdiagnosed failure. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
9cb3693fdf | test(switch_picker): tolerate Windows self-exit-1 on successful picker-create (#3427) | ||
|
|
81031bdd2a | chore: bump MSRV and toolchain to 1.96 (#3428) | ||
|
|
c532d2dabd | docs(llm-commits): bump Codex commit-generation model to gpt-5.6-luna (#3430) | ||
|
|
0b42d42670 | feat(step promote): add --format text|json flag (parity with rebase/push/etc.) (#3424) | ||
|
|
eba35d039c | fix(shell): stop PowerShell wrapper leaking a stray exit-code line to stdout (#3425) |