mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
codex/remove-codex-cloud-specific-tests
79 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
246c6bd919 |
fix(list): keep [list] columns out of the --format json plan (#3812)
Closes the `[list] columns` half of #3787, per the call in [this
comment](https://github.com/max-sixty/worktrunk/issues/3787#issuecomment-5273942067):
JSON always emits the same shape, and `list.columns` only affects the
actual columns.
Before, `--format json` planned `all_columns` (source `Default`)
*unioned* with the selection's forced-on columns, so the selection
reached JSON in one direction only — it couldn't narrow the emitted
fields, but a listed `ci` did force the forge fetch on without `--full`.
That made a presentation setting decide whether a machine-readable call
talks to GitHub, which is the thing the Neovim plugin in #3787 had to
pin `--config-set 'list.columns=[…]'` against. Now the JSON branch plans
`all_columns` alone; `--full` is the only switch for the gated data, and
it's the one a caller controls.
The table and the `wt switch` picker are untouched — a listed `ci` still
renders the CI column without `--full`, and the picker still unions the
selection in so its table matches `wt list`'s.
Only `ci` and `summary` are affected: every other column is ungated, so
`full_plan()` already covered them, and custom columns require no
background task.
**For the release note — this changes schema 1 too.** A caller with
`[list] columns = […, "ci"]` and no `--full` used to get the `ci` object
in schema-1 JSON and now won't; schema 1 has no `collected` envelope to
say why. The schema-1 `ci` row already documented `` `--full` only ``,
so the docs get *more* accurate, but the observable output changes for
anyone who was relying on the forcing path. Schema 2 reports the same
narrowing through `collected.ci`.
Docs updated in `after_long_help` (the `[list] columns` section plus the
schema-2 `pr`, `summary`, and `checks` rows — `summary` now names
`--full` alongside `[list] summary = true`, and `checks` names the
`--full` gate it shares with `pr`), with the generated mirrors,
`dev/config.example.toml`, and the `--help` snapshots regenerated. The
`CLAUDE.md` network inventory and the `collect` planning comment now
record the exemption too.
<details><summary>Test</summary>
`test_list_json_columns_selection_does_not_force_ci` in
`tests/integration_tests/list_config.rs` asserts schema 2's
`collected.ci` across three configs: unset (false), `columns =
["branch", "ci"]` without `--full` (false — the regression this fixes),
and the same with `--full` (true). `collected` records what the plan
requested rather than what a fetch returned, so the test needs no forge
and no `gh` on PATH. It sits next to
`test_list_json_ignores_columns_selection`, which owns the narrowing
direction, and `test_list_config_listed_column_overrides_full_gate`,
which owns the table's forcing behaviour and still passes unchanged.
Ran locally: full `cargo test --test integration` and `cargo test --lib
--bins`, plus `cargo clippy --all-targets` and `cargo fmt --check`. One
unrelated failure,
`test_copy_ignored_preserves_file_executable_permissions`, is a umask
artifact of this sandbox (expects `0644`, the runner's `umask 002`
produces `0664`); it touches no code in this diff.
The docs-row follow-up in
|
||
|
|
2d4b6d8ac1 |
docs(shell): state that install owns the wrapper path it writes (#3821)
`wt config shell install` replaces an existing `functions/{cmd}.fish`,
`completions/{cmd}.fish`, or `vendor/autoload/{cmd}.nu` whole. That is
the design — the path is named after the command, so it names the file
worktrunk owns — but neither the write site nor the FAQ's file inventory
said so, which leaves the overwrite reading as a missing ownership
check.
`is_worktrunk_managed_content` already carries the rule, for the
uninstall side that has to recognize a file without knowing its name. So
`configure_wrapper_file` states it in a clause and points there rather
than keeping a second copy. The FAQ sentence adds the contrast: rc files
hold the rest of a shell's setup, so install only appends to those.
No behavior change, and no ownership check added.
> _This was written by Claude Code on behalf of max-sixty_
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
92dfb686bb |
feat(approvals): let wt config approvals add --yes record approvals without a TTY (#3819)
`wt config approvals add` refused every non-interactive run — even with `--yes`, whose hint then suggested the flag already passed — so there was no way to pre-approve a project's commands unattended. An orchestrator (tend's Codex Cloud container was the motivating case) had to hand-write `approvals.toml` from `wt config approvals list --format=json` output, a third-party reimplementation of `add` that breaks whenever the schema changes. The `wt config approvals` docs already promised "`--yes` to bypass prompts in CI" and described `stale` entries as "what `--yes` would silently re-approve"; behavior now matches them. The two `--yes` meanings stay distinct: on a command that runs project commands it grants consent for that run alone and records nothing (unchanged), while on `add` — whose product is the record — it lists what it trusts and writes it. `add` no longer routes through `approve_command_batch` (the execution gate) for this: it prompts or announces, then saves itself, which also makes a failed `approvals.toml` write fail the command instead of warning behind a `✓ saved` line and exit 0 — an orchestrator reading only the exit code would otherwise walk into the prompt it just paid to avoid. The non-interactive hint's pre-approval suggestion now carries `--yes` (`run wt config approvals add --yes`), since a hint reached in CI must name a command that runs there. Per the existing `list --format=json` docs, `add --yes` re-approves templates edited since an earlier approval without comment; the `add` help now says so and points at the `stale` field for reading them first, and the worktrunk skill's escalation rule tells agents not to reach for it on a user's behalf. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
|
aa9d8c43df |
feat: add remote_repo variable (#3745)
Add a `remote_repo` variable that returns the repo name from the remote URL. Unlike `repo`, it stays consistent even if the clone was renamed. Feel free to reject, or suggest other names for the variable. But this change would improve my workflow. I hope you don't mind my submitting a PR before opening an issue. Thanks for an amazing developer tool! AI Disclosure 🤖: I used Claude Code to generate the changes, but reviewed every line and made adjustments. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
|
96c6c846f7 |
fix(shell): register completions under the --cmd name, not clap's (#3817)
## Problem
`wt config shell init <shell> --cmd <name>` renames the shell wrapper
and its lazy completion loader, but the registration that loader evals
comes from clap, which derives every identifier in it from its own
compile-time `Command` name (`wt`) — not from `argv[0]` and not from
`--cmd`. The two halves never agreed:
```console
$ wt config shell init zsh --cmd wot | grep _clap
if ! (( $+functions[_clap_dynamic_completer_wot] )); then
_clap_dynamic_completer_wot "$@"
$ COMPLETE=zsh wt | grep -oE '_clap_dynamic_completer_[a-z_]*' | sort -u
_clap_dynamic_completer_wt
```
Nothing completed, and because the guard never became true the
completion script was regenerated and re-evaluated on *every* TAB. Same
shape in bash (`_clap_complete_*`); PowerShell emitted
`Register-ArgumentCompleter -Native -CommandName wt`, so the `--cmd`
name was never registered at all. The documented `--cmd=git-wt` case
(the Windows Terminal conflict) was broken too — including for a binary
genuinely installed under that name, since clap's name comes from the
declaration rather than `argv[0]`.
There is a second, sharper edge: zsh's registration ends with `compdef
<completer> <cmd>`, so the first TAB on `wot` also bound worktrunk's
completer to plain `wt` — handing completions to the *other* `wt` that
`--cmd` exists to step around.
fish and nushell were unaffected. Both register a completer that shells
out to the binary rather than depending on a clap-emitted identifier, so
the reporter's "unverified" row for fish is a pass.
## Solution
The bash, zsh, and PowerShell loaders now pass the name they bind in
`WORKTRUNK_COMPLETE_NAME`, and `registration_name()` in
`src/completion.rs` emits the registration under that name (validated
through the same `validate_shell_command_name` guard `--cmd` uses, since
the value lands verbatim in generated shell code). The fallback is
`binary_name()`, which covers a binary installed as `git-wt` and invoked
directly. The templates apply clap's own `-` → `_` escaping to the
function they call, so `--cmd git-wt` guards on `_clap_complete_git_wt`
rather than the invalid `_clap_complete_git-wt`.
That fixes all four shells and the stray `compdef` in one place, rather
than pinning the templates to clap's internal naming:
```console
$ WORKTRUNK_COMPLETE_NAME=wot COMPLETE=zsh wt | grep -oE '_clap_dynamic_completer_[a-z_]*|compdef .*' | sort -u
_clap_dynamic_completer_wot
compdef _clap_dynamic_completer_wot wot
```
## Testing
Two reproduction tests in `tests/integration_tests/completion.rs`, both
failing before the change:
- `test_init_custom_cmd_defines_clap_completer_in_bash` drives the whole
chain through a real bash — generate the init script with `--cmd`, call
the loader it defines, then assert clap's completer function exists
afterwards. Printed `MISSING` before, `DEFINED` after. Cases for `wot`
and `git-wt`.
- `test_completion_registration_uses_shell_integration_cmd_name` covers
zsh and PowerShell, which CI can't drive: the identifier the init script
references must be the one the registration defines, and the `compdef` /
`-CommandName` target must be the `--cmd` name.
`cargo test --lib --bins` and `cargo test --test integration` are
otherwise green (one unrelated failure locally,
`test_copy_ignored_preserves_file_executable_permissions`, from this
sandbox's `umask 0002`), and `cargo clippy --all-targets --all-features`
/ `cargo fmt --check` are clean.
---
Closes #3816 — automated triage
---------
Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
|
||
|
|
bdce107d91 |
fix(config): rank env vars and --config-set above project entries (#3790)
Fixes #3788. Layer and specificity were separate steps. `load_with_warnings` flattened system config → user config → `WORKTRUNK_*` env vars → `--config-set` into one document, and the accessors then resolved specificity on that document, so a `[projects."<id>"]` entry answered for the global key of the same name whichever layer set it. `WORKTRUNK_WORKTREE_PATH` could therefore not override a project's `worktree-path`, and a global `--config-set` hit the same wall. Per @max-sixty in the issue thread — "env vars should indeed take precedence over the user project config, we should fix this throughout" — the two invocation layers now cross the axes: they're typed for one run, so they outrank a project entry as well as the global key. Load applies them at both scopes (`apply_invocation_layer_over_projects`, the last step before `finalize`): whatever the layer set is dropped from every project entry, leaving the global key it also set to answer for it. Two kinds of key are held back: - **Keys the layer restates under `projects."<name>"`** — `--config-set 'projects."github.com/owner/repo".worktree-path = …'` is both the highest layer *and* the most specific key, so it still wins over the same layer's global key. - **Composing keys** — hooks, aliases, and `step.copy-ignored.exclude` — whose project-scoped values append to the global ones rather than replacing them. Both already apply, so an env-set hook was never outranked, and dropping the project's copy would silently stop it running. Hook names come from `HooksConfig`'s schema, so a new hook can't be forgotten. Two sections have to go as a unit rather than leaf by leaf. `[commit.generation]`'s mutually exclusive pairs: `template` and `template-file` clear one another in `merge_with` *and* are rejected together by `validate`, so overriding either has to displace both at project scope — otherwise the project's partner would still win the merge. `exclusive_sibling` names those pairs. And `[list.custom-columns]`, which `ListConfig::merge_with` extends per whole column, so a partial removal leaves the project's column replacing the global one anyway — and `ListColumnConfig::template` is required, so it can also strand a column that no longer deserializes. `is_atomic_section` names that table. Both are enumerations, so the pass degrades as a unit behind them: the removals land on a candidate, kept only if it still deserializes and validates. That is the guarantee the env and `--config-set` layers already have, and without it the next required field would answer a stranded leaf with `UserConfig::default()` — costing the user their whole config for that invocation rather than one project entry's precedence. The precedence table now reads: | Source of `worktree-path` | Loses to | |---|---| | `--config-set 'worktree-path = …'` | — | | `WORKTRUNK_WORKTREE_PATH` | `--config-set` | | `[projects."github.com/owner/repo"]` in a config file | either invocation layer | | global `worktree-path` in a config file | all of the above | ## Docs The help text had no precedence section at all — the gap that made this read as a bug — so this adds one under **Environment variables**, plus a pointer from **User project-specific settings**. That supersedes #3789, which documented the old behavior; I'll close it in favour of this. ## Testing Nine unit tests in `src/config/user/tests.rs` cover the table-level rule (both layers, pattern entries, restated project-scoped overrides, untouched sibling keys, composing keys, the exclusive pair, the atomic custom column, a rolled-back layer, and the no-override no-op), and `test_switch_create_invocation_layers_outrank_project_worktree_path` proves it end-to-end — a real process is the only thing that reads `WORKTRUNK_WORKTREE_PATH` off the environment. That test keeps a control showing the project entry still beats the config file's own global key, so it can't pass by project entries having stopped applying. The reproduction from the issue now lands where it says it should: ```console $ WORKTRUNK_CONFIG_PATH="$tmp/wt.toml" WORKTRUNK_WORKTREE_PATH="$tmp/from-environment" \ wt switch --create feature --no-cd --no-hooks --yes --format=json {"action":"created","branch":"feature","path":"/tmp/tmp.jQiTAuNPFd/from-environment",…} ``` <details><summary>Local suite</summary> `cargo test --lib --bins` and `cargo test --test integration` are green apart from `test_copy_ignored_preserves_file_executable_permissions`, which fails in this sandbox because its umask is `0002` (file created `0664`, test expects `0644`) — unrelated to this change and not reproducible on a `0022` runner. `cargo fmt --check` and `cargo clippy --all-targets --all-features` are clean. </details> --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> |
||
|
|
7aba380f0c |
fix(remove): gate removal on the registration, not the repository (#3808)
`wt remove --force` deleted a live worktree of this same repository, uncommitted work included, whenever that worktree had been moved onto another worktree's registered path. The guard added in #3785 asks which *repository* the occupant answers to: a linked worktree's git dir sits under `<common>/worktrees/`, the main worktree's *is* the common dir, anything else is someone else's. A sibling worktree moved onto the path satisfies that — its git dir sits under `<common>/worktrees/` like any worktree of this repo — so it passed, and the fast path renamed the directory into trash and handed the `rm -rf` to a detached process. It is not prunable either: its gitdir file points at a location that exists, so the `is_prunable` arm from the same PR doesn't catch it. Git's own validation is one level finer. `validate_worktree` requires the directory to point back at *this registration*, and refuses this removal with `--force`: ```console $ git -C repo worktree remove --force ../repo.feature fatal: validation failed, cannot remove working tree: '.../repo.feature' does not point back to '.git/worktrees/repo.feature' ``` <details> <summary>Reproducer, verified against a build of main</summary> The occupant has to be *moved* onto the path rather than created there — `git worktree add` refuses a registered path, which is what leaves a plain `mv` as the way this state arises. ```console $ git -C repo worktree add ../repo.feature -b feature $ git -C repo worktree add ../repo.bar -b bar $ rm -rf ../repo.feature && mv ../repo.bar ../repo.feature $ echo PRECIOUS > ../repo.feature/precious.txt $ wt remove --force --yes feature ◎ Removing feature worktree (--force) & branch in background (same commit as main, _) $ ls ../repo.feature ls: ../repo.feature: No such file or directory ``` </details> ## The fix The gate is now git's comparison at git's granularity: the directory's `.git` must name *this* registration, and that registration's `gitdir` file must name the directory back. Repository-level ownership stays as the weaker half of the conjunction — it is what rejects a `.git` file pointing at another repository — and the main worktree is the same test where there is no registration to point back at. `ensure_belongs_to_repo` becomes `ensure_holds_this_worktree`, since it no longer merely asks about repository membership. Resolution moves to `Repository::git_dir_at`, the fs-only resolver the `wt list` prewarm already used (`derive_worktree_git_dir`), generalized to answer for a directory rather than for a known worktree of this repo: its main-worktree branch returned `git_common_dir()` on trust, and now canonicalizes the `.git` it actually found. It also never walks up to a parent, which is what git reads too — `git rev-parse --git-dir` in an emptied worktree can resolve the *enclosing* repository. That settles a second thing the old docstring got wrong. It claimed the plan→rename window was "narrower than `ensure_clean`'s"; in fact `ensure_clean` re-runs `git status` while this gate answered from `GIT_DIRS`, memoized process-wide, so the second call was vacuous and the window — which contains the approval prompt and the `pre-remove` hook — was unguarded. `git_dir_at` reads the filesystem on every call, so the check at the rename now re-decides. The refusal was `Directory @ … is not this repository's worktree`, which is false in the sibling case: it *is* one of this repository's worktrees, just not the one registered there. Its hint didn't fit either — "move the directory aside, then run `git worktree prune`" is a repo-wide prune, and with a sibling moved aside *both* registrations are prunable, so following it clears both and leaves a live checkout that has stopped being a worktree: ```console $ mv ../repo.feature ../repo.aside && git worktree prune -v Removing worktrees/repo.feature: gitdir file points to non-existent location Removing worktrees/repo.other: gitdir file points to non-existent location $ git -C ../repo.aside status fatal: not a git repository: (null) ``` So the error carries where the occupant's own registration records it, and each case gets the remedy that fits. Moving it back to that path leaves prune with only the stale entry to clear: ```console $ wt remove --force --yes feature ✗ Directory @ ../repo.feature does not hold the worktree registered there ↳ Removing it could destroy the worktree registered @ ../repo.other; move the directory back there, then run git worktree prune ``` That path is read through `canonicalize_with_parents`, because a relative `gitdir` entry resolves against `<common>/worktrees/<id>` and would otherwise reach the hint with the `..` chain still in it — and plain canonicalization can't normalize a directory that no longer exists, which is the case the arm is reached for. Normalizing there also makes `crate::path::paths_match`, the crate's canonicalizing comparison over that same helper, the right test for the gate, so there is no second comparison beside it. The gate's fail-closed behavior now rests on that helper resolving `..` through the filesystem rather than collapsing it lexically — across a symlink the two readings name different directories — so `src/path.rs` records the constraint where a lexical rewrite would otherwise read as a tidy-up. The FAQ's "What can Worktrunk delete?" paragraph carried the same "a *different* repository" framing and is corrected. ## Scope Pre-existing, and 0.73.0 already narrowed it — 0.72.0 had no ownership check at all and deleted foreign clones too. The guard has two call sites (`prepare_worktree_removal` at planning, `stage_worktree_removal` at the rename), so this reaches `wt merge --remove`, `wt step prune`, and picker removal, not only `wt remove`. One incidental tightening: `wt remove <bare-repo-path>` previously passed the guard (a bare root's git dir *is* the common dir) and was stopped only by the dirty check, which `--force` skips. It now refuses at the guard. <details> <summary>One residual, left alone</summary> `git worktree repair <path>` after the `mv` produces a *double registration*: both `worktrees/repo.bar/gitdir` and `worktrees/repo.feature/gitdir` come to record the same path, and `git worktree list` reports two worktrees there. In that state the new gate accepts the removal — the occupant does point at the `feature` registration, and that registration does point back — while git refuses, because its path→worktree lookup happens to match the `bar` entry first. Closing it means knowing the registration id at the gate, or scanning every `worktrees/*/gitdir` for duplicate claims. Unchanged by this PR, and reachable only via `mv` followed by `repair`. </details> ## Testing Five new tests, each confirmed to fail with the line it covers reverted and to leave the others passing. Three drive the binary: - **the sibling case** — follows #3785's data-safety model: asserts the filesystem afterwards, not just the exit code, since removal stages by rename and deletes in a detached process. Snapshots the refusal, so the hint and the path it names are pinned. Fails with the pointer-back conjunct removed, while the foreign-repo test still passes without it — the two cover different halves. - **the re-check at the rename** — a `pre-remove` hook repoints the worktree's `.git` at a sibling's registration after planning has already cleared it, which is what makes the second gate's freshness observable. Fails when resolution routes back through the `GIT_DIRS`-cached `git_dir()`. - **a relative `gitdir` entry** — removal succeeds, and git reads the rewritten entry back, which is what makes it the form git itself writes. Rewriting the entry rather than setting `worktree.useRelativePaths` keeps the test independent of the git version that introduced the option. Two sit at the gate, where the CLI can't reach: - **both worktree shapes are accepted** — including the main worktree, whose git dir *is* the common dir. `wt remove` rejects the main worktree well upstream of this gate and a bare repository's worktrees are all linked, so nothing through the CLI would notice that arm inverting. - **the refusal names a normalized path** — asserted against `Diagnostic::render`, since the path is in the hint and `Display` carries only the title. Local gate green: 4607 tests, lints, doctests, rustdoc under `-Dwarnings`. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
667c6efaf0 |
docs(switch): note that Alt-x never forces (#3811)
Requested by @max-sixty in [#3809](https://github.com/max-sixty/worktrunk/issues/3809#issuecomment-5273484502) — a couple of words clarifying that `Alt-x`'s removal is safe-only. The keybinding table read `Remove selected worktree/branch`, which doesn't say the removal never forces; that's what sent the reporter looking for a force-remove that isn't there. The picker hardcodes the safe path — [`prepare_removal`](https://github.com/max-sixty/worktrunk/blob/7a2a3e003e7eed138ff5f2dcd2296f6bbd8e86d4/src/commands/picker/mod.rs#L323-L330) passes `BranchDeletionMode::SafeDelete` and `force_worktree: false`. Deliberately scoped to the table cell, per the "(only)" in the request. The bigger question — whether `Alt-x` should ever pass `-D` — is still open on the issue and isn't touched here. Primary source is `after_long_help` in `src/cli/mod.rs`; the three mirrors and the `--help` snapshot are regenerated. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
|
cf822f75cf |
fix(worktree): guard a registered path that no longer holds its worktree (#3785)
A registered worktree's path was trusted to still hold that worktree.
Three defects followed, one of them destroying data, and the check that
would have caught them — where it existed at all — was `Path::exists()`.
## `wt remove --force` deleted an unrelated repository
A clone that came to sit at a stale registration's path was removed
whole, including uncommitted work and, for a repo never pushed, the only
copy of its objects. wt's own hint routed the user there: the dirty gate
reads `git status` in that directory, reports the occupant's changes as
this worktree's, and offers `--force` as the cure.
```console
$ wt remove feature
✗ Cannot remove worktree: feature has uncommitted changes
?? precious.txt # ← the other repo's file
↳ ... to lose uncommitted changes, run wt remove --force feature
```
git refuses that same removal, `--force` included (`validation failed …
is not a .git file`). Worktrunk's fast path renames the directory into
trash rather than asking git to, so git's validation never ran.
`ensure_belongs_to_repo` makes it, comparing the directory's git dir
against this repository's: a linked worktree's sits under
`<common>/worktrees/`, the main worktree's *is* the common dir, anything
else answers to someone else. One comparison covers both worktree kinds
and also rejects a `.git` file pointing at another repo, which git's
shape test accepts.
It runs at planning, ahead of the dirty gate, and again at the rename
for callers that stage without planning. Now:
```console
$ wt remove --force feature
✗ Directory @ ../repo.feature is not this repository's worktree
↳ Removing it could destroy unrelated data; move the directory aside, then run git worktree prune
```
## A recreated worktree directory leaked git's exit 128
`wt switch`, `wt merge`, and `wt step push` walked into `git rev-parse
--git-dir failed (exit 128)`. Two of them probed `Path::exists()` first,
which a deleted-and-recreated directory passes; the third asked nothing.
`worktree_is_unusable` is the union of both tests, because neither
implies the other. `exists()` catches the absent directory; git's
`prunable` catches the recreated one. `prunable` alone is *not* the
wider test it looks like — git withholds the attribute from a **locked**
worktree even when its directory is gone, since prunability is its
pruning policy and a lock means "don't prune this":
```console
$ git worktree list --porcelain # wt2 locked, all three directories removed
worktree /tmp/ptest/wt1
prunable gitdir file points to non-existent location
worktree /tmp/ptest/wt2
locked removable media # ← no prunable line
worktree /tmp/ptest/wt3
prunable gitdir file points to non-existent location
```
A locked worktree on an unmounted volume is exactly what
`prepare_worktree_removal`'s lock guard exists for, so a `prunable`-only
test would read it as healthy. All three commands now give the message
the merely-deleted case already gave.
`wt remove` keeps `exists()`, deliberately: there it is the precondition
for the cleanup path rather than a health test, since
`prune_worktree_entry` unregisters via `git worktree remove`, which
skips validation only while the directory is absent. No scoped git
command clears the recreated case, so it reports and names the repo-wide
`git worktree prune` that does.
## `wt switch docs/` missed a branch sitting right there
Git's ref format forbids a trailing `/`, so the branch lookup never had
a candidate — and shell completion produces exactly that spelling
whenever a `docs` directory sits beside the branch. Selectors are
normalized before resolution.
## Resolving selectors through one ladder
The three fixes landed in three of the four places that assemble "expand
shortcuts, try the branch, try the path, classify the failure" by hand.
Each gated its path attempt on "did something rewrite this token?",
answered by comparing an expansion's output against its input:
| where | the comparison |
|---|---|
| `resolve_worktree` | `branch == name` |
| `plan_switch` | `target.branch == branch` |
| `target_worktree_at_path` | `target.filter(\|t\| *t == resolved)` |
| `resolve_base_ref` | `resolved == base` |
That is a fact the rewriting step knows, re-derived downstream from its
output, and it is wrong in both directions. A shortcut can expand to the
token it was given — `-` pointing at the branch you are already on — and
string equality reads that as a literal, turning the path arm back on
for a token nobody typed. Normalization breaks it the other way, which
is why the trailing-separator fix needed threading through three call
sites.
`Selector` carries the fact instead: `expand_shortcut` reports whether
it fired, `wt switch` reports its `pr:`/`mr:` dispatch and remote-prefix
strip, and `names_a_path()` replaces all four comparisons.
`resolve_selector` is the ladder, and `plan_switch` expands into it
rather than re-implementing its phases.
`names_a_path()` gates both path steps together — the worktree-by-path
lookup and the directory verdict — which is what `wt switch --create`
needs: the argument names a branch to create, so `branch_only()` takes
the arm off at the producer rather than each consumer re-testing
`create`.
It also reaches the directory verdict, so `ResolvedWorktree` gains
`NoWorktreeAtPath` and the four sites that called `path_selector_error`
themselves stop re-deriving it. The docstring defending that laziness
didn't survive checking — the function returns on `is_valid_branch_name`
before touching the filesystem, so every ordinary branch name already
short-circuited.
| | before | after |
|---|---|---|
| `normalize_selector` call sites | 3 | 1 |
| `path_selector_*` call sites | 4 | 2 |
| "was it rewritten?" comparisons | 4 | 0 |
## Navigating the diff
- `src/git/repository/mod.rs` — `Selector`, `normalize_selector`, the
new `ResolvedWorktree` variant.
- `src/git/repository/worktrees.rs` — `expand_shortcut`,
`expand_selector`, `resolve_selector`, `usable_worktree_for_branch`.
- `src/git/repository/working_tree.rs` — `ensure_belongs_to_repo`, the
ownership check.
- `src/git/remove.rs`, `src/commands/repository_ext.rs` — where it gates
removal, and why before the dirty gate.
- Call sites: `commands/worktree/switch.rs`,
`commands/worktree/push.rs`, `commands/merge.rs`, `commands/remove.rs`,
`git/repository/config.rs`.
## Size
Comments and docstrings are the largest share: the ownership check and
the four conditions behind the directory verdict all look like things to
simplify away, so the reason each exists is recorded where it's
enforced.
| | + | − |
|---|---:|---:|
| Production code | 277 | 142 |
| Comments & docstrings | 320 | 75 |
| Tests | 325 | 8 |
| Snapshots | 186 | 0 |
| Docs | 6 | 0 |
| **Total** | **1114** | **225** |
## Testing
Seven new tests. The data-safety one drives the real binary and asserts
the filesystem afterwards, not just the exit code — removal stages by
rename and deletes in a detached process, so a passing exit would not
have caught a staged-then-deleted tree. The others cover the recreated
directory (switch and remove), the trailing separator, `--create`
against a worktree registered at that path, and, at the unit boundary,
the four states of `worktree_is_unusable` — healthy, absent,
locked-and-absent, recreated — and the selector's path-ness, including
the degenerate case string equality got wrong. Each new test was
confirmed to fail with its fix reverted.
One more covers an omitted merge target in a repo whose default branch
can't be determined. `^` had a test for that error; the omitted-target
route to the same message had none. The gap predates this branch — the
closure is byte-identical to the one it replaces and codecov records
those lines as missed at the base commit too — but relocating them into
`resolve_target_selector` re-counted them as patch lines, which is what
surfaced it.
Local gate green: 4593 tests, lints, doctests, rustdoc under
`-Dwarnings`.
<details>
<summary>Behavioral matrix, verified against a build</summary>
```console
docs/ (trailing sep) ▲ Worktree for docs @ ../repo.docs
detached by path ▲ Worktree for detached worktree @ ../repo.det
leftover dir ✗ No worktree @ ../repo.leftover
recreated dir ✗ Worktree directory missing for rec
shortcut ^ ▲ Worktree for main @ ../repo
remove leftover ✗ No worktree @ ../repo.leftover
--base docs/ ✓ Created branch nf from docs
foreign-repo remove ✗ Directory @ ../repo.frn is not this repository's worktree
precious.txt survives
```
</details>
<details>
<summary>Also swept, and one thing left alone</summary>
Three more instances of the same shape, fixed here:
- `resolve_base_ref` was the fourth copy of the comparison, so `--base
docs/` now resolves too.
- `hint_for_repo` suggested `wt switch ^` after an existence probe a
recreated directory passes, pointing at a worktree the switch then
refuses.
- The pre-switch hook's `target` var used the bare shortcut expander, so
a hook saw `docs/` where the switch resolved `docs`.
The identical unborn/stale default-branch block in
`require_target_branch` and `require_target_ref` is extracted. The rest
of that pair differs in its existence predicate, extra arms, and final
error; sharing it would cost more in parameters than the duplication
does.
Left alone: `live_sibling_checkout` decides whether another worktree
still holds a branch during removal, and also uses `exists()`. Switching
it to `prunable` would make branch deletion *more* likely in a corner
case where the detached path already answers the other way. That is a
data-safety surface and a separate decision.
</details>
> _This was written by Claude Code on behalf of max-sixty_
|
||
|
|
497551e076 |
fix(nix): give the devShell every tool the test suite shells out to (#3768)
`devShells.default` listed `bash`, `zsh`, `fish` under a `# For shell integration tests` comment, but that feature drives two more shells and shells out to `jq`. So `nix develop` could not run `--features shell-integration-tests`, which is what the repo's own gate runs (`.config/wt.toml` → `cargo insta test … --all-features`). This adds `nushell`, `powershell` and `jq`. The same dependency claim was stated, wrongly, in three other places. All four now name the real set: | Where | Was | Now | |---|---|---| | `flake.nix` devShell | bash, zsh, fish | + nushell, powershell, jq | | `Cargo.toml` feature comment | bash, zsh, fish | + nu, pwsh, jq | | `tests/CLAUDE.md` | bash/zsh/fish + PTY | + nushell, pwsh, jq | | `docs/content/faq.md` | bash, zsh, fish, nushell | + pwsh, jq | `flake.nix` also referenced a `CLAUDE.md → "Shell/PTY Integration Tests"` heading that exists nowhere in the repo; it now points at the section that does. ## Verification There is no `nix` on the machine this was written on, so the two claims were checked separately rather than by entering the shell. **Is the list right?** A symlink farm modelling a devShell's `PATH` — nixpkgs stdenv's own tools plus the candidate list, and nothing else — with the full `--all-features` suite run under it. `configure_pty_command` propagates the test process's `PATH` into PTY children, so this reaches the shell tests. <details> <summary>Runs (the control is what makes the failures attributable)</summary> | PATH | Result | |---|---| | Full ambient PATH (control) | 4572 passed, 0 failed | | stdenv + every tool the suite needs | 4572 passed, 0 failed | | …minus `pwsh` | 3 `shell_powershell` tests fail | | …minus `jq` | `test_worktree_remove_hook_skips_path_holding_no_worktree` fails | | …minus `python3` / `lsof` / `ps` | 6 failed: 2 `for_each`/`post_start` (python3), 1 `remove::test_remove_reap_kills_process` (lsof), 3 pgid/process-probe (ps) | The control run matters: it establishes that every failure above is caused by the withheld tool rather than by a local flake. The last row is about what the *suite* needs, not what the shell was missing — see the correction below. </details> **Does the flake still evaluate?** `nixos/nix` in a container, evaluating `devShells.<system>.default` for all four systems `flake-utils` covers, against unmodified `main` as a control. All four evaluate, and every tool resolves on each. Not verified: nothing here was *built*, only evaluated, so a package that evaluates but fails to build would not have been caught. The nightly `nix-flake` job runs `nix flake check` on PRs touching `flake.nix`, which covers that on x86_64-linux. <details> <summary>A correction: python3/procps/lsof were never missing</summary> The first version of this PR also added `python3`, `procps` and `lsof`, claiming the shell could not run a plain `cargo test`. That was wrong, and worktrunk-bot caught it. `craneLib.devShell` sets `inputsFrom = builtins.attrValues checks ++ inputsFrom`, and `mkShell` folds each `inputsFrom` derivation's `nativeBuildInputs` into its own. `checks` includes `worktrunk-tests`, whose `nativeBuildInputs` already carry `git`, `python3`, `procps` and `lsof` — so the devShell inherited all four. Evaluating the unmodified `main` tree confirms it: its `x86_64-linux` devShell derivation already contains `python3`, `procps` and `lsof`, and contains no `nushell`, `powershell` or `jq`. The symlink farm could not have caught this: it modelled stdenv plus the literal `packages` list, so crane's inherited inputs were invisible to it by construction. The experiment established what the *suite* needs; it said nothing about what the *shell already had*. Those three lines are dropped. `git` remains listed in both places — pre-existing, and left alone here. </details> <details> <summary>A guard I added and then removed</summary> `powershell` first went in behind `lib.meta.availableOn`, because nixos-unstable's PowerShell has no `x86_64-darwin` source. Testing that guard against nixpkgs HEAD showed the actual cause: nixpkgs 26.11 dropped Intel macOS wholesale, so the entire flake fails to evaluate there regardless of the guard. On every system nixpkgs still supports, PowerShell has a build — the guard protected against nothing, and its comment justified it with the wrong mechanism. Removed; `powershell` is listed plainly with the other shells. </details> ## Notes `task setup-web` checks for `bash zsh fish nu` and not `pwsh`/`jq` — the same gap in a different environment, left alone here since its install mechanics are unrelated. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
|
bb580ed46b |
fix(plugin): WorktreeRemove hook skips a path holding no worktree (#3767)
## Problem The `WorktreeRemove` hook guards with `[ -e "$p" ]` — #3493's narrowing, which made the hook a no-op when the recorded worktree path is gone. #3753 reports the third state between "gone" and "live": a path that **exists but holds no worktree**. A skeleton directory left by an interrupted create or remove has no `.git`, is invisible to both `git worktree list` and `wt list`, and passes the existence guard, so `wt remove <path>` runs and resolves the path as a *branch* name: ``` ✗ No branch named /…/.claude/worktrees/<name> ↳ To list branches, run wt list --branches --remotes # exit 1 ``` An out-of-tree skeleton gives `fatal: not a git repository` instead. Claude Code reads a nonzero `WorktreeRemove` as a failed removal and keeps the session row, so the finished background session can never be deleted with Ctrl+X — the same end symptom as #3488, but permanent: prune ignores a directory that was never a worktree, so nothing heals it. ## Solution Test for git's marker rather than for the directory: ```diff - [ -e "$p" ] || exit 0 + [ -e "$p/.git" ] || exit 0 ``` Every linked worktree carries a `.git` file, and a dirty, locked, or unmerged one still does, so genuine failures keep surfacing loudly — the #2939 spirit the #3493 guard was written to preserve. Leniency stays scoped to "no worktree lives here" rather than becoming a blanket success, and the change stays inside the single-quoted `bash -c` body, so outer login-shell (fish/zsh/bash) parsing is untouched. `-e` rather than `-f` is deliberate: the *main* worktree's `.git` is a directory, so `-e` keeps `✗ The main worktree cannot be removed` loud where `-f` would silently no-op it. This composes with #3754 rather than overlapping it: the guard now runs before `-C "$p"`, so `-C` only ever resolves a path that really is a worktree. One state changes beyond the reported one, in the same direction: a registered worktree whose `.git` file was deleted by hand already failed the hook (`fatal: not a git repository`), and now exits 0, leaving a prunable registration for `git worktree prune`. Nothing `wt remove` could previously remove is skipped. ## Testing `test_worktree_remove_hook_skips_path_holding_no_worktree` runs the real command out of `hooks.json` under `bash`, feeding it the recorded path on stdin exactly as Claude Code does. It pins three directions, so neither a blanket `exit 0` nor a swallowed `wt remove` failure can satisfy it: a skeleton is a no-op and is left on disk (both in-tree and out-of-tree), a dirty worktree still fails with `uncommitted changes` and stays put, and that same worktree once clean is still removed. <details><summary>Mutation evidence and hand-verified states</summary> Each mutation was applied to `hooks.json` and the test re-run: | mutation | result | |---|---| | `[ -e "$p" ]` (the pre-fix guard) | fails — `hook must be a no-op for a path holding no worktree` | | whole body replaced with `exit 0` | fails — `hook must still refuse a dirty worktree, and for that reason` | | `wt remove … \|\| exit 0` (failure swallowed) | fails — same assertion | Hand-verified against the built binary via `WORKTRUNK_BIN`, firing the hook as Claude Code does: | state | before | after | |---|---|---| | skeleton dir, in-tree | exit 1, `No branch named …` | exit 0, directory untouched | | skeleton dir, out-of-tree | exit 1, `fatal: not a git repository` | exit 0, directory untouched | | recorded path gone | exit 0 | exit 0 | | clean worktree | removed | removed | | dirty worktree | exit 1, `has uncommitted changes` | unchanged | | unmerged branch | worktree removed, branch retained + `-D` hint | unchanged | | main worktree | exit 1, `The main worktree cannot be removed` | unchanged | | registered worktree, `.git` deleted by hand | exit 1, `fatal: not a git repository` | exit 0, prunable entry left | The test is gated on `all(unix, feature = "shell-integration-tests")`, which the required `test` jobs and the coverage run both enable; the hook parses its stdin with `jq`. **Not verified:** Claude Code's own session-row teardown — CI can't drive the agent UI. The evidence here is the hook's exit status, which is what Claude Code branches on per #3488/#3493. </details> The full pre-merge gate passes locally (4571 tests, clippy, doctests, docs sync, no pending snapshots). ## Relation to #3755 Supersedes worktrunk-bot's #3755, whose one-line hook edit is byte-identical to this one. The difference is test coverage: #3755's test passes against a hook with `|| exit 0` appended to `wt remove`, which would silently discard the dirty-worktree refusal — verified by running that test file against the mutation. This one also covers the out-of-tree skeleton. #3755's `flake.nix` addition of `jq` is left out: the devShell already omits nushell so it can't run the shell-integration suite regardless, and the nix test derivation runs default features only, where this test isn't compiled. Closes #3753. Thanks to @judewang for the report, the reproduction, and the fix direction — including the note that `git -C "$p" rev-parse --is-inside-work-tree` is not a usable test, since discovery walks up to the parent for a nested skeleton. > _This was written by Claude Code on behalf of max-sixty_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3817df0795 | fix(plugin): resolve WorktreeRemove against the worktree path, not the project dir (#3754) | ||
|
|
7f8ac8e5d7 |
feat(list): publish a JSON Schema for the schema-2 envelope (#3747)
`wt list --format=json` schema 2 now has a published, machine-readable contract at [worktrunk.dev/schema/list-v2.json](https://worktrunk.dev/schema/list-v2.json). The schema was already derived — `test_schema_generates` built one, asserted it compiled, and threw it away, with a comment saying "until the schema export ships." This ships it: `wt list --print-schema` prints the document (a developer entry point alongside `--help-page`, intercepted before clap), and a new step in `test_docs_are_in_sync` commits it to `docs/static/schema/list-v2.json`, the same generate-and-commit pattern as `llms.txt`. It shells out rather than calling `schema_for!` because `JsonEnvelope` lives in the bin-only `crate::commands` tree. Two things had to be fixed for the document to be usable. **The contract.** `schema_for!` generates under schemars' *deserialize* contract, which marks a `skip_serializing_if` field required — nothing supplies it on the way in. The first document I generated therefore required `default_branch`, `upstream`, `pr`, `checks`, `summary` and `vars` on every item, all of which the absence rule routinely omits, so it rejected the output it documents. Generating under `for_serialize()` fixes it. **The vocabularies.** Four fields — `checks.status`, `display.state`, `default_branch.integration.reason` and `worktree.operation` — were `&'static str`, so the schema described them as bare strings. They are now `JsonCheckStatus`, `JsonMainState`, `JsonIntegrationReason` and `JsonOperation`, each converted from its domain enum by an exhaustive match, so a new `CiStatus`, `MainState`, `IntegrationReason` or `InProgressOperation` variant is a compile error rather than a value silently missing from the published vocabulary. **The emitted JSON is unchanged**; the existing envelope snapshot passes untouched. <details> <summary>Before and after, for one item</summary> ```json // before — rejects its own output, and loses the vocabulary "required": ["default_branch", "upstream", "pr", "checks", "summary", "vars", "display"], "status": { "type": "string" } // after "required": ["branch", "head", "display"], "status": { "enum": ["passed", "running", "failed"] } ``` </details> ## Testing `test_schema_accepts_envelopes` validates a battery — every `CiStatus` over both sources, every `MainState`, a populated worktree row, an integrated row with an upstream and a dev server, plus the absent and null arms of the absence rule — against the same document `--print-schema` emits. Validating proves nothing about a type the battery never instantiates, so the test also pins every non-`Nullable_` type in the document to a path that must carry a non-null value. A new `Json*` type fails until the battery reaches it, and a row that stops populating one fails too — the check reports the type names rather than leaving the gap to a reader. This needed a `jsonschema` dev-dependency: schemars only generates, and derives the document from the types without ever seeing an envelope, so nothing otherwise tied the two together. The test was confirmed to fail on the bug it exists for. Reverting to `for_deserialize()` makes it report `pr`, `checks`, `summary`, `vars` and `display.columns` as wrongly required. The dependency is dev-only: `reqwest`, `rustls` and `async-trait` stay unselected so no HTTP stack comes along, and `cargo tree --package worktrunk --edges normal -i jsonschema` finds no path to it. One direction it deliberately does not cover: a *loosening*. If a field reverted to `&'static str` the schema would say `type: string` and anything would validate. That direction is held by the compiler instead, via the exhaustive matches. ## Notes for review - `schema_document()` lives in `json_v2.rs` beside the types, not in `help.rs`, so `--print-schema` and the test compile the same document rather than two constructions that could drift on the contract setting. - The lychee exclusion for `worktrunk.dev/schema/` follows the entry directly above it: a generated link that 404s until the site deploys. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
|
d4538f2047 |
fix(alias): carry the shell's cwd into alias and hook bodies (#3724)
## Problem Since #939 / #3344, `wt switch` and `wt remove` preserve the user's subdirectory position — from `monorepo.feature/subproject/` you land in `monorepo/subproject/`. #3723 reports that this is lost one layer down: with `[aliases] finish = "wt remove -y"`, `wt finish` drops you at the primary worktree root. The resolution reads the user's position from the wt process's own cwd (`resolve_subdir_in_target`, called with `std::env::current_dir()`). That answers "where is the user standing?" only for a top-level invocation. Alias and hook bodies run with the worktree root as their working directory, so the nested `wt` strips the source root off a cwd that *is* the source root, gets an empty relative path, and falls back to the destination root. ## Solution The CD directive file already travels to exactly the children that are allowed to move the user's shell. The shell's directory now travels with it: `apply_cd_directive_env` sets `WORKTRUNK_SHELL_CWD` wherever the CD file is re-added (`Cmd::stream` and the concurrent runner), and `scrub_directive_env_vars` strips it alongside the other directive vars, so an untrusted child neither keeps nor receives it. `shell_exec::shell_cwd()` reads it back, preferring the inherited value over the process cwd — which is what makes nesting compose, since each layer forwards the shell's directory rather than its own. Three sites ask that question and now go through it: `wt switch`, `wt remove` (`prepare_remove_directory_change`), and `wt step relocate`, whose existing comment already asks to behave identically to the other two. Nothing about the working directory of alias or hook *bodies* changes — `{{ cwd }}` is still the worktree root, per the documented contract. The only change is what a nested `wt` believes about the user's position. ## Testing Two integration tests in `tests/integration_tests/step_alias.rs`, both failing before the change with the exact symptom reported: ``` CD file should preserve the subdirectory (…/repo/apps/gateway), got: "…/repo\n" ``` - `test_alias_wrapping_remove_preserves_subdir` — the reported case (`[aliases] finish = "wt remove -y"` run from `feature/apps/gateway`). - `test_alias_wrapping_switch_preserves_subdir` — the same for `wt switch` inside an alias. The existing subdirectory-preservation tests in `directives.rs` (including the fall-back-to-root cases) still pass, as does the full integration suite — apart from `test_copy_ignored_preserves_file_executable_permissions`, which fails identically on `main` in this sandbox (umask `0002`, expects `0644` gets `0664`) and is unrelated. The open question from the first revision is answered: the new remove test leaves two processes with a cwd inside the worktree being removed (the alias parent in the subdirectory, the nested `wt` at the root) where the existing test has one, and `test (windows)` passes on it. Review follow-ups are in |
||
|
|
5c42c5b7d5 |
feat: machine-readable approval state and branch-removal outcomes (#3710)
Two of the five machine-readable-output requests NathanaelRea opened (#3696–#3700), reviewed as a set and implemented where the gap was real. ## `wt config approvals list --format=json` (#3698) The command already computed the four distinctions an orchestrator needs — no commands, approved, approval-required, and stale — read-only, without prompting or writing. It had no `--format` flag, so the only way to learn that a non-interactive run would stop for approval was to run the operation and catch `NotInteractive`, or to pass `--yes` and approve whatever was there. ```json { "state": "approval_required", "commands": [ {"phase": "post-start", "name": "dev", "template": "npm run dev", "approved": false}, {"phase": "pre-merge", "template": "cargo test", "approved": true} ], "stale": ["some removed command"] } ``` `state` is what a caller branches on. `stale` stays a separate list rather than a fourth `state`, because it co-occurs with all three — and those are the approvals `--yes` would silently re-approve after their command template changed, which is exactly what an orchestrator preserving the approval model needs to see. A flag on the existing read command rather than a new `status` verb, matching `wt config show`, `wt config state get`, `wt config state logs`, and `wt list`. Closes #3698. ## `branch_outcome` on removal (#3700, partly) `wt remove --format=json` reported the branch as one boolean, collapsing five internal outcomes into two values: | Internal outcome | `branch_deleted` was | |---|---| | `Deleted` | `true` | | `Deferred` — handed to a detached process, result never observed | `true` | | `NotAttempted` — no branch, or `--no-delete-branch` | `false` | | `Retained` — a sibling worktree has it checked out | `false` | | `Retained` — **the CAS refused; the ref moved under us** | `false` | The last row is the exact race #3700 asks for protection against. Worktrunk already deletes with `git update-ref -d <ref> <oid>` and already fails closed when the ref has moved — then reported it as the same `false` that means "you asked me not to". And `Deferred` reported `true` on intent. `branch_outcome` names it instead: `deleted`, `deferred`, `not_attempted`, `retained_unmerged`, `retained_checked_out`, `retained_raced`, `retained_failed`. A caller that sees `retained_raced` knows to re-read the ref and retry, which is what the guard detects it for. **This does not close #3700.** That issue asks for an *input* — a caller-supplied expected OID that makes `wt` fail closed against the orchestrator's own observation. This is an *output*. They land in the same place on the default path, because the integration check already refuses to delete unintegrated content, so the caller was never going to lose commits — they just couldn't classify the refusal. Where the gap is real is `--force-delete` / `-D`, which takes the early return in `delete_branch_if_safe` and runs `git branch -D` with no integration check and no CAS. If an `--expected-oid` flag lands, it has to gate that path. ## Notes - **Output-format break.** `branch_deleted` is replaced, not supplemented, on `wt remove --format=json` and on `wt step prune --format=json`'s live path. Per CLAUDE.md, output formatting is on the flexible side of the interface line; flagging it here so the release changelog picks it up. - **`wt step prune --dry-run` keeps `branch_deleted`.** A dry run predicts; it runs nothing to have an outcome. Different thing, different name, documented as such. - **`retained_raced` and `retained_checked_out` have no deterministic CLI trigger.** Both come from windows between `wt`'s own fresh read and the ref mutation, which no hook can be scheduled inside. They're covered at the unit level (`branch_fate_from_result_mapping`, `branch_fate_json_outcome_is_distinct_per_fate`, and `cas_rejects_delete_when_branch_advances` in `src/git/remove.rs`, which drives the race with a stale snapshot). The integration tests cover the two reachable contrasts: `retained_unmerged` via a `pre-remove` hook that commits, and `not_attempted` via `--no-delete-branch`. - **`print_json` lives under `src/commands/list/`** and now has a third caller from outside that module. Worth a more central home; not moved here. ## The other three Reviewed but not implemented: - **#3696** — already possible. `wt --config-set 'list.json-schema = 2' list --format=json` pins the schema per invocation above every config layer, as does `WORKTRUNK_LIST__JSON_SCHEMA`. Answered on the issue; what's left is a docs gap and making an out-of-range value fail rather than degrade in JSON mode. - **#3697** — the machine-readable error channel. A real gap and the one policy call in the set; not started. - **#3699** — aimed at `wt config state logs --format=json`, which is a directory listing reconstructed from paths, under a model that overwrites. The append-only run record it wants is `commands.jsonl`. ## Testing `cargo run -- hook pre-merge --yes` green: 4533 tests, clippy, fmt, doctests, rustdoc, docs sync. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
60661a0d7e |
docs(signing): carry the SignPath attribution on the install section (#3709)
SignPath Foundation's OSS program requires the attribution notice, and a route to the code signing policy, on a project's home page and download/release pages. Worktrunk had both only on the policy page itself — nothing on the README or the docs landing page. This puts the notice in the install section's Windows block, beside the artifacts it actually describes: > Free code signing provided by [SignPath.io](https://signpath.io/), certificate by [SignPath Foundation](https://signpath.org/) — [policy](https://worktrunk.dev/code-signing/). The edit is one line in `docs/content/worktrunk.md`; the README's Install→Further reading block is generated from it, so it propagates there and to both skill mirrors. The policy page leaves the docs navigation in the same change, so this is the single place it's linked from. `hide_from_nav = true` in a page's `[extra]` skips it in all three loops over `docs_section.pages`: the desktop TOC (`macros.html`), the mobile menu (`base.html`), and the prev/next flow nav (`page.html`). The page stays published and reachable at `/code-signing/` — unlisted, not removed. In the nav loops the skip wraps the whole per-page body, so a hidden page can't emit a stray group heading. In the prev/next loop it guards only the *candidate* assignments — the current-page test stays unguarded, because viewing a hidden page directly must still flip `found_current` or its own neighbours compute against the wrong page. Verified both directions: FAQ ends at `← Tips & Patterns` with no forward link, and the policy page keeps `← FAQ` back out into the docs. <details><summary>Why this came up, and the placement tradeoff</summary> Found while debugging why the `release-signing` signing policy shows INVALID in the SignPath console. That turned out to be unrelated and not fixable here — its certificate ("Release certificate 2026", subject `CN=SignPath Foundation`, on SignPath's HSM) is in `CSR PENDING` with no validity dates, awaiting CA issuance. The policy's own configuration is complete and correct. Worktrunk currently signs with the test certificate, which is VALID. The attribution gap was the one thing found on our side. Whether it bears on the pending review is unknown — the console exposes no application status. On placement: the notice sits inside the collapsed `<details>`, so it isn't visible until a reader expands "Windows & other". That's deliberate — the signing is Windows-specific and the notice reads better next to it than in the page chrome — but it is the least prominent placement that still counts as a link, and the terms ask for the notice *on* the home and download pages. Worth knowing if placement is ever queried during review. </details> > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
eddd280ce1 |
docs(step): prefix the copy-ignored --require-include example with $ (#3706)
Nightly sweep finding: the `--require-include` example in `wt step copy-ignored`'s long help was the only `console` code block in `src/cli/step.rs` missing the `$ ` prompt prefix. Per the doc-sync convention (`docs/CLAUDE.md`: "All shell commands use `$ ` prefix in ` ```console ` blocks"), a bare `console` block converts to a plain ` ```bash ` fence in the web docs, whereas a `$ `-prefixed one becomes a `terminal` shortcode. So this single example rendered inconsistently with every other command block on the [step page](https://worktrunk.dev/step/) — as a plain code fence rather than a styled terminal line. The primary source is `after_long_help` in `src/cli/step.rs`; the three generated mirrors (`docs/content/step.md`, `skills/worktrunk/reference/step.md`, `plugins/worktrunk/skills/worktrunk/reference/step.md`) were regenerated by `cargo test --test integration test_docs_are_in_sync`. No `--help` snapshot changed — the terminal help renderer already styles the line identically with or without the prefix; the fix is web-docs-only. No regression test: this is a pure documentation-string change, and `test_docs_are_in_sync` already enforces the mirror consistency. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
|
4a6fd1ec44 |
refactor(merge): leave target-worktree changes in place, drop the autostash (#3703)
`wt merge` / `wt step push` previously moved a dirty target worktree's uncommitted changes aside with an autostash (`git stash push -u`) and restored them after the push. That design entered `refs/stash` — a repo-global namespace any process can mutate, the source of #3683's race class — restored staged changes as unstaged, and existed only because the fast-forward's `receive.denyCurrentBranch=updateInstead` refuses any dirty worktree at all. Both strategies now advance the target through one `advance_target`: - a compare-and-swap `update-ref` (fails cleanly if the target moved since the snapshot; reflog entries are labeled), - `update-index -q --refresh` + `read-tree -m -u <old> <new>` in the target worktree — git's documented lenient `push-to-checkout` policy (githooks(5)), - a CAS rollback when the sync can't apply, so branch and worktree move together or not at all. Uncommitted changes at paths the push doesn't touch never move: unstaged edits stay unstaged, staged entries stay staged, untracked files stay put, and `refs/stash` is never involved. The autostash machinery (`TargetWorktreeStash`, `StashData`, `stash_restore_failed` and its exit-code path from #3693) is deleted — including the staged-restored-as-unstaged flaw, which disappears with the restore itself. Behavior changes: - Receive hooks no longer fire on the fast-forward path — there is no `git push`. A `git merge` run in the target wouldn't run them either, which is the line the module spec draws. - A sync that can't apply fails the whole command with the ref rolled back; `--no-ff` previously warned and left the worktree stale behind its own branch. - An untracked-path collision is refused upfront, naming the file, instead of stashed and later maybe-conflicting. Review highlights (three adversarial passes over the diff): - The upfront conflict check reads `status --porcelain -uall`, so untracked files inside untracked directories get the named upfront refusal rather than a generic sync failure (and one subprocess is dropped). - A commit racing into the CAS→sync window is detected by a post-sync ref re-read — receive-pack used to give the fast-forward path this check structurally — and warned about. - `read-tree` runs under `-c submodule.recurse=false`, keeping #1604 fixed for `submodule.recurse=true` users. - The ignored-file carve-out (an ignored file at a path the push tracks is overwritten, as a `git merge` there would) is pinned by an end-to-end test: two review passes claimed `read-tree` refuses it; experiment on git 2.55 refuted both. > _This was written by Claude Code on behalf of max-sixty_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
02a12c7f59 |
feat(config): match [projects."…"] keys by pattern, and carry forge there (#3701)
## Problem Forge platform is readable only from project config (`[forge].platform`) or a brand substring in the remote hostname. A self-hosted host carrying none of `github`/`gitlab`/`gitea` — a GitLab at `git.company.example`, a company git server — needs the same `[forge]` block in every repository's `.config/wt.toml`. Closes #3678. The user-level `[projects."…"]` table is where per-repository settings already live without touching each repo, but its keys are exact, so covering a host means one entry per repository. ## Solution **Pattern keys.** A `[projects]` key containing `*` matches any run of characters, `/` included, so one entry covers every repository on a host, nested groups and all. `*` is the only metacharacter. ```toml [projects."git.company.example/*"] forge.platform = "gitlab" [projects."git.company.example/platform/*"] worktree-path = ".worktrees/{{ branch | sanitize }}" ``` Every matching entry applies, least- to most-specific, so a narrower key wins where two set the same field and leaves the rest alone. A literal key is the most specific of all; specificity is the count of non-`*` characters. Rules and rationale: the `project_match` module docstring. **`forge` on `[projects]`.** Same shape as the repository's own block, carrying `platform` and `hostname`. Both describe the host rather than the repository — which is why an SSH alias resolved through `~/.ssh/config`, a name local to one machine, belongs in user config rather than a repository's committed one. A repository's own `[forge]` still wins field by field, being the more specific of the two: a repository that sets only `platform` still takes a matching entry's `hostname`. **One resolver.** `wt list`, its statusline, `wt switch pr:`, and CI-platform detection each read project config separately, so a configured platform could resolve in one command and read `unknown` in the next. They now share `Repository::configured_forge_platform` (and `forge_hostname` for the API host). ## Approvals `approved-commands` matches by the same rules, so a pattern entry approves its commands for every repository it covers. That widening is the user's to opt into — only a hand-written key is ever a pattern: - `wt config approvals add` and the interactive prompt record under the exact project identifier, so approving in one repository never reaches another. An identifier that itself contains `*` (a starred remote URL or no-remote path fallback) is refused outright — persisting it verbatim would create an entry reads treat as a pattern; the interactive flow degrades to a warning plus a per-run approval. - `wt config approvals clear` empties only the exact entry, leaving a pattern other repositories share intact — and both its outcomes end with a hint naming any pattern entries still approving commands for the project, so a surviving approval is traceable to the hand-written entry supplying it. - `--stale` judges only the exact entry, so one repository's config can't revoke approvals the others rely on. ## Tests `project_match` unit tests cover `*` spanning `/`, `.` staying literal, specificity ordering, and the lexicographic tie-break. Config tests cover a host-wide entry applying to nested groups, exact-over-pattern precedence, field-by-field layering, hooks appending across both entries, and forge platform/hostname. Forge resolution tests cover the unbranded host, nested groups, a narrower entry winning, project config overriding, falling through to inference, and an invalid value leaving the host unresolved. Approvals tests cover pattern lookup plus the two exactness guarantees above. ## Docs `src/cli/mod.rs` (the primary source) gains "Matching several repositories with one entry" and "Forge platform and hostname" under user project-specific settings, plus a pointer from the project-config forge section. Generated mirrors and `--help` snapshots regenerated. ## Review hardening An adversarial review pass surfaced eight findings, all fixed: - **Approval widening (moderate)**: the starred-identifier refusal above. Previously such an approval persisted verbatim and silently approved its commands for every repository the star matched. - **Literal-key tie (moderate)**: a pattern whose stars all match empty (`github.com/owner/repo*`) ties the exact key on literal count and sorted after it, so its values won the fold. Literal keys now outrank any pattern outright. - **Docs vs behavior (moderate)**: the layering paragraph claimed "most specific wins" for everything; hooks and aliases actually append across matching entries (all run, least-specific first). Docs now say so, and state the forge field-by-field precedence. - **Minor**: `matches()` is a two-pointer byte glob (was a per-call regex compile, ~0.7 ms per pattern key, a few hundred calls per `wt list`), pinned by an exhaustive differential test against a reference matcher; the invalid-platform diagnostics name their two possible config homes; a root `[forge]` in user config now points at `[projects."<id>"].forge`; docs note a host-wide key should end in `/*`; `approve_command` delegates to `approve_commands`, unifying their dedup predicates. ## Relationship to #3681 This is an alternative to #3681, which adds a bespoke `[forge-hosts]` section for the same issue. Both can't land — they'd be two ways to write one sentence. This one puts the setting in the table that already carries per-repository user config, and the pattern keys are reusable for the workspace-scoped ask in #3654 where repositories share a host or namespace. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
|
03f49ce93a |
fix(merge): exit non-zero when the target autostash can't be restored (#3693)
Follows #3684. The Windows test un-gating that was stacked here is split out into #3695, now merged; this PR is the exit-code change alone. ## Problem `wt merge` reported success when the target worktree's autostash failed to replay. The warning named the recovery command, but exit 0 said the user's uncommitted changes were back in their worktree while they were still in a stash — and that warning scrolls past under the worktree-removal and post-merge-hook output that follows it. ## Solution The restore outcome travels out through `PushResult`. The command finishes everything it started — ref advanced, worktree removed, hooks run, `--format=json` payload printed — and only then returns `AlreadyDisplayed { exit_code: 1 }`. Aborting at the restore instead would leave a landed merge with its cleanup half-done, trading a recoverable stash for a worse mess. The shell wrapper applies its `cd` directive whenever the directive file is non-empty, independent of exit code, so a non-zero exit strands nobody in a removed directory. Both output channels name the failure: the `--format=json` payloads of `wt merge` and `wt step push` carry `stash_restore_failed`, present on every payload like the other outcome booleans. The exit code alone would leave a consumer reading stdout with a success-shaped object and no signal. This also closes a gap the change surfaced: `handle_no_ff_merge`'s already-up-to-date early return never called `restore_stash`, so a dirty target worktree with nothing to merge restored through `Drop` and reported nothing. It restores on that path too now, which is what makes the guarantee hold — every remaining `Drop` of the guard happens on a path already returning an error. ## On diverging from git `git rebase --autostash` exits 0 in this situation: it prints "applying them resulted in conflicts" and still reports "Successfully rebased". The difference is what the user is left looking at. git's failure leaves conflict markers in the working tree, met immediately; a failed `git stash apply` here can leave the worktree untouched — an untracked path re-created underneath it, for instance — so nothing but the exit code outlives the warning. The reason is recorded on the field the exit code hangs off, so it doesn't read later as an oversight. ## Testing `test_merge_autostash_restore_failure_exits_non_zero_after_cleanup` covers the guarantee end to end: exit 1, `stash_restore_failed` in the JSON, ref advanced, source worktree removed, stash entry still present for recovery. `test_push_autostash_restore_failure_warns` moves from asserting `success()` to asserting exit 1 plus the push having landed. Also verified against a real build outside the suite, on both commands: the merge lands, the worktree is cleaned up, exit is 1, the JSON reports `"stash_restore_failed": true`, and the warning names the exact `git stash apply <sha>`. `wt step push --help` gained the failure contract, since it previously described only the success path; the generated mirrors are regenerated with it. Local (macOS): `cargo run -- hook pre-merge --yes` green — 4500 tests, clippy, fmt, doc sync. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
23b707ca94 |
docs(readme): refresh status stamp to August 2026 (#3686)
Nightly maintenance: the README status blockquote opens with a month+year stamp that should track the current month (per the README date check in the `running-tend` skill). It read **July 2026**; today is 2026-08-01, so this refreshes it to **August 2026**. No test — this is a one-word documentation refresh with no behavioral surface. The blockquote month isn't asserted by any snapshot (`readme_example_list_branches` covers the `wt list` example output further down, not this line). --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
|
e1745db105 |
feat(list): abbreviate the table's SHA with git, not a fixed slice (#3676)
The Commit cell sliced `&head[..8]` while `--format=json`'s `short_sha` carried git's `%h`, so one commit read `1b9f1d96` in the table and `1b9f1d9` in JSON, and `core.abbrev` reached only the JSON. #3675 gave a detached row's Branch cell the same slice, so the disagreement showed up twice on one row. `ListItem::short_sha` becomes the only abbreviation of `head` anywhere: the Commit cell, a detached row's Branch cell, the statusline, and JSON all render it. `abbreviated_head()` is gone. ## Column widths `COMMIT_HASH_WIDTH = 8` is gone. The Commit column and the Branch column's detached budget both measure the SHAs they will render, so `core.abbrev = 12` no longer truncates mid-hash and the default 7 stops reserving a column nothing fills — the freed character goes to Message. ## Latency `collect()` folds `%h` onto the rows before layout instead of after the skeleton. The batch carrying it already gates the skeleton for `%ct` sort order, so this is a map lookup rather than new I/O, and both cells are identity columns with no placeholder — they still paint in the first frame. Measured on a 40-worktree / 400-branch fixture: git subprocess counts are identical (5 pre-skeleton, 108 for the full run). Pre-skeleton wall time is unchanged; running both binaries in each order, the sign of the difference follows run order rather than the binary (+1.5 ms with this branch second, −0.5 ms with it first), so the residual sits inside drift. ## Behavior change Where the commit-details batch fails, the Commit cell is now empty rather than a slice of a SHA git refused. Age and Message already report that failure the same way, under the same warning, and two snapshots show it. `render_text_cell` also stops styling empty text, so a blank cell no longer emits an escape pair around nothing. ## Reading the diff 160 files, but the hand-written part is +91/−79 in `src/commands/list/` plus a +71 test. The rest is generated. The docs mirrors and help snapshots are symmetric. Of the snapshot lines, content is +799/−799 — every changed line a 1-for-1 hash swap — while +1396 is insta `env:` metadata refreshing on the 128 snapshots this happens to touch. `test_list_abbreviated_sha_follows_git` pins the invariant: the table's hash equals JSON's `short_sha` at git's default and at `core.abbrev = 12`, and a longer prefix is ruled out. It fails against the old fixed slice. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
|
0f2d562541 |
fix(forge): classify a forge by the brand in the hostname, not by DNS label (#3673)
Reverts the branded half of the exact-label classifier and deletes the diagnostic built to explain it. `github-enterprise.acme.com`, `mygithub.com`, `gitlab-internal.company.com`, and the `github-personal` SSH alias resolve to their forge again, so CI status, `wt switch --prs`, and `repo.provider` work with no config. ## Why the label boundary goes It looked like an ownership check and wasn't one. An attacker controls their own DNS, so `github.attacker.example` has the exact label `github` and classified fine; `gitlab.evil.co.uk` likewise. What the rule actually excluded was the self-hoster who put the brand in a hyphenated name. It failed open for the adversary and closed for the customer. The residual case for it doesn't survive either. The hostname comes out of the user's own `.git/config`, and whoever can put a host there can put code there too — the trust decision happens at clone time, and by the time worktrunk reads the remote the user is already building from it. All the classification decides is which forge CLI (`gh`, `glab`, `tea`, `az`) runs against it. So the rule is recall-first: any host carrying `github`, `gitlab`, or `gitea` matches, first match winning. The cost is a host that merely sounds like a forge getting a forge CLI run at it, which surfaces as that CLI's error rather than as silence — the better of the two failures, and `forge.platform` overrides it. ## What stays Azure DevOps keeps suffix matching on its two service domains, and for a reason unrelated to security: those are service domains rather than a brand in the host, so every real hosted instance already matches, and the on-prem edition carries neither string. `dev.azure.com.attacker.example` and `evil-visualstudio.com` are outside the domains and carry no brand to fall back on, so they stay unclassified. Userinfo still resolves to the network host, so `https://github.com@attacker.example/…` is `attacker.example`. ## What goes `LegacyForgeAlias`, `Repository::legacy_forge_alias`, `legacy_forge_alias_diagnostic`, and its three emit sites in `wt list`, `wt switch --prs`, and `wt config show --full`. Every host the diagnostic fired on now classifies, so it could only ever return `None`. A host with no brand at all still reaches the existing generic hint, which is the right message there — there is no platform to infer. The end-to-end warning-dedup test goes with it, since no warning is raised from both the collect and `--prs` threads any more; `stash_warning_preserves_order` keeps the mechanism covered. ## Docs `## Forge platform` in `src/cli/mod.rs` described the override as being for SSH aliases and self-hosted instances, which now detect on their own. It states the rule and scopes the override to hosts carrying no brand — a Forgejo instance at `forge.example.com`. Mirrors, `dev/wt.example.toml`, and the two `config` help snapshots regenerate from it. --- The branch's history has a false start — the first commit widened the diagnostic, the second deletes it in favour of relaxing classification — plus a merge of `main` after v0.71.0 shipped. The net diff is the second approach; it all squashes on merge. Also supersedes [#3672](https://github.com/max-sixty/worktrunk/pull/3672), the triage bot's PR for the same issue — it widens the diagnostic rather than removing the need for one, so it should be closed too. Closes #3671 > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9eb473e056 |
feat(list): name a detached row by its short hash, not - (#3675)
The Branch cell hardcoded `"-"` for a worktree with no branch. It reads as missing data rather than as a state, and it was the odd one out: the skeleton row, the statusline, and `worktree_display_name` all reach for `branch_name()`'s `"(detached)"`, so the same cell changed label as the row settled. Detached worktrees aren't exotic here any more — Codex creates one per session under `~/.codex/worktrees/`, and they sit in `wt list` alongside everything else. The cell now carries the row's abbreviated HEAD in dim yellow. Yellow keeps it from reading as a branch that happens to be named like a SHA; dim keeps a row that isn't on a branch quieter than one that is. `should_dim`'s removable dim still reaches the row's Path and Message cells, so that signal survives the override. ``` Branch Status Path Commit Branch Status Path Commit @ main ^| . 1243e9c0 @ main ^| . 1243e9c0 + - ! ⚑↓ ../codex/… bdc5c663 → + bdc5c663 ! ⚑↓ ../codex/… bdc5c663 + 1p ⊂ ../wt.1p bdc5c663 + 1p ⊂ ../wt.1p bdc5c663 ``` ### What to look at `display_name()` gains a HEAD-prefix fallback so it answers before the `%h` batch lands post-skeleton — the skeleton and settled rows now print the same text in the same style, with no restyle as the row fills in. Both the Branch cell of a detached row and the Commit cell of every row render the new `abbreviated_head()`, so one commit gets one spelling: sourcing the Branch cell from `short_sha` (`core.abbrev`-aware) instead put `1b9f1d9` beside the Commit column's `1b9f1d96` on the same row. The Branch column budgets `COMMIT_HASH_WIDTH` when any row is detached. Sized off branch names alone it truncated the hash — and it already truncated the skeleton's `(detached)` to `(detac` behind a short branch set, so that was a latent bug rather than a new constraint. The picker's matcher text and the statusline follow the display. A detached row now filters by the hash on screen rather than a `"(detached)"` token that matches nothing visible and collapses every detached row onto one key, and a prompt names the same worktree the same way its `wt list` row does. `⚑` on a detached row stays as it was. The flag's axis is "not at home", and a worktree with no branch has no home path to be at — but the docs described only "branch name doesn't match the worktree path", which doesn't cover the case that has no branch at all. They now name it. ### Testing Covered by the existing detached-head list snapshots (all four now show the hash), a new layout test pinning the column width against a short branch set, a unit test for the `display_name` / `abbreviated_head` pair across the skeleton boundary, and the statusline detached test rewritten to assert the hash. Full suite green locally: 4493 tests, clippy and pre-commit clean. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
|
1eded27943 |
Simplify forge, shell integration, and removal internals (#3662)
This consolidates cross-cutting models that had accumulated parallel representations, while preserving the CLI and config interfaces. ## What changed - Forge identity now flows through one `ForgeKind`, with boundary-safe network-host classification shared by CI, remote references, and structured repository metadata. Branded SSH aliases such as `github-personal` remain outside provider dispatch and receive an actionable `forge.platform` diagnostic when forge data is requested. - Worktree removal now uses one owned target type and rechecks uncached worktree topology immediately before compare-and-swap branch deletion, retaining branches that gained a live or locked checkout. - Shell integration no longer implements the retired single-file directive writer. Stale wrappers receive repair guidance, execution fails closed, and child processes cannot inherit the retired sourceable-file capability. - Zsh and Git-version probes are shared, while dead mocks, redundant dependency edges, serializer detours, pass-through types, and duplicate tests are removed. The removal guard deliberately distinguishes live, stale-prunable, and locked registrations. The detached cleanup path performs the same record-aware check before deleting a branch ref. ## Testing `cargo run -- hook pre-merge --yes` passed after merging current `origin/main`: 4,489 tests, Clippy, formatting, lockfile checks, docs, doctests, and snapshot review. > _This was written by Claude Code on behalf of max_. |
||
|
|
6d09125b7b |
test: converge suite on semantic boundaries (#3663)
This follows the first test-simplification tranche by converging the remaining suite around distinct semantic and pragmatic contracts rather than raw case count. The branch removes false-confidence tests, invalid setup variants, repetitive snapshots, and expensive PTY overlap while strengthening the retained route, precondition, and interaction proofs. ## What changed - Replace obsolete CI-status integration mocks and blank snapshots with direct provider semantics, mixed-priority cases, and strict GitHub/GitLab route assertions. - Remove free-riding merge, push, remove, list, security, config, and switch cases whose setup never reached the named behavior; consolidate repetitive direct cases into labeled tables. - Reduce the switch picker from 42 PTYs to 19 distinct terminal contracts, using causal release gates for asynchronous loading and repaint behavior. - Add a cached main-only picker fixture, eliminating 138 unnecessary Git subprocesses across the retained PTYs, and integrate it with main's generated hermetic standard fixture. - Tighten test guidance around proving setup preconditions and mock invocation routes, and correct the comments-tab help text and generated mirrors. ## Reviewer map - `tests/integration_tests/ci_status.rs` and `src/commands/list/ci_status/`: provider semantics and route coverage. - `tests/integration_tests/switch_picker.rs`, `src/commands/picker/`, and `src/testing/`: retained PTY contracts, causal mocks, and fixture design. - `tests/integration_tests/config_show.rs`, `src/config/deprecation.rs`, `src/config/expansion.rs`, and worktree type/resolve tests: direct-boundary consolidation. - `tests/CLAUDE.md`: the testing rules extracted from the false-confidence cases found during the survey. The measured loop removed 98 tests, 65 snapshots, and 23 picker PTYs. Controlled warm Nextest execution improved from a 79.593-second mean to 72.574 seconds (8.8%), while comparable production-line coverage moved from 97.32% to 97.23%. The tracked PR diff is a net deletion of more than 5,700 lines. ## Validation - `cargo run -- hook pre-merge --yes` after syncing current `main`: 4,468 passed, one configured skip; docs, doctests, clippy, formatting, policy checks, and snapshots green. - `task coverage` on the completed change before the base sync: 4,465 passed, one configured skip; 97.23% comparable production-line coverage. - Three independent final audits found no remaining lost beliefs, fixture hazards, or safe PTY consolidations. > _This was written by Claude Code on behalf of max_. |
||
|
|
cfd6f099bc |
fix(codex): clear activity marker on session end (#3660)
Codex now exposes `SessionEnd`, but Worktrunk’s Codex plugin only returned the activity marker to idle at turn end. This adds a main-session exit hook that clears the marker, using Codex’s three-second maximum hook timeout so cleanup has the best chance to complete without delaying shutdown. The CLI help, plugin-layout guidance, user documentation, generated mirrors, metadata test, and help snapshot now describe and verify the complete Codex lifecycle. Tested with `cargo run -- hook pre-merge --yes` (4,564 tests passed; 1 skipped). > _This was written by Claude Code on behalf of max_. |
||
|
|
79824f7122 |
feat(styling): underline every hyperlink (#3643)
The statusline underlined its PR reference but not the dev-server port, so nothing marked the port as clickable: ``` ~/w/worktrunk.test-suite-cpu ↕|💬 ↑17 ↓11 ^+585 -258 #3604 :11486 Fable 5 🌕 30% ``` Both links now route through a shared `worktrunk::styling::hyperlink(url, text)`, which emits the OSC 8 sequence and the underline together. It closes with `[24m` rather than a full reset, so a wrapping color (the CI verdict) or dim (a port nothing answers on) survives the link. The rule is recorded as policy in the `writing-user-outputs` skill. Link text is sized to fit a column (`#3604`, `:11486`) and reads as ordinary content, and color already carries state, so the underline is the only thing marking text as clickable. Text that is not a link stays plain: on a terminal without OSC 8 support, `wt list` still prints the dev-server URL in full, unadorned. Tests: a unit test pins the helper output, and a statusline test asserts both segments carry a helper-built link. Snapshots updated for the reordered escapes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a27cbd42be |
feat(skill): create the worktree by name, fall back to a path (#3636)
Closes #3555. `/wt-switch-create` now creates through `EnterWorktree({name})` for the common invocation, which asks the user to confirm nothing, and falls back to `wt` plus a path entry for the cases that route can't serve. The confirmation Claude Code 2.1.206 added fires on a call that passes `path` and targets a worktree outside `.claude/worktrees/`. `{name}` passes no `path`, so it never fires. With the plugin installed, `{name}` runs worktrunk's own `WorktreeCreate` hook (`wt switch --create`), so the worktree is the one `wt` would have made, and it shows up in `wt list`. ## Routes - **New branch, this repo** → `EnterWorktree({name})`. No confirmation. - **Anything else** → `wt -C <repo> switch --create … --format=json`, then `EnterWorktree({path})`, keeping the outcome handling below. Three things `{name}` cannot do, each announcing its own fallback in the error it returns, so the skill tries the cheap call and reads the result rather than pre-checking: | Case | What `{name}` returns | |---|---| | Repo argument given | no `-C` equivalent; skipped before the call | | Branch already exists | the hook exits nonzero with `✗ Branch <branch> already exists` | | Session already entered a worktree | `Already in a worktree session. Pass `path` to switch into another existing worktree` | ## The tradeoff, taken deliberately A `{name}` worktree the session never touched is removed when the session ends, branch included, through the plugin's `WorktreeRemove` hook (`wt remove`). Verified end to end: create, `/exit`, and neither the worktree nor the branch remains; one untracked file is enough to keep both. That is wanted at this scale, since a research task that wrote nothing leaves nothing to prune, but it does mean "the worktree persists" was no longer true of every route. Cleanup and the rationale now say which worktrees persist; the user docs stay out of the mechanics. ## A bug fixed along the way A user declining the path-entry confirmation returns an ordinary permission denial, which the skill's single rejection branch caught, and that branch's documented recovery is to `cd` into the worktree and work there. So a declined entry became a worktree entered anyway. Three wordings that asked the agent to classify the denial text failed context-blind probes in turn: branches labeled by who refused, "the denial reports a decision", and that plus the verbatim denial string quoted as an example. Entry outcomes now split structurally instead. The tool's own `Cannot enter` errors key the reachability test; a denial of the call stops and asks by default, with one exception: a session with no user to ask (its denial says it couldn't prompt) takes the recovery, since nothing was decided. A recovery that follows a denial invites the model to route any denial into it, so the denial branch leads with stopping. Context-blind agents route both denial directions and the four invocation shapes above through their intended routes. ## Verification All of it re-run live against Claude Code 2.1.220 in scratch repos, since the analysis in #3555 rested on properties pinned to 2.1.173/177 and could not be checked from CI. Two claims died that way: an earlier draft credited the exit removal to the harness when it is worktrunk's own hook, and asserted that a `permissions.deny` rule produces a classifiable denial, when it removes the tool from the session entirely. Also recorded in the rationale: the confirmation offers no always-allow and `permissions.allow` cannot suppress it, it fires in `default`, `acceptEdits`, and `auto` while `bypassPermissions` allows silently, and a session that cannot prompt denies without asking. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Fable 5 (1M context) <noreply@anthropic.com> |
||
|
|
9e7ad0144d |
fix(hooks): expand everything but vars.* in a preview (#3638)
Follow-up to #3635, from two reviews that landed after it merged. ## The `vars.*` preview was worse than #3635 claimed `render_template_preview` short-circuits on `template_references_var(template, "vars")`, returning the raw template. So one `vars.` token disabled expansion for the entire command: ``` pre-commit = "deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }}" before: deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }} after: deploy --branch=main --repo=repo --env={{ vars.env }} ``` #3635 described this as "a `vars.*` template renders raw", which is true but understates it: `{{ branch }}` and `{{ repo }}` stopped expanding too, in a command whose whole job is to show the expansion. That short-circuit predates #3635 and has been degrading `wt hook <type> --dry-run` the same way; #3635 only extended it to `wt hook show --expanded`. The fix is at the source rather than at either caller. A preview now injects a stand-in object for `vars` that renders each reference back as itself, nested access included (`{{ vars.config.port }}` round-trips), while every other variable expands normally. `VarsMode::Resolve` keeps execution reading real values from git config; only previews pass `VarsMode::Literal`. A preview also no longer spawns the git read that resolving `vars` required. `vars.*` stays literal on purpose: those values are read when the step runs, after an earlier step in the pipeline may have written them, so a value resolved at preview time can differ from the one the run uses. Nothing covered this, which is why the suite stayed green through the regression. `test_hook_show_expanded_matches_dry_run` now sets a var and asserts the listing and the dry-run both leave it alone while expanding `{{ branch }}` beside it. ## The syntax gate is a type error now #3635 moved the template syntax check out of `prepare_steps` into a free `validate_pipeline_syntax` that both execution funnels had to remember to call. `prepare_steps` now returns a `PreparedPipeline` the caller must resolve: `.validated()` for the paths that run hooks, `.into_unvalidated()` for the listing, which annotates a broken template in place rather than blanking itself. Forgetting is a compile error, the same property `ApprovedHookPlan` gives hook approval. ## Smaller items Four cross-references went stale when the syntax check moved: `PreparedCommand.template`, `validate_template_syntax`, the `switch.rs` skip comment, and `HOOK_INFRASTRUCTURE_VARS` (which still named two deleted functions). The `--expanded` behavior is now documented in the sentence that already owns `{{ vars.<key> }}` semantics, with its three generated mirrors regenerated. `PreparedStep::commands()` replaces two hand-rolled matches in `hooks.rs`. `default_branch` moves inside `build_manual_hook_template_vars` — only the commit-hook arm reads it, and resolving it can cost a `git ls-remote` on a fresh clone, so the other eight hook types no longer pay for it. The listing carries its expansion state in an `Option<String>` instead of re-deriving "was this expanded?" from whether a context exists. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4af04c2260 |
fix(remove, prune, merge): target the named worktree, retain shared branches (#3533)
Follow-up to the discussion on #3480: [@max-sixty asked](https://github.com/max-sixty/worktrunk/pull/3480#issuecomment-5039137116) to have `wt remove` honor an explicit path generally. #3607 has since landed the resolution half, `Repository::resolve_worktree`, which turns a path into a worktree. This is the removal half, and it is the part resolution cannot decide: which worktree to act on is a naming question, whether the branch may be deleted is not. ## Problem `wt remove` threw the resolved answer away. For any non-current worktree it re-targeted by branch name, and `prepare_worktree_removal` mapped that branch back to git's *first-listed* worktree. Two failures followed, both silent. **The wrong worktree is removed.** With `feature` checked out twice: ```console $ git worktree list …/dup [feature] …/first [feature] $ wt remove …/first ◎ Removing feature worktree & branch in background (same commit as main, _) ``` `…/dup` is gone. `…/first`, the one named, is still there. **The survivor is left broken.** The shared branch is deleted with it. Worktrunk deletes branches with `git update-ref -d`, git's compare-and-swap primitive, which unlike `git branch -d` does not refuse a ref that is checked out somewhere: ```console $ git worktree list …/first 0000000 [feature] $ git -C …/first rev-parse HEAD fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree. ``` `wt step prune` reached the same deletion unattended, and `wt merge` reached it with a freshly integrated branch, so nothing else declined. A branch gets a second worktree only through `git worktree add --force`; worktrunk never does it itself. ## Fix **Remove the worktree that was named.** `wt remove` drops non-current worktrees via `RemoveTarget::Path`. `wt step prune` does the same: its candidates already carry a path, and targeting a *stale* entry by branch name resolved to a live worktree that the same prune had just skipped as too young, then removed it. **Retain a branch another worktree holds.** One predicate, `live_sibling_checkout`, answers "would deleting this ref orphan a checkout?", and every path that can delete a branch asks it: `prepare_worktree_removal`'s worktree and pruned-branch-only arms (covering `wt remove`, `wt step prune`, and the picker, which already targeted by path) and `wt merge`'s finish. A hit forces `BranchDeletionMode::Keep`, the single chokepoint every deletion path honors, and names the surviving checkout: ```console $ wt remove …/dup ◎ Removing feature worktree in background ○ Branch feature retained; still checked out @ …/first ``` A sibling whose *directory* is already gone is stale metadata, not a checkout with anything to lose, so it does not retain: removing the last live checkout still deletes the branch. **`-D` is refused out loud.** Everywhere else `-D` is the override that wins, so one that cannot be honored warns rather than passing quietly: ```console $ wt remove …/dup -D ◎ Removing feature worktree in background ▲ Branch feature retained despite -D; still checked out @ …/first ``` The ordinary single-checkout case is unchanged, and a retained branch skips the integration check entirely rather than computing a verdict it would discard. #3480's duplicate-checkout hint now points at `wt remove <path>`, which this makes the safe answer. ## Testing Full gate green. New coverage, each case asserting the survivor still resolves `HEAD`, which is the corruption in question: - `remove`: by path, by name, refused `-D`, the pruned-directory fallback, and the mirror case where a stale sibling must *not* retain. - `step prune`: a stale entry whose branch is live in an age-skipped worktree. This test is what surfaced the wrong-worktree bug in prune. - `merge`: merging a branch that a `--force` duplicate also holds. ## Not addressed `wt step prune`'s summary counts candidates rather than outcomes, so a retained branch still reports `✓ Pruned 1 branch`. The per-item line above it already says the branch was retained. Fixing the count means threading removal outcomes back through prune's accounting, which is a separate change. > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
63071709af |
feat(config): deprecate list.task-timeout-ms (#3615)
Removes the `[list] task-timeout-ms` per-task command timeout and the thread-local machinery behind it. The key stops having any effect immediately; a config that still carries it loads, warns, and is stripped by `wt config update`. The timeout killed any git command that outlived its budget, on every collect worker, through a thread-local that `Cmd::run` consulted on each invocation and clamped an explicit `.timeout()` against. Progressive rendering removed the reason for it: `wt list` and the picker paint from local data and stream results in behind the frame, so no single git command can hold up the first paint. What it bounded instead was completion, which `[list] timeout-ms` already bounds directly, and the drain falls back to a hardcoded 120s `DRAIN_TIMEOUT` whenever that is unset (`collect/mod.rs` `drain_deadline`), so removing this cannot introduce an unbounded wait. Both keys default to unset, so the default path never had a per-task timeout at all. `[list] timeout-ms`, the wall-clock budget for the whole collect phase, stays. It is the surviving knob and the more direct expression of the same goal. With the thread-local gone, the two-source `min()` in `Cmd::run` collapses to the command's own `self.timeout`, so every explicit `.timeout()` caller keeps its bound unclamped: `PROBE_TIMEOUT` in `git/reap.rs`, the fsmonitor stop/lsof bounds in `git/remove.rs`, the version check in `config/show.rs`, and `REMOTE_DETECTION_TIMEOUT`. ## Deprecation A `Structural` row in `DEPRECATION_RULES` strips the key from `[list]` in both the section and inline forms, top-level and per-project, following the `[switch.picker] timeout-ms` precedent (also a strip with no equivalent key to migrate into): ``` ▲ User config: list.task-timeout-ms is no longer used — list.timeout-ms bounds the collect phase ``` The env overlay (`WORKTRUNK__LIST__TASK_TIMEOUT_MS`) and `--config-set` route through the same rule and migrate silently, since neither layer has a file for `wt config update` to materialize. Neither errors. ## Testing New unit tests cover detection and migration for the section, inline, and per-project forms plus the warning text, and two cases join the `test_warning_fires_iff_update_changes` battery that pins the warn-iff-update-changes invariant. The three integration tests that exercised the feature are gone; `Cmd::timeout` keeps its own coverage in `shell_exec.rs`, so dropping the four thread-local unit tests loses nothing for the surviving path. Verified end to end that a config carrying the key loads, warns, and has it stripped by `wt config update` with sibling keys intact. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
14580de79c |
feat(worktree): accept a worktree path wherever a branch is accepted (#3607)
Follow-up to the [`wt remove <path>` discussion on #3480](https://github.com/max-sixty/worktrunk/pull/3480#issuecomment-5039137116), widened from that one command to the whole surface. #3480 has since landed and is merged in here — its duplicate-checkout warning composes with this: the warning names the shadowed worktrees, and a path is how you then address one. ## Audit Verified against the built binary. wt had three answers to "what does this token mean?": | Route | `@` `-` `^` | worktree path | `pr:N` | |---|---|---|---| | `wt switch` (`resolve_switch_target`) | yes | only if absolute or ≥2 components, and not `--create` | yes | | `wt remove` (`resolve_worktree_arg`) | yes | any token | no | | everything else (raw `worktree_for_branch`) | **no** | **no** | no | Same token, same cwd, two answers: ```console $ wt remove inner # ✓ Removed innerbranch worktree & branch $ wt switch inner # ✗ No branch named inner ``` And outside switch/remove the shortcuts didn't work at all — `wt step diff --branch @` was `✗ Branch @ has no worktree`, while `wt config state marker set --branch @` silently wrote state under the literal key `@`. Separately, wt prints paths as `~/…` but wouldn't accept that form back. ## Change One canonicalizer in the lib, `Repository::resolve_worktree`, absorbing the path fallback that lived in the bin crate's `resolve_worktree_arg` (now deleted). Resolution order is documented once, on that function: `@`, then `-`/`^`, then a branch with a worktree, then a path naming a registered worktree, then the branch alone. **Branch-first, everywhere.** A directory never shadows a branch that shares its name; a path answers only what a branch cannot — a detached worktree, or one of two checkouts of the same branch (#3480's case). The `looks_like_path` shape gate is gone, so a single-component path resolves like any other. Two shapes cover what callers need: `require_worktree` for commands that need a worktree to operate in, `require_selected_branch` for arguments that key by branch. The merge/rebase target validators fall through to the same path lookup, so a target can be named by the worktree it's checked out in. Routed through it: `switch` (including `--base`), `remove`, `step commit --branch`, `step diff --branch` and its target, `step copy-ignored --from`/`--to`, `step promote`, `step relocate`, `config state --branch` (9 sites), and `merge` / `step rebase` / `step squash` / `step push` targets. `resolve_input_path` — already documented as the one resolution point for user-supplied paths — now expands a leading `~`, so the tilde form worktrunk prints is a form it reads back. `~user` stays literal; wt doesn't reimplement that shell feature. ## Documentation A path is an alias, not a second addressing scheme, so it is stated once rather than on every argument: one paragraph in `wt switch`'s help and one sentence on the addressing line in `worktrunk.md`. Argument descriptions still read as branches. The two exceptions are the arguments whose descriptions are already catalogues of accepted forms — `wt switch`'s (`Branch, worktree path, shortcut, or PR/MR URL`) and `wt remove`'s, which has named the path since before this branch. The Worktree Model section of `CLAUDE.md` records which way to document it, so the next argument doesn't grow its own copy. ## Two silent no-ops fixed along the way - `wt step relocate <unmatched>` matched arguments against branch names by string equality, so a typo filtered everything out and the empty result rendered as `○ All worktrees are at expected paths` — a success message for work that never happened. Every way an argument can fail to land on a relocatable worktree now errors, including the detached and prunable cases the new path route makes reachable. - A selector matching nothing was reported as a branch without a worktree, hinting `wt switch <token>` — which creates a worktree only when the branch exists, so for a mistyped path it would just fail again. `WorktreeSelectorNotFound` now says `No branch or worktree named X`; a branch that genuinely exists without a checkout keeps the create hint. ## Testing Full gate green: 4596 tests, lints, docs sync, `--features shell-integration-tests` clippy. `codecov/patch` is 99.25% of diff hit against a 97.93% target. New coverage: - Unit: branch-and-path equivalence, branch-beats-same-named-directory, detached-by-path (and its `require_selected_branch` refusal), shortcuts never treated as paths, branch-only fallthrough, and the two distinct not-found errors. Plus `expand_tilde` round-tripping `format_path_for_display`. - Integration: `switch` by relative/single-component/absolute/tilde path, `--base` by path, `step diff --branch` by path and `@` (asserted equal to the by-branch output), `config state --branch` set via `@` and read via the worktree path, and both new relocate errors. `wt remove`'s resolution is unchanged — it already had this rule; it now shares the implementation. The 106-test `remove::` suite is untouched and green. - Integration: `wt step push <worktree-path>` (the `require_target_branch` half of the target fallback), and `wt step relocate` against a prunable worktree. One diff line is unhit: `expand_tilde`'s fallback when `home_dir()` returns `None`, which has no deterministic trigger. The `@`-resolution backstop in `resolve_worktree` is untested for the same reason — no CLI route reaches it — so it kept its original `match` arm rather than being re-indented into the diff. ## Left out `wt config state default-branch set` and `previous-branch set` take a branch name as a *value to store* rather than a selector, so they still take it literally. > _This was written by Claude Code on behalf of Maximilian Roos_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
04d8d587f2 |
feat(list): flag a branch checked out in more than one worktree (#3606)
## Problem Follow-up to #3480. That PR made a duplicated branch checkout (`git worktree add --force <path> <branch>`) visible at *resolution* time: `worktree_for_branch` warns once per branch, then resolves to whichever worktree git lists first. `wt list` said nothing about it. Two rows named `feature`, and no column that explains why. The nearest thing to a signal was accidental. A force-added duplicate usually lands off-template, since the original holds the template path, so it picks up `⚑` for the location mismatch — while the worktree *at* the template path, the one `wt` actually resolves to, carried no flag at all. Exactly backwards from what's useful. The framing from the request: a worktree in the wrong location gets a status flag; a worktree sharing its branch should get one too. ## Solution `⚑` now covers both, on every worktree of the duplicated branch, resolved one included. Which worktree `wt` picks is git's listing order, so singling out the shadowed rows would imply a legitimacy the ordering doesn't carry. ``` @ main ^| | . 05a4a45d 16h Initial commit + feature ⚑_ ../repo.feature 05a4a45d 16h Initial commit + feature ⚑_ ../repo.feature-dup 05a4a45d 16h Initial commit ``` This started as a seventh glyph (`⧉`) and collapsed onto `⚑` in the second commit. The Status column is a dense alphabet the reader has to learn, and the two states say one thing: this worktree's place in the branch ⇔ worktree map is irregular. Off-template path and branch-claimed-twice are both instances. The table already distinguishes them without a glyph — a repeated Branch cell is the duplicate, an odd Path cell the mismatch — so the flag only has to say "not a rendering glitch, look at the Path column". Sharing the glyph means sharing its dim-yellow styling, since the codebase treats a symbol's color as part of its identity; #3480's warning remains the loud channel, firing the moment any command resolves the branch. **The Path column comes along.** It previously appeared only for a location mismatch, on the reasoning that the path is otherwise redundant with the branch. A duplicate inverts that: the branch name no longer identifies the row, and the path is the only thing telling the two apart. The layout flag is renamed from `has_branch_worktree_mismatch` to `path_is_informative` to say what it now means. **The data model keeps the distinction.** JSON has no cardinality budget, and reporting a duplicate that sits at the template path as `branch_worktree_mismatch` would be false — its path does match. Schema 1's `worktree.state` names the cause (`"duplicate_branch"`), schema 2 gets its own `worktree.duplicate_branch` bool beside `branch_mismatch`, matching that schema's one-fact-per-field shape. The priority between the two `⚑` states now decides only which cause JSON reports. Detection is one pass over the worktree list (`duplicated_branches`, next to #3480's `worktree_paths_for_branch`), in memory, pre-skeleton, no git calls. ## Testing - `test_worktree_paths_for_branch_detects_duplicates` gains the set form and a detached-HEAD worktree, which has no branch to duplicate. - `test_metadata_worktree_state_priority` covers the two `⚑` states' ordering and both yielding to `⊟`/`⊞`. - `test_list_duplicate_branch` snapshots the table above, showing both flagged rows and the Path column earning its place. - `test_list_duplicate_branch_json` asserts both schemas flag exactly the two duplicated rows. The flag only fires on a state no prior test sets up, so the only snapshot churn is the help pages and the schema-2 envelope's new field. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9645e3e132 |
fix(shell): reclaim the legacy wrapper paths instead of inspecting them (#3602)
`wt config shell install` decided whether the file at a legacy location
was worktrunk's by reading it — a substring test for the wrapper header,
falling back to "every code line looks like an integration line".
Anything that didn't match was left in place. Fish sources `conf.d` at
startup, so whatever `conf.d/{cmd}.fish` defines is already loaded by
the time fish would autoload the `functions/{cmd}.fish` the install just
wrote — the stale definition wins and the new wrapper may never load.
Ownership is now the path. `conf.d/{cmd}.fish` and the stranded nushell
`{cmd}.nu` candidates are paths worktrunk itself computes for the
command name being installed, so install takes them back whole without
reading them. Only that exact filename is touched: a neighbour under
another name is not worktrunk's, and each removal is still reported.
`wt config shell uninstall` still reads the header, because it takes no
`--cmd` and so genuinely doesn't know the name — it lists the
shell-owned directories and has to tell our `{cmd}.fish` from the user's
own files beside it. That is the one place ownership can't come from the
path, and it prompts and previews every file before removing it, which
install does neither of. `is_worktrunk_managed_content`'s docstring now
says so; `is_worktrunk_managed_nushell` existed only to gate the
install-time deletion and is gone.
Audited the rest of the surface for the same principle: the OpenCode
plugin (`~/.config/opencode/plugins/worktrunk.ts`) is already removed by
path with no inspection, the Claude plugin delegates to `claude plugin
uninstall`, Gemini installs through `gemini extensions install` and
leaves nothing of ours on disk, and fish is the only shell with a
separate completion file — which uninstall already covers. No other
gaps.
## Data safety
This deliberately widens what install deletes, so it's worth naming: two
tests asserted the old conservatism and now assert the new boundary —
`test_configure_shell_fish_reclaims_conf_d_path` (was
`..._preserves_user_conf_d_file`, from #3589) and
`test_nushell_install_reclaims_only_the_command_name` (was
`..._keeps_unmanaged_legacy_file`, from #2992). Both put a neighbour
file in the same directory to pin that only `{cmd}.{ext}` is taken. The
FAQ's "What can Worktrunk delete?" inventory is updated to describe
path-based ownership rather than the old content-marker rule.
> _This was written by Claude Code on behalf of max_
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
8865f20ab8 |
fix(git): bound the default-branch remote query, and make the bound bound (#3603)
Follow-up to #3596, which walled the test suite off from the network. That PR left the same unbounded call live for real users: `Repository::default_branch()` is the one detection helper allowed to fall through to `git ls-remote`, and nothing limited how long it could take *not* to answer — an unanswered SYN costs ~127 s per address on Linux (`tcp_syn_retries=6`) and git tries each of a host's addresses in turn, so a remote behind a dropped VPN or a dead host stalled `wt list --full` and `wt switch` for minutes. ## `Cmd::timeout` didn't bound wall-clock Adding a bound alone would have been decorative, which is the substance of this PR. `run_with_timeout_impl` killed only the direct child, and a grandchild inherits the child's stderr pipe — so a surviving one held the write end open and `read_to_end` blocked until it exited. Measured on a 3 s timeout over an `ls-remote` whose upload-pack sleeps 120 s: | | elapsed | |---|---| | before | 120.03 s | | after | 3.20 s | `git-remote-https` sitting in `connect()` is exactly that shape: it doesn't notice git died. So a timed child spawns into its own process group and expiry tears down the group — `killpg` with TERM → KILL escalation on Unix, `taskkill /T /F` on Windows, matching `wt step tether`. Every existing `.timeout()` caller (the fsmonitor stop/lsof probes, `reap.rs`, the shell probe) was latently unbounded the same way and is fixed with it. The isolation costs the kernel's tty broadcast: a Ctrl-C no longer reaches a timed child, so the user waits out the remaining bound. That's seconds, against an orphan holding a pipe for as long as its own operation takes, once per spawn. Recorded at `run_with_timeout_impl` and in CLAUDE.md's Signal Handling section, whose "only the current child does" claim was no longer true. ## The cache is the other half A timed-out query takes the local-inference fallback but is *not* persisted to `worktrunk.default-branch`. That cache is what stops later calls from re-detecting, so a guess made while the network was down would otherwise become the repo's permanent answer. `detect_from_remote` now returns a `RemoteDetection` enum so the cacheability decision is explicit and exhaustive rather than an `Option` that loses the distinction. Only a timeout separates cleanly, via `ErrorKind::TimedOut`. `ls-remote` exits 128 whether the network is down or the remote simply has no HEAD, so telling those apart would mean reading git's error text — and re-querying on every command is the cost the cache exists to avoid. Those stay cached, as before. ## Reviewing - `src/shell_exec.rs` — the process-group teardown; its docstring carries the rationale - `src/git/repository/config.rs` — `RemoteDetection`, the 10 s bound, and the no-persist path - `src/git/repository/mod.rs` — `run_command` now delegates to a `run_command_bounded` that takes the bound Also routes the four remaining hand-rolled git test envs (`src/git/remove.rs`, `src/git/repository/tests.rs`, `tests/integration_tests/bare_repository.rs`) through `configure_git_env`, so they carry #3596's `GIT_ALLOW_PROTOCOL` deny rather than re-deriving a subset of it. All four run local-only git commands today, so this is completeness, not a live hole — and a full suite run under `GIT_TRACE` confirms zero `git-remote-*` transport-helper spawns across 4374 tests. ## Testing Three tests, none of which touch the network: the grandchild case via `sh -c 'sleep 30; :'` (which stops the shell `exec`ing sleep, so there really is a grandchild), the end-to-end no-persist behavior via `remote.origin.uploadpack` pointed at a `sleep`, and the fail-fast unresolvable remote, which had no coverage before. The second waits out the real 10 s bound; both hanging-remote tests are `#[cfg(unix)]` since the vehicle needs a POSIX `sleep`, so `test (windows)` doesn't cover the no-persist path. > _This was written by Claude Code on behalf of max_ |
||
|
|
b843051906 |
docs(faq): scope the worktree-lock advice to removal (#3601)
The FAQ recommended `git worktree lock` "for worktrees containing
precious ignored data (databases, caches, large assets)". The lock does
not do that — it blocks removal and nothing else. Someone who locked a
worktree to keep a local database safe has protection against `wt remove
--force`, and none at all against the thing that actually rewrites files
there.
An ignored file in a locked worktree is still replaced when commits
arriving via `wt merge` or `wt step push` track a file at the same path.
Reproduced against this FAQ's own example: a locked worktree holding
`db.sqlite` had it overwritten by the branch's tracked file, exit 0, no
warning.
That behavior is git's, not worktrunk's. A plain `git merge` in that
worktree does the same, and draws the line in the same place — it
refuses when the colliding file is untracked, and overwrites when it's
ignored:
```
ignored locally → Fast-forward, db.sqlite | 1 + (content: TRACKED-FROM-BRANCH)
untracked locally → error: The following untracked working tree files
would be overwritten by merge (content: REAL-LOCAL-DATABASE)
```
So the fix is one sentence: the opener now says what the lock buys
(removal protection) instead of implying it guards ignored data. Nothing
in the paragraph below it changes — `wt remove --force` and `git
worktree remove --force` do both still refuse on a locked worktree,
checked against a scratch repo while editing.
> _This was written by Claude Code on behalf of Maximilian Roos_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
||
|
|
b623d7f951 |
fix(shell): don't delete user data that only mentions the init command
Three places decided whether text was worktrunk's by testing a blob for
substrings. Two of them deleted the user's data when they guessed wrong.
`wt config shell uninstall` asked two unlinked questions of an rc line —
is an exec keyword anywhere in it, and is `<name> config shell init`
anywhere in it — so `alias setup='echo "run: wt config shell init fish |
source"'` classified as worktrunk's and was removed. One question
replaces both: does the line invoke the init command, with the name in
command position? The exec-keyword scan is gone rather than kept
alongside; it read English words out of the whole line and rejected
nothing the positional rule doesn't.
`wt config shell install` had the same defect one file over and one step
worse: `is_worktrunk_managed_content` tested the whole file for
`config shell init` and `| source`, and `cleanup_legacy_fish_conf_d`
deleted a user's own `conf.d/wt.fish` outright — during an install, which
never offered to remove anything. The cmd-agnostic twin beside it already
answered correctly, so the substring version is gone.
Uninstall now prints every line it takes, in the confirmation and again
after removal, since shell quoting stays undecidable without running the
line.
Sweeping for the same shape found it in `az` failure classification,
where `stderr.contains("login")` alone meant "not authenticated" and
`bail!` discarded the real error; `az` states the remedy itself, so its
text now reaches the user. Provider host matching moved from
`contains("github")` to label-boundary comparison, so
`dev.azure.com.attacker.example` and `mygithubmirror.com` no longer
resolve to Azure DevOps and GitHub.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
b17a1364f1 |
docs(step): render wt step rebase & wt step push on the docs site (#3578)
Follow-up from #3568, which named these two as the one plain oversight
in its audit of `after_long_help` bodies that never reach a docs page.
`wt step rebase` and `wt step push` were the only two of twelve step
operations with no `<!-- subdoc: -->` marker, so their help was
terminal-only, and the only two bullets in `## Operations` left
unlinked. Both markers are added between `squash` and `diff`, per the
ordering invariant in `src/cli/step.rs`.
Both openers restated their `about` line, which reads as immediate
self-repetition on the web where `combine_command_docs()` concatenates
the two. Both bodies are rewritten.
## What reviewing the text against the code turned up
The old rebase body said "Conflicts abort immediately; use `git rebase
--abort` to recover", which is self-contradictory and wrong. Nothing
aborts: the worktree is left mid-rebase with git's conflict markers, and
git's output names `--continue`, `--skip`, and `--abort`. `wt merge`'s
pipeline step 3 carried the same sentence.
Four review passes then falsified nine more claims, every one reproduced
against a scratch repo before editing:
- **`--no-ff` runs no `git push` at all.** It builds the merge with
`commit-tree` + `update-ref` and syncs the worktree with `read-tree`; a
`-vv` trace shows zero `git push` invocations. The opener attributed the
whole command to `git push` with `--no-ff` as an example four lines
below. Its worktree sync is also best-effort, so the ref can move while
the worktree stays behind.
- **The Outcomes table was falsified by the upstream-swap topology.**
With local `main` diverged and behind `origin/main`, `wt step rebase
main` prints `Already up to date with origin/main` — `main` is not an
ancestor of the branch, and the result names a ref never typed.
- **"Nothing here reaches a remote" was false on a fresh clone.** With
no target argument and no cached `worktrunk.default-branch`,
`default_branch()` falls through to `git ls-remote`. The claim is now
"no commits leave the repository", which holds unconditionally.
- **The generated Arguments table contradicted the prose.** `[TARGET]
Target branch` sat two paragraphs below "the target is any commit-ish";
a blind reader nearly concluded rebase needs a branch name.
- **The table implied mutually exclusive rows.** `is_rebased_onto` is
checked first, and a branch at the target's tip satisfies both of the
first two.
- Plus: an orphan branch matched no row; "git's own output names the
ways out" is false under `advice.mergeConflict false`; "refused rather
than forced" raised the force question without closing it.
Adjacent, in files this change already touched: `wt merge`'s step 3
contradicted the new table and step 5 restated `wt step push`'s rule
with no link, so half the pipeline deferred and half repeated. Step 2
promised a backup ref unconditionally, but `create_safety_backup` runs
only when working-tree changes were staged — a clean-tree squash
rewrites commits and writes no `refs/wt-backup/` ref. That is a
data-safety claim, so it is corrected rather than left.
`docs/CLAUDE.md`'s subdoc "Use cases" cited `wt config create`, which
has no marker and no section.
## The code changes
**An annotated-tag target was never peeled.** Adding a tag example to
the rebase help is what made it worth running, and it did not work.
`is_rebased_onto` compared `git merge-base`, which peels an annotated
tag to its commit, against a bare `rev-parse`, which returns the tag
object's SHA. Those never match, so an annotated-tag target always
looked like it needed a rebase:
```
annotated v1.2.0 → "outcome": "rebased" (HEAD unchanged)
lightweight v1.2.0-lw → "outcome": "up_to_date" (same graph, same commit)
```
Fixed at the root by peeling with `^{commit}`, rather than documenting
the quirk. The test pairs the two tag types on one commit so the control
is a single factor away; it fails without the fix.
The reviewer caught that this test had gone missing, and it was right
about the consequence — without it, removing the `^{commit}` peel passes
CI. The cause was a merge resolution on this branch: `checkout --theirs`
on `tests/integration_tests/merge.rs` took main's whole file, dropping
`test_step_rebase_annotated_tag_is_peeled` along with the duplicate
squash test it was meant to drop. Restored, and re-verified in both
directions.
**`wt step push` ran out of a half-finished operation.** Mid-rebase, the
detached HEAD is a linear extension of the target, so the fast-forward
check passed and `wt step push main` printed `✓ Pushed to main (1
commit)` — moving the target branch onto a half-replayed history while
the worktree kept its conflict markers and the rebase stayed open. It
now runs the `ensure_no_operation_in_progress` gate `wt step rebase` and
`wt merge` have used since
|
||
|
|
a0c1b662d2 |
perf(fsmonitor): resolve all daemons in one lsof spawn (#3581)
## Why `wt remove`'s end-of-command fsmonitor sweep forked one `lsof` per *machine-wide* `git fsmonitor--daemon` — and with `core.fsmonitor` enabled globally, a machine accumulates one daemon per repo ever touched (100+ is routine). So every `wt remove` paid ~100 process spawns. The cost is superlinear under load, not linear: a fork storm makes macOS Gatekeeper/XProtect assess each new image under contention, inflating per-spawn cost for everything else on the box, sweeps included. The `~50ms each` figure in the old docstring was measured *under that load* and read as a fixed per-call cost — so the sweep looked merely slow rather than self-amplifying. Two concurrent test suites driving hundreds of `wt remove` calls pinned all 18 cores (load average 142) with nothing hot in `htop`. This was diagnosed and measured before (#2814 added the sweep; #3401 wrote the cost into the docstring and explicitly changed no behavior) but never fixed. ## The fix Resolve every daemon in a single `lsof -a -p <pid1,pid2,…> -U -F pn` call and split its output on the `p<pid>` record boundary, handing each PID exactly the output the per-PID call produced — so classification and the whole data-safety contract in the module docs are unchanged. Verified end-to-end against a live 108-daemon machine with a PATH shim counting invocations: **108 spawns → 1**. Two details the batched path must preserve, each covered by a new test: - **Don't gate on `lsof`'s exit status.** It exits non-zero when *any* requested PID vanished mid-scan, while still printing full records for every survivor. Gating would drop the whole sweep whenever one daemon happened to exit — the common case on a busy machine. - **A PID with no record is dropped**, never synthesized as a socket-less entry. A socket-less entry reads as orphan class 1 and gets SIGTERM/SIGKILL — on a possibly-recycled PID. ## Also here - **Test consolidation.** Three integration fixtures built their branch set by spawning git per loop iteration. A new `TestRepo::create_branches` creates them in one `git update-ref --stdin`. The over-threshold completion test drops **10.5s → 1.4s** (~230 git spawns → 5); `switch_picker` and `statusline` branch loops get the same treatment. `mock-stub` gains an opt-in `MOCK_CALL_LOG_DIR` call log so a test can assert *spawn count*, not just response — deliberately outside the repo under test, since a log written inside the tree would dirty it mid-`wt merge`. - **Docs.** Dropped the "disable `core.fsmonitor` globally" workaround from the troubleshooting docs (canonical `skills/`, mirror regenerated). It stays enabled by choice, and the daemons were never the actual fseventsd CPU driver. - **CI hardening.** pre-commit.ci's `typos` hook maps the token `pn`→`on` and had auto-rewritten `lsof -F pn` to the broken `lsof -F on` (offset+name — no `p<pid>` records, so the parser reaps nothing). Pinned `pn` in `.typos.toml` next to the existing `PN`-from-`PNGs` entry that documents the same mapping, and added a test covering the empty-PID-set early return (flagged by `codecov/patch`). ## Testing New unit tests cover `daemons_from_batched_lsof` against real captured `lsof -F pn` output, a vanished-PID gap, and empty output; the rewritten integration test asserts exactly one `lsof` spawn with a comma-joined PID list. Full integration suite green (1960 passed), clippy and fmt clean. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
4549715af1 |
docs(claude-code): link the statusline segment details instead of restating them (#3576)
The `## Statusline` section of `claude-code.md` paraphrased three facts that the `wt list statusline` reference already owns: | Fact | `claude-code.md` said | `src/cli/list.rs` says | |---|---|---| | Pace maths | "1.4× the pace that would exactly fill that window, coloured by how much of it would be spent locked out at the cap" | "2.9× the pace that would exactly fill that window… colour deepens with severity as the forecast lockout grows" | | Link degradation | "clickable links where the terminal supports them, plain text where it doesn't" | "Both links are OSC 8, which a terminal that doesn't support them discards" | | CI fetch latency | "runs it in the background… 1–2 second CI fetch invisible" | "reach the network for a second or two, so it fits a statusline the host renders in the background" | #3568 removed the near-verbatim copies from this section but left these paraphrases behind, which is arguably the worse state: two wordings of one fact drift apart independently, and a reader can't tell whether they're the same claim or two different ones. ## What stays The cut isn't to zero. The page shows an example line, so it needs enough gloss for a reader to parse the line in front of them — a bare pointer would make the section a stub. It keeps one sentence naming what the stdin JSON contributes, with "how the links behave" folded into the pointer that was already there. The latency clause also stays, though it is the third duplicated fact. Without it the opening paragraph only restates the command name, and it is the one "why" that justifies the `settings.json` block below it. ## Side effect This leaves the `OSC 8` wording in `src/cli/list.rs` as the single home for the link behaviour. A precise term earns its place in an exhaustive `--help` reference and grated on an end-user integration page — so the jargon was really a symptom of the fact living in two places. Docs-only. Mirrors regenerated by `test_docs_are_in_sync`; `zola build` clean (14 pages, anchors resolve). > _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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
c6eb148e62 | docs(hook): correct pre-switch {{ branch }} resolution note (#3516) | ||
|
|
d44d68797a | docs(extending): add workz to custom-subcommand examples (#3513) |