mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
v0.70.0
4732 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maximilian Roos
|
1faa7b9e8b |
docs(changelog): cover #3639 and #3641 in 0.70.0 (#3642)
Two commits landed on `main` while #3640's CI ran, so they ship in 0.70.0 but weren't in its changelog. The release's drift check caught them before the tag. - **#3641** — a user-facing fix: `wt hook show` printed a section heading and nothing else for a hook type declared with an empty command list. Added as its own Fixed entry. - **#3639** — help-text only, no behavior change, correcting `wt config alias dry-run`'s description of the `vars.*` preview. Folded into the existing hook-previews bullet, since it's the same feature's documentation catching up rather than a separate change. No version change; `v0.70.0` is tagged after this lands so the tag covers both. > _This was written by Claude Code on behalf of Maximilian_v0.70.0 |
||
|
Maximilian Roos
|
946e360447 |
Release v0.70.0 (#3640)
Cuts 0.70.0 — version bump plus the changelog for the 45 commits since
v0.69.2.
**Minor bump, not patch.** `cargo semver-checks` reports 6 breaking
library changes (`set_command_timeout` and `ListConfig::task_timeout_ms`
removed, `WorkingTree::stage` arity changed, three enum variants added
to exhaustive enums). Worktrunk ships breaking library changes freely,
but semver still puts a break at minor while pre-1.0.
## Release validation
- Local gate green on the release commit: 4631 tests, lints, doctests.
- `nightly` dispatched on the cut-from tip (`a27cbd42`) for the full
cross-platform suite — `full-tests` green on linux, macOS, and Windows,
plus minimal-versions, nix-flake, crate-build, and the release targets.
## Data-loss surface review
The cumulative diff was audited against the deletion surface. One
deliberate widening, signed off for this release with follow-ups to
file:
- **#3602** removes the content check from `wt config shell install`'s
legacy cleanup, so `conf.d/{cmd}.fish` and the stranded nushell
`{cmd}.nu` are now deleted by path, unread. Only that exact filename is
touched and each removal is reported, but the deletion is absent from
both `--dry-run` and the confirmation prompt, and the already-configured
path skips the prompt entirely.
Also noted, none blocking:
- The `!path.exists()` check precedes the lock guard on the
`Path`/`Current` removal arm, so a locked worktree whose directory is
absent loses its branch. This already governed the branch-targeted route
in v0.69.2; #3533 unified the other arms onto it. The FAQ's "Neither
`git worktree remove` nor `wt remove` (even with `--force`) will delete
them" is absolute where the behavior isn't.
- On the default background removal path, `ensure_clean` and
`stop_fsmonitor_daemon` swapped order, so the safety gate is now
answered by the live fsmonitor daemon rather than a full re-stat.
Bounded by trash staging with 24-hour retention, and the foreground and
picker paths were already daemon-served.
Net *improvements* to the same surface: shared-branch retention across
remove/prune/merge (#3533), outcome-accurate removal reporting (#3633,
#3637), and the removal of the thread-local command timeout that could
kill in-flight git commands on worker threads (#3615).
## Changelog accuracy
Entries were verified against the actual diffs rather than commit
messages, which corrected several drafts: the prune figures were one PR
stale (~2.9 s → the real ~0.6 s), `wt merge` takes no worktree argument
so it only gained the retention half of #3533, the Azure DevOps report
is behind `--full`, #3608 never touched `nightly.yaml`, and #3601
inverted what the FAQ change actually said. Two omissions were added —
the `install-statusline` foreign-statusline fix (#3595) and the shipped
`/wt-switch-create` skill change (#3636).
> _This was written by Claude Code on behalf of Maximilian_
|
||
|
Maximilian Roos
|
0e7454890e |
fix(hooks): an empty command list no longer leaves a bare heading (#3641)
`wt hook show` decided whether a section had printed anything from
whether the config held an entry for each hook type, not from whether
any command was rendered. A hook type declared with an empty command
list has an entry but no commands, so the section printed its heading
and then stopped:
```console
$ cat .config/wt.toml
post-switch = []
$ wt hook show
PROJECT HOOKS @ /path/.config/wt.toml
```
Both sections carried it, since the loop and the `(none configured)`
fallback were duplicated in `render_user_hooks` and
`render_project_hooks`. `render_hook_commands` now reports whether it
wrote any rows, and the loop plus fallback live once in
`render_hook_section`, which both callers delegate to. The flag means
"something was printed", which is what the `(none configured)` line
claims.
The execution path was already correct: an empty list announces nothing,
and the JSON output omits it. Verified against the built binary across
four cases (project-only empty, user-only empty, an empty list alongside
a real hook, and a filter naming the empty type), and pinned by
`test_hook_show_empty_command_lists`, which covers both the user and
project halves. Reverting the flag to its old meaning fails the test on
both.
Also corrects a docstring in the same file that still described the
preview's pre-#3638 behavior ("shows a `vars.*` template raw"), the same
drift #3639 fixed for `wt config alias dry-run`.
> _This was written by Claude Code on behalf of max_
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
a07fe6d6f2 |
docs(alias): correct the vars.* claim in alias dry-run (#3639)
#3638 changed the preview renderer so a template expands everything around a `{{ vars.<key> }}` reference and leaves only that reference literal. `wt config alias dry-run` shares that renderer, but its `--help` text and the `handle_alias_dry_run` docstring still described the previous all-or-nothing behavior, where a single `vars.` token left the whole template raw. This corrects both. The docstring's mention of a separate syntax-validation step goes with them: expansion reports syntax errors itself, so there is no longer a distinct validation pass to describe. No behavior change. The `dry-run` long help isn't rendered into any generated doc or snapshot, so no mirrors move with it. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
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> |
||
|
Maximilian Roos
|
9e7ad0144d |
fix(hooks): expand everything but vars.* in a preview (#3638)
Follow-up to #3635, from two reviews that landed after it merged. ## The `vars.*` preview was worse than #3635 claimed `render_template_preview` short-circuits on `template_references_var(template, "vars")`, returning the raw template. So one `vars.` token disabled expansion for the entire command: ``` pre-commit = "deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }}" before: deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }} after: deploy --branch=main --repo=repo --env={{ vars.env }} ``` #3635 described this as "a `vars.*` template renders raw", which is true but understates it: `{{ branch }}` and `{{ repo }}` stopped expanding too, in a command whose whole job is to show the expansion. That short-circuit predates #3635 and has been degrading `wt hook <type> --dry-run` the same way; #3635 only extended it to `wt hook show --expanded`. The fix is at the source rather than at either caller. A preview now injects a stand-in object for `vars` that renders each reference back as itself, nested access included (`{{ vars.config.port }}` round-trips), while every other variable expands normally. `VarsMode::Resolve` keeps execution reading real values from git config; only previews pass `VarsMode::Literal`. A preview also no longer spawns the git read that resolving `vars` required. `vars.*` stays literal on purpose: those values are read when the step runs, after an earlier step in the pipeline may have written them, so a value resolved at preview time can differ from the one the run uses. Nothing covered this, which is why the suite stayed green through the regression. `test_hook_show_expanded_matches_dry_run` now sets a var and asserts the listing and the dry-run both leave it alone while expanding `{{ branch }}` beside it. ## The syntax gate is a type error now #3635 moved the template syntax check out of `prepare_steps` into a free `validate_pipeline_syntax` that both execution funnels had to remember to call. `prepare_steps` now returns a `PreparedPipeline` the caller must resolve: `.validated()` for the paths that run hooks, `.into_unvalidated()` for the listing, which annotates a broken template in place rather than blanking itself. Forgetting is a compile error, the same property `ApprovedHookPlan` gives hook approval. ## Smaller items Four cross-references went stale when the syntax check moved: `PreparedCommand.template`, `validate_template_syntax`, the `switch.rs` skip comment, and `HOOK_INFRASTRUCTURE_VARS` (which still named two deleted functions). The `--expanded` behavior is now documented in the sentence that already owns `{{ vars.<key> }}` semantics, with its three generated mirrors regenerated. `PreparedStep::commands()` replaces two hand-rolled matches in `hooks.rs`. `default_branch` moves inside `build_manual_hook_template_vars` — only the commit-hook arm reads it, and resolving it can cost a `git ls-remote` on a fresh clone, so the other eight hook types no longer pay for it. The listing carries its expansion state in an `Option<String>` instead of re-deriving "was this expanded?" from whether a context exists. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
c90c4b1b77 |
refactor(remove): report a removal's branch fate, not its plan's intent (#3637)
Follow-up to #3633, which patched the most visible symptom (prune's summary counting candidate kinds). This lands the structural fix behind that class: the removal chain returned nothing from execution, so everything downstream reported the plan's intent as if it were the outcome. The type even said so — `RemoveResult` was produced by `prepare_worktree_removal` before anything ran, and the codebase couldn't agree on what it was (`RemovePlans` in remove.rs, `plan` in prune, `result` in the handler). ## The plan/outcome split `RemoveResult` → `RemovalPlan` (variant `RemovedWorktree` → `Worktree`), and `handle_remove_output` now returns a `BranchFate`: `Deleted` / `Retained` / `NotAttempted` observed synchronously, `Deferred` only for the detached legacy fallback whose outcome this process never sees. Consumers report the fate: - `wt remove --format=json` said `"branch_deleted": true` while stderr said `▲ Removed worktree but kept branch feature (not integrated)` — a pre-remove hook had advanced the branch, the SafeDelete re-check declined, and stdout reported the plan anyway. Now `false`. - `wt step prune`'s summary counts executed outcomes, so a deletion declined mid-run reads `✓ Pruned 1 worktree`, matching the per-item line above it. A declined *orphan* deletion (no worktree entry, nothing removed at all) now drops out of the removed list entirely instead of being counted as either lie. - The detached fallback emits the "kept branch" correction in the one case it can know (no CAS tail to append), instead of letting the progress message's "worktree & branch" stand uncorrected. The picker's `foreground: true, silent: true` contradiction collapses into one `RemovalExecution::{Foreground, Background(fallback), Silent}` axis, and the four hand-rolled capture-refs-then-CAS-delete sites share one `worktrunk::git::execute_branch_deletion` — the single spelling of "the plan said delete; do it now, against fresh refs". ## For the reviewer `src/output/handlers.rs` carries the bulk: each execution path constructs its fate where it observes the result, and rendering decisions (foreground propagates deletion errors, background warns, silent does neither) are unchanged. The merge with #3631's concurrent prune rework is the diff's riskiest region — the worker result channel widened from `Result<bool>` to `Result<Option<bool>>` so fates flow back through the done-channel and the collector stamps `candidate.deletes_branch` before the summary and JSON read it. Deliberately *not* done: reusing the planner from `wt merge`'s finish path (its divergences — clean-check strictness, preserve-vs-error on primary, merge-target — are intentional), and threading the fallback mode structurally (it just relocates an unreachable arm). Zero snapshot churn: the change is provably invisible wherever intent matched outcome. The divergence cases are pinned end-to-end by tests whose hook must be the branch tip to pass. The one deterministically unreachable arm is the detached-fallback warning (needs a cross-filesystem rename failure); expect `codecov/patch` to flag those few lines. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
4888d5246f |
refactor(hooks): derive the hook preview from the execution path (#3635)
`wt hook show --expanded` rebuilt the hook pipeline's template context by hand in `expand_command_template`, duplicating what `prepare_steps` already does for what actually runs: the same `build_hook_context` call, the same `hook_type` and `hook_name` inserts, the same `args` default, the same POSIX escape mode. Two copies of one rule, so the preview drifted from the executed command whenever the execution path gained a context key, with nothing to catch it. The listing now prepares its commands through `prepare_steps` itself (`hook_command_rows` in `hook_commands.rs`) and renders them through `render_template_preview`, the renderer `wt hook <type> --dry-run` already used. `prepare_steps` is the sole producer of hook command contexts, so a key added there reaches both with no second edit. ## The `args` divergence The old preview inserted `args = "[]"` unconditionally; `prepare_steps` defaults it only when unset, because manual `wt hook <type>` supplies real args upstream via `extra_vars`. The shared path keeps the conditional default and the listing simply does not supply `args`. The values coincide (a listing has no CLI args to forward, which is exactly what the default encodes), and keeping the conditional form means the rule stays written once, in the place that has a caller who needs the other branch. ## Where the syntax check went `prepare_steps` used to reject an unparsable template, so a pipeline that could not render in full never started. A listing wants the opposite: `wt hook show` is what you run when your hooks are broken, so it annotates the bad template in place and shows the rest. That check is an execution policy rather than part of building a command, so it moved out of `prepare_steps` into `validate_pipeline_syntax`, called by the two funnels every hook-running path goes through: `prepare_and_check` (foreground, background, dry-run, filtered) and `render_planned` (the plan-backed hooks behind `execute_planned_hook` and `register_planned`). Both are mutation-verified: removing either call fails a test. A newtype that made forgetting the gate a compile error would be stronger, but it threads a wrapper through `SourcedStep`, `ForegroundStep`, and the background pipeline spec for a guard whose failure mode is degraded fail-fast rather than incorrectness (a syntax error still surfaces when its step renders). ## User-visible changes Preview expansion errors now name the hook (`Failed to expand project:lint: ...`) instead of the generic `hook preview`. A template referencing `vars.*` renders raw in `--expanded`, matching `--dry-run`, where before it resolved against git config at preview time. Raw is the honest preview: those values resolve when the step runs, and an earlier step in the pipeline may write them. ## Tests `test_hook_show_expanded_matches_dry_run` pins the listing and the dry-run to the same rendering of `hook_type`, `hook_name`, and `args`. `test_foreground_pipeline_syntax_error_aborts_before_first_step` pins the relocated gate; it runs through `wt merge`'s pre-commit hooks because `wt hook <type>` cannot reach it (the CLI pre-parses every template for shorthand-argument routing and errors first). Also folded in: adding `source: HookSource` to the listing renderer left `approval_context: Option<(&Approvals, Option<&str>)>` encoding the same user-vs-project discriminator, so both sites now call one `needs_approval`. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
062cf57d8e |
ci(affected): pin cargo-affected to the published 0.3.2 (#3634)
Both `Install cargo-affected` steps in `affected.yaml` installed from git with no rev, so the tool version on any run was whatever had last merged to cargo-affected's default branch. That cut both ways today: an upstream regression made `cargo affected run` abort with `git diff stdout was not valid UTF-8: invalid utf-8 sequence of 1 bytes from index 136455` on all three OSes, turning the advisory legs red repo-wide, and the fix (cargo-affected #69) then arrived the same way. Neither change is visible in this repo's history. cargo-affected publishes to crates.io as of 0.3.1, so it can carry a `version: "=X.Y.Z"` pin like every other `baptiste0928/cargo-install` block here. `=0.3.2` is what the unpinned installs already resolve to, so nothing changes behaviorally today. Two things it buys: - The crate joins the weekly tend pin sweep, which reads `version:` strings and now covers `affected.yaml`. The `cargo-install` action's own drift annotation starts firing for it too. - The existing "bump the `db-v{N}` cache marker if cargo-affected ships an on-disk schema change" rule becomes enforceable. It was unfollowable against a floating version, since the schema could change with no diff to notice. The pin appears twice, once per job, which is a new way to be wrong. A drift between the two hits `migrate_legacy_tables`, which drops and rebuilds the coverage tables rather than erroring, so selection silently degrades while the advisory job stays green. That is recorded in the workflow's cache-contract comment and in the tend skill's bump list. `cargo-affected`'s `rust-version` is 1.94, against this repo's pinned 1.96.0 toolchain. ## Testing The `affected tests (…, advisory)` legs on this PR exercise the change directly: they resolve `=0.3.2` from crates.io rather than from git. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
67945d722a |
fix(hooks): make the warm build and the gate agree on their flags (#3627)
The post-merge warm build and the pre-merge insta gate now agree on RUSTFLAGS and the feature set, so the gate reuses the warm build's artifacts instead of rebuilding from scratch. Both use --all-targets --all-features. |
||
|
Maximilian Roos
|
e91b5fa885 |
perf(prune): parallelize removals on the scan lock's read side (#3631)
`wt step prune` executed removals one at a time under the write side of `check_lock` while the candidate scan fanned out on the read side. On the rust-scale fixture that serialized ~2.6 s of a ~2.9 s live prune. Removals now run concurrently: the main thread streams scan results into a FIFO queue of per-candidate jobs, and a worker pool sized like the rayon pool executes them under the read side of the lock. **Why the read side is safe.** The lock exists for the Windows `.git/config` race (#2801): `git branch -D` rewrites config via lockfile plus atomic rename, and a concurrent reader fails on the rename. The removal chain has since moved to the CAS `git update-ref -d`, and (verified empirically, by inode-watching `.git/config` across each command) neither it nor `git worktree remove` / `git worktree prune` touches config; `branch -D` remains reachable only through arms prune never takes. Concurrent `git worktree prune` runs are idempotent and exit 0, and `update-ref -d` does not consult worktree checkout state, so nothing in the hook-free chain needs exclusivity. The write side remains for the removals that do: hook-bearing candidates (a foreground `pre-remove` body is an arbitrary command with streamed output), `--foreground` candidates (TTY spinner), the fail-hard metadata-pruning candidates (`StaleDetached`, plan-less `Prunable`), and the deferred current worktree. `removal_needs_write` in `src/commands/step/prune.rs` is the decision table; the `check_lock` field doc carries the full analysis. **Output and determinism.** Skip lines ride the same FIFO queue as removals, so `RAYON_NUM_THREADS=1` (one worker) reproduces the exact serial output order the snapshot tests pin; with more workers, whole lines interleave (one `anstream` `eprintln!` writes a line under a single stderr lock). Live `--format=json` output is now sorted by scan index, matching dry-run, with the current worktree still last. The first failing removal flips an abort flag that drains the rest of the queue unexecuted, matching the serial loop's abort-on-first-error. **Measured.** `prune_e2e/live` criterion: ~615 ms to ~398 ms. Rust-scale live one-shot (36 worktrees, 24 candidates, warm): 2.9 s to 0.56 s wall, all 24 removals overlapping in the trace (6.5 s of subprocess time). **Abort semantics.** The first failing removal flips an abort flag: in-flight removals complete (their per-candidate lines already printed), queued ones drain unexecuted, no summary prints, and the first error propagates — the same abort-on-first-error the serial loop had, where an error mid-loop likewise left earlier removals done and later candidates untouched. A mid-scan check error takes the same abort path. **Testing.** New `test_prune_removals_run_concurrently` proves the overlap causally: two orphan-branch removals barrier on each other inside a git shim, and serialized removals trip the shim's timeout sentinel (verified: the test fails under forced `RAYON_NUM_THREADS=1`). `test_prune_removal_failure_aborts_remaining_queue` and `test_prune_concurrent_removal_failures_report_first` pin the abort semantics deterministically (single-worker FIFO, and a barrier forcing two concurrent failures). The config-race canary keeps its role with its premise rewritten: a regression to a config-rewriting deletion is what it would now catch. The deterministic-output snapshot tests pass unchanged. The one patch line no test reaches deterministically is the worker's early `return` when the result receiver is gone (a scan error racing an in-flight removal). Accepted residuals, documented in code: a `packed-refs.lock` contended past git's ~1 s retry budget reads as `RetainedRaced` (fail-closed; unobserved at 24-way concurrency on local disks), and concurrent fail-soft `git worktree prune` races leave at worst stale metadata for the next prune to collect. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
4c845597b2 |
fix(prune, remove, ci): count what a removal took, not what it selected (#3633)
Three follow-ups from the review of #3533, which landed shared-branch retention across the removal surface. Each is small and independent; they share a branch because they came out of the same pass. ## `wt step prune` counted kinds, not outcomes A candidate's kind is what the scan selected, not what the removal took. A branch a sibling worktree still has checked out is retained, so a worktree candidate can take the worktree and leave the branch standing. The summary counted the kind anyway: ```console ○ Worktree directory missing for feature; pruned ↳ branch checked out at ~/code/repo.feature ✓ Pruned 1 branch ← the branch is right there ``` `prune_summary` now counts the planned outcome, so that reads `✓ Pruned 1 worktree` — the stale entry, which is all that went. Both `--format=json` payloads gained `branch_deleted` for the same reason: a consumer reading `{"branch": "feature", "kind": "branch_only"}` would reasonably conclude the branch is gone. The predicate has one home now. `RemoveResult::to_json` already computed `branch_deleted` inline; it moved to `RemoveResult::deletes_branch()` and both callers share it. That incidentally fixes a detached worktree reporting `"branch_deleted": true` beside a null branch — it has no branch to delete. `wt step prune --dry-run` had the same defect and no per-item line to contradict it, so it now predicts retention too. A `Prunable` item has no plan until `try_remove` prunes its stale entry, so the dry run asks `live_sibling_checkout` — the same predicate the plan would. ## `RemoveTarget::Branch` now means "a branch with no worktree" All three callers resolve first and pass `Path` for anything with a worktree, so the arm handling "the branch turned out to have a worktree" was unreachable at selection time — codecov confirmed it never executed. Deleting it would have been enough, but the arm was a live hazard rather than dead weight: the picker builds a fresh `Repository` and re-lists worktrees between selecting a row and removing it, so a `wt switch` in another terminal can give a branch-only row a worktree mid-flight. The old code would then have removed that worktree, from a row that said "delete this branch" — the same defect class #3533 fixed. ```console ✗ Branch feature gained a worktree @ ~/code/repo.feature since it was selected; to remove that worktree, run wt remove ~/code/repo.feature ``` `test_prepare_removal_refuses_branch_that_gained_a_worktree` covers the picker path. I checked it fails when the guard is neutralized rather than passing trivially. ## `affected tests (windows, advisory)` was permanently red git calls a file binary only when it finds a NUL byte in the first 8000, and two of the committed loose objects under `tests/fixtures/*/_git/` are zlib streams small enough to have none. Those diff as text, so `git diff` writes raw deflate into its output, and `cargo affected` aborts decoding it as a string before running a test. Reproduced with `git show <commit> -- <object>` (invalid UTF-8, byte `0x95` at position 5403) and confirmed fixed — the same command now reports `Binary files … differ`. The rest of `_git/` stays text worth reading. ## Testing `prune_summary_counts_a_retained_branch_as_worktree_only` covers the counting; `test_prune_retains_branch_checked_out_in_another_worktree` gained an assertion on the summary line, and I verified it fails when the guard is removed. The six `.snap` changes are all the same added `branch_deleted` key. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
4af04c2260 |
fix(remove, prune, merge): target the named worktree, retain shared branches (#3533)
Follow-up to the discussion on #3480: [@max-sixty asked](https://github.com/max-sixty/worktrunk/pull/3480#issuecomment-5039137116) to have `wt remove` honor an explicit path generally. #3607 has since landed the resolution half, `Repository::resolve_worktree`, which turns a path into a worktree. This is the removal half, and it is the part resolution cannot decide: which worktree to act on is a naming question, whether the branch may be deleted is not. ## Problem `wt remove` threw the resolved answer away. For any non-current worktree it re-targeted by branch name, and `prepare_worktree_removal` mapped that branch back to git's *first-listed* worktree. Two failures followed, both silent. **The wrong worktree is removed.** With `feature` checked out twice: ```console $ git worktree list …/dup [feature] …/first [feature] $ wt remove …/first ◎ Removing feature worktree & branch in background (same commit as main, _) ``` `…/dup` is gone. `…/first`, the one named, is still there. **The survivor is left broken.** The shared branch is deleted with it. Worktrunk deletes branches with `git update-ref -d`, git's compare-and-swap primitive, which unlike `git branch -d` does not refuse a ref that is checked out somewhere: ```console $ git worktree list …/first 0000000 [feature] $ git -C …/first rev-parse HEAD fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree. ``` `wt step prune` reached the same deletion unattended, and `wt merge` reached it with a freshly integrated branch, so nothing else declined. A branch gets a second worktree only through `git worktree add --force`; worktrunk never does it itself. ## Fix **Remove the worktree that was named.** `wt remove` drops non-current worktrees via `RemoveTarget::Path`. `wt step prune` does the same: its candidates already carry a path, and targeting a *stale* entry by branch name resolved to a live worktree that the same prune had just skipped as too young, then removed it. **Retain a branch another worktree holds.** One predicate, `live_sibling_checkout`, answers "would deleting this ref orphan a checkout?", and every path that can delete a branch asks it: `prepare_worktree_removal`'s worktree and pruned-branch-only arms (covering `wt remove`, `wt step prune`, and the picker, which already targeted by path) and `wt merge`'s finish. A hit forces `BranchDeletionMode::Keep`, the single chokepoint every deletion path honors, and names the surviving checkout: ```console $ wt remove …/dup ◎ Removing feature worktree in background ○ Branch feature retained; still checked out @ …/first ``` A sibling whose *directory* is already gone is stale metadata, not a checkout with anything to lose, so it does not retain: removing the last live checkout still deletes the branch. **`-D` is refused out loud.** Everywhere else `-D` is the override that wins, so one that cannot be honored warns rather than passing quietly: ```console $ wt remove …/dup -D ◎ Removing feature worktree in background ▲ Branch feature retained despite -D; still checked out @ …/first ``` The ordinary single-checkout case is unchanged, and a retained branch skips the integration check entirely rather than computing a verdict it would discard. #3480's duplicate-checkout hint now points at `wt remove <path>`, which this makes the safe answer. ## Testing Full gate green. New coverage, each case asserting the survivor still resolves `HEAD`, which is the corruption in question: - `remove`: by path, by name, refused `-D`, the pruned-directory fallback, and the mirror case where a stale sibling must *not* retain. - `step prune`: a stale entry whose branch is live in an age-skipped worktree. This test is what surfaced the wrong-worktree bug in prune. - `merge`: merging a branch that a `--force` duplicate also holds. ## Not addressed `wt step prune`'s summary counts candidates rather than outcomes, so a retained branch still reports `✓ Pruned 1 branch`. The per-item line above it already says the branch was retained. Fixing the count means threading removal outcomes back through prune's accounting, which is a separate change. > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dependabot[bot]
|
21ea6d6f1a | chore: bump skim from 5.1.0 to 5.5.0 (#3626) | ||
|
Maximilian Roos
|
835d5f0cc1 |
perf(switch): compute only the template vars --execute names, and give the context a type (#3628)
`wt switch --execute` built its template context with `referenced:
None`, so every invocation resolved every standard variable whether or
not the command named one. That context feeds `expand_template` and
nothing else: the payload reaches the child as a shell string through
the EXEC directive file (or `sh -c`), never as JSON on stdin, and
`--execute` renders no `-v` variables table. Every unnamed variable was
a git subprocess whose result was discarded.
`var_default_branch` is the one with teeth. On a clone with no
`refs/remotes/origin/HEAD` and no cached `worktrunk.default-branch`, it
falls through to `git ls-remote`, so `wt switch feature -x 'echo hi'`
reached the network for a variable the command never mentions. Traced on
a scratch repo in that state:
| `--execute` | before | after |
| --- | --- | --- |
| `echo hi` | 13 subprocesses, 1 `ls-remote` | 8, none |
| `echo {{ commit }}` | 13, 1 | 9, none |
| `echo {{ default_branch }}` | 13, 1 | 11, 1 (correctly still fetches)
|
`referenced_vars_for_templates` unions minijinja's
`undeclared_variables` across the command and its trailing args, which
are two expansion positions over one context map. It does not error on
an unparsable template: `validate_switch_templates` has already parsed
both positions against `ValidationScope::SwitchExecute` before the
switch runs, and reports a syntax error against the position that
carried it.
## The second commit: why `None` was reachable at all
`build_hook_context` returned a bare `HashMap<String, String>`, so each
of its seven callers had to know four separate things about the result:
how to reborrow it as `HashMap<&str, &str>` for `expand_template` (five
copies), how to serialize it for a child's stdin (four copies, two
carrying the same `expect` string), what to insert into it afterwards,
and whether it had been filtered. The doc comment on that last point
said "hooks pass `None`", which described the callers rather than the
rule, so `--execute` inherited `None` by sitting next to code that
needed it.
`TemplateContext` (`src/config/expansion.rs`) owns all of it, with
`expand` / `to_json` / `insert` / `get` / `remove` as the interface and
the map private. It serializes transparently, so the JSON a hook child
reads on stdin and the `PipelineSpec` the background runner deserializes
are the same flat object as before.
The filter argument becomes `VarScope` rather than
`Option<&BTreeSet<String>>`. `VarScope::All` has to be typed out, and
its doc states the discriminator: `Referenced` when only the templates
read the context, `All` when a child reads the JSON or the command's
output is the variable listing. The same enum replaces the second
`Option<&BTreeSet<String>>` on `format_alias_variables`, which meant the
same thing (which vars render as dim `(unused)`).
Two changes fell out of that rather than being planned. `src/llm.rs` had
its own private `TemplateContext` for LLM prompt data, which would have
made two types of that name in one crate; it is now `PromptContext`. And
`expand_shell_template` shrank to a one-line wrapper but stays, because
it still fixes POSIX for its four callers and converts the error type.
Not included: `render_hook_commands`'s preview path reimplements what
`prepare_steps` does (build context, insert `hook_type`, insert
`hook_name`, default `args`, expand POSIX), so the preview drifts from
what actually runs whenever the execution path gains a context key. That
touches `wt hook` output and wants its own change.
## Testing
`test_switch_execute_computes_only_referenced_vars` reads `var_*` spans
out of the `-vv` `trace.jsonl` and pairs the variable-free case with a
`{{ commit }}` control, so an empty span set cannot pass by the trace
simply missing them. Reverted to `None` it fails with `got
["var_commit", "var_default_branch", "var_primary_worktree",
"var_remote"]`. Two unit tests cover `referenced_vars_for_templates`
directly (union across positions, unparsable template contributes
nothing).
The refactor is covered by the existing suite rather than new tests,
which is the point: 4606 tests pass with no snapshot movement, so no
user-visible output changed.
> _This was written by Claude Code on behalf of max_
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Worktrunk Bot
|
648f1a866e | docs(test): drop stale line-number reference in porcelain-parse test (#3630) | ||
|
Maximilian Roos
|
a285ed308d |
refactor(tests): bring the mock-stub env vars under the WORKTRUNK_TEST_ prefix (#3621)
`MOCK_CONFIG_DIR` and `MOCK_CALL_LOG_DIR` were the only two worktrunk-invented environment variables without the `WORKTRUNK_` prefix. That is not just a naming inconsistency: `isolate_subprocess_env` scrubs the parent environment by prefix — `GIT_*` and `WORKTRUNK_*` — so an unprefixed name is the one thing a test child inherits from whoever ran the suite. Renaming them brings them under that scrub. - `MOCK_CONFIG_DIR` → `WORKTRUNK_TEST_MOCK_CONFIG_DIR` - `MOCK_CALL_LOG_DIR` → `WORKTRUNK_TEST_MOCK_CALL_LOG_DIR` `TEST` rather than a bare `WORKTRUNK_` because both are read only by `tests/helpers/mock-stub` — they are the protocol between the harness and its helper binary, and `wt` itself never reads either one. That matches the ~15 existing `WORKTRUNK_TEST_*` knobs. ## The snapshot half Mechanical but not a substitution: the `env:` block is byte-sorted by key, so the renamed entry moves position within it and a `sed` in place would leave it where the old name sorted. All 971 affected blocks were rewritten by dropping the old line, inserting the new one, and re-sorting — the aggregate diff is exactly one removed and one added line per file: ``` 971 files changed, 971 insertions(+), 971 deletions(-) - MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" ``` The redaction placeholder follows its neighbours' convention in `add_standard_env_redactions` (`WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR` → `[TEST_NU_VENDOR_AUTOLOAD]`), so it now reads `[TEST_MOCK_CONFIG]` rather than repeating the full key. `MOCK_CALL_LOG_DIR` appears in no snapshot — its two call sites are `.output()` assertion tests — so it needs no redaction. ## Keeping it from regressing `tests/CLAUDE.md` gains the rule under "Where a new environment variable goes": name it `WORKTRUNK_TEST_*`, and the rule covers the harness↔helper-binary protocol, not just knobs `wt` itself reads. Without it the next helper-binary variable gets named `MOCK_*` again and the hermeticity hole reopens. ## Verification `cargo run -- hook pre-merge --yes` passes (exit 0) on the merged tree: 4607 tests including `--features shell-integration-tests`, `pre-commit run --all-files`, clippy, doctests. No pending snapshots. A repo-wide sweep finds no remaining unprefixed spelling. ## Merged main #3620 landed while this was in flight and regenerated several `for_each` snapshots that still carried the old key, so main is merged in here. It resolved with no conflicts, and the result is what you'd want rather than what git happened to produce: those blocks now carry #3620's new keys (`GIT_ALLOW_PROTOCOL`, `CLAUDE_CONFIG_DIR`, `WORKTRUNK_TEST_PARENT_SHELL`) *and* the renamed key, each in sorted position. The sweep and the gate above both ran after the merge. #3620 also rewrote the "Where a new environment variable goes" section this branch adds to — three layers became four. Both edits survived; the new naming paragraph follows the updated layer list. The two advisory `affected tests` checks are red for the same reason, and merging clears them: `cargo affected` errors on `git diff stdout was not valid UTF-8`, because the diff from this PR's base contains 16 binary files — the `tests/fixtures/standard/` git objects and index files that #3620 deleted. This branch's own commit contributes none. A PR based after #3620 won't see them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
dependabot[bot]
|
e6ca358846 |
chore: bump clechasseur/rs-cargo from 5.0.6 to 5.0.7 (#3624)
Bumps [clechasseur/rs-cargo](https://github.com/clechasseur/rs-cargo) from 5.0.6 to 5.0.7. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/clechasseur/rs-cargo/releases">clechasseur/rs-cargo's releases</a>.</em></p> <blockquote> <h2>v5.0.7</h2> <p>Patch release with updates to vulnerable dependencies.</p> <h2>What's Changed</h2> <ul> <li>chore(deps): bump fast-xml-parser from 5.9.3 to 5.10.1 in the npm_and_yarn group across 1 directory by <a href="https://github.com/dependabot"><code>@dependabot</code></a>[bot] in <a href="https://redirect.github.com/clechasseur/rs-cargo/pull/427">clechasseur/rs-cargo#427</a></li> <li>fix: update <code>@clechasseur/rs-actions-core</code> to 8.0.3, run <code>npm update</code> to get vulnerability fixes by <a href="https://github.com/clechasseur"><code>@clechasseur</code></a> in <a href="https://redirect.github.com/clechasseur/rs-cargo/pull/429">clechasseur/rs-cargo#429</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/clechasseur/rs-cargo/compare/v5.0.6...v5.0.7">https://github.com/clechasseur/rs-cargo/compare/v5.0.6...v5.0.7</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/clechasseur/rs-cargo/commit/8ccb6817fc46c9af3b058a5a7b31932edb82cf13"><code>8ccb681</code></a> Merge pull request <a href="https://redirect.github.com/clechasseur/rs-cargo/issues/429">#429</a> from clechasseur/fix/update-deps</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/a11eddeaaecf8997cf0bcc3c24becc1f09624173"><code>a11edde</code></a> fix: bump version to 5.0.7</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/76685bb9f52e212a4c00f3ddc44842dc5281ca3d"><code>76685bb</code></a> fix: <code>npm update</code></li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/48ea33b367c4c1b8b6fe7fefc203bce3d4e7158a"><code>48ea33b</code></a> fix: update <code>@clechasseur/rs-actions-core</code> to 8.0.3</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/b5a8184fad32ef9f11f0e29418d46300fa1a1986"><code>b5a8184</code></a> chore(deps): bump fast-xml-parser (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/427">#427</a>)</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/5b3bb18517d1d95a02ac4aa47ff2a192c63da94a"><code>5b3bb18</code></a> chore(deps): update dependency prettier to ^3.9.6 (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/422">#422</a>)</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/95bd32da1610f3c33ea573b0248b59d5416eefee"><code>95bd32d</code></a> chore(deps): update dependency oxlint to ^1.74.0 (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/420">#420</a>)</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/de86f7e954c523dbb840a840196e75d2b3f5a56c"><code>de86f7e</code></a> chore(deps): update actions-rust-lang/setup-rust-toolchain action to v1.17.0 ...</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/24b0f4bdb30da53e2b0bd096f5936f516c683c38"><code>24b0f4b</code></a> chore(deps): update dependency <code>@types/node</code> to ^24.13.3 (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/423">#423</a>)</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/3ad8301e71b20505890be273a34f0840a9f7701b"><code>3ad8301</code></a> chore(deps): update actions/setup-node action to v7 (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/426">#426</a>)</li> <li>See full diff in <a href="https://github.com/clechasseur/rs-cargo/compare/v5.0.6...v5.0.7">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
8ef63150b6 |
chore: bump taiki-e/install-action from 2.85.0 to 2.85.2 (#3623)
Bumps [taiki-e/install-action](https://github.com/taiki-e/install-action) from 2.85.0 to 2.85.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/taiki-e/install-action/releases">taiki-e/install-action's releases</a>.</em></p> <blockquote> <h2>2.85.2</h2> <ul> <li> <p>Update <code>prek@latest</code> to 0.4.11.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.13.</p> </li> <li> <p>Update <code>kingfisher@latest</code> to 1.109.0.</p> </li> </ul> <h2>2.85.1</h2> <ul> <li> <p>Update <code>vacuum@latest</code> to 0.30.0.</p> </li> <li> <p>Update <code>uv@latest</code> to 0.11.32.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.12.</p> </li> <li> <p>Update <code>cyclonedx@latest</code> to 0.33.1.</p> </li> <li> <p>Update <code>cargo-neat@latest</code> to 0.5.2.</p> </li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/taiki-e/install-action/blob/main/CHANGELOG.md">taiki-e/install-action's changelog</a>.</em></p> <blockquote> <h2>[2.85.2] - 2026-07-26</h2> <ul> <li> <p>Update <code>prek@latest</code> to 0.4.11.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.13.</p> </li> <li> <p>Update <code>kingfisher@latest</code> to 1.109.0.</p> </li> </ul> <h2>[2.85.1] - 2026-07-25</h2> <ul> <li> <p>Update <code>vacuum@latest</code> to 0.30.0.</p> </li> <li> <p>Update <code>uv@latest</code> to 0.11.32.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.12.</p> </li> <li> <p>Update <code>cyclonedx@latest</code> to 0.33.1.</p> </li> <li> <p>Update <code>cargo-neat@latest</code> to 0.5.2.</p> </li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/taiki-e/install-action/commit/41049aa56687c35e0afa74eed4f09cec4f9afabf"><code>41049aa</code></a> Release 2.85.2</li> <li><a href="https://github.com/taiki-e/install-action/commit/dfcf36552b1b743e910b9b9b6b114a08e56a1e5e"><code>dfcf365</code></a> Update <code>prek@latest</code> to 0.4.11</li> <li><a href="https://github.com/taiki-e/install-action/commit/eea03ccfa855b965a301aa52424915fddc32f855"><code>eea03cc</code></a> Update <code>mise@latest</code> to 2026.7.13</li> <li><a href="https://github.com/taiki-e/install-action/commit/81ca2feb84748ed3fa514707ee1004f4f3ad715a"><code>81ca2fe</code></a> Update martin manifest</li> <li><a href="https://github.com/taiki-e/install-action/commit/cc90ed04bc1a258ea90a83789f24b759a77c9671"><code>cc90ed0</code></a> Update <code>kingfisher@latest</code> to 1.109.0</li> <li><a href="https://github.com/taiki-e/install-action/commit/55639a3362f508fda5154d3451072205f5c32ca6"><code>55639a3</code></a> Update cargo-shear manifest</li> <li><a href="https://github.com/taiki-e/install-action/commit/3d7d7cd5ac7f994c1892ae0c06165095b9139094"><code>3d7d7cd</code></a> Release 2.85.1</li> <li><a href="https://github.com/taiki-e/install-action/commit/d09ccb4fe2105ac9ead6376f7a423ff88defeb57"><code>d09ccb4</code></a> Update <code>vacuum@latest</code> to 0.30.0</li> <li><a href="https://github.com/taiki-e/install-action/commit/ac43dee1a92e482c0e244bdb0bb126908159e245"><code>ac43dee</code></a> Update <code>uv@latest</code> to 0.11.32</li> <li><a href="https://github.com/taiki-e/install-action/commit/49b16979f38f30e44b1f68af9115be9a6ffc5215"><code>49b1697</code></a> Update prek manifest</li> <li>Additional commits viewable in <a href="https://github.com/taiki-e/install-action/compare/v2.85.0...v2.85.2">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Maximilian Roos
|
4202cffe61 |
fix(ci): split ci by cadence so coverage uploads on every main commit (#3608)
## What prompted this Getting #3605 green ran into codecov reporting a `base_commit` three commits older than the real merge-base. This audits whether our config causes that. ## The cause Codecov picks a PR's base by walking back to the newest ancestor that has a coverage report. It used the real merge-base for PRs #3480, #3532 and #3602, and a stale one for #3603 and #3605. The difference is whether the merge-base uploaded a report. **29 of the last 40 main commits did not.** `ci` had one concurrency group for main pushes, and GitHub cancels the *pending* run in a group whenever a newer one joins, even with `cancel-in-progress: false`. So the question is how long a run holds the group, and a run isn't done until its slowest job is: | job | duration on main | |-----|------------------| | `fast-checks` | 2 min | | `code-coverage` | 3-4 min | | `test (windows)` | 11 min | | `collect affected coverage (windows)` | 110-129 min | Each main run held the group for ~2 hours, so nearly every subsequent main push was cancelled while queued, taking the 4-minute coverage job with it. Every cancelled main run's `updated_at` lands within a second of the next push's `created_at`. The 2 hours is real work, not queue: 2-5s from `created_at` to `started_at`, then 108 minutes inside `cargo affected collect` — 4181 tests under `-C instrument-coverage` with a per-test LLVM profile, ~5 GB of profraw. ## The fix: one workflow per cadence The three groups of jobs have incompatible needs, and one group was serving all of them. | workflow | cadence on main | why | |----------|-----------------|-----| | `ci` | every commit, ~11 min | required gate + fast checks | | `coverage` | every commit, keyed per-sha | a skipped upload leaves later PRs on a stale base | | `affected` | sampled, ~2 h | a DB a few commits old still anchors a correct superset | `affected` keeps exactly the grouping it has today, so its sampling is unchanged and deliberate. It just no longer drags the other two along. ### Scope of the impact The posted `codecov/patch` check scopes to the PR's own GitHub diff, so a stale base did **not** score PRs against other people's lines. On #3605 the posted 91.66% is exactly `github.rs`'s 11/12, while the stale-base compare object reported 64/65 across 13 files. What a stale base costs: - `codecov/project` reports "compared to \<stale sha\>" - the patch `auto` target is the stale base's project coverage (0.02pp here) - the compare API object widens to `base..head`, which is what made the investigation look like silence Separately, `test`/`lint`/`fast-checks` also stopped completing on main. Nothing load-bearing rode on that (they already ran on the PR), but it left `tend-ci-fix` with nothing to watch, since it doesn't fire on cancelled runs. ## Two smaller fixes - `ignore: "**/tests/**"` compiles to `.*/tests/.*` (confirmed against codecov's validator), which needs a leading directory and so never matched `tests/` itself. Inert today since `cargo llvm-cov` reports only `src/` (verified against a downloaded `cobertura.xml`), but now correct if that changes. Now `tests/**`. - `fail_ci_if_error` gated on `github.repository_owner`, which is the *base* repo's owner on a fork PR too, so the soft-fail its comment describes never applied. It keys off the head repo now. ## Docs The API behaviour was ours to misuse, not codecov's to explain. Three traps, all confirmed against the live API: - `file_report/<path>/` 404s with `coverage info not found` because the route swallows the trailing slash into the path. Without it the endpoint returns `line_coverage`. - `?pullid=N` always compares the PR's **current** head. `?base=&head=` asks about an earlier commit. - the compare response has no `patch_totals` key, and `.name` is `{base, head}` rather than a string, so a filename lookup silently matches nothing. A working recipe already existed in `running-tend`, but that skill is scoped to CI. `tests/CLAUDE.md` owns coverage investigation, so the queries go there and `running-tend` points at them instead of keeping a second copy. Re-running the corrected query against #3605's failing commit reproduces the miss exactly: `src/git/remote_ref/github.rs:164`, the `gh repo set-default` hint, matching what the session eventually found by hand. ## This PR demonstrates it It changes no Rust at all, only YAML and markdown. Codecov still reported a **10-file, 111-line patch** on its first commit, because it based the comparison on `203603909` rather than the real merge-base `32f380a27`. Every main commit in between has no report: | commit | ci run | report | |--------|--------|--------| | `32f380a27` | queued | no | | `9645e3e13` | cancelled | no | | `bcd1ffdfd` | cancelled | no | | `8865f20ab` | cancelled | no | Every one of those 111 patch lines belongs to somebody else's merged commit. It passed at 100% only because those commits are well covered. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Maximilian Roos
|
9cb128328f |
refactor(tests): deny network transports for in-process git, consolidate fixtures (#3620)
An in-process `Repository::run_command()` spawns git with the test process's own environment, so none of the harness env isolation reaches it — no `GIT_CONFIG_GLOBAL`, and crucially no `GIT_ALLOW_PROTOCOL`, meaning a unit test driving the library directly could make a real network connect (reproduced: `ls-remote` against a fixture-style `https://` URL attempted the wire). The repo's own config is the one layer such a spawn still reads, so every fixture repo now carries `LOCAL_TEST_CONFIG` (identity, `commit.gpgsign`, `protocol.allow = never` with a `file` exception), written by a single `TestRepo::assemble` that every constructor routes through. The consolidation on top of that fix: - **`git_test_env()`** is the one home for the nine git-isolation settings; `configure_git_cmd` (Command), `configure_git_env` (`Cmd`), and `pty_env_vars` (PTY) all consume it. Composes with #3618's PTY layering — the env doc in `tests/CLAUDE.md` now reads four layers. - **The test gitconfig is one process-static file** (`test_gitconfig_path()`); the per-fixture copies (including two hand-rolled, drifted ones in `bare_repository.rs`) are gone. - **`BareRepoTest` and `NestedBareRepoTest` compose over `TestRepo`** (new `TestRepo::bare_at()`), so they inherit `assemble`'s guarantees structurally. Their public APIs are unchanged, deliberately — ~220 call-site references and the bare-repo snapshots are written against them. - **The committed `tests/fixtures/standard/` is deleted** (83 files), along with its Taskfile generator and the `_git` rename/sed dance. `build_standard_fixture` constructs the template once per `target/<profile>` (private-dir build + atomic rename, so parallel nextest processes race benignly) and `copy_standard_fixture` keeps only copy + per-copy gitdir rewrite. Two review-worthy subtleties in the fixture rebuild: - The old fixture's commits were stamped `-08:00` (the generator machine's timezone) and **~170 snapshots embed the resulting commit SHAs**, so the builder pins that timestamp as an explicit constant and `test_standard_fixture_template_reproduces_pinned_shas` asserts the four exact SHAs — the template reproduces the committed fixture bit-for-bit. - The old fixture's index files carried an untracked-cache extension from the generator machine's config, which made `git status` print `warning: untracked cache is disabled…` into exactly two `for_each` snapshots. The clean-room template drops it; those two snapshots are regenerated here (that's the whole content diff, plus documented-cosmetic env-block lines). Tested by the full pre-merge gate including `shell-integration-tests` (4583 tests) plus a post-merge-with-main revalidation. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
e548196f89 |
docs(bench): refresh rust-scale prune one-shots after the removal-chain trim (#3622)
Re-measurement on the rebuilt `rust-prune-12-24` fixture after #3617 trimmed the removal chain: live one-shot ~12 s to ~2.9 s (two runs, 2.91 s / 2.89 s), worktree candidates ~155-210 ms each now that the status is fsmonitor-served, branch-only ~40-100 ms. Full-cold dry-run unchanged in shape. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
5ecdf2bb78 |
perf(remove, prune): one check per guarantee in the removal chain (#3617)
`wt step prune` scans in parallel, but each removal ran a serial chain of ~17 git subprocesses under the scan write lock: a re-run of `prepare_worktree_removal` (the scan had already run it and kept only `is_ok()`), a post-hook "safety refresh" (status + fresh snapshot + integration re-compute) that ran even when no pre-remove hooks executed, and a final pre-rename clean check that ran right after the fsmonitor daemon stop, paying a full un-fsmonitored re-stat. At rust-lang/rust scale that post-stop status is the 0.5-1.7 s per-removal driver. This trims the chain to one mechanism per guarantee, with no behavioral additions: - **Prune passes the scan-time plan through** (`CheckOutcome.plan`) instead of re-preparing under the write lock. `Prunable` candidates still prepare in `try_remove` because preparing them prunes stale metadata, which must stay serialized. - **`refresh_removal_safety_after_pre_remove` is deleted.** `prepare_worktree_removal` now computes the integration verdict and effective target for display (carried on `RemoveResult::RemovedWorktree`), the pre-rename clean check catches hook-dirtied worktrees, and `delete_branch_if_safe`'s CAS re-decides the branch deletion against fresh refs. The clean check also moves into `remove_worktree_with_cleanup`, so the foreground and picker paths get their final gate adjacent to the rename rather than relying on the deleted refresh. - **The clean check runs before the fsmonitor daemon stop.** The stop must stay before the rename: on Windows the daemon holds a handle on the worktree that would fail the rename, and git's graceful stop resolves the daemon by worktree path, so it cannot reach it after the move (verified: a healthy daemon self-exits and unlinks its socket within ~500 ms of the rename; the client from the renamed path reports "not running"). - **`prune-gather` stops shelling out**: the main worktree comes from `git worktree list` ordering (documented: main lists first; `list_worktrees()` filters bare entries) instead of a `git rev-parse --git-dir` per worktree. Criterion on the `prune-4-8` fixture: `dry_run_probe_cold` 197 ms to 147 ms (-25%), `dry_run_warm` 110 ms to 62 ms (-43%), `live` 783 ms to 615 ms (-21%). A `prune-12-8` one-shot timeline (24 candidates, live): wall 3.14 s to 2.09 s; per-worktree removals 170-210 ms to 112-134 ms, now dominated by the ~60 ms daemon stop. The rust-scale numbers in `benches/CLAUDE.md` are marked for re-measurement on the next fixture rebuild. **Consistency corner, unchanged in kind but wider in trigger:** a worktree that becomes dirty after `handle_removed_worktree_output` starts (in practice: a pre-remove hook dirtying it) now fails at the pre-rename check, which is after the cd directive is written (and, in background mode, after the "Removing … in background" message and retention hint print), and the shell wrapper applies the cd file regardless of exit code. So in that corner the shell moves to the primary worktree while the dirty worktree is preserved with an error. The same outcome already existed for a file appearing between the refresh and the final check; nothing is removed in either case. Testing: full pre-merge gate green (4,578 tests, no snapshot changes), plus a new shim-based test covering the skip path when a stale (`Prunable`) candidate's preparation fails at removal time. The hook-dirtied corner has no dedicated test; existing dirty-worktree removal tests cover the command-entry gate. The one remaining uncovered added line is the `?` propagation closing the live scan closure (`prune.rs:1023`), which has no deterministic trigger. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
951dac773f |
fix(tests): give the picker's directive files a temp directory of their own, and make a PTY failure explain itself (#3619)
Chasing the Windows-only failure of `test_switch_picker_runs_execute_command` that @max-sixty flagged after #3607 merged: it failed the advisory `affected tests (windows)` leg while the required `test (windows)` leg **passed the same test on the same SHA** in the same run. ## The failure message was the whole problem ``` assertion `left == right` failed: Expected exit code 0 for picker switch with --execute left: 1 right: 0 ``` That is everything CI had. wt prints an error before exiting non-zero, so there was a message — it just never reached the captured frame, because of two harness gaps: - **The post-exit drain was a single non-blocking sweep**, taken the instant `try_wait` reported the child reaped. The child's last writes can still be in the PTY at that moment, so the tail — wt's `✗` line — was dropped. This is also why #3427 concluded a picker-create "self-exited 1 after fully-correct work with no error on screen": the error was there, just not drained. - **`child.kill()` ran unconditionally after the exit poll.** On Windows that is `TerminateProcess(proc, 1)`, so a hung picker and a picker that failed on its own both report exit 1 — the ambiguity that made #3427 reason from wall-clock time to rule out a kill. Both are fixed here: drain until the output goes quiet, and panic on a hang rather than killing silently, so every exit code a caller asserts on is one the child chose. The four directive tests now also dump the screen (and the EXEC file) in their exit-code assertions. ## What reproducing it turned up Loop-based reproduction of the reported symptom is hopeless by the numbers: across the last 38 PR runs, exactly one advisory job failed — this one — i.e. ~1 failure per 160,000 test executions. What *did* reproduce was a different Windows failure in the same five tests, at a rate 1000× higher, once the loop ran with realistic neighbours (the full suite running alongside, as the failing leg had at test 3111/4204): ``` thread '…switch_picker::test_switch_picker_emits_cd_directive_by_default' panicked at tests\integration_tests\switch_picker.rs:2531:45: failed to create cd temp file: … PathError { path: "D:\\tmp\\.tmp3XI9wb", err: Os { code: 5, kind: PermissionDenied, message: "Access is denied." } } ``` **4 of 80 runs of the five directive tests.** `directive_files()` called `NamedTempFile::new()` in the shared temp directory; `tempfile` retries a name collision only when it surfaces as `AlreadyExists`, and on Windows `create_new` against a name held by a *directory* — or by a file in delete-pending state — returns `PermissionDenied`, which it hands back to the caller. A full suite run fills that directory with `.tmpXXXXXX` entries (every `TestRepo` makes one), so the collision is routine rather than astronomical. The helpers now take a `TempDir` and put fixed-named files inside it. `TempDir::new` isn't exposed to the same problem — a directory collision surfaces as `AlreadyExists`, which tempfile retries — and fixed names inside a private directory can't collide with anything. That covers all 75 call sites of `directive_files()` / `legacy_directive_file()`, not just the picker's five, and the switch_picker copy of the helper is gone. ## The reported exit-1 is not proven fixed Stated plainly: the panic above is a *different symptom* from the reported exit 1, so this PR does not prove that failure is gone. What the evidence does say, and what a reader should check first if it recurs: - The failing run took 3.34s where the same test passing on the required leg took 3.11s and the same switch *without* `--execute` takes 2.42s. The `--execute` tail — `build_hook_context`'s git lookups, ~0.7s of the difference — had therefore run, so the failure was at the end of the pipeline, not in the picker or the switch. - After that tail, the only fallible step left is the EXEC directive append. Every other step is infallible or `.ok()`-swallowed. - That write's error was context-free, which is why nobody could place it. `wt` rendered `✗ No such file or directory (os error 2)` — no file, no operation, and positioned right under `◎ Executing (--execute):`, where it reads as if the *command* were missing. It now reads: ``` ✗ Failed to write the command to the directive file /tmp/tmp.aBc No such file or directory (os error 2) ``` The message lives in `append_line` / `write_cd_path` rather than at their four call sites, so every directive write says which file and what didn't get written — including the legacy-protocol arms, which no test can drive to failure. So the next occurrence prints its own diagnosis, in one look, instead of `left: 1, right: 0`. ## Also here **The four directive tests select their row by cursor, not by a typed query.** They typed `target` and waited for `target-branch`'s preview pane before Enter. That gate can only assert what skim *painted*: the filtered item list is swapped into `ItemList` during a render, while `Accept` reads `item_list.selected()` — the cursor's slot in whatever list is current at that moment. The list-pane `>` pointer comes from the same render state the accept reads, which is why this file's own `wait_for_cursor_on_row` calls it the race-free signal. One Down reaches `target-branch` (the picker sorts the current worktree first) and the arrow is re-issued until the pointer settles, so no query — and no matcher — is in the accept path at all. It also drops the wait on the preview pane rendering. ## Testing - Reproducer and confirmation both on Windows CI, via a temporary workflow (deleted in the last commit): 80 runs of the five directive tests with the full suite running alongside. **4 failures before the fix, 0 after.** The concurrent full suite passed 4208/4208 in both. - Local: `switch_picker` (68), plus every suite that calls these helpers — `directives`, `switch`, `remove`, `security`, `bare_repository`, `step_alias`, `user_hooks`, `step_relocate` (861 tests) — green, plus clippy `-Dwarnings` on all targets with `--features shell-integration-tests`. - The two directive-write messages are pinned by tests of their own (`test_exec_directive_write_failure_names_the_file`, `test_cd_directive_write_failure_names_the_file`): an unwritable directive path fails the same way on every platform, so the paths that produce these messages are covered rather than checked by hand. ## Follow-ups, not in here - `build_hook_context` is called with `referenced: None` on the `--execute` path, so a `-x 'echo hi'` with no template variables still runs the `default_branch`, `primary_worktree`, `commit`, `short_commit`, `remote`, and `upstream` lookups — roughly doubling the command's subprocess count (30ms → 60ms locally, 2.42s → 3.11s in the Windows test timings). The alias path already passes its referenced set; a `--execute` command receives no JSON context, so the reason hooks pass `None` doesn't apply to it. - `cancel_background_commands()` is a no-op on Windows (`signal_background_pid`), so the picker's "the picker's exit cancels the unfinished request" promise isn't kept there: preview git children keep reading `.git/config` while the accept path writes `worktrunk.history` and `worktrunk.default-branch` to it. Issue #2564 is the same shape of concurrent-config-access failure on Windows. - `assert_valid_create_exit_code`'s Windows tolerance (#3427) and `assert_valid_abort_exit_code`'s acceptance of `1` both rest on the kill-vs-self-exit ambiguity this PR removes; they can be tightened once a Windows run confirms nothing else produces a stray 1. > _This was written by Claude Code on behalf of Maximilian Roos_ |
||
|
Maximilian Roos
|
9032308400 |
refactor(tests): give the PTY test environment one home (#3618)
Follow-up to #3616, which fixed a PTY snapshot flake by adding one env knob — and to add it I had to touch three separate env builders, none of which knew about the others. This consolidates that surface. ## What was there The environment a test subprocess runs in was assembled at nine sites: five copies of the PTY prologue (`env_clear`, HOME, PATH, the Windows block, coverage passthrough) and four partial restatements of the determinism knobs. There was no rule for which belonged where, so adding a knob meant finding every copy, and a missed copy surfaced later as a flake somewhere unrelated. ## What's there now Three named layers, each with one home in `src/testing/mod.rs`: | Layer | Home | Contents | |---|---|---| | Baseline | `STATIC_TEST_ENV_VARS` | knobs every child needs, whatever it's attached to | | Terminal | `PTY_TEST_ENV_VARS` (new) | knobs only a TTY triggers — `WORKTRUNK_TEST_SPINNERS=0` | | Fixture | `pty_env_vars(TestEnvPaths { … })` (new) | the paths that vary per fixture | `configure_cli_command` and `configure_pty_command` apply them by transport. That turns the standing `// NOTE: TERM is intentionally NOT in STATIC_TEST_ENV_VARS` comment into a consequence rather than an exception: `TERM` is transport-level, so it can't sit in a baseline both transports share. `WORKTRUNK_TEST_SPINNERS` stays out of the shared baseline deliberately. It's inert on a pipe, and insta-cmd records the whole environment into every snapshot it writes (`Info::from_std_command` builds it unconditionally from `cmd.get_envs()` — there's no hook to suppress it), so putting it there would add a no-op line to 1043 snapshot files. `configure_pty_command` is now the only place a PTY child's isolation is set up. `shell_command`, `execute_shell_script`, `configure_pty_environment`, `exec_in_pty_shell`, `exec_bash_truly_interactive` and two `wt switch` spawns all delegate to it. `shell_wrapper`'s `STANDARD_TEST_ENV` and `bare_repository`'s hand-rolled `test_env_vars` are gone, as are `configure_shell`'s hand-copied knobs and four redundant `CLICOLOR_FORCE` lines in `switch_picker`. Net −149 lines. One spawn stays outside: the Windows ConPTY smoke test, which runs PowerShell against a deliberately bare environment and isn't a wt child at all. ## Reviewing Start at `src/testing/mod.rs` — the three layers and their doc comments are the whole design. Everything under `tests/` is deletion plus a delegation call. Two snapshots change: `install_preview_with_gutter` and `install_preview_declined` now carry ANSI, because those two tests previously ran without `CLICOLOR_FORCE`. Text is identical. Arguably a fix — the test named "with_gutter" couldn't see the gutter (a background-color block), while its own prompt line was already colored, so the file was internally inconsistent. ## Testing Full `wt hook pre-merge --yes` green: 4601 tests, `--features shell-integration-tests`, `RUSTFLAGS='-D warnings'`, `insta --check`. The knob's delivery path was verified by probe rather than by inspection. With `sleep 6` in the mock `llm`, `test_readme_example_hooks_pre_merge` passes; flipping `PTY_TEST_ENV_VARS` to `"1"` reproduces the original failure byte for byte: ``` +␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (4s)␛[22m␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (5s)␛[22m ``` So the knob reaches the shell-wrapper PTY child through the shared setup, not through a surviving copy. Both probe edits are reverted. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
d6ef23d573 |
test(pty): suppress the TTY spinners in PTY captures (#3616)
`test_readme_example_hooks_pre_merge` flakes when the whole suite runs together. The mock `commit.generation` command gets starved past the watchdog's 4s threshold, and the PTY capture records the redraw frames a terminal would have erased: ``` ↳ Waiting for the commit generation command (4s) ``` The elapsed seconds vary run to run, so accepting the snapshot just moves the flake. ## The fix Both spinners in `src/progress.rs` now check `WORKTRUNK_TEST_SPINNERS` alongside the existing TTY (and, for `Watchdog`, verbosity) gate, and the PTY env builders set it to `0`. This is the shape the repo already uses for `WORKTRUNK_TEST_DELAYED_STREAM_MS=-1`, whose comment names the same failure mode: "slow CI triggers progress messages that don't appear on faster systems". Only the render is gated, so the counters behind the summary lines the snapshots do assert are untouched. Gating both spinner types rather than the one call site closes the latent exposure elsewhere: `Progress` renders after 300ms in any PTY test that copies or removes files, and `Watchdog` also wraps `wt switch` and the version check. ## Why not filter the frames out of the capture A line-scoped filter in `add_pty_filters` would be a one-liner, but it only covers the single-row block. Past 10s the watchdog escalates to two rows and redraws with a cursor-up, which spans a newline the filter cannot follow, and the revealed command line carries a tempdir path. Closing that would mean modeling cursor arithmetic in a regex that silently deletes bytes from captured output. ## Evidence Reproduced first, by making the mock command slow directly rather than waiting for ambient CPU starvation to do it. The resulting diff is byte-identical to the reported flake, including the `(4s)` / `(5s)` counters: - `sleep 6` in the mock, before the fix: FAIL, same diff as reported - `sleep 6`, after the fix: pass - `sleep 12` (past the 10s escalation tier, where a filter would not have helped): pass - negative control, `sleep 6` with only `WORKTRUNK_TEST_SPINNERS` flipped from `0` to `1`: FAIL again, so the knob is what engages Full gate green: 4601 tests, no pending snapshots. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
63071709af |
feat(config): deprecate list.task-timeout-ms (#3615)
Removes the `[list] task-timeout-ms` per-task command timeout and the thread-local machinery behind it. The key stops having any effect immediately; a config that still carries it loads, warns, and is stripped by `wt config update`. The timeout killed any git command that outlived its budget, on every collect worker, through a thread-local that `Cmd::run` consulted on each invocation and clamped an explicit `.timeout()` against. Progressive rendering removed the reason for it: `wt list` and the picker paint from local data and stream results in behind the frame, so no single git command can hold up the first paint. What it bounded instead was completion, which `[list] timeout-ms` already bounds directly, and the drain falls back to a hardcoded 120s `DRAIN_TIMEOUT` whenever that is unset (`collect/mod.rs` `drain_deadline`), so removing this cannot introduce an unbounded wait. Both keys default to unset, so the default path never had a per-task timeout at all. `[list] timeout-ms`, the wall-clock budget for the whole collect phase, stays. It is the surviving knob and the more direct expression of the same goal. With the thread-local gone, the two-source `min()` in `Cmd::run` collapses to the command's own `self.timeout`, so every explicit `.timeout()` caller keeps its bound unclamped: `PROBE_TIMEOUT` in `git/reap.rs`, the fsmonitor stop/lsof bounds in `git/remove.rs`, the version check in `config/show.rs`, and `REMOTE_DETECTION_TIMEOUT`. ## Deprecation A `Structural` row in `DEPRECATION_RULES` strips the key from `[list]` in both the section and inline forms, top-level and per-project, following the `[switch.picker] timeout-ms` precedent (also a strip with no equivalent key to migrate into): ``` ▲ User config: list.task-timeout-ms is no longer used — list.timeout-ms bounds the collect phase ``` The env overlay (`WORKTRUNK__LIST__TASK_TIMEOUT_MS`) and `--config-set` route through the same rule and migrate silently, since neither layer has a file for `wt config update` to materialize. Neither errors. ## Testing New unit tests cover detection and migration for the section, inline, and per-project forms plus the warning text, and two cases join the `test_warning_fires_iff_update_changes` battery that pins the warn-iff-update-changes invariant. The three integration tests that exercised the feature are gone; `Cmd::timeout` keeps its own coverage in `shell_exec.rs`, so dropping the four thread-local unit tests loses nothing for the surviving path. Verified end to end that a config carrying the key loads, warns, and has it stripped by `wt config update` with sibling keys intact. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
14580de79c |
feat(worktree): accept a worktree path wherever a branch is accepted (#3607)
Follow-up to the [`wt remove <path>` discussion on #3480](https://github.com/max-sixty/worktrunk/pull/3480#issuecomment-5039137116), widened from that one command to the whole surface. #3480 has since landed and is merged in here — its duplicate-checkout warning composes with this: the warning names the shadowed worktrees, and a path is how you then address one. ## Audit Verified against the built binary. wt had three answers to "what does this token mean?": | Route | `@` `-` `^` | worktree path | `pr:N` | |---|---|---|---| | `wt switch` (`resolve_switch_target`) | yes | only if absolute or ≥2 components, and not `--create` | yes | | `wt remove` (`resolve_worktree_arg`) | yes | any token | no | | everything else (raw `worktree_for_branch`) | **no** | **no** | no | Same token, same cwd, two answers: ```console $ wt remove inner # ✓ Removed innerbranch worktree & branch $ wt switch inner # ✗ No branch named inner ``` And outside switch/remove the shortcuts didn't work at all — `wt step diff --branch @` was `✗ Branch @ has no worktree`, while `wt config state marker set --branch @` silently wrote state under the literal key `@`. Separately, wt prints paths as `~/…` but wouldn't accept that form back. ## Change One canonicalizer in the lib, `Repository::resolve_worktree`, absorbing the path fallback that lived in the bin crate's `resolve_worktree_arg` (now deleted). Resolution order is documented once, on that function: `@`, then `-`/`^`, then a branch with a worktree, then a path naming a registered worktree, then the branch alone. **Branch-first, everywhere.** A directory never shadows a branch that shares its name; a path answers only what a branch cannot — a detached worktree, or one of two checkouts of the same branch (#3480's case). The `looks_like_path` shape gate is gone, so a single-component path resolves like any other. Two shapes cover what callers need: `require_worktree` for commands that need a worktree to operate in, `require_selected_branch` for arguments that key by branch. The merge/rebase target validators fall through to the same path lookup, so a target can be named by the worktree it's checked out in. Routed through it: `switch` (including `--base`), `remove`, `step commit --branch`, `step diff --branch` and its target, `step copy-ignored --from`/`--to`, `step promote`, `step relocate`, `config state --branch` (9 sites), and `merge` / `step rebase` / `step squash` / `step push` targets. `resolve_input_path` — already documented as the one resolution point for user-supplied paths — now expands a leading `~`, so the tilde form worktrunk prints is a form it reads back. `~user` stays literal; wt doesn't reimplement that shell feature. ## Documentation A path is an alias, not a second addressing scheme, so it is stated once rather than on every argument: one paragraph in `wt switch`'s help and one sentence on the addressing line in `worktrunk.md`. Argument descriptions still read as branches. The two exceptions are the arguments whose descriptions are already catalogues of accepted forms — `wt switch`'s (`Branch, worktree path, shortcut, or PR/MR URL`) and `wt remove`'s, which has named the path since before this branch. The Worktree Model section of `CLAUDE.md` records which way to document it, so the next argument doesn't grow its own copy. ## Two silent no-ops fixed along the way - `wt step relocate <unmatched>` matched arguments against branch names by string equality, so a typo filtered everything out and the empty result rendered as `○ All worktrees are at expected paths` — a success message for work that never happened. Every way an argument can fail to land on a relocatable worktree now errors, including the detached and prunable cases the new path route makes reachable. - A selector matching nothing was reported as a branch without a worktree, hinting `wt switch <token>` — which creates a worktree only when the branch exists, so for a mistyped path it would just fail again. `WorktreeSelectorNotFound` now says `No branch or worktree named X`; a branch that genuinely exists without a checkout keeps the create hint. ## Testing Full gate green: 4596 tests, lints, docs sync, `--features shell-integration-tests` clippy. `codecov/patch` is 99.25% of diff hit against a 97.93% target. New coverage: - Unit: branch-and-path equivalence, branch-beats-same-named-directory, detached-by-path (and its `require_selected_branch` refusal), shortcuts never treated as paths, branch-only fallthrough, and the two distinct not-found errors. Plus `expand_tilde` round-tripping `format_path_for_display`. - Integration: `switch` by relative/single-component/absolute/tilde path, `--base` by path, `step diff --branch` by path and `@` (asserted equal to the by-branch output), `config state --branch` set via `@` and read via the worktree path, and both new relocate errors. `wt remove`'s resolution is unchanged — it already had this rule; it now shares the implementation. The 106-test `remove::` suite is untouched and green. - Integration: `wt step push <worktree-path>` (the `require_target_branch` half of the target fallback), and `wt step relocate` against a prunable worktree. One diff line is unhit: `expand_tilde`'s fallback when `home_dir()` returns `None`, which has no deterministic trigger. The `@`-resolution backstop in `resolve_worktree` is untested for the same reason — no CLI route reaches it — so it kept its original `match` arm rather than being re-indented into the diff. ## Left out `wt config state default-branch set` and `previous-branch set` take a branch name as a *value to store* rather than a selector, so they still take it literally. > _This was written by Claude Code on behalf of Maximilian Roos_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
b8299aca34 |
docs(push): record that destination-worktree safety matches git (#3614)
Comment-only. It records a decision rather than changing one: `wt merge`
and `wt step push` overwrite an ignored file in the destination worktree
whose path the incoming commits track, and that is intended.
`prepare_target_worktree` works from `git status --porcelain`, which
omits ignored files. So the existing protection covers modified-tracked
and untracked collisions (refused as `ConflictingChanges`) and
autostashes the rest of a dirty tree, while an ignored file at a
colliding path is neither refused nor stashed.
git draws the line in the same place. A plain `git merge` run in that
worktree, against a scratch repo:
```
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 a probe that refused here would make `wt` stricter than the tool it
wraps, over data git treats as expendable. Three places say so now, each
where someone would otherwise reach the opposite conclusion:
- **`src/commands/worktree/push.rs`** — the module spec, at the point
where such a probe would be added. It names the mechanisms rather than
arguing by analogy: the fast-forward hands the checkout to
`receive.denyCurrentBranch=updateInstead`, `--no-ff` syncs with
`read-tree -m -u`, and both run git's unpack-trees checks. Confirmed
against a scratch repo — `read-tree -m -u` silently overwrites an
ignored file at a colliding path and refuses an untracked one (`exit
128`, file intact), the same split `git merge` produces.
- **`src/commands/repository_ext.rs`** — `prepare_target_worktree`,
which owns the status read, pointing at that spec from the
implementation site. (A `[`crate::…`]` intra-doc link was the first
attempt; `push` is a private module, so `RUSTDOCFLAGS=-D warnings`
rejected it. Plain path instead.)
- **`CLAUDE.md`** — Data Safety, bounding "prefer failure over silent
loss" at git's edge. Read unqualified, that rule is what invites the
stricter check.
Follows #3601, which fixed the user-facing half: the FAQ had recommended
`git worktree lock` for "precious ignored data", which the lock does not
protect — it blocks removal only.
## Verification
`RUSTDOCFLAGS="-D warnings" cargo doc`, `cargo clippy --all-targets --
-D warnings`, and pre-commit all clean.
The local gate flagged `test_readme_example_hooks_pre_merge`, which is
the known load flake — an extra `Waiting for the commit generation
command (4s)` spinner line under full-suite CPU contention. It passes in
isolation with no retries. The diff contains no executable lines at all
(every changed line is `///`, `//!`, or markdown), so it cannot be
causal.
> _This was written by Claude Code on behalf of Maximilian Roos_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
||
|
Maximilian Roos
|
04d8d587f2 |
feat(list): flag a branch checked out in more than one worktree (#3606)
## Problem Follow-up to #3480. That PR made a duplicated branch checkout (`git worktree add --force <path> <branch>`) visible at *resolution* time: `worktree_for_branch` warns once per branch, then resolves to whichever worktree git lists first. `wt list` said nothing about it. Two rows named `feature`, and no column that explains why. The nearest thing to a signal was accidental. A force-added duplicate usually lands off-template, since the original holds the template path, so it picks up `⚑` for the location mismatch — while the worktree *at* the template path, the one `wt` actually resolves to, carried no flag at all. Exactly backwards from what's useful. The framing from the request: a worktree in the wrong location gets a status flag; a worktree sharing its branch should get one too. ## Solution `⚑` now covers both, on every worktree of the duplicated branch, resolved one included. Which worktree `wt` picks is git's listing order, so singling out the shadowed rows would imply a legitimacy the ordering doesn't carry. ``` @ main ^| | . 05a4a45d 16h Initial commit + feature ⚑_ ../repo.feature 05a4a45d 16h Initial commit + feature ⚑_ ../repo.feature-dup 05a4a45d 16h Initial commit ``` This started as a seventh glyph (`⧉`) and collapsed onto `⚑` in the second commit. The Status column is a dense alphabet the reader has to learn, and the two states say one thing: this worktree's place in the branch ⇔ worktree map is irregular. Off-template path and branch-claimed-twice are both instances. The table already distinguishes them without a glyph — a repeated Branch cell is the duplicate, an odd Path cell the mismatch — so the flag only has to say "not a rendering glitch, look at the Path column". Sharing the glyph means sharing its dim-yellow styling, since the codebase treats a symbol's color as part of its identity; #3480's warning remains the loud channel, firing the moment any command resolves the branch. **The Path column comes along.** It previously appeared only for a location mismatch, on the reasoning that the path is otherwise redundant with the branch. A duplicate inverts that: the branch name no longer identifies the row, and the path is the only thing telling the two apart. The layout flag is renamed from `has_branch_worktree_mismatch` to `path_is_informative` to say what it now means. **The data model keeps the distinction.** JSON has no cardinality budget, and reporting a duplicate that sits at the template path as `branch_worktree_mismatch` would be false — its path does match. Schema 1's `worktree.state` names the cause (`"duplicate_branch"`), schema 2 gets its own `worktree.duplicate_branch` bool beside `branch_mismatch`, matching that schema's one-fact-per-field shape. The priority between the two `⚑` states now decides only which cause JSON reports. Detection is one pass over the worktree list (`duplicated_branches`, next to #3480's `worktree_paths_for_branch`), in memory, pre-skeleton, no git calls. ## Testing - `test_worktree_paths_for_branch_detects_duplicates` gains the set form and a detached-HEAD worktree, which has no branch to duplicate. - `test_metadata_worktree_state_priority` covers the two `⚑` states' ordering and both yielding to `⊟`/`⊞`. - `test_list_duplicate_branch` snapshots the table above, showing both flagged rows and the Path column earning its place. - `test_list_duplicate_branch_json` asserts both schemas flag exactly the two duplicated rows. The flag only fires on a state no prior test sets up, so the only snapshot churn is the help pages and the schema-2 envelope's new field. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
4d1a669b6f |
fix(list): honor task-timeout-ms for a command with its own bound (#3613)
Two follow-ups from #3603. ## `[list] task-timeout-ms` didn't bound a command that set its own timeout `Cmd::run` resolved its two timeout sources as a precedence chain — `self.timeout.or_else(thread_local)` — so a command carrying an explicit `.timeout()` escaped a shorter budget its caller had set. `wt list` sets exactly such a budget on every collect worker from `[list] task-timeout-ms`, and the 10s bound #3603 put on remote default-branch detection sits inside one. With `task-timeout-ms = 50`, that command ran for the full 10s. Both values are ceilings — one bounds a command, the other bounds every command on the thread — so the tighter of the two now wins. The key's own documentation already promises that it "kills individual git commands that exceed this duration", so this makes the documented behavior true rather than changing it; no doc update goes with it. I confirmed the defect rather than inferring it: with the old line restored, the new test's command runs the full 10.01s against a 50ms budget, 200× over. The setting is unset by default, so nothing changes for a default config. The one nearby thing this could have clamped is the picker's 2s pager bound, and it can't — `pager.rs` calls `child.wait_timeout` on a raw `Child`, not through `Cmd`. `COMMAND_TIMEOUT`'s doc named the `wt switch` picker as its driver and omitted the config key that actually feeds it, which reads as though the thread-local has no production callers at all. It now names the key and says it's unset by default. ## A Windows CI flake in `test_cmd_run_file_current_dir_is_errored` The test created a `NamedTempFile` purely to get a path that isn't a directory, and on Windows the creation itself failed: ``` PathError { path: "D:\\tmp\\.tmpWKLe5F", err: Os { code: 5, kind: PermissionDenied, message: "Access is denied." } } ``` tempfile retries only a *reported* collision (`AlreadyExists` / `AddrInUse`), so a denial propagates. The error the test actually exercises comes from `check_spawn_preconditions` — `metadata` plus `is_dir`, before anything spawns — so the test needs nothing more than an existing non-directory path. The test binary is one, and using it means the test creates nothing under the shared temp root. ### What I did not do The obvious next step is a prefix-separation sweep: `TempDir` and `NamedTempFile` draw names from one namespace (a `.tmp` prefix plus six random characters), and this suite feeds it ~260 default-prefix `tempdir()` sites plus a temp directory per test repo, so a temp *file* name can land on a temp *directory* name. That would explain a denial rather than a collision report — but only if Windows returns `ACCESS_DENIED` where a file creation hits an existing directory name, instead of `FILE_EXISTS`. I can't test that from macOS, and it's the load-bearing link, so I left the other eleven `NamedTempFile::new()` sites alone rather than sweep production (`src/commands/step/commit.rs`, `src/git/repository/working_tree.rs`) on an unverified premise. Worth doing if someone can confirm the semantic. ## Testing The timeout fix carries a regression test that fails without it (`Ok` after 10s) and passes with it. The Windows fix is verified only by the local suite — the flake it removes is rare and Windows-only, so CI going green here is consistent with it but doesn't demonstrate it. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
cad89eaebb |
fix(relocate): skip worktree blocked by an immovable occupant (#3530)
## Problem `wt step relocate` could strand a worktree in the internal staging directory and then error out. When worktree **A**'s target is occupied by worktree **B**, A depends on B vacating. But if B is itself *blocked* — its own target is a non-worktree path and `--clobber` wasn't passed — B never moves. The dependency-resolution loop's no-progress branch assumes "no progress ⟹ a cycle to break," so it temp-moved A into `.git/wt/staging/relocate/`. `finalize_temp_relocations` then ran `git worktree move <temp> <A's target>` into the still-occupied path, which fails and propagates via `?` — leaving A relocated into the staging dir rather than at either its original or expected path. ### Reproduction - A at `/w/a` wants `/w/b`; B at `/w/b` wants `/w/c`; `/w/c` is a plain non-worktree file, no `--clobber`. - B is classified `target_blocked` and never moves. - A is temp-moved by `break_cycle`, then `finalize` fails with "Failed to move worktree from temp to final location", stranding A in staging. ## Fix `is_target_empty` only distinguished "occupant moved" from "occupant present"; it couldn't tell a still-pending occupant from a permanently-blocked one. A new `blocked_occupant` helper detects when a dependent's target is held by a worktree already in the `blocked` set, and the loop propagates the block to the dependent (skipping it cleanly) instead of handing it to `break_cycle`. Setting `made_progress` re-drives the loop so a chain of dependents (A → B → C-blocked) is blocked transitively. After the fix both worktrees are skipped and the command succeeds: ```console ▲ Skipping beta (target blocked: repo.beta) ↳ To backup blocking paths, use --clobber ▲ Skipping alpha (blocked by beta, which can't be relocated) ○ Relocated 0 worktrees, skipped 2 worktrees ``` ## Test `test_relocate_blocked_occupant_skips_dependent` sets up exactly this shape and asserts the command succeeds, both worktrees stay put, and nothing is stranded in `.git/wt/staging/relocate/`. Before the fix the command exited non-zero and `alpha` was left in the staging dir, so both the snapshot (`success: true`) and the location assertions would have failed. Found during the nightly code-quality survey of `src/commands/relocate.rs`. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Worktrunk Bot
|
901b79ab52 |
ci: bump pinned worktrunk to 0.69.2 (#3612)
Weekly CI pin bump. Only `worktrunk` drifted since last week — every other `baptiste0928/cargo-install` pin (cargo-msrv `0.19.3`, cargo-llvm-cov `0.8.7`, cargo-insta `1.48.0`, cargo-nextest `0.9.140`, cargo-udeps `0.1.61`, lychee `0.24.2`), plus `setup-nu` `0.114.1` and `zola@0.22.1`, is already at the latest upstream release. - **worktrunk** `=0.68.0` → `=0.69.2` (3 sites: `ci.yaml` ×2, `nightly.yaml`) Compatibility: worktrunk `0.69.2` declares `rust-version = 1.96`, matching the pinned toolchain (`rust-toolchain.toml` channel `1.96.0`), so it builds under `baptiste0928/cargo-install`. MSRV/toolchain needed no bump this week: current stable is `1.97.1`, so latest−1 is `1.96`, which is already what `Cargo.toml`, `tests/helpers/wt-perf/Cargo.toml`, and `rust-toolchain.toml` pin. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Worktrunk Bot
|
e9b0bca15c | docs(log-files): init() creates three sinks, not two (#3610) | ||
|
Worktrunk Bot
|
4cf3006041 | fix(taskfile): drop codex from bench list without leaving an empty element (#3611) | ||
|
Maximilian Roos
|
32f380a27b |
refactor(switch): forward the forge CLI's own error text (#3605)
Follow-up to #3600, independent of it (no shared files). GitHub and GitLab rewrote forge API failures into messages of our own, discarding what the CLI said. GitLab picked its three by prose — `error_text.starts_with("404")` — the pattern #3595/#3597 removed elsewhere. ## Before / after A GitLab MR that doesn't exist: ``` - ✗ MR !9999 not found + ✗ glab api failed for MR !9999 + ▎ glab: 404 Not found (HTTP 404) ``` A GitHub PR that doesn't exist — the one message kept, now with gh's verdict under it rather than instead of it: ``` ✗ PR #999999 not found on max-sixty/worktrunk (gh default). Check that `gh repo set-default` points to the correct repository. + ▎ gh: Not Found (HTTP 404) ``` ## The rule, now written down `src/git/remote_ref/mod.rs` gets an **Error Messages** section: forward the CLI's own line, and write our own only where the CLI *structurally cannot* report the condition. Two cases qualify — GitHub's 404 (it answers about an owner/repo *we* chose, from `gh repo set-default` or a remote; gh can't know that) and Azure's missing `azure-devops` extension (a precondition `az extension list` answers, which `az` never names). Reading more nicely than the CLI doesn't qualify. `cli_api_error` and `cli_api_error_details` gained docstrings for the mechanism: our context as the headline, the CLI's words in the gutter, so a provider with something to add passes it as `message` rather than bailing. ## Evidence Measured, not assumed — both CLIs render an API failure the same way: | | stdout | stderr | |---|---|---| | `gh api …/pulls/999999` | `{"message":"Not Found",…,"status":"404"}` | `gh: Not Found (HTTP 404)` | | `glab api …/merge_requests/…` | `{"message":"401 Unauthorized"}` | `glab: 401 Unauthorized (HTTP 401)` | `cli_api_error_details` prefers stderr, so the gutter gets the human line, not the JSON. That also surfaced a test bug: **the glab mocks never set stderr**, so they'd have enshrined output real glab doesn't produce. Fixed to match the probe. `gh` puts the status in a structured field, so its 404 arm keys on `status`. `glab` puts it only inside the message text — nothing to key on, which is why prose-matching was the only way to keep those arms. ## What this gives up A bad or expired token no longer gets "run gh auth login" / "run glab auth login"; it gets `gh: Bad credentials (HTTP 401)`. The far more common case — no auth configured at all — is unaffected: gh prints its own "please run: gh auth login" and always did fall through to forwarding. Happy to restore the GitHub 401 arm (it's structurally keyable); GitLab's can't come back without prose-matching. ## Testing `cargo run -- hook pre-merge --yes` → 4572 passed. Six snapshots reviewed individually before accepting (five updated, one new). Both paths also driven through the **real** `gh` binary (a live 404 and a live 401), not only mocks. Clippy clean with `--features shell-integration-tests`. Coverage: the GitHub 404 arm and the forwarding fallthrough each have a test, on both forges. `codecov/patch` also flagged one line — re-indenting the 404 block pulled a pre-existing gap into the patch, and it turned out to be real: the `gh repo set-default` half of the hint, the fork workflow the message exists for, had never been tested. `test_switch_pr_not_found_gh_default` closes it. > _This was written by Claude Code on behalf of Maximilian Roos_ |
||
|
Maximilian Roos
|
9645e3e132 |
fix(shell): reclaim the legacy wrapper paths instead of inspecting them (#3602)
`wt config shell install` decided whether the file at a legacy location
was worktrunk's by reading it — a substring test for the wrapper header,
falling back to "every code line looks like an integration line".
Anything that didn't match was left in place. Fish sources `conf.d` at
startup, so whatever `conf.d/{cmd}.fish` defines is already loaded by
the time fish would autoload the `functions/{cmd}.fish` the install just
wrote — the stale definition wins and the new wrapper may never load.
Ownership is now the path. `conf.d/{cmd}.fish` and the stranded nushell
`{cmd}.nu` candidates are paths worktrunk itself computes for the
command name being installed, so install takes them back whole without
reading them. Only that exact filename is touched: a neighbour under
another name is not worktrunk's, and each removal is still reported.
`wt config shell uninstall` still reads the header, because it takes no
`--cmd` and so genuinely doesn't know the name — it lists the
shell-owned directories and has to tell our `{cmd}.fish` from the user's
own files beside it. That is the one place ownership can't come from the
path, and it prompts and previews every file before removing it, which
install does neither of. `is_worktrunk_managed_content`'s docstring now
says so; `is_worktrunk_managed_nushell` existed only to gate the
install-time deletion and is gone.
Audited the rest of the surface for the same principle: the OpenCode
plugin (`~/.config/opencode/plugins/worktrunk.ts`) is already removed by
path with no inspection, the Claude plugin delegates to `claude plugin
uninstall`, Gemini installs through `gemini extensions install` and
leaves nothing of ours on disk, and fish is the only shell with a
separate completion file — which uninstall already covers. No other
gaps.
## Data safety
This deliberately widens what install deletes, so it's worth naming: two
tests asserted the old conservatism and now assert the new boundary —
`test_configure_shell_fish_reclaims_conf_d_path` (was
`..._preserves_user_conf_d_file`, from #3589) and
`test_nushell_install_reclaims_only_the_command_name` (was
`..._keeps_unmanaged_legacy_file`, from #2992). Both put a neighbour
file in the same directory to pin that only `{cmd}.{ext}` is taken. The
FAQ's "What can Worktrunk delete?" inventory is updated to describe
path-based ownership rather than the old content-marker rule.
> _This was written by Claude Code on behalf of max_
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Worktrunk Bot
|
bcd1ffdfde |
feat(worktree): warn when a branch is checked out in multiple worktrees (#3480)
## Problem `git worktree add --force <path> <branch>` bypasses git's "already used by worktree" guard, so the same branch can be live in two worktrees at once — breaking worktrunk's branch ⇔ worktree bijection. worktrunk never creates that state itself, but once it exists [`worktree_for_branch`](https://github.com/max-sixty/worktrunk/blob/cfd7fe469/src/git/repository/worktrees.rs#L64) silently resolved to whichever worktree git listed first (roughly creation order), and that choice flowed into every resolution path — `wt switch`, `wt push`, `wt step diff`, `wt context`, the picker — with no warning, error, or way to name the shadowed worktree. This implements the **detect-and-warn** option from #3392: make the invisible choice visible without changing resolution semantics. ## Solution - `worktree_for_branch` now collects **all** worktrees on the branch via a pure `worktree_paths_for_branch` helper; when more than one exists it warns once (per branch, deduplicated per process) and still resolves to the first — semantics are unchanged. - The warning names every path and points at a concrete fix: ``` ▲ Branch feature is checked out in 2 worktrees; wt uses the first: ┃ ~/repo.feature ┃ ~/repo.feature-dup ↳ To drop a duplicate, run git worktree remove ~/repo.feature-dup ``` Detection is separated from emission so the pure list logic is unit-testable and the ambiguity check stays cheap — `list_worktrees()` already holds the full list. ## Testing - Unit test `test_worktree_paths_for_branch_detects_duplicates` — the pure helper returns both paths in git's listing order for a duplicated branch, one path for a unique branch, and none for an absent one. - Integration test `test_step_diff_duplicate_branch_warns` — creates the duplicate with `git worktree add --force`, runs `wt step diff --branch=feature`, and snapshots the rendered warning. The warning only fires when a duplicate exists (which no prior test sets up), so existing snapshots are unaffected. --- Closes #3392 — automated triage --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Maximilian Roos
|
8865f20ab8 |
fix(git): bound the default-branch remote query, and make the bound bound (#3603)
Follow-up to #3596, which walled the test suite off from the network. That PR left the same unbounded call live for real users: `Repository::default_branch()` is the one detection helper allowed to fall through to `git ls-remote`, and nothing limited how long it could take *not* to answer — an unanswered SYN costs ~127 s per address on Linux (`tcp_syn_retries=6`) and git tries each of a host's addresses in turn, so a remote behind a dropped VPN or a dead host stalled `wt list --full` and `wt switch` for minutes. ## `Cmd::timeout` didn't bound wall-clock Adding a bound alone would have been decorative, which is the substance of this PR. `run_with_timeout_impl` killed only the direct child, and a grandchild inherits the child's stderr pipe — so a surviving one held the write end open and `read_to_end` blocked until it exited. Measured on a 3 s timeout over an `ls-remote` whose upload-pack sleeps 120 s: | | elapsed | |---|---| | before | 120.03 s | | after | 3.20 s | `git-remote-https` sitting in `connect()` is exactly that shape: it doesn't notice git died. So a timed child spawns into its own process group and expiry tears down the group — `killpg` with TERM → KILL escalation on Unix, `taskkill /T /F` on Windows, matching `wt step tether`. Every existing `.timeout()` caller (the fsmonitor stop/lsof probes, `reap.rs`, the shell probe) was latently unbounded the same way and is fixed with it. The isolation costs the kernel's tty broadcast: a Ctrl-C no longer reaches a timed child, so the user waits out the remaining bound. That's seconds, against an orphan holding a pipe for as long as its own operation takes, once per spawn. Recorded at `run_with_timeout_impl` and in CLAUDE.md's Signal Handling section, whose "only the current child does" claim was no longer true. ## The cache is the other half A timed-out query takes the local-inference fallback but is *not* persisted to `worktrunk.default-branch`. That cache is what stops later calls from re-detecting, so a guess made while the network was down would otherwise become the repo's permanent answer. `detect_from_remote` now returns a `RemoteDetection` enum so the cacheability decision is explicit and exhaustive rather than an `Option` that loses the distinction. Only a timeout separates cleanly, via `ErrorKind::TimedOut`. `ls-remote` exits 128 whether the network is down or the remote simply has no HEAD, so telling those apart would mean reading git's error text — and re-querying on every command is the cost the cache exists to avoid. Those stay cached, as before. ## Reviewing - `src/shell_exec.rs` — the process-group teardown; its docstring carries the rationale - `src/git/repository/config.rs` — `RemoteDetection`, the 10 s bound, and the no-persist path - `src/git/repository/mod.rs` — `run_command` now delegates to a `run_command_bounded` that takes the bound Also routes the four remaining hand-rolled git test envs (`src/git/remove.rs`, `src/git/repository/tests.rs`, `tests/integration_tests/bare_repository.rs`) through `configure_git_env`, so they carry #3596's `GIT_ALLOW_PROTOCOL` deny rather than re-deriving a subset of it. All four run local-only git commands today, so this is completeness, not a live hole — and a full suite run under `GIT_TRACE` confirms zero `git-remote-*` transport-helper spawns across 4374 tests. ## Testing Three tests, none of which touch the network: the grandchild case via `sh -c 'sleep 30; :'` (which stops the shell `exec`ing sleep, so there really is a grandchild), the end-to-end no-persist behavior via `remote.origin.uploadpack` pointed at a `sleep`, and the fail-fast unresolvable remote, which had no coverage before. The second waits out the real 10 s bound; both hanging-remote tests are `#[cfg(unix)]` since the vehicle needs a POSIX `sleep`, so `test (windows)` doesn't cover the no-persist path. > _This was written by Claude Code on behalf of max_ |
||
|
Worktrunk Bot
|
203603909d |
test(picker): release --prs loading marker on parent exit, not a timer (#3532)
## Problem `test_switch_picker_prs_shows_loading_marker` intermittently times out on `test (windows)` — it flaked on #3531 (a docs-only change), which is what prompted this fix. The test asserts a *transient* frame: while a mocked, delayed `gh pr list` is in flight, the picker header shows `↳ Loading open PRs…` and the `#42` row has not yet streamed in. The mock held that loading state for a fixed time (originally 3s). On a loaded Windows runner, the PTY boot sequence — `boot_picker_pty`'s skim-ready wait plus its initial `wait_for_stable`, together with the async worktree-column churn (`·` placeholders resolving) — can consume that whole window. By the time the stabilization helper first polls for `Loading open PRs`, the mocked fetch has already returned and the rows have streamed in, so the marker is gone and the wait times out at `STABILIZE_TIMEOUT` (30s). ## Fix Per review feedback ([comment](https://github.com/max-sixty/worktrunk/pull/3532#issuecomment-5039146240)), tie the marker's lifetime to the picker's, with **no timing number at all** — the file's "poll, don't pick a time" rule applied to the producer side. `mock-stub` gains a `hold_until_parent_exit` response mode: the mocked `gh pr list` holds until `wt` exits instead of sleeping a fixed `delay_ms`. Detection needs no platform primitive (`getppid` / `OpenProcess`): - While `wt` lives, its `--prs` fetch thread runs the mock with a piped stdout and blocks reading that pipe to EOF, so as long as the mock neither writes a full response nor exits, the loading marker stays. - When the picker aborts, `wt` detaches the fetch thread (`drop(prs_handle)`) and exits, which closes the read end of the mock's stdout pipe. The mock's next write then fails with `BrokenPipe` (Rust ignores `SIGPIPE`, so the broken write surfaces as an `Err` rather than killing the process), and the mock exits. Polling for that write error is a **causal** parent-death signal — release is tied to `wt` exiting, not to a timer — and it behaves identically on Unix and Windows, so there's no platform-specific FFI to get right. The one-byte probes written into stdout are never observed (this mode's contract is that the parent aborts without parsing the response, and the fetch thread drains the pipe until process exit). A 60s poll cap bounds a stuck orphan if the pipe somehow never breaks. This removes the race the earlier 30s bump only widened: the marker is present for the entire window the poll could observe it, on any boot latency, and no orphaned sleeper is left behind — the mock exits the moment the picker does. ## Testing `cargo test --test integration --features shell-integration-tests test_switch_picker_prs_shows_loading_marker` passes in ~1.3s, confirming the capture-and-abort path stays fast and the mock releases promptly on `wt` exit. The sibling `--prs` tests (`test_switch_picker_prs_{github,gitlab}_list`, `..._rows_survive_alt_x_removal`) still pass through the refactored `mock_forge_env` helper. Reasoning is documented inline at the mock-stub call site and in `wait_for_parent_exit`. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Maximilian Roos
|
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)
|
||
|
Maximilian Roos
|
bcf6a287c1 |
refactor(error): give the unmerged-index phrase one home (#3599)
`{count} {paths} with unresolved conflicts` was built twice,
pluralization and all — once in `GitError::UnmergedPaths`
(`src/git/error.rs`), once in the warning `wt step relocate` prints when
it skips a worktree it can't commit (`src/commands/relocate.rs`).
The two can't share an error type: relocate warns and carries on, since
an unresolved conflict blocks one worktree out of the many it walks,
while `UnmergedPaths` refuses outright. So only the phrase moves, into
`format_unresolved_conflicts`.
Output is unchanged — the four snapshots covering both sites are
untouched and pass.
> _This was written by Claude Code on behalf of Maximilian_
|
||
|
Worktrunk Bot
|
8fd289ac48 |
test(alignment): drop dead no-op cross-row alignment check (#3575)
Nightly survey finding in
`tests/integration_tests/column_alignment_verification.rs`.
The block commented "Additional check: verify that ALL rows have the
same column start positions" computed a `matching_boundary` for each
header boundary and then did nothing with it — the only branch was `if
matching_boundary.is_none() { continue; }`, and the match case had no
body. It never asserted, recorded an error, or affected the test result,
so a genuine cross-row misalignment would have passed it silently. The
comment implied a verification that wasn't happening, which is worse
than no check — it reads as coverage that doesn't exist.
Removed the dead block along with the now-unused `all_row_boundaries`
accumulator and its per-row `push`. The remaining per-row check (each
column measured against the header position, lines 205–260) still runs;
since every row is verified against the same header, cross-row
consistency is already covered transitively — the deleted block was
redundant even had it worked.
No behavior change to the assertions: the five
`column_alignment_verification` tests plus the other alignment suites
still pass (`cargo test --test integration column_alignment` → 10
passed).
No new regression test — this removes dead test code with no
production-behavior change to guard; the existing alignment tests remain
the coverage.
_Note, not changed here:_ the per-column check for non-`Path` columns
(lines ~240–258) only `println!`s a warning rather than pushing to
`errors`, so today only a `Path`-column misalignment can actually fail
this test. That looks deliberate (Path was the historical alignment
bug), so I left it alone rather than tightening it and risking false
failures on intentionally-sparse columns — flagging it in case a
maintainer wants it made assertive.
Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
|
||
|
Maximilian Roos
|
88319b8bf5 |
docs(changelog): document 0.69.1 drift fixes (#3567, #3579, #3560) (#3583)
Documents three user-facing fixes that landed on `main` after the v0.69.1 release branch was cut. They shipped in the v0.69.1 binary (the release squash-merged onto the current `main` tip) but weren't yet in the `## 0.69.1` changelog section, so this adds them under 0.69.1 where they belong — no version change. - **#3567** — `wt -C` honoured by `wt list statusline` - **#3579** — `wt step squash`/`wt step commit` refuse to commit unresolved conflicts; unified `wt list` operation gutter symbol - **#3560** — `wt switch` picker cancels its orphaned background preview processes on exit Each was independently reviewed for the data-loss surface as part of the release (all clean; #3579 is safety-positive). > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
ae363c2b4c |
test: stop the suite reaching the network (#3596)
`ci_status::test_list_full_with_configured_platform_github` hit nextest's 180s timeout in an advisory CI job while the same 2586-test run passed everything else. Its unloaded runtime is 2.2s, and a `-vv` trace of the same fixture shows exactly one non-local subprocess: `git ls-remote --symref origin HEAD` against `https://bitbucket.org/test-owner/test-repo.git`, the URL the test sets on itself. `Repository::default_branch()` falls through to the wire when neither the worktrunk cache nor `origin/HEAD` resolves, which is sanctioned behavior, but nothing bounds it, so the test blocks for as long as the connect takes. Reproduced by pointing that remote at an unroutable address: 11s on macOS, and on Linux an unanswered SYN costs ~127s per address (`tcp_syn_retries=6`), with `bitbucket.org` publishing six. It isn't one test. Setting `GIT_TRACE` across the harness shows a single full-suite run spawning 180 `git-remote-https` processes: 63 to github.com, 42 to gitlab.com, 27 to dev.azure.com, 3 to bitbucket.org, and the rest to names that don't resolve. Every one is unbounded, on three OSes, on every PR. So the fix goes in the harness rather than in the one test. `GIT_ALLOW_PROTOCOL=file` joins `STATIC_TEST_ENV_VARS`, which covers every `wt` subprocess and every PTY env builder including the hand-rolled ones, and the two git-only builders set it as well for direct `git_command()` spawns that don't consult that table. Local paths and `file://` keep working; anything else fails in milliseconds with `transport 'https' not allowed`, and default-branch detection falls back to local inference, so output is unchanged. `GIT_TERMINAL_PROMPT` was never enough on its own: it only suppresses the credential prompt that the host's 401 triggers, by which point the request has already gone out. The rust-lang/rust benchmark fixture clone is the one git call that *should* reach upstream. It opts back in through `allow_network_transports`, which lives next to the constant so the deny and the opt-out can't drift apart. `switch_pr_self_hosted_tea_authed` recorded the resolver's `Could not resolve host` in its snapshot, which is the same defect class (asserted output depending on DNS). It now records the transport refusal. Also folded in: `build_worktree_config_bare_layout` in `src/git/repository/tests.rs` carried a nine-line hand-rolled copy of `configure_git_env`, so it now calls it. Not addressed here: nothing bounds that `ls-remote` for real users either, so an unreachable remote blocks `wt list --full` and `wt switch` for minutes on the first call per repo. That needs a decision about whether a timed-out detection should write the `worktrunk.default-branch` cache. > _This was written by Claude Code on behalf of max_ |
||
|
Maximilian Roos
|
5f1ce2341f |
refactor(commit): give the staging gate one name and one StageMode mapping (#3598)
Three simplifications to the staging gate #3588 introduced, all deletions. ## `stage` names the commit instead of taking a caller's name `WorkingTree::stage` took an `action` string purely so its refusal could name the command the user typed. But staging on the user's behalf happens for exactly one reason — a commit is about to run — so the string was a constant wearing a parameter's clothes. It now names the commit, and three call-site strings go with it. The entry gates keep their `action`, and the reason is worth stating because it's what stopped me deleting the parameter outright. An entry point often doesn't yet know it will commit at all: ```rust current_wt.ensure_no_unmerged_paths("merge")?; // src/commands/merge.rs:179 ... let ResolvedMergeFlags { commit, .. } = flags.resolve(&resolved.merge); // :187 ``` `wt merge --no-commit` resolves its flags *after* that gate, and requires a clean tree — which an unmerged path is not. So a conflicted `--no-commit` run hits the entry gate, and `Cannot commit` would be actively wrong for a command explicitly told not to commit. The two gates know different things, and now say different things for that reason. This also settles the seam #3588 documented as a tradeoff: `wt merge --no-squash` reporting `Cannot commit` isn't a wrong label to be fixed later, it's the funnel correctly naming the only step it guards, at a point where the flags have resolved and the commit is certain. ## `CommitOptions::warn_about_untracked` was dead It defaulted to `true`, and both assignments set it to `stage_mode == StageMode::All` immediately after assigning `stage_mode`: ```rust options.stage_mode = stage_mode; options.warn_about_untracked = stage_mode == super::commit::StageMode::All; ``` while the consumer already ANDs with the same condition: ```rust if self.warn_about_untracked && self.stage_mode == StageMode::All { ``` No caller could make it `false` while the mode was `All`, so the field never decided anything. ## One home for the `StageMode` → `git add` mapping The mapping existed as two matches: one in `stage`, one in `preview_commit` for the `--dry-run` temp index. Adding a variant meant finding both, and missing the preview would silently produce a `--dry-run` that disagreed with the real run. `StageMode::add_args` now owns it. `stage`'s two per-arm `.context()` strings collapse into one, which costs nothing the args don't already say. ## Also `src/commands/step/squash.rs` justified its entry gate as refusing "ahead of the hooks and the LLM call" — but `wt.stage` is ahead of both too, so that reason no longer distinguished it. The gate is still worth keeping; what it uniquely precedes is the pre-commit approval prompt, so the comment says that now. ## Verification No user-visible message changes, and the tests confirm it rather than assert it: the pre-merge gate reports **no snapshots to review**. Every refusal a test pins comes from an entry gate, which kept its name, and the one funnel-driven snapshot (`step_commit_refuses_hook_created_conflict`) already read `Cannot commit`. `cargo run -- hook pre-merge --yes` green (4562/4562, clippy, fmt, doctests, doc sync). Also `cargo nextest run --features shell-integration-tests` (4562/4562) and `cargo clippy --all-targets --features shell-integration-tests`, which the gate doesn't compile — the blind spot that matters for removing a struct field. `RUSTDOCFLAGS=-Dwarnings cargo doc` clean for the new intra-doc link. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
58444440d7 |
fix(ci-status): classify tea failures by shape; report azure-devops extension (#3597)
Two follow-ups from #3595, both in code that talks to a forge CLI. ## Gitea CI status had the same exit-0 blind spot as the switch path `tea api` never reads the HTTP status — it copies the response body to stdout and exits 0 — so the `!output.status.success()` gate in `src/commands/list/ci_status/gitea.rs` never fired for a 4xx/5xx. #3595 fixed this for `wt switch pr:<n>`; the CI-status backend is the second instance of the same bug. The commit-status path was the quieter one: every field of `GiteaCombinedStatus` carries `#[serde(default)]`, so Gitea's `APIError` body deserialized as `{state: "", total_count: 0}` — an error indistinguishable from "this commit has no statuses". The PR-list path was noisier but less harmful: the error body failed to parse and logged `Failed to parse tea api pulls JSON`, blaming an API change for what was an API error. `tea_api_error` now reads the response shape — the `APIError` body, whose message feeds the existing retriable sniff — and a non-zero exit keeps meaning what it always meant, `tea` itself failing (transport error, no login). A proxy's HTML error page stays unclassified, as before: sniffing arbitrary bodies is the string-heuristic pile this removes. ## `wt config show --full` reports the azure-devops extension The whole `az repos` command group ships in that extension, so without it CI status stays blank however well `az` is authenticated — a persistent, user-fixable condition, which is what this surface is for. `azure_devops_extension_installed` (added in #3595) answers it in JSON: ```console ✓ az installed & authenticated ▲ azure-devops extension not installed; run az extension add --name azure-devops ``` An `az` that can't answer counts as installed, so an unanswered check never becomes an accusation. The `wt switch pr:<n>` bail now uses the same sentence — one wording for one condition, which is the one-line change to `switch_pr_azure_extension_not_installed.snap`. ## Testing The two new Gitea tests were checked against the unfixed source: reverting `gitea.rs` alone fails both, so they pin the fix rather than describe it. The two pre-existing retriable tests kept their exit-1 shape but lost their fabricated payload — `exit 1` alongside `429 Too Many Requests` is the combination `tea` can't produce — and now carry a real transport error, which is the case that does exit non-zero. `config show --full` gets a case per extension state. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
c136cbcd6a |
fix(switch): classify forge-CLI failures by structure, not tool prose (#3595)
Follow-ups from #3589's audit of brittle string matching, plus one live bug the audit turned up. ## Gitea: the error path was unreachable From tea's own `cmd/api.go`: `runApi` never inspects `resp.StatusCode` — it copies the response body to stdout and returns nil. So `tea api` **exits 0 for 404, 401, and 403**, which put the whole 404/401/403 classification behind an `if !output.status.success()` that those responses never reach. A missing PR surfaced as `Failed to parse Gitea API response for PR #N. This may indicate a Gitea API change.` The fabricated `{"message":"403 Forbidden"}` fixtures hid this by mocking an exit code tea does not produce. The response *shape* now discriminates — the PR object, or Gitea's `APIError` body whose message is surfaced verbatim. A non-zero exit means `tea` itself failed (no login configured, transport error) and keeps the existing `cli_api_error` path, which shows tea's own stderr. ## Azure: the install command, from JSON #3589 dropped the appended `az extension add --name azure-devops` because detecting the case needed prose matching. `az extension list --output json` answers it structurally instead, and the whole `az repos` command group ships in that extension — so it's a precondition of the call, not one guess among many. The check runs only after a failure, so the happy path pays nothing, and an `az` that can't answer counts as installed: an unanswered check never becomes an accusation the user would act on. ## Statusline detection `is_statusline_configured` matched the substring `"wt "`, which `newt status` also satisfies. That is not only a wrong diagnostic line — `install-statusline` early-returns on it, so a foreign statusline made install refuse. It now asks for the adjacent token pair `list statusline`, which is the actual question and stays agnostic to `wt` / `git-wt` / an absolute path. ## Worktree removal: comment only The remaining `error.contains("not empty")` stays. There is no structured form to recover: the text is `git worktree remove`'s own stderr, so the failure never passes through a Rust `io::Error`, and `remaining_entries` can't substitute because permission-denied also leaves entries behind. The comment claiming an `io::ErrorKind` was lost upstream is corrected. ## Testing Every changed path is covered by an integration test with realistic mocks: the four Gitea tests now mock exit 0 with `APIError` bodies (`gitea_server_error` became `gitea_request_failed`, since a 5xx no longer exits non-zero), the Azure tests mock `az extension list` on both sides — present, and failing so the check comes back unanswered — and a new `install_statusline_foreign_command` test asserts a foreign statusline gets replaced rather than mistaken for worktrunk's. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
f1f1b50d85 |
docs(release): trim the signing archaeology now the path is proven (#3593)
Follow-up to #3590, now that v0.69.2 has shipped a signed Windows binary — verified against the published asset rather than the logs: ``` git-wt.exe: certificate table 8480 bytes wt.exe: certificate table 8472 bytes c4343fb5…1a43 *worktrunk-x86_64-pc-windows-msvc.zip (published .sha256 matches) ``` Comments only; no behavior change. The signing comments were written while the mechanism was still being guessed at, so they carried the failed attempts — a ten-line account of the zip-of-a-zip failure, a six-line account of the `…zip.zip` collision. The path is proven and the workflow now verifies itself, so what a reader needs is the current mechanism. Trimmed to that. One thing added rather than removed. `Recompute checksum` sits between two `continue-on-error` steps and looks like it should fold into the replace step above it. It must not: the signing steps are tolerant because publishing can't depend on a self-signed test certificate, while a checksum that doesn't match the shipped zip has to fail the release. That asymmetry is invisible in the YAML, so it's now stated — the next person to simplify this shouldn't have to rediscover it. > _This was written by Claude Code on behalf of max_ |
||
|
Maximilian Roos
|
5e03e2863c |
docs(changelog): document the v0.69.2 drift (#3592)
Eleven commits landed on `main` while #3590 sat in CI, so they ship in v0.69.2 with no changelog entry. Most are user-facing, and three are data-loss fixes — exactly the drift the release skill's step-12 check exists to catch. The tag is held until this lands. Entries added, most-impactful first: - **#3589** — two ways shell integration deleted user data on a substring guess: an rc line that only *quotes* the init command, and a user's own `conf.d/wt.fish` deleted outright by `install`'s legacy cleanup. Plus forge detection moving from `host.contains("github")` to label-wise matching. - **#3585 + #3591** — truncate-in-place rc writes replaced by write-temp-then-rename, then generalized to every user-file write. - **#3578** — despite its `docs(step):` subject this carries three behavior fixes, including `wt step push` succeeding mid-rebase and moving the target branch onto a half-replayed history. - **#3588 + #3587** — conflict markers can no longer reach a commit via `wt step relocate --commit`, and `wt merge` refuses in its own name. - **#3554** — OpenCode marker writes could land in another session's worktree (thanks @4i3n6, who both reported and fixed it). Not documented, per convention: #3574 (doc comments only, no runtime or docs-site surface) and the three dependency bumps. Also corrects `.github/CLAUDE.md`, which lists three required status checks; there are four — `fast-checks` is required too, confirmed against the branch-protection API. Entries verified against the diffs by subagent. Two corrections came back and are folded in: the #3589 entry originally promised "two of them deleted" and described only one, and the #3578 entry mis-quoted the success line as `✓ Pushed to main` when it carries a commit count. > _This was written by Claude Code on behalf of max_v0.69.2 |