From cea4ff184ebc9edcebea71810c1322668945af30 Mon Sep 17 00:00:00 2001 From: JeePee Date: Fri, 14 Aug 2026 08:54:33 +0200 Subject: [PATCH] fix(distill): only prune catalog entries whose origin project is visible here Reconcile guards against pruning blind by checking that ProjectsDir exists, but ~/.claude/projects exists on every machine running Claude Code, so that check says nothing about any individual project. Each entry then falls through to os.ReadFile on //memory/; on a workstation that has never opened that project the path is legitimately absent, ErrNotExist is read as "the user deleted it", and the entry file is removed. Because the catalog lives inside the sync work-tree, the daemon's `git add -A` then propagates that deletion to every other machine -- so running `distill --prune` on a second workstation silently destroys entries distilled on the first. Skip entries whose origin project directory is not present, and count them. `claude-memsync distill` reports the count, so a conservative prune reads as a deliberate decision rather than as the command doing nothing. The cost is that an entry whose project directory the user really did delete now stays until removed by hand. That asymmetry is deliberate: a missed prune is an annoyance, a wrong prune is data loss across every machine. The existing TestReconcilePrunesStaleEntries still passes unchanged -- a source deleted under a project that *is* present is still pruned -- and a new test covers prune and skip happening in the same run, so the fix is selective rather than a blanket disable. --- cmd/claude-memsync/distill.go | 4 ++++ docs/distilling-memories.md | 11 ++++++++- internal/distill/distill.go | 34 ++++++++++++++++++++++------ internal/distill/distill_test.go | 39 ++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 8 deletions(-) diff --git a/cmd/claude-memsync/distill.go b/cmd/claude-memsync/distill.go index 30b4eca..1e2c2cd 100644 --- a/cmd/claude-memsync/distill.go +++ b/cmd/claude-memsync/distill.go @@ -72,6 +72,10 @@ func report(res distill.Result, dryRun bool) { if res.Pruned > 0 { fmt.Printf("pruned %d stale %s\n", res.Pruned, plural(res.Pruned, "entry", "entries")) } + if res.Skipped > 0 { + fmt.Printf("kept %d %s whose origin project isn't on this machine (can't judge from here)\n", + res.Skipped, plural(res.Skipped, "entry", "entries")) + } if len(res.Pending) > 0 { fmt.Printf("\n%d marked %s awaiting distillation (run /distill to generalize):\n", len(res.Pending), plural(len(res.Pending), "memory", "memories")) diff --git a/docs/distilling-memories.md b/docs/distilling-memories.md index 710aa14..60c62f4 100644 --- a/docs/distilling-memories.md +++ b/docs/distilling-memories.md @@ -146,6 +146,14 @@ claude-memsync distill --dry-run # show what would change, write nothing claude-memsync distill --prune # also remove entries whose source is gone ``` +`--prune` only judges entries whose **origin project exists on the machine you +run it from**. The catalog is shared, so an entry distilled on one workstation +from a repo you have never opened here has a legitimately absent source — +pruning it would push that deletion to all your machines. Those entries are +reported as `kept … (can't judge from here)` and left alone. To retire one, +either run `--prune` on the machine that owns the source project, or delete the +`.md` by hand. + The CLI prints: - how many entries are in the catalog, @@ -191,7 +199,8 @@ to know what belongs. them into one generalized entry. - **A distilled lesson no longer applies.** Delete its `.md` from the catalog (or remove the `scope: environment` tag from the source and run - `claude-memsync distill --prune`). + `claude-memsync distill --prune` **on the machine holding that source + project** — prune skips entries it cannot verify from where it is run). - **`DISTILLED.md` looks stale.** It's a derived file, regenerated locally; run `claude-memsync distill` to rebuild it. It is intentionally not synced (each PC regenerates its own to avoid merge conflicts on the generated table). diff --git a/internal/distill/distill.go b/internal/distill/distill.go index ceeb205..2c2f445 100644 --- a/internal/distill/distill.go +++ b/internal/distill/distill.go @@ -95,6 +95,7 @@ type Conflict struct { type Result struct { Indexed int // catalog entries written to the index Pruned int // stale catalog entries removed (Reconcile) + Skipped int // entries left alone because their origin project isn't on this machine Pending []Origin // marked source memories with no catalog entry yet Conflicts []Conflict // same name, divergent content across sources } @@ -103,19 +104,19 @@ type Result struct { // entry point used by `claude-memsync distill`. func Run(opts Options, prune bool) (Result, error) { opts.applyDefaults() - var pruned int + var pruned, skipped int if prune { r, err := Reconcile(opts) if err != nil { return Result{}, err } - pruned = r.Pruned + pruned, skipped = r.Pruned, r.Skipped } res, err := BuildIndex(opts) if err != nil { return res, err } - res.Pruned = pruned + res.Pruned, res.Skipped = pruned, skipped return res, nil } @@ -161,9 +162,22 @@ func analyze(opts Options, entries []Entry) Result { } // Reconcile removes catalog entries whose originating memory no longer carries -// the marker or no longer exists. It is conservative: if the projects tree is -// not visible at all, it prunes nothing (avoids wiping the catalog on a machine -// that only consumes it). +// the marker or no longer exists. It is conservative in two ways, because the +// catalog is shared: a wrong prune deletes the entry file, and the daemon's +// `git add -A` then propagates that deletion to every other workstation. +// +// - If the projects tree is not visible at all, it prunes nothing (a machine +// that only consumes the catalog must not wipe it). +// - If an entry's *origin project* is not present on this machine, it is +// skipped rather than pruned. ~/.claude/projects exists on every machine +// running Claude Code, so the tree-level check above says nothing about any +// individual project: an entry distilled on another workstation, from a repo +// never opened here, has a legitimately absent source. Reading that as "the +// user deleted it" would destroy the entry globally. +// +// The cost of this is that an entry whose project directory the user really did +// delete stays in the catalog until removed by hand. That asymmetry is +// deliberate — a missed prune is an annoyance, a wrong prune is data loss. func Reconcile(opts Options) (Result, error) { opts.applyDefaults() var res Result @@ -180,7 +194,13 @@ func Reconcile(opts Options) (Result, error) { if e.OriginProject == "" || e.OriginFile == "" { continue // hand-authored entry with no source; leave it alone } - src := filepath.Join(opts.ProjectsDir, e.OriginProject, "memory", e.OriginFile) + memDir := filepath.Join(opts.ProjectsDir, e.OriginProject, "memory") + if _, err := os.Stat(memDir); err != nil { + // Origin project absent on this workstation — unjudgeable, not stale. + res.Skipped++ + continue + } + src := filepath.Join(memDir, e.OriginFile) content, err := os.ReadFile(src) stale := false switch { diff --git a/internal/distill/distill_test.go b/internal/distill/distill_test.go index 92eada4..5fdfce6 100644 --- a/internal/distill/distill_test.go +++ b/internal/distill/distill_test.go @@ -203,6 +203,45 @@ func TestReconcilePrunesStaleEntries(t *testing.T) { } } +// The catalog is shared across workstations, so an entry distilled on PC1 from +// a repo that PC2 has never opened has a legitimately absent source on PC2. +// Pruning it there would push the deletion back through git and destroy the +// entry everywhere. Reconcile must prune only what it can actually judge. +func TestReconcileSkipsEntriesFromProjectsNotOnThisMachine(t *testing.T) { + dir := t.TempDir() + projects := filepath.Join(dir, "projects") + distilled := filepath.Join(dir, "distilled") + + // Source under a project we have, still marked -> kept. + writeFile(t, filepath.Join(projects, "P1", "memory", "live.md"), sourceMemory("live", true, "x")) + writeFile(t, filepath.Join(distilled, "live.md"), catalogEntry("live", "d", "feedback", "P1", "live.md", "x")) + // Source under a project we have, marker removed -> genuinely stale, prune. + writeFile(t, filepath.Join(projects, "P1", "memory", "untagged.md"), sourceMemory("untagged", false, "y")) + writeFile(t, filepath.Join(distilled, "untagged.md"), catalogEntry("untagged", "d", "feedback", "P1", "untagged.md", "y")) + // Source under a project this machine has never opened -> unjudgeable, skip. + writeFile(t, filepath.Join(distilled, "from-pc2.md"), catalogEntry("from-pc2", "d", "feedback", "P2", "remote.md", "z")) + + res, err := Reconcile(Options{DistilledDir: distilled, ProjectsDir: projects}) + if err != nil { + t.Fatal(err) + } + if res.Pruned != 1 { + t.Errorf("Pruned = %d, want 1 (only the locally-untagged source)", res.Pruned) + } + if res.Skipped != 1 { + t.Errorf("Skipped = %d, want 1 (the entry from another workstation)", res.Skipped) + } + if _, err := os.Stat(filepath.Join(distilled, "from-pc2.md")); err != nil { + t.Errorf("entry whose origin project is absent here must survive: %v", err) + } + if _, err := os.Stat(filepath.Join(distilled, "live.md")); err != nil { + t.Errorf("live.md should have survived: %v", err) + } + if _, err := os.Stat(filepath.Join(distilled, "untagged.md")); !os.IsNotExist(err) { + t.Errorf("untagged.md should have been pruned") + } +} + func TestReconcileNeverPrunesBlindWhenSourcesInvisible(t *testing.T) { dir := t.TempDir() distilled := filepath.Join(dir, "distilled")