Merge pull request 'ci: trigger the catalog job deliberately, pin agent docs to one file' (#1) from chore/workflow-guardrails into main

Reviewed-on: #1

.gitea/workflows/ci.yml

@@ -9,9 +9,23 @@ name: CI
 # before being written here, so every step below is a transcription of
 # something observed working rather than something expected to.
 
+# **A branch push and its PR are the same commit, and testing it twice
+# costs the only runner there is.**  `branches: ['**']` here meant every
+# PR booked four runs — `check` and `e2e` for the branch push, then both
+# again for `refs/pull/N/head` — on a host with capacity 1, where the
+# queue is shared with an index build that can hold it for three hours.
+#
+# `pull_request` covers feature branches, and `main` is kept because a
+# post-merge run is the record of the trunk's health.  Since main now
+# refuses direct pushes, that run happens exactly once per merge.
+#
+# The trade is explicit: a branch pushed with **no** PR open gets no CI.
+# That is consistent with the workflow this repo committed to — every
+# change goes through a PR — and the signal returns the moment one is
+# opened, on the same commit.
 on:
   push:
-    branches: ['**']
+    branches: [main]
   pull_request:
   workflow_dispatch:
 

.gitea/workflows/index-artifact.yml

@@ -7,31 +7,29 @@ name: Search index maintenance
 #   import older than 6mo -> rebuild (re-import from the newest dump)
 #   otherwise             -> refresh (fold in new incremental listens)
 #
-# A refresh is cheap and no-ops when nothing new has been published, so
-# running it on every push to main is safe.
+# **There is deliberately no `push` trigger, and restoring one is a
+# decision rather than a cleanup.** A refresh is individually cheap, so
+# running it on every push to main looked free; what it actually does is
+# put an unattended job that mutates the only copy of a ~205 GB catalog
+# on the same trigger as an ordinary code change, on a runner with
+# capacity 1.
 #
-# ---------------------------------------------------------------------
-# TEMPORARY (2026-08-17): the `push` trigger is off while the catalog
-# rebuilds.
+# That is not hypothetical. On 2026-08-17 `fix(database): retire a table
+# whose shape the schema moved past` landed on main, green — the CI
+# database is deliberately in the older encoding, so the stale-shape
+# repair judged its `explore_index` stale and dropped it, and this job
+# fell back to a full import from the dumps. `fix(database): never
+# retire the catalog the index build derives` stops that specific repair
+# and cannot undo it. Every push to main then booked another `budget`
+# (3h) of the one runner while ordinary CI queued behind it.
 #
-# `fix(database): retire a table whose shape the schema moved past`
-# dropped this job's `explore_index` on its first run -- the CI database
-# is deliberately in the older encoding, so the repair judged it stale --
-# and the job fell back to a full ~205 GB import from the dumps.
-# `fix(database): never retire the catalog the index build derives`
-# stops it happening again but cannot undo it.
-#
-# Until that import reports complete, every push to main books another
-# `budget` (3h) of a runner with capacity 1, and ordinary CI queues
-# behind it. The weekly cron and workflow_dispatch still resume the
-# build, which is all it needs: indexbuild picks up from its checkpoint.
-#
-# RESTORE the two `push` lines below once a run reports
-# `complete=true`. Nothing else here changed.
-# ---------------------------------------------------------------------
+# So the rule this file is an instance of: **a job that mutates state
+# which cannot be rebuilt in ten minutes is triggered deliberately, not
+# by a push.** The weekly cron keeps the catalog current, and
+# workflow_dispatch resumes or forces a build — indexbuild picks up from
+# its checkpoint either way, so nothing is lost by not running on every
+# merge. See docs/index-cache.md for the snapshot and the restore.
 on:
-  # push:
-  #   branches: [main]
   schedule:
     # Weekly update pass. The 6-month rebuild is triggered by the same
     # command when it notices the import has aged out.
@@ -53,6 +51,10 @@ on:
 
 # Runs share one persistent working directory, so they must not overlap.
 # A push landing mid-build waits rather than corrupting the checkpoint.
+#
+# That directory holds the only copy of a catalog nothing can cheaply
+# re-derive: see docs/index-cache.md for the snapshot it takes and the
+# restore, which is minutes against the hours a rebuild costs.
 concurrency:
   group: search-index
   cancel-in-progress: false

.planning/NOTES.md

@@ -3404,3 +3404,44 @@ Three things worth keeping from it:
   rebuilding. That is the right default, and it means the next schema
   change touching `explore_index` needs a deliberate plan for this one
   database rather than none.
+
+## Two guards for the index cache, and what each one is worth (2026-08-17)
+
+Both come out of the incident above, and they protect different halves
+of it.
+
+**`TestNoCacheTableIsRetiredHere` asserts the outcome, not the
+mechanism.** The test that shipped with the fix pins one table in one
+wrong shape, which is the failure that happened; what actually cost the
+rebuild was a destructive repair added at `database.NewDB` — the
+chokepoint every binary here shares — without asking which binary it was
+in. The next one will have a different name and a different reason. So
+this puts *every* `datamap` Cache table into a shape the schema has
+moved past, opens the database the way `cmd/indexbuild` does, and
+requires all of them to still be there.
+
+Three things it got right by being written this way. The table list is
+`datamap.ByKind(Cache)`, so the two credit tables added the same day
+were covered without anyone adding them — flipping the policy back fails
+on **five** tables including `artist_credit_part` and
+`artist_credit_ref`, where the single-table test fails on one. It
+asserts rows survive as well as the table, because SQLite does an
+implicit DELETE before a DROP and a repair that recreated the table
+would otherwise look identical. And it *accepts* an error from `NewDB`,
+because that is the trade the fix documents: loud failure instead of a
+silent day of downloading.
+
+**`scripts/index-cache-snapshot.sh` covers the half no test can.** The
+volume held the only copy of a catalog whose rebuild is hours of someone
+else's bandwidth. `VACUUM INTO` rather than `cp`, because a byte copy of
+a live SQLite file is a corrupt file of plausible size; the staging
+directory is deliberately not copied, since a build resumes without it;
+and the snapshot is reopened and asked for its catalog row count before
+any rotation happens. Both failure paths were exercised rather than
+argued: a corrupt source and an empty catalog each exit non-zero, delete
+their own output, and leave the previous snapshots in place.
+
+`docs/index-cache.md` is the restore procedure, and the number that
+makes it worth having: a restored snapshot resolves to `refresh` and
+folds in the incremental listens since — minutes, against the 3–23 h a
+rebuild was estimating.

AGENTS.md

@@ -0,0 +1 @@
+CLAUDE.md

CLAUDE.md

@@ -263,6 +263,29 @@ rather than renaming them.
     plan 013's stated "delete and rescan" — and `Derived` from Owned.
     A table the schema no longer describes at all goes too; 013 left
     seven behind plus `schema_migrations`.
+  - **Whether a stale `Cache` table may be rebuilt is a build tag**, and
+    it is the most expensive thing in this file to get wrong. In the app
+    the catalog is *downloaded*, so a wrong shape costs a minute of
+    re-fetching the artifact and keeping it costs every Explore read. In
+    `cmd/indexbuild` the catalog is *derived*, and the only way back is
+    the ~205 GB dump stream the `/cache` volume exists to avoid — so
+    `retireStaleCache` is false there (`staleshape_policy_indexbuild.go`)
+    and `TestTheCatalogSurvivesAStaleShape` fails the moment it is not.
+    `TestNoCacheTableIsRetiredHere` is the same assertion made of *every*
+    `datamap` Cache table rather than one, because the risk is not that
+    shape recurring — it is the next destructive repair added to
+    `database.NewDB`, the chokepoint every binary here shares, without
+    asking which binary it is in.
+    This is written down because it already happened: the repair shipped
+    without the distinction and dropped the real CI catalog on its first
+    run, with `reason="column entity_type is TEXT, schema declares
+    INTEGER"`. The mismatch was genuine — that database is deliberately
+    kept in the older encoding, which `fix(indexexport): read an index
+    older than the binary` exists to tolerate — so it would have been
+    dropped on *every* run. The consequence is that a future
+    `explore_index` column fails the index job loudly on `applySchema`
+    rather than silently costing it a rebuild, which is the trade a
+    human should get to make.
   - **The drops are one transaction with `defer_foreign_keys`.** Those
     legacy tables reference each other, so dropping them in any order
     fails on whichever goes first, and turning foreign keys *off*
@@ -553,6 +576,56 @@ selected as a literal `0`. Adding the column to the importer's SELECT
 list without that is how a published artifact — which nobody can re-cut
 retroactively — starts failing with `no such column`.
 
+**A credit is ordered parts, and the string is derived from them.** A
+track credited to several artists had exactly one navigable artist and
+the rest were punctuation: `primaryArtist()` string-parses the credit,
+strips a " feat. " clause and discards the guest, and deliberately does
+not split on `&`, `with` or `,` because those live inside real artist
+names ("Simon & Garfunkel"). Measured on a real 26,069-file library,
+**13%** of recordings are multi-artist upstream while only **0.86%** of
+files carry a structured multi-artist tag — mp3 carries *zero* files
+with multiple `MUSICBRAINZ_ARTISTID` across 19,840 — so this cannot be
+a tag-parsing feature. (The "3 credits of 2,823" figure that justified
+plan 013's removal of the credit tables measured our own *writer*:
+`cachedLinkArtist` ran once per credit, so a collaboration could never
+have been recorded. Dropping the join table was still right on cost.)
+
+`artist_credit_part` / `artist_credit_ref` carry the decomposition for
+multi-artist credits only — a single-artist credit is already
+`explore_index`'s own `artist_name`, and storing those would triple the
+table to say nothing. Five things about it are load-bearing:
+
+- **Join phrases are assembly instructions, not disassembly ones.**
+  `creditLink` concatenates parts, so link boundaries are known by
+  construction. Locating a `credited_name` *inside* the stored credit
+  string would reintroduce the fault this exists to fix: that string may
+  come from the file's tags while the parts come from the catalog, and
+  the two disagree for ~1 in 3 multi-artist credits (`'Skrillex feat.
+  Swae Lee'` tagged against `'Skrillex & Swae Lee'` upstream).
+- **`credited_name` is stored per row**, never joined from `artists`:
+  MusicBrainz credits "Snoop Dogg" on a track by the artist called
+  "Snoop Doggy Dogg". Display follows the credit, navigation the MBID.
+- **The lookup is keyed on the recording MBID**, which the catalog and a
+  local file both carry (`library.Track.RecordingMBID`), so one binding
+  serves Explore and the library's own lists — which is why this needed
+  no local table. `file_artists` remains the offline-resilience step and
+  is deliberately *not* declared until something writes it.
+- **Absence is cached as an answer.** `credit-store.ts` stores `[]` for
+  a single-artist credit — *asked*, not *answered* — or the ~87% that
+  have nothing to decompose are re-requested on every render forever.
+  `request()` is per-row and coalesces into one call per frame, because
+  a virtualized list cannot hand over "the whole list": 50,000 rows is
+  100 queries for the ~30 on screen.
+- **The dump is a third source, and it had to be.** The canonical dump
+  CI already streams has no join phrases and no as-credited names, and
+  the JSON dumps cover 153,691 recordings of ~35M with *zero* overlap
+  against a real library. So `mbdump.tar.bz2` — 7.1 GB, ~13.7 min in
+  pure-Go bzip2, whose members are alphabetical, which is what lets one
+  pass resolve an entity's credit without buffering 35M recordings. The
+  pass runs on **every** mode, because a complete import means
+  `refresh`, which never enters the importer at all, and it reports
+  whether it populated anything so `changed` republishes the artifact.
+
 **A 0.6 GB download asks about the connection first.** `explore`'s
 catalog artifact had no network awareness at all, which on a phone is a
 month's data allowance spent without being asked (plan 016 B4).

Makefile

@@ -179,10 +179,12 @@ bindings-check: ## Fail if the generated bindings are stale
 css-check: ## Fail if a css`` literal was ended early by a backtick in a comment
 	@cd frontend && node scripts/check-css-literals.mjs
 
-# .pi/ documents commands, and a skill that documents a command wrongly
-# is worse than no skill: an agent runs it confidently.  Every command
-# in there is a make target on purpose, so this is checkable.
-skill-check: ## Fail if .pi/ documents a make target that does not exist
+# .pi/ and CLAUDE.md document commands, and a doc that documents a
+# command wrongly is worse than no doc: an agent runs it confidently.
+# Every command in them is a make target on purpose, so this is
+# checkable.  It also asserts AGENTS.md is a symlink to CLAUDE.md, so the
+# two harnesses cannot drift onto two descriptions of one project.
+skill-check: ## Fail if the agent docs name a missing make target, or AGENTS.md is not a symlink
 	@./scripts/skill-check.sh
 
 # Conventional Commits, which CLAUDE.md claimed CI enforced for a long

backend/download/service_test.go

@@ -20,6 +20,30 @@ func newServiceFixture(t *testing.T) serviceFixture {
 	mf := newManagerFixture(t)
 	svc := NewService(slogDiscard(), mf.manager, mf.store, NewMemSecretStore())
 
+	// Every test here is about the durable Request that `StartDownload`
+	// leaves behind, and none of them is about the download itself -- but
+	// their fixture is an anchored four-track request with a healthy
+	// provider, which is exactly what `AutoPickable` says yes to. So
+	// `Manager.Start` was firing `go m.grab(...)`, detached and with
+	// `context.WithoutCancel`, and the test then raced it.
+	//
+	// It lost, twice, in CI (`check` on c03c0b8, and nowhere locally):
+	//
+	//	service_test.go:66: state = "satisfied", want wanted
+	//	testing.go:1369: TempDir RemoveAll cleanup: ... directory not empty
+	//
+	// The first is the request reaching its *next* state before the
+	// assertion read it; the second is that same goroutine still writing
+	// into `t.TempDir()` after the test returned. One cause, two shapes.
+	//
+	// Putting the candidate outside the auto-pick size window stops the
+	// grab from ever starting, which is better than waiting for it: there
+	// is no goroutine to be slow, so the tests state what they mean
+	// ("the request exists, in this state") without a timing assumption
+	// underneath. A test that does want the download has `managerFixture`
+	// and sets its own preferences.
+	mf.manager.SetPreferences(AutoDownloadPrefs{MaxSizeMB: 1})
+
 	return serviceFixture{managerFixture: mf, svc: svc}
 }
 

cmd/indexbuild/staleschema_test.go

@@ -12,6 +12,7 @@ import (
 	_ "modernc.org/sqlite"
 
 	"yellowjacket/backend/database"
+	"yellowjacket/backend/datamap"
 	"yellowjacket/backend/system"
 )
 
@@ -214,3 +215,97 @@ func TestTheCatalogSurvivesAStaleShape(t *testing.T) {
 		)
 	}
 }
