From 539f20e251a8883a16c8fa539c196b476d98fcab Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 20 Sep 2026 15:08:38 -0700 Subject: [PATCH 1/7] Revert the cache epoch; put the remediation in the changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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..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) --- CHANGELOG.md | 2 +- CLAUDE.md | 2 +- docs/src/content/docs/faq.md | 3 +- .../skills/worktrunk/reference/faq.md | 3 +- skills/worktrunk/reference/faq.md | 3 +- src/cache.rs | 233 +----------------- src/commands/config/state.rs | 14 -- src/commands/picker/summary.rs | 5 +- src/testing/mod.rs | 4 - tests/integration_tests/config_state.rs | 14 -- tests/integration_tests/remove.rs | 61 ----- 11 files changed, 8 insertions(+), 336 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bb5b151f8..34ad026e9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ - **`wt merge` on a dirty worktree describes what it commits**: the generated message covered the branch's earlier commits and said nothing about the work the merge had just staged into the same commit. `wt step squash --dry-run` now stages into a temp index so its preview covers what the run would commit, as `wt step commit --dry-run` does. ([#4178](https://github.com/max-sixty/worktrunk/pull/4178)) -- **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 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. The integration check caches its answer keyed on the branch and target tips, with no expiry, so an answer an earlier version got wrong is still on disk — if either setting is in your git config, run `wt config state cache clear` once to drop it. ([#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..aafa708164 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 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. Nothing invalidates these on an upgrade, and that's the decision rather than an omission: a cache version — a stamp, or a version segment in the path — is permanent machinery for a transition that has come up once, so the remediation goes in the changelog instead. An entry that changes what a cached read reports says what the old answers get wrong and that `wt config state cache clear` drops them. Ask whenever a generator behind one of `sha_cache`'s kinds changes, not only for code inside `cache.rs`. ## 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/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) From 6912c1b95d088c8611a8896ca81885dac044448f Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 20 Sep 2026 16:00:48 -0700 Subject: [PATCH 2/7] Fold in the review: name .gitmodules, per-repository, and keep the summary caveat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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..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) --- CHANGELOG.md | 4 +++- CLAUDE.md | 2 +- src/summary.rs | 7 +++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3952ad27b..4f8740afa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,9 @@ ### 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. The integration check caches its answer keyed on the branch and target tips, with no expiry, so an answer an earlier version got wrong is still on disk — if either setting is in your git config, run `wt config state cache clear` once to drop it. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147)) +- **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. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147)) + +- **Cached integration answers from before 0.79.0 can still be wrong**: the check keys on the branch and target tips with no expiry, so an answer an earlier version computed under `diff.relative` or `submodule..ignore` is still on disk. Run `wt config state cache clear` in each repository where either applies — `submodule..ignore` counts whether it comes from your git config or the repository's `.gitmodules`. ([#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 aafa708164..171fd19a3f 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 leaves the old answers in place.** 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. Nothing invalidates these on an upgrade, and that's the decision rather than an omission: a cache version — a stamp, or a version segment in the path — is permanent machinery for a transition that has come up once, so the remediation goes in the changelog instead. An entry that changes what a cached read reports says what the old answers get wrong and that `wt config state cache clear` drops them. Ask whenever a generator behind one of `sha_cache`'s kinds changes, not only for code inside `cache.rs`. +**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 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. Nothing invalidates these on an upgrade, and that's the decision rather than an omission: a cache version — a stamp, or a version segment in the path — is permanent machinery for a transition that has come up once, so the remediation goes in the changelog instead. A change that alters what a cached read reports owes its changelog entry a line naming what the old answers get wrong and telling the reader that `wt config state cache clear` drops them. Ask whether you owe that line whenever a generator behind one of `sha_cache`'s kinds changes, not only for code inside `cache.rs`. ## Releases 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 From 7967eb79d67c6624d0869043815d3ebfd1088ada Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 20 Sep 2026 16:06:58 -0700 Subject: [PATCH 3/7] Fold the cache advisory back into the entry it belongs to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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..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) --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f8740afa9..9dde155281 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +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. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147)) - -- **Cached integration answers from before 0.79.0 can still be wrong**: the check keys on the branch and target tips with no expiry, so an answer an earlier version computed under `diff.relative` or `submodule..ignore` is still on disk. Run `wt config state cache clear` in each repository where either applies — `submodule..ignore` counts whether it comes from your git config or the repository's `.gitmodules`. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147)) +- **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` run from a subdirectory the changes sat outside, and `submodule..ignore` (from git config or `.gitmodules`) on a submodule bump. Every diff Worktrunk parses now runs plumbing, which ignores that configuration. Answers cached before this release don't expire, so run `wt config state cache clear` in each repository where either setting applies. ([#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)) From adfca9943c70e31e767b67f8087d8dfdac3dc97e Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 20 Sep 2026 16:10:55 -0700 Subject: [PATCH 4/7] Say the bug was a corner case, and make the release skill require it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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..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) --- .claude/skills/release/SKILL.md | 5 +++++ CHANGELOG.md | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 2aa2c951d5..f1cb21d8c6 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -150,6 +150,11 @@ awk '/^## /{if (f) exit; f=1} f' CHANGELOG.md \ **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 9dde155281..b2dc75f05d 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` run from a subdirectory the changes sat outside, and `submodule..ignore` (from git config or `.gitmodules`) on a submodule bump. Every diff Worktrunk parses now runs plumbing, which ignores that configuration. Answers cached before this release don't expire, so run `wt config state cache clear` in each repository where either setting applies. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147)) +- **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 the branch's only change. Every diff Worktrunk parses now runs plumbing, which ignores that configuration. Answers cached before this release don't expire, so run `wt config state cache clear` if either case applied to you. ([#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)) From b0ee1511de27215e701d81e66ff2c47c96f85648 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 20 Sep 2026 16:13:51 -0700 Subject: [PATCH 5/7] Keep the changelog descriptive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .claude/skills/release/SKILL.md | 2 ++ CHANGELOG.md | 2 +- CLAUDE.md | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index f1cb21d8c6..625e93f5cf 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -145,6 +145,8 @@ 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; they don't instruct.** An entry records what the old behavior was and what it is now. Leave out remediation steps and "run X if Y" advice — that belongs in the PR description, alongside the reasoning. + **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]`." diff --git a/CHANGELOG.md b/CHANGELOG.md index b2dc75f05d..5904f44df6 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 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 the branch's only change. Every diff Worktrunk parses now runs plumbing, which ignores that configuration. Answers cached before this release don't expire, so run `wt config state cache clear` if either case applied to you. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147)) +- **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 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 171fd19a3f..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 leaves the old answers in place.** 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. Nothing invalidates these on an upgrade, and that's the decision rather than an omission: a cache version — a stamp, or a version segment in the path — is permanent machinery for a transition that has come up once, so the remediation goes in the changelog instead. A change that alters what a cached read reports owes its changelog entry a line naming what the old answers get wrong and telling the reader that `wt config state cache clear` drops them. Ask whether you owe that line whenever a generator behind one of `sha_cache`'s kinds changes, not only for code inside `cache.rs`. +**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 From 30e937c5c00d94acd337a4d6c1474f236d06ac5f Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 20 Sep 2026 16:26:12 -0700 Subject: [PATCH 6/7] Draw the changelog rule at delivery versus advice "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) --- .claude/skills/release/SKILL.md | 2 +- CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 625e93f5cf..8510bac464 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -145,7 +145,7 @@ 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; they don't instruct.** An entry records what the old behavior was and what it is now. Leave out remediation steps and "run X if Y" advice — that belongs in the PR description, alongside the reasoning. +**Entries describe; they don't advise.** An entry records what the old behavior was and what it is now. When the change can't reach anyone using the feature without a step of theirs — a static wrapper that has to be reinstalled — that step is part of what changed, so name it in one clause. Remediation for a corner case and precautions are advice, and belong 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. diff --git a/CHANGELOG.md b/CHANGELOG.md index 5904f44df6..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 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 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)) +- **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)) From 22c87dbfc3f27a26b406ce86fcbaa4df77561b68 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 20 Sep 2026 16:29:39 -0700 Subject: [PATCH 7/7] Changelog entries describe; imperatives only for widespread material 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) --- .claude/skills/release/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 8510bac464..999cce396f 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -145,7 +145,7 @@ 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; they don't advise.** An entry records what the old behavior was and what it is now. When the change can't reach anyone using the feature without a step of theirs — a static wrapper that has to be reinstalled — that step is part of what changed, so name it in one clause. Remediation for a corner case and precautions are advice, and belong in the PR description. +**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.