fix(task): recover hybrid worktree cutovers (#2531)

Co-authored-by: Kandev Agent <agent@kandev.dev>
This commit is contained in:
João Salavisa
2026-08-11 23:30:31 +02:00
committed by GitHub
parent 9de669558b
commit 205e775d2b
8 changed files with 356 additions and 18 deletions
@@ -440,6 +440,20 @@ func TestPostgresNormalizeTaskWorktreeOwnershipIsNoOpOnFreshSchema(t *testing.T)
}
}
func TestPostgresCutoverHybridNormalizedEnvironmentWithLegacySessionWorktrees(t *testing.T) {
db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t))
if _, err := NewWithDB(db, db, nil); err != nil {
t.Fatalf("seed final postgres schema: %v", err)
}
seed := seedHybridCutoverState(t, db, "postgres")
repo, err := NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("upgrade hybrid postgres schema: %v", err)
}
assertHybridCutoverResult(t, repo, seed)
}
func TestPostgresWorkflowHiddenRoundTrip(t *testing.T) {
ctx := context.Background()
db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t))
@@ -111,6 +111,14 @@ func (r *Repository) normalizeTaskWorktreeOwnership() error {
if err := r.cutoverEnsureLegacyBranchSlug(tx); err != nil {
return err
}
legacyEnvColumns, err := r.cutoverLegacyEnvironmentColumns(tx)
if err != nil {
return err
}
legacyRepoColumns, err := r.tableColumns(tx, "task_environment_repos")
if err != nil {
return err
}
cut := &worktreeCutover{
envs: make(map[string]*legacyEnv),
taskEnvs: make(map[string][]*legacyEnv),
@@ -125,7 +133,7 @@ func (r *Repository) normalizeTaskWorktreeOwnership() error {
loserEnvIDs: make(map[string]bool),
executorTypes: make(map[string]string),
}
if err := cut.loadLegacy(tx); err != nil {
if err := cut.loadLegacy(tx, legacyEnvColumns, legacyRepoColumns); err != nil {
return err
}
if err := cut.normalize(tx); err != nil {
@@ -156,6 +164,18 @@ func (r *Repository) normalizeTaskWorktreeOwnership() error {
return nil
}
func (r *Repository) cutoverLegacyEnvironmentColumns(tx *sqlx.Tx) (map[string]bool, error) {
columns := make(map[string]bool, 4)
for _, column := range []string{"repository_id", "worktree_id", "worktree_path", "worktree_branch"} {
has, err := r.columnExists(tx, "task_environments", column)
if err != nil {
return nil, err
}
columns[column] = has
}
return columns, 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.
@@ -9,6 +9,7 @@ import (
"github.com/jmoiron/sqlx"
dbutil "github.com/kandev/kandev/internal/db"
"github.com/kandev/kandev/internal/task/models"
)
// ---------------------------------------------------------------------------
@@ -224,6 +225,94 @@ func TestCutover_FreshSchemaHasFinalShape(t *testing.T) {
}
}
func TestCutover_HybridNormalizedEnvironmentWithLegacySessionWorktrees(t *testing.T) {
dbPath := filepath.Join(t.TempDir(), "hybrid.db")
dbConn, err := dbutil.OpenSQLite(dbPath)
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
db := sqlx.NewDb(dbConn, "sqlite3")
t.Cleanup(func() { _ = db.Close() })
if _, err := NewWithDB(db, db, nil); err != nil {
t.Fatalf("seed final schema: %v", err)
}
seed := seedHybridCutoverState(t, db, "sqlite")
repo, err := NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("upgrade hybrid schema: %v", err)
}
assertHybridCutoverResult(t, repo, seed)
}
func seedHybridCutoverState(t *testing.T, db *sqlx.DB, suffix string) legacySeed {
t.Helper()
now := time.Now().UTC().Truncate(time.Second)
seed := legacySeed{
envID: "env-hybrid-" + suffix, taskID: "task-hybrid-" + suffix,
repoID: "repo-hybrid-" + suffix, sessionID: "sess-hybrid-" + suffix,
}
seedLegacyTask(t, db, seed, now)
if _, err := db.Exec(db.Rebind(`
INSERT INTO task_environments (
id, task_id, executor_type, status, workspace_path, created_at, updated_at
) VALUES (?, ?, 'worktree', 'ready', '/tasks/hybrid', ?, ?)`),
seed.envID, seed.taskID, now, now); err != nil {
t.Fatalf("seed normalized environment: %v", err)
}
seedLegacyEnvRepo(t, db, "env-repo-hybrid", seed.envID, seed.repoID,
"wt-hybrid", "/tasks/hybrid/repo", "feature/hybrid", now)
if _, err := db.Exec(db.Rebind(`
UPDATE task_environment_repos
SET status = 'deleted', merged_at = ?, deleted_at = ?
WHERE id = 'env-repo-hybrid'`), now, now); err != nil {
t.Fatalf("seed normalized repository lifecycle: %v", err)
}
if _, err := db.Exec(legacySessionWorktreeDDL); err != nil {
t.Fatalf("recreate stale legacy table: %v", err)
}
seedLegacySessionWorktree(t, db, seed.sessionID, "wt-session-only", seed.repoID+"-session-only", "",
"/tasks/hybrid/session-only", "feature/session-only", "active", now)
return seed
}
func assertHybridCutoverResult(t *testing.T, repo *Repository, seed legacySeed) {
t.Helper()
exists, err := repo.tableExists("task_session_worktrees")
if err != nil {
t.Fatalf("probe stale legacy table: %v", err)
}
if exists {
t.Fatal("stale legacy table must be removed")
}
env, err := repo.GetTaskEnvironment(context.Background(), seed.envID)
if err != nil {
t.Fatalf("get normalized environment: %v", err)
}
if len(env.Repos) != 2 {
t.Fatalf("normalized repositories = %+v", env.Repos)
}
got := make(map[string]*models.TaskEnvironmentRepo, len(env.Repos))
for _, envRepo := range env.Repos {
got[envRepo.RepositoryID] = envRepo
}
assertHybridWorktree(t, got[seed.repoID], "wt-hybrid", "/tasks/hybrid/repo", "feature/hybrid")
if got[seed.repoID].Status != "deleted" || got[seed.repoID].MergedAt == nil ||
!got[seed.repoID].MergedAt.Equal(got[seed.repoID].CreatedAt) || got[seed.repoID].DeletedAt == nil ||
!got[seed.repoID].DeletedAt.Equal(got[seed.repoID].CreatedAt) {
t.Fatalf("normalized repository lifecycle = %+v", got[seed.repoID])
}
assertHybridWorktree(t, got[seed.repoID+"-session-only"], "wt-session-only",
"/tasks/hybrid/session-only", "feature/session-only")
}
func assertHybridWorktree(t *testing.T, got *models.TaskEnvironmentRepo, worktreeID, path, branch string) {
t.Helper()
if got == nil || got.WorktreeID != worktreeID || got.WorktreePath != path || got.WorktreeBranch != branch {
t.Fatalf("normalized worktree = %+v, want id=%q path=%q branch=%q", got, worktreeID, path, branch)
}
}
func tableColumnSet(t *testing.T, db *sqlx.DB, table string) map[string]bool {
t.Helper()
cols := map[string]bool{}
@@ -53,7 +53,8 @@ type legacyEnvRepo struct {
id, envID, repositoryID, branchSlug string
worktreeID, worktreePath, worktreeBranch string
position int
errorMessage string
errorMessage, status string
mergedAt, deletedAt *time.Time
createdAt, updatedAt time.Time
}
@@ -67,28 +68,33 @@ type legacySessionWorktree struct {
// loadLegacy reads every legacy ownership row into memory. The transaction
// holds the writer/advisory locks, so the snapshot is consistent.
func (c *worktreeCutover) loadLegacy(tx *sqlx.Tx) error {
if err := c.loadLegacyEnvs(tx); err != nil {
func (c *worktreeCutover) loadLegacy(tx *sqlx.Tx, legacyEnvColumns, legacyRepoColumns map[string]bool) error {
if err := c.loadLegacyEnvs(tx, legacyEnvColumns); err != nil {
return err
}
if err := c.loadLegacySessions(tx); err != nil {
return err
}
if err := c.loadLegacyEnvRepos(tx); err != nil {
if err := c.loadLegacyEnvRepos(tx, legacyRepoColumns); err != nil {
return err
}
return c.loadLegacySessionWorktrees(tx)
}
func (c *worktreeCutover) loadLegacyEnvs(tx *sqlx.Tx) error {
rows, err := tx.Query(`
SELECT id, task_id, repository_id, executor_type, executor_id,
func (c *worktreeCutover) loadLegacyEnvs(tx *sqlx.Tx, columns map[string]bool) error {
// Every interpolated expression comes from the fixed deprecated-column allowlist.
//nolint:gosec
rows, err := tx.Query(fmt.Sprintf(`
SELECT id, task_id, %s, executor_type, executor_id,
executor_profile_id, control_port, status,
COALESCE(worktree_id, ''), COALESCE(worktree_path, ''),
COALESCE(worktree_branch, ''), COALESCE(workspace_path, ''),
%s, %s, %s, COALESCE(workspace_path, ''),
COALESCE(container_id, ''), COALESCE(sandbox_id, ''),
COALESCE(task_dir_name, ''), created_at, updated_at
FROM task_environments`)
FROM task_environments`,
legacyEnvColumnExpr(columns, "repository_id"),
legacyEnvColumnExpr(columns, "worktree_id"),
legacyEnvColumnExpr(columns, "worktree_path"),
legacyEnvColumnExpr(columns, "worktree_branch")))
if err != nil {
return fmt.Errorf("cutover: read legacy environments: %w", err)
}
@@ -107,6 +113,13 @@ func (c *worktreeCutover) loadLegacyEnvs(tx *sqlx.Tx) error {
return rows.Err()
}
func legacyEnvColumnExpr(columns map[string]bool, column string) string {
if columns[column] {
return "COALESCE(" + column + ", '')"
}
return "CAST('' AS TEXT)"
}
func (c *worktreeCutover) loadLegacySessions(tx *sqlx.Tx) error {
// GROUP BY lists every non-aggregate column so PostgreSQL accepts the
// query (SQLite is lenient about functional dependency). started_at is
@@ -158,29 +171,50 @@ func parseLegacyTimestamp(raw string) time.Time {
return time.Time{}
}
func (c *worktreeCutover) loadLegacyEnvRepos(tx *sqlx.Tx) error {
rows, err := tx.Query(`
func (c *worktreeCutover) loadLegacyEnvRepos(tx *sqlx.Tx, columns map[string]bool) error {
// Every interpolated expression comes from the fixed lifecycle-column allowlist.
//nolint:gosec
rows, err := tx.Query(fmt.Sprintf(`
SELECT id, task_environment_id, repository_id, COALESCE(branch_slug, ''),
COALESCE(worktree_id, ''), COALESCE(worktree_path, ''),
COALESCE(worktree_branch, ''), position, COALESCE(error_message, ''),
created_at, updated_at
FROM task_environment_repos`)
%s, created_at, updated_at, %s, %s
FROM task_environment_repos`,
legacyRepoColumnExpr(columns, "status", "CAST('active' AS TEXT)"),
legacyRepoColumnExpr(columns, "merged_at", "NULL"),
legacyRepoColumnExpr(columns, "deleted_at", "NULL")))
if err != nil {
return fmt.Errorf("cutover: read legacy environment repos: %w", err)
}
defer func() { _ = rows.Close() }()
for rows.Next() {
var row legacyEnvRepo
var mergedAt, deletedAt sql.NullTime
if err := rows.Scan(&row.id, &row.envID, &row.repositoryID, &row.branchSlug,
&row.worktreeID, &row.worktreePath, &row.worktreeBranch, &row.position,
&row.errorMessage, &row.createdAt, &row.updatedAt); err != nil {
&row.errorMessage, &row.status, &row.createdAt, &row.updatedAt, &mergedAt, &deletedAt); err != nil {
return fmt.Errorf("cutover: scan legacy environment repo: %w", err)
}
if mergedAt.Valid {
t := mergedAt.Time
row.mergedAt = &t
}
if deletedAt.Valid {
t := deletedAt.Time
row.deletedAt = &t
}
c.envRepos = append(c.envRepos, row)
}
return rows.Err()
}
func legacyRepoColumnExpr(columns map[string]bool, column, fallback string) string {
if columns[column] {
return column
}
return fallback
}
func (c *worktreeCutover) loadLegacySessionWorktrees(tx *sqlx.Tx) error {
rows, err := tx.Query(`
SELECT session_id, worktree_id, repository_id, COALESCE(branch_slug, ''),
@@ -58,9 +58,11 @@ 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)
if err := target.claimWorktree(row.worktreeID, row.worktreePath, row.worktreeBranch, row.position, row.errorMessage, row.createdAt, row.updatedAt, worktreeRepoStatusActive); err != nil {
if err := target.claimWorktree(row.worktreeID, row.worktreePath, row.worktreeBranch, row.position, row.errorMessage, row.createdAt, row.updatedAt, row.status); err != nil {
return err
}
target.mergedAt = row.mergedAt
target.deletedAt = row.deletedAt
return target.mergeCreation(row.createdAt, row.updatedAt)
}
@@ -349,7 +351,7 @@ func (c *worktreeCutover) legacyWorktreeInventory() map[string]bool {
inventory[worktreeInventoryKey(wt.worktreeID, wt.worktreePath, wt.worktreeBranch)] = true
}
for _, row := range c.envRepos {
if row.worktreeID == "" {
if row.worktreeID == "" || row.status == worktreeRepoStatusDeleted || row.deletedAt != nil {
continue
}
inventory[worktreeInventoryKey(row.worktreeID, row.worktreePath, row.worktreeBranch)] = true
@@ -0,0 +1,86 @@
---
spec: docs/specs/session-delete-resource-cleanup/spec.md
created: 2026-08-11
status: implemented
---
# Fix Plan: Recover Hybrid Worktree Cutover Schemas
## Overview
Repair startup for databases where `task_environments` already has its final
normalized shape while the legacy `task_session_worktrees` table still exists.
The cutover currently uses that table as its only legacy sentinel and then
unconditionally selects removed flat environment columns. The repair makes the
legacy environment read reflect the columns actually present while preserving
the existing transactional normalization, validation, and fail-closed behavior.
Confirmed root cause: `normalizeTaskWorktreeOwnership` starts whenever
`task_session_worktrees` exists, but `loadLegacyEnvs` always selects
`repository_id`, `worktree_id`, `worktree_path`, and `worktree_branch` from
`task_environments`. Intermediate or alternate branches can leave only the
sentinel table behind, causing startup to fail before normalization.
## Backend
### Hybrid-schema compatibility
- Update `apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go`
and `worktree_ownership_normalize.go` to determine which deprecated flat
`task_environments` columns exist inside the cutover transaction.
- Project typed empty SQL values for missing flat columns so the existing
normalization pipeline can still consume canonical `task_environment_repos`
and legacy `task_session_worktrees` rows.
- Keep the legacy-table sentinel, shadow-table swap, inventory validation,
PostgreSQL locking, SQLite transaction, and rollback behavior unchanged.
## Tests
- **What:** The repository initializer upgrades a hybrid SQLite schema with a
final `task_environments` table and stale `task_session_worktrees` table.
**File:** `apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go`.
**How:** Initialize a fresh database, recreate only the legacy session table,
seed a session-worktree row backed by normalized environment/repository data,
reopen through `NewWithDB`, and assert startup succeeds, inventory survives,
and the legacy table is removed. The test must first fail with the reported
`no such column: repository_id` error.
- **What:** Fully legacy, final, rollback, and conflict fixtures remain valid.
**File:** same migration test file.
**How:** Run all `TestCutover` tests and the complete repository package.
- **What:** PostgreSQL uses the same hybrid-schema recovery behavior through
its `information_schema` column probes.
**File:** `apps/backend/internal/task/repository/sqlite/postgres_schema_test.go`.
**How:** Run the environment-gated PostgreSQL counterpart locally when
`KANDEV_TEST_POSTGRES_DSN` is available and in the Backend Postgres CI job.
## Implementation Waves And Parallel Candidates
Wave 1, sequential:
- [x] [task-01-recover-hybrid-schema](task-01-recover-hybrid-schema.md)
The migration and regression fixture share schema state and are not
parallel-safe.
## Out Of Scope
- Repairing arbitrary missing required columns outside the four deprecated flat
worktree fields.
- Changing post-cutover task, session, worktree, cleanup, filesystem, or Git
behavior.
- Adding a permanent dual-read or dual-write compatibility path.
## Verification Results
- RED: `go test ./internal/task/repository/sqlite -run
TestCutover_HybridNormalizedEnvironmentWithLegacySessionWorktrees -count=1 -v`
failed with `cutover: read legacy environments: no such column: repository_id`.
- GREEN: the same focused command passed after the repair.
- `go test ./internal/task/repository/sqlite -run TestCutover -count=1` passed.
- `go test ./internal/task/repository/sqlite -count=1` passed.
- `go test ./internal/task/repository/sqlite -run
TestPostgresCutoverHybridNormalizedEnvironmentWithLegacySessionWorktrees
-count=1 -v` compiled and skipped locally because
`KANDEV_TEST_POSTGRES_DSN` is unset; the Backend Postgres CI job owns the live
dialect execution.
- `git diff --check` passed.
@@ -0,0 +1,88 @@
---
id: "01-recover-hybrid-schema"
title: "Recover hybrid cutover schemas"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/session-delete-resource-cleanup/spec.md"
---
# Task 01: Recover Hybrid Cutover Schemas
## Acceptance
- A hybrid database with normalized `task_environments` and a stale
`task_session_worktrees` table starts successfully without manual edits.
- Canonical environment/repository data and remaining session-worktree inventory
survive normalization, and the stale legacy table is removed.
- Fully legacy conflicts still fail closed and all existing cutover fixtures
retain their behavior.
## Verification
```bash
cd apps/backend && go test ./internal/task/repository/sqlite -run TestCutover_HybridNormalizedEnvironmentWithLegacySessionWorktrees -count=1 -v
go test ./internal/task/repository/sqlite -run TestPostgresCutoverHybridNormalizedEnvironmentWithLegacySessionWorktrees -count=1 -v
go test ./internal/task/repository/sqlite -run TestCutover -count=1
go test ./internal/task/repository/sqlite -count=1
```
## Files Likely Touched
- `apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go`
- `apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go`
- `apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go`
- `apps/backend/internal/task/repository/sqlite/postgres_schema_test.go`
- `docs/specs/session-delete-resource-cleanup/spec.md`
- `docs/plans/task-worktree-cutover-hybrid-schema/plan.md`
- `docs/plans/task-worktree-cutover-hybrid-schema/task-01-recover-hybrid-schema.md`
## Dependencies
None.
## Parallelism
Sequential. Production and test changes share the cutover schema and migration
contract.
## Inputs
- Confirmed `main` reproduction through `NewWithDB` with the exact error
`cutover: read legacy environments: no such column: repository_id`.
- Existing `openLegacyDB`, schema-shape assertions, inventory checks, and
cutover rollback tests in `worktree_ownership_migration_test.go`.
- The transactional cutover contract in the linked spec.
## Output Contract
Report the red failure, green test results, files changed, compatibility scope,
remaining risks, synchronized task/plan status, commit, and PR URL.
## Results
- Added a real-initializer hybrid-schema regression that preserves canonical
normalized ownership and removes the stale legacy table.
- The cutover probes the four deprecated flat environment columns inside its
transaction and substitutes typed empty values only for columns already
absent. Required columns and downstream inventory validation are unchanged.
- RED: the focused hybrid test failed with the reported missing-column error.
- GREEN: the focused hybrid test, all cutover tests, and the complete SQLite
repository package passed.
- The PostgreSQL counterpart compiled and used the standard environment gate;
it skipped locally because `KANDEV_TEST_POSTGRES_DSN` is unset. The PR's
Backend Postgres job executes the live-dialect test.
- `git diff --check` passed.
- External side effects: none. Tests used temporary SQLite databases only.
- Files changed: the cutover migration/normalizer, SQLite and PostgreSQL
migration tests, the worktree cleanup spec, and this plan/task package.
- Compatibility scope: only hybrid schemas missing one or more of the four
deprecated flat `task_environments` worktree columns are relaxed. Required
columns, conflict detection, inventory validation, and rollback remain strict.
- Remaining risk: unrecognized intermediate schemas missing required columns
still fail closed by design; local PostgreSQL execution requires the external
test DSN and is therefore delegated to the existing CI job.
- Task and plan status are synchronized as `done` and `implemented`.
- Initial implementation commit: `2d3384b01789d8aff735151249f66defd3db5863`.
- Pull request: https://github.com/kdlbs/kandev/pull/2531
@@ -316,6 +316,11 @@ remain the authorization boundary for physical cleanup.
the surviving flat environment's repository slot, **WHEN** the new binary
starts, **THEN** the flat environment remains the owner and the cutover
completes.
- **GIVEN** `task_environments` already has the normalized schema but a stale
`task_session_worktrees` table remains from an intermediate build, **WHEN**
the new binary starts, **THEN** the cutover preserves normalized environment
and repository data, imports any remaining session-worktree inventory, and
removes the stale legacy table without requiring manual database edits.
- **GIVEN** a legacy session of one task is bound to another task's environment
and carries a session-worktree row for that shared workspace, **WHEN** the new
binary starts, **THEN** the worktree is normalized onto the owning task's