Fix store migration so the live database moves to the new scheme

docs/operations.md

@@ -15,6 +15,8 @@
 
 State lives in the `byre` directory under Go's `os.UserConfigDir()` (on macOS, `~/Library/Application Support/byre`): `byre.db` and `worktrees/<project-id>/<task-id>`. Task branches are `byre/<task-id>` in the project's repo.
 
+Schema changes are the `migrations` slice in `internal/store/store.go`: named entries, each with a short kebab-case name saying what it does. On open, byre applies, in slice order, every entry whose name isn't in the `migrations` table, each in its own transaction that also records the name. byre is deployed from task branches that each append their own migrations, so after a merge a migration can land before others the database has already applied, and it still runs. Give a new migration a name no other branch will use, and make any data migration safe to run twice. `sqlite3 byre.db 'SELECT * FROM migrations'` lists what has been applied. A database from before the table existed is moved over on open: the first `PRAGMA user_version` entries are recorded as applied, and `user_version` is no longer read or written.
+
 ## Closing a task
 
 A task ends `merged` or `discarded`; either way its sessions are stopped, its worktree and branch are removed, its `open` inbox items become `dismissed` (resolved by whoever closed it), and its record and events remain. `GET /api/tasks/{id}/unmerged` reports what would be lost or merged: commits on the branch that aren't on the default branch, and files, lines and uncommitted paths that differ from the merge base. The UI shows it in the confirm dialog of both buttons.

docs/plan.md

@@ -451,6 +451,7 @@ Issues identified in review that implementation must account for:
 - **Squash-and-reset under a running agent.** Agents may notice their commits disappearing and try to repair history; step 7 of the merge cycle addresses this and needs testing per vendor.
 - **Event log growth.** Stream-json is verbose and tool outputs are large. SSE resume and scrubbing need efficient queries; consider separating large payloads into blobs.
 - **Migration numbering across branches.** Migrations are tracked by position (`PRAGMA user_version`). Each task branch appends its own, and byre is deployed from task branches, so two branches can claim the same number: the database records it as applied and the other branch's migration silently never runs. Fix: track applied migrations by name in a table. Before switching, make every existing data migration safe to run twice, since one recorded only by number would run again under its name.
+  - **Status: fixed.** Migrations are named and tracked in a `migrations` table (see `docs/operations.md`, "Data").
 
 ---
 

internal/store/migrate_internal_test.go

