From 88bf44dfca9c5bcd48d27e7257b91b2d2281dacc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 12 Aug 2026 12:11:33 +0100 Subject: [PATCH] fix: preserve canonical worktree during cutover (#2554) --- ...worktree_ownership_flat_precedence_test.go | 217 ++++++++++++++++++ .../sqlite/worktree_ownership_migration.go | 1 + .../sqlite/worktree_ownership_normalize.go | 19 ++ .../worktree_ownership_postgres_test.go | 27 +++ .../sqlite/worktree_ownership_targets.go | 15 +- .../plan.md | 132 +++++++++++ .../task-01-normalize-divergent-flat-owner.md | 83 +++++++ .../session-delete-resource-cleanup/spec.md | 23 ++ 8 files changed, 516 insertions(+), 1 deletion(-) create mode 100644 apps/backend/internal/task/repository/sqlite/worktree_ownership_flat_precedence_test.go create mode 100644 docs/plans/task-worktree-cutover-flat-conflict/plan.md create mode 100644 docs/plans/task-worktree-cutover-flat-conflict/task-01-normalize-divergent-flat-owner.md diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_flat_precedence_test.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_flat_precedence_test.go new file mode 100644 index 0000000000..c2f8cc0ab1 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_flat_precedence_test.go @@ -0,0 +1,217 @@ +package sqlite + +import ( + "context" + "fmt" + "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" +) + +func TestCutover_CanonicalRepoSupersedesDivergentFlatOwner(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{ + envID: "env-flat-conflict", + taskID: "task-flat-conflict", + repoID: "repo-flat-conflict", + sessionID: "sess-flat-conflict", + } + seedLegacyTask(t, db, seed, now) + seedLegacyFlatEnv(t, db, seed, "wt-flat", "/tasks/flat", "feature/flat", now) + seedLegacyEnvRepo(t, db, "env-repo-canonical", seed.envID, seed.repoID, + "wt-canonical", "/tasks/canonical", "feature/canonical", 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, err := NewWithDB(db, db, log) + 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-canonical" { + t.Fatalf("normalized repos = %+v, want canonical wt-canonical", env.Repos) + } + entries := logs.FilterMessage("cutover: duplicate legacy worktrees demoted to history").All() + if len(entries) != 1 { + t.Fatalf("flat demotion logs = %d, want one", len(entries)) + } + logged := fmt.Sprint(entries[0].ContextMap()["worktrees"]) + for _, want := range []string{"env-flat-conflict", "repo-flat-conflict", "wt-flat", "/tasks/flat", "feature/flat", "wt-canonical"} { + if !strings.Contains(logged, want) { + t.Fatalf("flat demotion log %q must mention %q", logged, want) + } + } +} + +func TestCutover_PreservesDivergentFlatOutsideCanonicalSlot(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{ + envID: "env-flat-slot", + taskID: "task-flat-slot", + repoID: "repo-flat-slot", + sessionID: "sess-flat-slot", + } + seedLegacyTask(t, db, seed, now) + seedLegacyFlatEnv(t, db, seed, "wt-flat-slot", "/tasks/flat-slot", "feature/flat", now) + if _, err := db.Exec(db.Rebind(` + INSERT INTO task_environment_repos ( + id, task_environment_id, repository_id, branch_slug, + worktree_id, worktree_path, worktree_branch, position, + error_message, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, 0, '', ?, ?)`), + "env-repo-slot", seed.envID, seed.repoID, "feature/canonical", + "wt-canonical-slot", "/tasks/canonical-slot", "feature/canonical", now, now); err != nil { + t.Fatalf("seed canonical repository row: %v", err) + } + + 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) + } + slots := make(map[string]string, len(env.Repos)) + for _, envRepo := range env.Repos { + slots[envRepo.RepositoryID+"\x00"+envRepo.BranchSlug] = envRepo.WorktreeID + } + if len(slots) != 2 || slots[seed.repoID+"\x00"] != "wt-flat-slot" || + slots[seed.repoID+"\x00feature/canonical"] != "wt-canonical-slot" { + t.Fatalf("normalized repository slots = %+v", env.Repos) + } +} + +func TestCutover_DuplicateCanonicalRowsRetainSurvivingEnvironmentPrecedence(t *testing.T) { + for _, reverse := range []bool{false, true} { + name := "surviving row first" + if reverse { + name = "surviving row last" + } + t.Run(name, func(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + older := now.Add(-time.Hour) + seed := legacySeed{ + envID: "env-duplicate-survivor", + taskID: "task-duplicate-canonical", + repoID: "repo-duplicate-canonical", + sessionID: "sess-duplicate-canonical", + } + seedLegacyTask(t, db, seed, now) + seedLegacyFlatEnv(t, db, seed, "wt-flat-duplicate", "/tasks/flat-duplicate", "feature/flat", now) + seedLegacyFlatEnv(t, db, legacySeed{ + envID: "env-duplicate-loser", + taskID: seed.taskID, + repoID: seed.repoID, + }, "", "", "", older) + + rows := []struct { + id, envID string + }{ + {id: "env-repo-survivor", envID: seed.envID}, + {id: "env-repo-loser", envID: "env-duplicate-loser"}, + } + if reverse { + rows[0], rows[1] = rows[1], rows[0] + } + for _, row := range rows { + seedLegacyEnvRepo(t, db, row.id, row.envID, seed.repoID, + "wt-canonical-duplicate", "/tasks/canonical-duplicate", "feature/canonical", now) + } + + 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-canonical-duplicate" { + t.Fatalf("normalized repos = %+v, want canonical duplicate owner", env.Repos) + } + }) + } +} + +func TestCutover_RehomedCanonicalRowSupersedesSurvivingFlatOwner(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{ + envID: "env-rehome-survivor", + taskID: "task-rehome-canonical", + repoID: "repo-rehome-canonical", + sessionID: "sess-rehome-canonical", + } + seedLegacyTask(t, db, seed, now) + seedLegacyFlatEnv(t, db, seed, "wt-flat-rehome", "/tasks/flat-rehome", "feature/flat", now) + seedLegacyFlatEnv(t, db, legacySeed{ + envID: "env-rehome-loser", + taskID: seed.taskID, + repoID: seed.repoID, + }, "", "", "", now.Add(-time.Hour)) + seedLegacyEnvRepo(t, db, "env-repo-rehomed", "env-rehome-loser", seed.repoID, + "wt-canonical-rehome", "/tasks/canonical-rehome", "feature/canonical", now) + + 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-canonical-rehome" { + t.Fatalf("normalized repos = %+v, want rehomed canonical owner", env.Repos) + } +} + +func TestCutover_DoesNotLogRolledBackFlatDemotion(t *testing.T) { + db := openLegacyDB(t) + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{ + envID: "env-flat-rollback", + taskID: "task-flat-rollback", + repoID: "repo-flat-rollback", + sessionID: "sess-flat-rollback", + } + seedLegacyTask(t, db, seed, now) + seedLegacyFlatEnv(t, db, seed, "wt-flat-rollback", "/tasks/flat-rollback", "feature/flat", now) + seedLegacyEnvRepo(t, db, "env-repo-rollback", seed.envID, seed.repoID, + "wt-canonical-rollback", "/tasks/canonical-rollback", "feature/canonical", 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") + } + if entries := logs.FilterMessage("cutover: duplicate legacy worktrees demoted to history").All(); len(entries) != 0 { + t.Fatalf("rolled-back flat 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 ac765a0e7f..c08d75b06c 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go @@ -127,6 +127,7 @@ func (r *Repository) normalizeTaskWorktreeOwnership() error { sessionEnvIDs: make(map[string]string), sessionWorktreeSuperseded: make(map[string]bool), demotedWorktrees: make(map[string]bool), + demotedFlatEnvironments: make(map[string]bool), authoritativeWorktreeIDs: make(map[string]bool), tasks: make(map[string]*taskWorktreeTargets), taskEnvIDs: make(map[string]string), 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 2e758daf13..0ff8e21db2 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go @@ -21,6 +21,7 @@ type worktreeCutover struct { sessionEnvIDs map[string]string sessionWorktreeSuperseded map[string]bool demotedWorktrees map[string]bool + demotedFlatEnvironments map[string]bool demotions []string authoritativeWorktreeIDs map[string]bool envRepos []legacyEnvRepo @@ -263,6 +264,7 @@ type taskWorktreeTargets struct { type envRepoTarget struct { key string repositoryID, branchSlug string + canonical bool worktreeID string worktreePath string worktreeBranch string @@ -355,6 +357,10 @@ func (c *worktreeCutover) mergeFlatEnvironmentFields() { continue } targets := c.targetsForTask(env.taskID) + if canonical := targets.canonicalOwnerForSlot(env.repositoryID, ""); canonical != nil && env.worktreeID != "" && env.worktreeID != canonical.worktreeID { + c.demoteFlatEnvironment(env, canonical) + continue + } if err := targets.mergeFlatEnv(env); err != nil { c.conflicts = append(c.conflicts, fmt.Sprintf("environment %s flat worktree fields: %v", env.id, err)) continue @@ -363,6 +369,19 @@ func (c *worktreeCutover) mergeFlatEnvironmentFields() { } } +// demoteFlatEnvironment records deprecated flat metadata that lost the exact +// empty-branch slot to a non-deleted canonical repository row. The migration +// keeps the physical flat worktree untouched and logs it after commit. +func (c *worktreeCutover) demoteFlatEnvironment(env *legacyEnv, winner *envRepoTarget) { + if c.demotedFlatEnvironments[env.id] { + return + } + c.demotedFlatEnvironments[env.id] = true + c.demotions = append(c.demotions, fmt.Sprintf( + "environment %s flat worktree %s (repository %s, path %q, branch %q) superseded by canonical worktree %s", + env.id, env.worktreeID, env.repositoryID, env.worktreePath, env.worktreeBranch, winner.worktreeID)) +} + // worktreeSlot is one normalized repository slot — a (repository, branch // slug) pair of a single task — plus the legacy session rows competing to own // it. 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 index 830ed268db..a31e9dda58 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_postgres_test.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_postgres_test.go @@ -72,6 +72,33 @@ func TestCutoverPostgres_NormalizesLegacyFlatEnvironment(t *testing.T) { } } +func TestCutoverPostgres_CanonicalRepoSupersedesDivergentFlatOwner(t *testing.T) { + db := openLegacyPostgres(t) + now := time.Now().UTC().Truncate(time.Second) + seed := legacySeed{ + envID: "env-pg-flat-conflict", + taskID: "task-pg-flat-conflict", + repoID: "repo-pg-flat-conflict", + sessionID: "sess-pg-flat-conflict", + } + seedLegacyTask(t, db, seed, now) + seedLegacyFlatEnv(t, db, seed, "wt-pg-flat", "/tasks/pg-flat", "feature/flat", now) + seedLegacyEnvRepo(t, db, "env-pg-repo-canonical", seed.envID, seed.repoID, + "wt-pg-canonical", "/tasks/pg-canonical", "feature/canonical", now) + + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("postgres cutover: %v", err) + } + env, err := repo.GetTaskEnvironment(context.Background(), seed.envID) + if err != nil { + t.Fatalf("GetTaskEnvironment: %v", err) + } + if len(env.Repos) != 1 || env.Repos[0].WorktreeID != "wt-pg-canonical" { + t.Fatalf("normalized postgres repos = %+v, want canonical wt-pg-canonical", env.Repos) + } +} + func TestCutoverPostgres_SessionOnlyNormalization(t *testing.T) { db := openLegacyPostgres(t) ctx := context.Background() 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 22479a8dba..11d28c1c19 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go @@ -58,6 +58,7 @@ func (t *taskWorktreeTargets) targetForWorktree(worktreeID string) *envRepoTarge // physical-worktree identity, which must agree everywhere). func (t *taskWorktreeTargets) mergeLegacyEnvRepo(row legacyEnvRepo) error { target := t.rowForKey(row.repositoryID, row.branchSlug) + target.canonical = true if err := target.claimWorktree(row.worktreeID, row.worktreePath, row.worktreeBranch, row.position, row.errorMessage, row.createdAt, row.updatedAt, row.status); err != nil { return err } @@ -66,6 +67,18 @@ func (t *taskWorktreeTargets) mergeLegacyEnvRepo(row legacyEnvRepo) error { return target.mergeCreation(row.createdAt, row.updatedAt) } +// canonicalOwnerForSlot returns the active canonical owner for a normalized +// repository slot. Rows from collapsed environments are already re-homed into +// this task target, so their original environment ID is not part of ownership. +func (t *taskWorktreeTargets) canonicalOwnerForSlot(repositoryID, branchSlug string) *envRepoTarget { + target, ok := t.byKey[envRepoKey(repositoryID, branchSlug)] + if !ok || !target.canonical || target.worktreeID == "" || + target.status == worktreeRepoStatusDeleted || target.deletedAt != nil { + return nil + } + return target +} + // mergeFlatEnv merges the deprecated flat worktree columns of the surviving // environment. func (t *taskWorktreeTargets) mergeFlatEnv(env *legacyEnv) error { @@ -357,7 +370,7 @@ func (c *worktreeCutover) legacyWorktreeInventory() map[string]bool { inventory[worktreeInventoryKey(row.worktreeID, row.worktreePath, row.worktreeBranch)] = true } for _, env := range c.envs { - if env.worktreeID == "" || c.loserEnvIDs[env.id] || canonicalIDs[env.worktreeID] { + if env.worktreeID == "" || c.loserEnvIDs[env.id] || c.demotedFlatEnvironments[env.id] || canonicalIDs[env.worktreeID] { continue } inventory[worktreeInventoryKey(env.worktreeID, env.worktreePath, env.worktreeBranch)] = true diff --git a/docs/plans/task-worktree-cutover-flat-conflict/plan.md b/docs/plans/task-worktree-cutover-flat-conflict/plan.md new file mode 100644 index 0000000000..9b239dfa6f --- /dev/null +++ b/docs/plans/task-worktree-cutover-flat-conflict/plan.md @@ -0,0 +1,132 @@ +--- +spec: docs/specs/session-delete-resource-cleanup/spec.md +created: 2026-08-12 +status: done +--- + +# Fix Plan: Recover Divergent Flat Worktree Cutovers + +## Overview + +Repair the one-time task-worktree ownership cutover for a valid legacy state. +A canonical repository row and deprecated flat fields can name different +worktrees for one repository slot. The cutover will keep the canonical row and +record the flat worktree as history. + +## Root Cause + +The legacy launch path updated the flat environment before it refreshed the +repository rows. For single-repository launches, it did not refresh those rows +when the launch response omitted the multi-repository list. A later worktree +materialization could therefore update only the flat worktree ID. + +The cutover loads canonical repository rows before the deprecated flat fields. +It accepts stale flat path and branch data only when both sources have the same +worktree ID. If the IDs differ, `mergeFlatEnv` reports `worktree A conflicts +with B` before the slot election can classify the weaker source as history. + +A throwaway repository test reproduced the reported error. The test used the +reported environment and worktree IDs with a canonical row and a divergent flat +row for the same empty branch slot. The real repository initializer returned +the reported `flat worktree fields` conflict. The throwaway test was removed. + +## Backend + +### Canonical source precedence + +Update +`apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go`. +When a non-deleted canonical row already owns the same repository and empty +branch slot, keep that row if the flat source has a different worktree ID. +Classify the flat source as historical evidence instead of adding a conflict. + +Use the normalized task, repository, and empty branch slot for this rule. +Canonical rows from environments collapsed into the surviving task environment +are already re-homed into that normalized target and must retain precedence. +Do not suppress a flat worktree when the canonical row belongs to another task, +repository, or non-empty branch slot. Keep the current merge behavior when the +canonical row has no worktree ID. + +Do not add the demoted flat ID to the authoritative worktree index. Add one +diagnostic entry to the existing post-commit demotion log. Include the +environment, repository, worktree ID, path, branch, and canonical winner. + +### Inventory validation + +Update +`apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go`. +Exclude a demoted flat source from the legacy inventory. This rule keeps the +merge result and inventory validation consistent. + +Keep all transaction, backup, shadow-table, rollback, and replay behavior. +The migration must not change the file system or Git state. + +## Tests + +- **What:** A canonical row wins over divergent flat fields for the same empty + branch slot. + **File:** + `apps/backend/internal/task/repository/sqlite/worktree_ownership_flat_precedence_test.go`. + **How:** Seed the reported conflict shape. Run the real SQLite repository + initializer. Make sure that the cutover completes and retains the canonical + worktree. +- **What:** The migration records the demoted flat worktree only after commit. + **File:** the same test file. + **How:** Capture the migration logger. Make sure that a successful cutover + includes the flat identity and path. Inject a cutover failure and make sure + that it emits no demotion warning. +- **What:** Source precedence is limited to the exact empty branch slot. + **File:** the same test file. + **How:** Give the canonical row a non-empty branch slug. Make sure that the + normalized environment keeps both repository slots. +- **What:** Duplicate canonical rows preserve precedence regardless of legacy + row order. + **File:** the same test file. + **How:** Give duplicate environments the same canonical slot and divergent + flat fields on the surviving environment. Run both canonical row orders and + make sure that the canonical worktree remains authoritative. +- **What:** Re-homed canonical rows retain precedence over surviving flat data. + **File:** the same test file. + **How:** Put flat fields on the surviving environment and the only canonical + row on a collapsed older environment. Make sure that the cutover keeps the + canonical worktree. +- **What:** PostgreSQL uses the same precedence and transaction behavior. + **File:** + `apps/backend/internal/task/repository/sqlite/worktree_ownership_postgres_test.go`. + **How:** Add an environment-gated PostgreSQL fixture for the reported state. +- **What:** Existing cutover election, conflict, rollback, and replay behavior + remains valid. + **File:** the existing SQLite repository test package. + **How:** Run all cutover tests and the complete repository package. + +## Verification Results + +- `cd apps/backend && go test ./internal/task/repository/sqlite -run 'TestCutover_(CanonicalRepoSupersedesDivergentFlatOwner|PreservesDivergentFlatOutsideCanonicalSlot|DuplicateCanonicalRowsRetainSurvivingEnvironmentPrecedence|RehomedCanonicalRowSupersedesSurvivingFlatOwner|DoesNotLogRolledBackFlatDemotion)|TestCutoverPostgres_CanonicalRepoSupersedesDivergentFlatOwner' -count=1` — 7 SQLite tests passed; the PostgreSQL test is environment-gated and skipped because `KANDEV_TEST_POSTGRES_DSN` is unset. +- `cd apps/backend && go test ./internal/task/repository/sqlite -run 'TestCutover' -count=1` — 54 tests passed. +- `cd apps/backend && go test ./internal/task/repository/sqlite -count=1` — 421 tests passed. + +## Implementation Waves And Parallel Candidates + +Wave 1, sequential: + +- [x] [task-01-normalize-divergent-flat-owner](task-01-normalize-divergent-flat-owner.md) + +No task is parallel-safe. The normalization state, inventory, logs, and tests +share one migration boundary. + +## Risks + +- A broad precedence rule can hide a worktree from another repository slot. + The rule must match the environment, repository, and empty branch slug. +- Merge and inventory rules can diverge. Both paths must use the same demotion + state. +- A warning before commit can describe a rollback as a completed migration. + Reuse the existing post-commit demotion logger. + +## Out of Scope + +- Manual edits to an affected database. +- File-system or Git reconciliation during startup. +- Changes to task or session behavior after the one-time cutover. +- Public recovery documentation. The backup and rollback procedure is + unchanged. diff --git a/docs/plans/task-worktree-cutover-flat-conflict/task-01-normalize-divergent-flat-owner.md b/docs/plans/task-worktree-cutover-flat-conflict/task-01-normalize-divergent-flat-owner.md new file mode 100644 index 0000000000..0aba6df52b --- /dev/null +++ b/docs/plans/task-worktree-cutover-flat-conflict/task-01-normalize-divergent-flat-owner.md @@ -0,0 +1,83 @@ +--- +id: "01-normalize-divergent-flat-owner" +title: "Normalize divergent flat worktree owner" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/session-delete-resource-cleanup/spec.md" +--- + +# Task 01: Normalize Divergent Flat Worktree Owner + +## Acceptance + +- A non-deleted canonical row wins when deprecated flat fields name another + worktree for the same normalized task, repository, and empty branch slot, + including when the canonical row was re-homed from a collapsed environment. +- The migration logs the demoted flat identity only after commit. It does not + change the directory, Git registration, or branch. +- A canonical row in another task, repository, or slot does not suppress the + flat worktree. +- SQLite and PostgreSQL retain the existing rollback, replay, and fail-closed + behavior for unrelated conflicts. + +## Verification + +```bash +cd apps/backend && go test ./internal/task/repository/sqlite -run 'TestCutover_(CanonicalRepoSupersedesDivergentFlatOwner|PreservesDivergentFlatOutsideCanonicalSlot|DuplicateCanonicalRowsRetainSurvivingEnvironmentPrecedence|RehomedCanonicalRowSupersedesSurvivingFlatOwner|DoesNotLogRolledBackFlatDemotion)|TestCutoverPostgres_CanonicalRepoSupersedesDivergentFlatOwner' -count=1 +cd apps/backend && go test ./internal/task/repository/sqlite -run 'TestCutover' -count=1 +cd apps/backend && go test ./internal/task/repository/sqlite -count=1 +``` + +## Files Likely Touched + +- `apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go` +- `apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go` +- `apps/backend/internal/task/repository/sqlite/worktree_ownership_flat_precedence_test.go` +- `apps/backend/internal/task/repository/sqlite/worktree_ownership_postgres_test.go` +- `docs/specs/session-delete-resource-cleanup/spec.md` +- `docs/plans/task-worktree-cutover-flat-conflict/plan.md` +- `docs/plans/task-worktree-cutover-flat-conflict/task-01-normalize-divergent-flat-owner.md` + +## Dependencies + +None. + +## Parallelism + +Sequential. The source classifier, inventory, log, and migration tests share +one persistence boundary. + +## Inputs + +- The schema-normalization, failure-mode, and scenario sections of the repair + spec. +- The root-cause and canonical-precedence sections of `plan.md`. +- Existing flat metadata precedence and slot-election tests. +- Existing post-commit demotion log behavior. + +## Output Contract + +Report the source rule, changed files, and exact test results. Report all +temporary-file cleanup. Update this task and `plan.md` in the same conversation. + +## Results + +- Canonical repository rows now supersede divergent deprecated flat worktree + fields for the same normalized task, repository, and empty branch slot, + including canonical rows re-homed from collapsed environments. +- Demoted flat worktrees are excluded from normalized inventory validation and + are reported through the existing post-commit demotion warning with their + environment, repository, identity, path, branch, and canonical winner. +- A canonical row in another task, repository, or branch slot remains + independent. Empty flat metadata does not trigger this demotion rule. +- Changed files: `worktree_ownership_normalize.go`, + `worktree_ownership_targets.go`, `worktree_ownership_migration.go`, + `worktree_ownership_flat_precedence_test.go`, + `worktree_ownership_postgres_test.go`, and the repair spec/plan files. +- `cd apps/backend && go test ./internal/task/repository/sqlite -run 'TestCutover_(CanonicalRepoSupersedesDivergentFlatOwner|PreservesDivergentFlatOutsideCanonicalSlot|DuplicateCanonicalRowsRetainSurvivingEnvironmentPrecedence|RehomedCanonicalRowSupersedesSurvivingFlatOwner|DoesNotLogRolledBackFlatDemotion)|TestCutoverPostgres_CanonicalRepoSupersedesDivergentFlatOwner' -count=1` — 7 SQLite tests passed; the PostgreSQL test is environment-gated and skipped because `KANDEV_TEST_POSTGRES_DSN` is unset. +- `cd apps/backend && go test ./internal/task/repository/sqlite -run 'TestCutover' -count=1` — 54 tests passed. +- `cd apps/backend && go test ./internal/task/repository/sqlite -count=1` — 421 tests passed. +- No throwaway reproduction files remain. Test databases use `t.TempDir()` and + are cleaned up by the test framework. diff --git a/docs/specs/session-delete-resource-cleanup/spec.md b/docs/specs/session-delete-resource-cleanup/spec.md index b565ce8c1f..0de526d517 100644 --- a/docs/specs/session-delete-resource-cleanup/spec.md +++ b/docs/specs/session-delete-resource-cleanup/spec.md @@ -125,6 +125,16 @@ 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`. +A non-deleted canonical repository row also takes precedence over deprecated +flat fields for the same normalized task-owned repository slot: the repository +and empty branch slot within the surviving task environment. This rule applies +when the two sources contain different physical `worktree_id` values, including +when the canonical row was re-homed from a collapsed legacy environment. The +original environment ID is not an ownership condition after re-homing. The +upgrade records the flat worktree as historical evidence and logs its identity, +path, and branch after the transaction commits. The upgrade does not remove its +directory, Git registration, or branch. + 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 @@ -241,6 +251,11 @@ remain the authorization boundary for physical cleanup. cannot block migration solely because its path or branch metadata is stale, including when the session is resumable. The higher-precedence repository or flat environment metadata remains authoritative. +- Deprecated flat fields cannot block migration when a non-deleted canonical + row owns the same normalized task, repository, and empty branch slot. The + canonical row remains authoritative even when it was re-homed from a + collapsed environment, and the upgrade records the flat worktree as + historical evidence. - A session bound to another task's environment does not block migration and does not create a second owner for the shared worktree. - If migration fails after shadow tables are populated or after legacy DDL has @@ -304,6 +319,14 @@ remain the authorization boundary for physical cleanup. row for the same physical worktree, **WHEN** the new binary starts, **THEN** the canonical repository path and branch are retained and the cutover completes. +- **GIVEN** a non-deleted canonical repository row and deprecated flat fields + contain different worktree IDs for the same empty branch slot, **WHEN** the + new binary starts, **THEN** the canonical row remains the owner, the flat + worktree is logged as history, and the cutover completes. +- **GIVEN** a surviving flat environment and a canonical row only in a legacy + environment that collapses into it, **WHEN** the new binary starts, **THEN** + the re-homed canonical row remains the owner of the normalized slot and the + flat worktree is logged as history. - **GIVEN** a resumable legacy session and a canonical task-owned repository row carry the same `worktree_id` but different path or branch metadata, **WHEN** the new binary starts, **THEN** the canonical metadata is retained and the