fix(relations): no second pool read while the backlink cursor is open (PLAN-2857 U5)

Codex round 7. One P1, and it is a class this repo has already been bitten
by once.

THE FIELD-LABEL LOOKUP I ADDED LAST ROUND ISSUED A SELECT PER DISTINCT
COLLECTION WHILE THE ROWS CURSOR WAS STILL OPEN. That needs a SECOND pool
connection, and when the pool is saturated the second connection never
arrives — an application deadlock no SQLSTATE names, visible only under
load. It is BUG-2409's class, the one
TestCollectionSlugWritersDoNoPoolIOUnderTheLock exists to guard for the
collection writers. It was also an N+1.

`c.schema` is selected in the one query now, so the label parse reads row
data already in hand and the second connection is never wanted. Fixing
the deadlock removed the N+1 as a side effect rather than the other way
round.

IT HAS A DETERMINISTIC GUARD, modelled on the existing one: with
MaxOpenConns(1) the open cursor owns the only connection, so any second
pool read hangs reliably instead of rarely. The fixture puts the two
sources in DIFFERENT collections on purpose — a single-collection fixture
would let the broken shape pass on a lucky cache hit, which is the kind
of fixture that reports a green for the wrong reason.

Mutation: restoring the per-collection SELECT KILLS it. That is the guard
demonstrated rather than asserted, which matters more here than usual
because the test passes on a tree with no bug in it either.

NIT, and it is the one I mind most: this file's header claimed the
counterfactual lived in "TestRelationLinks_RemovedHookGoesStale at the
bottom". No such test exists. The counterfactual is real — it is a
MUTATION, run every round, recorded in the commit messages with its
verdict — but the comment described the file's own contents wrongly, and
nothing checks a comment. It now says what the counterfactual actually is
and where its record lives.

Gates: go test ./... 0, gofmt clean, lint 0 issues.

Claude-Session: https://claude.ai/code/session_01PRcuviXka4Un1FGrBrX8jc

internal/store/relation_links.go