@@ -0,0 +1,178 @@
+package store
+
+import (
+	"database/sql"
+	"path/filepath"
+	"slices"
+	"testing"
+)
+
+func openTestDB(t *testing.T, path string) *sql.DB {
+	t.Helper()
+	db, err := sql.Open("sqlite", "file:"+path+"?_pragma=foreign_keys(1)&_txlock=immediate")
+	if err != nil {
+		t.Fatalf("open db: %v", err)
+	}
+	t.Cleanup(func() { db.Close() })
+	return db
+}
+
+func appliedNames(t *testing.T, db *sql.DB) []string {
+	t.Helper()
+	rows, err := db.Query(`SELECT name FROM migrations ORDER BY rowid`)
+	if err != nil {
+		t.Fatalf("list migrations: %v", err)
+	}
+	defer rows.Close()
+
+	names := []string{}
+	for rows.Next() {
+		name := ""
+		err = rows.Scan(&name)
+		if err != nil {
+			t.Fatalf("scan migration: %v", err)
+		}
+		names = append(names, name)
+	}
+	if rows.Err() != nil {
+		t.Fatalf("list migrations: %v", rows.Err())
+	}
+	return names
+}
+
+func migrationNames(ms []migration) []string {
+	names := []string{}
+	for _, m := range ms {
+		names = append(names, m.name)
+	}
+	return names
+}
+
+func insertReport(t *testing.T, db *sql.DB, id, payload string) {
+	t.Helper()
+	_, err := db.Exec(`
+		INSERT OR IGNORE INTO users (login, display_name, created_at) VALUES ('alice', 'Alice', 'x');
+		INSERT OR IGNORE INTO projects (id, name, repo_path, default_branch, remote, created_by, created_at) VALUES ('p1', 'demo', '/nonexistent', 'main', 'none', 'alice', 'x');
+		INSERT OR IGNORE INTO tasks (id, project_id, title, branch, worktree_path, base_ref, status, created_by, created_at) VALUES ('t1', 'p1', 't1', 't1', '/nonexistent', 'x', 'active', 'alice', 'x');
+		INSERT OR IGNORE INTO agent_sessions (id, task_id, vendor, model, vendor_session_id, state, role, created_by, created_at) VALUES ('s1', 't1', 'claude', '', 's1', 'idle', 'implementer', 'alice', 'x');
+	`)
+	if err != nil {
+		t.Fatalf("insert parents: %v", err)
+	}
+	_, err = db.Exec(`
+		INSERT INTO inbox_items (id, project_id, task_id, agent_session_id, thread_id, kind, priority, blocking, payload, state, created_at)
+		VALUES (?, 'p1', 't1', 's1', ?, 'report', 0, 0, ?, 'open', 'x')
+	`, id, id, payload)
+	if err != nil {
+		t.Fatalf("insert report: %v", err)
+	}
+}
+
+func reportPayload(t *testing.T, db *sql.DB, id string) string {
+	t.Helper()
+	payload := ""
+	err := db.QueryRow(`SELECT payload FROM inbox_items WHERE id = ?`, id).Scan(&payload)
+	if err != nil {
+		t.Fatalf("read report: %v", err)
+	}
+	return payload
+}
+
+func TestMigrateFreshDatabase(t *testing.T) {
+	db := openTestDB(t, filepath.Join(t.TempDir(), "byre.db"))
+
+	err := migrate(db, migrations)
+	if err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	err = migrate(db, migrations)
+	if err != nil {
+		t.Fatalf("migrate again: %v", err)
+	}
+
+	got := appliedNames(t, db)
+	want := migrationNames(migrations)
+	if !slices.Equal(got, want) {
+		t.Fatalf("applied %v, want %v", got, want)
+	}
+}
+
+func TestMigrateFromUserVersion(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "byre.db")
+	db := openTestDB(t, path)
+	for _, m := range migrations[:9] {
+		_, err := db.Exec(m.sql)
+		if err != nil {
+			t.Fatalf("old migration %s: %v", m.name, err)
+		}
+	}
+	_, err := db.Exec(`PRAGMA user_version = 9`)
+	if err != nil {
+		t.Fatalf("set user_version: %v", err)
+	}
+	insertReport(t, db, "r1", `{"text":"- shipped"}`)
+	db.Close()
+
+	db = openTestDB(t, path)
+	err = migrate(db, migrations)
+	if err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+
+	got := appliedNames(t, db)
+	want := migrationNames(migrations)
+	if !slices.Equal(got, want) {
+		t.Fatalf("applied %v, want %v", got, want)
+	}
+	payload := reportPayload(t, db, "r1")
+	if payload != `{"text":"- shipped"}` {
+		t.Fatalf("report payload %s, want it unchanged", payload)
+	}
+}
+
+func TestMigrateRunsMigrationAddedBeforeApplied(t *testing.T) {
+	db := openTestDB(t, filepath.Join(t.TempDir(), "byre.db"))
+	first := migration{"create-a", `CREATE TABLE a (x TEXT)`}
+	other := migration{"create-b", `CREATE TABLE b (x TEXT)`}
+	last := migration{"create-c", `CREATE TABLE c (x TEXT)`}
+
+	err := migrate(db, []migration{first, last})
+	if err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	err = migrate(db, []migration{first, other, last})
+	if err != nil {
+		t.Fatalf("migrate with other branch: %v", err)
+	}
+
+	got := appliedNames(t, db)
+	want := []string{"create-a", "create-c", "create-b"}
+	if !slices.Equal(got, want) {
+		t.Fatalf("applied %v, want %v", got, want)
+	}
+	_, err = db.Exec(`INSERT INTO b (x) VALUES ('y')`)
+	if err != nil {
+		t.Fatalf("table from other branch: %v", err)
+	}
+}
+
+func TestFlattenReportPayloadsRunsTwice(t *testing.T) {
+	db := openTestDB(t, filepath.Join(t.TempDir(), "byre.db"))
+	err := migrate(db, migrations)
+	if err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	insertReport(t, db, "r1", `{"done":["one","two"]}`)
+
+	i := slices.IndexFunc(migrations, func(m migration) bool { return m.name == "flatten-report-payloads-to-text" })
+	for range 2 {
+		_, err = db.Exec(migrations[i].sql)
+		if err != nil {
+			t.Fatalf("flatten: %v", err)
+		}
+		payload := reportPayload(t, db, "r1")
+		if payload != `{"text":"- one\n- two"}` {
+			t.Fatalf("report payload %s", payload)
+		}
+	}
+}

