fix(task): recover cutover from stale session metadata (#2471)

This commit is contained in:
Carlos Florêncio
2026-08-09 22:45:46 +01:00
committed by GitHub
parent 52be8b8e47
commit 84ce139b35
7 changed files with 371 additions and 35 deletions
@@ -111,16 +111,17 @@ func (r *Repository) normalizeTaskWorktreeOwnership() error {
return err
}
cut := &worktreeCutover{
envs: make(map[string]*legacyEnv),
taskEnvs: make(map[string][]*legacyEnv),
sessions: make(map[string]*legacySession),
sessionTasks: make(map[string]string),
sessionEnvIDs: make(map[string]string),
historicalWorktreeSuperseded: make(map[string]bool),
tasks: make(map[string]*taskWorktreeTargets),
taskEnvIDs: make(map[string]string),
loserEnvIDs: make(map[string]bool),
executorTypes: make(map[string]string),
envs: make(map[string]*legacyEnv),
taskEnvs: make(map[string][]*legacyEnv),
sessions: make(map[string]*legacySession),
sessionTasks: make(map[string]string),
sessionEnvIDs: make(map[string]string),
sessionWorktreeSuperseded: make(map[string]bool),
authoritativeWorktreeIDs: make(map[string]bool),
tasks: make(map[string]*taskWorktreeTargets),
taskEnvIDs: make(map[string]string),
loserEnvIDs: make(map[string]bool),
executorTypes: make(map[string]string),
}
if err := cut.loadLegacy(tx); err != nil {
return err
@@ -569,6 +569,68 @@ func TestCutover_CanonicalRepoWinsOverStaleFlatMetadata(t *testing.T) {
}
}
// TestCutover_PrefersCanonicalMetadataForResumableSession proves that stale
// session metadata cannot override a canonical row for the same worktree.
func TestCutover_PrefersCanonicalMetadataForResumableSession(t *testing.T) {
db := openLegacyDB(t)
now := time.Now().UTC().Truncate(time.Second)
seed := legacySeed{envID: "env-resumable", taskID: "task-resumable", repoID: "repo-resumable", sessionID: "sess-resumable"}
seedLegacyTask(t, db, seed, now)
if _, err := db.Exec(db.Rebind(`UPDATE task_sessions SET state = 'WAITING_FOR_INPUT' WHERE id = ?`), seed.sessionID); err != nil {
t.Fatalf("set session state: %v", err)
}
seedLegacyFlatEnv(t, db, seed, "wt-resumable", "/tasks/resumable/repo", "feature/current", now)
seedLegacyEnvRepo(t, db, "env-repo-resumable", seed.envID, seed.repoID, "wt-resumable", "/tasks/resumable/repo", "feature/current", now)
seedLegacySessionWorktree(t, db, seed.sessionID, "wt-resumable", seed.repoID, "", "/tasks/resumable/repo", "feature/stale-session", "active", 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)
}
session, err := repo.GetTaskSession(context.Background(), seed.sessionID)
if err != nil {
t.Fatalf("get task session: %v", err)
}
if len(env.Repos) != 1 || env.Repos[0].WorktreeBranch != "feature/current" ||
session.State != "WAITING_FOR_INPUT" {
t.Fatalf("normalized environment = %+v, session state = %q", env.Repos, session.State)
}
}
// TestCutover_PrefersFlatMetadataForSameWorktree proves that the surviving
// flat environment beats stale session metadata when no canonical row exists.
func TestCutover_PrefersFlatMetadataForSameWorktree(t *testing.T) {
for _, state := range []string{"RUNNING", "WAITING_FOR_INPUT", "CANCELLED"} {
t.Run(state, func(t *testing.T) {
db := openLegacyDB(t)
now := time.Now().UTC().Truncate(time.Second)
seed := legacySeed{envID: "env-flat", taskID: "task-flat", repoID: "repo-flat", sessionID: "sess-flat"}
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", "/tasks/flat/repo", "feature/current", now)
seedLegacySessionWorktree(t, db, seed.sessionID, "wt-flat", seed.repoID, "", "/tasks/flat/repo", "feature/stale-session", "active", 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].WorktreeBranch != "feature/current" {
t.Fatalf("normalized environment = %+v", env.Repos)
}
})
}
}
// TestCutover_IgnoresDeletedHistoricalSessionConflict proves that a deleted
// session reference cannot override an existing task-owned repository row.
func TestCutover_IgnoresDeletedHistoricalSessionConflict(t *testing.T) {
@@ -14,19 +14,20 @@ import (
// worktreeCutover holds the legacy inventory and the normalized result.
type worktreeCutover struct {
envs map[string]*legacyEnv
taskEnvs map[string][]*legacyEnv
sessions map[string]*legacySession
sessionTasks map[string]string
sessionEnvIDs map[string]string
historicalWorktreeSuperseded map[string]bool
envRepos []legacyEnvRepo
sessionWts []legacySessionWorktree
tasks map[string]*taskWorktreeTargets
taskEnvIDs map[string]string
loserEnvIDs map[string]bool
executorTypes map[string]string
conflicts []string
envs map[string]*legacyEnv
taskEnvs map[string][]*legacyEnv
sessions map[string]*legacySession
sessionTasks map[string]string
sessionEnvIDs map[string]string
sessionWorktreeSuperseded map[string]bool
authoritativeWorktreeIDs map[string]bool
envRepos []legacyEnvRepo
sessionWts []legacySessionWorktree
tasks map[string]*taskWorktreeTargets
taskEnvIDs map[string]string
loserEnvIDs map[string]bool
executorTypes map[string]string
conflicts []string
}
type legacyEnv struct {
@@ -300,7 +301,9 @@ func (c *worktreeCutover) mergeLegacyEnvRepoRows() {
targets := c.targetsForTask(env.taskID)
if err := targets.mergeLegacyEnvRepo(row); err != nil {
c.conflicts = append(c.conflicts, fmt.Sprintf("environment %s repository row: %v", row.envID, err))
continue
}
c.recordAuthoritativeWorktree(env.taskID, row.worktreeID)
}
}
@@ -314,7 +317,9 @@ func (c *worktreeCutover) mergeFlatEnvironmentFields() {
targets := c.targetsForTask(env.taskID)
if err := targets.mergeFlatEnv(env); err != nil {
c.conflicts = append(c.conflicts, fmt.Sprintf("environment %s flat worktree fields: %v", env.id, err))
continue
}
c.recordAuthoritativeWorktree(env.taskID, env.worktreeID)
}
}
@@ -327,7 +332,7 @@ func (c *worktreeCutover) mergeSessionWorktrees() {
continue // already reported in load
}
targets := c.targetsForTask(session.taskID)
if err := targets.mergeSessionWorktree(wt, c.isSupersededHistoricalWorktree(wt)); err != nil {
if err := targets.mergeSessionWorktree(wt, c.isSupersededSessionWorktree(wt)); err != nil {
c.conflicts = append(c.conflicts, fmt.Sprintf(
"session %s worktree %s: %v", wt.sessionID, wt.worktreeID, err))
}
@@ -455,20 +460,25 @@ func (c *worktreeCutover) linkSessions() {
}
}
// isSupersededHistoricalWorktree reports whether a legacy row is no longer an
// ownership source because a canonical repository row already represents the
// same task/repository slot. A historical row with no canonical replacement
// remains eligible for backfill.
func (c *worktreeCutover) isSupersededHistoricalWorktree(wt legacySessionWorktree) bool {
// 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.
func (c *worktreeCutover) isSupersededSessionWorktree(wt legacySessionWorktree) bool {
cacheKey := wt.sessionID + "\x00" + wt.worktreeID
if superseded, ok := c.historicalWorktreeSuperseded[cacheKey]; ok {
if superseded, ok := c.sessionWorktreeSuperseded[cacheKey]; ok {
return superseded
}
session, hasSession := c.sessions[wt.sessionID]
superseded := false
if isLegacyDeletedWorktree(wt) {
switch {
case isLegacyDeletedWorktree(wt):
superseded = true
} else if session, ok := c.sessions[wt.sessionID]; ok && isLegacyHistoricalSession(session.state) {
case hasSession && c.authoritativeWorktreeIDs[authoritativeWorktreeKey(session.taskID, wt.worktreeID)]:
superseded = true
case hasSession && isLegacyHistoricalSession(session.state):
for _, row := range c.envRepos {
env, ok := c.envs[row.envID]
if !ok || env.taskID != session.taskID || row.worktreeID == "" {
@@ -480,10 +490,21 @@ func (c *worktreeCutover) isSupersededHistoricalWorktree(wt legacySessionWorktre
}
}
}
c.historicalWorktreeSuperseded[cacheKey] = superseded
c.sessionWorktreeSuperseded[cacheKey] = superseded
return superseded
}
func (c *worktreeCutover) recordAuthoritativeWorktree(taskID, worktreeID string) {
if worktreeID == "" {
return
}
c.authoritativeWorktreeIDs[authoritativeWorktreeKey(taskID, worktreeID)] = true
}
func authoritativeWorktreeKey(taskID, worktreeID string) string {
return taskID + "\x00" + worktreeID
}
// registerWorktreeIdentities ensures every physical worktree is owned by
// exactly one environment-repository key.
func (c *worktreeCutover) registerWorktreeIdentities() {
@@ -268,7 +268,7 @@ func (r *Repository) validateCutover(c *worktreeCutover, tx *sqlx.Tx) error {
// Every session that referenced a worktree must resolve to an
// environment that owns that worktree.
for _, wt := range c.sessionWts {
if c.isSupersededHistoricalWorktree(wt) {
if c.isSupersededSessionWorktree(wt) {
continue
}
envID := c.sessionEnvIDs[wt.sessionID]
@@ -333,7 +333,7 @@ func (c *worktreeCutover) legacyWorktreeInventory() map[string]bool {
}
}
for _, wt := range c.sessionWts {
if wt.worktreeID == "" || c.isSupersededHistoricalWorktree(wt) {
if wt.worktreeID == "" || c.isSupersededSessionWorktree(wt) {
continue
}
inventory[worktreeInventoryKey(wt.worktreeID, wt.worktreePath, wt.worktreeBranch)] = true
@@ -0,0 +1,131 @@
---
spec: docs/specs/session-delete-resource-cleanup/spec.md
created: 2026-08-09
status: implemented
---
# Fix Plan: Preserve Physical Identity Precedence During Worktree Cutover
## Overview
Repair the remaining startup regression in the one-time task-worktree ownership
cutover introduced by PR #2456 and partially repaired by PR #2463. The cutover
already loads sources from strongest to weakest, but it still verifies stale
session path and branch metadata against a higher-precedence row for the same
physical `worktree_id`. The repair makes that precedence consistent in merge and
inventory validation while preserving fail-closed behavior for different
physical identities.
Confirmed root cause: `mergeSessionWorktree` calls `verifyWorktree` for a
non-historical session even after `targetForWorktree` resolves the identical
physical worktree from `task_environment_repos` or the surviving flat
`task_environments` row. `legacyWorktreeInventory` likewise retains that stale
session metadata unless the row satisfies the narrower deleted/terminal
classification. A valid legacy database therefore reports branch conflicts and
aborts startup despite having one unambiguous worktree owner.
## Backend
### Source-aware physical identity precedence
- Update
`apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go`
and
`apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go`
so a session-worktree row is classified as a lower-precedence duplicate when
a canonical repository row or the surviving flat environment already carries
the same task and physical `worktree_id`.
- Apply the same classification while merging sources and while constructing
the legacy inventory. This prevents the merge from rejecting stale metadata
and prevents validation from reintroducing the discarded metadata variant as
a missing inventory item.
- Keep the existing deleted/terminal compatibility rules. Continue failing
closed when live rows claim different physical worktree identities for the
same repository slot or when ownership cannot otherwise be resolved.
### Upgrade safety
- Keep the pre-migration snapshot, single transaction, shadow-table checks,
schema rollback, and replay behavior unchanged.
- Do not read or mutate Git worktrees during migration. Resolve precedence only
from the persisted physical identity and the established source ordering.
## Tests
- **What:** A resumable `WAITING_FOR_INPUT` session with stale branch metadata
cannot override a canonical `task_environment_repos` row carrying the same
`worktree_id`.
**File:**
`apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go`.
**How:** Seed the three legacy representations with one physical ID and
different session metadata; run the full repository initializer and assert
that cutover completes with canonical path/branch and unchanged session state.
- **What:** A legacy session row cannot override the surviving flat environment
when no canonical repository row exists and both carry the same physical ID.
**File:** same migration test file.
**How:** Seed flat and session sources with branch drift and assert that the
normalized repository row retains the flat metadata. Cover resumable and
terminal session states so precedence is not state-dependent.
- **What:** Different live physical worktree IDs remain an unresolved conflict.
**File:** same migration test file.
**How:** Retain and run `TestCutover_ConflictingWorktreesFailClosed`, including
its assertions that the legacy schema and data survive rollback.
- **What:** All existing SQLite cutover compatibility, failpoint, replay, and
inventory cases remain valid.
**File:** same migration test file.
**How:** Run the focused cutover test group and then the repository package.
## Verification Results
- RED: the new canonical-versus-resumable and flat-versus-session fixtures
failed with the same stale branch conflict reported by the affected database.
- `go test ./internal/task/repository/sqlite -run
'TestCutover_(PrefersCanonicalMetadataForResumableSession|PrefersFlatMetadataForSameWorktree|ConflictingWorktreesFailClosed)'
-count=1` - passed.
- `go test ./internal/task/repository/sqlite -run 'TestCutover' -count=1` -
passed.
- `go test ./internal/task/repository/sqlite -count=1` - passed.
- A throwaway `-tags fts5` repository-initializer test against a copied affected
database completed the cutover, removed the legacy ownership table, and
produced 295 normalized repository rows. The harness and copies were removed;
the live database and workspaces were untouched.
## Implementation Waves And Parallel Candidates
Wave 1, sequential:
- [x] [task-01-repair-identity-precedence](task-01-repair-identity-precedence.md) - done
No task is parallel-safe because the normalization logic and its migration
fixtures share the same persistence boundary.
## Risks
- Treating path or branch equality as ownership would preserve the current
false-positive failure. The bypass must require an identical non-empty
physical `worktree_id` from a higher-precedence source.
- Broadly ignoring session conflicts would conceal competing worktrees. Existing
different-ID fail-closed coverage must remain unchanged.
- Merge and inventory classification can drift apart and fail later in
validation. Both paths must use the same source-aware predicate.
## Out Of Scope
- Manual edits to user databases or ownership rows.
- Filesystem or Git reconciliation during startup.
- Changes to task/session lifecycle behavior after the one-time cutover.
- Additional public recovery documentation; the current operations and
Kubernetes pages already describe snapshot-based rollback and forbid manual
row deletion.
## PR Fixup Results
- Resolved the invalid request to remove the repair spec amendment: the spec is
`draft`, and the repository `/fix` workflow requires the affected behavioral
spec to be amended before implementation.
- Added explicit `RUNNING` coverage to the flat-environment precedence fixture
and simplified the classifier to perform one session lookup.
- `GOCACHE=/tmp/kandev-go-cache go test
./internal/task/repository/sqlite -run
'TestCutover_(PrefersCanonicalMetadataForResumableSession|PrefersFlatMetadataForSameWorktree|ConflictingWorktreesFailClosed)'
-count=1` - passed.
@@ -0,0 +1,104 @@
---
id: "01-repair-identity-precedence"
title: "Repair cutover physical identity precedence"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/session-delete-resource-cleanup/spec.md"
---
# Task 01: Repair Cutover Physical Identity Precedence
## Acceptance
- A higher-precedence repository or surviving flat environment row wins stale
path/branch metadata from a session row only when both carry the same non-empty
physical `worktree_id` for the task.
- Merge and inventory validation apply the same source-aware duplicate
classification for resumable, terminal, and deleted legacy sessions.
- Different live physical identities remain fatal and the cutover transaction
preserves the full legacy schema and data on failure.
- The migration remains database-only, replay-safe, and covered for SQLite and
PostgreSQL through the existing shared normalization path.
## Verification
```bash
cd apps/backend && go test ./internal/task/repository/sqlite -run 'TestCutover_(PrefersCanonicalMetadataForResumableSession|PrefersFlatMetadataForSameWorktree|ConflictingWorktreesFailClosed)' -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_migration_test.go`
- `docs/specs/session-delete-resource-cleanup/spec.md`
- `docs/plans/task-owned-worktree-cutover-identity-precedence/plan.md`
- `docs/plans/task-owned-worktree-cutover-identity-precedence/task-01-repair-identity-precedence.md`
## Dependencies
None.
## Parallelism
Sequential. The source precedence predicate, merge behavior, inventory
validation, and regression fixtures share one migration boundary.
## Inputs
- The schema normalization and failure-mode sections of the repair spec.
- Existing `mergeSessionWorktree`, `isSupersededHistoricalWorktree`,
`legacyWorktreeInventory`, and `targetForWorktree` behavior.
- Existing canonical-precedence, terminal-history, failpoint, replay, SQLite,
and PostgreSQL cutover tests.
- Read-only affected database evidence: all 16 reported conflicts reuse the
exact higher-precedence `worktree_id`; 9 are canonical repository versus
`WAITING_FOR_INPUT` session drift and 7 are flat environment versus terminal
session drift.
## Output Contract
Report the predicate and precedence changes, exact test commands and results,
any state or identity assumptions, cleanup of temporary reproduction artifacts,
and synchronized task/plan status.
## Results
- Added a task-scoped index of physical IDs already owned by canonical
repository rows or the surviving flat environment. Legacy session rows use
the same superseded-source predicate during merge and inventory validation.
- Preserved the existing strict verifier for session-only duplicates and the
existing fail-closed behavior for different live physical IDs.
- Added SQLite migration fixtures for canonical-versus-resumable and
flat-environment-versus-session branch drift, covering `WAITING_FOR_INPUT`
and `CANCELLED` session states.
- RED: `go test ./internal/task/repository/sqlite -run
'TestCutover_(PrefersCanonicalMetadataForResumableSession|PrefersFlatMetadataForSameWorktree)'
-count=1` failed with the expected stale branch conflicts.
- GREEN: `go test ./internal/task/repository/sqlite -run
'TestCutover_(PrefersCanonicalMetadataForResumableSession|PrefersFlatMetadataForSameWorktree|ConflictingWorktreesFailClosed)'
-count=1` passed.
- `go test ./internal/task/repository/sqlite -run 'TestCutover' -count=1`
passed.
- `go test ./internal/task/repository/sqlite -count=1` passed.
- Copied the affected SQLite database to `/tmp` and ran the repository
initializer against the copy with `go test -tags fts5`; the cutover completed,
removed the legacy ownership table, and produced 295 normalized repository
rows while retaining at least the 129 pre-existing canonical rows.
- The first copied-database run without a temporary `GOCACHE` was blocked by
sandbox cache permissions; the next run without `-tags fts5` failed because
the developer SQLite driver lacked FTS5. The tagged rerun passed.
- Removed the temporary copied databases and throwaway reproduction test. No
filesystem, Git worktree, or live user database was mutated.
- PR fixup added explicit `RUNNING` coverage for flat-environment precedence
and replaced duplicate session-map lookups with one lookup before the
supersession decision.
- The first focused fixup test could not write the sandboxed default Go cache;
`GOCACHE=/tmp/kandev-go-cache go test
./internal/task/repository/sqlite -run
'TestCutover_(PrefersCanonicalMetadataForResumableSession|PrefersFlatMetadataForSameWorktree|ConflictingWorktreesFailClosed)'
-count=1` passed.
@@ -107,6 +107,11 @@ Legacy sessions are assigned to the matching existing environment or to a
normalized task-owned environment created for their connected worktree group.
Canonical `task_environment_repos` rows take precedence when the legacy flat
environment fields or session references repeat the same physical worktree.
When no canonical repository row exists, the surviving task environment's flat
worktree fields take precedence over a legacy session reference to the same
physical `worktree_id`. This source precedence applies regardless of session
lifecycle state: path or branch drift on a lower-precedence row does not create
a second owner when the physical identity is unchanged.
Legacy session rows marked `deleted` (or carrying `deleted_at`) and stale
references from terminal sessions are historical evidence, not additional
owners, and bypass validation only when a canonical repository owner exists.
@@ -211,6 +216,10 @@ remain the authorization boundary for physical cleanup.
- Historical deleted session-worktree rows and stale references from terminal
sessions do not block migration when a canonical task-owned repository row
exists; the canonical row remains authoritative.
- A legacy session row that repeats a higher-precedence physical `worktree_id`
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.
- If migration fails after shadow tables are populated or after legacy DDL has
begun, the database transaction restores the complete legacy schema and data.
- If SQLite cannot create its pre-upgrade snapshot, startup stops before the
@@ -272,6 +281,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 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
cutover completes without changing the session lifecycle state.
- **GIVEN** no canonical repository row exists and a legacy session reference
repeats the surviving task environment's `worktree_id` with stale path or
branch metadata, **WHEN** the new binary starts, **THEN** the flat environment
metadata is retained 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