fix(relations): hydration survives a NULL item_number and honours the declared collection (PLAN-2857 U6)

Codex round 7's two read-side P2s, both about legacy rows the write side
has refused since U1 but which still exist.

NULL ITEM_NUMBER TOOK DOWN THE WHOLE RESPONSE. The hydration query scanned
items.item_number into an `int`, and that column is nullable — migration
006 adds it with no constraint and nothing since makes it NOT NULL.
Scanning NULL into an int ERRORS, this function is best-effort, and its
caller drops the entire map on error: so ONE pre-006 row anywhere in a
page silently removed `relation_targets` from every item in the response.
Scanned as sql.NullInt64 now; such a target hydrates with its title and no
ref, because there is no number to build one from — not id-only, and not
an error.

THE DECLARED COLLECTION WAS NOT CHECKED. Any visible same-workspace id
hydrated to {ref, title}, including one pointing at an item in a
different collection than the field declares. That renders a Car as
though it were the Colour the field promised. It is id-only now, the same
shape a dangling value gets and for the same reason: the honest answer is
"this points somewhere I will not dress up".

Not everything round 7 raised is a defect. It called the store signature
changes a P1 on the grounds that "existing callers outside this diff
cannot compile". internal/store is an INTERNAL package — nothing outside
this module can import it — so the compiler has already proved every
caller was updated, which is exactly why a required parameter was the
right shape rather than a variadic that would let a caller silently omit
the predicate.

instructions.md now DOCUMENTS the feature rather than only bumping its
heading: the three accepted spellings and their order, collection-scoped
matching, `ambiguous` versus `not_found`, that only items you can see
count, and that an id-only `relation_targets` entry means gone OR hidden
and must not be rendered as either.

MUTATION:

  M20  item_number scanned as int again          KILLED
  M21  declared-collection check removed         KILLED

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

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

internal/store/relation_referents.go

@@ -753,6 +753,14 @@ func (s *Store) itemByRefQ(q Queryer, workspaceID, prefix string, number int) (*
 	return s.GetItemQ(q, id)
 }
 
+// relationWant is one stored relation value plus the collection slug its field
+// DECLARES, carried together so hydration can check the second against the
+// first without re-walking the schemas.
+type relationWant struct {
+	value      string
+	targetSlug string
+}
+
 // RelationTargetNotPortable — a carried relation value that cannot cross a
 // workspace boundary. Not a defect in the value: it names a live item in the
 // SOURCE workspace, and PLAN-2857 v1 excludes cross-workspace relation
@@ -1463,8 +1471,9 @@ func (s *Store) HydrateRelationTargetsQ(
 	items []models.Item,
 	schemas map[string]models.CollectionSchema,
 ) (map[string]map[string]models.RelationTarget, error) {
-	// item ID -> field key -> stored value
+	// item ID -> field key -> the stored value and the collection its field
-	perItem := map[string]map[string]string{}
+	// DECLARES as the target.
+	perItem := map[string]map[string]relationWant{}
 	wanted := map[string]struct{}{}
 
 	for i := range items {
@@ -1497,9 +1506,9 @@ func (s *Store) HydrateRelationTargetsQ(
 				continue
 			}
 			if perItem[items[i].ID] == nil {
-				perItem[items[i].ID] = map[string]string{}
+				perItem[items[i].ID] = map[string]relationWant{}
 			}
-			perItem[items[i].ID][def.Key] = value
+			perItem[items[i].ID][def.Key] = relationWant{value: value, targetSlug: def.Collection}
 			wanted[value] = struct{}{}
 		}
 	}
