fix(orchestrator): gate worktree reuse on live inventory (#2987)

This commit is contained in:
Carlos Florêncio
2026-08-24 19:50:42 +00:00
committed by GitHub
parent 54d507b8d7
commit 42cd897dfb
7 changed files with 405 additions and 26 deletions
@@ -1682,23 +1682,23 @@ func workspaceReuseAllowed(existingEnv *models.TaskEnvironment, requestedExecuto
if !required || existingEnv == nil {
return required
}
if existingEnv.ExecutorType == "" {
// Older environments did not persist their executor type. They remain
// attachable for local/container launchers, but a worktree launch may
// attach only when the durable inventory contains a physical worktree.
// Inventory-only rows deliberately have no WorktreeID and cannot satisfy
// the worktree preparer's attach-only contract.
if requestedExecutorType == string(models.ExecutorTypeWorktree) {
for _, repo := range existingEnv.Repos {
if repo != nil && repo.WorktreeID != "" && repo.DeletedAt == nil && repo.Status != taskEnvironmentRepoStatusFailed && repo.Status != taskEnvironmentRepoStatusDeleted {
return true
}
}
return false
if existingEnv.ExecutorType != "" && existingEnv.ExecutorType != requestedExecutorType {
return false
}
if requestedExecutorType == string(models.ExecutorTypeWorktree) {
return hasLiveWorktreeRepo(existingEnv)
}
return true
}
func hasLiveWorktreeRepo(env *models.TaskEnvironment) bool {
for _, repo := range env.Repos {
if repo == nil || repo.WorktreeID == "" || repo.DeletedAt != nil || repo.Status == taskEnvironmentRepoStatusFailed || repo.Status == taskEnvironmentRepoStatusDeleted {
continue
}
return true
}
return existingEnv.ExecutorType == requestedExecutorType
return false
}
func mergeEnv(req *LaunchAgentRequest, env map[string]string) {
@@ -387,20 +387,22 @@ func TestLaunchPreparedSession_ReuseRejectsIncompleteCanonicalRepositoryInventor
seedMultiRepoTask(t, repo, taskID)
seedWorktreeExecutor(repo)
environment := &models.TaskEnvironment{
ID: "env-incomplete",
TaskID: taskID,
ExecutorType: string(models.ExecutorTypeWorktree),
Status: models.TaskEnvironmentStatusReady,
}
repo.taskEnvironments[environment.ID] = environment
repo.taskEnvironmentRepos[environment.ID] = []*models.TaskEnvironmentRepo{{
TaskEnvironmentID: environment.ID,
environmentRepos := []*models.TaskEnvironmentRepo{{
TaskEnvironmentID: "env-incomplete",
RepositoryID: "repo-front",
BranchSlug: "main",
WorktreeID: "wt-front",
Status: "active",
}}
environment := &models.TaskEnvironment{
ID: "env-incomplete",
TaskID: taskID,
ExecutorType: string(models.ExecutorTypeWorktree),
Status: models.TaskEnvironmentStatusReady,
Repos: environmentRepos,
}
repo.taskEnvironments[environment.ID] = environment
repo.taskEnvironmentRepos[environment.ID] = environmentRepos
repo.sessions[sessionID] = &models.TaskSession{
ID: sessionID, TaskID: taskID, TaskEnvironmentID: environment.ID,
AgentProfileID: "profile-123", ExecutorID: models.ExecutorIDWorktree,
@@ -0,0 +1,127 @@
package executor
import (
"testing"
"time"
"github.com/kandev/kandev/internal/task/models"
)
// @covers AC-TASKS-RUNTIME-CLEANUP-001.7
func TestWorkspaceReuseAllowedRequiresLiveWorktreeRepo(t *testing.T) {
deletedAt := time.Unix(123, 0)
tests := []struct {
name string
env *models.TaskEnvironment
requestedType string
reuseRequired bool
wantReuseAllowed bool
}{
{
name: "persisted worktree with deleted row",
env: &models.TaskEnvironment{
ExecutorType: string(models.ExecutorTypeWorktree),
Repos: []*models.TaskEnvironmentRepo{{
WorktreeID: "wt-deleted",
Status: "active",
DeletedAt: &deletedAt,
}},
},
requestedType: string(models.ExecutorTypeWorktree),
reuseRequired: true,
wantReuseAllowed: false,
},
{
name: "persisted worktree with failed row",
env: &models.TaskEnvironment{
ExecutorType: string(models.ExecutorTypeWorktree),
Repos: []*models.TaskEnvironmentRepo{{
WorktreeID: "wt-failed",
Status: taskEnvironmentRepoStatusFailed,
}},
},
requestedType: string(models.ExecutorTypeWorktree),
reuseRequired: true,
wantReuseAllowed: false,
},
{
name: "persisted worktree with deleted status",
env: &models.TaskEnvironment{
ExecutorType: string(models.ExecutorTypeWorktree),
Repos: []*models.TaskEnvironmentRepo{{
WorktreeID: "wt-deleted-status",
Status: taskEnvironmentRepoStatusDeleted,
}},
},
requestedType: string(models.ExecutorTypeWorktree),
reuseRequired: true,
wantReuseAllowed: false,
},
{
name: "persisted worktree without physical id",
env: &models.TaskEnvironment{
ExecutorType: string(models.ExecutorTypeWorktree),
Repos: []*models.TaskEnvironmentRepo{{
Status: "active",
}},
},
requestedType: string(models.ExecutorTypeWorktree),
reuseRequired: true,
wantReuseAllowed: false,
},
{
name: "persisted worktree with live row",
env: &models.TaskEnvironment{
ExecutorType: string(models.ExecutorTypeWorktree),
Repos: []*models.TaskEnvironmentRepo{{
WorktreeID: "wt-live",
Status: "active",
}},
},
requestedType: string(models.ExecutorTypeWorktree),
reuseRequired: true,
wantReuseAllowed: true,
},
{
name: "executor type mismatch",
env: &models.TaskEnvironment{
ExecutorType: string(models.ExecutorTypeLocal),
Repos: []*models.TaskEnvironmentRepo{{
WorktreeID: "wt-live",
Status: "active",
}},
},
requestedType: string(models.ExecutorTypeWorktree),
reuseRequired: true,
wantReuseAllowed: false,
},
{
name: "matching non-worktree executor",
env: &models.TaskEnvironment{ExecutorType: string(models.ExecutorTypeLocal)},
requestedType: string(models.ExecutorTypeLocal),
reuseRequired: true,
wantReuseAllowed: true,
},
{
name: "reuse not required",
env: &models.TaskEnvironment{
ExecutorType: string(models.ExecutorTypeWorktree),
Repos: []*models.TaskEnvironmentRepo{{
WorktreeID: "wt-live",
Status: "active",
}},
},
requestedType: string(models.ExecutorTypeWorktree),
reuseRequired: false,
wantReuseAllowed: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := workspaceReuseAllowed(tt.env, tt.requestedType, tt.reuseRequired); got != tt.wantReuseAllowed {
t.Fatalf("workspaceReuseAllowed() = %t, want %t", got, tt.wantReuseAllowed)
}
})
}
}
@@ -0,0 +1,118 @@
---
created: 2026-08-24
status: done
requirements:
- REQ-TASKS-RUNTIME-CLEANUP-001
system_design:
- ../../specs/tasks/system-design/runtime-cleanup.md
legacy_specs: []
---
# Implementation Plan: Worktree Resume After Unarchive
## Overview
Correct resume routing for an archived and then unarchived worktree task. The
executor will reserve attach-only reuse for environments with a matching
executor and a live physical worktree row; otherwise it will use the existing
worktree recreate path. One sequential work order owns the regression test,
the narrow decision-helper change, and targeted verification.
## Confirmed root cause
`buildResumeRequest` requests workspace reuse when the existing environment was
not materialized by the resuming session. `workspaceReuseAllowed` validates a
live worktree row only for legacy environments whose `ExecutorType` is empty.
For a normal worktree environment it currently returns true solely because the
executor types match, even if every repository row is deleted, failed, or
tombstoned. This selects `worktree.Manager.reuseRequiredWorktree`, whose
attach-only contract correctly rejects the non-active or missing worktree with
`ErrReuseWorktreeUnavailable`.
## Scope
### In scope
- Require a live physical worktree repository row before enabling attach-only
reuse for every worktree environment, including environments with a persisted
executor type.
- Preserve executor-type matching and all non-worktree reuse behavior.
- Add table-driven regression coverage for deleted, failed, live, mismatched,
non-worktree, and `required=false` cases.
- Exercise the existing manager recovery test that proves a released worktree
row can be reactivated after archive cleanup.
### Out of scope
- Weakening `reuseRequiredWorktree` or allowing attach-only reuse of deleted
paths.
- Changing archive cleanup, branch recovery, unarchive APIs, schema, or store
persistence.
- Recovering a branch that is absent both locally and remotely.
- Changing reuse behavior for a live worktree or any non-worktree executor.
## Technical approach
In `executor_execute.go`, keep the current early return when reuse is not
required or no environment exists. For an environment with a persisted
executor type, reject a requested type mismatch first. For any requested
worktree executor, delegate to a small `hasLiveWorktreeRepo` helper. The helper
uses the existing live-row predicate: a non-empty `WorktreeID`, nil
`DeletedAt`, and a status other than failed or deleted. Legacy non-worktree
environments remain attachable as before.
When the helper returns false during resume, existing downstream behavior is
already suitable:
- `applyResumeWorktreeConfig` stamps `TaskDirName` and `RepoName`.
- `reuseExistingEnvironment` may carry a historical worktree ID but does not
require attach-only preparation.
- `worktree.Manager.Create` can resolve a soft-deleted row by ID, recreate the
recoverable branch and directory, and update the row to active with
`deleted_at` cleared.
- If no historical ID exists, `SQLiteStore.CreateWorktree` uses an upsert on
`(task_environment_id, repository_id, branch_slug)`, so a soft-deleted row
does not cause a unique-constraint failure.
No repository or schema method needs to change.
## Tests
- **AC-TASKS-RUNTIME-CLEANUP-001.7:** Add
`TestWorkspaceReuseAllowedRequiresLiveWorktreeRepo` in a new executor test
file. The current `executor_environment_test.go` is already above the backend
800-line file limit, so the sibling file avoids extending an oversized test.
The regression case must fail before the production change because a normal
worktree environment with only a deleted row is incorrectly attachable.
- Preserve `TestWorkspaceReuseAllowedRequiresMatchingExecutorType` and the
`TestReuseExistingEnvironment_*` suite.
- Run `TestCreate_RestoresReleasedWorktreeAfterArchive` as existing integration
evidence for recreate/reactivation and soft-deleted-row persistence.
## Work orders
- [done] [Task 01: Gate Worktree Reuse on Live Inventory](task-01-gate-worktree-reuse.md)
## Verification results
- RED: `go test ./internal/orchestrator/executor -run '^TestWorkspaceReuseAllowedRequiresLiveWorktreeRepo$' -race`
failed on the four invalid inventory cases before the production change.
- GREEN: the focused regression passed with 8 cases.
- `go test ./internal/orchestrator/executor/... -run 'WorkspaceReuse|ReuseExistingEnvironment' -race`
passed with 31 tests.
- `go test ./internal/worktree/... -run 'TestCreate_RestoresReleasedWorktreeAfterArchive' -race`
passed with 1 test in 2 packages.
- `go test ./internal/orchestrator/... ./internal/worktree/... -race` passed with
3,362 tests across 11 packages.
- `make -C apps/backend lint` passed with 0 issues.
## Risks
- The live-row predicate must remain identical to the canonical environment
inventory predicate so attach eligibility and later validation cannot drift.
- A worktree branch absent locally and remotely still fails with the existing
typed recovery error; this fix changes only the incorrect attach-only route.
- Multi-repository environments need only one live row for this coarse routing
decision; the existing canonical inventory validator still requires exactly
one live row for every requested repository/branch slot when attach-only
reuse proceeds.
@@ -0,0 +1,109 @@
---
id: "01-gate-worktree-reuse"
title: "Gate worktree reuse on live inventory"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-TASKS-RUNTIME-CLEANUP-001
acceptance_criteria:
- AC-TASKS-RUNTIME-CLEANUP-001.7
system_design:
- ../../specs/tasks/system-design/runtime-cleanup.md
---
# Task 01: Gate Worktree Reuse on Live Inventory
## Summary
Make the executor select attach-only worktree reuse only when the existing task
environment contains a live physical worktree row. Archived and unarchived
environments with deleted inventory will enter the existing recreate path.
## In scope
- Add a failing unit regression for normal worktree environments whose only
physical row is deleted or failed.
- Extract and use a live-worktree-row helper in `workspaceReuseAllowed`.
- Preserve executor mismatch, legacy, non-worktree, and `required=false`
behavior.
- Run the existing worktree recreate/reactivation integration test.
## Out of scope
- Changes to worktree attach validation, archive cleanup, branch recovery,
persistence, migrations, frontend behavior, or public documentation.
- New recovery behavior when the historical branch is absent locally and
remotely.
## Acceptance
- A requested worktree executor returns false for attach-only reuse when every
environment repository row is deleted, failed, tombstoned, or lacks a
worktree ID, regardless of whether `ExecutorType` is legacy-empty or set.
- A matching worktree environment with a live row remains reusable, while
executor mismatch and `required=false` return false and matching non-worktree
executors remain reusable.
- Existing environment reuse and released-worktree recreation tests pass.
## Verification
Run the new regression before the production change and confirm it fails for
the deleted-row assertion. After the minimal fix, run:
```bash
# From apps/backend:
rtk go test ./internal/orchestrator/executor/... -run 'WorkspaceReuse|ReuseExistingEnvironment' -race
rtk go test ./internal/worktree/... -run 'TestCreate_RestoresReleasedWorktreeAfterArchive' -race
rtk go test ./internal/orchestrator/... ./internal/worktree/... -race
# From the repository root:
rtk make -C apps/backend lint
```
## Files likely touched
- `apps/backend/internal/orchestrator/executor/executor_execute.go`
- `apps/backend/internal/orchestrator/executor/executor_workspace_reuse_test.go`
- `docs/plans/worktree-resume-after-unarchive/plan.md`
- `docs/plans/worktree-resume-after-unarchive/task-01-gate-worktree-reuse.md`
## Dependencies
None.
## Risks
- A divergent live-row predicate could route an unsafe inventory into
attach-only preparation. Reuse the existing failed/deleted status constants
and exact tombstone checks.
- The existing environment may contain a historical ID. Normal preparation must
remain allowed to use it for branch recovery; do not clear it merely because
attach-only reuse is disabled.
## Parallelism
`sequential`
## Inputs
- `REQ-TASKS-RUNTIME-CLEANUP-001` and
`AC-TASKS-RUNTIME-CLEANUP-001.7`.
- `docs/specs/tasks/system-design/runtime-cleanup.md`.
- Existing `workspaceReuseAllowed`, `canonicalInventoryMatches`,
`reuseExistingRepositoryWorktrees`, `applyResumeWorktreeConfig`, and
`worktree.Manager.Create` behavior.
## Results
- Added `TestWorkspaceReuseAllowedRequiresLiveWorktreeRepo` covering deleted,
failed, tombstoned, missing-ID, live, mismatch, non-worktree, and optional
reuse cases.
- `workspaceReuseAllowed` now gates attach-only worktree reuse on a live
physical environment row while preserving executor matching and legacy
non-worktree reuse.
- Corrected the multi-repository inventory test fixture to provide the same
hydrated environment rows as the production repository implementation.
- Verification passed: the focused executor/worktree race tests, the complete
orchestrator/worktree race suite (3,362 tests), and backend lint (0 issues).
@@ -2,7 +2,7 @@
status: draft
system: tasks
created: 2026-06-22
updated: 2026-08-08
updated: 2026-08-24
owners:
- cfl
---
@@ -28,3 +28,7 @@ and safe when runtimes or task rows are already gone.
- **AC-TASKS-RUNTIME-CLEANUP-001.4:** When an agent or agentctl process does not stop within its grace period, the system shall terminate the complete process group and shall not report shutdown complete while descendants remain attached to PID 1.
- **AC-TASKS-RUNTIME-CLEANUP-001.5:** When startup reconciliation sees a stale runtime row, the system shall remove only a confirmed-dead local runtime, preserve alive, unknown, remote, or generically failed rows for retry, and emit bounded diagnostics for fail-closed outcomes.
- **AC-TASKS-RUNTIME-CLEANUP-001.6:** When lifecycle cleanup begins, the system shall persist an operation snapshot and retry state before mutating task state; repeated delivery shall reuse the durable job, and unarchiving shall prevent a pending archive retry from deleting active resources.
- **AC-TASKS-RUNTIME-CLEANUP-001.7:** When an archived task is unarchived after
cleanup removed its physical worktree, resuming the task shall recreate or
reactivate the recoverable task-owned worktree instead of requiring
attach-only reuse of the deleted workspace.
@@ -4,7 +4,7 @@ system: tasks
requirements:
- REQ-TASKS-RUNTIME-CLEANUP-001
created: 2026-06-22
updated: 2026-08-08
updated: 2026-08-24
owners:
- cfl
---
@@ -124,6 +124,16 @@ worktrees, or executor rows behind and the machine slowly runs out of memory.
- Cleanup preserves historical archived-task worktree records and branch metadata
used by unarchive recovery. Filesystem removal does not imply recovery-history
removal.
- A worktree resume uses attach-only preparation only when the requested
executor matches the environment owner and the durable environment inventory
contains a live physical worktree row. When no live physical repository row
exists, normal preparation runs so Git recovery can run.
- Normal worktree preparation retains the historical worktree ID when one is
available. The worktree manager resolves the deleted record, recreates its
recoverable branch and directory, then marks the row active and clears
`deleted_at`. If no historical ID is available, creation persists the new
worktree by updating any soft-deleted row with the same environment,
repository, and branch identity.
## Data Model
@@ -316,6 +326,10 @@ The durable cleanup job wraps that resource lifecycle:
- If an archived task is unarchived before cleanup completes, the worker cancels
remaining archive cleanup. Already completed resource removal remains valid and
the unarchive branch-recovery flow recreates the environment when possible.
- If an unarchived worktree environment has no live physical repository row,
resume does not enter attach-only preparation. It enters normal worktree
preparation, where unavailable branch recovery remains a typed worktree
recovery failure rather than `ErrReuseWorktreeUnavailable`.
- If deleting a session fails, no physical workspace operation has been attempted;
the task-owned worktree record remains authoritative regardless of whether the
session mutation committed.
@@ -476,6 +490,10 @@ The durable cleanup job wraps that resource lifecycle:
- **GIVEN** archive cleanup removed a local worktree, **WHEN** the task is
unarchived, **THEN** its historical worktree branch metadata remains available
for local/remote recovery.
- **GIVEN** an unarchived task environment whose worktree repository row is
deleted, failed, or tombstoned, **WHEN** its session resumes, **THEN** the
executor selects normal worktree preparation and recreates or reactivates the
recoverable task-owned worktree instead of demanding attach-only reuse.
## Out of Scope
@@ -492,4 +510,5 @@ The durable cleanup job wraps that resource lifecycle:
## Implementation plan
[Backend failure containment](../../../plans/backend-failure-containment/plan.md)
- [Backend failure containment](../../../plans/backend-failure-containment/plan.md)
- [Worktree resume after unarchive](../../../plans/worktree-resume-after-unarchive/plan.md)