74 Commits

Author	SHA1	Message	Date
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_.	2026-07-30 00:01:32 -07:00
Maximilian Roos	a285ed308d	refactor(tests): bring the mock-stub env vars under the WORKTRUNK_TEST_ prefix (#3621) ... `MOCK_CONFIG_DIR` and `MOCK_CALL_LOG_DIR` were the only two worktrunk-invented environment variables without the `WORKTRUNK_` prefix. That is not just a naming inconsistency: `isolate_subprocess_env` scrubs the parent environment by prefix — `GIT_*` and `WORKTRUNK_*` — so an unprefixed name is the one thing a test child inherits from whoever ran the suite. Renaming them brings them under that scrub. - `MOCK_CONFIG_DIR` → `WORKTRUNK_TEST_MOCK_CONFIG_DIR` - `MOCK_CALL_LOG_DIR` → `WORKTRUNK_TEST_MOCK_CALL_LOG_DIR` `TEST` rather than a bare `WORKTRUNK_` because both are read only by `tests/helpers/mock-stub` — they are the protocol between the harness and its helper binary, and `wt` itself never reads either one. That matches the ~15 existing `WORKTRUNK_TEST_*` knobs. ## The snapshot half Mechanical but not a substitution: the `env:` block is byte-sorted by key, so the renamed entry moves position within it and a `sed` in place would leave it where the old name sorted. All 971 affected blocks were rewritten by dropping the old line, inserting the new one, and re-sorting — the aggregate diff is exactly one removed and one added line per file: ``` 971 files changed, 971 insertions(+), 971 deletions(-) - MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" ``` The redaction placeholder follows its neighbours' convention in `add_standard_env_redactions` (`WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR` → `[TEST_NU_VENDOR_AUTOLOAD]`), so it now reads `[TEST_MOCK_CONFIG]` rather than repeating the full key. `MOCK_CALL_LOG_DIR` appears in no snapshot — its two call sites are `.output()` assertion tests — so it needs no redaction. ## Keeping it from regressing `tests/CLAUDE.md` gains the rule under "Where a new environment variable goes": name it `WORKTRUNK_TEST_*`, and the rule covers the harness↔helper-binary protocol, not just knobs `wt` itself reads. Without it the next helper-binary variable gets named `MOCK_*` again and the hermeticity hole reopens. ## Verification `cargo run -- hook pre-merge --yes` passes (exit 0) on the merged tree: 4607 tests including `--features shell-integration-tests`, `pre-commit run --all-files`, clippy, doctests. No pending snapshots. A repo-wide sweep finds no remaining unprefixed spelling. ## Merged main #3620 landed while this was in flight and regenerated several `for_each` snapshots that still carried the old key, so main is merged in here. It resolved with no conflicts, and the result is what you'd want rather than what git happened to produce: those blocks now carry #3620's new keys (`GIT_ALLOW_PROTOCOL`, `CLAUDE_CONFIG_DIR`, `WORKTRUNK_TEST_PARENT_SHELL`) *and* the renamed key, each in sorted position. The sweep and the gate above both ran after the merge. #3620 also rewrote the "Where a new environment variable goes" section this branch adds to — three layers became four. Both edits survived; the new naming paragraph follows the updated layer list. The two advisory `affected tests` checks are red for the same reason, and merging clears them: `cargo affected` errors on `git diff stdout was not valid UTF-8`, because the diff from this PR's base contains 16 binary files — the `tests/fixtures/standard/` git objects and index files that #3620 deleted. This branch's own commit contributes none. A PR based after #3620 won't see them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-07-26 21:44:57 -07:00
Maximilian Roos	951dac773f	fix(tests): give the picker's directive files a temp directory of their own, and make a PTY failure explain itself (#3619) ... Chasing the Windows-only failure of `test_switch_picker_runs_execute_command` that @max-sixty flagged after #3607 merged: it failed the advisory `affected tests (windows)` leg while the required `test (windows)` leg **passed the same test on the same SHA** in the same run. ## The failure message was the whole problem ``` assertion `left == right` failed: Expected exit code 0 for picker switch with --execute left: 1 right: 0 ``` That is everything CI had. wt prints an error before exiting non-zero, so there was a message — it just never reached the captured frame, because of two harness gaps: - **The post-exit drain was a single non-blocking sweep**, taken the instant `try_wait` reported the child reaped. The child's last writes can still be in the PTY at that moment, so the tail — wt's `✗` line — was dropped. This is also why #3427 concluded a picker-create "self-exited 1 after fully-correct work with no error on screen": the error was there, just not drained. - **`child.kill()` ran unconditionally after the exit poll.** On Windows that is `TerminateProcess(proc, 1)`, so a hung picker and a picker that failed on its own both report exit 1 — the ambiguity that made #3427 reason from wall-clock time to rule out a kill. Both are fixed here: drain until the output goes quiet, and panic on a hang rather than killing silently, so every exit code a caller asserts on is one the child chose. The four directive tests now also dump the screen (and the EXEC file) in their exit-code assertions. ## What reproducing it turned up Loop-based reproduction of the reported symptom is hopeless by the numbers: across the last 38 PR runs, exactly one advisory job failed — this one — i.e. ~1 failure per 160,000 test executions. What *did* reproduce was a different Windows failure in the same five tests, at a rate 1000× higher, once the loop ran with realistic neighbours (the full suite running alongside, as the failing leg had at test 3111/4204): ``` thread '…switch_picker::test_switch_picker_emits_cd_directive_by_default' panicked at tests\integration_tests\switch_picker.rs:2531:45: failed to create cd temp file: … PathError { path: "D:\\tmp\\.tmp3XI9wb", err: Os { code: 5, kind: PermissionDenied, message: "Access is denied." } } ``` **4 of 80 runs of the five directive tests.** `directive_files()` called `NamedTempFile::new()` in the shared temp directory; `tempfile` retries a name collision only when it surfaces as `AlreadyExists`, and on Windows `create_new` against a name held by a *directory* — or by a file in delete-pending state — returns `PermissionDenied`, which it hands back to the caller. A full suite run fills that directory with `.tmpXXXXXX` entries (every `TestRepo` makes one), so the collision is routine rather than astronomical. The helpers now take a `TempDir` and put fixed-named files inside it. `TempDir::new` isn't exposed to the same problem — a directory collision surfaces as `AlreadyExists`, which tempfile retries — and fixed names inside a private directory can't collide with anything. That covers all 75 call sites of `directive_files()` / `legacy_directive_file()`, not just the picker's five, and the switch_picker copy of the helper is gone. ## The reported exit-1 is not proven fixed Stated plainly: the panic above is a *different symptom* from the reported exit 1, so this PR does not prove that failure is gone. What the evidence does say, and what a reader should check first if it recurs: - The failing run took 3.34s where the same test passing on the required leg took 3.11s and the same switch *without* `--execute` takes 2.42s. The `--execute` tail — `build_hook_context`'s git lookups, ~0.7s of the difference — had therefore run, so the failure was at the end of the pipeline, not in the picker or the switch. - After that tail, the only fallible step left is the EXEC directive append. Every other step is infallible or `.ok()`-swallowed. - That write's error was context-free, which is why nobody could place it. `wt` rendered `✗ No such file or directory (os error 2)` — no file, no operation, and positioned right under `◎ Executing (--execute):`, where it reads as if the *command* were missing. It now reads: ``` ✗ Failed to write the command to the directive file /tmp/tmp.aBc No such file or directory (os error 2) ``` The message lives in `append_line` / `write_cd_path` rather than at their four call sites, so every directive write says which file and what didn't get written — including the legacy-protocol arms, which no test can drive to failure. So the next occurrence prints its own diagnosis, in one look, instead of `left: 1, right: 0`. ## Also here **The four directive tests select their row by cursor, not by a typed query.** They typed `target` and waited for `target-branch`'s preview pane before Enter. That gate can only assert what skim *painted*: the filtered item list is swapped into `ItemList` during a render, while `Accept` reads `item_list.selected()` — the cursor's slot in whatever list is current at that moment. The list-pane `>` pointer comes from the same render state the accept reads, which is why this file's own `wait_for_cursor_on_row` calls it the race-free signal. One Down reaches `target-branch` (the picker sorts the current worktree first) and the arrow is re-issued until the pointer settles, so no query — and no matcher — is in the accept path at all. It also drops the wait on the preview pane rendering. ## Testing - Reproducer and confirmation both on Windows CI, via a temporary workflow (deleted in the last commit): 80 runs of the five directive tests with the full suite running alongside. **4 failures before the fix, 0 after.** The concurrent full suite passed 4208/4208 in both. - Local: `switch_picker` (68), plus every suite that calls these helpers — `directives`, `switch`, `remove`, `security`, `bare_repository`, `step_alias`, `user_hooks`, `step_relocate` (861 tests) — green, plus clippy `-Dwarnings` on all targets with `--features shell-integration-tests`. - The two directive-write messages are pinned by tests of their own (`test_exec_directive_write_failure_names_the_file`, `test_cd_directive_write_failure_names_the_file`): an unwritable directive path fails the same way on every platform, so the paths that produce these messages are covered rather than checked by hand. ## Follow-ups, not in here - `build_hook_context` is called with `referenced: None` on the `--execute` path, so a `-x 'echo hi'` with no template variables still runs the `default_branch`, `primary_worktree`, `commit`, `short_commit`, `remote`, and `upstream` lookups — roughly doubling the command's subprocess count (30ms → 60ms locally, 2.42s → 3.11s in the Windows test timings). The alias path already passes its referenced set; a `--execute` command receives no JSON context, so the reason hooks pass `None` doesn't apply to it. - `cancel_background_commands()` is a no-op on Windows (`signal_background_pid`), so the picker's "the picker's exit cancels the unfinished request" promise isn't kept there: preview git children keep reading `.git/config` while the accept path writes `worktrunk.history` and `worktrunk.default-branch` to it. Issue #2564 is the same shape of concurrent-config-access failure on Windows. - `assert_valid_create_exit_code`'s Windows tolerance (#3427) and `assert_valid_abort_exit_code`'s acceptance of `1` both rest on the kill-vs-self-exit ambiguity this PR removes; they can be tightened once a Windows run confirms nothing else produces a stray 1. > _This was written by Claude Code on behalf of Maximilian Roos_	2026-07-26 15:41:35 -07:00
Maximilian Roos	9032308400	refactor(tests): give the PTY test environment one home (#3618) ... Follow-up to #3616, which fixed a PTY snapshot flake by adding one env knob — and to add it I had to touch three separate env builders, none of which knew about the others. This consolidates that surface. ## What was there The environment a test subprocess runs in was assembled at nine sites: five copies of the PTY prologue (`env_clear`, HOME, PATH, the Windows block, coverage passthrough) and four partial restatements of the determinism knobs. There was no rule for which belonged where, so adding a knob meant finding every copy, and a missed copy surfaced later as a flake somewhere unrelated. ## What's there now Three named layers, each with one home in `src/testing/mod.rs`: | Layer | Home | Contents | |---|---|---| | Baseline | `STATIC_TEST_ENV_VARS` | knobs every child needs, whatever it's attached to | | Terminal | `PTY_TEST_ENV_VARS` (new) | knobs only a TTY triggers — `WORKTRUNK_TEST_SPINNERS=0` | | Fixture | `pty_env_vars(TestEnvPaths { … })` (new) | the paths that vary per fixture | `configure_cli_command` and `configure_pty_command` apply them by transport. That turns the standing `// NOTE: TERM is intentionally NOT in STATIC_TEST_ENV_VARS` comment into a consequence rather than an exception: `TERM` is transport-level, so it can't sit in a baseline both transports share. `WORKTRUNK_TEST_SPINNERS` stays out of the shared baseline deliberately. It's inert on a pipe, and insta-cmd records the whole environment into every snapshot it writes (`Info::from_std_command` builds it unconditionally from `cmd.get_envs()` — there's no hook to suppress it), so putting it there would add a no-op line to 1043 snapshot files. `configure_pty_command` is now the only place a PTY child's isolation is set up. `shell_command`, `execute_shell_script`, `configure_pty_environment`, `exec_in_pty_shell`, `exec_bash_truly_interactive` and two `wt switch` spawns all delegate to it. `shell_wrapper`'s `STANDARD_TEST_ENV` and `bare_repository`'s hand-rolled `test_env_vars` are gone, as are `configure_shell`'s hand-copied knobs and four redundant `CLICOLOR_FORCE` lines in `switch_picker`. Net −149 lines. One spawn stays outside: the Windows ConPTY smoke test, which runs PowerShell against a deliberately bare environment and isn't a wt child at all. ## Reviewing Start at `src/testing/mod.rs` — the three layers and their doc comments are the whole design. Everything under `tests/` is deletion plus a delegation call. Two snapshots change: `install_preview_with_gutter` and `install_preview_declined` now carry ANSI, because those two tests previously ran without `CLICOLOR_FORCE`. Text is identical. Arguably a fix — the test named "with_gutter" couldn't see the gutter (a background-color block), while its own prompt line was already colored, so the file was internally inconsistent. ## Testing Full `wt hook pre-merge --yes` green: 4601 tests, `--features shell-integration-tests`, `RUSTFLAGS='-D warnings'`, `insta --check`. The knob's delivery path was verified by probe rather than by inspection. With `sleep 6` in the mock `llm`, `test_readme_example_hooks_pre_merge` passes; flipping `PTY_TEST_ENV_VARS` to `"1"` reproduces the original failure byte for byte: ``` +␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (4s)␛[22m␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (5s)␛[22m ``` So the knob reaches the shell-wrapper PTY child through the shared setup, not through a surviving copy. Both probe edits are reverted. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 14:59:48 -07:00
Worktrunk Bot	203603909d	test(picker): release --prs loading marker on parent exit, not a timer (#3532) ... ## Problem `test_switch_picker_prs_shows_loading_marker` intermittently times out on `test (windows)` — it flaked on #3531 (a docs-only change), which is what prompted this fix. The test asserts a *transient* frame: while a mocked, delayed `gh pr list` is in flight, the picker header shows `↳ Loading open PRs…` and the `#42` row has not yet streamed in. The mock held that loading state for a fixed time (originally 3s). On a loaded Windows runner, the PTY boot sequence — `boot_picker_pty`'s skim-ready wait plus its initial `wait_for_stable`, together with the async worktree-column churn (`·` placeholders resolving) — can consume that whole window. By the time the stabilization helper first polls for `Loading open PRs`, the mocked fetch has already returned and the rows have streamed in, so the marker is gone and the wait times out at `STABILIZE_TIMEOUT` (30s). ## Fix Per review feedback ([comment](https://github.com/max-sixty/worktrunk/pull/3532#issuecomment-5039146240)), tie the marker's lifetime to the picker's, with **no timing number at all** — the file's "poll, don't pick a time" rule applied to the producer side. `mock-stub` gains a `hold_until_parent_exit` response mode: the mocked `gh pr list` holds until `wt` exits instead of sleeping a fixed `delay_ms`. Detection needs no platform primitive (`getppid` / `OpenProcess`): - While `wt` lives, its `--prs` fetch thread runs the mock with a piped stdout and blocks reading that pipe to EOF, so as long as the mock neither writes a full response nor exits, the loading marker stays. - When the picker aborts, `wt` detaches the fetch thread (`drop(prs_handle)`) and exits, which closes the read end of the mock's stdout pipe. The mock's next write then fails with `BrokenPipe` (Rust ignores `SIGPIPE`, so the broken write surfaces as an `Err` rather than killing the process), and the mock exits. Polling for that write error is a **causal** parent-death signal — release is tied to `wt` exiting, not to a timer — and it behaves identically on Unix and Windows, so there's no platform-specific FFI to get right. The one-byte probes written into stdout are never observed (this mode's contract is that the parent aborts without parsing the response, and the fetch thread drains the pipe until process exit). A 60s poll cap bounds a stuck orphan if the pipe somehow never breaks. This removes the race the earlier 30s bump only widened: the marker is present for the entire window the poll could observe it, on any boot latency, and no orphaned sleeper is left behind — the mock exits the moment the picker does. ## Testing `cargo test --test integration --features shell-integration-tests test_switch_picker_prs_shows_loading_marker` passes in ~1.3s, confirming the capture-and-abort path stays fast and the mock releases promptly on `wt` exit. The sibling `--prs` tests (`test_switch_picker_prs_{github,gitlab}_list`, `..._rows_survive_alt_x_removal`) still pass through the refactored `mock_forge_env` helper. Reasoning is documented inline at the mock-stub call site and in `wait_for_parent_exit`. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-25 13:42:06 -07:00
Maximilian Roos	a0c1b662d2	perf(fsmonitor): resolve all daemons in one lsof spawn (#3581) ... ## Why `wt remove`'s end-of-command fsmonitor sweep forked one `lsof` per *machine-wide* `git fsmonitor--daemon` — and with `core.fsmonitor` enabled globally, a machine accumulates one daemon per repo ever touched (100+ is routine). So every `wt remove` paid ~100 process spawns. The cost is superlinear under load, not linear: a fork storm makes macOS Gatekeeper/XProtect assess each new image under contention, inflating per-spawn cost for everything else on the box, sweeps included. The `~50ms each` figure in the old docstring was measured *under that load* and read as a fixed per-call cost — so the sweep looked merely slow rather than self-amplifying. Two concurrent test suites driving hundreds of `wt remove` calls pinned all 18 cores (load average 142) with nothing hot in `htop`. This was diagnosed and measured before (#2814 added the sweep; #3401 wrote the cost into the docstring and explicitly changed no behavior) but never fixed. ## The fix Resolve every daemon in a single `lsof -a -p <pid1,pid2,…> -U -F pn` call and split its output on the `p<pid>` record boundary, handing each PID exactly the output the per-PID call produced — so classification and the whole data-safety contract in the module docs are unchanged. Verified end-to-end against a live 108-daemon machine with a PATH shim counting invocations: **108 spawns → 1**. Two details the batched path must preserve, each covered by a new test: - **Don't gate on `lsof`'s exit status.** It exits non-zero when *any* requested PID vanished mid-scan, while still printing full records for every survivor. Gating would drop the whole sweep whenever one daemon happened to exit — the common case on a busy machine. - **A PID with no record is dropped**, never synthesized as a socket-less entry. A socket-less entry reads as orphan class 1 and gets SIGTERM/SIGKILL — on a possibly-recycled PID. ## Also here - **Test consolidation.** Three integration fixtures built their branch set by spawning git per loop iteration. A new `TestRepo::create_branches` creates them in one `git update-ref --stdin`. The over-threshold completion test drops **10.5s → 1.4s** (~230 git spawns → 5); `switch_picker` and `statusline` branch loops get the same treatment. `mock-stub` gains an opt-in `MOCK_CALL_LOG_DIR` call log so a test can assert *spawn count*, not just response — deliberately outside the repo under test, since a log written inside the tree would dirty it mid-`wt merge`. - **Docs.** Dropped the "disable `core.fsmonitor` globally" workaround from the troubleshooting docs (canonical `skills/`, mirror regenerated). It stays enabled by choice, and the daemons were never the actual fseventsd CPU driver. - **CI hardening.** pre-commit.ci's `typos` hook maps the token `pn`→`on` and had auto-rewritten `lsof -F pn` to the broken `lsof -F on` (offset+name — no `p<pid>` records, so the parser reaps nothing). Pinned `pn` in `.typos.toml` next to the existing `PN`-from-`PNGs` entry that documents the same mapping, and added a test covering the empty-PID-set early return (flagged by `codecov/patch`). ## Testing New unit tests cover `daemons_from_batched_lsof` against real captured `lsof -F pn` output, a vanished-PID gap, and empty output; the rewritten integration test asserts exactly one `lsof` spawn with a comma-joined PID list. Full integration suite green (1960 passed), clippy and fmt clean. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-07-24 17:27:36 -07:00
Worktrunk Bot	9cb3693fdf	test(switch_picker): tolerate Windows self-exit-1 on successful picker-create (#3427)	2026-07-12 08:18:01 -07:00
Worktrunk Bot	bd504f0998	feat(switch): run --execute against the picked worktree (#3394)	2026-07-10 04:30:14 -07:00
Maximilian Roos	7f6f34d0cb	feat(picker): surface alt-x decline reasons in the header (#3336) ... When `alt-x` in the `wt switch` picker declines to remove a row, the row visibly stays put but the reason only drained to stderr *after* the picker exited — the user pressed the key, nothing seemed to happen, and the "why" scrolled past on quit. This adds an in-picker echo of that reason. A new generation-guarded `HeaderFlash` shows a one-line message in the picker header (skim reserves exactly one non-selectable chrome line — the header — and has no footer slot) for a beat, then self-clears back to the column labels. It's wired into all three "can't remove" surfaces, each styled to match the canonical representation that path already stashes for the on-exit drain: - current worktree → `○ Can't remove the current worktree` (info — a by-design decline) - unmerged branch-only row → `○ Kept <branch> — branch is unmerged` (info — a by-design retain) - main worktree / dirty / locked → `✗ The main worktree cannot be removed` (error — a genuine rejection) The full diagnostic still drains to stderr on exit; the flash is additive. This closes the `TODO(picker-feedback)` the keep paths carried. The same change also aligns the `--prs` loading marker with the watchdog and the picker's other in-flight placeholders (`↳ <dim>Fetching…</>`): ` loading open PRs…` → `↳ Loading open PRs…`. The hint symbol sits in the pointer gutter and "Loading" stays aligned at column 2 (`↳ ` is the same two columns the old indent used). ## Implementation notes `HeaderFlash` is shared picker-lifetime between the header item (which renders it) and `AltXRemover` (which sets it). `flash_header` sets the message, repaints, and spawns a short-lived timer that clears it after `HEADER_FLASH_DURATION` and repaints again. The clear is generation-guarded — a newer flash arriving mid-beat replaces the old one and isn't wiped by the old timer — and `clear_if_current` reads the generation under the message lock so the check and clear are atomic against a concurrent `set`. ## Testing Unit tests cover the `HeaderFlash` mechanism (set / generation-guarded clear / display priority of flash › loading › labels), the keep-path symbol, and the timer driven to completion (so the detached thread's clear+repaint is covered under nextest's process-per-test). A PTY integration test confirms the flash actually paints on `alt-x` of an unremovable row. The loading-marker assertions were updated in lockstep. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-30 20:56:08 -07:00
Maximilian Roos	603efc9447	test(switch_picker): fix Windows prompt-echo-lag flake; forbid retries (#3331) ... Removes a Windows-only false-positive on the advisory `affected tests (windows)` leg so it can eventually be promoted to a required gate. `test_switch_picker_alt_x_keeps_unmerged_branch_row` flaked on the advisory Windows leg while the required `test (windows)` leg passed on the same SHA. From the captured CI frame, the panic was the content assertion, not the exit code: the old assertion counted the branch name across skim's query-echo prompt line plus the data row (`>= 2`), and under Windows CI load the prompt echo trails its keystrokes — the final character can still be unrendered once the row is already on screen — collapsing the count to 1. The fix gates on the gutter-prefixed cursor row. A local branch with no worktree renders the `/` gutter (`BranchScope::Local`), so `/ unmerged-orphan` names the data row and excludes skim's gutterless prompt, and it renders from collect data so it's immune to echo lag. This is the same robust idiom the sibling alt-x tests already use. Also adds a "No Retries" section to `tests/CLAUDE.md`: worktrunk sets no nextest `retries` and writes no retry loops, with the two root-fix paths (racy assertion → make it deterministic; resource pressure → bound concurrency). This does not touch the separate, lower-rate `STATUS_DLL_INIT_FAILED` (`-1073741502`) flake, which is Windows process-spawn resource exhaustion under concurrent load rather than a test-logic bug. The "No Retries" section documents the deterministic lever (bounding Windows test concurrency) for whenever the advisory leg is promoted. The picker fix targets Windows-specific timing and passes on macOS, but the flake itself can only be confirmed gone on the Windows CI legs. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-29 14:13:14 -07:00
Maximilian Roos	b1bf48a965	fix(picker): keep the empty-query list in collect order (#3301) ... The `wt switch` picker's default view (no query typed) reordered rows by where each name's last `/` falls, instead of showing them in collect order. Any slash-bearing row — a `feature/…` PR head branch, a `perf/…` worktree branch, or a `/`-gutter local branch — sank toward the bottom and intermixed with rows of other kinds. A worktree like `perf/list-prune-…` would appear stranded among the `#` PR rows rather than up with the other worktrees. **Cause.** The picker set a custom skim tiebreak `[Score, PathName, Begin, End]` (plus `last_match(true)`). On an empty query skim's `MatchAllEngine` scores every row `(score=0, begin=0)`, so with all scores tied the whole list is ordered by the second criterion. `PathName`'s key is `path_name_offset - begin`, which at `begin=0` collapses to `path_name_offset` — the byte offset after the last `/`. Names without a slash get `0` (kept in input order); names with one get a positive offset and sink. `PathName` was added to rank typed-query matches on their leaf segment (`feature/auth` on `auth`), but skim is a plain crates.io dependency with no engine-injection hook, so it can't be made inert on an empty query without forking skim — which this project deliberately avoids. **Fix.** Revert to skim's default `[Score, Begin, End]` and drop `last_match(true)`. On an empty query every row ties on `[0,0,0]`, so skim's stable sort preserves input order, which is collect's order: current, main, newest-first worktrees, then branches, then the appended `--prs` rows. The cost is confined to typed queries — leaf-segment matches no longer win a *score tie* — but the shared `~/workspace/` prefix is already stripped from each row's match text in `progressive_handler::on_skeleton`, and the frizbee matcher penalizes non-boundary matches, so `feature/auth` still ranks well on `auth`. A regression test creates a `feature/auth` worktree before a `plain-branch` one and asserts `feature/auth` stays ahead on the empty-query view; it was confirmed to fail on the old tiebreak and pass on the new one. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-28 17:29:23 -07:00
Maximilian Roos	14c79c161b	fix(picker): show the resolved config path in disabled-summary hints (#3290) ... When LLM summaries are disabled, the `wt switch` picker's Summary tab is seeded with one of two hints — "Summaries not configured" when there's no `[commit.generation]` command, or "Summaries off" when `[list] summary = false`. Both previously told the user to edit a hardcoded `~/.config/worktrunk/config.toml`, which is wrong whenever wt loads config from elsewhere: a `--config` flag, the `WORKTRUNK_CONFIG_PATH` env var, or a non-default `$XDG_CONFIG_HOME`. A user following the hint would edit a file wt never reads. Both hints now show the path wt actually loads from, resolved once at picker startup via `worktrunk::config::config_path()` (priority: `--config` → `WORKTRUNK_CONFIG_PATH` → platform default) and rendered through `format_path_for_display` so it shows `~/…`. When no location can be determined, it falls back to the canonical literal. This mirrors the resolved-path idiom already in `src/commands/worktree/resolve.rs`. The hints are also reshaped into the gutter house style — a bold H4-subject first line and a fenced ` ```toml ` config block — structured so the PTY snapshot is platform-stable. `render_summary` word-wraps prose to the preview width, which is one column narrower under Windows' PTY, so any prose body long enough to wrap breaks on a different word there and a single cross-platform snapshot can't match. To avoid that, the only prose is a short lead line that can't wrap, the resolved path sits alone on its own line (one unbreakable token, so there's no wrap boundary to shift), and the config block is never wrapped (code blocks aren't). The H4 subject is never wrapped either. ## Tests `test_switch_picker_preview_panel_summary_disabled` is added to cover the "configured but `summary = false`" arm, which had no test before (the existing summary test only exercises the not-configured arm) — closing a `codecov/patch` gap. Both arms' PTY snapshots render the resolved path; under the tests' `WORKTRUNK_CONFIG_PATH` isolation that path is a per-run temp file, which the existing snapshot filter redacts to a stable `[TEST_CONFIG]` placeholder, so the snapshots stay host-independent and `test_no_host_specific_paths_in_snapshots` is satisfied. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-28 13:21:33 -07:00
Maximilian Roos	c1994055c5	feat(picker): refresh previews on alt-r, not just the row list (#3293) ... ## What `alt-r` in the `wt switch` picker re-ran `collect` (refreshing the rows, CI, and worktree inventory) but the **preview pane kept serving stale content**. The in-memory preview cache is keyed by `(branch, mode)` with no SHA, and the same cache was deliberately shared "warm" across reloads — so a refresh never recomputed the working-tree / log / branch-diff / upstream / summary tabs. Edit a tracked file, hit `alt-r`, and the diff pane still showed the pre-edit state. This clears the in-memory preview cache on the refresh spawn (`PipelineFactory::spawn`, gated on `rebuild_repo` — true only on `alt-r`), so each rebuilt row recomputes against its current `item.head()`. The SHA- and diff-hash-keyed on-disk caches make an unchanged branch a cheap re-read; only genuinely changed content pays a recompute. `pr` / `comments` are cleared too, so a refresh also re-fetches their forge data. ## Also: a unifying spec There was no single write-up of the picker's preview-caching system — the knowledge was scattered across four module docstrings. This adds a module-level spec at the top of `preview_orchestrator.rs` (the hub that owns the in-memory cache, the `fill` choke point, and the precompute tiers): the two tiers, what backs each mode on a miss, the invalidation rules, and exactly what `alt-r` does and doesn't refresh. Back-pointers added from `preview_cache.rs` and `items.rs`. ## Known limitations (documented, not fixed here) Both trace to one root — the orchestrator is built once and shares the **startup** repo, with no spawn generation: 1. **Stale BranchDiff base** — if the *default* branch moves externally mid-session, BranchDiff recomputes the row's fresh head against a stale base SHA. Pre-existing and orthogonal to `alt-r`; not worsened here. 2. **Narrow stale-fill race** — a prior spawn's still-draining precompute task can fill the just-cleared cache with stale content that the new task then defers to. Opens only on a large repo / slow summaries when content moved in the drain window; the common "I edited the branch I'm viewing" case doesn't hit it (that branch's precompute finished at picker open); self-heals on the next refresh. The structural fix for both is to give the orchestrator the current spawn's repo plus a generation counter (mirroring `prs_epoch`); left as a follow-up since it's a refactor of a concurrency-critical module. ## Testing - Unit test: cache cleared on a refresh spawn, kept warm on the initial spawn. - End-to-end PTY test: open the picker on a clean tree, edit a tracked file, `alt-r`, assert the diff appears (and the stale "no uncommitted changes" pane is gone). Both were confirmed to fail with the one-line clear neutralized. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-28 12:26:27 -07:00
Maximilian Roos	0862c8e7c6	test(picker): restore auto-refresh test with cursor-nav selection (#3277) ... Re-enables `test_switch_picker_preview_auto_refreshes_when_compute_lands` (in `tests/integration_tests/switch_picker.rs`), which was `#[ignore]`d in #3264 for flaking under CI contention. ## Root cause The test drives a `wt switch --prs` picker with two rows — the `main` worktree row and a not-checked-out PR row (`flaky`) — and must select the PR row before opening its `comments` tab to assert the preview auto-refreshes (a comment surfaces with no keystroke once a delayed `gh pr view` lands). It selected the PR row by typing skim's inverse filter `!main`. That was the flake: typing a query both engaged skim's `PathName` tiebreak (which reads the random temp path) and emptied the result set during the window before the async `--prs` row had streamed in — so the cursor was lost and skim didn't reselect the lone row when it arrived, timing the gate out. ## Fix Select the PR row by driving the cursor with a re-issued Down, not a filter. Typing no query keeps the list insertion-stable (worktree rows render at skeleton; `--prs` rows append below), so a Down that clamps at the bottom row lands on the PR row deterministically — the same primitive the non-flaky `test_switch_picker_worktree_row_comments_tab_shows_thread` uses. This removes the query-induced non-determinism at the source rather than working around it. Also tightens the shared `cursor_points_at` helper to match only the list pane (cols `0..LIST_WIDTH`): it was scanning each full-screen row (list + preview concatenated), so a token that also renders in the preview could satisfy the pointer check from the wrong row. ## Validation Probed the row order 202 times (sequential, 32-way parallel, and under full CPU saturation): the worktree row sorted first every time, the PR row last. The redesigned test then passed 149+ stress runs including under CPU saturation, with the full `switch_picker` module green (0 ignored) and the full pre-merge gate passing (4257 tests). Local macOS can't reproduce the original contention failure (the CI failures were on Windows and the instrumented coverage lane), so the deterministic acceptance check is this PR's CI — the `code-coverage` lane in particular. Closes #3269 > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-27 18:55:14 -07:00
Maximilian Roos	41db03754b	fix(picker): keep --prs rows visible after an alt-x removal (#3275) ... ## The bug In the `wt switch --prs` interactive picker, removing a worktree row with `alt-x` made the streamed PR/MR rows vanish from the list until the user pressed `alt-r` to refresh. The worktree/branch rows survived; only the `--prs` rows disappeared. ## Root cause The `alt-x` removal rework (`d66bc6af5`) replaced the old `reload(remove {})` with a synchronous `resync_pool` that rebuilds skim's item pool from the picker's `shared_items` Vec. But `shared_items` only ever held the skeleton (worktree/branch) rows: `on_skeleton` populates it, while the `--prs` thread streams its rows straight to skim's item channel (`prs::fetch_and_stream` → `tx.send`) and never recorded them in `shared_items`. So when `resync_pool` rebuilt the pool from `shared_items`, the PR rows were dropped. They only reappeared when `alt-r` re-ran the whole collect + `--prs` pipeline. (The old `reload` path had the same blind spot; it matters more now that `alt-x` is the sole removal path.) ## The fix The `--prs` thread now appends its PR/MR rows into `shared_items` as well as streaming them to skim — the same way it already extends `shortcut_table`. With `shared_items` holding the full row set (header + worktree/branch + PR/MR rows), `resync_pool` preserves the PR rows on an `alt-x` removal for free. The append is guarded by a per-spawn epoch counter (`PipelineFactory::prs_epoch`, handed to each spawn's `--prs` thread via `PrsShared`). An `alt-r` refresh spawns a fresh `--prs` thread while the prior spawn's forge call may still be in flight; without the guard, that stale call (whose skim channel is already dropped) would re-add now-duplicate rows to the list a newer spawn rebuilt. The epoch is read under the `shared_items` lock so the check pairs with the next spawn's `on_skeleton` overwrite, which holds the same lock. The append is ordered before `tx.send` so the rows reach `shared_items` no later than they reach skim's pool (favoring a sub-microsecond transient-duplicate window over re-dropping rows, were the order reversed). The no-flash cursor behavior from `d66bc6af5` is unchanged: the rebuilt list is just longer, so the cursor holds its index and the row that slides into the removed slot lands under it. ## Testing New PTY regression test `test_switch_picker_prs_rows_survive_alt_x_removal` drives the drop path (a clean, integrated worktree) in `--prs` mode and asserts the `#42` PR row survives the removal. The mock answers `gh pr list --state` (the `--prs` fetch) with PR #42 but `gh pr list --head <branch>` (the per-worktree CI fetch) with an empty list, so `#42` appears only as a `--prs` row, never folded into a worktree row's CI cell. Confirmed: the test fails without the fix and passes with it. Full pre-merge gate green (4257 tests). > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-27 18:41:48 -07:00
Maximilian Roos	57d0458592	fix(switch): eliminate the alt-x picker cursor flash (#3268) ... ## What Removing a row in the `wt switch` picker with `alt-x` flashed the `>` pointer to the top of the list for a frame before it settled on the right row. This eliminates that flash. ## Root cause The flash was structural to skim's `reload`, not a lagging reposition. `alt-x` was bound to `reload(remove {})`, and skim's `handle_reload` clears the item pool and restarts the matcher *before* the new rows stream in — so the matcher runs once against the empty pool, `Replace`s `item_list` with nothing, and skim's render clamp (`items.is_empty() → current = 0`) resets the cursor to the top. A `reposition` Custom action then snapped it back, but the reset frame was already painted. (`no_clear_if_empty` doesn't help: its protective "keep stale rows" branch is gated on skim's interactive mode, which the picker doesn't use — verified with a steady-state A/B.) ## The fix Replace the `reload`-based removal with a **synchronous in-place pool resync**. `alt-x` is now a single Custom keybinding callback that runs the removal dispatch (`AltXRemover::apply`) and rebuilds skim's pool from the mutated row list (`resync_pool`) on the same event-loop tick. The pool is filled before the matcher runs, so the matcher only ever sees the post-removal list — never empty — and `current` is preserved (clamped to the shrunk list). The row that slides into the removed slot lands under the cursor for free, with no flash, under a fuzzy filter too. This lets the whole reposition apparatus go — `reposition_cursor_action`, `send_reposition`, the `drop_landing` neighbour capture, the settle counters — for a net deletion. The removal dispatch moved off the collector into a `Send` `AltXRemover` (skim requires the keybinding callback to be `Send`, so it can't carry the collector's `Rc<PipelineFactory>`); `PickerCollector` now only serves the `alt-r` refresh. Keep/morph repaint in place; the rare failed-removal restore queues a `resync_pool_action`. ## Two follow-on fixes surfaced while validating **Last-row preview.** skim auto-repaints the preview across the matcher's `Replace` only when the selected row's text changes — which a last-row drop doesn't produce (`current` goes briefly out of range, then clamps onto the new last row with no text change). The pane kept showing the removed row's preview; `run_preview_when_settled` restores the missing repaint. (Found in review.) **`--prs` streaming order.** The alt-x rework tipped a pre-existing microsecond race: `on_skeleton` woke the `--prs` thread (`grid_slot.set`) *before* sending the skeleton batch, so with a fast forge call the PR rows could reach skim's channel first and a PR row would take the reserved header slot (`header_lines(1)`), displacing the header — the cursor then couldn't reach the PR row. That's what made `test_switch_picker_preview_auto_refreshes_when_compute_lands` a long-standing flake. Fixed deterministically: the `--prs` thread is now woken only after the skeleton is sent, so PR rows always append after it. ## Navigating the diff `src/commands/picker/mod.rs` (the alt-x rework, a net deletion), `progressive_handler.rs` (the one-statement `--prs` ordering fix), and `tests/integration_tests/switch_picker.rs` (new tests). ## Testing The eight existing alt-x cursor PTY tests pass unchanged; removal unit tests drive `AltXRemover::apply` directly. Added PTY tests for the last-row preview refresh and the failed-removal restore, and a unit test for the unmorphable-row drop fallback. Verified frame-by-frame under tmux (drop, morph, keep, filtered drop, last-row drop) that the pointer never jumps to the top row. The full pre-merge gate passes locally (4255 tests). > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-27 14:30:36 -07:00
Maximilian Roos	e3f9e849e8	test(picker): de-flake preview-auto-refresh by filtering to the PR row (#3264)	2026-06-27 01:32:40 -07:00
Maximilian Roos	8e2ab04c7d	feat(switch): rework the alt-x picker removal — morph in place, decline current worktree, identity cursor (#3262) ... Overhauls the `wt switch` picker's `alt-x` (remove) so a removal updates the selected row in place instead of re-collecting the whole list, declines removals that can't safely happen with an explanation, and lands the cursor on the right row afterward — including under an active filter. ## What changes for the user `alt-x` on a row now behaves by what the removal does: - **Unmerged worktree** → the row morphs in place to a `/ branch` row (worktree gone, branch kept); the worktree removal runs in the background. No flicker, cursor stays put. - **Merged worktree** → the row drops; the cursor lands on the row that slid up into its slot. - **Current worktree (`@`), main, dirty, locked** → kept, with the same diagnostic `wt remove` prints (drained to stderr on picker exit) instead of a silent dead keypress or a disruptive cd-home. Before, `alt-x` re-collected the entire list (a visible flicker, cursor reset to top), and removing the current worktree forced a `cd` elsewhere mid-render. ## The cursor fix (last commit) The post-removal reposition scrolled to the removed row's index in the full `shared_items` list. Under a fuzzy query, skim's `item_list` is filtered and reordered, so that index pointed at the wrong row — the cursor jumped +N rows down (N = filtered-out rows above it), compounding on a second removal. Reposition is now by **row identity**: it walks `item_list` to the target row's `output()` token. The drop's landing row (the display-neighbor) is captured before the reload via a native `alt-x` binding (`[capture, reload]`); keep/morph/restore land on their own row's token. ## Reviewing The bulk is `src/commands/picker/mod.rs`. Key pieces: - `invoke` dispatch (`removal_targets_current_worktree` → `worktree_removal_keeps_branch` → `removal_will_remove_target`) decides keep / morph / drop. - `morph_and_remove_in_background` does the in-place row morph (optimistic, reverted if the worktree unexpectedly survives). - `reposition_cursor_action` / `install_remove_keybinding` are the identity-based cursor landing. ## Testing Picker unit tests plus PTY integration tests through real skim, including the new `test_switch_picker_alt_x_lands_on_neighbor_under_filter` (removes a row with filtered-out decoys above it — confirmed to fail on the old index-based reposition and pass on identity). Multi-row cursor tests cover both drop and keep paths. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-26 22:33:22 -07:00
Maximilian Roos	4d644d04bc	refactor(picker): unify worktree and PR rows into one PickerRow (#3259) ... Collapse the picker's two `SkimItem` types into one. `WorktreeSkimItem` (a checked-out worktree row) and `PrSkimItem` (a listed `--prs` row) become a single `PickerRow` whose only branching axis is `local: Option<LocalCheckout>` — `Some` for a worktree row, `None` for a listed PR. This is the type-level completion of #3252, which made the two row kinds behave identically; they now share one `text()`, `display()`, `output()`, `preview()`, and PR-pane renderer instead of two parallel implementations. ## What changed - `PrSkimItem` and its standalone `impl SkimItem` are deleted. Listed `--prs` rows are built by `prs::listed_pr_row` (shared by `fetch_and_stream` and the row tests) as `PickerRow { local: None, … }` with a static `pr_status` slot pre-filled by the new `PrEntry::display_status()`. - The worktree-only fields (`has_upstream`, `summaries_enabled`, `local_content`) move into a `LocalCheckout` sub-struct behind `local`. The frozen `Arc<ListItem>` handle is gone, replaced by a precomputed `output_token`. - `pr_status` and the preview cache are shared by both row kinds, keyed by `PickerRow::preview_key()` — the branch for a worktree row, `pr:N`/`mr:N` for a listed PR. A `--prs` row's `pr` pane is memoized in that session-long cache, so `listed_pr_row` drops the prior `(pr:N, Pr)` entry on each build; an `alt-r` reload then re-renders the freshly fetched PR metadata instead of the pre-reload pane (the worktree-row analog of `on_update`'s invalidation). ## Behavior change A worktree row tracking a draft PR now shows a `state: draft` line in its `pr` pane. Previously only `--prs` rows surfaced draft state. This falls out of both kinds sharing `render_pr_pane_body`. ## Reviewer orientation - `src/commands/picker/items.rs` — the unified `PickerRow`/`LocalCheckout`, `preview_key()`, `render_pr_pane_body()`, `render_listed_pr_mode()`. - `src/commands/picker/prs.rs` — `PrSkimItem` removed; `PrEntry::display_status()` and the `listed_pr_row` constructor (with its cache invalidation). - `src/commands/picker/progressive_handler.rs` — worktree-row construction with `local: Some(LocalCheckout { … })`. - The remaining files are call-site and doc renames. Rendered output is unchanged — the integration snapshots still pass, and the `loading_placeholder` snapshots change only their `expression:` metadata line. Rebased on main's #3253 (the `↳` loading-placeholder glyph), which is preserved. > _This was written by Claude Code on behalf of max_	2026-06-26 11:18:38 -07:00
Worktrunk Bot	aedbd93010	test(picker): de-flake both Windows switch-picker PTY tests (#3242)	2026-06-26 07:26:48 -07:00
Worktrunk Bot	6047b76555	test(picker): de-flake switch_picker_with_branches by settling async columns (#3256)	2026-06-26 06:36:20 -07:00
Maximilian Roos	e3ca4c2b51	fix(switch): auto-refresh picker preview when a background compute lands (#3247) ... ## Problem The `wt switch` picker's preview pane is served from a `DashMap` cache filled by background workers on `COLLECT_POOL` (a `git diff HEAD`, a `git log`, a forge `gh pr view`). skim 4.8 re-reads that cache **only** inside `run_preview`, which fires only on `Event::RunPreview` — produced by a selection change or a preview-tab keystroke. The cache-insert path didn't poke skim, so a compute that finished *after* the single `RunPreview` the keystroke produced sat in the cache with no event to surface it: the pane stayed on its `Loading…` placeholder until the user pressed a key again. That `Press alt-N again to refresh` text was the manual workaround for exactly this gap, and it was a Windows-CI flake (PR #3238 papered over it test-side by re-issuing the tab keystroke). ## The skim mechanism this uses skim 4.8 hands the embedder its event sender at TUI init: `Skim::event_sender()` returns the `tokio::sync::mpsc::Sender<Event>` that drives the loop. The picker already captures it (as `render_tx`, a shared `Arc<OnceLock<…>>`) and pushes `Event::Render` through it for in-place row repaints. **Pushing `Event::RunPreview` through the same channel forces `run_preview` to re-read the cache for the currently-selected row + current mode** — the external injection point the orchestrator needed. The channel is `1024*1024`-capacity, so the `try_send` poke is never dropped. ## Approach New `PreviewNotifier` (`src/commands/picker/preview_notify.rs`) closes the producer → consumer loop: - **Consumer side:** every `*SkimItem::preview()` records the selected row's awaited `(row-key, mode)` via `note_awaiting` — *before* it reads the cache. That ordering makes the hand-off race-free: if the read misses, the fill that satisfies it necessarily lands after the read, so it observes the awaited key already set. - **Producer side:** the orchestrator routes **every** cache fill through a single `PreviewOrchestrator::fill` / `fill_external` path, which calls `notify_filled(key)`. That injects `Event::RunPreview` **iff** the filled key matches what the selected row is awaiting. A fill for an off-screen row or a tab the user isn't on matches nothing and injects nothing — so background pre-compute never thrashes the visible preview. `preview()` is only ever called for the selected row, so the single shared `awaiting` slot always reflects what's on screen; when the selection changes, the next `RunPreview` updates it. Two producers feed the panes, both now covered: - **Orchestrator cache fills** (diff / log / summary / the `--prs` comments & log fetch) → `notify_filled(key)`, exact-key match. - **The collect handler's `on_update`** mirrors a row's live `pr_status` (the `pr` / `comments` panes) and `local_content` (the diff tabs' dim state) — not cache fills → `notify_row_changed(row_key)`, which re-runs the selected row's preview on *any* tab when that row's data lands. This is what flips the `pr` tab from "Fetching PR status…" to the resolved PR on its own. Wiring: `render_tx` is constructed before the orchestrator in `handle_picker` and handed in; it's still published once, inside `run_skim` after `init_tui`. `generate_and_cache_summary` became `generate_summary_for_item` (returns the pane; the orchestrator inserts via `fill`). ## User-visible change The placeholders drop the now-obsolete "press alt-N … to refresh" wording — the pane fills in on its own: ``` ○ Loading working-tree diff… (was: ○ Loading working-tree diff. Press alt-1 again to refresh.) ○ Generating summary… ○ Fetching PR status for feature… (was: … press alt-6 to refresh) ⓘ Loading comments… (--prs deferred tabs; was: … press alt-2 again to refresh) ``` ## Testing - `test_switch_picker_preview_auto_refreshes_when_compute_lands` (PTY): mocks `gh pr view --json comments` behind a 3 s delay, opens a `--prs` row's comments tab mid-fetch — the comment surfaces **with no further input** (the orchestrator-fill path). - `test_switch_picker_pr_tab_auto_resolves_from_fetching` (PTY): the per-row CI fetch (`gh pr list --head`) is delayed 3 s and unseeded, so the `pr` tab opens on "Fetching PR status…" and resolves to the live PR on its own (the `on_update` path). - Both verified to **time out (stay stranded) when the poke is disabled**, so they genuinely exercise the mechanism rather than passing on a cache hit. - `fill_notifies_only_awaited_key` / `notify_row_changed_pokes_only_the_selected_row` (unit): the poke fires for the visible row+mode (resp. row, any mode) and nothing for off-screen / other-tab keys — the no-thrash guarantee. - The PTY driver's keystroke re-issue (`nudge` / `is_alt_digit_tab` / `PREVIEW_REISSUE_INTERVAL`, PR #3238) is removed — the product now auto-refreshes, so the tests genuinely verify it rather than papering over a strand. All 37 `switch_picker` PTY tests pass; full `cargo run -- hook pre-merge --yes` green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-25 12:24:23 -07:00
Maximilian Roos	94b989c8f3	test(switch): assert alt-l/alt-h no-op via before/after equality (#3245) ... Rewrites test_switch_picker_alt_l_does_not_hscroll as a before/after equality invariant (deleting its layout/timing-coupled snapshot), dedupes the picker PTY boilerplate into boot_picker_pty/abort_and_exit_code/list_pane_text, and keeps the ConPTY master alive for the session (the Windows BrokenPipe fix).	2026-06-25 12:13:51 -07:00
Maximilian Roos	523814ea43	test(switch): deflake comments-tab preview wait by re-issuing the tab key (#3238)	2026-06-25 09:10:25 +00:00
Maximilian Roos	830cc850dd	feat(list): show the main…± column by default; --full gates only off-machine columns (#3236) ... Move the `main…±` branch-diff column (line diffs since the merge-base) into the default `wt list` view — it's pure local git backed by a persistent content-addressed cache, so the original blocking-walk concern no longer applies. `--full` now gates only the two off-machine columns: CI status (network) and LLM branch summaries. The interactive picker (`wt switch`) follows suit and is effectively `wt list --full`; on narrow terminals with the preview shown, CI clips past the split and alt-p reveals it. Also adds a `.typos.toml` ignore rule for truncated word fragments glued to the … ellipsis, so the narrower Message column's truncated quickstart embed doesn't get spell-"corrected" by pre-commit.ci.	2026-06-25 01:49:47 -07:00
Maximilian Roos	de97744718	feat(switch): picker shortcuts — copy, open, refresh, remap remove to alt-x (#3233) ... Adds four keyboard shortcuts to the interactive `wt switch` picker: `alt-y` (copy the selected branch), `alt-o` (open the row's PR/MR), `alt-r` (refresh the list), and `alt-x` (remove, remapped from `alt-r`). `alt-y`/`alt-o` are native skim `Action::Custom` callbacks reading the selected row off `App.item_list` (no reload, so the cursor stays put and `--prs` rows aren't dropped); the row → branch/URL lookup is extracted into `resolve_shortcut_branch`/`resolve_shortcut_url` and unit-tested. `alt-r` refresh re-enters the collect pipeline via a new `PipelineFactory`. `alt-y` no-ops on a detached worktree (no branch). New cross-platform deps: `arboard` (clipboard) and `open` (browser), gated behind the `cli` feature. Merged over `codecov/patch` (89.76% vs 97.61% target) with explicit approval: the uncovered residual is the picker's clipboard/browser/thread-spawn and skim-`App`-bound closures, which a headless CI runner can't exercise. All required jobs green; reviewed and approved by worktrunk-bot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-25 00:21:55 -07:00
Maximilian Roos	d8fe755e31	test(switch): decline the picker approval with CR for Windows (#3230) ... Two follow-ups from #3221 (running the interactive picker tests on Windows). ## `test_switch_picker_pre_switch_hook_requires_approval` on Windows #3221 landed this test `#[cfg_attr(windows, ignore)]` with a TODO that suspected the picker's approval prompt couldn't read stdin after skim released the ConPTY. The actual cause, from the failing run's screen (prompt shown, `n` echoed, yet exit 1) and its timing (`FAIL [ 7.963s]` — the harness's 5s kill window plus setup): a **hang**, not an error. Once skim releases the terminal, the approval prompt's `read_line` runs in the OS line discipline (cooked mode). Windows' console line-input terminates a line on **CR** (the Enter key) and never on a bare **LF** — so the test's `"n\n"` left `read_line` blocked until the harness killed the hung process. Every other `[y/N]` `read_line` PTY test (`approval_pty.rs`, `config_show`'s `plugin_prompt_pty`) is `#[cfg(unix)]`-gated, so this picker test was the first to drive one on Windows. This was a test input-convention bug, not a product gap — a real Windows user presses Enter (CR) and is unaffected. The fix sends `"n\r"`: Windows reads it as Enter, and Unix's ICRNL maps it to LF. Verified on macOS (passes in ~3.5s, no hang); the Windows leg is confirmed by this PR's CI. ## Doc: `--help` snapshot regen Editing help text changes two generated artifacts — the doc mirrors (regenerated by `test_docs_are_in_sync`) and the rendered `--help` snapshots (`test_help`). Only `docs/CLAUDE.md` documented the second, so an agent following root `CLAUDE.md` or `src/commands/CLAUDE.md` would leave the help snapshots stale and fail required CI. Added the regen step to both. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 23:22:07 -07:00
Maximilian Roos	11d525dda2	feat(switch): unify picker preview tabs and dim empty diff tabs (#3223) ... ## What this does This makes the interactive `wt switch` picker's preview tab bar reflect each row's *real* data, instead of row-type policy or always-on assumptions. Two related changes: 1. **`--prs` only decides which rows are listed** — not which tabs a row shows or how a tab behaves. The `comments` tab is the clearest case: previously a worktree row's comments tab just pointed the user at `--prs` ("Comments show on `wt switch --prs` rows"), while a `--prs` row's showed the real thread. Now a worktree row whose branch has an open PR fetches and renders the same thread. 2. **The diff tabs dim when their diff is empty** — `working_tree`, `branch_diff`, and `upstream` no longer show a bright (selectable) number over a "nothing here" pane. ## Part 1 — `--prs` only controls listing The preview tabs split along two genuine data axes, and `--prs` touches neither: - **Local-checkout tabs** (working-tree, branch-diff, upstream, summary) — present on a worktree row, empty on a listed-PR row because it has no local copy. This is the only legitimate content difference, now expressed as `LocalTabs` rather than a `--prs` flag. - **PR-backed tabs** (`pr` and `comments`) — available together, gated by whether the row's branch has an open PR. `TabAvailability::pull_request()` (the special-case that forced the reduced treatment and hard-coded `comments` differently between row types) is gone. `from_axes(LocalTabs, has_pr)` is the single constructor; `comments` availability now equals `pr` availability on every row, so the comments tab behaves identically regardless of how the row got listed. The substance: a worktree row whose branch has an open PR fetches and renders the real comment thread — the same background `gh pr view <n> --json comments` / `glab api …/notes` fetch a `--prs` row makes, read back from the shared preview cache. ## Part 2 — diff tabs dim once their diff is known empty The same `TabAvailability` system marked `working_tree` / `branch_diff` / `upstream` as available on *every* worktree row, regardless of content — a clean working tree, a branch with no commits ahead, or one up to date with its upstream all rendered a bright number over an empty pane. These three tabs now follow the row's live diff content, reusing the loading-then-dim shape the `pr` tab already uses: available while the signal is still loading (so nothing dims prematurely during the async window — strictly the old always-bright behavior), dim once the diff is known empty. Each predicate mirrors exactly what its pane renders, so a dimmed number never contradicts a non-empty pane: - **working-tree** tracks `git diff HEAD` — staged/modified/renamed/deleted, *not* untracked (which `git diff HEAD` doesn't show); a branch-only row has no working tree, so it's empty immediately. - **branch-diff** follows the commits-ahead count vs the **local** default branch (`item.counts`) — deliberately the same base the pane diffs against (`default_branch_sha()`), *not* `has_file_changes`, which measures against the integration target and could dim the tab over a non-empty pane when the local default is stale vs its upstream. An orphan (no merge base) is kept available. - **upstream** combines the synchronous `has_upstream` floor (no tracking ref dims immediately, with no loading window) with the live ahead/behind — a present-but-even upstream dims too. The async signals are carried by a new per-row `LocalContent` slot, mirrored from the row's `ListItem` in `on_update` exactly like the existing `pr_status` slot — the frozen item snapshot the `WorktreeSkimItem` holds can't carry them (its `counts` / `upstream` / `working_tree_status` are `None` at skeleton and never update). ## Where to look - **`src/commands/picker/items.rs`** — `TabAvailability` reshaped around `LocalTabs` + `from_axes`; `LocalContent` (the live diff-content signals) + the `worktree(...)` mapping that dims a diff tab once its signal is known empty; `render_comments_pane` renders the fetched thread when the branch has a PR. - **`src/commands/picker/prs.rs`** — `spawn_comments_fetch` is the shared comments-fetch path for both row types; `spawn_worktree_comments_fetch` picks the forge CLI from the repo's **platform** (not the PR reference's `#`/`!` sigil — `#` is shared by GitHub, Gitea, and Azure). - **`src/commands/picker/progressive_handler.rs`** — the per-row live slots (`pr_status`, `comments`, and the new `local_content`) primed at skeleton and mirrored from each `ListItem` as the list pipeline lands. ## Testing Unit tests cover the comments-tab states, the once-per-PR / re-fetch-on-number-change comments guard, the `LocalContent::from_item` predicates (including untracked-only → empty and the `has_upstream` floor), and the loading-vs-known availability mapping; a snapshot pins the rendered bar with every diff tab dimmed. The `on_update` slot mirroring is covered through the existing handler test. The end-to-end PTY tests drive a real `wt switch` and confirm the worktree comments tab renders the actual thread with no `--prs` pointer. `CLAUDE.md`'s Network Access inventory records the worktree-row comments fetch. This branch also refreshes one **pre-existing** stale PTY snapshot surfaced by merging `main`: `switch_picker_alt_l_ignored_list` recorded `…main↕ CI Age`, inconsistent with its sibling `switch_picker_multiple_worktrees_list` (`…main↕ Remote⇅ CI`) even though the alt-l test asserts it captures "the same frame." The render had since settled on the sibling-consistent frame but the snapshot was never regenerated — it fails on current `main` HEAD too (verified by building `origin/main` and running the test). Regenerated to match. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 23:02:41 -07:00
Maximilian Roos	84efcb3b50	fix(switch): repaint the picker preview after the alt-r sticky reposition (#3225) ... After alt-r removes a worktree, the cursor sticks to the row that slides up, but the preview pane kept showing the row skim last previewed. The reposition action now returns Event::RunPreview on landing so the preview tracks the cursor. Also fixes the flaky test_switch_picker_alt_r_keeps_cursor_sticky by gating on the list-pane cursor pointer instead of the async preview pane.	2026-06-24 21:52:21 -07:00
Maximilian Roos	8f17cb4a91	test(switch): run the interactive picker tests on Windows (#3221) ... PR #3217 made `wt switch`'s interactive picker run on Windows, but its tests stayed `#[cfg(unix)]` — so the Windows CI leg proved the picker *compiled* without ever exercising its runtime. This ungates them so Windows CI drives the real thing. **What's ungated** (the test infra — `mock_commands`, the ConPTY handling in `tests/common/pty.rs` — was already cross-platform, so this is largely a gate flip): - `switch_picker_dry_run.rs` — the headless dry-run pipeline (collect → render → preview cache → summary spawn → warning drain). - The `--prs` dry-run tests in `switch.rs` — forge fetch + row render via the cross-platform mock `gh`/`glab`. - The two requires-tty snapshots — the no-TTY bail is uniform across platforms. - `switch_picker.rs` — the PTY/vt100 TUI snapshot tests. **Test-quality fix folded in:** the four `--prs` dry-run tests asserted only `output.status.success()`, but the fetch path is fail-soft (any error → stashed warning → exit 0). On a platform where the mock didn't resolve they would pass vacuously. They now assert the fetched row actually rendered (a `pr:42`/`mr:7` preview-cache entry, or the empty-list warning). **Also:** `/bin/cat` → `cat` in the summary test (resolves via the platform shell on Unix and Git Bash alike); removed the stale "picker is Unix-only" line that #3217 left in the `wt switch` docs (and regenerated the help snapshot). **Windows results (first full run):** of the ~50 ungated tests, 46 passed on Windows on the first run — including every PTY/vt100 snapshot test (skim renders byte-identically under ConPTY), the accept-path drain tests, and the `cat`-via-Git-Bash summary test. Two issues surfaced and are handled here: - The three interactive `--prs` picker tests failed because the test helper `mock_forge_env` joined mock-bin onto `PATH` with a hardcoded `:` separator — malformed on Windows (`;`), so the mock `gh.exe`/`glab.exe` was never found. Fixed to use `std::env::join_paths` (OS-aware), matching the dry-run path's helper. - `test_switch_picker_pre_switch_hook_requires_approval` exits 1 instead of 0 on Windows: declining the hook returns `Ok` on both platforms, so the failure is in the interactive approval prompt's stdin handoff after skim releases the ConPTY. Not reproducible locally — `#[cfg_attr(windows, ignore)]` with a TODO until a Windows box can debug it (still compiles on Windows, runs on Unix). This is a genuine picker-on-Windows gap surfaced by this work, tracked for follow-up. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 21:45:58 -07:00
Maximilian Roos	7ac7656d06	fix(switch): suppress skim's alt-l/alt-h horizontal scroll in the picker (#3226) ... ## What `alt-l` (and its mirror `alt-h`) horizontally scrolled the `wt switch` picker's list, sliding each row left under the fixed cursor gutter and clipping the leading worktree-status sigil (`@`/`^`/`+`/`⊂`) and the branch name — with no ellipsis to mark the cut. This binds both keys to skim's `ignore` no-op so the carefully laid-out table stops sliding. ## Why it happened The picker never bound `alt-l`/`alt-h`, so they fell through to skim 4.8's default keymap, where they map to `ScrollRight(1)` / `ScrollLeft(1)`. Those actions mutate skim's `manual_hscroll` offset. `no_hscroll(true)` (added in #3213 specifically to keep the sigil gutter stable) only zeros the *automatic*, match-following shift — `calc_hscroll_for_width` still adds `manual_hscroll` on top, so the manual-scroll keys escaped the guard entirely. Two visible symptoms, both reproduced live in a real picker: - One `alt-l` press shifts every row's content left by a column; the leading sigil scrolls off and one extra char appears on the right. Repeated presses chew further into the branch name (`picker-alt-l-gutter` → `er-alt-l-gutter`). The skim `>` cursor lives in a separate prefix gutter that is *not* scrolled, so it visibly detaches from the content it normally sits beside. - `manual_hscroll` is an `i32` clamped to `>= 0` only at render time (`.max(0)`), not at the state level, so `alt-h` drives it negative. After scrolling back you must "pay off" that negative backlog with `alt-l` before scrolling resumes — the keys feel unresponsive. ## Fix ```rust "alt-h:ignore".to_string(), "alt-l:ignore".to_string(), ``` `alt-h`/`alt-l` are the *only* default keys that produce `ScrollLeft`/`ScrollRight`, and skim's query word-navigation uses `alt-b`/`alt-f`/`alt-d` instead — so suppressing these two fully closes the manual-hscroll path with no collateral to query editing. The preview-tab keys (`alt-1`…`alt-7`, `Tab`) are unaffected (verified live). This is the manual-scroll counterpart to #3213's `no_hscroll(true)`; that PR closed the automatic path, this one closes the manual path the same guard left open. ## Testing Adds `test_switch_picker_alt_l_does_not_hscroll`, a PTY test that drives `alt-l`/`alt-l`/`alt-h` and snapshots the list with its gutter sigils intact. Verified the test **fails** when the fix is removed (the `@`/`+` sigils clip: `> @ main` → `> main`) and passes with it. Full `pre-merge` gate green (4199 tests, all lints, `--features shell-integration-tests`). > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 21:40:02 -07:00
Maximilian Roos	ef453797ac	fix(switch): keep the alt-r picker row when its target isn't actually removed (#3211) ... ## Summary In the `wt switch` interactive picker, pressing `alt-r` dropped a row optimistically and then removed the worktree on a background thread. When the removal didn't actually remove the target — a worktree that became dirty, or a branch-only row whose unmerged branch `SafeDelete` keeps — the worktree/branch was preserved by design (data safety holds), but the displayed row had already vanished. The list showed a removal that never happened, and the row that did come back flickered off and on. This closes the producer→consumer loop so a row's presence always matches reality, and decides as much as possible **up front** so the common cases never flicker. ## Approach The key observation: `prepare_worktree_removal` — which `invoke` already calls synchronously on skim's event loop — *already* computes whether a removal will succeed (`ensure_clean` for worktrees, `integration_reason` for branch-only rows, from the same `Repository::integration_reason` that `delete_branch_if_safe` later consults). So the outcome is knowable before the row is dropped; we don't need to run the removal to find out. Two complementary mechanisms, duals of each other: - **`removal_will_remove_target`** (predict, *before* dropping). A worktree always removes (it passed `ensure_clean`); a branch-only row removes only when `delete_branch_if_safe` would (integrated, or force, never `Keep`). `invoke` drops the row and removes in the background only when removal is predicted. An **unmerged branch-only row** — the one case that reliably flickered under the optimistic drop — now stays put and surfaces the canonical `Branch X retained; has unmerged changes` + `To delete the unmerged branch, run wt remove -D X` info/hint that `wt remove` itself prints, decided synchronously with no background work. - **`removal_target_still_present`** + **`restore_failed_removal`** (observe, *after*). The data-safety backstop for failures the up-front prediction can't see, because they're genuinely asynchronous: a clean-check race against `ensure_clean`, a failing approved `pre-remove` hook, or a branch that raced integrated→unmerged. These are decided from **primary evidence** — does the worktree directory or branch ref still exist after `do_removal`? — not from the removal's `Result`, which is the wrong signal in both directions (a `RemovedWorktree` removal can return `Err` after the worktree is already trashed when a `post-remove`/`post-switch` hook fails to render/spawn during the announcer flush; a refusal returns `Ok` while keeping the target). When the target survives, the row is re-inserted at its slot, a `Kept X — could not remove it` warning is stashed, and the picker reloads so the cursor lands back on the row. The happy path (successful removal) and the sticky-cursor reposition from #3199 are untouched. ### Behavior matrix | `alt-r` on… | target after | row | |------|----------------------|-----| | clean worktree (common case) | gone | drops, no flicker | | worktree, dirty at `alt-r` | present | not dropped (caught by `prepare`, no flicker) | | **unmerged branch-only row** | **present** | **stays put, no flicker** — surfaces `retained; unmerged` | | branch-only, integrated | gone | drops, no flicker | | worktree clean→dirty race / failing approved `pre-remove` | present | dropped → restored (rare async backstop) | | branch raced integrated→unmerged | present | dropped → restored (rare async backstop) | The only residual flicker is the genuinely-rare async races in the last two rows, which can't be known up front and where restoring is the data-safety backstop. ## Key files Most changes are in `src/commands/picker/mod.rs`: `removal_will_remove_target` (the up-front predictor) and `removal_target_still_present` (the after-the-fact observer), with `invoke` dispatching to `drop_and_remove_in_background` or `keep_unremovable_row`, plus `restore_failed_removal` / `removal_failure_subject` for the backstop path and a single `send_reposition` helper for cursor moves. The canonical "retained; unmerged" message is shared with `wt remove` via `retained_unmerged_branch_messages` in `src/output/handlers.rs`, so the two emit paths can't drift. ## Testing `cargo run -- hook pre-merge --yes` passes (4191 tests, clippy, fmt). New/updated coverage: - `test_removal_will_remove_target` — the predictor's matrix (worktree, integrated/unmerged/force/keep branch-only). - `test_invoke_keeps_unmerged_branch_only_row` — synchronous keep through `invoke`: row never dropped, canonical hint stashed, branch preserved. - `test_invoke_restores_row_when_removal_fails` — the backstop, end-to-end through `invoke` via an approved-yet-failing `pre-remove` hook. - `test_switch_picker_alt_r_keeps_unmerged_branch_row` — a real-PTY integration test that filters to an unmerged branch, presses `alt-r`, and asserts the row survives (the inverse of the existing "removed row empties the list" test). #3199's sticky-cursor PTY test still passes. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 20:34:35 -07:00
Maximilian Roos	c70eedf152	feat(switch): run the interactive picker on Windows (#3217) ... ## Run the `wt switch` interactive picker on Windows The picker was gated `#[cfg(unix)]` because its preview-tab switching (alt-1…7 jump to a tab; tab/shift-tab cycle) was implemented as skim `execute-silent` keybindings that shelled out to `echo`/`tr`/`mv` through a per-process state file. skim runs keybind commands through the platform shell — `cmd.exe` on Windows, which has neither `tr` nor `mv` — so that was the hard blocker. skim 4.x (the ratatui/crossterm rewrite worktrunk already depends on) supports Windows. This replaces the shell keybindings with native handling: the active tab is now a process-wide in-memory `AtomicU8` (`PreviewStateData`), and the keys are bound to `Action::Custom` callbacks inserted directly into skim's `options.keymap` (resolved with skim's own `parse_key`, so they match its event-loop lookup exactly). Each callback sets the mode and returns `Event::RunPreview`. This drops the state file, the `ModeWatcher` background poller, and `shell_escape::unix` — a net simplification on every platform, not just a Windows shim. With the shell dependency gone, the `#[cfg(unix)]` gate comes off the whole picker, along with the now-stale gates on its dependencies — both in source (`GitHubPrInfo`, `open_pr_status`, `SwitchPipeline`, the column-grid types, `ShowConfig`, `PickerProgressHandler`, `format_aligned`, `generate_summary`) and in `Cargo.toml`, where the picker's TUI stack (`skim`/`ratatui`/`ansi-to-tui`/`tokio`) moved out of `[target.'cfg(unix)'.dependencies]` into the main table so it's present in the Windows dependency graph. The FAQ is updated accordingly. ### Where to look - `src/commands/picker/preview.rs` — `PreviewStateData` is now in-memory; `PreviewMode::next`/`prev` rotation. - `src/commands/picker/mod.rs` — `install_preview_tab_keybindings` (the native bindings) and a `ModeWatcher`-free `run_skim`. - `Cargo.toml` — TUI deps relocated out of the unix-only target table. - `src/commands/{mod,worktree/mod,worktree/switch}.rs`, `src/main.rs` — picker / `SwitchPipeline` gate removal. - `src/commands/list/{ci_status,layout,collect,render}.rs`, `src/summary.rs` — transitive gate / dead-code-suppression removal. ### Testing Unit tests cover the rotation logic (`PreviewMode::next`/`prev`) and the keymap wiring; the existing PTY integration tests in `tests/integration_tests/switch_picker.rs` drive the real picker and assert tab switching end-to-end (alt-N jump, tab/shift-tab cycle + wrap). CI is green on all three platforms — `test (windows)` confirms skim 4.8 + frizbee and their transitive deps compile and the suite passes on Windows MSVC, which is the question this PR set out to answer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 18:52:46 -07:00
Maximilian Roos	321494be75	feat(switch): show a scrollbar on the interactive picker list (#3198) ... The picker drives skim with default-features = false, so skim's clap-gated `▐` scrollbar default never applied — the library Default for the field is the empty string, which skim reads as "no scrollbar". Set the indicator explicitly so a thumb renders when the list overflows. Known limitation: skim styles the thumb with no color of its own, so the `▐` inherits the color of the row beneath it (most visibly the current-line highlight). The upstream fix is skim-rs/skim#1097, which styles the thumb with the theme's border color; once it lands and the skim dep bumps, the thumb stays uniform with no further change here.	2026-06-24 11:10:19 -07:00
Maximilian Roos	599c4bab91	feat(switch): keep the alt-r picker cursor on the removed row's slot (#3199) ... ## Sticky cursor after `alt-r` removal in the switch picker Removing a worktree with `alt-r` in the `wt switch` picker used to snap the cursor back to the first row every time, because skim clears `item_list` on every `reload` (skim #1695). Removing several rows in a row was jarring — the selection jumped to the top after each one. Now the cursor stays on the slot the removed row vacated: the row that slides up into its place (the "next" item), or the new last row when the removed row was last. ## Why it's done this way skim 4.8 offers no clean lever for "keep the cursor after reload": - `handle_reload` calls `item_list.clear()` (resets the cursor to the top) unless `no_clear_if_empty` is set — and that flag is the wrong tool: the matcher runs once on the just-cleared empty pool and writes an empty `Replace`, which re-empties the list and resets the cursor anyway. Its stale-keeping path is also gated on `interactive` mode, which the picker isn't. - `select-row(n)` looks promising but only inserts into the multi-select set; it never moves the cursor. - `down(n)` / `first` / `last` take a fixed integer parsed at bind-time, so the bind string can't carry the dynamic pre-removal index. The lever that does work is `Action::Custom(ActionCallback)`: its callback runs with `&mut App`, and `App.item_list` exposes public cursor methods. After a removal, `PickerCollector::invoke` injects a Custom action (through skim's event sender — the same `render_tx` the progressive handler already uses) that, once the reloaded rows land, repositions via `jump_to_first()` + `scroll_by(target)`. Because the reload repopulates `item_list` asynchronously (reader → matcher → render), the action re-arms itself until the rows exist, and stops once the matcher has *settled* on an empty result so removing the sole match of an active query can't spin the event loop. Sleeping inside the callback isn't an option — `ActionCallback::call` blocks on the future, so an await would hold `&mut App` and starve the very render that loads the rows. ## Where to look Everything is in `src/commands/picker/mod.rs`: - `sticky_reposition_target` — pure index math (removed `shared_items` position → `item_list` data-row index), unit-tested. - `reposition_cursor_action` — the self-re-arming `Action::Custom`, gated on `item_list.count()` with a matcher-settled stop and a hard backstop. - `PickerCollector::invoke` — computes the target and injects the action. - The `PickerCollector` / module docstrings explain the skim mechanics. ## Limitations Under an active fuzzy query the displayed order diverges from `shared_items` order, so the landing row is approximate — a valid nearby row, clamped into range, rather than the exact next row. The no-query case (the common one) is exact. ## Testing Unit test covers the index math (including the removed-last-row and header-only edge cases). Behavior was verified interactively against a multi-worktree repo via tmux: middle-row removal lands on the next row, last-row removal lands on the new last row, a sequence of removals from one position keeps the cursor planted, rapid-fire removals never reset to the top, and removing the sole match of a query leaves the picker fully responsive (no spin). Full TUI behavior isn't unit-testable without a PTY, so that surface relies on the interactive checks. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 10:25:32 -07:00
Maximilian Roos	b3e1fc06b4	feat(switch): log + comments preview tabs for --prs picker rows (#3169) ... Adds background-loaded `log` and `comments` preview tabs to `wt switch --prs` picker rows, with a width-aware tab bar that compacts to fit the 7th tab. The log tab uses a local `git log` when the head commit is present, else a forge-fetched list.	2026-06-23 19:10:15 -07:00
Maximilian Roos	81f9260a46	feat(switch): browse open PRs/MRs in the interactive picker (#3128) ... Adds `wt switch --prs`: browse the repository's open PRs (GitHub) / MRs (GitLab) in the interactive picker, with a live CI column (same data as `wt list --full`), a `pr` preview tab showing the markdown-rendered description, and `#`-gutter-sigil filtering. Selection routes through the existing `pr:`/`mr:` fetch-and-switch path, so there's no new switch logic.	2026-06-22 16:34:04 -07:00
Maximilian Roos	c5c2742058	feat(switch): make the picker filter text match the gutter sigil (#3143) ... Typing into the `wt switch` picker filtered only on branch name + path, so typing a gutter sigil like `+` matched nothing. Each row now folds its gutter glyph into the fuzzy-search text via a canonical `ItemKind::gutter_glyph` (the single source shared with the rendered Gutter column, replacing `BranchScope::gutter_sigil`), so `+` filters to linked worktrees and `@` to the current worktree. The glyph is a skeleton-time fact (`is_current`/`is_main` set at construction, `BranchScope` structural), so folding it in keeps fuzzy ranks stable across the picker's progressive column updates, and the rendered Gutter column stays byte-identical. `^` and `|` collide with skim's prefix-anchor and OR query operators (so `^` matches everything and `|` nothing), and `/` over-matches because every path contains it; the picker help documents which sigils filter cleanly, and an engine-level test pins the operator semantics against skim 4.8's default stack. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 15:36:11 -07:00
Maximilian Roos	958b9c188a	refactor(picker): migrate to skim 4.8 (ratatui), drop vendored skim-tuikit (#3137) ... Migrates the `wt switch` picker from skim 0.20.5 (tuikit backend) to skim 4.8.0 (ratatui/crossterm), removes the `vendor/skim-tuikit/` patch tree we carried against the old line, and makes the full test suite green under the new backend. ## Why the vendor tree goes away We vendored skim-tuikit for two patches: `alt-<digit>` key parsing and an `Output::flush` `write_all` fix for dropped bytes under PTY pressure. Both are moot in 4.8. Key parsing is native — `binds.rs` routes every single character (digits included) through `KeyCode::Char` rather than a hardcoded `alt-a..alt-z` arm list. The flush fix is subsumed by stdlib `BufWriter::flush` in the ratatui backend, which loops over partial writes correctly. So the migration carries zero vendor patches against skim, and the build machinery that kept the vendored source alive (Taskfile `vendor-diff`, flake `vendorSrc`/`extraDummyScript`) is deleted too. ## The picker rewrite skim 4.x is a different API. `WorktreeSkimItem::display()` now returns `ratatui::text::Line` instead of an `AnsiString`. alt-r removal rides skim's `reload(remove {})` token (the selected row's `output()`, expanded into the reload command) instead of the old `as_any().downcast_ref` path (which never worked across compilation units) plus a signal file. Preview-tab switching and the progressive list redraw are driven by `Skim::event_sender()` + `Event::Render`/`Event::RunPreview`. ## The regression that motivated the bulk of this skim 4.x renders on demand. There is no 100ms tuikit heartbeat re-rendering the frame, so the progressive `wt switch` list rendered blank while collection ran in the background — the original report. The fix pokes `Event::Render` from the list-collect callbacks (throttled to 16ms). The same on-demand model surfaced four more regressions, all fixed here and verified head-to-head against the 0.20 picker in a terminal: preview-tab lag, current-row highlight, Shift-Tab back-cycle (crossterm reports Shift-Tab under three distinct `KeyEvent` shapes, so all three are bound), and alt-r removal (4.x `execute-silent` is fire-and-forget and raced its reader). ## Help text and the test harness Two things the integration suite caught that the unit tests did not (the prior pass ran `--lib --bins` only): - **Styled help.** skim 0.20 transitively pulled in `clap/unstable-markdown`, which renders worktrunk's `///` doc-comments as styled help. skim 4 with `default-features = false` drops it, so help reverted to raw text (`[experimental]` → `\[experimental\]`). Restored by depending on `unstable-markdown` explicitly, keeping help output identical and the `test_help` / `test_step_alias` / `test_docs_are_in_sync` snapshots passing unchanged. - **PTY harness.** skim 4.x queries cursor position (`ESC[6n`) at startup in partial-height mode and blocks in `select()` for the reply; `portable_pty` is a bare PTY and never answered, so every `switch_picker` test failed init with "Cursor position detection timed out." The Unix harness now answers the query, mirroring the existing ConPTY responder. skim 4.x also draws the list/preview separator one column left, so the snapshot panel-split columns shifted to match. The regenerated picker snapshots show two cosmetic changes: the match counter no longer overlaps the preview tab header, and the HEAD column shows the full short-SHA instead of a truncated one. ## Minimal-versions CI The skim 4.8 dep tree pins tighter floors than our manifests declared, so the nightly `minimal-versions` job needed updating. It now runs `-Z direct-minimal-versions` — minimize only our own direct deps and let transitive crates resolve normally — and the manifests raise each under-specified floor to the minimum the workspace builds against (largely mirroring skim 4.8's own requirements). Full `-Z minimal-versions` would instead drag in skim's transitive TUI/image stack (ansi-to-tui, ratatui's `instability` macro, color-eyre, ratatui-image → image/avif), whose crates under-declare their floors and don't compile at the picked versions; direct minimization confines the check to floors we own, so no transitive pins are needed. Normal resolution (the committed `Cargo.lock`) is unchanged. Full floor list and the `signal-hook` libc-pin detail are in the `ci(min-versions)` commit message. ## Reviewing Start at `src/commands/picker/mod.rs` (the `run_skim` entry point, the action keybinds, and `parse_reload_remove_token`), then `progressive_handler.rs` (the render pokes) and `items.rs` (the `Line`-based `display()`). Test-harness changes are in `tests/common/pty.rs` (the `ESC[6n` responder) and `tests/integration_tests/switch_picker.rs` (the panel-split columns). The rest is the dependency swap, the vendor and build-machinery deletions, and regenerated snapshots. All 3980 tests pass (`cargo run -- hook pre-merge --yes`), including the `shell-integration-tests` PTY suite that exercises the picker end-to-end across the list, previews, scroll, create/remove, and accept flows. No automated test drives a real terminal; the interactive surface was also checked by hand against the 0.20 picker. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-20 03:41:20 -07:00
Maximilian Roos	7d3c923ef0	feat(picker): free digit keys for typing; preview tabs to Alt + Tab/Shift-Tab (#3079)	2026-06-15 01:59:29 -07:00
Maximilian Roos	e4dd591731	refactor(switch): unify the picker and argument-path switch pipelines (#2858) ... ## Summary `wt switch <branch>` and the interactive picker (`wt switch` with no argument) each ran the same switch sequence as separate, parallel code. [#2845](https://github.com/max-sixty/worktrunk/pull/2845) made the two paths *behave* identically; this makes the *code* identical too — a single `SwitchPipeline` that both entry points build and `.run()`. `SwitchPipeline::run` (in `src/commands/worktree/switch.rs`) owns the whole sequence: the bare-repo worktree-path fix-up, pre-switch hooks, source-identity capture, `plan_switch` → `approve_switch_hooks` → `validate_switch_templates` → `execute_switch`, output, background hooks, and `--execute`. Each caller now only resolves a branch identifier and loads config. The picker-vs-argument differences (`--execute`, the shell-integration offer, source-identity capture) are struct field values, not divergent branches. ## Bug fixed The duplication hid a real bug: the picker passed `yes = true` to `run_pre_switch_hooks`, **auto-approving project `pre-switch` hooks without a prompt** — unapproved code from a freshly cloned `.config/wt.toml` running silently. Every other hook the picker runs (`post-switch`, `pre-create`, `post-create`) already went through the approval prompt. With one shared `run_pre_switch_hooks` call gated by the pipeline's single `verify`/`yes` pair, the picker (which has no `--yes`) now prompts for project `pre-switch` hooks like `wt switch <branch>` does — and the two paths can't drift on hook approval again. ## Reviewer notes - One intentional reorder on the argument path: `offer_bare_repo_worktree_path_fix` now runs before `run_pre_switch_hooks` (the picker already used this order). The fix only mutates `worktree-path` config, which pre-switch hooks never read — behavior-neutral. - `capture_switch_source` runs inside `run()` after pre-switch hooks — the same relative position as the old `run_switch`. - Eight now-internal helpers were narrowed from `pub`/`pub(crate)` to private; `worktree/mod.rs` re-exports trimmed accordingly. ## Testing Pure refactor — the existing `test_switch_*`, `test_switch_format_json_*`, and `test_switch_picker_*` suites cover behavior preservation. `test_switch_picker_pre_switch_hook_requires_approval` is a new regression test for the bug fix (verified to fail under the bug). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-21 09:47:18 -07:00
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>	2026-05-21 16:03:03 +00:00
Maximilian Roos	ff4e481a4c	fix(switch): interactive picker runs the full switch pipeline (#2845) ... With `[switch] cd = false` or `wt switch --no-cd`, opening the interactive picker (`wt switch` with no branch argument) and selecting a worktree printed the branch name and exited — no switch, no hooks, and `Alt-c` created nothing. #1445 introduced this as an explicit print-only mode, but it surprises anyone who sets `cd = false` as a standing navigation preference (driving worktree switches through tmux or cmux): the picker silently stops doing anything. The picker now runs the same `plan_switch` → `execute_switch` pipeline as `wt switch <branch>`, suppressing only the cd directive. Pre- and post-switch hooks fire, `Alt-c` creates the worktree, and `--format=json` works in the picker too (`requires = "branch"` and the `--branches`/`--remotes` conflict are dropped from the flag). JSON emission is extracted into a shared `emit_switch_json` so the argument path and the picker produce identical output. This is a behavior change: the print-only picker mode is gone. Scripts that captured its bare-branch-name output should switch to `wt switch --format=json`, which both performs the switch and prints a structured `action`/`branch`/`path` result — now shown as a `pick` alias example in tips-patterns. The cmux recipe drops its `pick` alias workaround, since the picker fires `pre-switch` hooks directly. Closes #2837. Thanks to @endigma for the discussion that pinned down the intended behavior: the picker should be identical to passing the branch as an argument. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-20 21:05:19 -07:00
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)	2026-05-20 19:31:50 -07:00
Maximilian Roos	1ad14b290f	test(switch_picker): cursor-nav preview tests; remove dead row-count gate (#2769) ... Follow-up to #2767, which fixed the observed macOS picker flake on the target-selection tests by gating on preview-pane text instead of `expected_rows: Some(1)`. That left the same matcher-lag root cause live in the four `preview_panel_*` tests, and left the `expected_rows` gate machinery in place even though it had no remaining users. The `preview_panel_*` tests typed `"feature"` to filter the picker to one row, then gated on `expected_rows: Some(1)`. skim's matcher runs on a separate thread; under heavy parallel macOS load its row redraw can lag the keystroke-driven prompt past the 30s gate timeout and panic with the `saw 2` diagnostic. These tests select `feature` by Down-arrow navigation instead — the matcher only runs on query changes, so cursor navigation never invokes it and the selection is deterministic regardless of load. The flake is structurally impossible here now, not papered over with a longer timeout. With no remaining `Some(n)` call site, the entire `expected_rows` machinery is dead. This removes the third tuple element across all PTY-test call sites, both PTY-helper signatures, the loop bindings, `visible_list_rows()`, the `display_ready` branch, and the line-525 `saw N` timeout panic (the historical #2334/#2345 flake site). `wait_for_stable_with_content` now gates only on content + stability. Also deletes `test_switch_picker_switch_to_existing_worktree`: a strict subset of `test_switch_picker_emits_cd_directive_by_default` (identical setup/inputs/default command) whose `screen.contains("target-branch") || "Switched" || "cd "` assertion passed even on a wrong selection, since `target-branch` is in the picker list itself. The cd-directive test already verifies the real switch outcome via the directive file. Reviewer note: the four `preview_panel_*` `_list` snapshots now show two rows (`@ main` + cursor on `+ feature`) instead of one filtered row. That is the intended, deterministic consequence of dropping the filter query, not a regression — the tests assert preview-panel content; the list panel is incidental scaffolding. Net diff is a reduction (−173/+65 in the test file). Verified: all 18 `switch_picker` tests pass, `cargo fmt --check` clean, `cargo clippy --tests --features shell-integration-tests` clean. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-15 15:43:10 -07:00
Maximilian Roos	9bea859428	test(switch_picker): gate target-selection tests on preview pane, not row count (#2767) ... The `expected_rows: Some(1)` gate (added in #2729) waits for skim's row list to filter down after the test types its query. But skim's matcher runs on a separate thread, and under heavy parallel macOS load the row redraw can lag the matcher by more than the 30s timeout — even though skim's selection cursor and preview pane have already advanced to the matched item. The gate then panics with `saw 2`, while the test would have selected the right item anyway: Enter acts on the cursor, not on which rows happen to be painted. This is the flake reported on `test_switch_picker_no_cd_prints_branch_without_switching` (local) and `test_switch_picker_switch_to_existing_worktree` (CI on #2764). These three `target`-selection tests don't snapshot the list panel, so they don't need the filter applied — only the cursor on the right item. They now gate on the preview-pane text that's emitted once skim's selection moves to target-branch, which tracks the cursor rather than the lagging redraw. If selection genuinely never moves, the content gate still fails loudly, so the diagnostic value of #2729 is preserved. Reproduced the original flake at ~2/5 with 5 parallel `cargo test test_switch_picker -- --test-threads=20`. After the fix: 0 occurrences of the line-528 panic across 48-run focused stress, the original 5×20 recipe, and a 16× pathological run. The `preview_panel_*` snapshot tests share the same matcher-lag root cause but genuinely need the filter applied (they snapshot the list); they weren't in scope here and remain as follow-up. Also deletes `test_switch_picker_no_cd_suppresses_directive` — a strict subset of `test_switch_picker_no_cd_prints_branch_without_switching` (identical setup/env/inputs, both assert exit 0 + empty cd file; the latter additionally asserts the printed branch name). Same belief, no coverage lost. Net diff is a reduction. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-15 14:33:32 -07:00
Maximilian Roos	c73098b8bc	docs(test): correct switch_picker comments about skim's on-screen counters (#2733) ... Two comment-only fixes following #2729 — no behavioral change, no snapshot change. - `is_skim_ready` no longer claims skim "displays item count like 1/3"; the picker uses `no_info(true)`, so it doesn't render that. - The snapshot filter that normalizes the trailing `N/M` on the preview pane's first line now correctly attributes it to skim's previewer vertical-scroll indicator (`{vscroll_offset}/{content.len()}`, drawn in reverse video at the right edge — see `skim::previewer::Previewer::draw`), rather than the matcher's matched/total counter. The earlier comment's misattribution is what led #2345's stability check to parse the wrong thing; #2729 fixed the check, this fixes the comment so the misunderstanding doesn't resurface. The filter regex and replacement string are untouched. Co-authored-by: Claude <noreply@anthropic.com>	2026-05-11 18:45:23 -07:00
Maximilian Roos	3cc687909d	fix(test): gate switch_picker stability on matcher row count, not preview vscroll (#2729) ... ## Summary The flake's mechanism was that skim's matcher runs on a separate thread and, under heavy parallel load (the failure first surfaced in a 3641-test nextest run), can lag seconds behind the prompt repaint. The test types `feature`, skim renders `> feature` into the query line, but the items list still has both rows. The prior consistency check (`display_matches_count`, added in #2345) was supposed to detect exactly this and reject "stable" screens with row counts that disagreed with skim's `N/M` counter — except `no_info(true)` hides the real counter, so the regex was actually matching the previewer's `{vscroll_offset}/{content.len()}` indicator (e.g. `1/28`). That coincidentally equalled `visible_list_rows == 1` in the filtered steady state, so it looked like it worked, but under matcher lag the wait simply timed out at 30 s and `wait_for_stable_with_content` warned-and-proceeded — capturing the stale unfiltered screen with `@ main` still visible. ## Fix Replaced the indirect counter-parsing gate with an explicit `expected_rows: Option<usize>` parameter threaded through the input tuple and `wait_for_stable_with_content`. Stability now requires `visible_list_rows == expected_rows` when set, and the timeout path **panics with a diagnostic dump** (instead of silently proceeding) so a future matcher-lag regression fails at the actual cause rather than at the snapshot assertion downstream. The four `(feature, …)` filter tests (`uncommitted`, `log`, `main_diff`, `summary`) and the four `(target, …)` selection tests now pass `Some(1)`. Tests that don't depend on a specific row count pass `None`. `parse_match_count` and `display_matches_count` are deleted — they were checking the wrong thing. ## Test plan - [x] `cargo run -- hook pre-merge --yes` — 3666 tests pass. - [x] Switch picker suite (19 tests) passes under `--test-threads=8`. - [ ] Stress repro: the original flake reproduced ~1/19 in the very first repro attempt under contention, then did not reproduce in 100+ isolated iterations nor 14 iterations under explicit CPU stress. The fix is hypothesis-based but firmly grounded in the captured `.snap.new` (both rows visible despite the filter) plus skim's source (matcher on a separate thread; previewer's vscroll indicator that the old regex was actually matching). Closes #2334 (which #2345 only accidentally guarded). Co-authored-by: Claude <noreply@anthropic.com>	2026-05-11 17:55:51 -07:00
Maximilian Roos	ad0b572b4d	fix(picker): pre-flight template validation in picker-create (#2712) ... The interactive picker's create path (Alt-C in `wt switch`) skipped the template pre-flight that `wt switch --create` performs. If a user's project config had a broken hook template (syntax error, undefined variable), picking a branch with Alt-C succeeded at `git worktree add`, then failed when `pre-start` expansion errored — and the worktree existed, blocking re-running with the same name. `run_switch` already calls `validate_switch_templates` between `approve_switch_hooks` and `execute_switch` for exactly this reason (see its module docstring, "Why only switch needs pre-flight validation"). This PR adds the same call in `handle_picker`. The helper goes from `fn` to `pub(crate) fn` and is re-exported from `commands/worktree`. A PTY test in `switch_picker.rs` commits a syntax-broken `pre-start = "echo {{ unclosed"` to the user config, drives picker-create via Alt-C, and asserts no branch + no worktree dir + non-zero exit. It then fixes the template and re-runs to confirm no half-state was left behind. Verified the test fails as expected when the picker fix is reverted (branch and worktree get created). Co-authored-by: Claude <noreply@anthropic.com>	2026-05-11 11:26:45 -07:00
Maximilian Roos	567ee61db9	test(switch_picker): gate stability on matcher/display consistency (#2345) ... Under heavy macOS parallel load, skim's matcher updates its `N/M` counter ahead of the list-panel repaint, so `wait_for_stable` could declare a screen stable while a filtered-out row was still visible — producing the `test_switch_picker_preview_panel_main_diff` snapshot diff tracked in #2334. Reproduced locally by running the test binary under 8-way parallel cargo test load (~1-in-120 failure rate). An instrumentation trace showed the captured t=0 screen with skim's counter at `1/4` but two list rows still visible. Fix: add a consistency check that rejects stable states where skim's parsed match count doesn't equal the visible list-row count, in both the primary and fallback return paths of `wait_for_stable_with_content`. Validated locally with 1480 iterations under 8-way parallel load on macOS: 0 failures. Full `switch_picker` suite (17 tests) passes. Closes #2334 > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude <noreply@anthropic.com>	2026-04-20 15:05:01 -07:00