@@ -1515,6 +1524,9 @@ func (s *Store) HydrateRelationTargetsQ(
 	sort.Strings(ids)
 
 	resolved := map[string]models.RelationTarget{}
+	// The collection each resolved target actually lives in, for the
+	// declared-collection check below.
+	resolvedIn := map[string]string{}
 	// Chunked: some drivers cap placeholders per statement, and a workspace
 	// with a long list read can exceed it.
 	const chunk = 200
@@ -1530,7 +1542,7 @@ func (s *Store) HydrateRelationTargetsQ(
 			args = append(args, id)
 		}
 		rows, err := q.Query(s.q(`
-			SELECT i.id, i.title, i.item_number, c.prefix
+			SELECT i.id, i.title, i.item_number, i.collection_id, c.prefix
 			FROM items i
 			JOIN collections c ON c.id = i.collection_id
 			WHERE i.workspace_id = ? AND i.id IN (`+placeholders+`) AND i.deleted_at IS NULL
@@ -1539,17 +1551,24 @@ func (s *Store) HydrateRelationTargetsQ(
 			return nil, fmt.Errorf("hydrate relation targets: %w", err)
 		}
 		for rows.Next() {
-			var id, title, prefix string
+			var id, title, collectionID, prefix string
-			var number int
+			// NULLABLE. item_number is added by migration 006 with no
-			if err := rows.Scan(&id, &title, &number, &prefix); err != nil {
+			// constraint and nothing since makes it NOT NULL, so a pre-006 row
+			// carries NULL — and scanning NULL into an int ERRORS. This
+			// function is best-effort and its caller drops the whole map on
+			// error, so one legacy row would have silently removed
+			// `relation_targets` from the ENTIRE response (codex round 7).
+			var number sql.NullInt64
+			if err := rows.Scan(&id, &title, &number, &collectionID, &prefix); err != nil {
 				rows.Close()
 				return nil, fmt.Errorf("hydrate relation targets scan: %w", err)
 			}
-			resolved[id] = models.RelationTarget{
+			target := models.RelationTarget{ID: id, Title: title}
-				ID:    id,
+			if number.Valid {
-				Ref:   fmt.Sprintf("%s-%d", prefix, number),
+				target.Ref = fmt.Sprintf("%s-%d", prefix, number.Int64)
-				Title: title,
 			}
+			resolvedIn[id] = collectionID
+			resolved[id] = target
 		}
 		if err := rows.Err(); err != nil {
 			rows.Close()
@@ -1558,14 +1577,38 @@ func (s *Store) HydrateRelationTargetsQ(
 		rows.Close()
 	}
 
+	// Declared target slug -> collection id, resolved once per distinct slug.
+	declaredIDs := map[string]string{}
+
 	out := map[string]map[string]models.RelationTarget{}
 	for itemID, fields := range perItem {
-		for key, value := range fields {
+		for key, want := range fields {
+			value := want.value
 			target, ok := resolved[value]
 			if !ok {
 				// Dangling: the stored value names nothing live. ID-only, so
 				// the read stays honest instead of erroring or pretending.
 				target = models.RelationTarget{ID: value}
+			} else if want.targetSlug != "" {
+				// THE VALUE MUST LIVE WHERE THE FIELD SAYS IT DOES. The write
+				// side refuses a relation pointing outside the declared
+				// collection, but legacy and corrupt rows predate that, and
+				// rendering one as `{ref, title}` presents a Task as though it
+				// were the Colour the field promised. ID-only is the honest
+				// shape — the same one a dangling value gets, for the same
+				// reason (codex round 7).
+				declaredID, known := declaredIDs[want.targetSlug]
+				if !known {
+					var derr error
+					declaredID, derr = s.collectionIDBySlugQ(q, workspaceID, want.targetSlug)
+					if derr != nil {
+						return nil, derr
+					}
+					declaredIDs[want.targetSlug] = declaredID
+				}
+				if declaredID == "" || resolvedIn[value] != declaredID {
+					target = models.RelationTarget{ID: value}
+				}
 			}
 			if out[itemID] == nil {
 				out[itemID] = map[string]models.RelationTarget{}

internal/store/relation_titles_test.go

@@ -404,3 +404,77 @@ func TestRelationTitle_SuppliedValueStillObeysVisibility(t *testing.T) {
 		t.Fatalf("a SUPPLIED title naming an item the caller cannot see must be not_found — the carried carve-out must not reach it: %v", issues)
 	}
 }
+
+// TestHydrateRelationTargets_WrongCollectionAndNullNumberStayHonest covers
+// codex round 7's two read-side P2s.
+//
+// Both are about legacy rows: the write side has refused wrong-collection
+// relation values since U1 and item_number has been populated since migration
+// 006, but rows predating either still exist and the READ has to hold up.
+func TestHydrateRelationTargets_WrongCollectionAndNullNumberStayHonest(t *testing.T) {
+	t.Parallel()
+	s := testStore(t)
+	ws, colors, cars, red := relationFixture(t, s)
+
+	// A live item in the WRONG collection for the `color` field.
+	sedan := createTestItem(t, s, ws.ID, cars.ID, "Sedan", "")
+	// A legitimate target whose item_number is NULL, as a pre-006 row is.
+	legacy := createTestItem(t, s, ws.ID, colors.ID, "Legacy Blue", "")
+	if _, err := s.db.Exec(s.q("UPDATE items SET item_number = NULL WHERE id = ?"), legacy.ID); err != nil {
+		t.Fatalf("null the item_number: %v", err)
+	}
+
+	mk := func(label, value string) models.Item {
+		it := createTestItem(t, s, ws.ID, cars.ID, label, "")
+		blob, _ := json.Marshal(map[string]any{"status": "open", "color": value})
+		fields := string(blob)
+		if _, err := s.UpdateItem(it.ID, models.ItemUpdate{Fields: &fields}); err != nil {
+			t.Fatalf("seed %s: %v", label, err)
+		}
+		got, err := s.GetItem(it.ID)
+		if err != nil {
+			t.Fatalf("reload %s: %v", label, err)
+		}
+		return *got
+	}
+
+	good := mk("Points at a colour", red.ID)
+	wrong := mk("Points at a car", sedan.ID)
+	nulled := mk("Points at a legacy colour", legacy.ID)
+
+	schemas := map[string]models.CollectionSchema{cars.ID: u1RelationSchema("colors")}
+	out, err := s.HydrateRelationTargetsQ(s.DB(), ws.ID, []models.Item{good, wrong, nulled}, schemas)
+	if err != nil {
+		t.Fatalf("hydrate: %v", err)
+	}
+
+	// A NULL item_number must not take the whole response down with it. This
+	// function is best-effort and its caller drops the entire map on error, so
+	// scanning NULL into an int would have removed relation_targets from every
+	// item in the response, not just this one.
+	if len(out) == 0 {
+		t.Fatal("hydration returned nothing at all; one legacy row with a NULL item_number erased the whole response")
+	}
+	if got := out[good.ID]["color"]; got.Ref != red.Ref || got.Title != red.Title {
+		t.Errorf("the ordinary target did not hydrate: %+v", got)
+	}
+
+	// Wrong collection: id-only. Rendering it fully would present a Car as
+	// though it were the Colour the field promised.
+	if got := out[wrong.ID]["color"]; got.Ref != "" || got.Title != "" {
+		t.Errorf("a value pointing OUTSIDE the declared collection hydrated as %+v; the field says colors and this is a car", got)
+	} else if got.ID != sedan.ID {
+		t.Errorf("the wrong-collection entry lost its stored id: %+v", got)
+	}
+
+	// NULL item_number: the row is a legitimate target in the right
+	// collection, so it hydrates — with a title and no ref, because there is
+	// no number to build one from. Not id-only, and not an error.
+	got := out[nulled.ID]["color"]
+	if got.Title != "Legacy Blue" {
+		t.Errorf("a NULL-numbered target lost its title: %+v", got)
+	}
+	if got.Ref != "" {
+		t.Errorf("a NULL-numbered target invented a ref: %+v", got)
+	}
+}