Repository navigation
Remove the cache epoch - #4203
Conversation
`CACHE_EPOCH` shipped in the 0.79.0 release PR to stop one class of stale entry: releases through 0.78.0 wrote `has-added-changes` from porcelain `git diff`, which under `diff.relative` (from a subdirectory the changes sit outside) or `submodule.<name>.ignore` recorded a branch with changes as having none — and that answer is what makes `wt remove` delete the branch without `-D`. The entries are keyed on the branch and target tips with no TTL, so on a finished branch the key never recurs. The mechanism is out of proportion to that. It fires at most once per repository, on the first command after upgrading past 0.78.0, and only pays off for a user who has one of those two settings and a live poisoned key. Against that it carried a stamp file, a wipe, a global mutex on the funnel every cache read and write passes through, a `TestRepo` that now creates a cache directory for every fixture, a documented hole when two worktrunk versions share a repository, and a failure path where an undeletable directory leaves the reader with the entries the wipe existed to discard. So it goes, and the changelog entry for #4146 tells the affected population the one command that fixes them: `wt config state cache clear`. CLAUDE.md keeps the durable half — these entries outlive the code that wrote them, so a generator change owes its changelog entry a line — and records that not versioning the cache is the decision. Reverts the epoch parts of #4181. `cargo run -- hook pre-merge --yes` green (4926 tests). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
worktrunk-bot
left a comment
There was a problem hiding this comment.
Not approving this one: per .claude/skills/running-tend/references/review-pr.md § "Data-Loss Surface: Hold for Human Review", a diff that loosens a check gating wt remove's branch deletion isn't the bot's to approve. Against main as it stands, this is what stops a has-added-changes: false entry an earlier release computed wrong from reaching check_integration, so for a user with one of those two settings and a live key the next wt remove <branch> deletes unmerged work without -D. The description's reasoning and its timing both hold — v0.78.0 is still the newest tag — but trading automatic remediation for a changelog line on a branch-deletion bug is your call, not the bot's.
The rest takes the changelog route as given, and is about whether that line reaches the people it's for.
The self-check misses .gitmodules. It gates on "if either setting is in your git config", but PlumbingDiff's doc comment in src/git/diff.rs says plumbing diffs "still honor submodule.<name>.ignore, from repository config or .gitmodules" — and .gitmodules is where that setting usually lives, committed and shared. Someone whose repo sets ignore = dirty there checks git config, finds nothing, skips the clear, and keeps the entry.
The clear is per repository. handle_cache_clear opens Repository::current(), so "once" is once per repository — and diff.relative is typically a ~/.gitconfig setting, so the affected user has it in all of them.
The inline suggestion fixes both by splitting the remediation into its own bullet. That also pulls the main entry back toward the changelog's own ceiling: the remediation took it from 78 words to 118, against the 80 the release skill allows a headline entry, and this section is re-drafted and verified at tag time — a trailing clause is the most trimmable place for the one instruction affected users need.
The summary-cache caveat leaves with the code. UNVERSIONED_KINDS' doc comment was the only record that summary keys on the diff hash while SUMMARY_TEMPLATE and llm::prepare_diff's filtering sit downstream of that hash — so rewording the prompt leaves finished branches holding the old prompt's summary, and closing it means hashing the rendered prompt rather than the diff. src/summary.rs's "Layout and invalidation" section doesn't carry that ("Content-addressed filename … No TTL, no separate staleness check"), and the new CLAUDE.md paragraph scopes the question to sha_cache's kinds, which excludes summary. Say the word and I'll move those two sentences into src/summary.rs.
…mmary caveat Three findings from tend's review, each verified against the source before applying: - The changelog's remediation gated on "in your git config", but `PlumbingDiff`'s doc (src/git/diff.rs:147) records that plumbing honors `submodule.<name>.ignore` from repository config *or* `.gitmodules`, and a scratch repo confirms git reads it there. That is where the setting usually lives, committed and shared, so the instruction missed the common case. It also read as one-time, while `handle_cache_clear` opens `Repository::current()` — it is once per repository. Splitting the remediation into its own entry fixes both and brings the pair under the release skill's ceiling: 69 and 64 words against 80 for a headline. - Two referents in CLAUDE.md dangled once `bumps CACHE_EPOCH` came out of the paragraph: "an entry" collided with the cache entries the preceding sentences discuss, and "Ask whenever…" no longer named a question. - `UNVERSIONED_KINDS`' docstring was the only record that the summary cache hashes the diff while the prompt template and `llm::prepare_diff`'s filtering sit downstream of that hash, so rewording the prompt strands every finished branch's summary. `src/summary.rs` says only "No TTL, no separate staleness check", and the new CLAUDE.md paragraph scopes to `sha_cache`'s kinds, which excludes `summary` — so the caveat moves into summary.rs's own "Layout and invalidation" section rather than leaving with the code that happened to carry it. `cargo run -- hook pre-merge --yes` green (4931 tests). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The split-out "Cached integration answers…" bullet described no change — it was an upgrade notice wearing a changelog entry's clothes. The remediation belongs to the entry for the change that created the need, as its closing clause. Keeps both corrections that motivated the split: `submodule.<name>.ignore` counts from `.gitmodules` as well as git config, and the clear is per repository rather than once. 94 words against a headline ceiling of 80; the alternative was a bullet that isn't a changelog entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The #4146 entry named the two settings but not what else each needed, so it read as though any user with `diff.relative` could lose a branch. Both cases need two conditions at once, verified against the v0.78.0 binary: `diff.relative` *and* a command run from a subdirectory the branch's changes sat entirely outside; `submodule.<name>.ignore` *and* a submodule bump as the branch's only change. The entry now joins each pair with "and" and calls them corner cases. The release skill gains the rule behind that: a correctness or data-loss entry states the conditions the bug needed, joined with "and", established by reproducing against the previous release rather than read off the diff — since a condition assumed is one the entry overstates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The #4146 entry ended by telling affected users to run `wt config state cache clear`. A changelog records what changed; it doesn't instruct, so the sentence goes and the entry ends on the change. That leaves the CLAUDE.md paragraph prescribing the very line just removed — "owes its changelog entry a line … telling the reader that `wt config state cache clear` drops them" — so it now routes the concern to the PR description, where the trade is decided. Slimmed while there. The release skill gains the rule, since its existing guidance let an imperative through twice: entries describe; remediation steps belong in the PR description. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
b0ee1511 leaves no user-facing remediation anywhere. The case for deleting CACHE_EPOCH was that the changelog would carry the instruction instead — it's the PR description's argument, and it was CLAUDE.md's wording until this commit ("so the remediation goes in the changelog instead"). That commit removes the changelog clause and redirects the obligation to the PR description, which nobody upgrading from 0.78.0 reads. Step 12 of the release skill is explicit that release.yaml builds the GitHub release notes from the tree at the tag, so CHANGELOG.md is the channel to the affected user, and the PR description is not a channel at all.
So the chain is now: automatic invalidation gone, and the one line that told an affected user to run wt config state cache clear gone with it. A 0.78.0 user with diff.relative in ~/.gitconfig and a live has-added-changes: false entry upgrades, sees a note that the bug is fixed, and the next wt remove <branch> still deletes an unmerged branch without -D. One of the two has to stay. The changelog suggestion restores the clause with the per-repository qualifier 7967eb79 had — handle_cache_clear opens Repository::current() and sha_cache::clear_all sweeps only that repository's .git/wt/cache/, while diff.relative is normally set once in ~/.gitconfig — and adds the missing "as" in the submodule clause.
The new release-skill rule is the durable half, and as written it reaches further than this entry. CHANGELOG.md already instructs, in each of the three releases before this one: "Pi users re-run it to get a Pi extension" (0.78.0), "Re-run wt config plugins opencode install to pick it up" (0.77.0), "Nushell's wrapper is a static file, so rerun wt config shell install to pick this up" (0.76.0). Each of those is a static artifact the upgrade can't rewrite for the user, which is the same shape as a cache entry it can't recompute. A blanket "leave out remediation steps" makes those entries retroactively wrong and suppresses the next one. The suggestion narrows it to what looks like the real target — reasoning and advice out, a required action in one clause kept.
Verdict stays withheld for the reason in the first review.
"Entries describe; they don't instruct" contradicted two entries from
recent releases that tell users to reinstall a static file ("Re-run `wt
config plugins opencode install` to pick it up"; "rerun `wt config shell
install` to pick this up"). Those steps are part of how the change
arrives for everyone using the feature, and the release notes are the
only place those users see them.
The rule now keeps them and still excludes what was removed from the
#4146 entry: a step every user of a feature must take is part of what
changed, while remediation for a corner case is advice for the PR
description.
Also adds a missing "as" to the #4146 entry.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…issues Replaces the delivery-versus-advice line with the maintainer's criterion: the changelog is descriptive, and imperative guidance is reserved for a material issue that affects a sizable share of users, which is rare. Other remediation belongs in the PR description. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes
CACHE_EPOCHand the machinery around it, added in #4181.Time-sensitive: 0.79.0 is merged to
mainbut not tagged and not on crates.io, so the epoch has never reached a user. Landing this before the tag means it never ships.What it was protecting
Two corner cases in 0.78.0 made
wt removedelete an unmerged branch without-D, each needing two conditions at once. Reproduced against the v0.78.0 binary:diff.relativeset and the command run from a subdirectory the branch's changes sat entirely outside. Run from the repository root with the same config, 0.78.0 correctly retains the branch.submodule.<name>.ignoreset (git config or.gitmodules) and a submodule bump as the branch's only change.In both, 0.78.0 reports
✓ Removed branch feature (no added changes on main, ⊂)and 0.79.0 retains it. #4146 fixed how the answer is computed, but 0.78.0 cached its wrong answer keyed on the branch and target tips with no expiry, so the fix can't reach an entry already on disk.Why it goes
The epoch fires at most once per repository, on the first command after upgrading, and pays off only for a user who hit one of those corner cases and has a live cached key and then removes that branch. Against that it carried a stamp file, a tree wipe, a global mutex on every cache read and write, a documented hole when two versions share a repository, and a failure path where an undeletable directory left the reader with exactly the entries the wipe existed to discard.
So nothing replaces it: a repository that hit one of the corner cases under 0.78.0 keeps that cached answer after upgrading, exactly as it would have kept it on 0.78.0.
wt config state cache cleardrops it for anyone who asks. The changelog entry describes both corner cases and stays descriptive — it names no remediation.No new negative behavior
0.78.0 and 0.79.0 read any entry already on disk identically — across
v0.78.0..HEADthe key functions, the kind set, the consumers of both integration probes, andcheck_integration's priority order are all unchanged, and no cached struct changed shape. They diverge only on a cache miss, where 0.79.0 is right and 0.78.0 was wrong. So 0.79.0 without the epoch is a strict improvement on 0.78.0, and at worst a tie.Also
src/summary.rs: keeps the caveat that the summary cache hashes the diff while the prompt sits downstream of that hash, which previously lived only in the docstring this removes.CLAUDE.md: records that not versioning the cache is the decision, and that a generator change says in its PR what the old answers get wrong.🤖 Generated with Claude Code