+
+// TestNoCacheTableIsRetiredHere is the general form of the accident
+// above, and it exists because the specific one is not the risk.
+//
+// `TestTheCatalogSurvivesAStaleShape` pins one table in one wrong shape,
+// which is the failure that happened. What cost the ~205 GB was not that
+// shape: it was a destructive repair added to `database.NewDB` -- the
+// one chokepoint every binary in this project shares -- without asking
+// which binary it was running in. The next such repair will have a
+// different name and a different reason, and this database still cannot
+// afford it.
+//
+// So the assertion is about the *outcome* rather than the mechanism: put
+// every Cache table in a shape the schema has certainly moved past, open
+// the database the way cmd/indexbuild does, and require that all of them
+// are still there afterwards. Any future repair that drops one fails
+// here regardless of how it decides to.
+//
+// Two things about it are deliberate.
+//
+// The table list comes from `datamap.ByKind(Cache)` rather than being
+// written out, so a Cache table added next year is covered by this test
+// on the day it is added -- the same reason `TestCatalogCoversSchema`
+// reads the schema instead of a list.
+//
+// And `NewDB` returning an error is *accepted*, because that is the
+// trade the fix documents: with Cache tables no longer rebuilt here, a
+// shape the schema moved past now fails this job loudly instead of
+// silently costing it a day of downloading. Loud is fine. Gone is not.
+func TestNoCacheTableIsRetiredHere(t *testing.T) {
+	logger := slog.New(slog.DiscardHandler)
+
+	t.Setenv("YJ_HOME", t.TempDir())
+
+	dataDir, err := system.GetUserDataDirPath()
+	if err != nil {
+		t.Fatalf("resolve data dir: %v", err)
+	}
+
+	dbPath := filepath.Join(dataDir, "yj.db")
+
+	if _, err := database.NewDB(logger); err != nil {
+		t.Fatalf("first open: %v", err)
+	}
+
+	// An FTS table is four shadow tables and cannot be given a "wrong
+	// shape" meaningfully; the repair skips them for the same reason and
+	// retires them with their parent, which the parents below cover.
+	var cache []string
+
+	for _, table := range datamap.ByKind(datamap.Cache) {
+		if table.FTS {
+			continue
+		}
+
+		cache = append(cache, table.Name)
+	}
+
+	if len(cache) == 0 {
+		t.Fatal("no Cache tables to check: the datamap or this test is wrong")
+	}
+
+	for _, name := range cache {
+		// A shape nothing in the current schema describes. What matters
+		// is only that it disagrees; the real mismatch was one column's
+		// type.
+		exec(t, dbPath, `
+			DROP TABLE IF EXISTS `+name+`;
+			CREATE TABLE `+name+` (id INTEGER PRIMARY KEY, moved_past TEXT);
+			INSERT INTO `+name+` (moved_past) VALUES ('irreplaceable');
+		`)
+	}
+
+	// The error is not the assertion: see the note above.
+	_, _ = database.NewDB(logger)
+
+	for _, name := range cache {
+		rows := count(t, dbPath,
+			`SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = '`+name+`'`)
+		if rows == 0 {
+			t.Errorf("%s was retired: in this database a Cache table is derived, "+
+				"not downloaded, and dropping one costs the ~205 GB dump stream", name)
+
+			continue
+		}
+
+		// Present but emptied is the same loss wearing a different
+		// shape: SQLite does an implicit DELETE before a DROP, and a
+		// repair that recreated the table would look identical here.
+		if n := count(t, dbPath, `SELECT count(*) FROM `+name); n == 0 {
+			t.Errorf("%s survived but was emptied", name)
+		}
+	}
+}

