refactor(git): RefSnapshot — close ambient ref-keyed cache staleness class#2528
Merged
Conversation
Records the design exploration behind the upcoming RefSnapshot refactor: inventory of ref-keyed RepoCache fields at risk after PR #2507, six options considered, and the rationale for landing on RefSnapshot (Option 1) — the only option that handles composite caches (integration_reasons, effective_integration_targets, ahead_behind) structurally rather than by naming-vector deletion alone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces a captured, immutable view of repository ref state. RefSnapshot is the structural answer to ref-name cache staleness — see docs/dev/cache-staleness.md for the full rationale. Repository::capture_refs runs one for-each-ref refs/heads/ + one for-each-ref refs/remotes/ and assembles the result into an immutable value. capture_refs_with_ahead_behind additionally batches ahead/behind counts (git ≥ 2.36). The snapshot is a value, not a cache field — no OnceCell, no Arc, no shared mutable state. Two captures within a command produce two distinct snapshots; neither invalidates the other. This is intentional: it removes the "invisible refresh" surface that ambient ref-keyed caching introduces. This commit is purely additive. The existing LocalBranchInventory and RepoCache ref-keyed fields are untouched; both paths coexist temporarily. Subsequent commits will (a) add SHA-taking sibling methods, (b) migrate wt list and the integration check to consume snapshots, and (c) delete the ambient ref caches plus the ref_is_ancestor workaround from #2507. Tests cover the freshness contract: capture, mutate ref via git commit, capture again, assert SHAs differ — and the earlier snapshot still reports the pre-write SHA, by design. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # src/git/mod.rs # src/git/repository/mod.rs
The ambient `commit_shas: DashMap<String, String>` and the composite caches derived from it (`integration_reasons`, `effective_integration_targets`, `ahead_behind`, `integration_target` OnceCell, `tree_shas`, `resolved_refs`) went stale whenever wt itself moved a ref mid-command — most prominently in `wt merge`, where `update-ref` advances the local target between the merge and the post-write integration check (PR #2507). Replace the ambient cache layer with explicit, point-in-time `RefSnapshot` values threaded through the read paths: - `wt list` captures one snapshot in the pre-skeleton phase and threads it into every task via `TaskContext`. Tasks call `_by_sha` siblings (`is_ancestor_by_sha`, `ahead_behind_by_sha`, …) instead of going through ref-keyed lookups. - `compute_integration_lazy`, `integration_reason`, `effective_integration_target`, `integration_target(s)`, `compute_integration_reason`, `delete_branch_if_safe`, and `remove_worktree_with_cleanup` all take a `&RefSnapshot`. - `finish_after_merge` captures a **fresh** snapshot AFTER `handle_no_ff_merge`/`handle_push` runs `update-ref` — this is the structural fix for PR #2507. The `ref_is_ancestor` workaround is deleted. `rev_parse_commit`, `rev_parse_tree`, and `resolve_preferring_branch` become uncached; the priming side-effect in `scan_local_branches` (writes to `commit_shas`/`resolved_refs`) is removed. The persistent SHA-keyed `merge_base` and `branch_diff_stats` caches stay (key contract tightened: SHAs only). Snapshot test updates reflect strictly better behavior: - `list_warns_when_commit_details_batch_fails` now embeds the resolved SHA in the error rather than the literal "main" — that's what was passed to git. - The three `list_*_with_nonexistent_default_branch` snapshots no longer show the same-commit `_` symbol when the configured default branch is missing (a false positive — there was no ref to compare against). PR #2507's regression test (`test_remove_merged_locally_when_upstream_diverged`) still passes without `ref_is_ancestor`. PR #2513's `test_list_integrated_when_merged_locally_with_upstream_diverged` passes. 3413/3413 tests under `wt hook pre-merge --yes`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # src/commands/step_commands.rs # src/git/repository/integration.rs # src/git/repository/mod.rs
…lone Three small follow-ups surfaced by /reviewing-code over the RefSnapshot cutover: - `branches.rs`: `local_branches` doc still claimed the scan primed `resolved_refs` and `commit_shas` — both gone. Replace with the current contract: `commit_sha` is a snapshot at scan time; current SHAs come from a `RefSnapshot`. - `diff.rs`: `merge_base` doc referenced "the cached `commit_shas` map" for resolution. Point at `resolve_to_commit_sha` instead, which is the actual path now (hex short-circuit + uncached `rev-parse`). - `step_commands.rs`: drop a `snapshot.clone()` before moving into an `Arc` — `snapshot` isn't read after that point in `step_prune`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The earlier docstring refresh introduced two broken intra-doc links that `RUSTDOCFLAGS=-Dwarnings cargo doc` rejects: - `branches.rs:111`: `[`RefSnapshot`]` doesn't resolve in `branches.rs` scope (the type lives in a sibling module). - `diff.rs:152`: `[`Self::resolve_to_commit_sha`]` is a public doc linking to a private item. Both fix the same way: drop the link bracket form, keep the prose. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
worktrunk-bot
approved these changes
May 2, 2026
3 tasks
max-sixty
added a commit
that referenced
this pull request
May 2, 2026
…hot cutover (#2530) ## Summary Two follow-ups deferred from #2528 (the `RefSnapshot` cutover that closed the ambient ref-keyed cache staleness class), plus reviewer cleanups. - **Thread `RefSnapshot` through `prepare_worktree_removal`** — adds `snapshot: Option<&RefSnapshot>`; `step_prune` and `validate_remove_targets` capture once and reuse across candidates instead of re-running `for-each-ref` per candidate (N+1 → 1). - **Drop `head_shas` cache** — the per-worktree HEAD SHA cache had no invalidation. Any flow that moved HEAD mid-command (rebase, hook-emitted commits) left a stale value that surfaced in `{{ commit }}` template variables for later hooks. `WorkingTree::head_sha` now always reads fresh; new `head_sha_tracks_head_movement` test guards against re-introducing a cache. - **Reviewer cleanups** — refreshed four stale comments referring to the deleted cache, collapsed a now-redundant short-circuit in `command_executor.rs::build_hook_context`, and replaced `Arc::new(snapshot.clone())` in `step_prune`'s rayon worker with `Arc::clone` (shares by refcount, no deep copy). ## Test plan - [x] `cargo run -- hook pre-merge --yes` — full test + lint suite (3425 tests pass; clippy / doc / doctest with `-Dwarnings` clean) - [x] New unit test `head_sha_tracks_head_movement` exercises the freshness contract directly - [x] Manual review of stale-comment fixes vs current behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>
Merged
max-sixty
added a commit
that referenced
this pull request
May 3, 2026
Cuts v0.47.0. Highlights: - **Integration detection fix** across `wt list`, `wt remove`, `wt merge`, `wt step prune` for the local/upstream-diverged case (#2507, #2513, #2515). - **Faster shell-command dispatch** (~10ms per invocation, 1.49× on trivial aliases) via event-driven signal forwarding (#2537, #2538), plus parallel config load (#2543) and merged cold-start prewarm rev-parse (#2541). - **`wt switch <number>` suggests `pr:N` / `mr:N`** instead of just `--create` (#2516). - **Library API breaking changes**: `RefSnapshot` cutover (#2528, #2530) and the `for-each` direct-exec change (#2465). Full list in `CHANGELOG.md`. See `CHANGELOG.md` for the complete entry. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com>
max-sixty
added a commit
that referenced
this pull request
May 11, 2026
The doc proposed and analyzed six options for closing the ref-keyed cache staleness class; the codebase adopted Option 5 (`RefSnapshot`) in #2528 and extended it with the content-addressed `ahead-behind/` SHA cache in #2704. The structural contract now lives in `src/git/repository/ref_snapshot.rs` (snapshot semantics, lifetime, construction) and the cache-kind landscape in `src/commands/list/collect/mod.rs`'s Caching section, both of which supersede what was in this file. What remained in the doc actively misleads: its `RepoCache` inventory table cites fields that no longer exist (`commit_shas`, `tree_shas`, `effective_integration_targets`, `integration_reasons`, `ahead_behind`, `head_shas`) and references a `batch_ahead_behind` helper that's been replaced. The file was an island — no inbound links from code, tests, or other docs, and `docs/dev/` is not a Zola content directory so it was never published — so deletion is clean. Anyone curious about the original design debate can find it at the merge commit for #2528. Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the ambient ref-keyed caches in
RepoCachewith an explicit, point-in-timeRefSnapshotvalue threaded through read paths. This closes the bug class behind PR #2507 (post-update-refintegration check seeing pre-write SHAs) — and several latent variants — by making the caches that could go stale unrepresentable.Why
RepoCachecached ref-name → SHA incommit_shas: DashMap<String, String>, plus composite caches keyed off it (integration_reasons,effective_integration_targets,ahead_behind,integration_targetOnceCell,tree_shas,resolved_refs). When wt itself moved a ref mid-command — most prominentlywt mergerunninggit update-ref refs/heads/<target>to advance the local target — those caches went stale, and any downstream read returned pre-write SHAs.PR #2507 patched one symptom by adding
ref_is_ancestor(a cache-bypass helper) at the integration-check call site. That works locally; every other ref-name read after a write was a latent bug.The structural fix: there is no longer an ambient ref-name → SHA cache. Callers explicitly capture a
RefSnapshotat known points and thread it through the read paths. After ref-mutating operations, callers capture a fresh snapshot — the discipline lives at the small set of write boundaries (currently one site,finish_after_merge), not at every ref-name accessor.Background analysis:
docs/dev/cache-staleness.md(commit ba8b239) walks through six options and the trade-offs.What changed
src/git/repository/ref_snapshot.rs—RefSnapshotvalue type. Onefor-each-refover local + remote refs, optional ahead/behind batch. Cloned by value, noOnceCell/Arc. ~330 lines including unit tests (the freshness contract test mutates a ref and re-captures)._by_shasiblings of cached methods (is_ancestor_by_sha,merge_base_by_sha,branch_diff_stats_by_sha,ahead_behind_by_sha,has_added_changes_by_sha,trees_match_by_sha,merge_integration_probe_by_sha, …). The persistent on-disksha_cache(already SHA-keyed) sits behind these.compute_integration_lazy,integration_reason,compute_integration_reason_uncached,integration_targets,delete_branch_if_safe,remove_worktree_with_cleanupall take&RefSnapshot.wt merge'sfinish_after_mergecaptures a fresh snapshot AFTERhandle_no_ff_merge/handle_pushrunsupdate-ref. This is the structural fix for PR fix(integration): OR over local + upstream for integration detection #2507. Theref_is_ancestorworkaround is deleted.wt listmigration — pre-skeleton phase captures one snapshot;TaskContextcarries it; tasks call_by_shasiblings via the snapshot. No regression intended (one extrafor-each-refupfront, lose per-taskrev-parsefallbacks).commit_shas,tree_shas,effective_integration_targets,integration_reasons,ahead_behind,integration_targetOnceCell,resolved_refsall gone fromRepoCache.LocalBranchInventory's priming side-effects (writes tocommit_shas/resolved_refs) removed.rev_parse_commit,rev_parse_tree,resolve_preferring_branchno longer cache; consumers that need SHA-stable lookups thread aRefSnapshot.sha_cache::*and the in-memorymerge_base/branch_diff_statscaches keep their SHA-keyed contract.Snapshot test deltas
Three
list_*_with_nonexistent_default_branch.snapsnapshots no longer show the same-commit_symbol when the configured default branch is missing — that was a false positive (no ref to compare against).list_warns_when_commit_details_batch_fails.snapnow embeds the resolved SHA in the error rather than the literal"main"ref name (strict improvement — the SHA is what was passed to git).Regression coverage
test_remove_merged_locally_when_upstream_diverged) passes withoutref_is_ancestor.test_list_integrated_when_merged_locally_with_upstream_divergedpasses.RefSnapshotcapture-mutate-recapture confirms two captures observe distinct SHAs afterupdate-ref.Out of scope (deferred)
Both flagged by
/reviewing-codeand explicitly deferred:prepare_worktree_removalre-captures a snapshot per candidate insidestep_prune— N+1for-each-refcalls. Threading an optional&RefSnapshotwould makestep_prunereuse its entry snapshot. Affects display consistency in unusual hook setups (apost-removehook that runsgit fetchbetween candidates can desync the displayed integration target from the safety decision); not a safety bug.head_shascache retained. Different staleness model (per-worktree, mutated bycommit/resetrather thanupdate-ref); design doc lists it as a remaining vector. Out of scope for this PR's integration-target focus.Test plan
cargo run -- hook pre-merge --yes(3413 tests pass before the merge oforigin/main)origin/maincargo check --all-targetsclean