diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 2aa2c951d5..999cce396f 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -145,11 +145,18 @@ awk '/^## /{if (f) exit; f=1} f' CHANGELOG.md \ **Calibrate against the ceiling, not against the last release.** Length ratchets: each release is drafted beside the previous section, and an abstract rule loses to a concrete neighbouring exemplar every time. Entries grew from 49 to 101 words on average across five releases while this skill said "be brief" throughout. Read the previous section for what it drifted to, then ignore it and write to the ceiling. +**Entries describe.** An entry records what the old behavior was and what it is now. Give imperative guidance only for a material issue that affects a sizable share of users, which is rare; otherwise remediation belongs in the PR description. + **No editorial framing.** Describe what changed, not what was wrong with the previous decision in subjective terms. Avoid words like "sledgehammer", "ugly", "noisy", "wrong" applied to past code. State the prior behavior neutrally and the new behavior plainly. **Good:** "Removed `.pi/` from the default excludes list; users who need it can add it via `[step.copy-ignored]`." **Bad:** "Removed `.pi/` — a sledgehammer fix from an unrelated debugging session that has no place as a project-agnostic default." +**Name the conditions a bug needed, so the entry carries its magnitude.** A correctness or data-loss entry reads as though it fired for everyone unless it says what it took. These are usually narrow — a non-default git config, a command run from a particular place, a change of a particular shape — and naming each condition lets a reader decide in one pass whether it reached them. Join the conditions with "and": "X set *and* run from Y" is a corner case, while "X or Y" reads as two common triggers. Establish each one by reproducing against the previous release's binary rather than reading it off the diff — a condition assumed is a condition the entry overstates, and overstating a data-loss bug is its own kind of inaccuracy. + +**Good:** "`diff.relative` set *and* the command run from a subdirectory the branch's changes sat entirely outside" +**Bad:** "`diff.relative` could hide a branch's changes from the integration check" + ### Credit External Contributors For any changelog entry where an external contributor (not the repo owner) authored the commit, add credit with their GitHub username: diff --git a/CHANGELOG.md b/CHANGELOG.md index 99297a4448..8cc1ac108d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ ### Fixed -- **Diff display config no longer changes what Worktrunk reads**: `color.ui = always` and `diff.external` reached the commit, squash, and summary prompts. Two settings could also hide a branch's changes from the integration check, so `wt remove` deleted the branch without `-D`: `diff.relative` when run from a subdirectory the changes sat outside, and `submodule..ignore` on a submodule bump. Every diff Worktrunk parses now runs plumbing, which ignores that configuration, and upgrading discards the cached answers. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147), [#4181](https://github.com/max-sixty/worktrunk/pull/4181)) +- **Diff display config no longer changes what Worktrunk reads**: `color.ui = always` and `diff.external` reached the commit, squash, and summary prompts. Two corner cases could also hide a branch's changes from the integration check, so `wt remove` deleted the branch without `-D`: `diff.relative` set *and* the command run from a subdirectory the branch's changes sat entirely outside, or `submodule..ignore` set (git config or `.gitmodules`) *and* a submodule bump as the branch's only change. Every diff Worktrunk parses now runs plumbing, which ignores that configuration. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147)) - **`wt config update --output ` asks before overwriting**: it replaced whatever was there, so `--output ~/.config/worktrunk/config.toml` from a repo with a deprecated project config wiped the user config and reported success. It now prompts, takes `--yes`, and refuses the config being migrated. (Breaking: `--output` naming the config being migrated used to succeed unless the migration dropped `approved-commands`.) ([#4153](https://github.com/max-sixty/worktrunk/pull/4153)) diff --git a/CLAUDE.md b/CLAUDE.md index e87a57c067..08bfa357fd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -243,7 +243,7 @@ No `get_*` — bare nouns follow Rust stdlib convention. `Repository` caches read-only values via `Arc` (cloning shares it). What is and isn't cached, the `list_worktrees()` post-mutation invariant, the two storage patterns, and the in-memory-`RepoCache`-vs-persistent-`sha_cache` decision (cheap-and-hot → in-memory get-or-create; expensive → disk; both → in-memory front over disk back): the `# Caching` section in `src/git/repository/mod.rs`. -**A change to what a cached value means bumps `CACHE_EPOCH`** (`src/cache.rs`). The persistent entries are keyed on SHA pairs with no TTL, so the key cannot see the generator's output: an entry written by an older `wt` keeps answering until its key recurs, and on a finished branch that is never. Moving the diffs onto plumbing is the worked example — it left `has-added-changes` entries saying an unmerged branch had no added changes, which is `wt remove` deleting it. So the question belongs to any change that alters what a cached read reports, not just to code inside `cache.rs`; ask it whenever a generator behind one of `sha_cache`'s kinds changes. +**A change to what a cached value means leaves the old answers in place.** The persistent entries are keyed on SHA pairs with no TTL, so an entry written by an older `wt` keeps answering until its key recurs, and on a finished branch that is never. Moving the diffs onto plumbing is the worked example — it left `has-added-changes` entries saying an unmerged branch had no added changes. Nothing invalidates these on an upgrade, deliberately: a cache version is permanent machinery for a transition that has come up once. So when a generator behind one of `sha_cache`'s kinds changes, the PR description says what the old answers get wrong, which is where that trade gets decided. ## Releases diff --git a/docs/src/content/docs/faq.md b/docs/src/content/docs/faq.md index 9f759af114..023dd1c263 100644 --- a/docs/src/content/docs/faq.md +++ b/docs/src/content/docs/faq.md @@ -138,7 +138,7 @@ Worktrunk stores repository state, caches, and logs under `.git/`: | Location | Purpose | Created by | |----------|---------|------------| | `git config worktrunk.*` | Cached default branch, switch history, branch markers, custom variables | Various commands | -| `.git/wt/cache/{kind}/*.json` | Cached CI status, the largest PR/MR number seen (sizes the `wt list` CI column), and git command results (merge-tree, integration probes, diff stats, ancestry checks, ahead/behind counts, merge bases), beside a stamp of what those results mean — a worktrunk that disagrees with it discards the tree | `wt list`, `wt merge`, `wt remove` | +| `.git/wt/cache/{kind}/*.json` | Cached CI status, the largest PR/MR number seen (sizes the `wt list` CI column), and git command results (merge-tree, integration probes, diff stats, ancestry checks, ahead/behind counts, merge bases) | `wt list`, `wt merge`, `wt remove` | | `.git/wt/cache/summary/{branch}/{hash}.json` | Cached LLM branch summaries, content-addressed by diff hash | `wt list --full`, `wt switch` (when `[list] summary = true`) | | `.git/wt/cache/picker-preview/*.json` | Rendered preview panes for the interactive picker | `wt switch` | | `.git/wt/logs/{branch}/**/*.log` | Background hook output (nested per branch) | Hooks, background `wt remove` | @@ -213,7 +213,6 @@ A branch checked out in a second worktree is retained regardless, `-D` included. - `wt merge` / `wt step push` — the target branch's checked-out worktree is updated to the merged commits, so a file those commits delete disappears from it, and an ignored file at a path they track is overwritten — the same result a `git merge` run in that worktree would produce. Uncommitted changes at paths the merge doesn't touch stay in place, staged or not; one at a path it does touch refuses the merge upfront, naming the file - `wt remove` — also stops the removed worktree's `git fsmonitor--daemon` (under `core.fsmonitor=true`), and sweeps `.git/wt/trash/` entries older than 24 hours along with fsmonitor daemons whose worktree no longer exists -- any command reading a cache — discards the cached git-command results under `.git/wt/cache/` when they were written by a worktrunk that computed them differently, so a stale answer can't outlive the code that produced it. Cached CI status and LLM summaries are keyed on what they describe, so they are left alone - `wt config state clear` — removes all worktrunk data from `.git/` (config keys, caches, markers, hints, variables, logs, stale trash) - `wt config shell install` — replaces an existing fish or Nushell wrapper file whole, and removes one an older version installed at a previous location (fish `conf.d/wt.fish`, Nushell `/vendor/autoload/wt.nu`), printing each removal; [Files created](/shell-integration/#files-created) names the file each shell gets - `wt config shell uninstall` — removes the lines that run `wt config shell init` from bash/zsh/PowerShell rc files, and deletes wrapper and completion files carrying worktrunk's content markers (fish `functions/`, `conf.d/`, and `completions/`; nushell `vendor/autoload`). Every removed line is printed diff --git a/plugins/worktrunk/skills/worktrunk/reference/faq.md b/plugins/worktrunk/skills/worktrunk/reference/faq.md index b9bbcc7320..e3e8e4d4ad 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/faq.md +++ b/plugins/worktrunk/skills/worktrunk/reference/faq.md @@ -134,7 +134,7 @@ Worktrunk stores repository state, caches, and logs under `.git/`: | Location | Purpose | Created by | |----------|---------|------------| | `git config worktrunk.*` | Cached default branch, switch history, branch markers, custom variables | Various commands | -| `.git/wt/cache/{kind}/*.json` | Cached CI status, the largest PR/MR number seen (sizes the `wt list` CI column), and git command results (merge-tree, integration probes, diff stats, ancestry checks, ahead/behind counts, merge bases), beside a stamp of what those results mean — a worktrunk that disagrees with it discards the tree | `wt list`, `wt merge`, `wt remove` | +| `.git/wt/cache/{kind}/*.json` | Cached CI status, the largest PR/MR number seen (sizes the `wt list` CI column), and git command results (merge-tree, integration probes, diff stats, ancestry checks, ahead/behind counts, merge bases) | `wt list`, `wt merge`, `wt remove` | | `.git/wt/cache/summary/{branch}/{hash}.json` | Cached LLM branch summaries, content-addressed by diff hash | `wt list --full`, `wt switch` (when `[list] summary = true`) | | `.git/wt/cache/picker-preview/*.json` | Rendered preview panes for the interactive picker | `wt switch` | | `.git/wt/logs/{branch}/**/*.log` | Background hook output (nested per branch) | Hooks, background `wt remove` | @@ -209,7 +209,6 @@ A branch checked out in a second worktree is retained regardless, `-D` included. - `wt merge` / `wt step push` — the target branch's checked-out worktree is updated to the merged commits, so a file those commits delete disappears from it, and an ignored file at a path they track is overwritten — the same result a `git merge` run in that worktree would produce. Uncommitted changes at paths the merge doesn't touch stay in place, staged or not; one at a path it does touch refuses the merge upfront, naming the file - `wt remove` — also stops the removed worktree's `git fsmonitor--daemon` (under `core.fsmonitor=true`), and sweeps `.git/wt/trash/` entries older than 24 hours along with fsmonitor daemons whose worktree no longer exists -- any command reading a cache — discards the cached git-command results under `.git/wt/cache/` when they were written by a worktrunk that computed them differently, so a stale answer can't outlive the code that produced it. Cached CI status and LLM summaries are keyed on what they describe, so they are left alone - `wt config state clear` — removes all worktrunk data from `.git/` (config keys, caches, markers, hints, variables, logs, stale trash) - `wt config shell install` — replaces an existing fish or Nushell wrapper file whole, and removes one an older version installed at a previous location (fish `conf.d/wt.fish`, Nushell `/vendor/autoload/wt.nu`), printing each removal; [Files created](https://worktrunk.dev/shell-integration/#files-created) names the file each shell gets - `wt config shell uninstall` — removes the lines that run `wt config shell init` from bash/zsh/PowerShell rc files, and deletes wrapper and completion files carrying worktrunk's content markers (fish `functions/`, `conf.d/`, and `completions/`; nushell `vendor/autoload`). Every removed line is printed diff --git a/skills/worktrunk/reference/faq.md b/skills/worktrunk/reference/faq.md index b9bbcc7320..e3e8e4d4ad 100644 --- a/skills/worktrunk/reference/faq.md +++ b/skills/worktrunk/reference/faq.md @@ -134,7 +134,7 @@ Worktrunk stores repository state, caches, and logs under `.git/`: | Location | Purpose | Created by | |----------|---------|------------| | `git config worktrunk.*` | Cached default branch, switch history, branch markers, custom variables | Various commands | -| `.git/wt/cache/{kind}/*.json` | Cached CI status, the largest PR/MR number seen (sizes the `wt list` CI column), and git command results (merge-tree, integration probes, diff stats, ancestry checks, ahead/behind counts, merge bases), beside a stamp of what those results mean — a worktrunk that disagrees with it discards the tree | `wt list`, `wt merge`, `wt remove` | +| `.git/wt/cache/{kind}/*.json` | Cached CI status, the largest PR/MR number seen (sizes the `wt list` CI column), and git command results (merge-tree, integration probes, diff stats, ancestry checks, ahead/behind counts, merge bases) | `wt list`, `wt merge`, `wt remove` | | `.git/wt/cache/summary/{branch}/{hash}.json` | Cached LLM branch summaries, content-addressed by diff hash | `wt list --full`, `wt switch` (when `[list] summary = true`) | | `.git/wt/cache/picker-preview/*.json` | Rendered preview panes for the interactive picker | `wt switch` | | `.git/wt/logs/{branch}/**/*.log` | Background hook output (nested per branch) | Hooks, background `wt remove` | @@ -209,7 +209,6 @@ A branch checked out in a second worktree is retained regardless, `-D` included. - `wt merge` / `wt step push` — the target branch's checked-out worktree is updated to the merged commits, so a file those commits delete disappears from it, and an ignored file at a path they track is overwritten — the same result a `git merge` run in that worktree would produce. Uncommitted changes at paths the merge doesn't touch stay in place, staged or not; one at a path it does touch refuses the merge upfront, naming the file - `wt remove` — also stops the removed worktree's `git fsmonitor--daemon` (under `core.fsmonitor=true`), and sweeps `.git/wt/trash/` entries older than 24 hours along with fsmonitor daemons whose worktree no longer exists -- any command reading a cache — discards the cached git-command results under `.git/wt/cache/` when they were written by a worktrunk that computed them differently, so a stale answer can't outlive the code that produced it. Cached CI status and LLM summaries are keyed on what they describe, so they are left alone - `wt config state clear` — removes all worktrunk data from `.git/` (config keys, caches, markers, hints, variables, logs, stale trash) - `wt config shell install` — replaces an existing fish or Nushell wrapper file whole, and removes one an older version installed at a previous location (fish `conf.d/wt.fish`, Nushell `/vendor/autoload/wt.nu`), printing each removal; [Files created](https://worktrunk.dev/shell-integration/#files-created) names the file each shell gets - `wt config shell uninstall` — removes the lines that run `wt config shell init` from bash/zsh/PowerShell rc files, and deletes wrapper and completion files carrying worktrunk's content markers (fish `functions/`, `conf.d/`, and `completions/`; nushell `vendor/autoload`). Every removed line is printed diff --git a/src/cache.rs b/src/cache.rs index 4a0c3b944e..87815603b5 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -29,198 +29,22 @@ //! errors so `wt config state clear` can report truthfully when it can't //! delete a file (e.g. permission denied). `NotFound` is counted as "already //! gone" so concurrent clearers don't fight each other. -//! -//! # Epoch -//! -//! A SHA-keyed entry has no TTL and no invalidation rule, so it answers for -//! whatever code wrote it until its key recurs — which for a finished branch -//! is never. That is correct while a given key means one thing, and wrong the -//! moment a release changes how the value is computed: the stale entry then -//! outranks the new code at the one call that would have corrected it. -//! `CACHE_EPOCH` is the version of that meaning. `ensure_epoch` discards the -//! kinds it governs when the stamp on disk disagrees, so a generator change is -//! one constant bump rather than a rename per affected kind. -//! `UNVERSIONED_KINDS` names the kinds it leaves alone and why each one holds. - -use std::collections::HashSet; + use std::fs; use std::path::{Path, PathBuf}; -use std::sync::{Mutex, OnceLock}; use std::time::SystemTime; use serde::{Serialize, de::DeserializeOwned}; use crate::git::Repository; -/// The meaning of the cached values, bumped whenever a change alters what any -/// of them says. -/// -/// 1 — v0.79.0 moved every diff `wt` parses onto plumbing, so display config -/// can no longer change what it reads. The porcelain reads that preceded it -/// wrote `has-added-changes` and `merge-add-probe` entries that, under -/// `diff.relative` or `submodule..ignore`, recorded a branch as having -/// no added changes when it had some. Those entries are keyed on the branch -/// and target tips, both of which sit still on a finished branch, so without -/// this stamp the first `wt remove` after the upgrade still reads the old -/// answer and deletes an unmerged branch. `diff-stats` and the diffs behind -/// the LLM prompts moved in the same change. -/// -/// The stamp records the last worktrunk to *check* a tree, not the version -/// that wrote each entry, so a v0.78.0 binary run against an already-stamped -/// tree writes porcelain answers this one will trust. Two binaries against one -/// repository is the case that costs; putting the epoch in each entry's path -/// is what would close it. -const CACHE_EPOCH: u32 = 1; - -/// Kinds the epoch leaves alone. `ci-status` and `pr-number` store the forge's -/// own answer rather than anything `wt` derives — the first under a TTL, the -/// second a ratchet under a constant key — so no change to how `wt` computes -/// things can make them wrong. -/// -/// `summary` is exempt on weaker grounds: its key is a hash of the diff, but -/// `SUMMARY_TEMPLATE` and `llm::prepare_diff`'s filtering sit downstream of -/// that hash, so rewording the prompt leaves every finished branch holding a -/// summary the old one wrote. It stays exempt because the value is -/// display-only and reaches no destructive decision, while rebuilding it is a -/// model call per branch. Closing that gap means hashing the rendered prompt, -/// not bumping this constant. -/// -/// A new kind left off this list is discarded on a bump, which is the safe -/// direction: that costs a recomputation, while wrongly exempting one costs -/// the branch the stale answer is consulted about. -const UNVERSIONED_KINDS: &[&str] = &["summary", "ci-status", "pr-number"]; - -/// Cache roots whose epoch this process has already checked. -/// -/// Keyed by path rather than held on the `Repository`, so the check covers a -/// second repository in the same process and stays inside this module. -static EPOCH_CHECKED: OnceLock>> = OnceLock::new(); - -/// The root holding every cache kind for a repository. -fn cache_root(repo: &Repository) -> PathBuf { - repo.wt_dir().join("cache") -} - -/// Discard a repository's cache tree when it was written under a different -/// [`CACHE_EPOCH`], then stamp it with the current one. -/// -/// Runs at most once per cache root per process, from [`cache_dir`] — the one -/// funnel every read and write passes through. `EPOCH_CHECKED`'s lock is held -/// across the removal, not just the bookkeeping, because `wt list` fans the -/// SHA-keyed reads over a thread pool and a worker that saw the root marked -/// mid-wipe would read the entries the wipe is there to discard. The stamp -/// goes when the cache does ([`clear_epoch`]): a cleared tree then re-stamps -/// on the next command at the cost of wiping nothing, and entries an older -/// worktrunk wrote into it meanwhile are discarded rather than trusted. -/// -/// Best-effort, like the rest of this module, and asymmetric in its failures: -/// everything under the root is regenerable, so a failed removal leaves the -/// tree unstamped for the next process to retry, while a removal that succeeds -/// and then fails to stamp costs one more wipe per process until it lands. -fn ensure_epoch(repo: &Repository) { - let root = cache_root(repo); - // Held across the removal, not just the set insert: `wt list` fans - // `has_added_changes_by_sha` out over a Rayon pool, and a worker that found - // the root already marked while the wipe was still running would read the - // entries it is there to discard. - let mut checked = EPOCH_CHECKED - .get_or_init(|| Mutex::new(HashSet::new())) - .lock() - .unwrap_or_else(|e| e.into_inner()); - if checked.contains(&root) { - return; - } - - if read_json::(&epoch_stamp(&root)) != Some(CACHE_EPOCH) { - match discard_governed_kinds(&root) { - Ok(()) => stamp_epoch(repo), - // Nothing better is available: the entries are undeletable, so - // every read for the rest of this process gets the stale tree — - // including the `has-added-changes` probe the epoch exists to - // protect. Retrying per call would repeat the failure without - // changing that. Left unstamped, so the next process tries again. - Err(e) => { - tracing::debug!(path = %root.display(), error = %e, "cache: failed to clear {} for epoch {}: {}", root.display(), CACHE_EPOCH, e); - } - } - } - checked.insert(root); -} - -/// Remove every kind directory under `root` that [`CACHE_EPOCH`] governs. -/// -/// Walks what is on disk rather than a list of known kinds, so a kind this -/// version no longer writes still goes; [`UNVERSIONED_KINDS`] is the exemption, -/// and anything else found is discarded. A missing root is nothing to do. -fn discard_governed_kinds(root: &Path) -> std::io::Result<()> { - let entries = match fs::read_dir(root) { - Ok(entries) => entries, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()), - Err(e) => return Err(e), - }; - // Accumulated with `and` rather than `?` or `try_fold`, both of which - // stop at the first error: one undeletable directory would strand the - // others, and which ones would follow `read_dir` order rather than - // anything meaningful. Every kind is attempted, the first error returned. - let mut outcome = Ok(()); - for entry in entries { - let result = entry.and_then(|entry| { - let name = entry.file_name(); - let exempt = name - .to_str() - .is_some_and(|name| UNVERSIONED_KINDS.contains(&name)); - if exempt || !entry.file_type()?.is_dir() { - return Ok(()); - } - fs::remove_dir_all(entry.path()) - }); - outcome = outcome.and(result); - } - outcome -} - -/// The stamp naming the [`CACHE_EPOCH`] a cache tree was written under. -fn epoch_stamp(root: &Path) -> PathBuf { - root.join(".epoch") -} - -/// Record a repository's cache tree as written under the current -/// [`CACHE_EPOCH`], leaving its entries alone. -/// -/// `ensure_epoch` calls this after discarding a tree from an older epoch. -/// A caller that puts entries in the tree itself — a test seeding what this -/// version would have produced — calls it beforehand, so the first read -/// doesn't sweep them as an older version's. -pub(crate) fn stamp_epoch(repo: &Repository) { - write_json(&epoch_stamp(&cache_root(repo)), &CACHE_EPOCH); -} - -/// Drop a repository's epoch stamp, as part of clearing its cache. -/// -/// The stamp describes the tree's contents, so it belongs to them: left behind -/// over an emptied tree it would vouch for whatever lands there next, -/// including entries an older worktrunk writes before this one runs again. -/// -/// Propagates like the other clear functions, so a stamp that can't be removed -/// fails the clear rather than leaving `wt config state cache clear` reporting -/// a tree it didn't finish clearing. The removal isn't counted as a cleared -/// entry — it is metadata, and its absence costs one wipe of an empty -/// directory. -pub fn clear_epoch(repo: &Repository) -> anyhow::Result<()> { - clear_one(&epoch_stamp(&cache_root(repo))).map(|_| ()) -} - /// The root directory for a named cache kind. /// /// Returns `/wt/cache//`. All worktrunk caches live /// here; the `kind` is the subdirectory name (e.g. `"ci-status"`, /// `"summary"`, `"is-ancestor"`). -/// -/// The first call per cache root in a process settles the tree's -/// `CACHE_EPOCH` (see `ensure_epoch`). pub fn cache_dir(repo: &Repository, kind: &str) -> PathBuf { - ensure_epoch(repo); - cache_root(repo).join(kind) + repo.wt_dir().join("cache").join(kind) } /// Read and deserialize a JSON cache entry. @@ -394,61 +218,8 @@ pub fn count_json_files(dir: &Path) -> usize { #[cfg(test)] mod tests { use super::*; - use crate::testing::TestRepo; use tempfile::TempDir; - /// The epoch's whole job is to answer "was this written by code that meant - /// something else". A missing stamp is only the v0.78.0 case; every bump - /// after this one turns on a stamp that disagrees, and on a matching stamp - /// leaving entries where they are. - #[test] - fn test_epoch_discards_a_tree_stamped_by_another_version() { - for (stamped, survives) in [ - (None, false), - (Some(CACHE_EPOCH + 1), false), - (Some(CACHE_EPOCH), true), - ] { - let test = TestRepo::new(); - let root = cache_root(&test.repo); - // Seeded off `cache_root`, not `cache_dir`: that call is itself the - // funnel, so seeding through it would sweep the tree first and - // land both entries in one already discarded. - let entry = root.join("has-added-changes").join("a-b.json"); - let exempt = root.join(UNVERSIONED_KINDS[0]).join("x.json"); - write_json(&entry, &true); - write_json(&exempt, &true); - - let stamp = epoch_stamp(&root); - match stamped { - Some(epoch) => write_json(&stamp, &epoch), - None => fs::remove_file(&stamp).unwrap(), - } - // `cache_dir` checks once per root per process, and `TestRepo` - // stamped this root as it built it. - EPOCH_CHECKED - .get_or_init(|| Mutex::new(HashSet::new())) - .lock() - .unwrap() - .remove(&root); - - let _ = cache_dir(&test.repo, "has-added-changes"); - assert_eq!( - read_json::(&entry).is_some(), - survives, - "stamp {stamped:?} against epoch {CACHE_EPOCH}" - ); - assert!( - read_json::(&exempt).is_some(), - "an unversioned kind survives any stamp, here {stamped:?}" - ); - assert_eq!( - read_json::(&epoch_stamp(&root)), - Some(CACHE_EPOCH), - "the tree is stamped current either way" - ); - } - } - #[derive(serde::Serialize, serde::Deserialize, PartialEq, Debug)] struct V { x: u32, diff --git a/src/commands/config/state.rs b/src/commands/config/state.rs index 6d2c141433..8cd876f1b7 100644 --- a/src/commands/config/state.rs +++ b/src/commands/config/state.rs @@ -58,14 +58,6 @@ //! it is a plain read. So a `clear` followed by the aggregate `get` shows the //! value gone, while the per-key `get` would re-resolve it. //! -//! One exception, and it is the cache's rather than this command's: every -//! cache read goes through `cache::cache_dir`, which discards entries an -//! older worktrunk wrote (`CACHE_EPOCH`). The first `get` after such an -//! upgrade therefore reports the git-command counts it left behind rather -//! than the ones it found. Nothing here can read past that without reporting -//! numbers the next command would invalidate anyway; the kinds that cost -//! something to rebuild are exempt from the discard. -//! //! # Log layout invariant //! //! Inside `wt_logs_dir()`, top-level *files* are shared logs (`commands.jsonl*`, @@ -1071,10 +1063,6 @@ pub fn handle_state_clear_all(yes: bool) -> anyhow::Result<()> { cleared_any |= clear_logs_reported(&repo)?; cleared_any |= clear_hints_reported(&repo)?; cleared_any |= clear_trash_reported(&repo)?; - // Last, as in `handle_cache_clear`: the category clears above go through - // `cache_dir`, which stamps. An emptied tree left stamped would vouch for - // whatever lands in it next. - worktrunk::cache::clear_epoch(&repo)?; if !cleared_any { eprintln!("{}", info_message("No stored state to clear")); @@ -1098,8 +1086,6 @@ pub fn handle_cache_clear() -> anyhow::Result<()> { cleared_any |= clear_summary_reported(&repo)?; cleared_any |= clear_git_commands_reported(&repo)?; cleared_any |= clear_hints_reported(&repo)?; - // Last: the category clears above go through `cache_dir`, which stamps. - worktrunk::cache::clear_epoch(&repo)?; if !cleared_any { eprintln!("{}", info_message("No cache to clear")); diff --git a/src/commands/picker/summary.rs b/src/commands/picker/summary.rs index 27ba43a937..18e8a8b97c 100644 --- a/src/commands/picker/summary.rs +++ b/src/commands/picker/summary.rs @@ -175,13 +175,10 @@ mod tests { fn test_write_handles_unwritable_path() { use crate::summary::CachedSummary; let (_t, repo) = temp_repo(); - // Block cache directory creation by placing a file where the directory - // should be. `TestRepo` stamps the cache epoch, so the directory is - // already there and has to go first. + // Block cache directory creation by placing a file where the directory should be let wt_dir = repo.wt_dir(); fs::create_dir_all(&wt_dir).unwrap(); let cache_parent = wt_dir.join("cache"); - fs::remove_dir_all(&cache_parent).unwrap(); fs::write(&cache_parent, "blocker").unwrap(); let cached = CachedSummary { diff --git a/src/summary.rs b/src/summary.rs index 0a3759a071..bde5d60404 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -14,6 +14,13 @@ //! file holds the summary for that exact diff. No TTL, no separate staleness //! check — see [`worktrunk::cache`] for the shared torn-write semantics. //! +//! The hash covers the diff, not the prompt built from it: [`SUMMARY_TEMPLATE`] +//! and [`prepare_diff`]'s filtering both sit downstream of it. Rewording the +//! prompt therefore leaves every finished branch holding the summary the old +//! one produced. Closing that means hashing the rendered prompt rather than the +//! diff; it stays open because the value is display-only and rebuilding it +//! costs a model call per branch. +//! //! # Prune on write //! //! After a successful write, the branch directory is trimmed to one entry diff --git a/src/testing/mod.rs b/src/testing/mod.rs index 547b9e5568..a5f48e11aa 100644 --- a/src/testing/mod.rs +++ b/src/testing/mod.rs @@ -1445,10 +1445,6 @@ impl TestRepo { let root = canonicalize(root).unwrap(); let repo = Repository::at(&root).unwrap(); write_local_test_config(&repo); - // A test that seeds `.git/wt/cache/` by hand means entries this - // version produced. Unstamped, the first read would discard them as an - // older epoch's (`cache::ensure_epoch`). - crate::cache::stamp_epoch(&repo); Self { temp_dir, diff --git a/tests/integration_tests/config_state.rs b/tests/integration_tests/config_state.rs index 6dab8acf80..72a51f240d 100644 --- a/tests/integration_tests/config_state.rs +++ b/tests/integration_tests/config_state.rs @@ -1505,14 +1505,6 @@ fn test_state_clear_all_comprehensive(repo: TestRepo) { !summary_feature_dir.exists(), "Summary cache dir for feature should be cleared" ); - // The command re-stamps the epoch on its way through `cache_dir`, so the - // drop has to come after the clears. A stamp left over an emptied tree - // vouches for whatever lands there next, including entries an older - // worktrunk writes before this one runs again. - assert!( - !git_dir.join("wt/cache/.epoch").exists(), - "epoch stamp should not survive over an emptied cache tree" - ); assert!(!log_dir.exists()); } @@ -1757,12 +1749,6 @@ fn test_state_cache_clear_with_entries(repo: TestRepo) { ✓ Cleared 1 git commands cache entry ✓ Cleared 1 hint "); - // Same invariant as `wt config state clear`: the emptied tree carries no - // stamp, so an older worktrunk's entries aren't trusted on the way back. - assert!( - !git_dir.join("wt/cache/.epoch").exists(), - "epoch stamp should not survive over an emptied cache tree" - ); } /// `cache clear` must leave authoritative state (markers, vars, diff --git a/tests/integration_tests/remove.rs b/tests/integration_tests/remove.rs index 2b4c1be4a2..03cb643a27 100644 --- a/tests/integration_tests/remove.rs +++ b/tests/integration_tests/remove.rs @@ -1451,67 +1451,6 @@ fn test_remove_branch_only_unmerged_from_subdirectory_with_diff_relative(repo: T ); } -/// The `has-added-changes` answer is keyed on the branch and target tips and -/// never expires, so on a finished branch it outlives the release that wrote -/// it. Releases through 0.78.0 wrote it from porcelain `git diff`, which under -/// `diff.relative` recorded a branch with changes outside the cwd as having -/// none — so moving the read to plumbing fixes nothing until that entry is -/// gone. The epoch stamp is what discards it. -#[rstest] -fn test_remove_branch_only_unmerged_ignores_cache_from_an_older_epoch(repo: TestRepo) { - repo.run_git(&["switch", "--create", "feature-unmerged"]); - fs::write(repo.root_path().join("feature.txt"), "new feature").unwrap(); - repo.run_git(&["add", "feature.txt"]); - repo.run_git(&["commit", "--message", "Add feature"]); - repo.run_git(&["checkout", "main"]); - - let sha = |rev: &str| { - String::from_utf8( - repo.git_command() - .args(["rev-parse", rev]) - .run() - .unwrap() - .stdout, - ) - .unwrap() - .trim() - .to_string() - }; - // What 0.78.0 left behind: the poisoned answer, and no epoch stamp. - let cache = repo.root_path().join(".git/wt/cache"); - fs::create_dir_all(cache.join("has-added-changes")).unwrap(); - fs::write( - cache.join("has-added-changes").join(format!( - "{}-{}.json", - sha("feature-unmerged"), - sha("main") - )), - "false", - ) - .unwrap(); - fs::remove_file(cache.join(".epoch")).unwrap(); - - let output = make_snapshot_cmd(&repo, "remove", &["feature-unmerged"], None) - .output() - .unwrap(); - - let branch = repo - .git_command() - .args([ - "rev-parse", - "--verify", - "--quiet", - "refs/heads/feature-unmerged", - ]) - .run() - .unwrap(); - assert!( - branch.status.success(), - "unmerged branch must survive a pre-epoch cache; wt remove said: {}", - String::from_utf8_lossy(&output.stderr) - ); -} - #[rstest] fn test_remove_branch_only_force_delete(repo: TestRepo) { // Create a branch with a unique commit (not in main)