docs/index-cache.md

@@ -0,0 +1,99 @@
+# The index cache, and why it has a snapshot
+
+`/srv/yellowjacket/index-cache` on the Gitea host is the `YJ_HOME` the
+search-index job keeps between runs — `.gitea/workflows/index-artifact.yml`
+mounts it at `/cache`. It holds the catalog every user eventually
+downloads, and it is the one database in this project that is
+**derived rather than downloaded**.
+
+That is the whole reason this document exists. An install with a broken
+catalog re-fetches the ~0.6 GB artifact and is fine in a minute. This
+database *is* what that artifact is cut from, so its only route back is
+re-streaming the MetaBrainz dumps: hours, at a rate that belongs to
+someone else's server, holding a runner of capacity 1 the entire time.
+
+## What happened on 2026-08-17
+
+A schema repair (`fix(database): retire a table whose shape the schema
+moved past`) dropped every table whose live shape disagreed with the
+schema, before `applySchema`. Correct for the app. Applied here it
+deleted the catalog 19 seconds into the first run:
+
+```
+retiring a table ... table=explore_index
+  reason="column entity_type is TEXT, schema declares INTEGER"
+index maintenance mode=build reason="no completed import yet"
+```
+
+The mismatch was real and deliberate: this database is kept in the older
+text encoding, which `artifactStoresText` and `sourceColumns` exist to
+tolerate. So it would have been judged stale on *every* run.
+
+Two things came out of it. `retireStaleCache` is now a build tag —
+false under `indexbuild`, true in the app — and
+`TestNoCacheTableIsRetiredHere` asserts the outcome rather than the
+mechanism, so the next destructive repair fails a test instead of a
+production volume. And the volume got the snapshot it should always have
+had, below.
+
+## Taking snapshots
+
+```sh
+scripts/index-cache-snapshot.sh [SOURCE_HOME] [DEST_DIR] [KEEP]
+```
+
+Defaults: `/srv/yellowjacket/index-cache`, `/srv/yellowjacket/index-snapshots`,
+keep 2. On the Gitea host, daily and away from the Monday 04:00 build:
+
+```
+30 5 * * * /path/to/index-cache-snapshot.sh >> /var/log/yj-index-snapshot.log 2>&1
+```
+
+Three properties worth knowing before trusting it:
+
+- **It uses `VACUUM INTO`, not `cp`.** The database may be open, and a
+  byte copy of a live SQLite file is a corrupt file of plausible size.
+  `VACUUM INTO` takes a read lock and writes a consistent, compacted
+  copy; it is safe to run while a build is in progress.
+- **It does not copy `data/explore-staging`.** That is a resumable
+  checkpoint of work in flight — large, constantly changing, and a build
+  resumes without it. What cannot be cheaply re-derived is the finished
+  catalog, which is in the database.
+- **It verifies before it rotates.** Each snapshot is reopened and asked
+  for its catalog row count; a run that produces an unreadable or empty
+  file fails loudly, deletes its own output, and leaves the previous
+  snapshots alone. Both paths are exercised, not assumed.
+
+## Restoring
+
+Stop anything that might be using the volume first — the job holds it
+for the length of a build, and the concurrency group (`search-index`)
+means a queued run will start the moment one ends.
+
+```sh
+cd /srv/yellowjacket
+mv index-cache/data/yj.db index-cache/data/yj.db.broken   # keep it until you are sure
+cp index-snapshots/yj-index-<stamp>.db index-cache/data/yj.db
+chown --reference=index-cache/data/yj.db.broken index-cache/data/yj.db
+```
+
+Then dispatch the workflow with `mode=auto`. A restored snapshot is
+older than the dumps, so `indexbuild` resolves to `refresh` and folds in
+the incremental listens since — which is minutes, not hours.
+
+Two notes on what a restore does *not* need. The staging directory can
+be deleted; it will be rebuilt if a build is needed. And the published
+artifact is untouched by any of this: users keep downloading the last
+good one until a run reports `complete=true` and `changed=true`
+republishes.
+
+## The trade this leaves open
+
+With Cache tables no longer retired under `indexbuild`, a future
+`explore_index` column change will fail this job **loudly** — at
+`applySchema`, or at the first query naming the column — rather than
+silently rebuilding. That is the right default: loud is recoverable and
+a silent day of downloading is not. It does mean the next schema change
+touching `explore_index` needs a deliberate plan for this one database:
+take a snapshot, apply the change to a copy, or accept a rebuild
+knowingly.