@@ -315,7 +315,7 @@ func (s *Store) GetRelationBacklinks(targetItemID, workspaceID string, limit, of
 
 	rows, err := s.db.Query(s.q(`
 		SELECT rl.source_item_id, rl.source_field_key,
-		       s.title, s.item_number, s.collection_id, c.prefix, c.slug
+		       s.title, s.item_number, s.collection_id, c.prefix, c.slug, c.schema
 		FROM item_relation_links rl
 		JOIN items s ON s.id = rl.source_item_id
 		JOIN collections c ON c.id = s.collection_id
@@ -335,24 +335,26 @@ func (s *Store) GetRelationBacklinks(targetItemID, workspaceID string, limit, of
 	}
 	defer rows.Close()
 
-	// Field LABELS come from each source collection's schema, parsed once per
-	// distinct collection rather than per row. The field was declared and the
-	// UI consumed it while nothing populated it, so every group rendered its
-	// raw key (codex round 6) — the choice was to populate it or delete it,
-	// and a label is what the schema author wrote for humans to read.
+	// Field LABELS come from the source collection's schema, which the query
+	// above already carries on every row — parsed once per distinct collection
+	// here rather than re-queried.
+	//
+	// The first version issued a SELECT per distinct collection WHILE THE ROWS
+	// CURSOR WAS STILL OPEN. That needs a SECOND pool connection, and when the
+	// pool is saturated the second connection never arrives — an application
+	// deadlock no SQLSTATE names, and the same class BUG-2409 closed for the
+	// collection writers (codex round 7). It was also an N+1. Selecting
+	// `c.schema` in the one query removes both.
 	labels := map[string]map[string]string{}
-	labelFor := func(collectionSlug, collectionID, key string) string {
+	labelFor := func(collectionID, schemaJSON, key string) string {
 		byKey, known := labels[collectionID]
 		if !known {
 			byKey = map[string]string{}
-			var schemaJSON sql.NullString
-			if err := s.db.QueryRow(s.q(`SELECT schema FROM collections WHERE id = ?`), collectionID).Scan(&schemaJSON); err == nil && schemaJSON.Valid {
-				var schema models.CollectionSchema
-				if json.Unmarshal([]byte(schemaJSON.String), &schema) == nil {
-					for _, def := range schema.Fields {
-						if def.Label != "" {
-							byKey[def.Key] = def.Label
-						}
+			var schema models.CollectionSchema
+			if json.Unmarshal([]byte(schemaJSON), &schema) == nil {
+				for _, def := range schema.Fields {
+					if def.Label != "" {
+						byKey[def.Key] = def.Label
 					}
 				}
 			}
@@ -368,14 +370,15 @@ func (s *Store) GetRelationBacklinks(targetItemID, workspaceID string, limit, of
 		// errors and would take the whole list down, the same defect U6 shipped
 		// in hydration (codex round 7 there).
 		var number sql.NullInt64
+		var schemaJSON sql.NullString
 		var prefix, collectionID string
-		if err := rows.Scan(&b.SourceItemID, &b.FieldKey, &b.SourceTitle, &number, &collectionID, &prefix, &b.CollectionSlug); err != nil {
+		if err := rows.Scan(&b.SourceItemID, &b.FieldKey, &b.SourceTitle, &number, &collectionID, &prefix, &b.CollectionSlug, &schemaJSON); err != nil {
 			return nil, fmt.Errorf("scan relation backlink: %w", err)
 		}
 		if number.Valid {
 			b.SourceRef = fmt.Sprintf("%s-%d", prefix, number.Int64)
 		}
-		b.FieldLabel = labelFor(b.CollectionSlug, collectionID, b.FieldKey)
+		b.FieldLabel = labelFor(collectionID, schemaJSON.String, b.FieldKey)
 		out = append(out, b)
 	}
 	if err := rows.Err(); err != nil {

internal/store/relation_links_test.go

@@ -3,6 +3,7 @@ package store
 import (
 	"encoding/json"
 	"testing"
+	"time"
 
 	"github.com/PerpetualSoftware/pad/internal/models"
 )
@@ -14,8 +15,14 @@ import (
 // hooks that maintain it, so a test that exercised the helper directly would
 // vouch for nothing — these go through the real write paths (CreateItem,
 // UpdateItem, DeleteItem, RestoreItem, MoveItem, UpdateCollection) and read
-// through the real query. The counterfactual that proves they are wired lives
-// in TestRelationLinks_RemovedHookGoesStale at the bottom.
+// through the real query.
+//
+// The counterfactual that proves they are wired is a MUTATION, not a test in
+// this file: replaceRelationLinks is stubbed to return early, the mutant is
+// checked to COMPILE, and every leg here must fail. It is recorded in the
+// commit messages with its verdict. An earlier version of this comment
+// pointed at a TestRelationLinks_RemovedHookGoesStale that does not exist —
+// a claim about the file's own contents that nothing checked (codex round 7).
 
 // unrestricted is the visibility vector for a viewer who sees everything. The
 // viewer-scoped legs build their own.
@@ -653,3 +660,54 @@ func TestRelationLinks_BacklinkCarriesTheFieldLabel(t *testing.T) {
 		t.Errorf("sidekick's label = %q, want empty — a field with no schema label must not have one invented for it", got)
 	}
 }
+
+// TestRelationBacklinksDoNoSecondPoolRead is the guard for the invariant round
+// 7's P1 cost me.
+//
+// GetRelationBacklinks holds the rows cursor open while it builds the result.
+// The first version of the field-LABEL lookup issued a SELECT per distinct
+// collection inside that loop — which needs a SECOND pool connection, and when
+// the pool is saturated the second connection never arrives. That is an
+// application deadlock no SQLSTATE names, the same class BUG-2409 closed for
+// the collection writers, and it only shows up under load.
+//
+// With MaxOpenConns(1) the open cursor owns the only connection, so ANY second
+// pool read here deadlocks deterministically instead of rarely. Works on both
+// dialects.
+func TestRelationBacklinksDoNoSecondPoolRead(t *testing.T) {
+	s := testStore(t)
+	ws, _, cars, red := relationIndexFixture(t, s)
+
+	// Two sources in DIFFERENT collections, so a per-collection lookup would
+	// have to issue more than one — a single-collection fixture would let the
+	// broken shape pass on a lucky cache hit.
+	carPointingAt(t, s, ws, cars, "Pool Car", red.ID)
+	other, err := s.CreateCollection(ws.ID, models.CollectionCreate{
+		Name:   "Garages Too",
+		Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","label":"Paint","type":"relation","collection":"colors"}]}`,
+	})
+	if err != nil {
+		t.Fatalf("CreateCollection: %v", err)
+	}
+	blob, _ := json.Marshal(map[string]any{"status": "open", "color": red.ID})
+	if _, err := s.CreateItem(ws.ID, other.ID, models.ItemCreate{Title: "Pool Garage", Fields: string(blob)}); err != nil {
+		t.Fatalf("CreateItem: %v", err)
+	}
+
+	// From here the pool has exactly one connection.
+	s.db.SetMaxOpenConns(1)
+
+	done := make(chan error, 1)
+	go func() {
+		_, err := s.GetRelationBacklinks(red.ID, ws.ID, 50, 0, unrestricted)
+		done <- err
+	}()
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatalf("GetRelationBacklinks: %v", err)
+		}
+	case <-time.After(30 * time.Second):
+		t.Fatal("GetRelationBacklinks did not complete under MaxOpenConns(1): it issued a pool read while holding its own rows cursor, and is waiting for a connection it is itself occupying")
+	}
+}