diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_election_test.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_election_test.go new file mode 100644 index 0000000000..3cc06dbf12 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_election_test.go @@ -0,0 +1,249 @@ +package sqlite + +import ( + "context" + "strings" + "testing" + "time" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" + + "github.com/kandev/kandev/internal/common/logger" + dbutil "github.com/kandev/kandev/internal/db" +) + +// TestCutover_ElectsNewestWorktreeForContestedSlot proves that two live +// sessions of the same task holding different worktrees for one repository +// slot no longer abort the upgrade: the newest worktree owns the slot and the +// older one is demoted to history. +func TestCutover_ElectsNewestWorktreeForContestedSlot(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + older := now.Add(-2 * time.Hour) + if _, err := db.Exec(db.Rebind(` + INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) + VALUES ('task-conflict', 'ws-1', 'legacy', ?, ?)`), older, now); err != nil { + t.Fatalf("seed task: %v", err) + } + for _, s := range []string{"sess-c1", "sess-c2"} { + if _, err := db.Exec(db.Rebind(` + INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) + VALUES (?, 'task-conflict', 'RUNNING', ?, ?)`), s, older, now); err != nil { + t.Fatalf("seed session: %v", err) + } + } + seedLegacySessionWorktree(t, db, "sess-c1", "wt-c1", "repo-c", "", "/t/c1", "feature/c1", "active", older) + seedLegacySessionWorktree(t, db, "sess-c2", "wt-c2", "repo-c", "", "/t/c2", "feature/c2", "active", now) + + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("cutover: %v", err) + } + env, err := repo.GetTaskEnvironmentByTaskID(context.Background(), "task-conflict") + if err != nil { + t.Fatalf("GetTaskEnvironmentByTaskID: %v", err) + } + if env == nil { + t.Fatal("contested task must keep an environment") + } + if len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-c2" { + t.Fatalf("elected repository rows = %+v, want one wt-c2 row", env.Repos) + } + if legacyTableExists(t, db, "task_session_worktrees") { + t.Fatal("legacy table must be dropped once the cutover completes") + } +} + +// TestCutover_FlatEnvironmentOutranksLiveSessionWorktree reproduces the +// upgrade failure from issue #2505: a database with no canonical repository +// row where a still-live session holds an older worktree for the slot the +// surviving flat environment owns. The flat environment keeps the slot. +func TestCutover_FlatEnvironmentOutranksLiveSessionWorktree(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{envID: "env-live", taskID: "task-live", repoID: "repo-live", sessionID: "sess-live"} + seedLegacyTask(t, db, seed, now.Add(-3*time.Hour)) + if _, err := db.Exec(db.Rebind(`UPDATE task_sessions SET state = 'IDLE' WHERE id = ?`), seed.sessionID); err != nil { + t.Fatalf("set session state: %v", err) + } + seedLegacyFlatEnv(t, db, seed, "wt-current", "/tasks/live/repo", "feature/current", now) + seedLegacySessionWorktree(t, db, seed.sessionID, "wt-old", "repo-live", "", "/tasks/live/old", "feature/old", "active", now.Add(-3*time.Hour)) + + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("cutover: %v", err) + } + env, err := repo.GetTaskEnvironment(context.Background(), seed.envID) + if err != nil { + t.Fatalf("get task environment: %v", err) + } + if len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-current" { + t.Fatalf("repository rows = %+v, want one wt-current row", env.Repos) + } + session, err := repo.GetTaskSession(context.Background(), seed.sessionID) + if err != nil { + t.Fatalf("get task session: %v", err) + } + if session.TaskEnvironmentID != seed.envID { + t.Fatalf("session environment = %q, want %q", session.TaskEnvironmentID, seed.envID) + } +} + +// TestCutover_LiveSessionOutranksTerminalSessionForSlot proves the election +// prefers a live session's worktree over a terminal session's newer one when +// no canonical owner exists for the slot. +func TestCutover_LiveSessionOutranksTerminalSessionForSlot(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + if _, err := db.Exec(db.Rebind(` + INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) + VALUES ('task-mixed', 'ws-1', 'legacy', ?, ?)`), now, now); err != nil { + t.Fatalf("seed task: %v", err) + } + for _, session := range []struct{ id, state string }{{"sess-live", "WAITING_FOR_INPUT"}, {"sess-done", "COMPLETED"}} { + if _, err := db.Exec(db.Rebind(` + INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) + VALUES (?, 'task-mixed', ?, ?, ?)`), session.id, session.state, now, now); err != nil { + t.Fatalf("seed session: %v", err) + } + } + seedLegacySessionWorktree(t, db, "sess-live", "wt-live", "repo-mixed", "", "/t/live", "feature/live", "active", now.Add(-time.Hour)) + seedLegacySessionWorktree(t, db, "sess-done", "wt-done", "repo-mixed", "", "/t/done", "feature/done", "active", now) + + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("cutover: %v", err) + } + env, err := repo.GetTaskEnvironmentByTaskID(context.Background(), "task-mixed") + if err != nil { + t.Fatalf("GetTaskEnvironmentByTaskID: %v", err) + } + if env == nil || len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-live" { + t.Fatalf("elected repository rows = %+v, want one wt-live row", env) + } +} + +// TestCutover_IrreconcilableMetadataFailsClosed proves the cutover still +// refuses to guess when two live sessions disagree about the path of the same +// physical worktree and no canonical owner can settle it. +func TestCutover_IrreconcilableMetadataFailsClosed(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + if _, err := db.Exec(db.Rebind(` + INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) + VALUES ('task-metadata', 'ws-1', 'legacy', ?, ?)`), now, now); err != nil { + t.Fatalf("seed task: %v", err) + } + for _, s := range []string{"sess-m1", "sess-m2"} { + if _, err := db.Exec(db.Rebind(` + INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) + VALUES (?, 'task-metadata', 'RUNNING', ?, ?)`), s, now, now); err != nil { + t.Fatalf("seed session: %v", err) + } + } + seedLegacySessionWorktree(t, db, "sess-m1", "wt-m", "repo-m", "", "/t/one", "feature/m", "active", now) + seedLegacySessionWorktree(t, db, "sess-m2", "wt-m", "repo-m", "", "/t/two", "feature/m", "active", now) + + if _, err := NewWithDB(db, db, nil); err == nil { + t.Fatal("expected cutover to fail on irreconcilable worktree metadata") + } else if !strings.Contains(err.Error(), "conflict") { + t.Fatalf("error must describe the conflict, got: %v", err) + } + // Pre-upgrade state must be intact. + if !legacyTableExists(t, db, "task_session_worktrees") { + t.Fatal("legacy table must survive a failed cutover") + } + envCols := tableColumnSet(t, db, "task_environments") + if !envCols["worktree_id"] || !envCols["repository_id"] { + t.Fatal("legacy flat columns must survive a failed cutover") + } +} + +// TestCutoverElection_RecordsDemotionDiagnostics proves every demoted legacy +// worktree is recorded with the directory and branch support needs to find it +// on disk, since the migration itself never touches the filesystem. +func TestCutoverElection_RecordsDemotionDiagnostics(t *testing.T) { + now := time.Now().UTC() + cut := &worktreeCutover{ + envs: map[string]*legacyEnv{}, + sessions: map[string]*legacySession{}, + sessionTasks: map[string]string{}, + sessionEnvIDs: map[string]string{}, + sessionWorktreeSuperseded: map[string]bool{}, + demotedWorktrees: map[string]bool{}, + authoritativeWorktreeIDs: map[string]bool{}, + tasks: map[string]*taskWorktreeTargets{}, + taskEnvIDs: map[string]string{}, + loserEnvIDs: map[string]bool{}, + executorTypes: map[string]string{}, + } + for _, id := range []string{"sess-old", "sess-new"} { + cut.sessions[id] = &legacySession{id: id, taskID: "task-1", state: "RUNNING"} + cut.sessionTasks[id] = "task-1" + } + cut.sessionWts = []legacySessionWorktree{ + {sessionID: "sess-old", worktreeID: "wt-old", repositoryID: "repo-1", + worktreePath: "/tasks/old", worktreeBranch: "feature/old", + createdAt: now.Add(-time.Hour), updatedAt: now.Add(-time.Hour), status: "active"}, + {sessionID: "sess-new", worktreeID: "wt-new", repositoryID: "repo-1", + worktreePath: "/tasks/new", worktreeBranch: "feature/new", + createdAt: now, updatedAt: now, status: "active"}, + } + + cut.electSlotWorktreeOwners() + + if !cut.demotedWorktrees[sessionWorktreeKey("sess-old", "wt-old")] { + t.Fatal("older worktree must be demoted") + } + if cut.demotedWorktrees[sessionWorktreeKey("sess-new", "wt-new")] { + t.Fatal("elected worktree must not be demoted") + } + if len(cut.demotions) != 1 { + t.Fatalf("demotions = %v, want exactly one entry", cut.demotions) + } + for _, want := range []string{"sess-old", "wt-old", "/tasks/old", "feature/old", "wt-new"} { + if !strings.Contains(cut.demotions[0], want) { + t.Fatalf("demotion %q must mention %q", cut.demotions[0], want) + } + } +} + +// TestCutoverElection_DoesNotLogRolledBackDemotions proves operators only see +// demotion diagnostics after the ownership transaction commits successfully. +func TestCutoverElection_DoesNotLogRolledBackDemotions(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + if _, err := db.Exec(` + INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) + VALUES ('task-log', 'ws-1', 'legacy', ?, ?)`, now, now); err != nil { + t.Fatalf("seed task: %v", err) + } + for _, sessionID := range []string{"sess-log-old", "sess-log-new"} { + if _, err := db.Exec(` + INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) + VALUES (?, 'task-log', 'RUNNING', ?, ?)`, sessionID, now, now); err != nil { + t.Fatalf("seed session: %v", err) + } + } + seedLegacySessionWorktree(t, db, "sess-log-old", "wt-log-old", "repo-log", "", "/tasks/old", "feature/old", "active", now.Add(-time.Hour)) + seedLegacySessionWorktree(t, db, "sess-log-new", "wt-log-new", "repo-log", "", "/tasks/new", "feature/new", "active", now) + + core, logs := observer.New(zapcore.WarnLevel) + log, err := logger.NewFromZap(zap.New(core)) + if err != nil { + t.Fatalf("create observer logger: %v", err) + } + repo := &Repository{ + db: db, ro: db, log: log, migrate: dbutil.NewMigrateLogger(db, log), + failCutoverAfter: "pre_swap", + } + if err := repo.initSchema(); err == nil { + t.Fatal("expected injected cutover failure") + } + message := "cutover: duplicate legacy worktrees demoted to history" + if entries := logs.FilterMessage(message).All(); len(entries) != 0 { + t.Fatalf("rolled-back demotion logs = %d, want none", len(entries)) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go index 599b679a21..472c10dce4 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/jmoiron/sqlx" + "go.uber.org/zap" "github.com/kandev/kandev/internal/db/dialect" ) @@ -117,6 +118,7 @@ func (r *Repository) normalizeTaskWorktreeOwnership() error { sessionTasks: make(map[string]string), sessionEnvIDs: make(map[string]string), sessionWorktreeSuperseded: make(map[string]bool), + demotedWorktrees: make(map[string]bool), authoritativeWorktreeIDs: make(map[string]bool), tasks: make(map[string]*taskWorktreeTargets), taskEnvIDs: make(map[string]string), @@ -150,9 +152,22 @@ func (r *Repository) normalizeTaskWorktreeOwnership() error { if err := tx.Commit(); err != nil { return fmt.Errorf("cutover: commit: %w", err) } + r.logCutoverDemotions(cut) return nil } +// logCutoverDemotions reports every legacy worktree that lost a contested +// repository slot. The migration performs no filesystem operation, so each +// listed directory and branch is still on disk and recoverable by hand. +func (r *Repository) logCutoverDemotions(c *worktreeCutover) { + if len(c.demotions) == 0 || r.log == nil { + return + } + r.log.Warn("cutover: duplicate legacy worktrees demoted to history", + zap.Int("count", len(c.demotions)), + zap.Strings("worktrees", c.demotions)) +} + // cutoverAcquireLocks serializes the cutover for the active database engine: // PostgreSQL takes a migration advisory lock plus exclusive locks on every // affected ownership table (aborting on lock timeout); SQLite relies on the diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go index 20caa68bfd..b1189685e5 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go @@ -3,14 +3,12 @@ package sqlite import ( "context" "path/filepath" - "strings" "testing" "time" "github.com/jmoiron/sqlx" dbutil "github.com/kandev/kandev/internal/db" - "github.com/kandev/kandev/internal/testutil" ) // --------------------------------------------------------------------------- @@ -456,44 +454,6 @@ func TestCutover_PurgesPreviewCleanupJobs(t *testing.T) { } } -// TestCutover_ConflictingWorktreesFailClosed proves that two sessions of the -// same task with different worktrees for the same repository slot abort the -// cutover and leave the legacy database intact. -func TestCutover_ConflictingWorktreesFailClosed(t *testing.T) { - db := openLegacyDB(t) - now := time.Now().UTC().Truncate(time.Second) - if _, err := db.Exec(` - INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) - VALUES ('task-conflict', 'ws-1', 'legacy', ?, ?)`, now, now); err != nil { - t.Fatalf("seed task: %v", err) - } - for _, s := range []string{"sess-c1", "sess-c2"} { - if _, err := db.Exec(` - INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) - VALUES (?, 'task-conflict', 'RUNNING', ?, ?)`, s, now, now); err != nil { - t.Fatalf("seed session: %v", err) - } - } - seedLegacySessionWorktree(t, db, "sess-c1", "wt-c1", "repo-c", "", "/t/c1", "feature/c1", "active", now) - seedLegacySessionWorktree(t, db, "sess-c2", "wt-c2", "repo-c", "", "/t/c2", "feature/c2", "active", now) - - _, err := NewWithDB(db, db, nil) - if err == nil { - t.Fatal("expected cutover to fail on conflicting worktree identities") - } - if !strings.Contains(err.Error(), "conflict") { - t.Fatalf("error must describe the conflict, got: %v", err) - } - // Pre-upgrade state must be intact. - if !legacyTableExists(t, db, "task_session_worktrees") { - t.Fatal("legacy table must survive a failed cutover") - } - envCols := tableColumnSet(t, db, "task_environments") - if !envCols["worktree_id"] || !envCols["repository_id"] { - t.Fatal("legacy flat columns must survive a failed cutover") - } -} - // TestCutover_RollbackAtEveryFailpoint injects a failure at each cutover // step and proves the transaction restores the complete legacy schema and // data. @@ -703,35 +663,6 @@ func TestCutover_PreservesTerminalWorktreeOutsideFlatOwnerSlot(t *testing.T) { } } -// TestCutover_RejectsLiveWorktreeDifferentFromFlatOwner proves that a -// non-terminal session with a different physical identity still fails closed. -func TestCutover_RejectsLiveWorktreeDifferentFromFlatOwner(t *testing.T) { - for _, state := range []string{"RUNNING", "WAITING_FOR_INPUT"} { - t.Run(state, func(t *testing.T) { - db := openLegacyDB(t) - now := time.Now().UTC().Truncate(time.Second) - seed := legacySeed{envID: "env-flat-live", taskID: "task-flat-live", repoID: "repo-flat-live", sessionID: "sess-flat-live"} - seedLegacyTask(t, db, seed, now) - if _, err := db.Exec(db.Rebind(`UPDATE task_sessions SET state = ? WHERE id = ?`), state, seed.sessionID); err != nil { - t.Fatalf("set session state: %v", err) - } - seedLegacyFlatEnv(t, db, seed, "wt-flat-current", "/tasks/flat-live/repo", "feature/current", now) - seedLegacySessionWorktree(t, db, seed.sessionID, "wt-flat-live", seed.repoID, "", "/tasks/flat-live/old", "feature/live", "active", now) - - _, err := NewWithDB(db, db, nil) - if err == nil { - t.Fatal("expected live worktree conflict") - } - if !strings.Contains(err.Error(), "conflict") { - t.Fatalf("error must describe the conflict, got: %v", err) - } - if !legacyTableExists(t, db, "task_session_worktrees") { - t.Fatal("legacy table must survive a failed cutover") - } - }) - } -} - // TestCutover_IgnoresDeletedHistoricalSessionConflict proves that a deleted // session reference cannot override an existing task-owned repository row. func TestCutover_IgnoresDeletedHistoricalSessionConflict(t *testing.T) { @@ -952,212 +883,3 @@ func linkSessionEnvironment(t *testing.T, db *sqlx.DB, sessionID, envID string) t.Fatalf("link session environment: %v", err) } } - -// --------------------------------------------------------------------------- -// PostgreSQL coverage -// --------------------------------------------------------------------------- - -func openLegacyPostgres(t *testing.T) *sqlx.DB { - t.Helper() - db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) - if _, err := NewWithDB(db, db, nil); err != nil { - t.Fatalf("seed baseline postgres schema: %v", err) - } - rewindToLegacySchema(t, db) - return db -} - -func TestCutoverPostgres_NormalizesLegacyFlatEnvironment(t *testing.T) { - db := openLegacyPostgres(t) - ctx := context.Background() - now := time.Now().UTC().Truncate(time.Second) - seed := legacySeed{envID: "env-pg1", taskID: "task-pg1", repoID: "repo-pg1", sessionID: "sess-pg1"} - seedLegacyTask(t, db, seed, now) - seedLegacySessionWorktree(t, db, "sess-pg1", "wt-pg1", "repo-pg1", "", "/tasks/pg1/kandev", "feature/pg", "active", now) - seedLegacyFlatEnv(t, db, seed, "wt-pg1", "/tasks/pg1/kandev", "feature/pg", now) - seedCleanupJob(t, db, "job-pg-session-delete", "task-pg1", "session_delete", "pending", now) - seedCleanupJob(t, db, "job-pg-archive", "task-pg1", "archive", "pending", now) - - repo, err := NewWithDB(db, db, nil) - if err != nil { - t.Fatalf("postgres cutover: %v", err) - } - env, err := repo.GetTaskEnvironment(ctx, "env-pg1") - if err != nil { - t.Fatalf("GetTaskEnvironment: %v", err) - } - if len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-pg1" || env.Repos[0].Status != "active" { - t.Fatalf("normalized repos = %+v", env.Repos) - } - session, err := repo.GetTaskSession(ctx, "sess-pg1") - if err != nil { - t.Fatalf("GetTaskSession: %v", err) - } - if session.TaskEnvironmentID != "env-pg1" { - t.Fatalf("session env = %q, want env-pg1", session.TaskEnvironmentID) - } - var tableCount int - if err := db.Get(&tableCount, ` - SELECT COUNT(*) FROM information_schema.tables - WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { - t.Fatalf("count legacy table: %v", err) - } - if tableCount != 0 { - t.Fatal("task_session_worktrees must be dropped on postgres") - } - var jobCount int - if err := db.Get(&jobCount, `SELECT COUNT(*) FROM task_resource_cleanup_jobs`); err != nil { - t.Fatalf("count jobs: %v", err) - } - if jobCount != 1 { - t.Fatalf("postgres cleanup jobs after cutover = %d, want 1", jobCount) - } -} - -func TestCutoverPostgres_SessionOnlyNormalization(t *testing.T) { - db := openLegacyPostgres(t) - ctx := context.Background() - now := time.Now().UTC().Truncate(time.Second) - seed := legacySeed{taskID: "task-pg2", sessionID: "sess-pg2"} - seedLegacyTask(t, db, seed, now) - seedLegacySessionWorktree(t, db, "sess-pg2", "wt-pg2", "repo-pg2", "", "/tasks/pg2/kandev", "feature/pg2", "active", now) - - repo, err := NewWithDB(db, db, nil) - if err != nil { - t.Fatalf("postgres cutover: %v", err) - } - env, err := repo.GetTaskEnvironmentByTaskID(ctx, "task-pg2") - if err != nil { - t.Fatalf("GetTaskEnvironmentByTaskID: %v", err) - } - if env == nil || len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-pg2" { - t.Fatalf("normalized env = %+v", env) - } -} - -func TestCutoverPostgres_ConflictingWorktreesFailClosed(t *testing.T) { - db := openLegacyPostgres(t) - now := time.Now().UTC().Truncate(time.Second) - if _, err := db.Exec(db.Rebind(` - INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) - VALUES (?, 'ws-1', 'legacy', ?, ?)`), "task-pg-conflict", now, now); err != nil { - t.Fatalf("seed task: %v", err) - } - for _, s := range []string{"sess-pgc1", "sess-pgc2"} { - if _, err := db.Exec(db.Rebind(` - INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) - VALUES (?, 'task-pg-conflict', 'COMPLETED', ?, ?)`), s, now, now); err != nil { - t.Fatalf("seed session: %v", err) - } - } - seedLegacySessionWorktree(t, db, "sess-pgc1", "wt-pgc1", "repo-pgc", "", "/t/pgc1", "feature/a", "active", now) - seedLegacySessionWorktree(t, db, "sess-pgc2", "wt-pgc2", "repo-pgc", "", "/t/pgc2", "feature/b", "active", now) - - _, err := NewWithDB(db, db, nil) - if err == nil { - t.Fatal("expected postgres cutover to fail on conflicting worktrees") - } - var tableCount int - if err := db.Get(&tableCount, ` - SELECT COUNT(*) FROM information_schema.tables - WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { - t.Fatalf("count legacy table: %v", err) - } - if tableCount != 1 { - t.Fatal("failed postgres cutover must leave the legacy table intact") - } -} - -// TestCutoverPostgres_ReplayIsNoOp proves a second boot skips the cutover. -func TestCutoverPostgres_ReplayIsNoOp(t *testing.T) { - db := openLegacyPostgres(t) - now := time.Now().UTC().Truncate(time.Second) - seed := legacySeed{envID: "env-pg3", taskID: "task-pg3", repoID: "repo-pg3", sessionID: "sess-pg3"} - seedLegacyTask(t, db, seed, now) - seedLegacySessionWorktree(t, db, "sess-pg3", "wt-pg3", "repo-pg3", "", "/tasks/pg3/kandev", "feature/pg3", "active", now) - seedLegacyFlatEnv(t, db, seed, "wt-pg3", "/tasks/pg3/kandev", "feature/pg3", now) - - if _, err := NewWithDB(db, db, nil); err != nil { - t.Fatalf("first boot: %v", err) - } - if _, err := NewWithDB(db, db, nil); err != nil { - t.Fatalf("second boot (replay): %v", err) - } -} - -// TestCutoverPostgres_RollbackAtEveryFailpoint mirrors the SQLite failpoint -// matrix on PostgreSQL. -func TestCutoverPostgres_RollbackAtEveryFailpoint(t *testing.T) { - for _, step := range []string{ - "create_shadow", "copy_envs", "backfill_repos", "link_sessions", - "validate", "pre_swap", "drop_legacy", "swap", "post_swap", - } { - t.Run(step, func(t *testing.T) { - db := openLegacyPostgres(t) - now := time.Now().UTC().Truncate(time.Second) - seed := legacySeed{envID: "env-pgr", taskID: "task-pgr", repoID: "repo-pgr", sessionID: "sess-pgr"} - seedLegacyTask(t, db, seed, now) - seedLegacySessionWorktree(t, db, "sess-pgr", "wt-pgr", "repo-pgr", "", "/tasks/pgr/kandev", "feature/pgr", "active", now) - seedLegacyFlatEnv(t, db, seed, "wt-pgr", "/tasks/pgr/kandev", "feature/pgr", now) - - repo := &Repository{db: db, ro: db, migrate: dbutil.NewMigrateLogger(db, nil), failCutoverAfter: step} - err := repo.initSchema() - if err == nil { - t.Fatalf("expected injected failure at %s", step) - } - var tableCount int - if err := db.Get(&tableCount, ` - SELECT COUNT(*) FROM information_schema.tables - WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { - t.Fatalf("count legacy table: %v", err) - } - if tableCount != 1 { - t.Fatalf("legacy table must survive rollback at %s", step) - } - var path string - if err := db.Get(&path, db.Rebind(` - SELECT worktree_path FROM task_session_worktrees WHERE worktree_id = ?`), "wt-pgr"); err != nil { - t.Fatalf("legacy worktree row lost after rollback at %s: %v", step, err) - } - if path != "/tasks/pgr/kandev" { - t.Fatalf("legacy worktree data changed after rollback at %s", step) - } - }) - } -} - -// TestCutoverPostgres_FreshSchemaIsFinal proves a fresh PostgreSQL database -// contains only the final schema. -func TestCutoverPostgres_FreshSchemaIsFinal(t *testing.T) { - db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) - if _, err := NewWithDB(db, db, nil); err != nil { - t.Fatalf("init fresh postgres schema: %v", err) - } - var tableCount int - if err := db.Get(&tableCount, ` - SELECT COUNT(*) FROM information_schema.tables - WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { - t.Fatalf("count legacy table: %v", err) - } - if tableCount != 0 { - t.Fatal("fresh postgres database must not contain task_session_worktrees") - } - // The environment FK must reference tasks and the repos FK must - // reference task_environments after the swap. - var fkCount int - if err := db.Get(&fkCount, ` - SELECT COUNT(*) FROM information_schema.table_constraints tc - JOIN information_schema.referential_constraints rc - ON rc.constraint_name = tc.constraint_name AND rc.constraint_schema = tc.constraint_schema - JOIN information_schema.constraint_column_usage ccu - ON ccu.constraint_name = rc.unique_constraint_name AND ccu.constraint_schema = rc.unique_constraint_schema - WHERE tc.table_name = 'task_environment_repos' - AND tc.constraint_type = 'FOREIGN KEY' - AND ccu.table_name = 'task_environments' - AND tc.table_schema = current_schema()`); err != nil { - t.Fatalf("count env-repo FK: %v", err) - } - if fkCount != 1 { - t.Fatalf("task_environment_repos FK to task_environments missing: %d", fkCount) - } -} diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go index 4d5c1d5373..e9fcdcc538 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go @@ -20,6 +20,8 @@ type worktreeCutover struct { sessionTasks map[string]string sessionEnvIDs map[string]string sessionWorktreeSuperseded map[string]bool + demotedWorktrees map[string]bool + demotions []string authoritativeWorktreeIDs map[string]bool envRepos []legacyEnvRepo sessionWts []legacySessionWorktree @@ -252,6 +254,7 @@ func (c *worktreeCutover) normalize(tx *sqlx.Tx) error { // worktree belongs to the environment the session resolves to, which is // not always an environment owned by the session's own task. c.linkSessions() + c.electSlotWorktreeOwners() c.mergeSessionWorktrees() c.registerWorktreeIdentities() c.bindTargetsToEnvironments() @@ -326,6 +329,132 @@ func (c *worktreeCutover) mergeFlatEnvironmentFields() { } } +// worktreeSlot is one normalized repository slot — a (repository, branch +// slug) pair of a single task — plus the legacy session rows competing to own +// it. +type worktreeSlot struct { + targets *taskWorktreeTargets + key string + rows []legacySessionWorktree +} + +// electSlotWorktreeOwners resolves the one legacy shape the task-owned model +// cannot represent: several distinct physical worktrees claiming the same +// (repository, branch slug) slot of one task. The per-session model produced +// these routinely — a handoff, a re-materialized workspace, or a second +// session each minted their own worktree for the same repository — so failing +// the upgrade would strand those databases on the previous binary. +// +// A canonical repository row or the surviving flat environment owns the slot +// outright; otherwise the newest live session worktree wins. Every other row +// is demoted to historical evidence: the migration touches no filesystem, so +// the demoted directory and branch stay on disk, but the normalized schema +// records exactly one owner per slot. +func (c *worktreeCutover) electSlotWorktreeOwners() { + slots, order := c.collectSlotContenders() + for _, id := range order { + slot := slots[id] + winner := slot.currentOwner() + if winner == "" { + winner = c.bestSlotOwner(slot.rows) + } + for _, row := range slot.rows { + if row.worktreeID == winner { + continue + } + c.demoteSessionWorktree(row, winner) + } + } +} + +// collectSlotContenders groups every legacy session worktree that still needs +// a home by the slot it would claim, preserving row order so the election is +// deterministic. +func (c *worktreeCutover) collectSlotContenders() (map[string]*worktreeSlot, []string) { + slots := make(map[string]*worktreeSlot) + var order []string + for _, wt := range c.sessionWts { + if wt.worktreeID == "" || isLegacyDeletedWorktree(wt) { + continue + } + taskID := c.sessionOwnerTaskID(wt.sessionID) + targets := c.targetsForTask(taskID) + if targets.targetForWorktree(wt.worktreeID) != nil { + continue // a canonical source already owns this physical worktree + } + key := envRepoKey(wt.repositoryID, wt.branchSlug) + id := taskID + "\x00" + key + slot, ok := slots[id] + if !ok { + slot = &worktreeSlot{targets: targets, key: key} + slots[id] = slot + order = append(order, id) + } + slot.rows = append(slot.rows, wt) + } + return slots, order +} + +// currentOwner returns the physical worktree a canonical source already +// homed on this slot, or "" when the slot is still unclaimed. +func (s *worktreeSlot) currentOwner() string { + target, ok := s.targets.byKey[s.key] + if !ok { + return "" + } + return target.worktreeID +} + +// bestSlotOwner picks the worktree that keeps an otherwise unclaimed slot. +func (c *worktreeCutover) bestSlotOwner(rows []legacySessionWorktree) string { + best := rows[0] + for _, row := range rows[1:] { + if c.outranksSlotOwner(row, best) { + best = row + } + } + return best.worktreeID +} + +// outranksSlotOwner orders two competing rows: a live session's worktree +// beats a terminal session's, then the most recently updated row wins, with +// creation time and identity as stable tie-breakers. +func (c *worktreeCutover) outranksSlotOwner(candidate, best legacySessionWorktree) bool { + candidateHistorical := c.isHistoricalSessionRow(candidate) + bestHistorical := c.isHistoricalSessionRow(best) + if candidateHistorical != bestHistorical { + return bestHistorical + } + if !candidate.updatedAt.Equal(best.updatedAt) { + return candidate.updatedAt.After(best.updatedAt) + } + if !candidate.createdAt.Equal(best.createdAt) { + return candidate.createdAt.After(best.createdAt) + } + return candidate.worktreeID > best.worktreeID +} + +// isHistoricalSessionRow reports whether the row's session is terminal (or +// gone), which makes its worktree the weaker claim on a contested slot. +func (c *worktreeCutover) isHistoricalSessionRow(wt legacySessionWorktree) bool { + session, ok := c.sessions[wt.sessionID] + return !ok || isLegacyHistoricalSession(session.state) +} + +// demoteSessionWorktree records a legacy row as historical evidence for a +// slot another worktree owns, and keeps a human-readable line so the losing +// directory can still be found on disk. +func (c *worktreeCutover) demoteSessionWorktree(wt legacySessionWorktree, winner string) { + key := sessionWorktreeKey(wt.sessionID, wt.worktreeID) + if c.demotedWorktrees[key] { + return + } + c.demotedWorktrees[key] = true + c.demotions = append(c.demotions, fmt.Sprintf( + "session %s worktree %s (repository %s, path %q, branch %q) superseded by worktree %s", + wt.sessionID, wt.worktreeID, wt.repositoryID, wt.worktreePath, wt.worktreeBranch, winner)) +} + // mergeSessionWorktrees folds every legacy session-worktree row into the // targets of the task that owns the session's environment. func (c *worktreeCutover) mergeSessionWorktrees() { @@ -333,6 +462,9 @@ func (c *worktreeCutover) mergeSessionWorktrees() { if _, ok := c.sessions[wt.sessionID]; !ok { continue // already reported in load } + if c.demotedWorktrees[sessionWorktreeKey(wt.sessionID, wt.worktreeID)] { + continue // another physical worktree owns this repository slot + } targets := c.targetsForTask(c.sessionOwnerTaskID(wt.sessionID)) if err := targets.mergeSessionWorktree(wt, c.isSupersededSessionWorktree(wt)); err != nil { c.conflicts = append(c.conflicts, fmt.Sprintf( @@ -472,12 +604,15 @@ func (c *worktreeCutover) linkSessions() { } // isSupersededSessionWorktree reports whether a legacy session row is no -// longer an ownership source. Repository rows and the surviving flat -// environment take precedence for the same physical identity regardless of -// session state. Historical rows with no authoritative replacement remain -// eligible for backfill. +// longer an ownership source. Rows demoted by the slot election, repository +// rows, and the surviving flat environment take precedence for the same +// physical identity regardless of session state. Historical rows with no +// authoritative replacement remain eligible for backfill. func (c *worktreeCutover) isSupersededSessionWorktree(wt legacySessionWorktree) bool { - cacheKey := wt.sessionID + "\x00" + wt.worktreeID + cacheKey := sessionWorktreeKey(wt.sessionID, wt.worktreeID) + if c.demotedWorktrees[cacheKey] { + return true + } if superseded, ok := c.sessionWorktreeSuperseded[cacheKey]; ok { return superseded } @@ -566,6 +701,11 @@ func authoritativeWorktreeKey(taskID, worktreeID string) string { return taskID + "\x00" + worktreeID } +// sessionWorktreeKey identifies one legacy session-worktree row. +func sessionWorktreeKey(sessionID, worktreeID string) string { + return sessionID + "\x00" + worktreeID +} + // registerWorktreeIdentities ensures every physical worktree is owned by // exactly one environment-repository key. func (c *worktreeCutover) registerWorktreeIdentities() { diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_postgres_test.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_postgres_test.go new file mode 100644 index 0000000000..830ed268db --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_postgres_test.go @@ -0,0 +1,266 @@ +package sqlite + +import ( + "context" + "testing" + "time" + + "github.com/jmoiron/sqlx" + + dbutil "github.com/kandev/kandev/internal/db" + "github.com/kandev/kandev/internal/testutil" +) + +// --------------------------------------------------------------------------- +// PostgreSQL coverage +// --------------------------------------------------------------------------- + +func openLegacyPostgres(t *testing.T) *sqlx.DB { + t.Helper() + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + if _, err := NewWithDB(db, db, nil); err != nil { + t.Fatalf("seed baseline postgres schema: %v", err) + } + rewindToLegacySchema(t, db) + return db +} + +func TestCutoverPostgres_NormalizesLegacyFlatEnvironment(t *testing.T) { + db := openLegacyPostgres(t) + ctx := context.Background() + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{envID: "env-pg1", taskID: "task-pg1", repoID: "repo-pg1", sessionID: "sess-pg1"} + seedLegacyTask(t, db, seed, now) + seedLegacySessionWorktree(t, db, "sess-pg1", "wt-pg1", "repo-pg1", "", "/tasks/pg1/kandev", "feature/pg", "active", now) + seedLegacyFlatEnv(t, db, seed, "wt-pg1", "/tasks/pg1/kandev", "feature/pg", now) + seedCleanupJob(t, db, "job-pg-session-delete", "task-pg1", "session_delete", "pending", now) + seedCleanupJob(t, db, "job-pg-archive", "task-pg1", "archive", "pending", now) + + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("postgres cutover: %v", err) + } + env, err := repo.GetTaskEnvironment(ctx, "env-pg1") + if err != nil { + t.Fatalf("GetTaskEnvironment: %v", err) + } + if len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-pg1" || env.Repos[0].Status != "active" { + t.Fatalf("normalized repos = %+v", env.Repos) + } + session, err := repo.GetTaskSession(ctx, "sess-pg1") + if err != nil { + t.Fatalf("GetTaskSession: %v", err) + } + if session.TaskEnvironmentID != "env-pg1" { + t.Fatalf("session env = %q, want env-pg1", session.TaskEnvironmentID) + } + var tableCount int + if err := db.Get(&tableCount, ` + SELECT COUNT(*) FROM information_schema.tables + WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { + t.Fatalf("count legacy table: %v", err) + } + if tableCount != 0 { + t.Fatal("task_session_worktrees must be dropped on postgres") + } + var jobCount int + if err := db.Get(&jobCount, `SELECT COUNT(*) FROM task_resource_cleanup_jobs`); err != nil { + t.Fatalf("count jobs: %v", err) + } + if jobCount != 1 { + t.Fatalf("postgres cleanup jobs after cutover = %d, want 1", jobCount) + } +} + +func TestCutoverPostgres_SessionOnlyNormalization(t *testing.T) { + db := openLegacyPostgres(t) + ctx := context.Background() + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{taskID: "task-pg2", sessionID: "sess-pg2"} + seedLegacyTask(t, db, seed, now) + seedLegacySessionWorktree(t, db, "sess-pg2", "wt-pg2", "repo-pg2", "", "/tasks/pg2/kandev", "feature/pg2", "active", now) + + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("postgres cutover: %v", err) + } + env, err := repo.GetTaskEnvironmentByTaskID(ctx, "task-pg2") + if err != nil { + t.Fatalf("GetTaskEnvironmentByTaskID: %v", err) + } + if env == nil || len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-pg2" { + t.Fatalf("normalized env = %+v", env) + } +} + +// TestCutoverPostgres_ElectsSingleSlotOwner mirrors the SQLite election: two +// legacy worktrees contesting one repository slot resolve to a single owner +// instead of aborting the upgrade. +func TestCutoverPostgres_ElectsSingleSlotOwner(t *testing.T) { + db := openLegacyPostgres(t) + now := time.Now().UTC().Truncate(time.Second) + older := now.Add(-2 * time.Hour) + if _, err := db.Exec(db.Rebind(` + INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) + VALUES (?, 'ws-1', 'legacy', ?, ?)`), "task-pg-conflict", older, now); err != nil { + t.Fatalf("seed task: %v", err) + } + for _, s := range []string{"sess-pgc1", "sess-pgc2"} { + if _, err := db.Exec(db.Rebind(` + INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) + VALUES (?, 'task-pg-conflict', 'COMPLETED', ?, ?)`), s, older, now); err != nil { + t.Fatalf("seed session: %v", err) + } + } + seedLegacySessionWorktree(t, db, "sess-pgc1", "wt-pgc1", "repo-pgc", "", "/t/pgc1", "feature/a", "active", older) + seedLegacySessionWorktree(t, db, "sess-pgc2", "wt-pgc2", "repo-pgc", "", "/t/pgc2", "feature/b", "active", now) + + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("postgres cutover: %v", err) + } + env, err := repo.GetTaskEnvironmentByTaskID(context.Background(), "task-pg-conflict") + if err != nil { + t.Fatalf("GetTaskEnvironmentByTaskID: %v", err) + } + if env == nil || len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-pgc2" { + t.Fatalf("elected postgres env = %+v, want one wt-pgc2 row", env) + } + var tableCount int + if err := db.Get(&tableCount, ` + SELECT COUNT(*) FROM information_schema.tables + WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { + t.Fatalf("count legacy table: %v", err) + } + if tableCount != 0 { + t.Fatal("completed postgres cutover must drop the legacy table") + } +} + +// TestCutoverPostgres_IrreconcilableMetadataFailsClosed proves postgres still +// rolls back when two live sessions disagree about one worktree's path. +func TestCutoverPostgres_IrreconcilableMetadataFailsClosed(t *testing.T) { + db := openLegacyPostgres(t) + now := time.Now().UTC().Truncate(time.Second) + if _, err := db.Exec(db.Rebind(` + INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) + VALUES (?, 'ws-1', 'legacy', ?, ?)`), "task-pg-metadata", now, now); err != nil { + t.Fatalf("seed task: %v", err) + } + for _, s := range []string{"sess-pgm1", "sess-pgm2"} { + if _, err := db.Exec(db.Rebind(` + INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) + VALUES (?, 'task-pg-metadata', 'RUNNING', ?, ?)`), s, now, now); err != nil { + t.Fatalf("seed session: %v", err) + } + } + seedLegacySessionWorktree(t, db, "sess-pgm1", "wt-pgm", "repo-pgm", "", "/t/one", "feature/m", "active", now) + seedLegacySessionWorktree(t, db, "sess-pgm2", "wt-pgm", "repo-pgm", "", "/t/two", "feature/m", "active", now) + + if _, err := NewWithDB(db, db, nil); err == nil { + t.Fatal("expected postgres cutover to fail on irreconcilable metadata") + } + var tableCount int + if err := db.Get(&tableCount, ` + SELECT COUNT(*) FROM information_schema.tables + WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { + t.Fatalf("count legacy table: %v", err) + } + if tableCount != 1 { + t.Fatal("failed postgres cutover must leave the legacy table intact") + } +} + +// TestCutoverPostgres_ReplayIsNoOp proves a second boot skips the cutover. +func TestCutoverPostgres_ReplayIsNoOp(t *testing.T) { + db := openLegacyPostgres(t) + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{envID: "env-pg3", taskID: "task-pg3", repoID: "repo-pg3", sessionID: "sess-pg3"} + seedLegacyTask(t, db, seed, now) + seedLegacySessionWorktree(t, db, "sess-pg3", "wt-pg3", "repo-pg3", "", "/tasks/pg3/kandev", "feature/pg3", "active", now) + seedLegacyFlatEnv(t, db, seed, "wt-pg3", "/tasks/pg3/kandev", "feature/pg3", now) + + if _, err := NewWithDB(db, db, nil); err != nil { + t.Fatalf("first boot: %v", err) + } + if _, err := NewWithDB(db, db, nil); err != nil { + t.Fatalf("second boot (replay): %v", err) + } +} + +// TestCutoverPostgres_RollbackAtEveryFailpoint mirrors the SQLite failpoint +// matrix on PostgreSQL. +func TestCutoverPostgres_RollbackAtEveryFailpoint(t *testing.T) { + for _, step := range []string{ + "create_shadow", "copy_envs", "backfill_repos", "link_sessions", + "validate", "pre_swap", "drop_legacy", "swap", "post_swap", + } { + t.Run(step, func(t *testing.T) { + db := openLegacyPostgres(t) + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{envID: "env-pgr", taskID: "task-pgr", repoID: "repo-pgr", sessionID: "sess-pgr"} + seedLegacyTask(t, db, seed, now) + seedLegacySessionWorktree(t, db, "sess-pgr", "wt-pgr", "repo-pgr", "", "/tasks/pgr/kandev", "feature/pgr", "active", now) + seedLegacyFlatEnv(t, db, seed, "wt-pgr", "/tasks/pgr/kandev", "feature/pgr", now) + + repo := &Repository{db: db, ro: db, migrate: dbutil.NewMigrateLogger(db, nil), failCutoverAfter: step} + err := repo.initSchema() + if err == nil { + t.Fatalf("expected injected failure at %s", step) + } + var tableCount int + if err := db.Get(&tableCount, ` + SELECT COUNT(*) FROM information_schema.tables + WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { + t.Fatalf("count legacy table: %v", err) + } + if tableCount != 1 { + t.Fatalf("legacy table must survive rollback at %s", step) + } + var path string + if err := db.Get(&path, db.Rebind(` + SELECT worktree_path FROM task_session_worktrees WHERE worktree_id = ?`), "wt-pgr"); err != nil { + t.Fatalf("legacy worktree row lost after rollback at %s: %v", step, err) + } + if path != "/tasks/pgr/kandev" { + t.Fatalf("legacy worktree data changed after rollback at %s", step) + } + }) + } +} + +// TestCutoverPostgres_FreshSchemaIsFinal proves a fresh PostgreSQL database +// contains only the final schema. +func TestCutoverPostgres_FreshSchemaIsFinal(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + if _, err := NewWithDB(db, db, nil); err != nil { + t.Fatalf("init fresh postgres schema: %v", err) + } + var tableCount int + if err := db.Get(&tableCount, ` + SELECT COUNT(*) FROM information_schema.tables + WHERE table_name = 'task_session_worktrees' AND table_schema = current_schema()`); err != nil { + t.Fatalf("count legacy table: %v", err) + } + if tableCount != 0 { + t.Fatal("fresh postgres database must not contain task_session_worktrees") + } + // The environment FK must reference tasks and the repos FK must + // reference task_environments after the swap. + var fkCount int + if err := db.Get(&fkCount, ` + SELECT COUNT(*) FROM information_schema.table_constraints tc + JOIN information_schema.referential_constraints rc + ON rc.constraint_name = tc.constraint_name AND rc.constraint_schema = tc.constraint_schema + JOIN information_schema.constraint_column_usage ccu + ON ccu.constraint_name = rc.unique_constraint_name AND ccu.constraint_schema = rc.unique_constraint_schema + WHERE tc.table_name = 'task_environment_repos' + AND tc.constraint_type = 'FOREIGN KEY' + AND ccu.table_name = 'task_environments' + AND tc.table_schema = current_schema()`); err != nil { + t.Fatalf("count env-repo FK: %v", err) + } + if fkCount != 1 { + t.Fatalf("task_environment_repos FK to task_environments missing: %d", fkCount) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go index b96b0d6165..2c7c98fd01 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go @@ -13,10 +13,16 @@ import ( "github.com/kandev/kandev/internal/db/dialect" ) +// envRepoKey is the normalized repository-slot key: one row per +// (repository, branch slug) pair within a task environment. +func envRepoKey(repositoryID, branchSlug string) string { + return repositoryID + "\x00" + branchSlug +} + // rowForKey returns (creating if needed) the normalized repo target for a // (repository, branch slug) key in this task. func (t *taskWorktreeTargets) rowForKey(repositoryID, branchSlug string) *envRepoTarget { - key := repositoryID + "\x00" + branchSlug + key := envRepoKey(repositoryID, branchSlug) if target, ok := t.byKey[key]; ok { return target } diff --git a/docs/specs/session-delete-resource-cleanup/spec.md b/docs/specs/session-delete-resource-cleanup/spec.md index 0f5c583b3e..eacf0c9550 100644 --- a/docs/specs/session-delete-resource-cleanup/spec.md +++ b/docs/specs/session-delete-resource-cleanup/spec.md @@ -120,15 +120,25 @@ environment across tasks. Such a borrowing task gains no environment of its own, and the shared physical worktree keeps exactly one owner. Legacy session rows marked `deleted` (or carrying `deleted_at`) and stale references from terminal sessions are historical evidence, not additional -owners. A terminal reference bypasses validation when a higher-precedence -task-owned source exists for the same repository and branch slot. This source -can be a canonical repository row or the surviving flat environment row. The -higher-precedence source remains the owner when the terminal reference carries -a different physical `worktree_id`. A terminal-only reference without a -higher-precedence owner still requires compatible identity, path, branch, and -repository data. A non-terminal session with a different physical identity -also requires compatible data. Unresolved ownership fails closed with a -diagnostic. +owners. A higher-precedence task-owned source for the same repository and +branch slot, either a canonical repository row or the surviving flat +environment row, remains the owner even when a terminal reference carries a +different physical `worktree_id`. + +The task-owned model holds exactly one physical worktree per (repository, +branch slug) slot, while the legacy per-session model let one task accumulate +several for the same slot through handoffs, re-materialized workspaces, and +additional sessions. When no higher-precedence task-owned source exists, the +upgrade elects one owner per contested slot instead of failing: a live +session's worktree wins over a terminal session's, then the most recently +updated row wins. Every other worktree for that slot becomes historical +evidence and is logged with its identity, repository, path, and branch. No +directory, registration, or branch is removed, so a demoted workspace remains +on disk. + +Irreconcilable data, including incompatible path or branch metadata for the +same physical worktree identity with no higher-precedence owner or a worktree +whose task has no resolvable environment, still fails closed with a diagnostic. After backfill validation, the same upgrade drops `task_session_worktrees` and the deprecated flat worktree columns from `task_environments`. It also removes @@ -311,10 +321,17 @@ remain the authorization boundary for physical cleanup. binary starts, **THEN** the worktree is normalized onto the owning task's environment, both sessions keep that environment, the borrowing task gains no environment of its own, and the cutover completes. -- **GIVEN** a non-terminal session references a worktree that conflicts with - the canonical owner and cannot be reconciled, **WHEN** the new binary starts, - **THEN** startup fails closed, the transaction rolls back, and the verified - pre-upgrade backup remains the recovery source. +- **GIVEN** several legacy sessions of one task registered different physical + worktrees for the same repository and branch slot, **WHEN** the new binary + starts, **THEN** the canonical owner — or the newest live session worktree + when there is none — keeps the slot, the other worktrees are recorded as + history and logged with their paths, no directory or branch is removed, and + the cutover completes. +- **GIVEN** a non-terminal session references a worktree whose path or branch + metadata contradicts the same physical worktree's owner and cannot be + reconciled, **WHEN** the new binary starts, **THEN** startup fails closed, the + transaction rolls back, and the verified pre-upgrade backup remains the + recovery source. ## Out of Scope