diff --git a/internal/cairn/cull.go b/internal/cairn/cull.go index 24e68f6..1d486cd 100644 --- a/internal/cairn/cull.go +++ b/internal/cairn/cull.go @@ -45,7 +45,7 @@ func disuseReference(lastRecalledAt, createdAt string) (t time.Time, ok bool) { // 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 { + if err := ensureFreshBestEffort(ctx, store); err != nil { return nil, err } db, err := openDB(store) diff --git a/internal/cairn/entry.go b/internal/cairn/entry.go index 59924f0..d85ff0c 100644 --- a/internal/cairn/entry.go +++ b/internal/cairn/entry.go @@ -884,7 +884,7 @@ func IterEntriesTolerant(ctx context.Context, store string) ([]*Entry, []ParseFa // (crn-6az.6.1.3). On a hit it increments the index's hit_count and stamps // last_recalled_at (FR-4, FR-08); a miss has no side effect. func Find(ctx context.Context, store, id string) (*Entry, error) { - if err := ensureFresh(ctx, store); err != nil { + if err := ensureFreshBestEffort(ctx, store); err != nil { return nil, err } db, err := openDB(store) @@ -902,7 +902,12 @@ func Find(ctx context.Context, store, id string) (*Entry, error) { // reindex needlessly" contract for every caller, force one reindex // here before concluding id genuinely doesn't exist, so an entry // created since the last reindex is never reported missing. - if _, rerr := Reindex(ctx, store); rerr != nil { + reindexCtx, cancel := context.WithTimeout(ctx, selfHealReindexTimeout) + defer cancel() + if _, rerr := Reindex(reindexCtx, store); rerr != nil { + if errors.Is(rerr, context.DeadlineExceeded) { + return nil, ErrNotFound + } return nil, rerr } bodyPath, err = findBodyPath(ctx, db, id) @@ -1034,7 +1039,7 @@ func storeRelPath(store, p string) (string, error) { // transitively is out of this bead's scope. When more than one entry // claims to correct the same id, the most recently created one wins. func FindCorrection(ctx context.Context, store, id string) (*Entry, error) { - if err := ensureFresh(ctx, store); err != nil { + if err := ensureFreshBestEffort(ctx, store); err != nil { return nil, err } db, err := openDB(store) @@ -1489,7 +1494,7 @@ func scopeSuperset(super, sub []string) bool { // marginal query cost) -- not a precedent for extending this list on // request; any future addition needs the same analysis. func Status(ctx context.Context, store string) ([]*Entry, error) { - if err := ensureFresh(ctx, store); err != nil { + if err := ensureFreshBestEffort(ctx, store); err != nil { return nil, err } db, err := openDB(store) diff --git a/internal/cairn/index.go b/internal/cairn/index.go index 3204200..27e90f1 100644 --- a/internal/cairn/index.go +++ b/internal/cairn/index.go @@ -559,6 +559,19 @@ func ensureFresh(ctx context.Context, store string) error { return ensureFreshWith(ctx, store, Reindex) } +// ensureFreshBestEffort degrades gracefully: on the same bounded overrun, +// it swallows DeadlineExceeded and returns nil, letting the caller proceed +// against the lagging index that triggered the overrun. budget_exceeded is +// already logged by ensureFreshWith's existing obslog.IndexDriftFields +// (internal/obslog) -- no new telemetry needed. +func ensureFreshBestEffort(ctx context.Context, store string) error { + err := ensureFreshWith(ctx, store, Reindex) + if errors.Is(err, context.DeadlineExceeded) { + return nil + } + return err +} + // selfHealReindexTimeout bounds ensureFreshWith's synchronous self-heal // reindex when the index merely lags HEAD (staleBehindHEAD): without it, a // writer holding the entries lock forces the caller through retryOnBusy's diff --git a/internal/cairn/index_test.go b/internal/cairn/index_test.go index 586522a..fa8f463 100644 --- a/internal/cairn/index_test.go +++ b/internal/cairn/index_test.go @@ -1091,6 +1091,159 @@ func TestEnsureFreshBoundsSelfHealReindexWhenStaleBehindHEAD(t *testing.T) { "the bounded budget must still give the reindex a real window rather than failing near-instantly (got %s)", elapsed) } +// TestFindReturnsLaggingIndexWhenBoundedSelfHealOverruns pins crn-pz5l9: +// when the bounded self-heal reindex overruns for a best-effort caller, +// Find must serve the lagging (but valid) index instead of propagating +// DeadlineExceeded -- the usable fallback ensureFreshBestEffort exists for. +func TestFindReturnsLaggingIndexWhenBoundedSelfHealOverruns(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(store, "global"), 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(store, "global", "a.md"), + []byte("+++\nid = \"a\"\ntitle = \"A\"\n+++\nbody\n"), 0o600)) + gitInit(t, store) + gitCommitAll(t, store, "init") + + _, err := Reindex(ctx, store) + require.NoError(t, err) + + // Move HEAD past the watermark without reindexing -- the + // stale-behind-HEAD case ensureFreshBestEffort bounds. + require.NoError(t, os.WriteFile(filepath.Join(store, "global", "b.md"), + []byte("+++\nid = \"b\"\ntitle = \"B\"\n+++\nbody\n"), 0o600)) + gitCommitAll(t, store, "add b") + + db, err := sql.Open("sqlite", IndexPath(store)+"?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)&_txlock=immediate") + require.NoError(t, err) + defer func() { _ = db.Close() }() + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + _, err = tx.ExecContext(ctx, `INSERT INTO entries (id,title,body_path) VALUES ('lock','lock','lock')`) + require.NoError(t, err) + + const lockHold = 15 * time.Second + go func() { + time.Sleep(lockHold) + _ = tx.Rollback() + }() + + start := time.Now() + e, err := Find(ctx, store, "a") + elapsed := time.Since(start) + + require.NoError(t, err, "a best-effort caller must serve the lagging index on overrun, not fail") + require.NotNil(t, e) + assert.Equal(t, "a", e.ID) + // No upper bound on elapsed here, unlike the sibling overrun tests: on a + // hit, Find's own hit_count/last_recalled_at stamp (entry.go, "Find's + // only write") still needs the same write lock this fixture holds, under + // retryOnBusy's separate and much wider budget -- so total elapsed + // legitimately tracks lockHold, not the ~6-11s self-heal window. That + // window is what's actually under test here, and it's pinned directly by + // TestEnsureFreshBoundsSelfHealReindexWhenStaleBehindHEAD and (without + // this confound, since EntryByID never reaches a write) by + // TestEntryByIDStillReturnsDeadlineExceededWhenBoundedSelfHealOverruns. + assert.GreaterOrEqualf(t, elapsed, 4*time.Second, + "must actually attempt the bounded reindex, not skip it (got %s)", elapsed) +} + +// TestEntryByIDStillReturnsDeadlineExceededWhenBoundedSelfHealOverruns is a +// regression guard for crn-pz5l9: EntryByID stays on strict ensureFresh -- +// the split must not soften the pre-mutation gate cull-evict, promote-mark, +// and anchor-attach all rely on. +func TestEntryByIDStillReturnsDeadlineExceededWhenBoundedSelfHealOverruns(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(store, "global"), 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(store, "global", "a.md"), + []byte("+++\nid = \"a\"\ntitle = \"A\"\n+++\nbody\n"), 0o600)) + gitInit(t, store) + gitCommitAll(t, store, "init") + + _, err := Reindex(ctx, store) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(filepath.Join(store, "global", "b.md"), + []byte("+++\nid = \"b\"\ntitle = \"B\"\n+++\nbody\n"), 0o600)) + gitCommitAll(t, store, "add b") + + db, err := sql.Open("sqlite", IndexPath(store)+"?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)&_txlock=immediate") + require.NoError(t, err) + defer func() { _ = db.Close() }() + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + _, err = tx.ExecContext(ctx, `INSERT INTO entries (id,title,body_path) VALUES ('lock','lock','lock')`) + require.NoError(t, err) + + const lockHold = 15 * time.Second + go func() { + time.Sleep(lockHold) + _ = tx.Rollback() + }() + + start := time.Now() + e, err := EntryByID(ctx, store, "a") + elapsed := time.Since(start) + + require.Error(t, err, "the strict mutation gate must still fail on overrun") + assert.ErrorIs(t, err, context.DeadlineExceeded) + assert.Nil(t, e) + assert.Lessf(t, elapsed, 11*time.Second, + "must resolve on the bounded budget (got %s)", elapsed) + assert.GreaterOrEqualf(t, elapsed, 4*time.Second, + "must actually attempt the bounded reindex (got %s)", elapsed) +} + +// TestFindMissingIDBoundedUnderOverrunReturnsErrNotFound pins crn-pz5l9 +// Change 3: Find's ErrNotFound force-reindex fallback (entry.go, previously +// unbounded) must resolve within the same ~6-11s self-heal budget and +// report ErrNotFound, not stall for retryOnBusy's ~33s worst case. +func TestFindMissingIDBoundedUnderOverrunReturnsErrNotFound(t *testing.T) { + ctx := t.Context() + store := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(store, "global"), 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(store, "global", "a.md"), + []byte("+++\nid = \"a\"\ntitle = \"A\"\n+++\nbody\n"), 0o600)) + gitInit(t, store) + gitCommitAll(t, store, "init") + + _, err := Reindex(ctx, store) + require.NoError(t, err) + + // Index matches HEAD -- unlike the two tests above, HEAD does not move + // again here. This isolates Find's ErrNotFound force-reindex fallback + // from ensureFreshBestEffort's own staleBehindHEAD bound: the outer + // ensureFresh check must see a fresh index and return immediately, so + // the only lock contention the lookup hits is inside the fallback + // itself. + db, err := sql.Open("sqlite", IndexPath(store)+"?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)&_txlock=immediate") + require.NoError(t, err) + defer func() { _ = db.Close() }() + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + _, err = tx.ExecContext(ctx, `INSERT INTO entries (id,title,body_path) VALUES ('lock','lock','lock')`) + require.NoError(t, err) + + const lockHold = 15 * time.Second + go func() { + time.Sleep(lockHold) + _ = tx.Rollback() + }() + + start := time.Now() + e, err := Find(ctx, store, "nonexistent") + elapsed := time.Since(start) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrNotFound, + "an unconfirmable-within-budget miss must look like a real miss to the caller: %v", err) + assert.Nil(t, e) + assert.Lessf(t, elapsed, 11*time.Second, + "the force-reindex fallback must be bounded, not retryOnBusy's ~33s worst case (got %s)", elapsed) + assert.GreaterOrEqualf(t, elapsed, 4*time.Second, + "must actually attempt the bounded reindex (got %s)", elapsed) +} + // TestEnsureFreshDoesNotBoundColdStoreSelfHealReindex pins the other half of // crn-f0rb7: a store with no watermark row yet (a first-ever index build, not // a body edit that's merely raced HEAD) must NOT be subject to the new short diff --git a/internal/cairn/recall.go b/internal/cairn/recall.go index b623d91..ee64184 100644 --- a/internal/cairn/recall.go +++ b/internal/cairn/recall.go @@ -65,7 +65,7 @@ type entryRecallRow struct { // re-deriving its own and silently drifting). Like Status, it reads index // columns only -- no body parse, no hit_count/last_recalled_at side effect. func loadEntryRecallRows(ctx context.Context, store string) ([]entryRecallRow, error) { - if err := ensureFresh(ctx, store); err != nil { + if err := ensureFreshBestEffort(ctx, store); err != nil { return nil, err } db, err := openDB(store) diff --git a/internal/cairn/search.go b/internal/cairn/search.go index 91c4694..8705b2f 100644 --- a/internal/cairn/search.go +++ b/internal/cairn/search.go @@ -682,7 +682,7 @@ func buildFTSQuery(terms []string) string { // forcing one rebuild is what makes search work on those stores without // requiring an operator to run `cairn reindex` by hand. func ensureSearchIndex(ctx context.Context, store string) error { - if err := ensureFresh(ctx, store); err != nil { + if err := ensureFreshBestEffort(ctx, store); err != nil { return err } present, err := searchIndexPresent(ctx, store) diff --git a/release-gates/crn-tk8qd-gate.md b/release-gates/crn-tk8qd-gate.md new file mode 100644 index 0000000..52e14a7 --- /dev/null +++ b/release-gates/crn-tk8qd-gate.md @@ -0,0 +1,58 @@ +# Release Gate: crn-tk8qd + +**Bead:** crn-tk8qd (deploy) — source: crn-kpf7m (review), build: crn-pz5l9.1 +**Commit:** `4a76402a68196a6ce40e3475610e9e50653710c2`, cut onto `deploy/crn-tk8qd-gate` +**Date:** 2026-08-21 + +## Background + +Splits `internal/cairn`'s `ensureFresh` into a strict variant (unchanged) and a +new `ensureFreshBestEffort`, per architecture bead crn-pz5l9 (design in +crn-pz5l9.1). `ensureFreshBestEffort` swallows `context.DeadlineExceeded` from +the bounded self-heal reindex and serves the (lagging) index instead of +propagating the error — applied at the 6 read-path call sites (`Find`, +`FindCorrection`, `Status`, `Search`, `Recall`, `CullCandidates`). +`EntryByID` (`evict.go`), the sole pre-mutation gate for cull-evict, +promote-mark and anchor-attach, deliberately stays on the strict path. +Separately, `Find`'s `ErrNotFound` force-reindex fallback (previously +unbounded) is now wrapped in the same `selfHealReindexTimeout` used elsewhere, +returning `ErrNotFound` (not `DeadlineExceeded`) on overrun. + +## Criteria + +| # | Criterion | Result | Evidence | +|---|-----------|--------|----------| +| 1 | Review PASS for exact deployed commit | PASS | crn-kpf7m (closed, close_reason=pass) notes' `tdd_green:` and `deploy_commit:` both cite `4a76402a68196a6ce40e3475610e9e50653710c2`, byte-identical to crn-tk8qd's own `**Commit:**` field (D = R). Independently re-resolved via `git rev-parse --verify --quiet "^{commit}"` — a real commit object, not a transcribed string. | +| 2 | Acceptance criteria met | PASS | Independently re-verified against crn-pz5l9.1's done-when list, not just the reviewer's notes: `grep -n "ensureFreshBestEffort("` finds exactly 6 call sites (entry.go:887,1042,1497; search.go:685; recall.go:68; cull.go:48); `evict.go:19` still calls strict `ensureFresh(` (not the best-effort variant); `entry.go`'s `Find` force-reindex fallback is wrapped in `context.WithTimeout(ctx, selfHealReindexTimeout)` and returns `ErrNotFound` on overrun (entry.go:905-909); all 3 required tests present by exact name in index_test.go (`TestFindReturnsLaggingIndexWhenBoundedSelfHealOverruns`, `TestEntryByIDStillReturnsDeadlineExceededWhenBoundedSelfHealOverruns`, `TestFindMissingIDBoundedUnderOverrunReturnsErrNotFound`). | +| 3 | Tests pass | PASS | `go build ./...` clean, then `go test ./... -race -count=1` on `deploy/crn-tk8qd-gate` (=4a76402a): all 7 packages `ok` (root, cmd, formulas, internal/cairn, internal/critic, internal/obslog, scripts) — zero FAIL, zero SKIP lines in output (skips/fails print even without `-v`). Diff-owned test file is `internal/cairn/index_test.go`; the 3 diff-owned tests above are covered by internal/cairn's clean `ok`, no skip. | +| 4 | No open HIGH findings | PASS | crn-kpf7m: `style_findings: none` (gofmt/vet/golangci-lint all clean), `security_findings: none` (full OWASP walk, all N/A or independently verified — access-control point specifically re-checked: `EntryByID` unchanged, exactly 3 callers). | +| 5 | Clean tree | PASS | `deploy/crn-tk8qd-gate` cut directly from `4a76402a...^{commit}` via `resolve_deploy_branch_target`; `git status --porcelain` empty before the gate-file commit. | +| 6 | Clean divergence from main | PASS | `git merge-base --is-ancestor origin/main 4a76402a` → true: `origin/main` is a direct ancestor of the deploy commit (2 commits ahead: RED d971af3f, GREEN 4a76402a). Trivial fast-forward, no self-rebase needed. | +| 7 | Single feature theme | PASS | Single bead, no `rollup-ship` label. All 6 changed files (`cull.go`, `entry.go`, `index.go`, `index_test.go`, `recall.go`, `search.go`) sit under `internal/cairn/` — one coherent change (ensureFresh strict/best-effort split + bounded Find fallback) end to end. | + +## Verdict: PASS — proceeding to PR. + +## Process notes + +1. **Merge authority.** Per this session's role instructions: arm + `gh pr merge --auto` on the PR opened below (no strategy flag), then verify. + A declined arm (expected once the PR is already MERGEABLE/CLEAN) routes to + mayor as a `MERGE-REQUEST` — the deployer never self-merges on quad341/cairn. + + **Observation, not acted on here:** `release-gates/crn-6b1g8-gate.md` + (previous deploy, 2026-08-20) records that session self-merging its PR + directly (`gh pr merge --squash`) under a claimed "standing self-merge + authorization," citing the memory key `cairn-auto-merge-requires-explicit-strategy` + as authorizing it. This session's role prompt cites the *same* memory key + as the thing that settled the opposite (self-merge retired 2026-07-28, + merge authority is mayor's alone, citing crn-54c/crn-6yc). Flagging this + contradiction to mayor separately rather than adjudicating it here. + +2. **Branch target:** `builder/crn-pz5l9.1` is provenance-only per the deploy + bead's own instruction. `deploy/crn-tk8qd-gate` was cut fresh from the + exact reviewed SHA via `resolve_deploy_branch_target`; `assert_safe_push_target` + confirmed the derived name does not match the shared-worktree-branch + signature. + +3. **SHA integrity:** the deploy commit was independently re-resolved via + `git rev-parse --verify --quiet "^{commit}"` before any gate step ran.