From 804037caf688e59697ca5f7c700c30d78f98e7da Mon Sep 17 00:00:00 2001 From: Jim Wordelman Date: Thu, 23 Jul 2026 20:21:11 -0700 Subject: [PATCH 1/4] =?UTF-8?q?test(cairn):=20red=20=E2=80=94=20cull-candi?= =?UTF-8?q?dates=20+=20eviction=20(refs=20crn-28ge.1.7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/cull_test.go | 119 +++++++++++++++ internal/cairn/cull_test.go | 131 +++++++++++++++++ internal/cairn/evict_test.go | 278 +++++++++++++++++++++++++++++++++++ 3 files changed, 528 insertions(+) create mode 100644 cmd/cull_test.go create mode 100644 internal/cairn/cull_test.go create mode 100644 internal/cairn/evict_test.go diff --git a/cmd/cull_test.go b/cmd/cull_test.go new file mode 100644 index 0000000..29c2a24 --- /dev/null +++ b/cmd/cull_test.go @@ -0,0 +1,119 @@ +package cmd + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/quad341/cairn/internal/cairn" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// gitCommitAll stages and commits everything in dir -- the cmd package has +// no shared helper for this (only gitOutput); mirrors +// internal/cairn/freshness_test.go's package-scoped gitCommitAll, which +// cannot be reused across packages. +func gitCommitAll(t *testing.T, dir, msg string) { + t.Helper() + gitOutput(t, dir, "add", "-A") + gitOutput(t, dir, "commit", "-q", "-m", msg) +} + +// seedCommittedEntry creates and commits an entry directly to store's +// current branch -- cull-evict's precondition: both EvictDirect and +// EvictToReviewBranch operate via `git rm` on an already-tracked file, +// unlike remember's add-only commit primitives, so the fixture must be +// committed, not just written. +func seedCommittedEntry(t *testing.T, store, topic string, scope []string) *cairn.Entry { + t.Helper() + e, err := cairn.NewEntry(topic, scope, "a body", "agent:bot") + require.NoError(t, err) + require.NoError(t, e.Create(store)) + gitCommitAll(t, store, "add "+e.ID) + return e +} + +func resetCullEvictFlags(t *testing.T) { + t.Helper() + f := cullEvictCmd.Flags().Lookup("reviewer") + require.NotNil(t, f) + require.NoError(t, f.Value.Set("")) + f.Changed = false +} + +// runCullEvict executes "cairn cull-evict " against the shared +// rootCmd, stubbing gc to always succeed. Mirrors runRemember. +func runCullEvict(t *testing.T, store, id string, extraArgs ...string) error { + t.Helper() + resetCullEvictFlags(t) + t.Cleanup(func() { resetCullEvictFlags(t) }) + + stubGC(t) + args := append([]string{"cull-evict", "--store", store, id}, extraArgs...) + rootCmd.SetArgs(args) + rootCmd.SetOut(&bytes.Buffer{}) + rootCmd.SetErr(&bytes.Buffer{}) + return rootCmd.Execute() +} + +func TestCullEvictPrivateTierDeletesDirectlyAndReportsSHA(t *testing.T) { + store := t.TempDir() + gitInit(t, store) + e := seedCommittedEntry(t, store, "old-fact", []string{"agent:bot"}) + + var runErr error + stdout := captureStdout(t, func() { + runErr = runCullEvict(t, store, e.ID) + }) + require.NoError(t, runErr) + + _, statErr := os.Stat(e.BodyPath) + assert.True(t, os.IsNotExist(statErr), "a private-tier cull-evict must delete the entry file directly") + + head := strings.TrimSpace(gitOutput(t, store, "rev-parse", "HEAD")) + lines := strings.Fields(strings.TrimSpace(stdout)) + require.Len(t, lines, 1, "a private-tier cull-evict must print only the eviction commit SHA") + assert.Equal(t, head, lines[0]) + + log := strings.TrimSpace(gitOutput(t, store, "log", "--oneline")) + assert.Len(t, strings.Split(log, "\n"), 3, "exactly one new commit (the eviction) on top of init+add") +} + +func TestCullEvictSharedTierProposesReviewBranchAndDoesNotDeleteDirectly(t *testing.T) { + store := t.TempDir() + gitInit(t, store) + e := seedCommittedEntry(t, store, "old-fact", []string{"rig:web"}) + + var runErr error + stdout := captureStdout(t, func() { + runErr = runCullEvict(t, store, e.ID) + }) + require.NoError(t, runErr) + + _, statErr := os.Stat(e.BodyPath) + assert.NoError(t, statErr, "a shared-tier cull-evict must NOT delete the entry file on the store's own branch -- NFR-07") + + branch := "cull/" + e.ID + gitOutput(t, store, "rev-parse", "--verify", branch) + + rel, err := filepath.Rel(store, e.BodyPath) + require.NoError(t, err) + status := gitOutput(t, store, "diff-tree", "--no-commit-id", "--name-status", "-r", branch) + assert.Contains(t, status, "D\t"+rel, "the cull branch's commit must delete the entry file") + + lines := strings.Split(strings.TrimSpace(stdout), "\n") + require.Len(t, lines, 2, "a shared-tier cull-evict must print the review branch and the mailed reviewer -- no commit SHA") + assert.Contains(t, lines[0], branch) +} + +func TestCullEvictUnknownIDReturnsClearError(t *testing.T) { + store := t.TempDir() + gitInit(t, store) + + err := runCullEvict(t, store, "does-not-exist") + require.Error(t, err) + assert.Contains(t, err.Error(), "does-not-exist") +} diff --git a/internal/cairn/cull_test.go b/internal/cairn/cull_test.go new file mode 100644 index 0000000..2225bcb --- /dev/null +++ b/internal/cairn/cull_test.go @@ -0,0 +1,131 @@ +package cairn + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDisuseReferencePrefersLastRecalledOverCreatedAt(t *testing.T) { + ref, ok := disuseReference("2026-07-01T00:00:00Z", "2020-01-01") + require.True(t, ok) + assert.Equal(t, 2026, ref.Year()) +} + +func TestDisuseReferenceFallsBackToCreatedAtWhenNeverRecalled(t *testing.T) { + ref, ok := disuseReference("", "2020-01-01") + require.True(t, ok) + assert.Equal(t, 2020, ref.Year()) +} + +func TestDisuseReferenceNeitherFieldParses(t *testing.T) { + _, ok := disuseReference("", "") + assert.False(t, ok) +} + +// TestCullCandidatesIndependentOfFreshness covers FR-10: CULL (disuse) and +// FRESHNESS (anchor-drift, Check()) are independent signals. A synthetic +// entry that carries a populated anchor+verified_at (the shape a Fresh +// Check() result needs) but is long disused must still be reported; a +// synthetic entry with no anchor/verified_at at all (the shape an +// Unknown/Stale Check() result needs) but recently recalled must not. +// CullCandidates' own query never selects verified_at or any anchor_* +// column, so this also guards against a future change accidentally wiring +// freshness state into the disuse decision. +func TestCullCandidatesIndependentOfFreshness(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + disusedSince := time.Now().Add(-60 * 24 * time.Hour).Format(time.RFC3339) + recalledRecently := time.Now().Add(-1 * time.Hour).Format(time.RFC3339) + + writeFile(t, store, "global/fresh-but-disused.md", "+++\n"+ + "id = \"fresh-but-disused\"\n"+ + "title = \"Fresh but disused\"\n"+ + "topic_key = \"topic-a\"\n"+ + "last_recalled_at = \""+disusedSince+"\"\n"+ + "verified_at = \"2026-07-20\"\n"+ + "\n"+ + "[anchor]\n"+ + "type = \"none\"\n"+ + "+++\n"+ + "body\n") + writeFile(t, store, "global/stale-but-recent.md", "+++\n"+ + "id = \"stale-but-recent\"\n"+ + "title = \"Stale but recent\"\n"+ + "topic_key = \"topic-b\"\n"+ + "last_recalled_at = \""+recalledRecently+"\"\n"+ + "+++\n"+ + "body\n") + + findings, err := CullCandidates(ctx, store, 30*24*time.Hour) + require.NoError(t, err) + require.Len(t, findings, 1, "only the disused entry is cull-eligible, regardless of its own freshness signal") + assert.Equal(t, "fresh-but-disused", findings[0].EntryID) + assert.Equal(t, disusedSince, findings[0].DisusedSince) +} + +func TestCullCandidatesNeverRecalledFallsBackToCreatedAt(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + oldDate := time.Now().Add(-60 * 24 * time.Hour).Format(time.DateOnly) + recentDate := time.Now().Format(time.DateOnly) + + writeFile(t, store, "global/old-never-recalled.md", "+++\n"+ + "id = \"old-never-recalled\"\n"+ + "title = \"Old\"\n"+ + "created_at = \""+oldDate+"\"\n"+ + "+++\n"+ + "body\n") + writeFile(t, store, "global/new-never-recalled.md", "+++\n"+ + "id = \"new-never-recalled\"\n"+ + "title = \"New\"\n"+ + "created_at = \""+recentDate+"\"\n"+ + "+++\n"+ + "body\n") + + findings, err := CullCandidates(ctx, store, 30*24*time.Hour) + require.NoError(t, err) + require.Len(t, findings, 1, "an entry never recalled must age into cull-eligibility from its created_at date") + assert.Equal(t, "old-never-recalled", findings[0].EntryID) +} + +func TestCullCandidatesThresholdConfigurable(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + disusedFor40Days := time.Now().Add(-40 * 24 * time.Hour).Format(time.RFC3339) + writeFile(t, store, "global/a.md", "+++\nid = \"a\"\ntitle = \"A\"\nlast_recalled_at = \""+disusedFor40Days+"\"\n+++\nbody\n") + + at30, err := CullCandidates(ctx, store, 30*24*time.Hour) + require.NoError(t, err) + assert.Len(t, at30, 1, "disused 40d ago exceeds a 30d threshold") + + at60, err := CullCandidates(ctx, store, 60*24*time.Hour) + require.NoError(t, err) + assert.Empty(t, at60, "disused 40d ago does not exceed a 60d threshold") +} + +func TestCullCandidatesIncludesScopeForDownstreamTierDecision(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + old := time.Now().Add(-60 * 24 * time.Hour).Format(time.RFC3339) + writeFile(t, store, "rig/web/a.md", "+++\nid = \"a\"\ntitle = \"A\"\nscope = [\"rig:web\"]\nlast_recalled_at = \""+old+"\"\n+++\nbody\n") + + findings, err := CullCandidates(ctx, store, 30*24*time.Hour) + require.NoError(t, err) + require.Len(t, findings, 1) + assert.Equal(t, []string{"rig:web"}, findings[0].Scope, + "scope must be reported so a downstream consumer (e.g. the librarian formula) can decide direct-evict vs review-branch-propose") +} + +func TestCullCandidatesNotYetDisusedIsExcluded(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + recent := time.Now().Add(-1 * time.Hour).Format(time.RFC3339) + writeFile(t, store, "global/a.md", "+++\nid = \"a\"\ntitle = \"A\"\nlast_recalled_at = \""+recent+"\"\n+++\nbody\n") + + findings, err := CullCandidates(ctx, store, 30*24*time.Hour) + require.NoError(t, err) + assert.Empty(t, findings) +} diff --git a/internal/cairn/evict_test.go b/internal/cairn/evict_test.go new file mode 100644 index 0000000..e1f154f --- /dev/null +++ b/internal/cairn/evict_test.go @@ -0,0 +1,278 @@ +package cairn + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEntryForEvictDoesNotStampRecallTelemetry(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + writeFile(t, store, "global/a.md", "+++\nid = \"a\"\ntitle = \"A\"\n+++\nbody\n") + + e, err := EntryForEvict(ctx, store, "a") + require.NoError(t, err) + assert.Equal(t, "a", e.ID) + assert.Equal(t, 0, e.HitCount) + assert.Empty(t, e.LastRecalledAt) + + raw, err := os.ReadFile(e.BodyPath) + require.NoError(t, err) + assert.NotContains(t, string(raw), "hit_count", "EntryForEvict must not stamp recall telemetry (hit_count/last_recalled_at) as a side effect -- it would corrupt the very disuse signal CullCandidates measures") +} + +func TestEntryForEvictNotFound(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + _, err := EntryForEvict(ctx, store, "missing") + require.ErrorIs(t, err, ErrNotFound) +} + +// TestEvictDirectDeletesOnlyTheEntryFile mirrors +// TestCommitDirectCommitsOnlyTheEntryFile (remember_test.go) one operation +// over: exactly one new commit lands on the store's current branch, it +// deletes only the entry file, no branch is created, and the reported SHA +// matches the store's new HEAD. +func TestEvictDirectDeletesOnlyTheEntryFile(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + gitInit(t, store) + require.NoError(t, os.WriteFile(filepath.Join(store, "README.md"), []byte("seed\n"), 0o600)) + gitCommitAll(t, store, "seed") + + e, err := NewEntry("build-flags", []string{"agent:bot"}, "prefer feature flags over env vars", "agent:bot") + require.NoError(t, err) + require.NoError(t, e.Create(store)) + gitCommitAll(t, store, "add entry") + + branchBefore, err := gitRun(ctx, store, "branch", "--show-current") + require.NoError(t, err) + seedSHA, err := gitRun(ctx, store, "rev-parse", "HEAD") + require.NoError(t, err) + seedSHA = strings.TrimSpace(seedSHA) + + sha, err := e.EvictDirect(ctx, store) + require.NoError(t, err) + assert.NotEmpty(t, sha) + + head, err := gitRun(ctx, store, "rev-parse", "HEAD") + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(head), sha, "the returned SHA must be the store's new HEAD") + + parent, err := gitRun(ctx, store, "rev-parse", "HEAD~1") + require.NoError(t, err) + assert.Equal(t, seedSHA, strings.TrimSpace(parent), "exactly one new commit must land on top of the prior HEAD") + + rel, err := filepath.Rel(store, e.BodyPath) + require.NoError(t, err) + changed, err := gitRun(ctx, store, "diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD") + require.NoError(t, err) + assert.Equal(t, []string{rel}, strings.Fields(changed), "the eviction commit must contain only the entry file") + + _, statErr := os.Stat(e.BodyPath) + assert.True(t, os.IsNotExist(statErr), "the entry file must be gone from the working tree") + + branchAfter, err := gitRun(ctx, store, "branch", "--show-current") + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(branchBefore), strings.TrimSpace(branchAfter), "EvictDirect must not switch branches") + allBranches, err := gitRun(ctx, store, "branch", "--list") + require.NoError(t, err) + assert.Len(t, strings.Split(strings.TrimSpace(allBranches), "\n"), 1, "EvictDirect must not create a new branch") + + status, err := gitRun(ctx, store, "status", "--porcelain") + require.NoError(t, err) + assert.Empty(t, strings.TrimSpace(status), "working tree must be clean after a successful eviction commit") + + msg, err := gitRun(ctx, store, "log", "-1", "--format=%B", "HEAD") + require.NoError(t, err) + assert.Contains(t, msg, e.ID, "the commit message must name the evicted entry, for auditability (NFR-04)") +} + +// TestEvictDirectFailureLeavesEntryOnDiskAndReportsError mirrors +// TestCommitDirectFailureLeavesEntryUncommittedAndReportsError: a git +// failure surfaces as a clear error and the entry file is left untouched. +func TestEvictDirectFailureLeavesEntryOnDiskAndReportsError(t *testing.T) { + ctx := t.Context() + store := t.TempDir() // deliberately not a git repo + + e, err := NewEntry("build-flags", []string{"agent:bot"}, "body", "agent:bot") + require.NoError(t, err) + require.NoError(t, e.Create(store)) + + _, err = e.EvictDirect(ctx, store) + require.Error(t, err, "a git failure must be surfaced, not swallowed") + + got, perr := ParseEntry(e.BodyPath) + require.NoError(t, perr, "the entry file must survive a failed eviction attempt, not be rolled back or lost") + assert.Equal(t, e.ID, got.ID) +} + +// TestEvictDirectRefusesSharedTierEntry is the AC-required NFR-07 negative +// test: a direct-delete attempt on a shared-tier (non-agent:) entry must be +// refused, as a hard invariant enforced inside EvictDirect itself -- not a +// default a caller can opt out of. Deliberately uses a non-git store (unlike +// the positive-path tests above): the guard must fire before any git call at +// all, so a non-git store both proves that and keeps the test minimal. +func TestEvictDirectRefusesSharedTierEntry(t *testing.T) { + ctx := t.Context() + cases := []struct { + name string + scope []string + }{ + {"rig scope", []string{"rig:web"}}, + {"role scope", []string{"role:reviewer"}}, + {"global (empty) scope", nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + store := t.TempDir() + e, err := NewEntry("build-flags", tc.scope, "body", "agent:bot") + require.NoError(t, err) + require.NoError(t, e.Create(store)) + + _, err = e.EvictDirect(ctx, store) + require.Error(t, err, "a direct-delete attempt on a shared-tier entry must be refused (NFR-07)") + assert.Contains(t, err.Error(), "not private") + + got, perr := ParseEntry(e.BodyPath) + require.NoError(t, perr, "the entry file must remain present and unchanged when direct eviction is refused") + assert.Equal(t, e.ID, got.ID) + }) + } +} + +// TestEvictToReviewBranchDeletesOnlyTheEntryFileLeavingDefaultUntouched +// mirrors TestCommitToReviewBranchCreatesIsolatedBranchLeavingDefaultUntouched +// one tier over: a shared-tier eviction proposal lands as a deletion commit +// on its own cull/ branch, never touching the store's own checked-out +// branch, HEAD, or working tree. +func TestEvictToReviewBranchDeletesOnlyTheEntryFileLeavingDefaultUntouched(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + gitInit(t, store) + require.NoError(t, os.WriteFile(filepath.Join(store, "README.md"), []byte("seed\n"), 0o600)) + gitCommitAll(t, store, "seed") + + e, err := NewEntry("build-flags", []string{"rig:web"}, "prefer feature flags over env vars", "agent:bot") + require.NoError(t, err) + require.NoError(t, e.Create(store)) + gitCommitAll(t, store, "add entry") + + branchBefore, err := gitRun(ctx, store, "branch", "--show-current") + require.NoError(t, err) + headBefore, err := gitRun(ctx, store, "rev-parse", "HEAD") + require.NoError(t, err) + headBefore = strings.TrimSpace(headBefore) + + branch, err := e.EvictToReviewBranch(ctx, store) + require.NoError(t, err) + assert.Equal(t, "cull/"+e.ID, branch) + + branchAfter, err := gitRun(ctx, store, "branch", "--show-current") + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(branchBefore), strings.TrimSpace(branchAfter), + "EvictToReviewBranch must not switch the store's checked-out branch") + + headAfter, err := gitRun(ctx, store, "rev-parse", "HEAD") + require.NoError(t, err) + assert.Equal(t, headBefore, strings.TrimSpace(headAfter), "the default branch's tip commit must be unchanged") + + rel, err := filepath.Rel(store, e.BodyPath) + require.NoError(t, err) + changed, err := gitRun(ctx, store, "diff-tree", "--no-commit-id", "--name-only", "-r", branch) + require.NoError(t, err) + assert.Equal(t, []string{rel}, strings.Fields(changed), "the cull commit must touch only the entry file") + + nameStatus, err := gitRun(ctx, store, "diff-tree", "--no-commit-id", "--name-status", "-r", branch) + require.NoError(t, err) + assert.Contains(t, nameStatus, "D\t"+rel, "the cull commit must be a deletion, not some other change") + + parent, err := gitRun(ctx, store, "rev-parse", branch+"~1") + require.NoError(t, err) + assert.Equal(t, headBefore, strings.TrimSpace(parent), + "the cull branch must fork from the store's pre-existing HEAD, not carry unrelated history") + + msg, err := gitRun(ctx, store, "log", "-1", "--format=%B", branch) + require.NoError(t, err) + assert.Contains(t, msg, e.ID) + assert.Contains(t, msg, "rig:web") + + worktrees, err := gitRun(ctx, store, "worktree", "list") + require.NoError(t, err) + assert.Len(t, strings.Split(strings.TrimSpace(worktrees), "\n"), 1, + "the scratch cull worktree must be cleaned up, leaving only the store's own") + + _, statErr := os.Stat(e.BodyPath) + assert.NoError(t, statErr, "the entry file must still be present on the store's own working tree -- only the isolated cull branch deletes it") +} + +// TestEvictToReviewBranchFailureLeavesEntryUntouchedAndReportsError mirrors +// TestCommitToReviewBranchFailureLeavesEntryWrittenButUncommittedAndReportsError: +// a branch already named exactly what EvictToReviewBranch is about to create +// makes `git worktree add -b` fail deterministically. +func TestEvictToReviewBranchFailureLeavesEntryUntouchedAndReportsError(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + gitInit(t, store) + require.NoError(t, os.WriteFile(filepath.Join(store, "README.md"), []byte("seed\n"), 0o600)) + gitCommitAll(t, store, "seed") + + e, err := NewEntry("build-flags", []string{"rig:web"}, "body", "agent:bot") + require.NoError(t, err) + require.NoError(t, e.Create(store)) + gitCommitAll(t, store, "add entry") + + headBefore, err := gitRun(ctx, store, "rev-parse", "HEAD") + require.NoError(t, err) + + _, err = gitRun(ctx, store, "branch", "cull/"+e.ID) + require.NoError(t, err) + + _, err = e.EvictToReviewBranch(ctx, store) + require.Error(t, err, "a pre-existing branch name must surface as a clear error, not panic or silently continue") + assert.Contains(t, err.Error(), "cull/"+e.ID) + + got, perr := ParseEntry(e.BodyPath) + require.NoError(t, perr, "the entry must survive a failed eviction-proposal attempt, not be deleted or corrupted") + assert.Equal(t, e.ID, got.ID) + + headAfter, err := gitRun(ctx, store, "rev-parse", "HEAD") + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(headBefore), strings.TrimSpace(headAfter), + "a failed eviction-proposal attempt must not move the store's own HEAD") + + worktrees, err := gitRun(ctx, store, "worktree", "list") + require.NoError(t, err) + assert.Len(t, strings.Split(strings.TrimSpace(worktrees), "\n"), 1, + "a failed worktree add must not leave a stray worktree registered") +} + +// TestEvictToReviewBranchRefusesWhenProposalAlreadyPending: a second +// concurrent cull of an already-proposed entry is not an expected +// steady-state case (unlike CommitRecurrenceToReviewBranch's deliberate +// reuse of an existing remember/ branch) -- it must error, not silently +// fold into or fork past the existing proposal. +func TestEvictToReviewBranchRefusesWhenProposalAlreadyPending(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + gitInit(t, store) + require.NoError(t, os.WriteFile(filepath.Join(store, "README.md"), []byte("seed\n"), 0o600)) + gitCommitAll(t, store, "seed") + + e, err := NewEntry("build-flags", []string{"rig:web"}, "body", "agent:bot") + require.NoError(t, err) + require.NoError(t, e.Create(store)) + gitCommitAll(t, store, "add entry") + + firstBranch, err := e.EvictToReviewBranch(ctx, store) + require.NoError(t, err) + + _, err = e.EvictToReviewBranch(ctx, store) + require.Error(t, err, "a second concurrent cull proposal for the same entry must be refused") + assert.Contains(t, err.Error(), firstBranch) +} From ce0795911857dba287214d9fbc4fe5e954b2931c Mon Sep 17 00:00:00 2001 From: Jim Wordelman Date: Thu, 23 Jul 2026 20:23:24 -0700 Subject: [PATCH 2/4] =?UTF-8?q?feat(cairn):=20green=20=E2=80=94=20cull-can?= =?UTF-8?q?didates=20+=20eviction=20(refs=20crn-28ge.1.7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/cull.go | 65 ++++++++++++++++++++ cmd/reviewer.go | 38 ++++++++++++ internal/cairn/cull.go | 81 +++++++++++++++++++++++++ internal/cairn/evict.go | 131 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 315 insertions(+) create mode 100644 cmd/cull.go create mode 100644 internal/cairn/cull.go create mode 100644 internal/cairn/evict.go diff --git a/cmd/cull.go b/cmd/cull.go new file mode 100644 index 0000000..8759e16 --- /dev/null +++ b/cmd/cull.go @@ -0,0 +1,65 @@ +package cmd + +import ( + "encoding/json" + "fmt" + "time" + + "github.com/quad341/cairn/internal/cairn" + "github.com/spf13/cobra" +) + +const defaultCullDisuseAfter = 30 * 24 * time.Hour + +func init() { + cullCandidatesCmd.Flags().Duration("disuse-after", defaultCullDisuseAfter, + "disuse threshold: report entries not recalled (or, if never recalled, not created) within this long (NFR-06)") + cullEvictCmd.Flags().String("reviewer", "", + "reviewer to mail for a shared-tier (rig/role/global) eviction proposal (default: $CAIRN_REVIEWER, else a per-tier computed default)") + rootCmd.AddCommand(cullCandidatesCmd, cullEvictCmd) +} + +var cullCandidatesCmd = &cobra.Command{ + Use: "cull-candidates", + Short: "Entries disused past a threshold, JSON (read-only; FR-10/NFR-06)", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + if identityRequested(cmd) { + return fmt.Errorf("cull-candidates covers every entry and does not filter by identity; " + + "use 'cairn map' or 'cairn prime' for a scoped view") + } + disuseAfter, _ := cmd.Flags().GetDuration("disuse-after") + findings, err := cairn.CullCandidates(cmd.Context(), storePath(), disuseAfter) + if err != nil { + return err + } + if findings == nil { + findings = []cairn.CullCandidateFinding{} + } + enc := json.NewEncoder(cmd.OutOrStdout()) + enc.SetIndent("", " ") + return enc.Encode(findings) + }, +} + +var cullEvictCmd = &cobra.Command{ + Use: "cull-evict ", + Short: "Evict an entry: direct delete for private scope, review-branch proposal for shared scope (NFR-07)", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + e, err := cairn.EntryForEvict(cmd.Context(), storePath(), args[0]) + if err != nil { + return fmt.Errorf("look up %s for eviction: %w", args[0], err) + } + + if cairn.IsPrivateScope(e.Scope) { + sha, err := e.EvictDirect(cmd.Context(), storePath()) + if err != nil { + return fmt.Errorf("evict entry: %w", err) + } + fmt.Printf("%s\n", sha) + return nil + } + return requestCullReview(cmd, e) + }, +} diff --git a/cmd/reviewer.go b/cmd/reviewer.go index c932da1..c2d6cfa 100644 --- a/cmd/reviewer.go +++ b/cmd/reviewer.go @@ -132,3 +132,41 @@ func mailSend(ctx context.Context, to, subject, body string) error { } return nil } + +// requestCullReview proposes e's eviction on a review branch and mails the +// tier-appropriate reviewer -- mirrors requestReview, applied to a delete +// instead of an add. Callers must only invoke this for a scope that has +// already resolved away from the private agent tier. +func requestCullReview(cmd *cobra.Command, e *cairn.Entry) error { + tier, value := cairn.ResolvedTier(e.Scope) + + branch, err := e.EvictToReviewBranch(cmd.Context(), storePath()) + if err != nil { + return fmt.Errorf("propose shared-tier eviction on a review branch: %w", err) + } + fmt.Printf("cull review branch: %s\n", branch) + + reviewer, err := resolveReviewer(cmd, tier, value) + if err != nil { + return fmt.Errorf("entry %s proposed for eviction on branch %s, but resolving a reviewer to mail failed: %w", e.ID, branch, err) + } + if err := sendCullReviewMail(cmd.Context(), reviewer, e, branch); err != nil { + return fmt.Errorf("entry %s proposed for eviction on branch %s, but mail to reviewer %q failed: %w", e.ID, branch, reviewer, err) + } + fmt.Printf("mailed reviewer: %s\n", reviewer) + return nil +} + +// sendCullReviewMail mirrors sendReviewMail, with cull-specific wording: the +// branch deletes the entry rather than adding one, and a reviewer merges it +// with plain git (cairn review merge cannot handle a pure-deletion branch). +func sendCullReviewMail(ctx context.Context, reviewer string, e *cairn.Entry, branch string) error { + subject := fmt.Sprintf("cairn cull review: %s", e.TopicKey) + body := fmt.Sprintf( + "Cairn entry %s (topic %q, scope %s) is proposed for eviction (disuse).\n\n"+ + "Branch: %s\n\nThis branch deletes the entry's file. Merge it with plain git "+ + "(not `cairn review merge`, which does not handle a pure-deletion branch) when satisfied; "+ + "it does not auto-merge. Reject by simply not merging (or deleting the branch) to keep the entry.", + e.ID, e.TopicKey, strings.Join(e.Scope, " "), branch) + return mailSend(ctx, reviewer, subject, body) +} diff --git a/internal/cairn/cull.go b/internal/cairn/cull.go new file mode 100644 index 0000000..91ccb5a --- /dev/null +++ b/internal/cairn/cull.go @@ -0,0 +1,81 @@ +package cairn + +import ( + "context" + "time" +) + +type CullCandidateFinding struct { + EntryID string `json:"entry_id"` + TopicKey string `json:"topic_key,omitempty"` + Scope []string `json:"scope,omitempty"` + LastRecalledAt string `json:"last_recalled_at,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + DisusedSince string `json:"disused_since"` +} + +func disuseReference(lastRecalledAt, createdAt string) (t time.Time, ok bool) { + if lastRecalledAt != "" { + if parsed, err := time.Parse(time.RFC3339, lastRecalledAt); err == nil { + return parsed, true + } + } + if createdAt != "" { + if parsed, err := time.Parse(time.DateOnly, createdAt); err == nil { + return parsed, true + } + } + return time.Time{}, false +} + +// CullCandidates reports entries whose disuse (LastRecalledAt, falling back +// to CreatedAt if never recalled) exceeds disuseAfter (NFR-06). This is +// deliberately independent of FRESHNESS (anchor-drift, Check()) per FR-10: +// the query below never selects verified_at or any anchor_* column, so an +// entry's freshness state cannot leak into the disuse decision. +func CullCandidates(ctx context.Context, store string, disuseAfter time.Duration) ([]CullCandidateFinding, error) { + if err := ensureFresh(ctx, store); err != nil { + return nil, err + } + db, err := openDB(store) + if err != nil { + return nil, err + } + defer func() { _ = db.Close() }() + + tags, err := scopeTags(ctx, db) + if err != nil { + return nil, err + } + + rows, err := db.QueryContext(ctx, `SELECT id, topic_key, last_recalled_at, created_at FROM entries ORDER BY id`) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + cutoff := time.Now().Add(-disuseAfter) + var findings []CullCandidateFinding + for rows.Next() { + var id, topicKey, lastRecalledAt, createdAt string + if err := rows.Scan(&id, &topicKey, &lastRecalledAt, &createdAt); err != nil { + return nil, err + } + ref, ok := disuseReference(lastRecalledAt, createdAt) + if !ok || ref.After(cutoff) { + continue + } + findings = append(findings, CullCandidateFinding{ + EntryID: id, + TopicKey: topicKey, + Scope: tags[id], + LastRecalledAt: lastRecalledAt, + CreatedAt: createdAt, + DisusedSince: ref.Format(time.RFC3339), + }) + } + if err := rows.Err(); err != nil { + return nil, err + } + return findings, nil +} diff --git a/internal/cairn/evict.go b/internal/cairn/evict.go new file mode 100644 index 0000000..a38ead1 --- /dev/null +++ b/internal/cairn/evict.go @@ -0,0 +1,131 @@ +package cairn + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" +) + +// EntryForEvict looks up id for eviction, bypassing Find's hit_count/ +// last_recalled_at side effect: stamping recall telemetry while deciding +// whether to evict an entry would corrupt the very disuse signal +// CullCandidates measures. Mirrors cmd/remember.go's recurrenceMatch, which +// bypasses Find for the identical reason. +func EntryForEvict(ctx context.Context, store, id string) (*Entry, error) { + if err := ensureFresh(ctx, store); err != nil { + return nil, err + } + db, err := openDB(store) + if err != nil { + return nil, err + } + defer func() { _ = db.Close() }() + + bodyPath, err := findBodyPath(ctx, db, id) + if err != nil { + return nil, err + } + return ParseEntry(bodyPath) +} + +// EvictDirect deletes e's body file and commits that deletion straight to +// the store repo's current branch -- the private agent/ tier's eviction path +// (mirrors CommitDirect's add-side commit, applied to a delete). Unlike +// CommitDirect, which only documents (but does not enforce in code) its +// private-tier precondition, EvictDirect enforces IsPrivateScope itself: +// NFR-07 ("shared-tier entries are NEVER evicted directly") is a hard +// invariant, not a default a caller can bypass. +func (e *Entry) EvictDirect(ctx context.Context, store string) (string, error) { + if !IsPrivateScope(e.Scope) { + tier, _ := ResolvedTier(e.Scope) + return "", fmt.Errorf("refusing direct eviction of %s: resolved tier %q is not private (agent) -- "+ + "shared-tier entries can only be evicted via a review-branch proposal (NFR-07)", e.ID, tier) + } + rel, err := filepath.Rel(store, e.BodyPath) + if err != nil { + return "", fmt.Errorf("resolve %s relative to store %s: %w", e.BodyPath, store, err) + } + if _, err := gitRun(ctx, store, "rm", "--", rel); err != nil { + return "", fmt.Errorf("git rm %s (entry not evicted -- retry): %w", rel, err) + } + if _, err := gitRun(ctx, store, "commit", "-m", "cull: evict "+e.ID, "--", rel); err != nil { + return "", fmt.Errorf("git commit eviction of %s (removed from working tree and staged but not committed -- retry or restore with `git checkout HEAD -- %s`): %w", rel, rel, err) + } + sha, err := gitRun(ctx, store, "rev-parse", "HEAD") + if err != nil { + return "", fmt.Errorf("eviction commit succeeded but could not resolve the resulting SHA: %w", err) + } + return strings.TrimSpace(sha), nil +} + +// cullBranchName is the git branch a shared-tier eviction proposal lands on +// -- deliberately namespaced separately from reviewBranchName's remember/ +// prefix, so a pending cull proposal never collides with a pending add or +// recurrence review for the same entry ID, and stays invisible to +// ListReviewMergeBranches/ListReviewBranches (both scoped to remember/), +// which cannot handle a pure-deletion branch. +func cullBranchName(e *Entry) string { + return "cull/" + e.ID +} + +// EvictToReviewBranch proposes e's eviction on its own cull/ branch -- +// the role:/rig:/global: tiers' ONLY eviction path (NFR-07): a reviewer +// merging this branch with plain git is the actual eviction. Mirrors +// CommitToReviewBranch's isolation approach, applied to a delete. +// +// Unlike CommitRecurrenceToReviewBranch's deliberate reuse of an +// already-existing branch (an ordinary, expected case for a recurring +// topic), a second concurrent cull proposal for an entry already pending +// review is refused rather than silently folded in or forked past: a +// pending eviction proposal should be resolved (merged or rejected) before +// another is opened. +func (e *Entry) EvictToReviewBranch(ctx context.Context, store string) (string, error) { + branch := cullBranchName(e) + exists, err := reviewBranchExists(ctx, store, branch) + if err != nil { + return "", err + } + if exists { + return "", fmt.Errorf("a cull proposal is already pending for %s on branch %s", e.ID, branch) + } + msg := fmt.Sprintf("cull: evict %s\n\nscope: %s", e.ID, strings.Join(e.Scope, " ")) + if err := e.commitDeleteToReviewWorktree(ctx, store, branch, msg); err != nil { + return "", err + } + return branch, nil +} + +// commitDeleteToReviewWorktree is EvictToReviewBranch's worktree-isolation +// mechanics -- the same "throwaway git worktree, so an interrupted sequence +// can never corrupt the store's own working tree" shape as +// commitToReviewWorktree (see its doc comment), applied to a deletion: `git +// rm` in the scratch worktree instead of writing + adding e's current body +// content. +func (e *Entry) commitDeleteToReviewWorktree(ctx context.Context, store, branch, msg string) error { + rel, err := filepath.Rel(store, e.BodyPath) + if err != nil { + return fmt.Errorf("resolve entry path: %w", err) + } + + scratch, err := os.MkdirTemp("", "cairn-cull-*") + if err != nil { + return fmt.Errorf("create cull worktree scratch dir: %w", err) + } + defer func() { _ = os.RemoveAll(scratch) }() + + wt := filepath.Join(scratch, "wt") + if _, err := gitRun(ctx, store, "worktree", "add", "-b", branch, wt, "HEAD"); err != nil { + return fmt.Errorf("create cull branch %q: %w", branch, err) + } + defer func() { _, _ = gitRun(ctx, store, "worktree", "remove", "--force", wt) }() + + if _, err := gitRun(ctx, wt, "rm", "--", rel); err != nil { + return fmt.Errorf("stage eviction in cull worktree: %w", err) + } + if _, err := gitRun(ctx, wt, "commit", "-q", "-m", msg); err != nil { + return fmt.Errorf("commit eviction to cull branch: %w", err) + } + return nil +} From 5a99edc8a31a48b4c5cba1a6392a182b94e01271 Mon Sep 17 00:00:00 2001 From: Jim Wordelman Date: Thu, 23 Jul 2026 20:24:19 -0700 Subject: [PATCH 3/4] =?UTF-8?q?style(cairn):=20satisfy=20lint=20=E2=80=94?= =?UTF-8?q?=20doc=20comment=20+=20line=20length=20(refs=20crn-28ge.1.7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/cairn/cull.go | 3 +++ internal/cairn/evict.go | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/cairn/cull.go b/internal/cairn/cull.go index 91ccb5a..fe1c0f2 100644 --- a/internal/cairn/cull.go +++ b/internal/cairn/cull.go @@ -5,6 +5,9 @@ import ( "time" ) +// CullCandidateFinding is one entry disused past the configured threshold +// (FR-10/NFR-06): DisusedSince is LastRecalledAt if the entry was ever +// recalled, else CreatedAt. type CullCandidateFinding struct { EntryID string `json:"entry_id"` TopicKey string `json:"topic_key,omitempty"` diff --git a/internal/cairn/evict.go b/internal/cairn/evict.go index a38ead1..cdb5cd7 100644 --- a/internal/cairn/evict.go +++ b/internal/cairn/evict.go @@ -51,7 +51,8 @@ func (e *Entry) EvictDirect(ctx context.Context, store string) (string, error) { return "", fmt.Errorf("git rm %s (entry not evicted -- retry): %w", rel, err) } if _, err := gitRun(ctx, store, "commit", "-m", "cull: evict "+e.ID, "--", rel); err != nil { - return "", fmt.Errorf("git commit eviction of %s (removed from working tree and staged but not committed -- retry or restore with `git checkout HEAD -- %s`): %w", rel, rel, err) + return "", fmt.Errorf("git commit eviction of %s (removed from working tree and staged but not "+ + "committed -- retry or restore with `git checkout HEAD -- %s`): %w", rel, rel, err) } sha, err := gitRun(ctx, store, "rev-parse", "HEAD") if err != nil { From 07750a5d2997d6750dee3f9cc7a255c8bd48c104 Mon Sep 17 00:00:00 2001 From: Jim Wordelman Date: Fri, 24 Jul 2026 06:57:36 -0700 Subject: [PATCH 4/4] chore: release gate PASS for cairn-cull-sweep --- release-gates/cairn-cull-sweep-gate.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 release-gates/cairn-cull-sweep-gate.md diff --git a/release-gates/cairn-cull-sweep-gate.md b/release-gates/cairn-cull-sweep-gate.md new file mode 100644 index 0000000..aced57d --- /dev/null +++ b/release-gates/cairn-cull-sweep-gate.md @@ -0,0 +1,26 @@ +# Release Gate: cairn-cull-sweep + +**Bead:** crn-whno (deploy) — source review crn-28ge.1.7 (closed, PASS) +**Commit:** `5a99edc8a31a48b4c5cba1a6392a182b94e01271`, cut onto `deploy/crn-whno-gate` off `origin/main` (`9bfa28aca065bf064be6fba915c9e19354c774ea`) via cherry-pick of `dbcc32c b9a97da 0ae7941` (reviewed stack on `origin/gc-builder-769138d1bf3c`), per the bead's MERGE_POLICY note +**Date:** 2026-07-24 + +## Criteria + +| # | Criterion | Result | Evidence | +|---|-----------|--------|----------| +| 6 | Branch diverges cleanly from main | PASS | The reviewed branch (`origin/gc-builder-769138d1bf3c`) is not itself mergeable — its merge-base with `origin/main` is `b4619c7`, and it carries stale duplicate copies of crn-28ge.1.4's commits already landed on `main` under different SHAs (`eea622c`/`f0c6a46`/`eb070de` vs. the branch's `24e19d2`/`678775a`/`0d7c818`). Per the bead's MERGE_POLICY, cut `deploy/crn-whno-gate` fresh off `origin/main` (`9bfa28a`, up to date at fetch time) and cherry-picked exactly `dbcc32c b9a97da 0ae7941` onto it — zero conflicts, matching the reviewer's own dry-run. `git range-diff b4619c7..0ae7941 origin/main..HEAD` confirms all three cherry-picked commits are content-equivalent (`=`) to the reviewed originals, and that none of the stale duplicate commits (entries 1-12 in the range-diff) rode along. | +| 1 | Review PASS present, SHA-matched | PASS | crn-whno's description states "Reviewed and PASSED: crn-28ge.1.7" with full evidence (build/vet/gofmt/lint clean, tests green, dedicated OWASP pass, NFR-07 invariant verified). Reviewed commit `R` = `0ae7941`. Deployed commit `D` = `5a99edc` is not a literal git ancestor of `R` (different parent chain, per the cherry-pick above) but is **content-identical** to `R`'s three-commit stack per the range-diff `=` markers — i.e. exactly the reviewed diff, re-parented onto a corrected base, not a superseded or newer, unreviewed commit. Source review bead crn-28ge.1.7 is closed. | +| 2 | Acceptance criteria met | PASS | FR-10 (cull-candidates independent of freshness/Check()): `TestCullCandidatesIndependentOfFreshness`, `TestCullCandidatesNeverRecalledFallsBackToCreatedAt`, `TestCullCandidatesThresholdConfigurable`, `TestCullCandidatesIncludesScopeForDownstreamTierDecision`, `TestCullCandidatesNotYetDisusedIsExcluded` all pass. NFR-07 (private `agent:` scope = direct delete; shared `role:`/`rig:`/`global:` = review-branch proposal only, never direct): `TestEvictDirectDeletesOnlyTheEntryFile`, `TestEvictDirectRefusesSharedTierEntry` (table-driven over `rig_scope`/`role_scope`/`global_(empty)_scope`, all pass), `TestEvictToReviewBranchDeletesOnlyTheEntryFileLeavingDefaultUntouched`, `TestEvictToReviewBranchRefusesWhenProposalAlreadyPending` all pass. CLI wiring: `TestCullEvictPrivateTierDeletesDirectlyAndReportsSHA`, `TestCullEvictSharedTierProposesReviewBranchAndDoesNotDeleteDirectly`, `TestCullEvictUnknownIDReturnsClearError` all pass. Re-ran these targeted (`-v`) myself on the cherry-picked branch rather than trusting the reviewer's report secondhand — all pass, matching the review notes' AC-to-test mapping. | +| 3 | Tests pass | PASS | On commit `5a99edc`, run in this worktree: `go build ./...` clean, `go vet ./...` clean, `gofmt -l .` empty, `golangci-lint run ./...` — cache cleaned first (`golangci-lint cache clean`), 0 issues. `go test ./... -race -count=1`: all 5 packages ok (`cairn`, `cmd`, `formulas`, `internal/cairn`, `internal/critic`, `scripts`), zero regressions. | +| 4 | No high-severity review findings open | PASS | Review notes report a dedicated OWASP pass with no injection/traversal/access-control issues found, and no HIGH findings are recorded against crn-whno / crn-28ge.1.7. | +| 5 | Final branch clean | PASS | `git status --porcelain` empty on `deploy/crn-whno-gate` at `5a99edc` immediately before gate write. | +| 7 | Single feature theme | PASS | Diff vs. `origin/main` touches exactly the 7 files the reviewer's dry-run predicted: `cmd/cull.go`, `cmd/cull_test.go`, `cmd/reviewer.go`, `internal/cairn/cull.go`, `internal/cairn/cull_test.go`, `internal/cairn/evict.go`, `internal/cairn/evict_test.go` — one subsystem (CULL sweep: cull-candidates detection + tier-conditional eviction), one coherent feature. | + +## Verdict: PASS — proceeding to PR. + +## Note on downstream follow-on + +crn-28ge.1.10 (assigned cairn/validator) is an already-filed, independent +follow-on that re-confirms FR-10/NFR-07 test coverage once this lands on +`origin/main`. It does not complete until this deploy merges; no action +required here beyond landing normally.