refactor(statusline): share the worktree lookup and collection plan - #3582
Conversation
Both statusline surfaces (JSON and the rendered line) each held their own copy of the worktree lookup and of the CollectOptions plan, kept in sync by a comment. Extract statusline_options() and current_worktree_item() so the plan and the lookup have one home; the lookup reports "no worktree here" rather than deciding, since the callers render that differently.
worktrunk-bot
left a comment
There was a problem hiding this comment.
Traced the refactor — it's behavior-preserving, and the one non-textual change (moving git_status_segments's "no default branch → just show the branch" early-out to run after current_worktree_item computes is_home) is safe: primary_worktree() resolves through the same cached default_branch() (no extra work, no new path to the wire), and is_home is unused on that early-out branch, so the output is unchanged.
The only red is codecov/patch, and it isn't a real coverage regression. The two missed lines are both the item.branch.as_deref().unwrap_or("HEAD").to_string() fallback — the default_branch().is_none() branch and the segments.is_empty() branch in git_status_segments. Both existed on main as wt.branch.as_deref().unwrap_or("HEAD") and were already uncovered there (no test exercises either fallback); the refactor only renamed wt → item on those lines, which pulls their pre-existing misses into the patch. Arithmetic: 2 misses / 31 patch lines = 93.54%.
Per CLAUDE.md, that's the "misses that predate the change, or a path with no deterministic trigger" case — not a reason to hold the refactor. Flagging it so the merge decision is explicit, since codecov/patch gates merge despite being marked non-required.
format_statusline_segments always emits the branch segment, so the segments.is_empty() fallback below it was unreachable. The remaining fallback — no default branch to compare against — had no test; add one.
Cuts v0.69.2, and fixes the Windows code-signing path it depends on. ## The signing fix v0.69.1 shipped an unsigned `wt.exe` under a green run and a **Completed** SignPath signing request. `archive: false` (#3566) had already made the GitHub artifact name `worktrunk-x86_64-pc-windows-msvc.zip`, and SignPath names its download after the artifact — so with `output-artifact-directory: target/distrib` the signed zip landed at `worktrunk-x86_64-pc-windows-msvc.zip.zip`, beside the untouched unsigned build. The checksum step and the release upload both kept reading the original. Confirmed by parsing the published asset's PE certificate table: `size=0`. The download now goes to a scratch directory and whatever single file lands there replaces the built zip, so SignPath's naming isn't load-bearing. ## Verification, because this failure is invisible Signing is `continue-on-error` by design (self-signed test certificate pending SignPath's OSS review), so nothing in the logs distinguishes "signed" from "silently unsigned" — which is how it slipped through twice, two different ways. `.github/verify-windows-signature.py` reads the zip's PE certificate tables directly, and runs at two points with distinct jobs: - **before the overwrite** — an unsigned release is tolerable while the certificate is a test one; a corrupt one never is, so the replacement has to verify before it can clobber a good build. - **after** — reports what the release actually ships, which is the question the logs never answered. ## Rehearsed before landing The signing path only runs on a tag, so each question about it used to cost a release. This chain was instead run on a Windows runner against the already-published v0.69.1 zip (identical bytes, no build): ``` saved to …\target\signpath\worktrunk-x86_64-pc-windows-msvc.zip.zip git-wt.exe: certificate table 8480 bytes wt.exe: certificate table 8472 bytes → mv → target/distrib/worktrunk-x86_64-pc-windows-msvc.zip target/distrib/worktrunk-x86_64-pc-windows-msvc.zip: all 2 executables signed worktrunk-x86_64-pc-windows-msvc.zip: OK (checksum matches) ``` That also settled the open question behind #3556: SignPath returns the signed zip itself, not a wrapper, so `skip-decompress: true` is correct — the extract mode does explode it into loose files. ## Release contents `wt remove`'s fsmonitor sweep (#3581), the troubleshooting doc trim, and this fix. The two refactors in range (#3582, #3584) are behavior-preserving and omitted per convention. Local gate green (4547 passed). Changelog entries verified against the diffs by subagent; two claims corrected (the doc entry's rationale, and an overclaim attributing v0.69.0's unsigned binary to this bug rather than the separate upload failure #3566 fixed). Data-loss surface reviewed across the cumulative diff. > _This was written by Claude Code on behalf of max_
The two statusline surfaces —
run_json(--format=json) andgit_status_segments(the rendered line) — each carried their own copy of "find my worktree, decide if it's home, build the item", and their own literalCollectOptions, kept in sync by a comment reading "Same plan as the other statusline path". Both now come from one place:statusline_options()holds the shared plan (all columns, full gates, CI on, summary off, untracked counted in the working diff), andcurrent_worktree_item()holds the lookup, returningOption<ListItem>so it reports "no worktree here" rather than deciding what to print — the callers render that case differently (an empty JSON result vs. a bare branch segment).Behavior-preserving. Verified by building the pre- and post-change binaries side by side and diffing their output across four directories (a dirty worktree, the primary worktree,
$HOME,/tmp) × default/--format=json, plus the--format=claude-codestdin path: byte-identical output and matching exit codes in every case.One ordering change is not a pure textual move and is where review attention is best spent:
git_status_segments's "no default branch → just show the branch" early-out now runs afterprimary_worktree()instead of before it. Both resolve through the same cacheddefault_branch(), so there's no extra work and no new path to the wire.Two of the moved lines were the branch-name fallbacks at the end of
git_status_segments, both uncovered on main and now counted as patch lines. Rather than leave them, the second one is gone —format_statusline_segmentspushes the branch segment unconditionally, so thesegments.is_empty()fallback below it was unreachable — and the first, "no default branch to compare against", gets the test it never had: a repo with two branches, neither named main/master/develop/trunk and no remote, leavesdefault_branch()nothing to infer from, and the statusline is the branch name alone.Otherwise no new tests: the refactor runs under existing coverage (107 statusline tests: 57 unit, 50 integration), with the A/B above as the end-to-end evidence, since the tests alone only pin what each surface does today and wouldn't catch the two plans silently diverging.