scripts/index-cache-snapshot.sh

@@ -0,0 +1,89 @@
+#!/bin/sh
+# Snapshot the index build's database, which is the only copy of it.
+#
+# `/srv/yellowjacket/index-cache` is the `YJ_HOME` the index job keeps
+# between runs (`.gitea/workflows/index-artifact.yml` mounts it at
+# `/cache`).  Its catalog is *derived*, not downloaded: the only way to
+# rebuild it is to re-stream the MetaBrainz dumps, which is hours at a
+# rate that is someone else's to decide.  On 2026-08-17 a schema repair
+# dropped it and cost exactly that.
+#
+# So it gets a snapshot, and this is the script a cron on that host runs.
+# It is deliberately not part of the workflow: a backup that only exists
+# while the thing it protects is being modified is not a backup.
+#
+# Usage (on the Gitea host):
+#
+#   scripts/index-cache-snapshot.sh [SOURCE_HOME] [DEST_DIR] [KEEP]
+#
+#   SOURCE_HOME  default /srv/yellowjacket/index-cache
+#   DEST_DIR     default /srv/yellowjacket/index-snapshots
+#   KEEP         how many to retain, default 2
+#
+# Suggested cron — daily, and nowhere near the Monday 04:00 build:
+#
+#   30 5 * * * /path/to/index-cache-snapshot.sh >> /var/log/yj-index-snapshot.log 2>&1
+#
+# Three things about it are load-bearing.
+#
+# **`VACUUM INTO`, not `cp`.** The database may be open, and a byte copy
+# of a live SQLite file is a corrupt file with a plausible size.
+# `VACUUM INTO` takes a read lock, writes a consistent compacted copy,
+# and is safe while the index job is running — it costs the snapshot's
+# own write, not the source's availability.
+#
+# **The staging directory is not copied.** `/cache/data/explore-staging`
+# is a resumable checkpoint of work in flight; it is large, it changes
+# constantly, and a build resumes without it. What cannot be re-derived
+# cheaply is the finished catalog, which is in the database.
+#
+# **A snapshot that is not verified is a belief.** Each one is opened
+# and asked for its catalog row count before the old ones are rotated
+# out, so a run that produced an unreadable file leaves the previous
+# good snapshot in place and fails loudly.
+set -eu
+
+SOURCE_HOME="${1:-/srv/yellowjacket/index-cache}"
+DEST_DIR="${2:-/srv/yellowjacket/index-snapshots}"
+KEEP="${3:-2}"
+
+DB="$SOURCE_HOME/data/yj.db"
+STAMP=$(date +%Y%m%d-%H%M%S)
+OUT="$DEST_DIR/yj-index-$STAMP.db"
+
+die() { echo "index-snapshot: $*" >&2; exit 1; }
+
+command -v sqlite3 >/dev/null 2>&1 || die "sqlite3 is not installed"
+[ -f "$DB" ] || die "no database at $DB (is SOURCE_HOME right?)"
+
+mkdir -p "$DEST_DIR"
+
+# Headroom: the copy is at most the size of the source, usually less
+# (VACUUM compacts).  Refusing here beats a half-written snapshot.
+need_kb=$(du -k "$DB" | cut -f1)
+free_kb=$(df -Pk "$DEST_DIR" | awk 'NR == 2 { print $4 }')
+[ "$free_kb" -gt "$need_kb" ] || die "not enough space in $DEST_DIR (need ~${need_kb}K, have ${free_kb}K)"
+
+# A failed snapshot must leave nothing behind. `VACUUM INTO` refuses an
+# existing file, so a partial one from a disk-full write would block
+# every later run -- and worse, rotation counts files by name, so it
+# would eventually be kept *instead of* a good one.
+cleanup() { [ -n "${KEPT:-}" ] || rm -f "$OUT"; }
+trap cleanup EXIT
+
+echo "index-snapshot: $DB -> $OUT"
+sqlite3 "$DB" "VACUUM INTO '$OUT'" || die "VACUUM INTO failed"
+
+rows=$(sqlite3 "$OUT" "SELECT count(*) FROM explore_index" 2>/dev/null) \
+	|| die "snapshot is unreadable: keeping the previous ones"
+[ "${rows:-0}" -gt 0 ] || die "snapshot has an empty catalog: keeping the previous ones"
+
+KEPT=1
+
+echo "index-snapshot: ok, $rows catalog rows, $(du -h "$OUT" | cut -f1)"
+
+# Rotate only after the new one has been verified.
+ls -1t "$DEST_DIR"/yj-index-*.db 2>/dev/null | tail -n +"$((KEEP + 1))" | while read -r old; do
+	echo "index-snapshot: removing $old"
+	rm -f "$old"
+done

