fix(migrations): say why foreign keys go off, accurately

The note on 0009 and 0012 said the drop would cascade and take every row with it.
It would not: nothing references `activity` or `attachments`, so dropping either
takes nothing along. That claim was carried over from 0008, where seven tables do
reference `tasks` and it is exactly right.

The real reason is the check that comes with the marker. With foreign keys on, the
copy is validated a row at a time against a half-rebuilt schema, and the first bad
row aborts with no idea how many others are like it. With them off,
`PRAGMA foreign_key_check` runs once before the commit and names what is actually
wrong — which matters, because these run against a live database on somebody's
next restart and a failed migration is a server that does not start.

The upgrade test now carries activity rows, an attachment and a group across all
five migrations rather than stopping at 0008, and checks that the columns added by
ALTER arrived with their defaults rather than as NULL.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

internal/store/migrations/0009_activity_survives_its_author.sql

@@ -21,6 +21,14 @@
 -- (created_at, id) matches the keyset the handler pages with — two rows written in
 -- the same second must not be able to hide each other across a page boundary.
 --
+-- The marker below is not here because the drop would cascade: nothing references
+-- `activity`, so it takes nothing with it. It is here for the check that comes
+-- with it. With foreign keys on, the copy is validated a row at a time against a
+-- schema that is half rebuilt, and the first bad row aborts with no idea how many
+-- others are like it. With them off, `PRAGMA foreign_key_check` runs once before
+-- the commit and names what is actually wrong — and this runs against a live
+-- database on somebody's next restart.
+--
 -- verdande:rebuild-tables
 
 CREATE TABLE activity_rebuilt (

internal/store/migrations/0012_groups_are_places.sql

@@ -10,9 +10,16 @@
 --
 -- The attachments table has to be rebuilt for the second, because its CHECK spells
 -- out exactly which parent columns may be set, and SQLite cannot alter a CHECK.
+--
 -- Foreign keys go off for the length of it — see the marker below and the note in
--- `applyMigration` — because dropping the old table with them on is a cascading
--- delete of every attachment in the instance.
+-- `applyMigration`. Not because the drop would cascade: nothing references
+-- `attachments`, so it takes nothing with it. It is for the check that comes with
+-- the marker. With foreign keys on, the copy is validated a row at a time against
+-- a half-rebuilt schema and the first bad row aborts with no idea how many others
+-- there are; with them off, `PRAGMA foreign_key_check` runs once before the commit
+-- and names what is actually wrong. This runs against a live database on somebody's
+-- next restart, and an attachment whose task was already gone would otherwise stop
+-- the server from starting with one row's worth of explanation.
 --
 -- The rule stays "exactly one parent": an attachment belongs to a task, a comment
 -- or a group, never two. An attachment with two parents is one that gets deleted

internal/store/store_test.go

@@ -484,6 +484,15 @@ func TestTheTasksRebuildCarriesTheDataAcross(t *testing.T) {
 		`INSERT INTO task_labels (task_id, label_id) VALUES ('t1', 'l1')`,
 		`INSERT INTO comments (id, task_id, user_id, body, created_at, updated_at)
 		 VALUES ('c1', 't1', 'u1', 'en kommentar', ` + fmt.Sprint(now) + `, ` + fmt.Sprint(now) + `)`,
+		// 0009 rebuilds activity and 0012 rebuilds attachments, so both need rows
+		// here: a rebuild that drops them looks exactly like one that works.
+		`INSERT INTO activity (id, project_id, task_id, user_id, event, payload_json, created_at)
+		 VALUES ('a1', 'p1', 't1', 'u1', 'task.created', '{}', ` + fmt.Sprint(now) + `),
+		        ('a2', 'p2', NULL, 'u1', 'project.created', '{}', ` + fmt.Sprint(now) + `)`,
+		`INSERT INTO attachments (id, task_id, filename, mime_type, size, path, uploaded_by, created_at)
+		 VALUES ('f1', 't1', 'tegning.pdf', 'application/pdf', 1024, 'ab/cd/abcd', 'u1', ` + fmt.Sprint(now) + `)`,
+		`INSERT INTO project_groups (id, owner_id, name, color, collapsed, sort_order, created_at, updated_at)
+		 VALUES ('g1', 'u1', 'Arbejde', 'graphite', 0, 1024, ` + fmt.Sprint(now) + `, ` + fmt.Sprint(now) + `)`,
 	} {
 		if _, err := sqlDB.ExecContext(ctx, stmt); err != nil {
 			t.Fatalf("seed: %v\n%s", err, stmt)
@@ -513,6 +522,48 @@ func TestTheTasksRebuildCarriesTheDataAcross(t *testing.T) {
 			"A drop that cascaded would look exactly like this", tasks, labels, comments)
 	}
 
+	// 0009 and 0012 rebuild activity and attachments by the same machinery. They
+	// run against a live database on the next restart, and the thing that goes
+	// wrong in a rebuild is rows quietly not arriving.
+	var activity, attachments, groups int
+	if err := db.QueryRow(`SELECT count(*) FROM activity`).Scan(&activity); err != nil {
+		t.Fatal(err)
+	}
+	if err := db.QueryRow(`SELECT count(*) FROM attachments`).Scan(&attachments); err != nil {
+		t.Fatal(err)
+	}
+	if err := db.QueryRow(`SELECT count(*) FROM project_groups`).Scan(&groups); err != nil {
+		t.Fatal(err)
+	}
+	if activity != 2 || attachments != 1 || groups != 1 {
+		t.Fatalf("after 0009 and 0012: %d activity rows, %d attachments, %d groups — want 2, 1, 1",
+			activity, attachments, groups)
+	}
+
+	// The attachment kept its parent and its bytes. A rebuild that lost the task_id
+	// leaves a row nothing can reach, which is worse than losing it outright.
+	var fileParent, filename string
+	if err := db.QueryRow(
+		`SELECT task_id, filename FROM attachments WHERE id = 'f1'`).Scan(&fileParent, &filename); err != nil {
+		t.Fatal(err)
+	}
+	if fileParent != "t1" || filename != "tegning.pdf" {
+		t.Errorf("the attachment came across as %q on %q", filename, fileParent)
+	}
+
+	// And the new columns arrived with their defaults rather than as NULL, which is
+	// what an ALTER that forgot NOT NULL DEFAULT would give.
+	var groupAbout, sidebar string
+	if err := db.QueryRow(`SELECT description FROM project_groups WHERE id = 'g1'`).Scan(&groupAbout); err != nil {
+		t.Fatal(err)
+	}
+	if err := db.QueryRow(`SELECT sidebar_collapsed FROM users WHERE id = 'u1'`).Scan(&sidebar); err != nil {
+		t.Fatal(err)
+	}
+	if groupAbout != "" || sidebar != "[]" {
+		t.Errorf("new columns on existing rows: description %q, sidebar_collapsed %q", groupAbout, sidebar)
+	}
+
 	// Every column came across, including the generated one and the sub-task's
 	// self-reference.
 	var content, description, parent string