ci: stop testing every commit twice on a runner there is one of

A branch push and its pull request are the same commit. With
`branches: ['**']` alongside `pull_request:`, opening a PR booked four
runs -- check and e2e for the branch, then both again for
refs/pull/N/head -- and this host has capacity 1, shared with an index
build that can hold it for three hours. PR #1's own checks queued two
runs deep behind exactly that.

`pull_request` covers feature branches. `main` stays because a
post-merge run is the record of the trunk's health, and now that main
refuses direct pushes it happens exactly once per merge.

The trade is that a branch pushed with no PR open gets no CI. That
matches the workflow this repo just committed to, and the signal returns
on the same commit the moment a PR exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh

.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
+# **There is deliberately no `push` trigger, and restoring one is a
-# running it on every push to main is safe.
+# 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.
 #
-# ---------------------------------------------------------------------
+# That is not hypothetical. On 2026-08-17 `fix(database): retire a table
-# TEMPORARY (2026-08-17): the `push` trigger is off while the catalog
+# whose shape the schema moved past` landed on main, green — the CI
-# rebuilds.
+# 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`
+# So the rule this file is an instance of: **a job that mutates state
-# dropped this job's `explore_index` on its first run -- the CI database
+# which cannot be rebuilt in ten minutes is triggered deliberately, not
-# is deliberately in the older encoding, so the repair judged it stale --
+# by a push.** The weekly cron keeps the catalog current, and
-# and the job fell back to a full ~205 GB import from the dumps.
+# workflow_dispatch resumes or forces a build — indexbuild picks up from
-# `fix(database): never retire the catalog the index build derives`
+# its checkpoint either way, so nothing is lost by not running on every
-# stops it happening again but cannot undo it.
+# merge. See docs/index-cache.md for the snapshot and the restore.
-#
-# 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.
-# ---------------------------------------------------------------------
 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.

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
+# .pi/ and CLAUDE.md document commands, and a doc that documents a
-# is worse than no skill: an agent runs it confidently.  Every command
+# command wrongly is worse than no doc: an agent runs it confidently.
-# in there is a make target on purpose, so this is checkable.
+# Every command in them is a make target on purpose, so this is
-skill-check: ## Fail if .pi/ documents a make target that does not exist
+# 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}
 }
 

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
+# Every command in the agent-facing docs is a `make` target on purpose:
-# the source of truth for *how* to invoke something, and the skill only
+# the Makefile is the source of truth for *how* to invoke something, and
-# decides *which* and *in what order*.  This check keeps that honest —
+# the docs only decide *which* and *in what order*.  This check keeps
-# a renamed or deleted target turns into a failing commit rather than
+# that honest — a renamed or deleted target turns into a failing commit
-# into an agent confidently running a command that no longer exists.
+# rather than into an agent confidently running a command that no longer
+# exists.
 #
-# It extracts every `make <target>` mentioned under .pi/ and asserts the
+# It checks two things.  Usage:  scripts/skill-check.sh
-# target exists.  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
+# A mention counts only when it is code: backticked (`make ui-test`)
-# the first thing on a line, as in a fenced block.  Bare prose is not
+# anywhere, or at the start of a line **inside a fenced block**.  Bare
-# scanned, because English says things like "a renamed make target".
+# prose is not scanned, because English says things like "a renamed make
-mentioned="$(grep -rhoE '(`|^)make [a-z][a-z0-9-]*' .pi --include='*.md' |
+# target".
-	sed 's/^`//' | awk '{print $2}' | sort -u)"
+#
+# 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