scripts/skill-check.sh

@@ -1,17 +1,48 @@
 #!/usr/bin/env bash
 #
-# Every command in .pi/ is a `make` target on purpose: the Makefile is
-# the source of truth for *how* to invoke something, and the skill only
-# decides *which* and *in what order*.  This check keeps that honest —
-# a renamed or deleted target turns into a failing commit rather than
-# into an agent confidently running a command that no longer exists.
+# Every command in the agent-facing docs is a `make` target on purpose:
+# the Makefile is the source of truth for *how* to invoke something, and
+# the docs only decide *which* and *in what order*.  This check keeps
+# that honest — a renamed or deleted target turns into a failing commit
+# rather than into an agent confidently running a command that no longer
+# exists.
 #
-# It extracts every `make <target>` mentioned under .pi/ and asserts the
-# target exists.  Usage:  scripts/skill-check.sh
+# It checks two things.  Usage:  scripts/skill-check.sh
+#
+# **Every `make <target>` named in an agent-facing doc exists.**  The
+# scanned set is `.pi/` *and* CLAUDE.md, which is the half that was
+# missing: CLAUDE.md names 27 targets and nothing verified one of them,
+# so the file the agents trust most was the file least checked.
+#
+# **AGENTS.md is a symlink to CLAUDE.md.**  This repo is worked on by
+# two agent harnesses that read different files by convention — Claude
+# Code reads CLAUDE.md, others read AGENTS.md — and two harnesses
+# reading two descriptions of one project is how they come to hold
+# different beliefs about it.  A symlink makes that impossible by
+# construction; a *copy* would pass every other check in this repo while
+# silently drifting, which is exactly the failure being prevented, so
+# the symlink itself is asserted rather than its contents compared.
 set -euo pipefail
 
 cd "$(dirname "$0")/.."
 
