mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
codex/remove-codex-cloud-specific-tests
138 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maximilian Roos
|
e1745db105 |
feat(list): abbreviate the table's SHA with git, not a fixed slice (#3676)
The Commit cell sliced `&head[..8]` while `--format=json`'s `short_sha` carried git's `%h`, so one commit read `1b9f1d96` in the table and `1b9f1d9` in JSON, and `core.abbrev` reached only the JSON. #3675 gave a detached row's Branch cell the same slice, so the disagreement showed up twice on one row. `ListItem::short_sha` becomes the only abbreviation of `head` anywhere: the Commit cell, a detached row's Branch cell, the statusline, and JSON all render it. `abbreviated_head()` is gone. ## Column widths `COMMIT_HASH_WIDTH = 8` is gone. The Commit column and the Branch column's detached budget both measure the SHAs they will render, so `core.abbrev = 12` no longer truncates mid-hash and the default 7 stops reserving a column nothing fills — the freed character goes to Message. ## Latency `collect()` folds `%h` onto the rows before layout instead of after the skeleton. The batch carrying it already gates the skeleton for `%ct` sort order, so this is a map lookup rather than new I/O, and both cells are identity columns with no placeholder — they still paint in the first frame. Measured on a 40-worktree / 400-branch fixture: git subprocess counts are identical (5 pre-skeleton, 108 for the full run). Pre-skeleton wall time is unchanged; running both binaries in each order, the sign of the difference follows run order rather than the binary (+1.5 ms with this branch second, −0.5 ms with it first), so the residual sits inside drift. ## Behavior change Where the commit-details batch fails, the Commit cell is now empty rather than a slice of a SHA git refused. Age and Message already report that failure the same way, under the same warning, and two snapshots show it. `render_text_cell` also stops styling empty text, so a blank cell no longer emits an escape pair around nothing. ## Reading the diff 160 files, but the hand-written part is +91/−79 in `src/commands/list/` plus a +71 test. The rest is generated. The docs mirrors and help snapshots are symmetric. Of the snapshot lines, content is +799/−799 — every changed line a 1-for-1 hash swap — while +1396 is insta `env:` metadata refreshing on the 128 snapshots this happens to touch. `test_list_abbreviated_sha_follows_git` pins the invariant: the table's hash equals JSON's `short_sha` at git's default and at `core.abbrev = 12`, and a longer prefix is ruled out. It fails against the old fixed slice. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
Maximilian Roos
|
0f2d562541 |
fix(forge): classify a forge by the brand in the hostname, not by DNS label (#3673)
Reverts the branded half of the exact-label classifier and deletes the diagnostic built to explain it. `github-enterprise.acme.com`, `mygithub.com`, `gitlab-internal.company.com`, and the `github-personal` SSH alias resolve to their forge again, so CI status, `wt switch --prs`, and `repo.provider` work with no config. ## Why the label boundary goes It looked like an ownership check and wasn't one. An attacker controls their own DNS, so `github.attacker.example` has the exact label `github` and classified fine; `gitlab.evil.co.uk` likewise. What the rule actually excluded was the self-hoster who put the brand in a hyphenated name. It failed open for the adversary and closed for the customer. The residual case for it doesn't survive either. The hostname comes out of the user's own `.git/config`, and whoever can put a host there can put code there too — the trust decision happens at clone time, and by the time worktrunk reads the remote the user is already building from it. All the classification decides is which forge CLI (`gh`, `glab`, `tea`, `az`) runs against it. So the rule is recall-first: any host carrying `github`, `gitlab`, or `gitea` matches, first match winning. The cost is a host that merely sounds like a forge getting a forge CLI run at it, which surfaces as that CLI's error rather than as silence — the better of the two failures, and `forge.platform` overrides it. ## What stays Azure DevOps keeps suffix matching on its two service domains, and for a reason unrelated to security: those are service domains rather than a brand in the host, so every real hosted instance already matches, and the on-prem edition carries neither string. `dev.azure.com.attacker.example` and `evil-visualstudio.com` are outside the domains and carry no brand to fall back on, so they stay unclassified. Userinfo still resolves to the network host, so `https://github.com@attacker.example/…` is `attacker.example`. ## What goes `LegacyForgeAlias`, `Repository::legacy_forge_alias`, `legacy_forge_alias_diagnostic`, and its three emit sites in `wt list`, `wt switch --prs`, and `wt config show --full`. Every host the diagnostic fired on now classifies, so it could only ever return `None`. A host with no brand at all still reaches the existing generic hint, which is the right message there — there is no platform to infer. The end-to-end warning-dedup test goes with it, since no warning is raised from both the collect and `--prs` threads any more; `stash_warning_preserves_order` keeps the mechanism covered. ## Docs `## Forge platform` in `src/cli/mod.rs` described the override as being for SSH aliases and self-hosted instances, which now detect on their own. It states the rule and scopes the override to hosts carrying no brand — a Forgejo instance at `forge.example.com`. Mirrors, `dev/wt.example.toml`, and the two `config` help snapshots regenerate from it. --- The branch's history has a false start — the first commit widened the diagnostic, the second deletes it in favour of relaxing classification — plus a merge of `main` after v0.71.0 shipped. The net diff is the second approach; it all squashes on merge. Also supersedes [#3672](https://github.com/max-sixty/worktrunk/pull/3672), the triage bot's PR for the same issue — it widens the diagnostic rather than removing the need for one, so it should be closed too. Closes #3671 > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
1eded27943 |
Simplify forge, shell integration, and removal internals (#3662)
This consolidates cross-cutting models that had accumulated parallel representations, while preserving the CLI and config interfaces. ## What changed - Forge identity now flows through one `ForgeKind`, with boundary-safe network-host classification shared by CI, remote references, and structured repository metadata. Branded SSH aliases such as `github-personal` remain outside provider dispatch and receive an actionable `forge.platform` diagnostic when forge data is requested. - Worktree removal now uses one owned target type and rechecks uncached worktree topology immediately before compare-and-swap branch deletion, retaining branches that gained a live or locked checkout. - Shell integration no longer implements the retired single-file directive writer. Stale wrappers receive repair guidance, execution fails closed, and child processes cannot inherit the retired sourceable-file capability. - Zsh and Git-version probes are shared, while dead mocks, redundant dependency edges, serializer detours, pass-through types, and duplicate tests are removed. The removal guard deliberately distinguishes live, stale-prunable, and locked registrations. The detached cleanup path performs the same record-aware check before deleting a branch ref. ## Testing `cargo run -- hook pre-merge --yes` passed after merging current `origin/main`: 4,489 tests, Clippy, formatting, lockfile checks, docs, doctests, and snapshot review. > _This was written by Claude Code on behalf of max_. |
||
|
Maximilian Roos
|
6d09125b7b |
test: converge suite on semantic boundaries (#3663)
This follows the first test-simplification tranche by converging the remaining suite around distinct semantic and pragmatic contracts rather than raw case count. The branch removes false-confidence tests, invalid setup variants, repetitive snapshots, and expensive PTY overlap while strengthening the retained route, precondition, and interaction proofs. ## What changed - Replace obsolete CI-status integration mocks and blank snapshots with direct provider semantics, mixed-priority cases, and strict GitHub/GitLab route assertions. - Remove free-riding merge, push, remove, list, security, config, and switch cases whose setup never reached the named behavior; consolidate repetitive direct cases into labeled tables. - Reduce the switch picker from 42 PTYs to 19 distinct terminal contracts, using causal release gates for asynchronous loading and repaint behavior. - Add a cached main-only picker fixture, eliminating 138 unnecessary Git subprocesses across the retained PTYs, and integrate it with main's generated hermetic standard fixture. - Tighten test guidance around proving setup preconditions and mock invocation routes, and correct the comments-tab help text and generated mirrors. ## Reviewer map - `tests/integration_tests/ci_status.rs` and `src/commands/list/ci_status/`: provider semantics and route coverage. - `tests/integration_tests/switch_picker.rs`, `src/commands/picker/`, and `src/testing/`: retained PTY contracts, causal mocks, and fixture design. - `tests/integration_tests/config_show.rs`, `src/config/deprecation.rs`, `src/config/expansion.rs`, and worktree type/resolve tests: direct-boundary consolidation. - `tests/CLAUDE.md`: the testing rules extracted from the false-confidence cases found during the survey. The measured loop removed 98 tests, 65 snapshots, and 23 picker PTYs. Controlled warm Nextest execution improved from a 79.593-second mean to 72.574 seconds (8.8%), while comparable production-line coverage moved from 97.32% to 97.23%. The tracked PR diff is a net deletion of more than 5,700 lines. ## Validation - `cargo run -- hook pre-merge --yes` after syncing current `main`: 4,468 passed, one configured skip; docs, doctests, clippy, formatting, policy checks, and snapshots green. - `task coverage` on the completed change before the base sync: 4,465 passed, one configured skip; 97.23% comparable production-line coverage. - Three independent final audits found no remaining lost beliefs, fixture hazards, or safe PTY consolidations. > _This was written by Claude Code on behalf of max_. |
||
|
Maximilian Roos
|
1b35950a9c |
test: simplify the integration suite (#3657)
This reduces duplicated and false-confidence integration coverage while preserving the suite's semantic and user-facing contracts. ## What changed - Replaces three overlapping list-layout suites with two representative CLI integrations, leaving exhaustive geometry at the direct layout layer. - Groups Git error render variants into labeled family snapshots and removes command-by-shell wrapper cross-products while retaining shell-specific conformance and regression cases. - Updates test-authoring guidance around boundary choice, minimal contrasts, and PTY use, and runs local and CI coverage through Nextest isolation. ## Results - Test catalog: 4,642 to 4,562 - Snapshots: 1,193 to 1,131 - Warm all-feature runtime: 84.70s to 78.17-80.35s - Full coverage: 97.32% of lines ## Testing - `cargo run -- hook pre-merge --yes` - `cargo llvm-cov nextest --features shell-integration-tests --summary-only` > _This was written by Claude Code on behalf of max_. |
||
|
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
|
f5d99d4fe5 |
fix(step): refuse to commit unresolved conflicts; one gutter symbol for every operation (#3579)
Two follow-ups from #3558, which stopped `wt merge` and `wt step rebase` from running out of a half-finished operation. Probing the sibling commands for the same root cause found one that was worse, and the symbol work is what that PR's detection change made possible. ## `wt step squash` committed conflict markers Mid-merge, invoked directly, it didn't refuse. It generated a commit message for the unresolved markers and committed them: ```console $ git merge side CONFLICT (content): Merge conflict in f $ wt step squash --yes ◎ Generating commit message and committing changes... (1 file, +6, no squashing needed) Merge branch 'side' This resolves a merge conflict between main and side branches. ✓ Committed changes @ e6262c6 $ git show HEAD:f <<<<<<< HEAD main ||||||| 56e6eb3 base ======= side >>>>>>> side ``` `MERGE_HEAD` is gone and the working tree is clean, so the broken merge reads as complete. `wt merge` was never exposed — its own gate stops it before it reaches squash — so this was the direct-invocation path only. ## The index is what knows The obvious fix is the gate #3558 added, and it is not sufficient. The hazard is narrower than "an operation is open", and also wider. `git add -A` collapses an unmerged path's three index stages into one entry. That resolves the conflict as far as the index is concerned, while `<<<<<<<` is still on disk — and it takes with it git's own refusal to commit an unmerged index, which is what would otherwise have stopped this. So the exposure is exactly the commands that stage on the user's behalf, and it does not need an operation to be open at all: ```console $ git stash pop CONFLICT (content): Merge conflict in f $ ls .git/MERGE_HEAD ls: .git/MERGE_HEAD: No such file or directory $ wt step commit --yes ✓ Committed changes @ 1f5387c # markers and all ``` A conflicted `git stash pop` leaves unmerged paths with no state file written, so no reading of `.git/` can see it. `WorkingTree::ensure_no_unmerged_paths` reads the index instead — `git diff --diff-filter=U`, the same question `git commit` asks — and both staging paths call it before staging: ```console $ wt step commit ✗ Cannot commit: 1 path with unresolved conflicts ┃ f ``` The paths are worth carrying where the operation refusal carried nothing: they are the one thing the user needs and git isn't being asked. `wt step squash` additionally takes the operation gate, ahead of its branch check, so mid-rebase it names the open rebase rather than blaming the detached HEAD and offering `git switch <branch>` — the one command that throws the rebase away, which is the same wrong remedy #3558 removed from `wt merge`. Both guards run before the pre-commit hooks and the LLM call. Neither is worth running for a commit that can't happen, and a refused commit runs no project commands — checked with a `pre-commit = "touch HOOK_RAN"` project config that never fires. `wt step commit --dry-run` is deliberately not gated. It mutates nothing, and guarding it displaced `test_step_commit_dry_run_propagates_git_add_failure`, which pins a distinct error path; a tested behavior is worth more than cosmetic parity. ## One symbol for every operation The Status column had `⤴` for rebase and `⤵` for merge, and nothing for the other three states git can leave open. A worktree stopped mid-cherry-pick, mid-revert, or mid-bisect rendered as idle — including the case #3558 closed, a multi-commit cherry-pick whose stop was resolved with `git commit`, which leaves a clean tree, no `CHERRY_PICK_HEAD`, and only the queued sequencer to say anything is wrong. Three more glyphs was the obvious fix and the wrong one. What the reader does about any of the five is identical: run `git status`, then finish or abort it. Splitting the column across a glyph per operation asked them to distinguish states that lead to the same next step, and the split is what left the other three invisible. So they collapse to one: ```console $ wt list Branch Status … Message @ main ↻^ … resolved by hand ``` `git status` names which operation it is, in git's own words — the same division of labor as the refusal message. `--format=json` keeps the identity the symbol drops, so a consumer that needs it still has it: `operation_state` gains `cherry_pick`, `revert`, and `bisect` alongside `rebase` and `merge`. That retires `ActiveGitOperation`, which existed only to re-encode `InProgressOperation` down to the two states the gutter knew. `WorktreeData.git_operation` is now `Option<Option<InProgressOperation>>` — outer `None` is "not loaded" — matching its neighbour `has_working_tree_conflicts`. `GitOperationTask` also stops swallowing errors: it reports a failed probe through the same `ctx.error` channel every other task uses, rather than reporting "no operation" for a probe that didn't answer. ## Verification Three integration tests, each asserting HEAD is unmoved rather than only matching the message: - `test_step_squash_refuses_mid_merge` — the case that committed markers. - `test_step_commit_refuses_unmerged_paths` — driven from a conflicted `git stash pop`, so it can only pass through the index read; the operation check cannot see that state. - `test_list_shows_symbol_for_bisect` — bisect because it is the only operation that leaves HEAD on the branch and the tree clean, so nothing else in the Status cell stands in for it. Snapshots pin both the symbol and `"operation_state": "bisect"`. Confirmed by hand against real repos: mid-merge, mid-rebase, and the conflicted-stash-pop state all refuse; a mid-merge commit whose conflicts *are* resolved still succeeds, as does a clean squash; a queued cherry-pick and a stopped revert both render `↻` and name themselves in JSON. Full gate green (4500/4500 tests, lints, fmt, doc sync), plus `cargo test --features shell-integration-tests` (2148/2148) and `cargo clippy --all-targets --features shell-integration-tests`, which the gate doesn't compile. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
f14fa69c0e |
feat(list): run merge analysis in a read-only object database (#3535)
When the Git object database is read-only, `wt list` and `wt list statusline` redirect their object-writing merge/conflict probes (write-tree, commit-tree, merge-tree --write-tree) into a temporary object database with the real one as a read-only alternate, so the full analysis runs in a managed read-only sandbox. Detection is a one-time writability probe; the writable path is unchanged. Scoped to the two observational list entry points (collect and populate_item); mutating commands keep the persistent database and fail loudly on a read-only store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
d12c55a320 |
feat(config): wt config update adopts json-schema = 2 instead of pinning 1 (#3436)
Flips the direction of the `[list] json-schema` pending-default handling that #3411 introduced: `wt config update` now writes `json-schema = 2` (adopting the upcoming default) instead of pinning `= 1` (preserving the current one). Running update is the migration; staying on schema 1 is the deliberate manual edit. The `wt list --format=json` nag flips to match, keeping the adopt action last for easy copying: ``` ▲ JSON output is schema 1; a future release switches the default to schema 2 ↳ To keep this format set [list] json-schema = 1; to adopt the new schema, run wt config update ``` Why: with pin-to-1, every `wt config update` run during the deprecation window entrenched users on the schema being retired, leaving a pinned cohort the default flip could never migrate. With adopt-2, update moves users forward as a reviewed config edit, and after the flip `= 2` is just a redundant default a future rule can strip. The trade: `wt config update --yes` in a script switches JSON output as a side effect of unrelated migrations (the interactive path shows the diff first). The system-config gate is unchanged — when the system layer defines the key, update leaves the user file alone; the test now covers the sharper direction (system `= 1` must not be overridden by a user-file `= 2`). All detection/warning invariants carry over; the diff is ~6 semantic lines plus pin→adopt wording and 65 one-line snapshot flips. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
b8134563f1 |
fix(list): unify task-failure output vocabulary (#3435)
Reviewing this real `wt list` output against the `writing-user-outputs`
guidelines surfaced several inconsistencies:
```
○ Showing 15 worktrees, 6 branches, 2 with changes, 8 ahead, 2 columns hidden. 1 task failed
▲ Some git operations failed:
plugins: working-tree-conflicts (fatal: Unable to create
'/Users/…/.git/worktrees/worktrunk.plugins/index.lock': File exists.)
↳ To create a diagnostic file, run with -vv
```
The footer and the warning used different vocabulary for the same
failure ("task" vs "git operations", the latter inaccurate since
CI/URL/summary tasks aren't git operations), the warning said "Some"
when the exact count was known, and the per-failure lines leaked
internal kebab-case `TaskKind` discriminants (`working-tree-conflicts`)
that appear nowhere in docs or help.
The same output now reads:
```
○ Showing 15 worktrees, 6 branches, 2 with changes, 8 ahead, 2 columns hidden; 1 task failed
▲ 1 task failed:
plugins: working-tree conflict check (fatal: Unable to create '…/index.lock': File exists.)
↳ To create a diagnostic file, run with -vv
```
## Changes
- **One term, matching counts.** Footer and warning both say "N task(s)
failed" ("task" is the documented term — the JSON docs already use it).
The mixed form becomes disjoint (`2 tasks failed, 3 timed out` instead
of `5 tasks failed (3 timed out)`), so the footer's "failed" count
always equals the number of lines in the warning below it.
- **Human task names.** `TaskKind::display_name()` maps each task to
user vocabulary (`working-tree conflict check`, `ahead/behind counts`,
`CI status`, …) in the failure warning, the stall footer ("waiting on
…"), and the drain-timeout diagnostic. The strum kebab-case form remains
for tracing and `-vv` diagnostics.
- **Footer clause join.** Semicolon instead of a period splice, per
house style; also fixes always-plural "1 worktrees"/"1 branches" in
`--branches` mode.
- **Buffered summary to stderr.** The `○ Showing …` line narrates the
table rather than being part of the answer (`--format=json` omits it),
so in buffered mode it moves to stderr and piped stdout ends cleanly
after the last row. The progressive path keeps it on stdout, where the
summary is a repainted row of the table region. This is most of the
198-file snapshot churn (the line moves from the stdout section to the
stderr section).
- **Docs generator reads both streams.** `parse_snapshot_raw` in
readme_sync previously embedded stderr *or* stdout; it now concatenates
both in terminal order. The generated docs are byte-identical before and
after the stream move, which is the check that the console blocks still
read like the terminal.
Two negative test assertions that guarded against surfacing these
failures (`working-tree-conflicts` in `tests/integration_tests/list.rs`)
were retargeted to the new display names so they don't pass vacuously.
## Testing
Pre-merge gate (all tests + lints) green; feature-gated clippy and the
PTY progressive-list tests (`shell-integration-tests`) run separately
and green, since the local gate skips them.
> _This was written by Claude Code on behalf of max_
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
dac0f80608 |
feat(config): wt config update pins [list] json-schema while unset (#3411)
Wires the `[list] json-schema` nag into the standard config-update machinery, so the warning comes with the standard one-command fix. `wt config update` now pins `json-schema = 1` (the behavior-preserving choice) when the key is unset, via a new `PendingDefault` variant in `DEPRECATION_RULES`: applied on the update pass only — the load path must not pin, or an in-memory `Some(1)` would silence the nag — and scoped to user config through a `ConfigFileKind` enum threaded through detection in place of the old string labels. Two guards keep the pin honest: it stays inert when the system config layer already defines the key (a user-file pin would override a system-level `= 2` and flip resolved output), and the nag's hint offers `wt config update` only when the same detection update runs would actually write the pin — a missing, unreadable, or malformed user config falls back to naming the manual setting. The detection-equals-migration invariant holds with the warning relocated: the pin's warning fires at the JSON-emitting surface (`resolve_json_schema`) exactly when update would change the file, while config load stays quiet (`DeprecationKind::is_pending_default` filters it, and pin-only configs skip the warning-dedup machinery entirely), so `wt switch` users never see it. `wt config show` renders the pending pin's diff — including for empty config files — but keeps its TOML dump: a pin is additive, unlike a deprecation diff that supersedes the dump. This departs from the plan reviewed in `design/list-json-v2.md` (#3357), which deferred the `wt config update` integration to the default flip. Deliberate tradeoff: users who run update during the window land pinned on schema 1 and will see the `= 1` deprecation round after the flip; in exchange, the warning ships with its fixer. **Testing:** unit tests pin the rule's iff (unset ⟺ update changes the file), kind scoping (System/Project inert, load pass inert), the PendingDefault-rule/kind coupling, and placement in existing or implicit `[list]` sections; integration tests cover both hint variants, the update flow end-to-end (pin applied, second run clean), `--print`, and the system-config deferral. The invariant battery runs with an explicit pin appended so each case exercises only its own rule. Full pre-merge gate green (4386 tests) plus `--features shell-integration-tests` clippy. 🤖 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
|
56e20a4a78 |
feat(list): add JSON schema 2 behind [list] json-schema (#3357)
Adds a second JSON schema for `wt list --format=json` and `wt list
statusline --format=json`, selected by a new `[list] json-schema` config
key. Schema 2 is an envelope (`schema`, `repo.default_branch`,
`repo.forge`, `collected`) over items carrying independent facts; schema
1 (the current bare array) is byte-identical to today and remains the
default. Unset emits schema 1 plus a once-per-process stderr nag naming
both settings; `= 1` pins silently; an invalid value warns and degrades
like any other config problem. The nag is suppressed on the statusline
surface, which would otherwise corrupt prompts.
**The semantic core is the absence rule**: absent = nothing to report
(not applicable, not requested per `collected`, or determined-empty),
null = requested but undetermined (probe pending, timed out, fetch
failed). Three mechanisms keep it honest — `integration` derives from
the same committed-content signals `wt remove` trusts
(`check_integration`) rather than the cleanliness-gated display
collapse; skip-seeded conservative defaults are recorded on
`ListItem.seeded` and serialize as null instead of masquerading as
determined facts (invariant-pinned: no seed can fabricate a positive
integration match); and orphan sentinel counts are guarded so they can't
read as a same-commit match.
**For reviewers, in reading order**: `src/commands/list/json_v2.rs` (the
serializer and its `Tri` tri-state), `src/commands/list/mod.rs`
(`resolve_json_schema` + wiring), `src/commands/statusline.rs`
(`run_json`), and small model/collect extensions (`SeededFacts`,
`Collected`, `UpstreamStatus.upstream_short`, task-plan union so a
listed `ci` column forces the fetch for JSON like the table).
**Design doc**: the full rationale, field-by-field mapping, and
migration plan were reviewed as `design/list-json-v2.md`, which rode
this branch as its first commit. Design docs are review-only by repo
convention, so a final commit removes it from the net diff; it remains
readable at
|
||
|
Maximilian Roos
|
a0f07ac6d7 |
fix(list): diff branches against the default branch's upstream tip (#3280)
## Problem
`wt list`'s `main↕` (ahead/behind) and `main…±` (branch diff) columns
measured every branch against the **local** default-branch tip. In a
fork whose local `main`/`master` lags its upstream, those columns
reported garbage. Local default branches go stale routinely: you fetch
far more often than you fast-forward local `main`, while the
remote-tracking ref refreshes on every fetch.
Concretely, in a fork of `skim-rs/skim` whose local `master` sat 42
commits behind `origin/master`, every feature branch displayed as `↑44`
ahead and `+∞ / -5K`, when each was only ~2 commits past the real
upstream tip. The workaround was a manual `git merge --ff-only
upstream/master` to un-stale the local base before `wt list` made sense
again.
## Root cause
`wt list` already carried two notions of "the mainline ref":
- **Integration status** (`⊂` / `↑` / `↓` …) resolves the default
branch's upstream and compares against the *superset* of `{local
default, its upstream}` via `Repository::integration_targets()`.
- **The two stat columns** ignored that and diffed against the raw local
`default_branch`.
That inconsistency is the bug. A stale local default inflated the counts
by every commit it was missing, and the integration symbol and the
numbers next to it could disagree.
## Fix
Route the two informational stat tasks through the same upstream-aware
superset ref the integration column already uses. A new
`TaskContext::comparison_base()` returns `integration_targets.primary`
(the superset side), falling back to the raw `default_branch` only when
integration targets could not be resolved (snapshot capture failed).
`AheadBehindTask` and `BranchDiffTask` now consume it instead of
`default_branch()`.
This reuses one mechanism rather than adding a second upstream resolver,
and is correct across all four local-vs-upstream relationships:
| Relationship | base | vs. before |
|---|---|---|
| no upstream / local == upstream | local | unchanged |
| local behind upstream (stale fork) | **upstream** | **fixed** |
| upstream behind local (unpushed local merge) | local | unchanged |
| diverged | local | unchanged |
The "upstream behind local" row is why naively preferring the upstream
ref would be wrong. After a local `wt merge` that has not been pushed,
the default branch leads its upstream, and sibling branches must still
measure against the local tip so the unpushed merge commits do not leak
into their counts. Reusing `integration_targets.primary` gets this case
right for free, because its superset selection already keeps the local
ref there.
### Deliberately unchanged
- **Conflict columns** (`✗ WouldConflict`, via `MergeTreeConflictsTask`
/ `WorkingTreeConflictsTask`) keep using the local default branch. They
predict the local `wt merge`, which targets the local ref.
- **`wt merge` target** and **`wt switch --base`** (writes) keep using
the local branch. You merge into and update a local ref, and a new
branch defaults onto a writable local branch; a remote-tracking ref is
neither.
### Side effect
The `↑`/`↓`/`↕` gutter symbols derive from these counts
(`is_same_commit` in `model/item.rs`), so they now also track the
upstream tip in a lagging fork. A branch sitting at the stale local-main
tip renders `⊂` (its content is already in the real mainline) rather
than `_`. Removal safety is unchanged: the safe-to-remove signals
(`is_ancestor`, `trees_match`) were already upstream-aware.
## Performance
The snapshot's batched `%(ahead-behind)` walk is keyed on the local
default-branch name, so the common case (superset == local) keeps the
batch. Only the lagging-fork case falls back to a per-pair
`ahead_behind_by_sha`, which is SHA-cache-backed, and that case had
wrong numbers before. The diverged case stays local, so it keeps the
batch too.
## Tests
Two regression tests in `tests/integration_tests/list.rs`:
- `test_list_branch_stats_use_upstream_when_local_default_lags` covers
the fix. It fails on `main` with `ahead 5` against the expected `2`.
- `test_list_branch_stats_stay_local_when_default_ahead_of_upstream`
pins that the base stays local when the default leads its upstream.
The full `wt hook pre-merge` gate is green (4262 tests, clippy,
docs-sync, rustdoc).
> _This was written by Claude Code on behalf of max_
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Jeremy Odell
|
0681a2e0d0 | feat(list): add structured repo metadata to JSON output (#3021) | ||
|
Maximilian Roos
|
c511d3fa2b |
feat(list): show PR/MR number in the CI column (#3041)
The CI column in `wt list --full` (and the statusline) previously showed a single colored dot. It now shows the branch's open PR/MR reference — `#3035` on GitHub/Gitea/Azure DevOps, `!3035` on GitLab — colored by CI status, dimmed when stale, and hyperlinked to the PR. When no number is available (branch workflows without a PR/MR, pre-number cache entries, or a number wider than the allocated column), the cell shows a bare `#` in the same colors. Fetch errors always render `⚠`, even when a number is known — Error and Conflicts share yellow, so a yellow `#3035` would read as a conflicted PR. The branch merges main's review-state feature (#3044): review colors (magenta/cyan) and draft dimming apply to the number cells exactly as they did to the dot, and the `--help` legend shows colored `#` samples for all seven states (the interim version had dropped the colored samples from the legend entirely). ## The width problem `wt list` renders skeleton-first: column widths are fixed before any CI data arrives, and the table never resizes mid-render. The PR number's width therefore has to be known up front. The solution is a repo-level ratchet cache (`.git/wt/cache/pr-number/max.json`) holding the largest PR number any fetch has seen — PR numbers are monotonic per repo, so the value needs no invalidation. Pre-skeleton, `collect` reads that one file and sizes the column exactly; on a cold cache the estimate is 5 chars (`#9999`). A number that outgrows the estimate renders as the bare `#` for that run and sizes correctly on the next run once the ratchet records it. The ratchet is deliberately separate from the per-branch `ci-status/` entries so the width hint isn't coupled to branch-entry retention, and `detect` re-ratchets on cache hits too, so a deleted or racily regressed `max.json` heals from locally cached numbers instead of waiting out the TTL. ## Reviewer's map - `src/commands/list/ci_status/mod.rs` — `PrRef` (number + forge sigil, `PrRef::pr`/`PrRef::mr` constructors), `PrStatus.number` (serde-default so pre-existing cache entries still deserialize, rendering `#` until their 30–60s TTL expires), `format_cell` width-aware renderer with the Error guard, ratchet in `detect` (both cache-hit and fetch paths) - `src/commands/list/ci_status/cache.rs` — `MaxPrNumber` ratchet (read/ratchet/clear) - `src/commands/list/ci_status/{github,gitlab,gitea,azure}.rs` — each fetcher populates the number (`gh --json number`, `iid`, Gitea `number`, `pullRequestId`); GitLab's mr-view-failure path carries the iid/URL/review state into the error status so the `⚠` stays clickable - `src/commands/list/layout.rs`, `collect/mod.rs` — width estimate threading - `src/commands/list/render.rs`, `model/item.rs` — table cell and statusline both go through `format_cell` - `src/commands/list/json_output.rs` — `ci.number` field - `src/commands/config/state.rs` — ratchet shown by `state get`/`cache get` (table + JSON) and swept with the CI cache category, including the deprecated `ci-status clear --all` path - `src/md_help.rs`, `src/help.rs` — legend colorization rules rewritten from `●` to `#` (terminal + website) Most of the diff is snapshot churn from the column width and glyph changes plus regenerated docs mirrors. Known trade-offs: concurrent statusline ratchet writes can transiently lose an update (monotonic, re-learns on the next render, documented at the write site); one anomalously high PR number widens the column until `wt config state cache clear`; an open Azure DevOps PR still shows gray `NoCI` instead of its pipeline status — a pre-existing gap, now marked `TODO(azure-pr-pipeline)`. Testing: unit tests for `format_cell` (including the Error-with-number and oversized-number link cases)/`pr_ref_width`/ratchet/width estimates; integration coverage for all four forges with real numbers (the Gitea mocks now exercise the number path too), review-state × number composition, the GitLab mr-view-failure `⚠` and branch-pipeline success paths, cache-TTL expiry → refetch, the statusline number view, and the `wt config state` surfaces. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
bd278e5d1f |
test: guard committed snapshots against host-specific paths (#3026)
## Guard committed snapshots against host-specific paths Follow-up to #3009, which fixed one instance of this class; investigating the rest of the corpus turned up two more committed leaks — from two *different* machines (`/var/folders/v3/…` and `/var/folders/wf/…`), direct proof of cross-machine churn. ### The mechanism insta compares only snapshot content (stdout/stderr/exit); the `info:` block insta-cmd records (`args:`, `env:`) is metadata that is written but never compared, and `add_filter` doesn't apply to it. A test missing a redaction bakes the generating machine's paths into the committed file while passing everywhere — surfacing only as churn when someone regenerates the snapshot elsewhere. Under libtest, the `repo` fixture's leaked settings binding (rstest has no teardown) can additionally mask the omission, so what gets committed depends on runner and thread scheduling. ### Enforcement at the boundary `test_no_host_specific_paths_in_snapshots` (in `snapshot_formatting_guard.rs`, whose walker now recurses over all ~1200 snapshots under src/tests/docs instead of 2 flat dirs) fails on any committed `.snap` line carrying a host-specific marker: macOS `/var/folders/`, the `wt-test-profraw` LLVM fallback dir, tempfile's `.tmpXXXXXX`, or a home path with a non-fake username. A detector self-test pins sensitivity in both directions, since a clean corpus would otherwise pass vacuously if a regex rotted. Pre-fix, the test caught exactly the two known leaks across 1182 files with zero false positives. Why not fix the fixture leak itself: storing the insta guard in `TestRepo` is structurally impossible (insta is a dev-dependency invisible to the lib crate defining `TestRepo`; the guard is `!Send` by design; rstest has no teardown), and a wrapper type would churn ~1700 test signatures to fix a divergence the corpus guard makes harmless. The fixture comment now documents the real contract instead of its previous (false) safety claim. ### The two leaks - `list_with_c_flag`: the temp repo path leaked through the **`args:` block** (`wt -C <root>`) — a channel `.env.*` redactions can't reach. Fixed with a `.args[]` dynamic redaction mirroring the body filters' `_REPO_…` form. - `quickstart_merge`: `WORKTRUNK_COMMIT__GENERATION__COMMAND` pointed at a mock llm binary in the per-test temp dir. Fixed subtractively — the test now uses the suite's standard inline `cat >/dev/null && echo '…'` command (snapshot body unchanged), and the now-unused `create_mock_llm_quickstart` plus the never-called `create_mock_llm_api` are deleted. Also: `test_list_empty_repo` holds a scoped settings guard instead of a cargo-culted `mem::forget`, and tests/CLAUDE.md documents the args channel, the guard test, and the libtest-vs-nextest caveat (nextest is authoritative). Local gate: 3906 tests + lints pass; review iterated to convergence. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
ba518b4f56 | fix(list): tolerate missing index in temp-index probe (#2884) | ||
|
Ben Drucker
|
f41493478d | feat(list): add repo_url to JSON output (#2941) | ||
|
Worktrunk Bot
|
f41614322b | fix(list, prune): degrade gracefully on unborn worktrees (#2937) | ||
|
Maximilian Roos
|
5eb17d441b |
refactor: delete dead code paths left by completed cut-overs (#2931)
Three independent dead-code removals, each a parallel implementation left behind by a completed cut-over and kept alive only by tests (or nothing). ~610 net lines removed, no behavior change (JSON output byte-identical, full test suite passes). - git: drop the by-ref integration API (resolve_ref + ref-taking probes); push.rs uses is_ancestor_by_sha. - list: drop the ListItem serde surface and pre-rendered display-string layer; JSON flows only through JsonItem. - worktree: drop the dead OperationMode enum and the now-constant params of resolve_worktree_arg. |
||
|
Worktrunk Bot
|
ec62580c29 |
revert(hooks): keep docs on pre-start/post-start; code accepts both (#2857)
Per @max-sixty's [direction in #2838](https://github.com/max-sixty/worktrunk/issues/2838#issuecomment-4509447593): revert the docs portion of #2840 and keep the code. Docs continue to recommend `pre-start`/`post-start`; both names work in code so anyone who already followed the briefly-changed docs (e.g. @EcksDy) isn't stranded once a release ships these aliases. ## User-visible — back to `pre-start`/`post-start` - README, docs site, skill mirrors, `dev/*.example.toml`, `plugins/worktrunk/README.md`, `flake.nix`, `.config/wt.toml` - `src/cli/mod.rs` / `src/cli/config.rs` / `src/cli/step.rs` / `src/help.rs` after_long_help and example snippets — and the auto-synced `docs/content/` and `skills/worktrunk/reference/` mirrors - `wt hook --help` canonical subcommand names; completion advertises `-start` only - `HookType` Display via strum, serde `rename`, and clap `ValueEnum` name — all `pre-start`/`post-start`. The Rust variant identifiers stay `PreCreate`/`PostCreate` (internal; we already paid for that rename in #2840, and now the eventual flip is a Display-only change) - `HooksConfig` serde canonical fields ## `*-create` still works (kept code) - `wt hook pre-create` / `post-create` — CLI alias on the canonical subcommand - `pre-create` / `post-create` in config: top-level, `[hooks.*]`, and per-project, in string, `[table]`, and `[[array-of-tables]]` form. Mechanism: serde `alias = ...` on the field, plus a silent in-memory rename in `migrate_content()` so the round-trip in `unknown_tree` doesn't flag table forms as schema-unknown. - The pre-0.32.0 `post-create` fatal-load-error machinery stays removed — the name is reclaimed, and both forms load without error. ## Smaller bits - `valid_user_config_keys()` / `valid_project_config_keys()` append `pre-create` / `post-create` so the unknown-field round-trip skips them. `test_valid_*_keys_all_deserialize` skips both aliases (they can't sit alongside the canonical without a duplicate-field error). - `DEPRECATED_SECTION_KEYS` drops the `pre-start`/`post-start` entries #2840 added — `pre-start`/`post-start` are canonical again. - `find_pre_start_from_doc` / `find_post_start_from_doc` / `find_renamed_hook_key` / `is_non_empty_item` / `migrate_start_hooks_doc` and their tests are removed; the migration direction flips via a new `migrate_create_hooks_doc` (silent, mirrors the prior shape). - Test files `e2e_shell_post_create.rs` and `post_create_commands.rs` rename back to `_post_start_` (via `git mv`, so the rename shows as a rename). ## Testing `cargo run -- hook pre-merge --yes` — 3806 tests pass; the 10 failures are all `case_4` of `shell_wrapper::unix_tests::*` (nu-shell case; `nu` isn't installed in this runner; same failures occur on `main`). Also manually verified that a fresh `wt switch --create` against a project config with `[post-create]` loads cleanly with no unknown-field warning and the hook fires as `post-start`. ## Follow-up Per @max-sixty: in a couple of weeks, once a release with both-names-work is out and users have had a chance to upgrade, the docs flip is straightforward (most of it is in `src/cli/mod.rs`'s `after_long_help` and the doc-sync test propagates). Re #2838. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
d7e3f88422 |
feat(hooks): rename worktree-creation hooks to pre-create/post-create (#2840)
Phase 1 of the staged hook rename tracked in #2838: the worktree-creation hooks `pre-start`/`post-start` become `pre-create`/`post-create`. The old names keep working with no deprecation warning yet (Phase 2, months out, adds the warning). ## What changes - `pre-create`/`post-create` are canonical everywhere: the `HookType` enum, the `HooksConfig` serde fields, the `wt hook` CLI, completion, and all docs. - Old names keep working: `migrate_content()` rewrites `pre-start`/`post-start` config keys to `-create` before serde, and `parse_hook_type` accepts the old CLI names as silent aliases. `wt config update` rewrites them on disk; `wt config show` shows the migration diff. `wt hook <type>` execution and `wt hook show` both accept the old names; completion and `--help` advertise only the canonical names. - `detect_deprecations()` flags the old keys so `update`/`show` act on them, but `format_deprecation_warnings()` stays silent (Phase 2 adds the warning). A new empty-warnings guard in `check_and_migrate` keeps a `-start`-only config from emitting a stray hint. - The dead pre-0.32.0 `post-create` machinery is removed: the fatal `POST_CREATE_REMOVED_MSG` load error, the vestigial `HooksConfig.post_create` merge-fold, and `find_post_create_from_doc`. `post-create` is reclaimed as the canonical background creation hook. ## Semantic flip Before v0.32.0, the key `post-create` named a *blocking* hook. It now names the *background* one. Since v0.44.0, a pre-0.32.0 `post-create` config is a fatal load error on the `check_and_migrate` paths: `ProjectConfig::load` and user/system config loading, which fire on essentially every `wt` command. A repo carrying one has been unusable ever since. The one path that skips that check is `project_config_at_ref` (the base-ref read behind `wt switch --create`), which applies only structural migration. A pre-0.32.0 `post-create` surviving solely on a base ref, never checked out into a worktree, would now load as a background hook rather than folding into the blocking `pre-start`. That edge case is accepted: once `post-create` is valid again, reclaiming the name and detecting the dead key are mutually exclusive. ## Reviewing this diff 205 files, but the substance is ~36 files under `src/`. The rest is regenerated snapshots and auto-synced doc mirrors. Start with: - `src/config/deprecation.rs` — detection (`find_renamed_hook_key`), migration (`rename_hook_key`), removal of the fatal block, the empty-warnings guard, and the `DEPRECATED_SECTION_KEYS` entries that stop unknown-field detection from flagging the migrated keys. - `src/config/hooks.rs`, `src/git/mod.rs` — the serde field and enum renames. - `src/config/project.rs` — `ProjectConfig::load` deserializes `check_and_migrate`'s migrated content, so a current-worktree config using the old keys loads into the canonical fields. - `src/cli/hook.rs`, `src/commands/hook_commands.rs`, `src/completion.rs`, `src/main.rs` — the CLI alias layer; `wt hook show` accepts the old type names as hidden value-parser aliases. - `src/cli/mod.rs` — the `wt hook` docs, including the soft-deprecation note linking #2838. The ~93 modified snapshots also pick up deterministic env-block lines (`GIT_*: ""`, `LLVM_PROFILE_FILE`) that pre-existing snapshots already carry. That is stale-snapshot drift surfaced by the regeneration, not a behavior change. ## Testing Full suite green (3799 tests). New coverage: `snapshot_migrate_start_to_create` (migration preserves value shape and position), `test_deprecated_start_hook_key_runs_silently` and `test_standalone_hook_start_alias_runs_silently` (old config and CLI names run with no warning), `test_config_show_displays_start_hook_migration` (`config show` reveals the diff without an "unknown field" warning), and `test_hook_show_accepts_deprecated_start_hooks` (a current-worktree config using the old keys loads, and `wt hook show` takes both the canonical and the deprecated type arguments). Part of #2838. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Maximilian Roos
|
d9b8eb12e4 |
fix: picker cache, switch ref resolution, and statusline scans (#2842)
Correctness and performance fixes for worktrunk's git-operation paths —
the interactive picker, `wt switch` reference resolution, and the
statusline — surfaced by an automated `clawpatch` review and then
independently re-reviewed against the `reviewing-code` checklist (all
SOLID / Clean-design, zero critical issues).
## Fixes
- **The picker uses fresh repository state for each removal** — after a
removal mutated git's worktree inventory, the next `alt-r` reload could
plan against a stale cache.
- **`wt switch` prefers an exact local branch over stripping a remote
prefix** — a local branch literally named `origin/foo` was previously
retargeted.
- **`wt switch` fails closed on a malformed config when selecting a PR
provider** — a typo in `forge.platform` was silently swallowed and fell
back to GitHub.
- **A single-row statusline skips the repo-wide ahead/behind scan** — a
performance fix for large repositories.
- **Benchmark fix** — the piped-list time-to-first-output benchmark
measured a boundary that emitted no output.
- A clippy `collapsible_if` cleanup.
## Scope
Pure code — the clawpatch tooling and its state files are excluded. One
commit from the source branch that made the statusline skip network
CI/summary tasks was intentionally dropped: a statusline configured with
a CI segment is treated as an explicit opt-in to that network access, so
the statusline keeps its current behavior.
## Coverage
`codecov/patch` reports a gap on `invoke()`'s `changed_directory`
branch. Covering it in-process requires calling `invoke()`, which
mutates process-global cwd (`std::env::set_current_dir`) — a global side
effect `tests/CLAUDE.md` ("No Global State Mutations in Tests") forbids.
Per that doc's own guidance — accept a coverage gap rather than take
global side effects for coverage — the branch is left uncovered, and the
`codecov/patch` miss on those lines is accepted as a justified false
positive.
## Testing
`cargo run -- hook pre-merge --yes` — 3791 tests pass, lints clean.
> _This was written by Claude Code on behalf of Maximilian Roos_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
00c4508671 |
refactor(ci): move CiPlatform to the lib crate, cache on RepoCache, drop the "override" framing (#2692)
Follows up on #2686 (the original "dedup the warning" fix), which parked the configured-platform state in a process-wide `OnceLock` because `CiPlatform` lived in the binary crate and `RepoCache` in the library crate. **Move `CiPlatform` to the lib crate.** It's now `worktrunk::git::CiPlatform` (a new `src/git/ci_platform.rs` module), with `Repository::ci_platform(remote_hint)` replacing the free `platform_for_repo` and a private `Repository::configured_ci_platform()` reading `forge.platform` / `ci.platform`. The configured value caches on a `OnceCell` field of `RepoCache` instead of a process global — same once-per-`wt list` warning dedup, but the repo-scoped data no longer lives in a process static. `src/commands/list/ci_status/platform.rs` is now pure forge dispatch (`gh` / `glab` / `az`) over a `CiPlatform`; the inherent `detect_*` methods became free functions so the enum could move without dragging the bin-crate backends along. **Drop the "override" framing.** `forge.platform` / `ci.platform` is now described everywhere as "the platform, falling back to URL detection when unset" rather than "overriding detection" — config docs (`## Forge platform`), the `ProjectForgeConfig` / `ProjectCiConfig` struct and accessor docs, `wt config state ci-status` help, and a few test names (`test_list_full_with_configured_platform_github`, `test_list_full_with_invalid_configured_platform`, `test_switch_pr_gitea_forge_platform`). Genuine "override" uses elsewhere (config-path/env-var overrides, `wt config state default-branch set`) are untouched. **Fix a pre-existing bug.** `forge.platform = "gitea"` is a valid value — the `wt switch pr:` shortcut uses it to pick `tea` — but worktrunk fetches CI status only from GitHub/GitLab, so `wt list` used to print `Invalid CI platform in config: 'gitea'. Expected 'github' or 'gitlab'.` for it. `configured_ci_platform()` now recognizes `gitea` as a known forge it just doesn't show CI for: no warning, blank CI column. Added `test_list_full_with_gitea_forge_platform` (asserts blank CI and empty stderr). Branch is merged with `main`, so the dispatch and detection handle the new `AzureDevOps` variant alongside GitHub and GitLab. `cargo run -- hook pre-merge --yes` passes. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
369923f535 |
fix(list): OR over local + upstream in integration column (#2513)
## Summary - `wt list`'s integration column now agrees with the safety path (`wt remove` / `wt merge`) in the local/upstream-diverged case introduced by #2507. A branch merged into local `main` while `origin/main` has unique commits no longer renders as unintegrated. - New `Repository::integration_targets()` returns a `(primary, optional secondary)` pair that mirrors `compute_integration_reason_uncached`'s preamble: a single ref for same-commit / strict-subset cases, both refs only when local and upstream have diverged. - Each integration task (`IsAncestor`, `CommittedTreesMatch`, `HasFileChanges`, `WouldMergeAdd`) combines signals across the pair — OR for positive signals (ancestor, trees match, patch-id), AND for inverted ones — and short-circuits when the primary alone proves integration, so the common one-sided case stays one git op. - `effective_integration_target` is still used by the `step prune` path; it's the column that needed the OR semantics. ## Test plan - [x] New regression test `test_list_integrated_when_merged_locally_with_upstream_diverged` mirrors `test_remove_merged_locally_when_upstream_diverged`. Verified to fail without the fix (renders `main_state == "behind"`) and pass with it. - [x] Symmetric `test_list_integrated_when_squash_merged_on_remote_with_local_diverged` covers the merged-on-remote direction. - [x] `cargo run -- hook pre-merge --yes` — 3407/3407 pass, no snapshots to review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
11caab2f39 |
refactor(cache): unify on-disk cache mechanics; add summary cache to state (#2407)
## Summary
Consolidates the three on-disk caches (`sha_cache`, `ci_status`,
`summary`) onto a shared `worktrunk::cache` module — one implementation
of torn-write semantics, error policy, LRU sweep, and clear mechanics
across all of them.
- **Shared primitives in `src/cache.rs`**: `read_json` / `write_json` /
`read` / `write_with_lru` / `sweep_lru` / `clear_one` /
`clear_json_files` / `count_json_files` / `cache_dir`. Torn writes are
treated as cache misses (no temp-file-plus-rename); reads/writes degrade
silently; user-initiated clear paths propagate non-`NotFound` I/O errors
so `wt config state clear` can't lie about success.
- **Content-addressed summary cache**: layout is
`summary/{branch}/{hash}.json` where `hash` is the SHA-256 prefix of the
combined diff. A cache hit is "does the file exist?" — no JSON parse for
the staleness check. Prune-on-write trims siblings for the same branch
via `cache::sweep_lru(branch_dir, 1)`, the same mechanism `sha_cache`
uses at cap 5000, just scoped to a per-branch directory.
- **`wt config state get/clear` surfaces summaries**: new SUMMARY CACHE
table + JSON section. `state clear` also clears summary entries. Parity
invariant between show and clear extended to cover the new cache.
- **sha_cache renamed directory** `summaries/` → `summary/` to match the
singular/operation-descriptive naming of every other kind (`ci-status`,
`is-ancestor`, etc.). Stale `summaries/` dirs on users' machines are
harmless; entries re-generate on next access.
- **Peer treatment across caches**: `sha_cache` now exposed as
`worktrunk::git::sha_cache`; deletes
`Repository::{clear,count}_git_commands_cache` one-line wrappers.
`CachedCiStatus::list_all` drops the redundant tuple (branch was in the
struct already). Both `list_all`s pre-sort (newest-first with branch
tiebreak), deleting four duplicated `sort_by` blocks in state.rs.
- **Net**: +1022 / -714 lines (tests + new summary cache code). Core
simplification: `sha_cache` shrinks from 345 → ~270 lines,
`prune_siblings` deleted, 30 lines of `put_*` boilerplate collapsed into
two cache-module helpers.
## Test plan
- [x] `cargo run -- hook pre-merge --yes` — 3351 tests pass, pre-commit
clean, rustdoc clean
- [x] Snapshots updated for `SUMMARIES CACHE` → `SUMMARY CACHE` heading
rename
- [x] New `CachedSummary` covered by roundtrip, prune-on-write,
list_all, clear_all tests
- [x] `wt config state get` and `wt config state clear` exercised
against populated CI + summary caches in integration tests
- [x] sha_cache tests still pass after the `write_with_lru` /
`sweep_lru` extraction
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
4c731d035a |
fix(state): surface unset_config failures instead of lying about them (#2394)
Follow-ups from review of #2392. That PR fixed the ci-status single-branch clear by replacing one `unset_config(...).unwrap_or(false)` with `?`, but the same actively-lying pattern was still present across `src/commands/config/state.rs` for markers, vars, previous-branch, and default-branch. When `git config --unset` fails for a real reason (corrupt config, permission denied) — not just "key didn't exist," which is a clean `Ok(false)` via git's exit code 5 — each site silently reports "No X to clear", the exact lie the ci-status fix was meant to eliminate. ## What changed - **`CachedCiStatus::clear_one` / `clear_all`** now return `anyhow::Result<bool>` / `anyhow::Result<usize>`. Both match on `ErrorKind::NotFound` to keep the normal "nothing to clear" path silent; other I/O errors propagate. `clear_all` also tolerates NotFound at the per-file level to absorb the race where another process deleted the file between `read_dir` and `remove_file`. Three ci-status call sites in `state.rs` use `?`. - **Eight remaining swallows in `state.rs` converted to `?`**: `unset_config(...).unwrap_or(false)`, `let _ = unset_config(...)`, and `matches!(repo.clear_default_branch_cache(), Ok(true))`. Covers previous-branch (single + aggregate), marker (branch-specific clear), vars (specific key + both `--all` loops), and default-branch (aggregate). The `Ok(false)` path still drives the "No X to clear" info message, so UX is unchanged for the common case. - **New `clear_all_markers` helper** (per review feedback) extracted next to `clear_all_vars` — the `--get-regexp` + `unset_config` loop appeared twice in `state.rs` (marker `--all` inside `handle_state_clear`, plus the aggregate `handle_state_clear_all`). Consolidates the `.unwrap_or_default()` rationale to one docstring. Git exits 1 when no keys match, which is the dominant "nothing to clear" signal — distinguishing that from genuine config failures would need a new `Repository` helper for exit codes, left as separate follow-up. - **Test fixtures canonicalized** from `"source":"pull-request"` (legacy serde alias) to `"source":"pr"` (canonical) — 10 occurrences across `config_state.rs` and `list.rs`. The `#[serde(alias = "pull-request")]` on `CiSource::PullRequest` is preserved so on-disk caches written by older versions still deserialize. - **Unit tests for the new error branches** (4 added to `cache.rs::tests`). codecov/patch on the initial push reported 75.55% of diff hit — the new non-NotFound branches aren't reachable from integration tests. Cover them by planting filesystem states that provoke specific `io::ErrorKind` values: directory-where-file-goes for `remove_file` EISDIR, file-where-dir-goes for `read_dir` NotADirectory, and a sibling `.json.tmp` to verify the extension filter keeps non-json entries intact. Patch coverage now 98.86%. ## Not touched `sha_cache::clear_all` keeps its silent-swallow shape. Its module docstring explicitly enshrines "All cache failures ... degrade silently. Callers must never observe cache failures" — purely internal best-effort cache with no user-facing success/info message. The asymmetry is warranted. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
390b025953 |
refactor(list): drop CommitDetailsTask and the per-SHA fallback (#2379)
Now that the pre-skeleton `commit_details_many` batch fetches `(timestamp, subject)` for every item SHA (PR #2369), `CommitDetailsTask` has been a pure DashMap lookup wrapped in rayon spawn + channel send + drain match — pure overhead per item. Drop the task type and populate `ListItem.commit` from the batch map directly in the post-skeleton phase. ## Scope - Remove `CommitDetailsTask` / `TaskKind::CommitDetails` / `TaskResult::CommitDetails` and their drain + dispatch + seeding plumbing. - Sort helpers (`sort_by_timestamp_desc_with_cache`, `sort_worktrees_with_cache`) now accept the full `&HashMap<String, (i64, String)>` — single canonical source. - Post-skeleton loop writes `item.commit` from the batch map directly. `populate_item()` (the statusline single-item entry point) also does its own one-SHA `commit_details_many` lookup so `wt statusline --format=json` keeps reporting real `commit.message` / `commit.timestamp`. - Drop the old silent fallback: on batch failure, the task used to fork `git log -1` per-SHA via `Repository::commit_details()`. The `collect()` path now prints a loud warning on batch failure and renders placeholders for that run. `populate_item()` is silent on failure — the statusline has no UI for warnings. - Dead-code cleanup: `Repository::commit_details()` (singular), the `RepoCache::commit_details` DashMap field, the cache-priming side effect inside `commit_details_many`, and the circular `commit_details_and_many_store_identical_cache_entries` test. ## What stays - Batch parser tests (spaces, multibyte UTF-8, duplicate SHAs, empty input, unknown-SHA error propagation) still cover `commit_details_many` directly. - Prunable worktrees are still skipped so Age/Message render `·` — preserves the old task-queue UX for items whose directory is missing from disk. ## What's unchanged - Skeleton latency: no pre-skeleton additions. - `wt list` display: all snapshot tests pass. - `batch_ahead_behind` / `ahead_behind()` fallback: kept — that's version-compat for git < 2.36, not error-recovery. ## Codecov `codecov/patch` reports 90.90% — below the 96.63% target. The ~5 uncovered lines are the "fail loudly" batch-error warning path in `collect()` and the `bail!` for malformed git output in `commit_details_many`. Driving those from an integration test would require a pathological repo setup; the unit-test `commit_details_many_fails_loudly_on_unknown_sha` pins the contract the warning path depends on. Merging with explicit approval. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Maximilian Roos
|
3c224cba7b |
fix(list): qualify remote refs so same-named local branch can't shadow them (#2365)
Git lets users create a local branch literally named `origin/foo`. When `wt list --remotes` passed the short name from a remote row into the integration helpers (ahead/behind, trees_match, is_ancestor, has_added_changes, merge_integration_probe, has_merge_conflicts, branch_diff_stats), git's `rev-parse` picked `refs/heads/origin/foo` over `refs/remotes/origin/foo` — and the remote row silently reported stats against the local branch. Adds `BranchRef::integration_ref()`, which returns `refs/remotes/<name>` for remote branches and the short name for local ones, then routes the seven task call sites in `tasks.rs` through it. The TODO at `src/git/mod.rs:427` about migrating `BranchRef` to store full refs directly stays — that broader refactor is out of scope here. Regression test at `tests/integration_tests/list.rs` sets up a remote `origin/foo` one commit ahead of main and a local branch literally named `origin/foo` two commits ahead, runs `wt list --remotes --format=json`, and asserts the remote row reports `main.ahead = 1`. Fails on main (reports 2), passes after the fix. A unit test covers the helper's three cases (remote, local, detached HEAD). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
1cd5bcf5c3 |
perf(config): batch git config reads, collapse ~30% off alias dispatch (#2344)
## Summary Replaces per-key `OnceCell`/`DashMap` caches in `RepoCache` with a single `git config --list -z` read, stored in an `IndexMap<String, Vec<String>>`. Every config-backed accessor now resolves via an O(1) map lookup instead of its own `git config …` subprocess. Affects: `is_bare`, `primary_remote`, `remote_url`, `primary_remote_url`, `all_remote_urls`, `default_branch` fast path, `init.defaultBranch`, `branch_marker`, `switch_previous`, `has_shown_hint`, `list_shown_hints`. Writes route through new `set_config_value` / `unset_config` helpers that update the on-disk config AND the in-memory map together, keeping in-process reads coherent with writes. All existing writers (in `state.rs`, `switch.rs`, `config.rs`) are routed through these. Secondary change: `default_branch()` fast path stops validating that the persisted value still resolves locally, and `invalid_default_branch_config()` + its two preflight warning sites are removed. A new `GitError::StaleDefaultBranch` surfaces from `require_target_branch` / `require_target_ref` (only when `target: None` — i.e., resolved from the cache, not user input) with a clear cache-reset hint. `wt list --branches` opportunistically warns when the cached default isn't in the already-enumerated branch set — no extra fork. Motivates issue #2322. ## Performance Benchmarked via the `benches/alias` harness (currently landing on a separate branch). Five variants; `wt_version` is the startup floor (no repo discovery), the rest dispatch a noop alias at 1 and 100 worktrees with warm and cold caches. | Variant | Before | After | Change | |---|---|---|---| | `wt_version` | 5.08 ms | 4.88 ms | −3% | | `warm/1` | 77.4 ms | 56.9 ms | **−29%** | | `cold/1` | 80.9 ms | 55.1 ms | **−31%** | | `warm/100` | 76.9 ms | 54.4 ms | **−28%** | | `cold/100` | 80.5 ms | 54.5 ms | **−30%** | ~25 ms saved per dispatch, matching the expected win from collapsing 4–5 cold `git config` fork+startup overheads into one bulk read. `wt_version` is flat, confirming the change only affects the Repository-touching paths. ## Not in scope / explicit escape hatches - `vars_entries` and `all_vars_entries` bypass the bulk cache and read git config directly — lazy template expansion in hook/alias pipelines depends on seeing writes that earlier steps made via their own `git config` subprocesses, and those don't round-trip through `set_config_value`. - `effective_remote_url` unchanged — `url.insteadOf` rewriting lives in `git remote get-url`, not raw config. - `core.fsmonitor` and `core.pager` aren't in `RepoCache` today; left on per-call reads. - Key lookups normalize to git's canonical form (section/variable lowercased, subsection preserved) so `init.defaultBranch` matches the emitted `init.defaultbranch`. - `IndexMap` preserves config-file order so `primary_remote` picks the first remote with a URL the same way the old `--get-regexp` path did. ## Test plan - [ ] `cargo run -- hook pre-merge --yes` green locally (3303 tests + clippy + fmt + docs) - [ ] CI passes New tests: - Unit tests for `parse_config_list_z` (multivars, newlines in values, `=` in values, empty) and `canonical_config_key` (2-part, 3-part, 4+-part keys) - `test_merge_with_stale_default_branch_cache` — verifies `StaleDefaultBranch` surfaces with clear/set hints when default-branch cache is stale - `test_list_branches_with_nonexistent_default_branch` — verifies the `wt list --branches` opportunistic warning fires - Existing `test_configured_default_branch_is_trusted_without_validation` (formerly `…_returns_none`) — verifies the persisted value is returned as-is 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
256115afbf |
fix(list): treat [gone] upstream as no upstream (#2262)
## Summary When a branch's configured upstream ref is gone — remote branch deleted, local tracking ref pruned — `wt list` was surfacing a raw git error in the Remote column: ``` <branch>: upstream (fatal: ambiguous argument 'origin/<branch>': unknown revision or path not in the working tree.) ``` `ahead_behind()` ran `git rev-parse` on a ref that no longer exists. `fetch_all_upstreams` now also reads `%(upstream:track)` — git's literal `[gone]` value is treated the same as an unconfigured upstream (returns `None` for the branch), so the Remote column blanks gracefully. One change point, one choke point: `Branch::upstream()` goes through this cache, and everything downstream (`ahead_behind`, column rendering) already handles `None`. No new state, no new code path. ## Test plan - [x] Added scenario 6 to `test_list_with_upstream_tracking` — push branch with `-u`, delete on remote, `fetch --prune`, verify the row renders identically to the existing no-upstream case. - [x] Full suite: 1473 tests pass. - [x] Lints: `cargo clippy --all-targets -- -D warnings` and `pre-commit run --all-files` clean. - [x] Verified live against the originally failing repo — error gone, row renders cleanly. > _This was written by Claude Code on behalf of Maximilian._ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
b174658e29 |
Split directive file into CD (raw path) and EXEC (shell) files (#2118)
The shell wrapper previously used a single `WORKTRUNK_DIRECTIVE_FILE` where wt wrote shell commands (`cd '/path'`, arbitrary `--execute` payloads). This meant the cd path went through shell parsing — any content wt wrote was sourced as shell. This splits the protocol into two files with different trust levels: - **`WORKTRUNK_DIRECTIVE_CD_FILE`** — raw path, read with `cd -- "$(< file)"`. No shell parsing, no escaping, no injection surface. Safe to pass through to alias/hook child processes. - **`WORKTRUNK_DIRECTIVE_EXEC_FILE`** — arbitrary shell (from `--execute`), sourced by the wrapper. Scrubbed from alias/hook child environments so hook bodies cannot inject shell into the parent session. When a nested `wt` inside an alias body tries `--execute` without the EXEC file, the command is dropped with a warning linking to #2101 for user feedback. The old `WORKTRUNK_DIRECTIVE_FILE` is silently honored for one release (users who upgrade wt without restarting their shell). Bash, zsh, fish, and PowerShell self-update on restart; nushell requires `wt config shell install`. Closes #2101 > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
8e9b2ebd61 |
Replace git stash create with write-tree for working tree conflict checks (#2119)
`WorkingTreeConflictsTask` in `wt list` used `git stash create` to snapshot dirty working trees before running `merge-tree`. This had two problems: stash create was expensive (~50-265ms per dirty worktree), and the stash commit SHA included a timestamp, making the persistent probe_cache always cold. Replaced with `git write-tree` (~15ms), which reads the index directly. When unstaged modifications or untracked files exist, copies the index to a temp file and runs `git add -A` before write-tree (same temp-index pattern as `step_diff`). On cache miss, creates an ephemeral commit via `commit-tree` for merge-tree's merge-base resolution; on cache hit, skips commit creation entirely. The cache key is `(base_commit_sha, branch_head_sha+tree_sha)` — a composite that captures all three inputs to the three-way merge. The branch HEAD SHA tracks the merge-base dependency (stash parent = branch HEAD), preventing false hits after rebases. Also extracted `run_merge_tree` from `has_merge_conflicts` to eliminate a redundant cache lookup in the tree-keyed path, and updated the caching spec in `collect/mod.rs` to document the new "Cached via tree SHA" category. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
d50bdfcde0 |
Cache remaining expensive tasks, unify wt list and picker (#2098)
Adds persistent SHA-keyed caching for `is_ancestor`, `has_added_changes`, and `branch_diff_stats` — the last three operations still running unconditionally in both `wt list` and the `wt switch` picker. With all five merge-base-dependent tasks now cached (joining `merge-tree-conflicts` and `merge-add-probe` from PR #2085), the `stale_branches` skip mechanism becomes redundant: the picker's "skip tasks on branches > 50 behind main" optimization was papering over slow first-run cost that the cache now handles eternally. ## What changed **New caches** (`src/git/repository/probe_cache.rs`): three new asymmetric SHA-keyed kinds — `is-ancestor`, `has-added-changes`, `diff-stats` — all using the same LRU-swept persistent file layout as the existing kinds. `branch_diff_stats` skips its cache when sparse checkout is active (path filters make the result environment-dependent). **Dead plumbing removed**: `EXPENSIVE_TASKS` constant, `CollectOptions::stale_branches` field, `skip_expensive_for_stale` parameter on `collect()`, the env-var gate in `list/mod.rs`, and two obsolete tests + snapshots. Both `wt list` and the picker now run the same task set. **`batch_ahead_behind` unconditional**: previously only the picker called it. Making it unconditional replaces N `rev-list --count` calls with one `for-each-ref`, a pure win for `wt list` too. **Branch-ref semantics unified across all tasks**: all task implementations now prefer the branch name over `ctx.branch_ref.commit_sha` when one is present. Previously, `IsAncestorTask`, `BranchDiffTask`, `MergeTreeConflictsTask`, and `CommittedTreesMatchTask` used the worktree's current HEAD sha, which during a rebase-in-progress is transiently at the replayed commit rather than the branch tip. This produced contradictory rows: `is_ancestor=true` + `1 ahead / 1 behind`. Now all tasks consistently report the branch's state, matching `git status` semantics. Addressed worktrunk-bot review feedback. **Caching docs updated**: the `## Caching` section in `collect/mod.rs` (from PR #2097) now lists the three tasks as cached rather than "cacheable but uncached". Also addressed worktrunk-bot PR overlap observation. **Benchmarks**: `invalidate_caches_auto()` now clears `.git/wt/cache/` so cold-cache benchmarks exercise cold state. The `warm_optimized` variant in `real_repo_many_branches` collapsed since it no longer measures anything different from `warm`. ## Testing New probe_cache unit tests cover roundtrip + tamper-based cache consultation for each new kind, plus a `clear_all_covers_all_kinds` regression test. Snapshot updates for `test_list_maximum_status_with_git_operation` and `test_list_json_with_git_operation` reflect the branch-ref fix — mid-rebase worktrees now show `✗` (real conflicts) and actual diff stats instead of the transient HEAD's empty state. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
741dbae138 |
Unify conflict-detection skip sets between wt list and wt switch (#2064)
Both `wt list` (non-full) and the `wt switch` picker now run both `WorkingTreeConflicts` and `MergeTreeConflicts`. The only remaining asymmetry is `SummaryGenerate`: `wt list` still skips it (doesn't render summaries inline), the picker still runs it (tab 5 preview). ## Why The picker's old skip set was inconsistent in two ways: 1. **Cost inverted.** It skipped the cheaper probe (`MergeTreeConflicts` = one `merge-tree HEAD base` call) and ran the more expensive one (`WorkingTreeConflicts` = `git status` + `git stash create` + `git merge-tree` for dirty worktrees). 2. **Fallback unreachable.** `WorkingTreeConflicts` short-circuits to `None` on clean worktrees with an explicit contract that the caller falls back to `MergeTreeConflicts` (see `src/commands/list/collect/tasks.rs:437`). The picker blocked that fallback, so clean worktrees in the picker produced no real conflict answer — they fell through to the struct default. `wt list` non-full's old choice was internally consistent (skip the working-tree probe, always use commit-level), but asymmetric with the picker. Adding `WorkingTreeConflicts` to `wt list` is strictly a correctness improvement: rows with a conflicted rebase-in-progress now render `✗` instead of a counts-based fallback. ## Safety Adding `MergeTreeConflicts` to the picker is safe because the per-symbol atomic rendering refactor on this branch makes gate 3 position 4 render as `⋯` when its inputs haven't resolved within the collect deadline. Tail cost is capped, not unbounded. ## Reviewer tour **Commit 1** (skip-set unification): - `src/commands/list/collect/mod.rs` — one line removed from non-full skip set. - `src/commands/picker/mod.rs` — one line removed from picker skip set; cargo fmt collapsed the now-2-element array. - `src/commands/list/layout.rs` — test helper `non_full_skip_tasks()` updated to match the new production skip set (its doc comment claimed to mirror non-full mode). - `tests/integration_tests/list.rs` — renamed `test_list_full_working_tree_conflicts` → `test_list_working_tree_conflicts` and updated its module doc + inline comments; the test now documents that both modes detect working-tree conflicts. - Two snapshot updates are the point of the change — one gains `✗` for a real conflict, the other gains a second "task failed" warning line (both conflict tasks now fail symmetrically on a broken HEAD). **Commit 2** (doc-comment cleanup, from auto-review feedback): - Dropped stale "--full only" qualifiers from 5 doc-comment locations in `tasks.rs`, `types.rs`, and `mod.rs`, plus one in the test file. No behavior change. ## Follow-up worth considering separately For dirty worktrees the picker now runs both `WorkingTreeConflicts` and `MergeTreeConflicts`, duplicating a `merge-tree` call. If that shows up in profiles, the right fix is conditional dispatch — skip `MergeTreeConflicts` when `WorkingTreeConflicts` produced a non-`None` answer — rather than reinstating the asymmetric skip sets. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
c84c87d47b |
refactor: remove lifetime guard field from TestRepo (#2000)
The `_lifetime_guard: Option<Box<dyn Any>>` field and `set_lifetime_guard()` method existed to hold insta snapshot filter guards alive via type erasure — insta is a dev-dependency and can't appear in `src/` code. Replace with `std::mem::forget(guard)` at the two call sites (the `repo()` rstest fixture and `test_list_empty_repo`). The insta settings are thread-local, so leaking the scope guard keeps them active without needing to store anything in TestRepo. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
c03aa3aaef |
refactor: consolidate TestRepo into single src/testing module (#1991)
Two `TestRepo` structs existed — a lightweight one in `src/testing.rs` (74 lines, for unit tests) and a full-featured one in `tests/common/mod.rs` (~1500 lines, for integration tests). This consolidates them into one struct in `src/testing/` that serves both. The key blocker was the integration TestRepo's `_snapshot_guard: insta::internals::SettingsBindDropGuard` field — `insta` is a dev-dependency and can't appear in `pub` code in `src/`. Replaced with `Option<Box<dyn Any>>` (type-erased). Snapshot guard setup moved to the `repo()` rstest fixture. **What moved to `src/testing/`:** - `TestRepo` struct with all ~80 methods - `BareRepoTest`, `TestRepoBase` trait - `mock_commands` module - Helper functions (env isolation, fixture copying, CLI command builders, wait utilities) - Constants (`TEST_EPOCH`, timing constants, etc.) **What stayed in `tests/common/`:** - rstest fixtures (`repo()`, `repo_with_*`, etc.) - Snapshot settings functions (all `insta::Settings` code) - PTY functions (`portable_pty` code) - Re-exports: `pub use worktrunk::testing::*` **Constructor semantics:** - `TestRepo::new()` — lightweight `git init` + identity (backward-compatible with unit tests) - `TestRepo::standard()` — fixture copy with remote + worktrees + mock gh (what integration `new()` was) - `TestRepo::with_initial_commit()` — lightweight + one commit - `TestRepo::empty()` — `git init` with no commits > _This was written by Claude Code on behalf of maximilian_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
53e11fa168 |
docs: fix marker position and use snapshot-backed example (#1970)
The marker help text incorrectly stated markers appear at the "start" of the Status column with examples like `🚧↑`. The rendering code (`status_symbols.rs:300`) places markers at the end, after all git symbols (`↑🚧`). Replaced the hand-written example with the `<!-- wt list (markers) -->` placeholder pattern, backed by a new `test_readme_example_list_marker` snapshot test. In terminal `--help`, this shows `wt list` as a command hint; in web docs, it expands to the actual colored table output. > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
b33cc8e9d6 |
Simplify forge detection with [forge] config section (#1826)
SSH host aliases (`git@work-ssh:owner/repo`) corrupt the hostname in
remote URLs but never the path. The previous forge detection code tried
to work around this by searching all remotes, auto-detecting GHE
hostnames, and maintaining multiple resolution strategies. This PR
replaces all of that with a simpler design: derive owner/repo from URL
paths, and let users set `forge.platform` / `forge.hostname` for corner
cases.
## Key changes
**New `[forge]` config section** with `platform` and `hostname` fields.
`ci.platform` is deprecated with migration support. Example configs and
inline help examples updated.
**`fetch_pr_info`** now builds literal API paths from the primary
remote's raw URL instead of relying on `gh`'s placeholder resolution.
`--hostname` only passed when explicitly configured.
**`platform_for_repo`** checks config > branch's remote > primary
remote. No longer searches all remotes. Loads config internally instead
of taking a `platform_override` parameter.
**`find_remote`** matches by owner/repo only (no host required),
handling SSH aliases where the local hostname differs from the API
hostname.
**`CiBranchName::from_branch_ref`** uses `split_once('/')` instead of
iterating `all_remote_urls()`, removing a git call per branch during `wt
list`.
## Deleted
- `resolve_gh_hostname` (GHE auto-detection heuristic)
- `find_forge_remote` (search all remotes by predicate)
- `all_remote_urls()` loop in branch name parsing
- `platform_override` parameter threading through callers
The spec in `src/git/repository/remotes.rs` documents the full design.
> _This was written by Claude Code on behalf of max-sixty_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
worktrunk-bot
|
82d02239f3 | fix: correct --full example text and add removable worktree to docs (#1775) | ||
|
Maximilian Roos
|
550ce8a70f |
Enrich quick start wt list output (#1760)
The quick start `wt list` example was sparse — only staged additions (+53), no removed lines, no commits ahead of main, and both branches showing the same "Initial commit" hash. Now feature-auth has a committed change plus staged WIP, so the output showcases more columns at a glance: ``` Branch Status HEAD± main↕ Remote⇅ Commit Age Message @ feature-auth + ↑ +27 -8 ↑1 4bc72dc9 2h Add authentication module ^ main ^⇡ ⇡1 0e631add 1d Initial commit ``` The test creates a two-phase setup: commit auth.rs (1 commit ahead of main), then stage WIP changes that extend auth.rs and restructure lib.rs (producing both additions and deletions in HEAD±). The explanatory text was updated to mention `↑1`. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
a2482f1da9 |
refactor: change test git_command() to return Cmd instead of Command (#1718)
The test infrastructure's `git_command()` returned
`std::process::Command`, so test git commands bypassed `Cmd`'s debug
logging (`$ git status [ctx]`) and timing traces (`[wt-trace]`). This
changes it to return `worktrunk::shell_exec::Cmd`.
## Approach
Added `configure_git_env(Cmd, &Path) -> Cmd` alongside the existing
`configure_git_cmd(&mut Command)`. The `Command` version is kept because
`configure_wt_cmd` (which configures wt binary commands) still needs it
— wt commands go through `Command`, not `Cmd`.
Key changes in `tests/common/mod.rs`:
- `TestRepoBase::git_command()` and `TestRepo::git_command()` now return
`Cmd`
- All wrapper methods (`run_git`, `git_output`, `head_sha`, etc.) use
`.run()` instead of `.output()`
- `BareRepoTest::new()` and `NestedBareRepoTest::new()` use
`Cmd::new("git")` for init
247 call sites across 25 files converted: `.output()` → `.run()` and
`.status()` → `.run()` on git command chains. Zero `Command::new("git")`
remaining in the test directory.
Follows #1714 and #1716 which converted raw `Cmd::new("git")` and
`Command::new("git")` in test bodies to `repo.run_command()`.
> _This was written by Claude Code on behalf of maximilian_
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||
|
worktrunk-bot
|
ce9ea1a6b6 |
refactor: consolidate .git/wt-* paths under .git/wt/ and stage removed worktrees in trash (#1583)
## Summary - Moves the rename-based staging directory from a visible sibling path (`project.wt-removing-<timestamp>`) into `.git/wt/trash/`, hiding it from the user's workspace - Consolidates all worktrunk-managed `.git/wt-*` directories under a single `.git/wt/` parent - Adds `Repository::wt_dir()` accessor as the single root for all worktrunk state - Renames `wt-relocate-tmp` to `wt/relocate-staging` for consistency with `wt/promote-staging` - Falls back to legacy `git worktree remove` if the trash directory can't be created - The `.git/` directory is always on the same filesystem as worktrees, so the instant rename guarantee is preserved ## Path migration | Before | After | |--------|-------| | `.git/wt-logs/` | `.git/wt/logs/` | | `.git/wt-cache/summaries/` | `.git/wt/cache/summaries/` | | `.git/wt-cache/ci-status/` | `.git/wt/cache/ci-status/` | | `.git/wt-promote-staging/` | `.git/wt/promote-staging/` | | `.git/wt-relocate-tmp/` | `.git/wt/relocate-staging/` | | (new) `.git/wt/trash/` | Staging for background removal | ## Context Users reported confusion when seeing `.wt-removing-*` directories in their workspace after `wt remove` (#1572). By staging in `.git/wt/trash/` instead, the directory is completely hidden — even if the background `rm -rf` is slow or gets interrupted. The `.git/wt-*` sibling directories were also consolidated into `.git/wt/` for tidiness per review feedback. ## Test plan - [x] Unit tests for `generate_removing_path` and `build_remove_command_staged` updated and passing - [x] All 105 remove-related integration tests passing - [x] `test_remove_background_path_gone_immediately` — verifies instant removal still works - [x] `test_remove_background_fallback_on_rename_failure` — verifies fallback when staging path is blocked - [x] `test_remove_stale_staging_dir_from_crashed_removal` — verifies stale dirs land inside `.git/` - [x] All 2496 tests pass (lib + bin + integration) - [x] Help snapshots, doc sync, and lint checks all pass Closes #1572 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> |
||
|
worktrunk-bot
|
1afa9c230b | refactor: rename get_* functions to bare nouns (#1586) | ||
|
Maximilian Roos
|
bb3efee9c1 |
fix(list): handle empty repos (no commits) gracefully (#1058)
* fix(list): handle empty repos (no commits) gracefully Skip commit-dependent tasks for unborn branches (null OID) using a COMMIT_TASKS constant, following the existing EXPENSIVE_TASKS pattern. Filter null OIDs from timestamp batching, accept unborn default branch in validation, and render empty commit/age cells instead of garbage. Closes #885 Co-Authored-By: Claude <noreply@anthropic.com> * fix: address codecov/patch coverage gaps - Remove unreachable COMMIT_TASKS check from work_items_for_branch (null OIDs only appear in worktree HEAD, never in git for-each-ref) - Restructure json_output null OID handling to eliminate dead branch - Pre-set default branch config in test to exercise is_unborn_head_branch path Co-Authored-By: Claude <noreply@anthropic.com> * fix: remove unused BranchRef::has_commits() (dead code) Only WorktreeInfo::has_commits() is called in the dispatch code. BranchRef::has_commits() was never referenced, causing a codecov gap. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
50d12a7e14 |
refactor: rename wt select references to wt switch interactive picker (#959)
* refactor: rename `wt select` references to `wt switch` interactive picker Update all documentation, comments, and test references from the deprecated `wt select` to `wt switch` interactive picker. Migrate TUI tests from select.rs to switch_picker.rs exercising `wt switch` directly. - Docs/comments: Replace `wt select` with `wt switch` interactive picker - Tests: Rename select.rs → switch_picker.rs, use `wt switch` CLI args - Remove redundant tests covered by existing switch.rs tests - Fix pre-existing incorrect binary/path in src/commands/CLAUDE.md - Leave alone: CHANGELOG, runtime deprecation warnings, [select] config section names, demo assets Co-Authored-By: Claude <noreply@anthropic.com> * fix: add missing switch_picker snapshot files for CI The switch_picker tests (migrated from select tests) generate snapshots via PTY execution, which need to be committed for CI to pass. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
1f239aced7 |
docs: add Quick Start section to front page (#864)
Reapply Quick Start documentation with fix for README rendering. The terminal output blocks now include explicit `$ ` prompts before commands, which makes GitHub's console syntax highlighting work correctly (commands styled differently from output). Changes: - Add Quick Start section showing switch, list, merge workflow - Add snapshot tests for Quick Start terminal output examples - Fix strip_html() to convert .cmd spans to `$ ` prefixed commands - Sync skill reference files from docs Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
a6d39d053f |
Revert "docs: add Quick Start section to front page (#851)"
This reverts commit
|
||
|
Maximilian Roos
|
69b3e47509 |
docs: add Quick Start section to front page (#851)
* docs: add Quick Start section to front page Shows the basic workflow with clear directory paths at each stage: - Create worktree with wt switch --create - Check status with wt list - Two merge paths: PR workflow (push + gh pr create + wt remove) and local merge (wt merge) - Parallel agents example with -x flag Co-Authored-By: Claude <noreply@anthropic.com> * docs: improve Quick Start with realistic examples and snapshot sync - Add snapshot-based output sync for quickstart examples (switch, list, merge) - Make examples more realistic: 53 lines of auth module changes instead of 9 - Show uncommitted changes in wt list demo (WIP state, not already committed) - Add wt step commit to PR workflow since changes are now uncommitted - Remove redundant git push from PR workflow (gh pr create auto-pushes) - Suppress worktree-path hint in quickstart tests for cleaner output - Add transform_zola_to_github() for converting HTML terminal markers to plain code blocks Co-Authored-By: Claude <noreply@anthropic.com> * Simplify code block formatting in README and tests Convert bash code blocks to console format with command prompts and output in a single block. Update regex pattern to optionally strip redundant bash preamble when converting AUTO-GENERATED-HTML terminal markers. * docs: remove redundant bash blocks before terminal examples The terminal shortcodes already include the command with $ prefix, so separate bash blocks showing just the command were redundant. Also fix broken anchor link in config.md. Co-Authored-By: Claude <noreply@anthropic.com> * feat(docs): make terminal prompt non-copyable via CSS Use CSS ::before pseudo-element to generate the $ prompt, making it structurally non-copyable. This works with both manual text selection and copy buttons. Changes: - Add .cmd::before { content: "$ "; } to generate prompt via CSS - Remove explicit <span class="prompt">$</span> from HTML output - Extract command from snapshot YAML header instead of parsing HTML - Update expand_command_placeholders for command pages (list.md, etc.) Co-Authored-By: Claude <noreply@anthropic.com> * fix(docs): show commit step in quick start merge example The merge example now shows staged changes being committed as part of the wt merge workflow, reflecting a realistic user experience where code is staged but not yet committed before merging. Co-Authored-By: Claude <noreply@anthropic.com> * style: fix cargo fmt formatting Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): use cross-platform mock LLM for quickstart_merge test The quickstart_merge test was using a shell script for the mock LLM which doesn't work on Windows. Now uses the mock-stub system which creates a cross-platform mock binary. Co-Authored-By: Claude <noreply@anthropic.com> * fix(test): use to_slash_lossy for Windows path compatibility Windows paths with backslashes trigger shell metacharacter handling, which wraps the command in `sh -c`. Bash can't parse Windows paths like `C:\Users\...`. Converting to forward slashes with to_slash_lossy() makes the path bash-compatible. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
a2335ba3f5 |
fix: address 5 bugs in template expansion, caching, and concurrency (#847)
- worktree_path_of_branch now respects shell_escape flag (expansion.rs) - Windows CI cache rename: remove target before rename to avoid failure - CI cache stores original branch name for proper display in state show - Symlink path comparison: canonicalize paths before comparing (macOS) - Zero semaphore: treat WORKTRUNK_MAX_CONCURRENT_COMMANDS=0 as "no limit" Co-authored-by: Claude <noreply@anthropic.com> |