internal/store/store.go

@@ -18,7 +18,12 @@ import (
 
 var ErrNotFound = errors.New("not found")
 
-var migrations = []string{`
+type migration struct {
+	name string
+	sql  string
+}
+
+var migrations = []migration{{"create-core-tables", `
 CREATE TABLE IF NOT EXISTS users (
 	login TEXT PRIMARY KEY,
 	display_name TEXT NOT NULL,
@@ -61,7 +66,7 @@ CREATE TABLE IF NOT EXISTS events (
 	payload TEXT NOT NULL,
 	created_at TEXT NOT NULL
 );
-`, `
+`}, {"add-agent-sessions", `
 ALTER TABLE events ADD COLUMN agent_session_id TEXT;
 
 CREATE TABLE agent_sessions (
@@ -78,7 +83,7 @@ CREATE TABLE agent_sessions (
 
 CREATE INDEX agent_sessions_task ON agent_sessions(task_id, created_at);
 CREATE INDEX agent_sessions_state ON agent_sessions(state);
-`, `
+`}, {"add-inbox", `
 CREATE TABLE inbox_items (
 	id TEXT PRIMARY KEY,
 	project_id TEXT NOT NULL REFERENCES projects(id),
@@ -113,28 +118,29 @@ CREATE TABLE inbox_responses (
 );
 
 CREATE INDEX inbox_responses_item ON inbox_responses(item_id, created_at);
-`, `
+`}, {"index-events-by-task", `
 CREATE INDEX events_task ON events(task_id, seq);
-`, `
+`}, {"add-turn-queue", `
 CREATE TABLE turn_queue (
 	session_id TEXT PRIMARY KEY REFERENCES agent_sessions(id),
 	actor TEXT NOT NULL,
 	text TEXT NOT NULL,
 	created_at TEXT NOT NULL
 );
-`, `
+`}, {"merge-response-types-into-other", `
 UPDATE inbox_responses SET type = 'other' WHERE type IN ('free_form', 'redirect', 'question');
 UPDATE inbox_items SET state = 'answered' WHERE state = 'redirected';
-`, `
-UPDATE inbox_items SET payload = json_object('text', coalesce((SELECT group_concat('- ' || value, char(10)) FROM json_each(inbox_items.payload, '$.done')), '')) WHERE kind = 'report';
-`, `
+`}, {"flatten-report-payloads-to-text", `
+UPDATE inbox_items SET payload = json_object('text', coalesce((SELECT group_concat('- ' || value, char(10)) FROM json_each(inbox_items.payload, '$.done')), ''))
+WHERE kind = 'report' AND json_type(payload, '$.done') IS NOT NULL;
+`}, {"rename-completed-tasks-to-discarded", `
 ALTER TABLE tasks RENAME COLUMN completed_at TO closed_at;
 ALTER TABLE tasks ADD COLUMN merge_commit TEXT NOT NULL DEFAULT '';
 UPDATE tasks SET status = 'discarded' WHERE status = 'completed';
-`, `
+`}, {"dismiss-inbox-of-closed-tasks", `
 UPDATE inbox_items SET state = 'dismissed', resolved_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now'), snoozed_until = NULL
 WHERE state = 'open' AND task_id IN (SELECT id FROM tasks WHERE status != 'active');
-`}
+`}}
 
 type Store struct {
 	db      *sql.DB
@@ -150,7 +156,7 @@ func Open(path string, onEvent func(api.Event)) (*Store, error) {
 		return nil, err
 	}
 
-	err = migrate(db)
+	err = migrate(db, migrations)
 	if err != nil {
 		db.Close()
 		return nil, err
@@ -159,40 +165,107 @@ func Open(path string, onEvent func(api.Event)) (*Store, error) {
 	return &Store{db: db, path: path, onEvent: onEvent}, nil
 }
 
-func migrate(db *sql.DB) error {
-	version := 0
-	err := db.QueryRow(`PRAGMA user_version`).Scan(&version)
+func migrate(db *sql.DB, migrations []migration) error {
+	err := createMigrationsTable(db, migrations)
 	if err != nil {
 		return err
 	}
 
-	for i := version; i < len(migrations); i++ {
-		tx, err := db.Begin()
-		if err != nil {
-			return err
+	applied, err := appliedMigrations(db)
+	if err != nil {
+		return err
+	}
+
+	for _, m := range migrations {
+		if applied[m.name] {
+			continue
 		}
 
-		_, err = tx.Exec(migrations[i])
+		err = applyMigration(db, m)
 		if err != nil {
-			tx.Rollback()
-			return fmt.Errorf("migration %d: %w", i, err)
-		}
-
-		_, err = tx.Exec(fmt.Sprintf(`PRAGMA user_version = %d`, i+1))
-		if err != nil {
-			tx.Rollback()
-			return fmt.Errorf("migration %d: %w", i, err)
-		}
-
-		err = tx.Commit()
-		if err != nil {
-			return fmt.Errorf("migration %d: %w", i, err)
+			return fmt.Errorf("migration %s: %w", m.name, err)
 		}
 	}
 
 	return nil
 }
 
+func createMigrationsTable(db *sql.DB, migrations []migration) error {
+	tx, err := db.Begin()
+	if err != nil {
+		return err
+	}
+	defer tx.Rollback()
+
+	exists := 0
+	err = tx.QueryRow(`SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = 'migrations'`).Scan(&exists)
+	if err != nil {
+		return err
+	}
+	if exists > 0 {
+		return nil
+	}
+
+	_, err = tx.Exec(`CREATE TABLE migrations (name TEXT PRIMARY KEY, applied_at TEXT NOT NULL)`)
+	if err != nil {
+		return err
+	}
+
+	version := 0
+	err = tx.QueryRow(`PRAGMA user_version`).Scan(&version)
+	if err != nil {
+		return err
+	}
+
+	for _, m := range migrations[:min(version, len(migrations))] {
+		_, err = tx.Exec(`INSERT INTO migrations (name, applied_at) VALUES (?, ?)`, m.name, now())
+		if err != nil {
+			return err
+		}
+	}
+
+	return tx.Commit()
+}
+
+func appliedMigrations(db *sql.DB) (map[string]bool, error) {
+	rows, err := db.Query(`SELECT name FROM migrations`)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+
+	applied := map[string]bool{}
+	for rows.Next() {
+		name := ""
+		err = rows.Scan(&name)
+		if err != nil {
+			return nil, err
+		}
+		applied[name] = true
+	}
+	return applied, rows.Err()
+}
+
+func applyMigration(db *sql.DB, m migration) error {
+	tx, err := db.Begin()
+	if err != nil {
+		return err
+	}
+	defer tx.Rollback()
+
+	_, err = tx.Exec(m.sql)
+	if err != nil {
+		return err
+	}
+
+	_, err = tx.Exec(`INSERT INTO migrations (name, applied_at) VALUES (?, ?)`, m.name, now())
+	if err != nil {
+		return err
+	}
+
+	return tx.Commit()
+}
+
 func (s *Store) Close() error {
 	return s.db.Close()
 }