+# The symlink half runs even without .pi/, since it is not about .pi/.
+if [ -e AGENTS.md ] || [ -L AGENTS.md ]; then
+	if [ ! -L AGENTS.md ]; then
+		echo "skill-check: AGENTS.md is a regular file, not a symlink to CLAUDE.md." >&2
+		echo "  Two harnesses would read two descriptions of one project." >&2
+		echo "  Fix:  rm AGENTS.md && ln -s CLAUDE.md AGENTS.md" >&2
+		exit 1
+	fi
+
+	target="$(readlink AGENTS.md)"
+
+	if [ "$target" != "CLAUDE.md" ]; then
+		echo "skill-check: AGENTS.md points at '$target', expected CLAUDE.md." >&2
+		exit 1
+	fi
+fi
+
 [ -d .pi ] || exit 0
 
 # `make -pq` prints the database including every rule, without running
@@ -21,11 +52,37 @@ targets="$({ make -pqRr 2>/dev/null || true; } |
 	awk '/^[a-zA-Z0-9][^$#\/\t=]*:([^=]|$)/ {sub(/:.*/, "", $0); print}' |
 	sort -u)"
 
-# A mention counts only when it is code: backticked (`make ui-test`) or
-# the first thing on a line, as in a fenced block.  Bare prose is not
-# scanned, because English says things like "a renamed make target".
-mentioned="$(grep -rhoE '(`|^)make [a-z][a-z0-9-]*' .pi --include='*.md' |
-	sed 's/^`//' | awk '{print $2}' | sort -u)"
+# A mention counts only when it is code: backticked (`make ui-test`)
+# anywhere, or at the start of a line **inside a fenced block**.  Bare
+# prose is not scanned, because English says things like "a renamed make
+# target".
+#
+# The fence is why this is awk rather than one grep.  Line-start alone is
+# not evidence of code in a file that is mostly hard-wrapped prose: the
+# sentence "Two green branches do not / make a green merge" wrapped onto
+# a line beginning `make a`, and the check duly failed on a target called
+# `a`.  Inside a fence it is code; outside one it is a sentence that
+# happened to break there, and a check that fails on reflow gets
+# disabled rather than fixed.
+#
+# AGENTS.md is deliberately not in this list: it is a symlink to
+# CLAUDE.md, asserted above, so scanning it would report every failure
+# twice under two names.
+mentioned="$({ find .pi -name '*.md' 2>/dev/null; echo CLAUDE.md; } |
+	xargs awk '
+		FNR == 1 { fence = 0 }
+		/^```/   { fence = !fence; next }
+		{
+			rest = $0
+			while (match(rest, /`make [a-z][a-z0-9-]*/)) {
+				print substr(rest, RSTART + 6, RLENGTH - 6)
+				rest = substr(rest, RSTART + RLENGTH)
+			}
+			if (fence && match($0, /^make [a-z][a-z0-9-]*/)) {
+				print substr($0, 6, RLENGTH - 5)
+			}
+		}
+	' | sort -u)"
 
 missing=""
 
@@ -36,10 +93,10 @@ for t in $mentioned; do
 done
 
 if [ -n "$missing" ]; then
-	echo "skill-check: .pi/ documents make targets that do not exist:" >&2
+	echo "skill-check: the agent docs name make targets that do not exist:" >&2
 	for t in $missing; do
 		echo "  make $t" >&2
-		grep -rln "make $t" .pi --include='*.md' | sed 's/^/      /' >&2
+		grep -rln "make $t" .pi CLAUDE.md --include='*.md' | sed 's/^/      /' >&2
 	done
 	echo "Fix the docs, or restore the target." >&2
 	exit 1