fix(task): prevent ready task environments with empty repo inventory (#3008)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -336,6 +336,9 @@ func seedMaterializerTask(t *testing.T, ctx context.Context, repo *sqliterepo.Re
|
||||
WorkspacePath: primaryPath,
|
||||
ExecutorType: "worktree", Status: "ready",
|
||||
CreatedAt: now, UpdatedAt: now,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{ID: "env-1-repo-primary", RepositoryID: "repo-1", WorktreePath: primaryPath, CreatedAt: now},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
|
||||
@@ -217,6 +217,17 @@ func handleE2EReset(
|
||||
return
|
||||
}
|
||||
|
||||
// Automation deletion must run while its bound tasks still exist. It
|
||||
// stops live turns before removing run rows and owns cleanup of hidden
|
||||
// automation tasks. Deleting tasks first can strand an open run on a
|
||||
// missing task and make the next test's reset fail.
|
||||
deletedAutomations, autoErr := deleteAutomationsForReset(ctx, automationSvc, workspaceID)
|
||||
if autoErr != nil {
|
||||
log.Error("e2e reset: failed to delete automations", zap.Error(autoErr))
|
||||
c.JSON(http.StatusInternalServerError, gin.H{errKey: autoErr.Error()})
|
||||
return
|
||||
}
|
||||
|
||||
// Route through the task service (rather than a raw SQL DELETE) so
|
||||
// each delete spawns the async cleanup goroutine that stops the
|
||||
// agentctl instance and releases its port. Without this, instances
|
||||
@@ -259,13 +270,6 @@ func handleE2EReset(
|
||||
return
|
||||
}
|
||||
|
||||
deletedAutomations, autoErr := deleteAutomationsForReset(ctx, automationSvc, workspaceID)
|
||||
if autoErr != nil {
|
||||
log.Error("e2e reset: failed to delete automations", zap.Error(autoErr))
|
||||
c.JSON(http.StatusInternalServerError, gin.H{errKey: autoErr.Error()})
|
||||
return
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"deleted_tasks": deletedTasks,
|
||||
"deleted_workflows": deletedWorkflows,
|
||||
|
||||
@@ -354,6 +354,17 @@ func TestResolveRepositoryIDForSubpathMatchesSanitizedRepositoryName(t *testing.
|
||||
if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
|
||||
ID: "env-1", TaskID: "task-1", ExecutorType: "worktree",
|
||||
WorkspacePath: "/tmp", Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{
|
||||
ID: "session-worktree-1",
|
||||
RepositoryID: "repo-1",
|
||||
WorktreeID: "worktree-1",
|
||||
WorktreePath: "/tmp/worktree",
|
||||
WorktreeBranch: "feature/test",
|
||||
BranchSlug: "test",
|
||||
Position: 0,
|
||||
},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
@@ -362,18 +373,6 @@ func TestResolveRepositoryIDForSubpathMatchesSanitizedRepositoryName(t *testing.
|
||||
"env-1", "session-1"); err != nil {
|
||||
t.Fatalf("link session to env: %v", err)
|
||||
}
|
||||
if err := repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{
|
||||
ID: "session-worktree-1",
|
||||
TaskEnvironmentID: "env-1",
|
||||
RepositoryID: "repo-1",
|
||||
WorktreeID: "worktree-1",
|
||||
WorktreePath: "/tmp/worktree",
|
||||
WorktreeBranch: "feature/test",
|
||||
BranchSlug: "test",
|
||||
Position: 0,
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskSessionWorktree: %v", err)
|
||||
}
|
||||
|
||||
got := resolveRepositoryIDForSessionSubpath(ctx, repo, "session-1", "kdlbs-kandev", log)
|
||||
if got != "repo-1" {
|
||||
|
||||
@@ -32,6 +32,8 @@ const legacyLocalPCExecutor = "local_pc"
|
||||
type workspaceSourceMaterializerRepo interface {
|
||||
GetTaskEnvironmentByTaskID(context.Context, string) (*models.TaskEnvironment, error)
|
||||
UpdateTaskEnvironment(context.Context, *models.TaskEnvironment) error
|
||||
ListTaskEnvironmentRepos(context.Context, string) ([]*models.TaskEnvironmentRepo, error)
|
||||
CreateTaskEnvironmentRepo(context.Context, *models.TaskEnvironmentRepo) error
|
||||
ListTaskSessions(context.Context, string) ([]*models.TaskSession, error)
|
||||
ListTaskRepositories(context.Context, string) ([]*models.TaskRepository, error)
|
||||
ListTaskWorkspaceFolders(context.Context, string) ([]*models.TaskWorkspaceFolder, error)
|
||||
@@ -143,6 +145,9 @@ func (m *workspaceSourceMaterializer) materializeHostWorkspaceSources(ctx contex
|
||||
if materializeErr != nil {
|
||||
return nil, materializeErr
|
||||
}
|
||||
if err = m.persistEnvironmentRepositoryInventory(ctx, state.environment.ID, state.environment.ExecutorType, batch, branchMaterializations); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if state.environment.WorkspacePath != materialization.root {
|
||||
state.environment.WorkspacePath, state.environment.UpdatedAt = materialization.root, time.Now().UTC()
|
||||
if err = m.repo.UpdateTaskEnvironment(ctx, state.environment); err != nil {
|
||||
@@ -490,9 +495,120 @@ func (m *workspaceSourceMaterializer) materializeRemoteWorkspaceSources(ctx cont
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := m.persistEnvironmentRepositoryInventory(ctx, state.environment.ID, state.environment.ExecutorType, batch, nil); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &taskservice.WorkspaceSourceMaterializationResult{WorkspacePath: state.environment.WorkspacePath, SessionIDs: ids}, nil
|
||||
}
|
||||
|
||||
// persistEnvironmentRepositoryInventory records the repository slots created
|
||||
// by this attachment after their runtime materialization succeeds. The resume
|
||||
// path validates task_repositories against this canonical inventory before it
|
||||
// reuses an environment, so a live workspace-source attachment must publish
|
||||
// its new slot before the next backend restart.
|
||||
func (m *workspaceSourceMaterializer) persistEnvironmentRepositoryInventory(
|
||||
ctx context.Context,
|
||||
environmentID, executorType string,
|
||||
batch *models.WorkspaceSourceBatch,
|
||||
branchMaterializations []*branchMaterialization,
|
||||
) error {
|
||||
if batch == nil || environmentID == "" {
|
||||
return nil
|
||||
}
|
||||
existing, err := m.loadEnvironmentRepositoryInventory(ctx, environmentID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
for _, row := range workspaceSourceInventoryRows(environmentID, executorType, batch, branchMaterializations, existing) {
|
||||
if err := m.repo.CreateTaskEnvironmentRepo(ctx, row); err != nil {
|
||||
return fmt.Errorf("persist task environment repository %q: %w", row.RepositoryID, err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *workspaceSourceMaterializer) loadEnvironmentRepositoryInventory(ctx context.Context, environmentID string) (map[string]struct{}, error) {
|
||||
rows, err := m.repo.ListTaskEnvironmentRepos(ctx, environmentID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list task environment repository inventory: %w", err)
|
||||
}
|
||||
existing := make(map[string]struct{}, len(rows))
|
||||
for _, row := range rows {
|
||||
if row != nil && row.RepositoryID != "" {
|
||||
existing[environmentRepoInventoryKey(row.RepositoryID, row.BranchSlug)] = struct{}{}
|
||||
}
|
||||
}
|
||||
return existing, nil
|
||||
}
|
||||
|
||||
func workspaceSourceInventoryRows(
|
||||
environmentID, executorType string,
|
||||
batch *models.WorkspaceSourceBatch,
|
||||
branchMaterializations []*branchMaterialization,
|
||||
existing map[string]struct{},
|
||||
) []*models.TaskEnvironmentRepo {
|
||||
materialized := make(map[string]*branchMaterialization, len(branchMaterializations))
|
||||
for _, branch := range branchMaterializations {
|
||||
if branch != nil && branch.repositoryID != "" {
|
||||
materialized[branch.repositoryID] = branch
|
||||
}
|
||||
}
|
||||
rows := make([]*models.TaskEnvironmentRepo, 0, len(batch.Sources))
|
||||
for _, source := range batch.Sources {
|
||||
row, ok := workspaceSourceInventoryRow(environmentID, executorType, source.Repository, materialized)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
if _, found := existing[environmentRepoInventoryKey(row.RepositoryID, row.BranchSlug)]; found {
|
||||
continue
|
||||
}
|
||||
rows = append(rows, row)
|
||||
}
|
||||
return rows
|
||||
}
|
||||
|
||||
func workspaceSourceInventoryRow(
|
||||
environmentID, executorType string,
|
||||
taskRepository *models.TaskRepository,
|
||||
materialized map[string]*branchMaterialization,
|
||||
) (*models.TaskEnvironmentRepo, bool) {
|
||||
if taskRepository == nil || taskRepository.RepositoryID == "" {
|
||||
return nil, false
|
||||
}
|
||||
branchSlug := workspaceSourceBranchSlug(taskRepository)
|
||||
branch := materialized[taskRepository.RepositoryID]
|
||||
if executorType == string(models.ExecutorTypeWorktree) && branch == nil {
|
||||
// A worktree source that was not materialized has no physical checkout
|
||||
// to publish. The next launch must prepare it first.
|
||||
return nil, false
|
||||
}
|
||||
row := &models.TaskEnvironmentRepo{
|
||||
TaskEnvironmentID: environmentID,
|
||||
RepositoryID: taskRepository.RepositoryID,
|
||||
BranchSlug: branchSlug,
|
||||
Position: taskRepository.Position,
|
||||
}
|
||||
if branch != nil && branch.worktree != nil {
|
||||
row.WorktreeID = branch.worktree.ID
|
||||
row.WorktreePath = branch.worktree.Path
|
||||
row.WorktreeBranch = branch.worktree.Branch
|
||||
row.BranchSlug = branch.slug
|
||||
}
|
||||
return row, true
|
||||
}
|
||||
|
||||
func workspaceSourceBranchSlug(taskRepository *models.TaskRepository) string {
|
||||
branch := taskRepository.CheckoutBranch
|
||||
if branch == "" {
|
||||
branch = taskRepository.BaseBranch
|
||||
}
|
||||
return worktree.SanitizeBranchSlug(branch)
|
||||
}
|
||||
|
||||
func environmentRepoInventoryKey(repositoryID, branchSlug string) string {
|
||||
return repositoryID + "\x00" + branchSlug
|
||||
}
|
||||
|
||||
// buildRemoteWorkspaceRepositoryBatch projects only the sources created by
|
||||
// this attachment operation. Existing durable siblings belong in a fresh
|
||||
// launch/resume projection, but revalidating them in a live execution would
|
||||
|
||||
@@ -174,6 +174,20 @@ func TestWorkspaceSourceMaterializer_RemoteMaterializesAdditionalRepositories(t
|
||||
if len(remote.calls) != 1 || len(remote.calls[0]) != 1 || remote.calls[0][0].Destination != "added-main" {
|
||||
t.Fatalf("remote projection=%+v; want only additional repository", remote.calls)
|
||||
}
|
||||
inventory, err := repo.ListTaskEnvironmentRepos(ctx, env.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("ListTaskEnvironmentRepos: %v", err)
|
||||
}
|
||||
found := false
|
||||
for _, row := range inventory {
|
||||
if row.RepositoryID == "repo-added" && row.BranchSlug == "main" {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatalf("environment inventory = %+v; want repo-added/main", inventory)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -221,6 +221,12 @@ func seedBranchSwitchSession(t *testing.T, startBranch string) (*Service, *mockG
|
||||
if err := testRepo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
|
||||
ID: "env-s1", TaskID: "t1", ExecutorType: "worktree",
|
||||
WorkspacePath: "/tmp", Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{
|
||||
ID: "wt-s1", WorktreeID: "wtree-s1", RepositoryID: "repo1",
|
||||
WorktreeBranch: startBranch, CreatedAt: now,
|
||||
},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("create environment: %v", err)
|
||||
}
|
||||
@@ -233,14 +239,6 @@ func seedBranchSwitchSession(t *testing.T, startBranch string) (*Service, *mockG
|
||||
if err := testRepo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("link session to environment: %v", err)
|
||||
}
|
||||
wt := &models.TaskEnvironmentRepo{
|
||||
ID: "wt-s1", TaskEnvironmentID: "env-s1",
|
||||
WorktreeID: "wtree-s1", RepositoryID: "repo1",
|
||||
WorktreeBranch: startBranch, CreatedAt: now,
|
||||
}
|
||||
if err := testRepo.CreateTaskEnvironmentRepo(ctx, wt); err != nil {
|
||||
t.Fatalf("create worktree: %v", err)
|
||||
}
|
||||
|
||||
svc := createTestService(testRepo, newMockStepGetter(), newMockTaskRepo())
|
||||
ghSvc := &mockGitHubService{}
|
||||
|
||||
@@ -559,6 +559,15 @@ func TestCheckSessionPR(t *testing.T) {
|
||||
if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
|
||||
ID: "env1", TaskID: "t1", ExecutorType: "worktree",
|
||||
WorkspacePath: "/tmp", Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{
|
||||
ID: "wt1",
|
||||
WorktreeID: "wtree1",
|
||||
RepositoryID: "repo1",
|
||||
WorktreeBranch: branch,
|
||||
CreatedAt: now,
|
||||
},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("failed to create environment: %v", err)
|
||||
}
|
||||
@@ -570,17 +579,6 @@ func TestCheckSessionPR(t *testing.T) {
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("link session to environment: %v", err)
|
||||
}
|
||||
wt := &models.TaskEnvironmentRepo{
|
||||
ID: "wt1",
|
||||
TaskEnvironmentID: "env1",
|
||||
WorktreeID: "wtree1",
|
||||
RepositoryID: "repo1",
|
||||
WorktreeBranch: branch,
|
||||
CreatedAt: now,
|
||||
}
|
||||
if err := repo.CreateTaskEnvironmentRepo(ctx, wt); err != nil {
|
||||
t.Fatalf("failed to create worktree: %v", err)
|
||||
}
|
||||
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
return svc
|
||||
@@ -1205,6 +1203,10 @@ func TestListTasksNeedingPRWatch(t *testing.T) {
|
||||
if err := testRepo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
|
||||
ID: "env-s1", TaskID: "t1", ExecutorType: "worktree",
|
||||
WorkspacePath: "/tmp", Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{ID: "wt-1", WorktreeID: "wtree-1", RepositoryID: "repo-front", WorktreeBranch: "feat/frontend", CreatedAt: now},
|
||||
{ID: "wt-2", WorktreeID: "wtree-2", RepositoryID: "repo-back", WorktreeBranch: "feat/backend", CreatedAt: now},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("create environment: %v", err)
|
||||
}
|
||||
@@ -1216,15 +1218,6 @@ func TestListTasksNeedingPRWatch(t *testing.T) {
|
||||
if err := testRepo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("link session to environment: %v", err)
|
||||
}
|
||||
worktrees := []*models.TaskEnvironmentRepo{
|
||||
{ID: "wt-1", TaskEnvironmentID: "env-s1", WorktreeID: "wtree-1", RepositoryID: "repo-front", WorktreeBranch: "feat/frontend", CreatedAt: now},
|
||||
{ID: "wt-2", TaskEnvironmentID: "env-s1", WorktreeID: "wtree-2", RepositoryID: "repo-back", WorktreeBranch: "feat/backend", CreatedAt: now},
|
||||
}
|
||||
for _, wt := range worktrees {
|
||||
if err := testRepo.CreateTaskEnvironmentRepo(ctx, wt); err != nil {
|
||||
t.Fatalf("worktree %s: %v", wt.ID, err)
|
||||
}
|
||||
}
|
||||
|
||||
svc := createTestService(testRepo, newMockStepGetter(), newMockTaskRepo())
|
||||
// Mock the watch list as if frontend already has a watch (the
|
||||
@@ -1300,6 +1293,10 @@ func TestEnsureSessionPRWatch_MultiRepo(t *testing.T) {
|
||||
if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
|
||||
ID: "env-s1", TaskID: "t1", ExecutorType: "worktree",
|
||||
WorkspacePath: "/tmp", Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{ID: "wt-1", WorktreeID: "wtree-1", RepositoryID: "repo-front", WorktreeBranch: "feat/frontend", CreatedAt: now},
|
||||
{ID: "wt-2", WorktreeID: "wtree-2", RepositoryID: "repo-back", WorktreeBranch: "feat/backend", CreatedAt: now},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("create environment: %v", err)
|
||||
}
|
||||
@@ -1311,14 +1308,6 @@ func TestEnsureSessionPRWatch_MultiRepo(t *testing.T) {
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("link session to environment: %v", err)
|
||||
}
|
||||
for _, wt := range []*models.TaskEnvironmentRepo{
|
||||
{ID: "wt-1", TaskEnvironmentID: "env-s1", WorktreeID: "wtree-1", RepositoryID: "repo-front", WorktreeBranch: "feat/frontend", CreatedAt: now},
|
||||
{ID: "wt-2", TaskEnvironmentID: "env-s1", WorktreeID: "wtree-2", RepositoryID: "repo-back", WorktreeBranch: "feat/backend", CreatedAt: now},
|
||||
} {
|
||||
if err := repo.CreateTaskEnvironmentRepo(ctx, wt); err != nil {
|
||||
t.Fatalf("worktree: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
ghSvc := &mockGitHubService{}
|
||||
|
||||
@@ -40,9 +40,25 @@ func (e *Executor) validateReuseEnvironmentInventory(ctx context.Context, req *L
|
||||
// Reuse setup below consumes this same inventory, avoiding a second query
|
||||
// and keeping cancellation attached to the caller's context.
|
||||
env.Repos = rows
|
||||
// Zero recorded rows means the canonical inventory was never captured at
|
||||
// all (for example: a launch whose prepare step failed before writing
|
||||
// any repo rows). That is recoverable — letting this launch through lets
|
||||
// reuseExistingRepositoryWorktrees fall through its own empty-inventory
|
||||
// check and rebuild fresh worktree/repo specs. A non-empty but wrong
|
||||
// inventory below is the guard's actual purpose and must still refuse.
|
||||
if len(rows) == 0 {
|
||||
req.WorkspaceReuseRequired = workspaceReuseAllowed(
|
||||
env,
|
||||
req.ExecutorType,
|
||||
req.WorkspaceReuseRequired,
|
||||
e.taskIsRepoBacked(ctx, req.TaskID),
|
||||
)
|
||||
return nil
|
||||
}
|
||||
for _, spec := range specs {
|
||||
if canonicalInventoryMatches(spec, rows, req.UseWorktree) != 1 {
|
||||
return fmt.Errorf("%w: canonical workspace repository inventory is incomplete", models.ErrWorkspaceReuseUnsafe)
|
||||
return fmt.Errorf("%w: canonical workspace repository inventory has no matching entry for repository %q branch %q",
|
||||
models.ErrWorkspaceReuseUnsafe, spec.RepositoryID, launchRepoBranchIdentitySlug(spec))
|
||||
}
|
||||
}
|
||||
return nil
|
||||
@@ -97,14 +113,24 @@ func (e *Executor) reuseExistingEnvironment(ctx context.Context, req *LaunchAgen
|
||||
zap.String("req_executor_type", req.ExecutorType))
|
||||
return
|
||||
}
|
||||
// A repo-backed environment with no canonical rows is being freshly
|
||||
// materialized. Do not forward any environment or sibling runtime handle:
|
||||
// those handles could reconnect to the incomplete workspace, and
|
||||
// PreviousExecutionID would route the new session through the resume path.
|
||||
if !req.WorkspaceReuseRequired && e.taskIsRepoBacked(ctx, req.TaskID) && len(env.Repos) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
if env.TaskDirName != "" && req.UseWorktree {
|
||||
req.TaskDirName = env.TaskDirName
|
||||
}
|
||||
// SSH uses the remote task directory as an environment-scoped attachment
|
||||
// handle. It is distinct from the per-session agentctl directory and must
|
||||
// therefore be forwarded for a sibling session without adopting its runtime.
|
||||
if req.ExecutorType == string(models.ExecutorTypeSSH) && env.WorkspacePath != "" {
|
||||
// therefore be forwarded for a sibling session without adopting its
|
||||
// runtime. Only forward it when reuse is actually required: a launch that
|
||||
// fell through to full materialization (see workspaceReuseAllowed) must
|
||||
// not see the old, possibly incomplete or stale, workspace path.
|
||||
if req.ExecutorType == string(models.ExecutorTypeSSH) && env.WorkspacePath != "" && req.WorkspaceReuseRequired {
|
||||
ensureLaunchMetadata(req)[lifecycle.MetadataKeySSHRemoteTaskDir] = env.WorkspacePath
|
||||
}
|
||||
|
||||
|
||||
+59
@@ -0,0 +1,59 @@
|
||||
package executor
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
// Regression coverage for the "failed prepare leaves a permanently bricked
|
||||
// workspace" defect: when a task_environments row was published ready with
|
||||
// zero task_environment_repos rows (the write-side bug, fixed separately),
|
||||
// validateReuseEnvironmentInventory used to treat that as a mismatch and
|
||||
// refuse every subsequent launch forever. Zero rows recorded means the
|
||||
// canonical inventory was never captured at all, which is recoverable by
|
||||
// letting the launch rebuild it — distinct from a non-empty but wrong
|
||||
// inventory, which must still be refused.
|
||||
func TestValidateReuseEnvironmentInventory_ZeroRowsIsRecoverable(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1"}
|
||||
e := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
req := &LaunchAgentRequest{
|
||||
TaskID: "task-1",
|
||||
WorkspaceReuseRequired: true,
|
||||
RepositoryID: "repo-1",
|
||||
}
|
||||
env := &models.TaskEnvironment{ID: "env-1"}
|
||||
|
||||
if err := e.validateReuseEnvironmentInventory(context.Background(), req, env); err != nil {
|
||||
t.Fatalf("validateReuseEnvironmentInventory() with zero recorded rows = %v, want nil (recoverable)", err)
|
||||
}
|
||||
if req.WorkspaceReuseRequired {
|
||||
t.Fatal("zero inventory kept WorkspaceReuseRequired=true, want fresh materialization")
|
||||
}
|
||||
}
|
||||
|
||||
// The read-side fix must not weaken the guard's actual purpose: a non-empty
|
||||
// but mismatched canonical inventory (wrong repository, wrong branch, or a
|
||||
// row explicitly marked failed/deleted) is still an unsafe reuse and must be
|
||||
// refused.
|
||||
func TestValidateReuseEnvironmentInventory_MismatchedRowsStillRefused(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
e := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
req := &LaunchAgentRequest{
|
||||
TaskID: "task-1",
|
||||
WorkspaceReuseRequired: true,
|
||||
RepositoryID: "repo-1",
|
||||
}
|
||||
env := &models.TaskEnvironment{ID: "env-1"}
|
||||
repo.taskEnvironmentRepos[env.ID] = []*models.TaskEnvironmentRepo{
|
||||
{RepositoryID: "repo-other", WorktreeID: "worktree-other"},
|
||||
}
|
||||
|
||||
err := e.validateReuseEnvironmentInventory(context.Background(), req, env)
|
||||
if !errors.Is(err, models.ErrWorkspaceReuseUnsafe) {
|
||||
t.Fatalf("validateReuseEnvironmentInventory() with mismatched rows = %v, want ErrWorkspaceReuseUnsafe", err)
|
||||
}
|
||||
}
|
||||
@@ -140,23 +140,42 @@ func TestReuseExistingEnvironment_WorktreeSkippedWhenNotRequested(t *testing.T)
|
||||
}
|
||||
|
||||
func TestWorkspaceReuseAllowedRequiresMatchingExecutorType(t *testing.T) {
|
||||
env := &models.TaskEnvironment{ExecutorType: string(models.ExecutorTypeLocal)}
|
||||
if workspaceReuseAllowed(env, string(models.ExecutorTypeWorktree), true) {
|
||||
env := &models.TaskEnvironment{
|
||||
ExecutorType: string(models.ExecutorTypeLocal),
|
||||
Repos: []*models.TaskEnvironmentRepo{{RepositoryID: "repo-1"}},
|
||||
}
|
||||
if workspaceReuseAllowed(env, string(models.ExecutorTypeWorktree), true, true) {
|
||||
t.Fatal("workspace reuse should be disabled when the executor type changes")
|
||||
}
|
||||
if !workspaceReuseAllowed(env, string(models.ExecutorTypeLocal), true) {
|
||||
if !workspaceReuseAllowed(env, string(models.ExecutorTypeLocal), true, true) {
|
||||
t.Fatal("workspace reuse should remain enabled for the owning executor type")
|
||||
}
|
||||
if workspaceReuseAllowed(&models.TaskEnvironment{}, string(models.ExecutorTypeWorktree), true) {
|
||||
if workspaceReuseAllowed(&models.TaskEnvironment{}, string(models.ExecutorTypeWorktree), true, true) {
|
||||
t.Fatal("legacy environments without a physical worktree should not be reused by worktree launches")
|
||||
}
|
||||
if !workspaceReuseAllowed(&models.TaskEnvironment{
|
||||
Repos: []*models.TaskEnvironmentRepo{{WorktreeID: "legacy-worktree", Status: "active"}},
|
||||
}, string(models.ExecutorTypeWorktree), true) {
|
||||
}, string(models.ExecutorTypeWorktree), true, true) {
|
||||
t.Fatal("legacy environments with a physical worktree should remain reusable")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkspaceReuseAllowedRequiresInventoryForRepoBackedExecutors(t *testing.T) {
|
||||
for _, executorType := range []string{
|
||||
string(models.ExecutorTypeLocal),
|
||||
string(models.ExecutorTypeLocalDocker),
|
||||
string(models.ExecutorTypeSSH),
|
||||
string(models.ExecutorTypeWorktree),
|
||||
} {
|
||||
t.Run(executorType, func(t *testing.T) {
|
||||
env := &models.TaskEnvironment{ExecutorType: executorType}
|
||||
if workspaceReuseAllowed(env, executorType, true, true) {
|
||||
t.Fatalf("workspace reuse allowed for %s with empty repository inventory", executorType)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForTaskEnvironmentReadyWaitsForMaterializer(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
env := &models.TaskEnvironment{ID: "env-wait", Status: models.TaskEnvironmentStatusCreating}
|
||||
@@ -348,6 +367,156 @@ func TestPersistTaskEnvironment_FinalizesCreatingEnvironmentWithInventory(t *tes
|
||||
}
|
||||
}
|
||||
|
||||
func TestPersistTaskEnvironment_RestoresMaterializationClaimOnFinalizeFailure(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
persistErr := errors.New("finalize failed")
|
||||
repo.finalizeTaskEnvironmentErr = persistErr
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-1",
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeWorktree),
|
||||
Status: models.TaskEnvironmentStatusCreating,
|
||||
MaterializationSessionID: "session-1",
|
||||
}
|
||||
repo.taskEnvironments[env.ID] = env
|
||||
e := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
session := &models.TaskSession{ID: "session-1", TaskID: "task-1", TaskEnvironmentID: env.ID}
|
||||
|
||||
err := e.persistTaskEnvironment(context.Background(), "task-1", session, env,
|
||||
&LaunchAgentRequest{TaskID: "task-1", ExecutorType: string(models.ExecutorTypeWorktree)},
|
||||
&LaunchAgentResponse{WorktreePath: "/tasks/task-1/repo", Worktrees: []RepoWorktreeResult{{
|
||||
RepositoryID: "repo-1", WorktreeID: "wt-1", WorktreePath: "/tasks/task-1/repo", WorktreeBranch: "feature/task-1",
|
||||
}}},
|
||||
executorConfig{ExecutorID: models.ExecutorIDWorktree})
|
||||
|
||||
if !errors.Is(err, persistErr) {
|
||||
t.Fatalf("persistTaskEnvironment error = %v, want %v", err, persistErr)
|
||||
}
|
||||
if env.Status != models.TaskEnvironmentStatusCreating || env.MaterializationSessionID != session.ID {
|
||||
t.Fatalf("materialization claim = status %q owner %q, want creating with original owner", env.Status, env.MaterializationSessionID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPersistTaskEnvironment_NonMaterializerSiblingPersistsReposBeforeReady
|
||||
// pins the fix for the "ready status precedes inventory commit" hazard: a
|
||||
// non-initial-materializer sibling must write its per-repo rows before
|
||||
// flipping the environment to ready, so a crash or concurrent read between
|
||||
// the two writes can never observe a ready environment whose canonical
|
||||
// inventory is still empty or stale.
|
||||
func TestPersistTaskEnvironment_NonMaterializerSiblingPersistsReposBeforeReady(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-1",
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeWorktree),
|
||||
Status: models.TaskEnvironmentStatusReady,
|
||||
}
|
||||
repo.taskEnvironments[env.ID] = env
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1"}
|
||||
e := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
session := &models.TaskSession{ID: "session-2", TaskID: "task-1", TaskEnvironmentID: env.ID}
|
||||
|
||||
if err := e.persistTaskEnvironment(context.Background(), "task-1", session, env,
|
||||
&LaunchAgentRequest{TaskID: "task-1", ExecutorType: string(models.ExecutorTypeWorktree)},
|
||||
&LaunchAgentResponse{WorktreePath: "/tasks/task-1/repo", Worktrees: []RepoWorktreeResult{{
|
||||
RepositoryID: "repo-1", WorktreeID: "wt-2", WorktreePath: "/tasks/task-1/repo", WorktreeBranch: "feature/task-1",
|
||||
}}},
|
||||
executorConfig{ExecutorID: models.ExecutorIDWorktree}); err != nil {
|
||||
t.Fatalf("persistTaskEnvironment: %v", err)
|
||||
}
|
||||
|
||||
if len(repo.writeCallLog) < 2 {
|
||||
t.Fatalf("write call log = %v, want at least a repo write followed by an env update", repo.writeCallLog)
|
||||
}
|
||||
repoIdx, envIdx := -1, -1
|
||||
for i, call := range repo.writeCallLog {
|
||||
switch call {
|
||||
case "create_repo":
|
||||
if repoIdx == -1 {
|
||||
repoIdx = i
|
||||
}
|
||||
case "update_env":
|
||||
if envIdx == -1 {
|
||||
envIdx = i
|
||||
}
|
||||
}
|
||||
}
|
||||
if repoIdx == -1 || envIdx == -1 || repoIdx > envIdx {
|
||||
t.Fatalf("write order = %v, want create_repo before update_env", repo.writeCallLog)
|
||||
}
|
||||
if got := repo.taskEnvironments[env.ID].Status; got != models.TaskEnvironmentStatusReady {
|
||||
t.Fatalf("status = %q, want ready once inventory is persisted", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPersistTaskEnvironment_LeavesEnvironmentUnchangedOnRepoWriteFailure(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
persistErr := errors.New("inventory write failed")
|
||||
repo.createTaskEnvironmentRepoErr = persistErr
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-1",
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeWorktree),
|
||||
Status: models.TaskEnvironmentStatusReady,
|
||||
}
|
||||
repo.taskEnvironments[env.ID] = env
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1"}
|
||||
e := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
session := &models.TaskSession{ID: "session-2", TaskID: "task-1", TaskEnvironmentID: env.ID}
|
||||
|
||||
err := e.persistTaskEnvironment(context.Background(), "task-1", session, env,
|
||||
&LaunchAgentRequest{TaskID: "task-1", ExecutorType: string(models.ExecutorTypeWorktree)},
|
||||
&LaunchAgentResponse{WorktreePath: "/tasks/task-1/repo", Worktrees: []RepoWorktreeResult{{
|
||||
RepositoryID: "repo-1", WorktreeID: "wt-2", WorktreePath: "/tasks/task-1/repo", WorktreeBranch: "feature/task-1",
|
||||
}}},
|
||||
executorConfig{ExecutorID: models.ExecutorIDWorktree})
|
||||
|
||||
if !errors.Is(err, persistErr) {
|
||||
t.Fatalf("persistTaskEnvironment error = %v, want %v", err, persistErr)
|
||||
}
|
||||
if session.TaskEnvironmentID != env.ID {
|
||||
t.Fatalf("session TaskEnvironmentID = %q, want unchanged %q", session.TaskEnvironmentID, env.ID)
|
||||
}
|
||||
if len(repo.updateTaskEnvironmentCalls) != 0 {
|
||||
t.Fatalf("UpdateTaskEnvironment calls = %d, want none after inventory failure", len(repo.updateTaskEnvironmentCalls))
|
||||
}
|
||||
}
|
||||
|
||||
// TestPersistTaskEnvironment_NonMaterializerSiblingWithEmptyReposDoesNotSetReady
|
||||
// covers the companion invariant: when this sibling's own launch produced no
|
||||
// repos and the environment had none recorded either, a repo-backed task's
|
||||
// environment must not be forced to ready — that would publish an empty
|
||||
// canonical inventory that permanently bricks reuse.
|
||||
func TestPersistTaskEnvironment_NonMaterializerSiblingWithEmptyReposDoesNotSetReady(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-1",
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
Status: models.TaskEnvironmentStatusCreating,
|
||||
}
|
||||
repo.taskEnvironments[env.ID] = env
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1"}
|
||||
e := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
session := &models.TaskSession{ID: "session-2", TaskID: "task-1", TaskEnvironmentID: env.ID}
|
||||
|
||||
if err := e.persistTaskEnvironment(context.Background(), "task-1", session, env,
|
||||
&LaunchAgentRequest{TaskID: "task-1", ExecutorType: string(models.ExecutorTypeSSH)},
|
||||
&LaunchAgentResponse{},
|
||||
executorConfig{ExecutorID: "ssh"}); err != nil {
|
||||
t.Fatalf("persistTaskEnvironment: %v", err)
|
||||
}
|
||||
|
||||
for _, call := range repo.writeCallLog {
|
||||
if call == "create_repo" {
|
||||
t.Fatalf("write call log = %v, want no repo rows created for an empty prepare result", repo.writeCallLog)
|
||||
}
|
||||
}
|
||||
if got := repo.taskEnvironments[env.ID].Status; got != models.TaskEnvironmentStatusCreating {
|
||||
t.Fatalf("status = %q, want status left untouched instead of forced to ready with empty inventory", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReuseExistingEnvironment_DockerBranchReuse(t *testing.T) {
|
||||
e := newEnvTestExecutor(t)
|
||||
req := &LaunchAgentRequest{TaskID: "task-1", ExecutorType: "local_docker"}
|
||||
@@ -671,6 +840,87 @@ func TestReuseExistingEnvironment_ReuseRequiredSSHUsesCanonicalRemoteTaskDir(t *
|
||||
}
|
||||
}
|
||||
|
||||
// TestReuseExistingEnvironment_ReuseNotRequiredSSHSkipsStaleRemoteTaskDir
|
||||
// pins the fix for the empty-inventory SSH reuse hazard: when
|
||||
// workspaceReuseAllowed has already forced a fresh materialization (reuse
|
||||
// not required), reuseExistingEnvironment must not forward the old
|
||||
// env.WorkspacePath, or the SSH executor's attach-only path would trust a
|
||||
// possibly incomplete or stale checkout instead of preparing a fresh one.
|
||||
func TestReuseExistingEnvironment_ReuseNotRequiredSSHSkipsStaleRemoteTaskDir(t *testing.T) {
|
||||
e := newTestExecutor(t, &mockAgentManager{}, newMockRepository())
|
||||
req := &LaunchAgentRequest{
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
WorkspaceReuseRequired: false,
|
||||
}
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "environment-1",
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
WorkspacePath: "/home/kandev/.kandev/tasks/task-1",
|
||||
}
|
||||
|
||||
e.reuseExistingEnvironment(context.Background(), req, env)
|
||||
|
||||
if got, ok := req.Metadata[lifecycle.MetadataKeySSHRemoteTaskDir]; ok {
|
||||
t.Fatalf("remote task directory = %v, want unset for a non-reuse launch", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReuseExistingEnvironment_FreshRepoRecoveryDoesNotAdoptSiblingExecution(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1",
|
||||
}
|
||||
repo.sessions["session-old"] = &models.TaskSession{
|
||||
ID: "session-old", TaskID: "task-1", TaskEnvironmentID: "env-1",
|
||||
}
|
||||
repo.executorsRunning["session-old"] = &models.ExecutorRunning{
|
||||
SessionID: "session-old",
|
||||
AgentExecutionID: "exec-old",
|
||||
}
|
||||
e := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
req := &LaunchAgentRequest{
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
WorkspaceReuseRequired: false,
|
||||
}
|
||||
env := &models.TaskEnvironment{ID: "env-1", TaskID: "task-1", ExecutorType: string(models.ExecutorTypeSSH)}
|
||||
|
||||
e.reuseExistingEnvironment(context.Background(), req, env)
|
||||
|
||||
if req.PreviousExecutionID != "" {
|
||||
t.Fatalf("PreviousExecutionID = %q, want empty during fresh repo recovery", req.PreviousExecutionID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReuseExistingEnvironment_FreshRepoRecoveryDropsContainerHandle(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1",
|
||||
}
|
||||
e := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
req := &LaunchAgentRequest{
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeLocalDocker),
|
||||
WorkspaceReuseRequired: false,
|
||||
}
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-1",
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeLocalDocker),
|
||||
ContainerID: "container-stale",
|
||||
}
|
||||
|
||||
e.reuseExistingEnvironment(context.Background(), req, env)
|
||||
|
||||
if req.PreviousExecutionID != "" {
|
||||
t.Fatalf("PreviousExecutionID = %q, want empty during fresh repo recovery", req.PreviousExecutionID)
|
||||
}
|
||||
if req.Metadata != nil {
|
||||
t.Fatalf("metadata = %#v, want no stale container handle during fresh recovery", req.Metadata)
|
||||
}
|
||||
}
|
||||
|
||||
// TestApplyExecutorRunningMetadata_SkipsSessionScopedKeys pins the guard
|
||||
// that prevents a SECOND session on the same task from inheriting the FIRST
|
||||
// session's session-scoped runtime resources — agentctl PID/port, remote
|
||||
|
||||
@@ -351,6 +351,23 @@ func (e *Executor) cleanupUnstartedExecutionAfterPersistError(
|
||||
e.stopUnstartedExecution(ctx, sessionID, agentExecutionID)
|
||||
}
|
||||
|
||||
func (e *Executor) markTaskEnvironmentMaterializationFailed(
|
||||
ctx context.Context,
|
||||
env *models.TaskEnvironment,
|
||||
sessionID string,
|
||||
) {
|
||||
if env == nil || env.Status != models.TaskEnvironmentStatusCreating || env.MaterializationSessionID != sessionID {
|
||||
return
|
||||
}
|
||||
env.Status = models.TaskEnvironmentStatusFailed
|
||||
env.MaterializationSessionID = ""
|
||||
if err := e.repo.UpdateTaskEnvironment(ctx, env); err != nil {
|
||||
e.logger.Warn("failed to mark workspace materialization failed",
|
||||
zap.String("task_environment_id", env.ID),
|
||||
zap.Error(err))
|
||||
}
|
||||
}
|
||||
|
||||
func (e *Executor) writeTaskReviewStateIfNoWorkingSessions(ctx context.Context, taskID, failedSessionID string) {
|
||||
if e.onTaskReviewStateReconcile != nil {
|
||||
e.onTaskReviewStateReconcile(ctx, taskID, failedSessionID)
|
||||
@@ -1288,7 +1305,12 @@ func (e *Executor) LaunchPreparedSession(ctx context.Context, task *v1.Task, ses
|
||||
}
|
||||
|
||||
// Create or update the task environment with launch results
|
||||
e.persistTaskEnvironment(ctx, task.ID, session, existingEnv, req, resp, execCfg)
|
||||
if err := e.persistTaskEnvironment(ctx, task.ID, session, existingEnv, req, resp, execCfg); err != nil {
|
||||
e.cleanupUnstartedExecutionAfterPersistError(ctx, sessionID, resp.AgentExecutionID, err)
|
||||
e.markTaskEnvironmentMaterializationFailed(ctx, existingEnv, session.ID)
|
||||
repositoryID, taskRepositoryID := failingLaunchRepositoryIdentity(req, err)
|
||||
return nil, e.handleLaunchFailure(ctx, task.ID, sessionID, repositoryID, taskRepositoryID, err)
|
||||
}
|
||||
|
||||
// Capture the current HEAD commit as the base commit for this session asynchronously.
|
||||
// This allows us to filter git log to only show commits made during the session.
|
||||
@@ -1629,7 +1651,8 @@ func (e *Executor) buildLaunchAgentRequest(ctx context.Context, task *v1.Task, s
|
||||
// the new backend instead of attaching to the old environment. Resolve this
|
||||
// before repository configuration so clone URL requirements are applied to
|
||||
// the fresh launch as well.
|
||||
workspaceReuseRequired = workspaceReuseAllowed(existingEnv, req.ExecutorType, workspaceReuseRequired)
|
||||
repoBacked := e.taskIsRepoBacked(ctx, task.ID)
|
||||
workspaceReuseRequired = workspaceReuseAllowed(existingEnv, req.ExecutorType, workspaceReuseRequired, repoBacked)
|
||||
req.WorkspaceReuseRequired = workspaceReuseRequired
|
||||
|
||||
// For remote executors (containerized *and* SSH), resolve only explicitly
|
||||
@@ -1688,13 +1711,16 @@ func (e *Executor) buildLaunchAgentRequest(ctx context.Context, task *v1.Task, s
|
||||
return req, execConfig, nil
|
||||
}
|
||||
|
||||
func workspaceReuseAllowed(existingEnv *models.TaskEnvironment, requestedExecutorType string, required bool) bool {
|
||||
func workspaceReuseAllowed(existingEnv *models.TaskEnvironment, requestedExecutorType string, required, repoBacked bool) bool {
|
||||
if !required || existingEnv == nil {
|
||||
return required
|
||||
}
|
||||
if existingEnv.ExecutorType != "" && existingEnv.ExecutorType != requestedExecutorType {
|
||||
return false
|
||||
}
|
||||
if repoBacked && len(existingEnv.Repos) == 0 {
|
||||
return false
|
||||
}
|
||||
if requestedExecutorType == string(models.ExecutorTypeWorktree) {
|
||||
return hasLiveWorktreeRepo(existingEnv)
|
||||
}
|
||||
@@ -2188,7 +2214,7 @@ func (e *Executor) persistTaskEnvironment(
|
||||
req *LaunchAgentRequest,
|
||||
resp *LaunchAgentResponse,
|
||||
execCfg executorConfig,
|
||||
) {
|
||||
) error {
|
||||
mu := e.taskEnvLock(taskID)
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
@@ -2198,7 +2224,11 @@ func (e *Executor) persistTaskEnvironment(
|
||||
// see existingEnv == nil from the original call and try to create a
|
||||
// duplicate.
|
||||
if existingEnv == nil {
|
||||
if fresh, err := e.repo.GetTaskEnvironmentByTaskID(ctx, taskID); err == nil && fresh != nil {
|
||||
fresh, err := e.repo.GetTaskEnvironmentByTaskID(ctx, taskID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("load task environment before persistence: %w", err)
|
||||
}
|
||||
if fresh != nil {
|
||||
existingEnv = fresh
|
||||
}
|
||||
}
|
||||
@@ -2208,12 +2238,19 @@ func (e *Executor) persistTaskEnvironment(
|
||||
if existingEnv != nil {
|
||||
materializationSessionID := existingEnv.MaterializationSessionID
|
||||
isInitialMaterializer := existingEnv.Status == models.TaskEnvironmentStatusCreating && materializationSessionID == session.ID
|
||||
previousStatus := existingEnv.Status
|
||||
previousMaterializationSessionID := existingEnv.MaterializationSessionID
|
||||
repos := environmentReposForLaunch(req, resp)
|
||||
// agent_execution_id is no longer stored on task_environments — the column
|
||||
// is being dropped (executors_running is the single source of truth).
|
||||
// Status, workspace, and container fields are still env-row-owned; the
|
||||
// physical worktree lives on task_environment_repos.
|
||||
existingEnv.Status = models.TaskEnvironmentStatusReady
|
||||
existingEnv.MaterializationSessionID = ""
|
||||
// physical worktree lives on task_environment_repos. Status is decided
|
||||
// per branch below rather than forced here: the initial materializer
|
||||
// always transitions to ready (its inventory publishes atomically in
|
||||
// the same call), but a non-materializing sibling must not force ready
|
||||
// onto an environment whose canonical inventory turned out empty for a
|
||||
// repo-backed task — that permanently bricks reuse (see
|
||||
// validateReuseEnvironmentInventory).
|
||||
// Refresh workspace + container/sandbox fields. The original update
|
||||
// branch only touched AgentExecutionID/Status, so envs created with
|
||||
// empty paths (e.g. before the worktree resolved) stayed permanently
|
||||
@@ -2244,32 +2281,71 @@ func (e *Executor) persistTaskEnvironment(
|
||||
// The initial materializer must publish its full repository inventory
|
||||
// in the same transaction as the ready transition. Otherwise a sibling
|
||||
// can bind to a ready environment whose canonical rows are incomplete.
|
||||
existingEnv.Status = models.TaskEnvironmentStatusReady
|
||||
existingEnv.MaterializationSessionID = ""
|
||||
if finalizer, ok := e.repo.(taskEnvironmentMaterializationFinalizer); ok {
|
||||
if err := finalizer.FinalizeTaskEnvironmentMaterialization(ctx, existingEnv, environmentReposForLaunch(req, resp), materializationSessionID); err != nil {
|
||||
existingEnv.Status = models.TaskEnvironmentStatusReady
|
||||
existingEnv.MaterializationSessionID = ""
|
||||
if err := finalizer.FinalizeTaskEnvironmentMaterialization(ctx, existingEnv, repos, materializationSessionID); err != nil {
|
||||
existingEnv.Status = previousStatus
|
||||
existingEnv.MaterializationSessionID = previousMaterializationSessionID
|
||||
e.logger.Warn("failed to finalize task environment materialization",
|
||||
zap.String("task_id", taskID), zap.String("env_id", existingEnv.ID), zap.Error(err))
|
||||
return
|
||||
return fmt.Errorf("finalize task environment materialization: %w", err)
|
||||
}
|
||||
session.TaskEnvironmentID = existingEnv.ID
|
||||
e.selfHealTaskRepositoryBaseBranches(ctx, taskID, req, resp)
|
||||
return
|
||||
return nil
|
||||
}
|
||||
}
|
||||
// Persist per-repo rows for launches that didn't have them yet *before*
|
||||
// the environment can be marked ready below. The environment-repository
|
||||
// rows are the only physical-worktree record, so single-repo launches
|
||||
// write one row here too. Ordering matters: writing status=ready first
|
||||
// and these rows second leaves a window where a crash or a concurrent
|
||||
// reader observes a ready environment with an empty inventory for a
|
||||
// repo-backed task — exactly the state the guards elsewhere in this PR
|
||||
// exist to prevent (see validateReuseEnvironmentInventory).
|
||||
if err := e.persistTaskEnvironmentRepos(ctx, existingEnv.ID, repos); err != nil {
|
||||
existingEnv.Status = previousStatus
|
||||
existingEnv.MaterializationSessionID = previousMaterializationSessionID
|
||||
return err
|
||||
}
|
||||
repoBacked := e.taskIsRepoBacked(ctx, taskID)
|
||||
// A sibling can finish while the initial materializer still owns a
|
||||
// CREATING environment. In that case the owner remains responsible for
|
||||
// publishing (or failing) the inventory, so leave the claim untouched
|
||||
// instead of turning the sibling's empty result into a launch error. Once
|
||||
// the claim is no longer CREATING, an empty repo-backed inventory is a
|
||||
// persistence failure and must fail the launch rather than publish an
|
||||
// unusable READY/STOPPED environment.
|
||||
if len(repos) == 0 && len(existingEnv.Repos) == 0 && repoBacked && previousStatus != models.TaskEnvironmentStatusCreating {
|
||||
existingEnv.Status = previousStatus
|
||||
existingEnv.MaterializationSessionID = previousMaterializationSessionID
|
||||
return fmt.Errorf("persist task environment: ready status requires repository inventory")
|
||||
}
|
||||
// A non-materializing sibling only advances the environment to ready
|
||||
// when inventory is present: already recorded before this launch,
|
||||
// about to be written by this launch, or not required because the
|
||||
// task has no configured repositories at all. A launch whose prepare
|
||||
// step failed produces an empty repos slice here — leaving the
|
||||
// environment's existing status untouched (it can only already be
|
||||
// ready or stopped by the time this branch runs) keeps that launch
|
||||
// from bricking future reuse instead of silently corrupting the row.
|
||||
if len(repos) > 0 || len(existingEnv.Repos) > 0 || !repoBacked {
|
||||
existingEnv.Status = models.TaskEnvironmentStatusReady
|
||||
}
|
||||
existingEnv.MaterializationSessionID = ""
|
||||
if err := e.repo.UpdateTaskEnvironment(ctx, existingEnv); err != nil {
|
||||
existingEnv.Status = previousStatus
|
||||
existingEnv.MaterializationSessionID = previousMaterializationSessionID
|
||||
e.logger.Warn("failed to update task environment",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("env_id", existingEnv.ID),
|
||||
zap.Error(err))
|
||||
return fmt.Errorf("update task environment: %w", err)
|
||||
}
|
||||
session.TaskEnvironmentID = existingEnv.ID
|
||||
// Persist per-repo rows for launches that didn't have them yet. The
|
||||
// environment-repository rows are the only physical-worktree record,
|
||||
// so single-repo launches write one row here too.
|
||||
e.persistTaskEnvironmentRepos(ctx, existingEnv.ID, environmentReposForLaunch(req, resp))
|
||||
e.selfHealTaskRepositoryBaseBranches(ctx, taskID, req, resp)
|
||||
return
|
||||
return nil
|
||||
}
|
||||
|
||||
env := &models.TaskEnvironment{
|
||||
@@ -2295,10 +2371,28 @@ func (e *Executor) persistTaskEnvironment(
|
||||
e.logger.Warn("failed to create task environment",
|
||||
zap.String("task_id", taskID),
|
||||
zap.Error(err))
|
||||
return
|
||||
return fmt.Errorf("create task environment: %w", err)
|
||||
}
|
||||
session.TaskEnvironmentID = env.ID
|
||||
e.selfHealTaskRepositoryBaseBranches(ctx, taskID, req, resp)
|
||||
return nil
|
||||
}
|
||||
|
||||
// taskIsRepoBacked reports whether a task has any configured repositories.
|
||||
// Fails closed on a read error (treats the task as repo-backed) so a
|
||||
// transient failure cannot mask the ready-with-empty-inventory bug the
|
||||
// persistTaskEnvironment status guard exists to prevent.
|
||||
func (e *Executor) taskIsRepoBacked(ctx context.Context, taskID string) bool {
|
||||
if e.repo == nil {
|
||||
return false
|
||||
}
|
||||
repos, err := e.repo.ListTaskRepositories(ctx, taskID)
|
||||
if err != nil {
|
||||
e.logger.Warn("failed to check task repositories for environment ready-status guard",
|
||||
zap.String("task_id", taskID), zap.Error(err))
|
||||
return true
|
||||
}
|
||||
return len(repos) > 0
|
||||
}
|
||||
|
||||
func (e *Executor) selfHealTaskRepositoryBaseBranches(
|
||||
@@ -2419,16 +2513,16 @@ func buildTaskEnvironmentRepos(worktrees []RepoWorktreeResult) []*models.TaskEnv
|
||||
// Used when an existing environment is reused (resume / re-launch on the same
|
||||
// task), including cases where stale or legacy rows need the successful launch
|
||||
// result written back for the next handoff.
|
||||
func (e *Executor) persistTaskEnvironmentRepos(ctx context.Context, envID string, repos []*models.TaskEnvironmentRepo) {
|
||||
func (e *Executor) persistTaskEnvironmentRepos(ctx context.Context, envID string, repos []*models.TaskEnvironmentRepo) error {
|
||||
if envID == "" || len(repos) == 0 {
|
||||
return
|
||||
return nil
|
||||
}
|
||||
existing, err := e.repo.ListTaskEnvironmentRepos(ctx, envID)
|
||||
if err != nil {
|
||||
e.logger.Warn("failed to list existing task_environment_repos before insert",
|
||||
zap.String("env_id", envID),
|
||||
zap.Error(err))
|
||||
return
|
||||
return fmt.Errorf("list task environment repositories: %w", err)
|
||||
}
|
||||
byKey := make(map[string]*models.TaskEnvironmentRepo, len(existing))
|
||||
legacyFlatByRepo := make(map[string]*models.TaskEnvironmentRepo)
|
||||
@@ -2445,12 +2539,16 @@ func (e *Executor) persistTaskEnvironmentRepos(ctx context.Context, envID string
|
||||
}
|
||||
key := w.RepositoryID + "\x00" + w.BranchSlug
|
||||
if row := byKey[key]; row != nil {
|
||||
e.refreshTaskEnvironmentRepo(ctx, row, w, i)
|
||||
if err := e.refreshTaskEnvironmentRepo(ctx, row, w, i); err != nil {
|
||||
return err
|
||||
}
|
||||
continue
|
||||
}
|
||||
if w.BranchSlug != "" {
|
||||
if row := legacyFlatByRepo[w.RepositoryID]; row != nil {
|
||||
e.refreshTaskEnvironmentRepo(ctx, row, w, i)
|
||||
if err := e.refreshTaskEnvironmentRepo(ctx, row, w, i); err != nil {
|
||||
return err
|
||||
}
|
||||
delete(legacyFlatByRepo, w.RepositoryID)
|
||||
byKey[key] = row
|
||||
continue
|
||||
@@ -2471,13 +2569,15 @@ func (e *Executor) persistTaskEnvironmentRepos(ctx context.Context, envID string
|
||||
zap.String("env_id", envID),
|
||||
zap.String("repository_id", w.RepositoryID),
|
||||
zap.Error(createErr))
|
||||
return fmt.Errorf("persist task environment repository %q: %w", w.RepositoryID, createErr)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (e *Executor) refreshTaskEnvironmentRepo(ctx context.Context, row, w *models.TaskEnvironmentRepo, position int) {
|
||||
func (e *Executor) refreshTaskEnvironmentRepo(ctx context.Context, row, w *models.TaskEnvironmentRepo, position int) error {
|
||||
if !taskEnvironmentRepoNeedsRefresh(row, w, position) {
|
||||
return
|
||||
return nil
|
||||
}
|
||||
row.BranchSlug = w.BranchSlug
|
||||
// Concrete launch results populate the physical tuple together; inventory-only
|
||||
@@ -2495,7 +2595,9 @@ func (e *Executor) refreshTaskEnvironmentRepo(ctx context.Context, row, w *model
|
||||
zap.String("repository_id", row.RepositoryID),
|
||||
zap.String("branch_slug", row.BranchSlug),
|
||||
zap.Error(err))
|
||||
return fmt.Errorf("update task environment repository %q: %w", row.RepositoryID, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func taskEnvironmentRepoNeedsRefresh(row, w *models.TaskEnvironmentRepo, position int) bool {
|
||||
|
||||
@@ -279,6 +279,8 @@ type mockRepository struct {
|
||||
getTaskSessionFunc func(ctx context.Context, id string) (*models.TaskSession, error)
|
||||
getTaskEnvironmentFunc func(ctx context.Context, id string) (*models.TaskEnvironment, error)
|
||||
getTaskEnvironmentByTaskIDFunc func(ctx context.Context, taskID string) (*models.TaskEnvironment, error)
|
||||
createTaskEnvironmentRepoErr error
|
||||
finalizeTaskEnvironmentErr error
|
||||
createTaskSessionFunc func(ctx context.Context, session *models.TaskSession) error
|
||||
updateTaskSessionStateFunc func(ctx context.Context, sessionID string, state models.TaskSessionState, errorMessage string) error
|
||||
listActiveTaskSessionsByTaskIDFunc func(ctx context.Context, taskID string) ([]*models.TaskSession, error)
|
||||
@@ -311,6 +313,12 @@ type mockRepository struct {
|
||||
finalizeTaskEnvironmentCalls []*models.TaskEnvironment
|
||||
updateTaskStateIfCurrentInCalls []updateTaskStateIfCurrentInCall
|
||||
updateTaskStateIfNotArchivedCalls []updateTaskStateIfNotArchivedCall
|
||||
|
||||
// writeCallLog records the relative order of environment-row and
|
||||
// environment-status writes (e.g. "create_repo", "update_env") so tests
|
||||
// can pin ordering invariants that a call-count assertion alone cannot
|
||||
// catch — see TestPersistTaskEnvironment_NonMaterializerSiblingPersistsReposBeforeReady.
|
||||
writeCallLog []string
|
||||
}
|
||||
|
||||
type sharedWorkspaceBindingCall struct {
|
||||
@@ -1153,23 +1161,31 @@ func (m *mockRepository) UpdateTaskEnvironment(_ context.Context, env *models.Ta
|
||||
if env.ID == "" {
|
||||
return nil
|
||||
}
|
||||
m.writeCallLog = append(m.writeCallLog, "update_env")
|
||||
m.updateTaskEnvironmentCalls = append(m.updateTaskEnvironmentCalls, env)
|
||||
m.taskEnvironments[env.ID] = env
|
||||
return nil
|
||||
}
|
||||
func (m *mockRepository) FinalizeTaskEnvironmentMaterialization(_ context.Context, env *models.TaskEnvironment, repos []*models.TaskEnvironmentRepo, _ string) error {
|
||||
if m.finalizeTaskEnvironmentErr != nil {
|
||||
return m.finalizeTaskEnvironmentErr
|
||||
}
|
||||
m.finalizeTaskEnvironmentCalls = append(m.finalizeTaskEnvironmentCalls, env)
|
||||
m.taskEnvironments[env.ID] = env
|
||||
m.taskEnvironmentRepos[env.ID] = repos
|
||||
return nil
|
||||
}
|
||||
func (m *mockRepository) CreateTaskEnvironmentRepo(_ context.Context, repo *models.TaskEnvironmentRepo) error {
|
||||
if m.createTaskEnvironmentRepoErr != nil {
|
||||
return m.createTaskEnvironmentRepoErr
|
||||
}
|
||||
if repo.ID == "" {
|
||||
repo.ID = repo.TaskEnvironmentID + "-repo-" + repo.RepositoryID
|
||||
if repo.BranchSlug != "" {
|
||||
repo.ID += "-branch-" + repo.BranchSlug
|
||||
}
|
||||
}
|
||||
m.writeCallLog = append(m.writeCallLog, "create_repo")
|
||||
m.taskEnvironmentRepos[repo.TaskEnvironmentID] = append(m.taskEnvironmentRepos[repo.TaskEnvironmentID], repo)
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -507,6 +507,44 @@ func TestResumeSession_MultiRepo_PopulatesRequestRepositories(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildResumeRequest_RejectsMismatchedEnvironmentInventory(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
const taskID = "task-resume-inventory-mismatch"
|
||||
const sessionID = "session-resume-inventory-mismatch"
|
||||
seedMultiRepoTask(t, repo, taskID)
|
||||
seedWorktreeExecutor(repo)
|
||||
repo.tasks[taskID] = &models.Task{ID: taskID, WorkspaceID: "ws-1", Title: "Resume"}
|
||||
repo.taskEnvironments["env-mismatch"] = &models.TaskEnvironment{
|
||||
ID: "env-mismatch",
|
||||
TaskID: taskID,
|
||||
ExecutorType: string(models.ExecutorTypeWorktree),
|
||||
Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{{
|
||||
TaskEnvironmentID: "env-mismatch",
|
||||
RepositoryID: "repo-unrelated",
|
||||
BranchSlug: "main",
|
||||
WorktreeID: "wt-unrelated",
|
||||
}},
|
||||
}
|
||||
repo.taskEnvironmentRepos["env-mismatch"] = repo.taskEnvironments["env-mismatch"].Repos
|
||||
repo.sessions[sessionID] = &models.TaskSession{
|
||||
ID: sessionID,
|
||||
TaskID: taskID,
|
||||
RepositoryID: "repo-front",
|
||||
ExecutorID: models.ExecutorIDWorktree,
|
||||
AgentProfileID: "profile-123",
|
||||
State: models.TaskSessionStateCancelled,
|
||||
StartedAt: time.Now(),
|
||||
UpdatedAt: time.Now(),
|
||||
}
|
||||
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
_, _, _, _, _, err := exec.buildResumeRequest(context.Background(), repo.tasks[taskID].ToAPI(), repo.sessions[sessionID], false)
|
||||
if !errors.Is(err, models.ErrWorkspaceReuseUnsafe) {
|
||||
t.Fatalf("buildResumeRequest error = %v, want ErrWorkspaceReuseUnsafe", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReuseExistingRepositoryWorktrees_EnvironmentRowsWinOverSessionRows(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
taskID := "task-env-wins"
|
||||
|
||||
@@ -748,7 +748,15 @@ func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSessio
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
e.persistTaskEnvironment(ctx, task.ID, session, existingEnv, req, resp, execCfg)
|
||||
if err := e.persistTaskEnvironment(ctx, task.ID, session, existingEnv, req, resp, execCfg); err != nil {
|
||||
e.cleanupUnstartedExecutionAfterPersistError(ctx, session.ID, resp.AgentExecutionID, err)
|
||||
e.markTaskEnvironmentMaterializationFailed(ctx, existingEnv, session.ID)
|
||||
if startAgent {
|
||||
e.rollbackResumeStateAfterFailure(ctx, task.ID, session.ID, resumeInitialState, err,
|
||||
resumeCredentialSnapshotBackupIfPersisted(credentialSnapshotPersisted, previousCredentialSnapshot))
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
|
||||
worktreePath := resp.WorktreePath
|
||||
worktreeBranch := resp.WorktreeBranch
|
||||
@@ -1022,6 +1030,7 @@ func (e *Executor) buildResumeRequestAtCredentialBoundary(
|
||||
existingEnv,
|
||||
req.ExecutorType,
|
||||
existingEnv != nil && existingEnv.MaterializationSessionID != session.ID,
|
||||
e.taskIsRepoBacked(ctx, task.ID),
|
||||
)
|
||||
|
||||
allRepos, err := e.resolveAllRepoInfoForSession(ctx, task.ID, session.ID)
|
||||
@@ -1038,6 +1047,9 @@ func (e *Executor) buildResumeRequestAtCredentialBoundary(
|
||||
req.RemoteSyncHandled = allRepos[0].RemoteSyncHandled
|
||||
req.RefreshRepository = allRepos[0].RefreshRepository
|
||||
}
|
||||
if err := e.validateReuseEnvironmentInventory(ctx, req, existingEnv); err != nil {
|
||||
return nil, "", execConfig, existingEnv, nil, err
|
||||
}
|
||||
|
||||
e.reuseExistingEnvironment(ctx, req, existingEnv)
|
||||
|
||||
|
||||
@@ -49,6 +49,39 @@ func TestResumeSession_RejectsArchivedTask(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeSession_PropagatesTaskEnvironmentPersistenceFailure(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
attachManagedGitHubRepositoryForResume(t, repo)
|
||||
persistErr := errors.New("inventory write failed")
|
||||
repo.createTaskEnvironmentRepoErr = persistErr
|
||||
repo.taskEnvironments["env-1"] = &models.TaskEnvironment{
|
||||
ID: "env-1",
|
||||
TaskID: "task-1",
|
||||
ExecutorType: string(models.ExecutorTypeLocal),
|
||||
Status: models.TaskEnvironmentStatusReady,
|
||||
}
|
||||
repo.sessions["sess-1"].TaskEnvironmentID = "env-1"
|
||||
|
||||
agentManager := &mockAgentManager{
|
||||
launchAgentFunc: func(_ context.Context, _ *LaunchAgentRequest) (*LaunchAgentResponse, error) {
|
||||
return &LaunchAgentResponse{
|
||||
AgentExecutionID: "exec-new",
|
||||
WorktreePath: "/workspace/task-1",
|
||||
Worktrees: []RepoWorktreeResult{{
|
||||
RepositoryID: "repo-1", WorktreeID: "wt-1", WorktreePath: "/workspace/task-1",
|
||||
}},
|
||||
}, nil
|
||||
},
|
||||
}
|
||||
exec := newTestExecutor(t, agentManager, repo)
|
||||
|
||||
_, err := exec.ResumeSession(context.Background(), repo.sessions["sess-1"], false)
|
||||
if !errors.Is(err, persistErr) {
|
||||
t.Fatalf("ResumeSession error = %v, want %v", err, persistErr)
|
||||
}
|
||||
}
|
||||
|
||||
// setupLiveResumeTestFixture seeds a repo + task + session + executor-running
|
||||
// record suitable for exercising the ResumeSession launch path.
|
||||
func setupLiveResumeTestFixture(repo *mockRepository) {
|
||||
|
||||
@@ -604,6 +604,54 @@ func TestLaunchPreparedSession_Success(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestLaunchPreparedSession_PropagatesTaskEnvironmentPersistenceFailure(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
persistErr := errors.New("inventory write failed")
|
||||
repo.createTaskEnvironmentRepoErr = persistErr
|
||||
repo.executors[models.ExecutorIDLocal] = &models.Executor{
|
||||
ID: models.ExecutorIDLocal, Type: models.ExecutorTypeLocal, Status: models.ExecutorStatusActive,
|
||||
}
|
||||
repo.repositories["repo-1"] = &models.Repository{
|
||||
ID: "repo-1", WorkspaceID: "workspace-1", SourceType: sourceTypeLocal, LocalPath: "/repo-1", Name: "repo-1",
|
||||
}
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1", Position: 0,
|
||||
}
|
||||
repo.taskEnvironments["env-1"] = &models.TaskEnvironment{
|
||||
ID: "env-1", TaskID: "task-1", ExecutorType: string(models.ExecutorTypeLocal), Status: models.TaskEnvironmentStatusReady,
|
||||
}
|
||||
repo.sessions["session-1"] = &models.TaskSession{
|
||||
ID: "session-1", TaskID: "task-1", AgentProfileID: "profile-1", ExecutorID: models.ExecutorIDLocal,
|
||||
TaskEnvironmentID: "env-1", State: models.TaskSessionStateCreated, StartedAt: time.Now(), UpdatedAt: time.Now(),
|
||||
}
|
||||
stopped := false
|
||||
agentManager := &mockAgentManager{
|
||||
launchAgentFunc: func(_ context.Context, _ *LaunchAgentRequest) (*LaunchAgentResponse, error) {
|
||||
return &LaunchAgentResponse{
|
||||
AgentExecutionID: "exec-new",
|
||||
Worktrees: []RepoWorktreeResult{{RepositoryID: "repo-1", WorktreeID: "wt-1"}},
|
||||
}, nil
|
||||
},
|
||||
stopAgentFunc: func(_ context.Context, executionID string, _ bool) error {
|
||||
if executionID == "exec-new" {
|
||||
stopped = true
|
||||
}
|
||||
return nil
|
||||
},
|
||||
}
|
||||
exec := newTestExecutor(t, agentManager, repo)
|
||||
|
||||
_, err := exec.LaunchPreparedSession(context.Background(), &v1.Task{
|
||||
ID: "task-1", WorkspaceID: "workspace-1", Title: "Task 1",
|
||||
}, "session-1", LaunchOptions{AgentProfileID: "profile-1", ExecutorID: models.ExecutorIDLocal})
|
||||
if !errors.Is(err, persistErr) {
|
||||
t.Fatalf("LaunchPreparedSession error = %v, want %v", err, persistErr)
|
||||
}
|
||||
if !stopped {
|
||||
t.Fatal("persistence failure did not stop the unstarted execution")
|
||||
}
|
||||
}
|
||||
|
||||
func TestLaunchPreparedSession_RejectsNilLaunchResponse(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.sessions["session-nil-response"] = &models.TaskSession{
|
||||
|
||||
@@ -15,6 +15,7 @@ func TestWorkspaceReuseAllowedRequiresLiveWorktreeRepo(t *testing.T) {
|
||||
env *models.TaskEnvironment
|
||||
requestedType string
|
||||
reuseRequired bool
|
||||
repoBacked bool
|
||||
wantReuseAllowed bool
|
||||
}{
|
||||
{
|
||||
@@ -119,7 +120,48 @@ func TestWorkspaceReuseAllowedRequiresLiveWorktreeRepo(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := workspaceReuseAllowed(tt.env, tt.requestedType, tt.reuseRequired); got != tt.wantReuseAllowed {
|
||||
if got := workspaceReuseAllowed(tt.env, tt.requestedType, tt.reuseRequired, tt.repoBacked); got != tt.wantReuseAllowed {
|
||||
t.Fatalf("workspaceReuseAllowed() = %t, want %t", got, tt.wantReuseAllowed)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// @covers AC-TASKS-RUNTIME-CLEANUP-001.7
|
||||
func TestWorkspaceReuseAllowedRequiresSSHInventory(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
env *models.TaskEnvironment
|
||||
repoBacked bool
|
||||
wantReuseAllowed bool
|
||||
}{
|
||||
{
|
||||
name: "repo-backed with empty inventory forces fresh materialization",
|
||||
env: &models.TaskEnvironment{ExecutorType: string(models.ExecutorTypeSSH)},
|
||||
repoBacked: true,
|
||||
wantReuseAllowed: false,
|
||||
},
|
||||
{
|
||||
name: "repo-backed with recorded inventory reuses",
|
||||
env: &models.TaskEnvironment{
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
Repos: []*models.TaskEnvironmentRepo{{RepositoryID: "repo-1"}},
|
||||
},
|
||||
repoBacked: true,
|
||||
wantReuseAllowed: true,
|
||||
},
|
||||
{
|
||||
name: "repo-less task reuses despite empty inventory",
|
||||
env: &models.TaskEnvironment{ExecutorType: string(models.ExecutorTypeSSH)},
|
||||
repoBacked: false,
|
||||
wantReuseAllowed: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := workspaceReuseAllowed(tt.env, string(models.ExecutorTypeSSH), true, tt.repoBacked)
|
||||
if got != tt.wantReuseAllowed {
|
||||
t.Fatalf("workspaceReuseAllowed() = %t, want %t", got, tt.wantReuseAllowed)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -141,20 +141,30 @@ func (p *process) start() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// stop signals the supervision loop to exit, waits for it to drain, and
|
||||
// kills any still-running process. Idempotent-safe to call at most once per
|
||||
// process (Manager.Stop removes the process from its map before calling
|
||||
// this, so a second Stop(id) for the same id is a no-op at the Manager
|
||||
// stop signals the supervision loop to exit and kills the current process
|
||||
// before waiting for the loop to drain. The kill must happen first because a
|
||||
// health ping can be blocked in the plugin protocol; terminating the process
|
||||
// releases that call and lets the supervisor exit. Idempotent-safe to call at
|
||||
// most once per process (Manager.Stop removes the process from its map before
|
||||
// calling this, so a second Stop(id) for the same id is a no-op at the Manager
|
||||
// level).
|
||||
func (p *process) stop() {
|
||||
close(p.stopCh)
|
||||
p.mu.Lock()
|
||||
current := p.current
|
||||
p.current = nil
|
||||
p.mu.Unlock()
|
||||
if current != nil {
|
||||
current.Kill()
|
||||
}
|
||||
p.wg.Wait()
|
||||
p.mu.Lock()
|
||||
if p.current != nil {
|
||||
p.current.Kill()
|
||||
p.current = nil
|
||||
}
|
||||
current = p.current
|
||||
p.current = nil
|
||||
p.mu.Unlock()
|
||||
if current != nil {
|
||||
current.Kill()
|
||||
}
|
||||
}
|
||||
|
||||
// remote returns the current live RemotePlugin, if any.
|
||||
|
||||
@@ -19,6 +19,33 @@ type fakeSpawnedProcess struct {
|
||||
killed bool
|
||||
}
|
||||
|
||||
type blockingPingProcess struct {
|
||||
pingStarted chan struct{}
|
||||
releasePing chan struct{}
|
||||
killOnce sync.Once
|
||||
}
|
||||
|
||||
func newBlockingPingProcess() *blockingPingProcess {
|
||||
return &blockingPingProcess{
|
||||
pingStarted: make(chan struct{}),
|
||||
releasePing: make(chan struct{}),
|
||||
}
|
||||
}
|
||||
|
||||
func (f *blockingPingProcess) Ping() error {
|
||||
close(f.pingStarted)
|
||||
<-f.releasePing
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *blockingPingProcess) Exited() bool { return false }
|
||||
|
||||
func (f *blockingPingProcess) Kill() {
|
||||
f.killOnce.Do(func() { close(f.releasePing) })
|
||||
}
|
||||
|
||||
func (f *blockingPingProcess) Remote() *pluginsdk.RemotePlugin { return nil }
|
||||
|
||||
func (f *fakeSpawnedProcess) Ping() error {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
@@ -258,6 +285,35 @@ func TestProcess_HandleFailureAndRestart_StopDuringBackoffSkipsSpawn(t *testing.
|
||||
}
|
||||
}
|
||||
|
||||
func TestProcess_StopKillsProcessBeforeWaitingForBlockedPing(t *testing.T) {
|
||||
blocked := newBlockingPingProcess()
|
||||
p := newProcess("test-plugin", testLogger(t), func() (spawnedProcess, error) {
|
||||
return blocked, nil
|
||||
}, nil)
|
||||
p.pingInterval = 0
|
||||
|
||||
if err := p.start(); err != nil {
|
||||
t.Fatalf("start: %v", err)
|
||||
}
|
||||
<-blocked.pingStarted
|
||||
|
||||
stopped := make(chan struct{})
|
||||
go func() {
|
||||
p.stop()
|
||||
close(stopped)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-stopped:
|
||||
case <-time.After(time.Second):
|
||||
// Release the test double so the old wait-before-kill implementation
|
||||
// cannot leak its supervisor goroutine after this assertion fails.
|
||||
blocked.Kill()
|
||||
<-stopped
|
||||
t.Fatal("stop waited for a blocked ping before killing the plugin process")
|
||||
}
|
||||
}
|
||||
|
||||
func equalBoolSlices(a, b []bool) bool {
|
||||
if len(a) != len(b) {
|
||||
return false
|
||||
|
||||
@@ -43,6 +43,21 @@ func (r *Repository) CreateTaskEnvironment(ctx context.Context, env *models.Task
|
||||
return err
|
||||
}
|
||||
|
||||
// A ready environment with zero repo rows is indistinguishable from "the
|
||||
// canonical inventory was never captured" and permanently bricks reuse
|
||||
// (validateReuseEnvironmentInventory refuses every later launch). Reject
|
||||
// at the boundary instead of trusting the caller: only a task with no
|
||||
// repositories configured at all may legitimately have none.
|
||||
if env.Status == models.TaskEnvironmentStatusReady && len(env.Repos) == 0 {
|
||||
hasRepos, err := r.taskHasRepositoriesTx(ctx, tx, env.TaskID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if hasRepos {
|
||||
return fmt.Errorf("create task environment: ready status requires repository inventory for repo-backed task (task=%s)", env.TaskID)
|
||||
}
|
||||
}
|
||||
|
||||
if _, err := tx.ExecContext(ctx, r.db.Rebind(`
|
||||
INSERT INTO task_environments (
|
||||
id, task_id, executor_type, executor_id, executor_profile_id,
|
||||
@@ -151,9 +166,19 @@ func (r *Repository) UpdateTaskEnvironment(ctx context.Context, env *models.Task
|
||||
if env.ExecutorType == string(models.ExecutorTypeWorktree) && env.WorkspacePath == "" && env.Status != models.TaskEnvironmentStatusCreating {
|
||||
return fmt.Errorf("update task environment: worktree-mode env requires workspace_path (id=%s)", env.ID)
|
||||
}
|
||||
tx, err := r.db.BeginTxx(ctx, nil)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer func() { _ = tx.Rollback() }()
|
||||
if env.Status == models.TaskEnvironmentStatusReady {
|
||||
if err := r.validateReadyTaskEnvironment(ctx, tx, env.ID); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
env.UpdatedAt = time.Now().UTC()
|
||||
|
||||
result, err := r.db.ExecContext(ctx, r.db.Rebind(`
|
||||
result, err := tx.ExecContext(ctx, r.db.Rebind(`
|
||||
UPDATE task_environments SET
|
||||
executor_type = ?, executor_id = ?, executor_profile_id = ?,
|
||||
control_port = ?, status = ?, materialization_session_id = ?,
|
||||
@@ -176,9 +201,49 @@ func (r *Repository) UpdateTaskEnvironment(ctx context.Context, env *models.Task
|
||||
if rows == 0 {
|
||||
return fmt.Errorf("%w: %s", ErrTaskEnvironmentNotFound, env.ID)
|
||||
}
|
||||
return tx.Commit()
|
||||
}
|
||||
|
||||
func (r *Repository) validateReadyTaskEnvironment(ctx context.Context, tx *sqlx.Tx, environmentID string) error {
|
||||
taskID, currentStatus, err := r.taskEnvironmentStateTx(ctx, tx, environmentID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := r.taskCleanupBarrierLocked(ctx, tx, taskID); err != nil {
|
||||
return err
|
||||
}
|
||||
// Workspace-source materialization updates an already-ready environment
|
||||
// after it adds a folder or repository. That update does not publish a new
|
||||
// ready state, so it must remain compatible with legacy environments whose
|
||||
// canonical inventory is still empty. Launch persistence separately rejects
|
||||
// that state before it calls this repository method.
|
||||
if currentStatus == models.TaskEnvironmentStatusReady {
|
||||
return nil
|
||||
}
|
||||
hasRepos, err := r.taskHasRepositoriesTx(ctx, tx, taskID)
|
||||
if err != nil || !hasRepos {
|
||||
return err
|
||||
}
|
||||
var inventoryCount int
|
||||
if err := tx.QueryRowContext(ctx, r.db.Rebind(`SELECT COUNT(1) FROM task_environment_repos WHERE task_environment_id = ?`), environmentID).Scan(&inventoryCount); err != nil {
|
||||
return err
|
||||
}
|
||||
if inventoryCount == 0 {
|
||||
return fmt.Errorf("update task environment: ready status requires repository inventory for repo-backed task (id=%s)", environmentID)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *Repository) taskEnvironmentStateTx(ctx context.Context, tx *sqlx.Tx, environmentID string) (string, models.TaskEnvironmentStatus, error) {
|
||||
var taskID string
|
||||
var status string
|
||||
err := tx.QueryRowContext(ctx, r.db.Rebind(`SELECT task_id, status FROM task_environments WHERE id = ?`), environmentID).Scan(&taskID, &status)
|
||||
if err == sql.ErrNoRows {
|
||||
return "", "", fmt.Errorf("%w: %s", ErrTaskEnvironmentNotFound, environmentID)
|
||||
}
|
||||
return taskID, models.TaskEnvironmentStatus(status), err
|
||||
}
|
||||
|
||||
// FinalizeTaskEnvironmentMaterialization writes the complete canonical
|
||||
// repository inventory and transitions the elected creating environment to
|
||||
// ready in one transaction. The materializer ID prevents a sibling session
|
||||
@@ -208,6 +273,19 @@ func (r *Repository) FinalizeTaskEnvironmentMaterialization(
|
||||
if err := r.taskCleanupBarrierLocked(ctx, tx, taskID); err != nil {
|
||||
return err
|
||||
}
|
||||
// See the matching guard in CreateTaskEnvironment: a materializer that
|
||||
// publishes ready with an empty inventory (e.g. because its prepare step
|
||||
// failed and produced no repos) must not be allowed to land, or the
|
||||
// environment becomes permanently unreusable.
|
||||
if len(repos) == 0 {
|
||||
hasRepos, err := r.taskHasRepositoriesTx(ctx, tx, taskID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if hasRepos {
|
||||
return fmt.Errorf("finalize task environment: ready status requires repository inventory for repo-backed task (id=%s)", env.ID)
|
||||
}
|
||||
}
|
||||
if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM task_environment_repos WHERE task_environment_id = ?`), env.ID); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -243,6 +321,17 @@ func (r *Repository) FinalizeTaskEnvironmentMaterialization(
|
||||
return tx.Commit()
|
||||
}
|
||||
|
||||
// taskHasRepositoriesTx reports whether a task has any task_repositories
|
||||
// rows, i.e. whether it is repo-backed and therefore expected to carry a
|
||||
// non-empty canonical inventory whenever its environment is ready.
|
||||
func (r *Repository) taskHasRepositoriesTx(ctx context.Context, tx *sqlx.Tx, taskID string) (bool, error) {
|
||||
var count int
|
||||
if err := tx.QueryRowContext(ctx, r.db.Rebind(`SELECT COUNT(1) FROM task_repositories WHERE task_id = ?`), taskID).Scan(&count); err != nil {
|
||||
return false, err
|
||||
}
|
||||
return count > 0, nil
|
||||
}
|
||||
|
||||
func (r *Repository) TransferTaskEnvironmentToTask(ctx context.Context, envID, taskID string) error {
|
||||
result, err := r.db.ExecContext(ctx, r.db.Rebind(`
|
||||
UPDATE task_environments SET
|
||||
|
||||
@@ -188,6 +188,42 @@ func TestFinalizeTaskEnvironmentMaterializationPublishesInventoryAtomically(t *t
|
||||
}
|
||||
}
|
||||
|
||||
func TestFinalizeTaskEnvironmentMaterializationAllowsEmptyInventoryForRepoLessTask(t *testing.T) {
|
||||
repo := newRepoForEntityTests(t)
|
||||
ctx := context.Background()
|
||||
seedWorkspace(t, repo, "workspace-finalize-repoless")
|
||||
if err := repo.CreateTask(ctx, &models.Task{ID: "task-finalize-repoless", WorkspaceID: "workspace-finalize-repoless", Title: "Repo-less"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-finalize-repoless",
|
||||
TaskID: "task-finalize-repoless",
|
||||
ExecutorType: string(models.ExecutorTypeLocal),
|
||||
Status: models.TaskEnvironmentStatusCreating,
|
||||
MaterializationSessionID: "session-materializer-repoless",
|
||||
}
|
||||
if err := repo.CreateTaskEnvironment(ctx, env); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
|
||||
env.Status = models.TaskEnvironmentStatusReady
|
||||
env.MaterializationSessionID = ""
|
||||
if err := repo.FinalizeTaskEnvironmentMaterialization(ctx, env, nil, "session-materializer-repoless"); err != nil {
|
||||
t.Fatalf("FinalizeTaskEnvironmentMaterialization: %v", err)
|
||||
}
|
||||
|
||||
persisted, err := repo.GetTaskEnvironment(ctx, env.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskEnvironment: %v", err)
|
||||
}
|
||||
if persisted.Status != models.TaskEnvironmentStatusReady || persisted.MaterializationSessionID != "" {
|
||||
t.Fatalf("environment = status %q owner %q, want ready with no owner", persisted.Status, persisted.MaterializationSessionID)
|
||||
}
|
||||
if len(persisted.Repos) != 0 {
|
||||
t.Fatalf("canonical inventory = %#v, want empty for repo-less task", persisted.Repos)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateTaskSessionWithWorkspaceBindingElectsAndAttaches(t *testing.T) {
|
||||
repo := newRepoForEntityTests(t)
|
||||
ctx := context.Background()
|
||||
@@ -500,3 +536,150 @@ func TestTransferTaskEnvironmentMissingReturnsSentinel(t *testing.T) {
|
||||
t.Fatalf("TransferTaskEnvironmentToTask error = %v, want ErrTaskEnvironmentNotFound", err)
|
||||
}
|
||||
}
|
||||
|
||||
// seedRepoBackedTask creates a task with one linked task_repositories row, so
|
||||
// the task reads as repo-backed to the ready-requires-inventory guards below.
|
||||
func seedRepoBackedTask(t *testing.T, repo *Repository, workspaceID, taskID string) {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
seedWorkspace(t, repo, workspaceID)
|
||||
if err := repo.CreateTask(ctx, &models.Task{ID: taskID, WorkspaceID: workspaceID, Title: taskID}); err != nil {
|
||||
t.Fatalf("CreateTask: %v", err)
|
||||
}
|
||||
if err := repo.CreateRepository(ctx, &models.Repository{ID: taskID + "-repo", WorkspaceID: workspaceID, Name: taskID + "-repo"}); err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
if err := repo.CreateTaskRepository(ctx, &models.TaskRepository{TaskID: taskID, RepositoryID: taskID + "-repo", BaseBranch: "main"}); err != nil {
|
||||
t.Fatalf("CreateTaskRepository: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Regression test for the "failed prepare leaves a permanently bricked
|
||||
// workspace" defect: a launch whose prepare step fails produces an empty
|
||||
// repos slice, and the materializer must not be allowed to publish that as
|
||||
// ready inventory for a repo-backed task. FinalizeTaskEnvironmentMaterialization
|
||||
// hardcodes the ready status in its UPDATE, so this must be enforced inside
|
||||
// the function itself, not by the caller.
|
||||
func TestFinalizeTaskEnvironmentMaterializationRefusesEmptyInventoryForRepoBackedTask(t *testing.T) {
|
||||
repo := newRepoForEntityTests(t)
|
||||
ctx := context.Background()
|
||||
seedRepoBackedTask(t, repo, "workspace-finalize-empty-inventory", "task-finalize-empty-inventory")
|
||||
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-finalize-empty-inventory",
|
||||
TaskID: "task-finalize-empty-inventory",
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
Status: models.TaskEnvironmentStatusCreating,
|
||||
MaterializationSessionID: "session-materializer-empty",
|
||||
}
|
||||
if err := repo.CreateTaskEnvironment(ctx, env); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
|
||||
env.Status = models.TaskEnvironmentStatusReady
|
||||
env.MaterializationSessionID = ""
|
||||
if err := repo.FinalizeTaskEnvironmentMaterialization(ctx, env, nil, "session-materializer-empty"); err == nil {
|
||||
t.Fatal("FinalizeTaskEnvironmentMaterialization published ready with empty inventory for a repo-backed task, want error")
|
||||
}
|
||||
|
||||
persisted, err := repo.GetTaskEnvironment(ctx, env.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskEnvironment: %v", err)
|
||||
}
|
||||
if persisted.Status != models.TaskEnvironmentStatusCreating {
|
||||
t.Fatalf("environment after refused finalize = status %q, want unchanged creating", persisted.Status)
|
||||
}
|
||||
if len(persisted.Repos) != 0 {
|
||||
t.Fatalf("environment after refused finalize has inventory = %#v, want none", persisted.Repos)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateTaskEnvironmentRefusesReadyWithEmptyInventoryForRepoBackedTask(t *testing.T) {
|
||||
repo := newRepoForEntityTests(t)
|
||||
ctx := context.Background()
|
||||
seedRepoBackedTask(t, repo, "workspace-update-empty-inventory", "task-update-empty-inventory")
|
||||
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-update-empty-inventory",
|
||||
TaskID: "task-update-empty-inventory",
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
Status: models.TaskEnvironmentStatusCreating,
|
||||
}
|
||||
if err := repo.CreateTaskEnvironment(ctx, env); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
|
||||
env.Status = models.TaskEnvironmentStatusReady
|
||||
if err := repo.UpdateTaskEnvironment(ctx, env); err == nil {
|
||||
t.Fatal("UpdateTaskEnvironment published ready with empty inventory for repo-backed task, want error")
|
||||
}
|
||||
|
||||
persisted, err := repo.GetTaskEnvironment(ctx, env.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskEnvironment: %v", err)
|
||||
}
|
||||
if persisted.Status != models.TaskEnvironmentStatusCreating {
|
||||
t.Fatalf("environment after refused update = status %q, want unchanged creating", persisted.Status)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateTaskEnvironmentAllowsExistingReadyEnvironmentWithoutInventory(t *testing.T) {
|
||||
repo := newRepoForEntityTests(t)
|
||||
ctx := context.Background()
|
||||
workspaceID := "workspace-update-existing-ready"
|
||||
taskID := "task-update-existing-ready"
|
||||
seedWorkspace(t, repo, workspaceID)
|
||||
if err := repo.CreateTask(ctx, &models.Task{ID: taskID, WorkspaceID: workspaceID, Title: taskID}); err != nil {
|
||||
t.Fatalf("CreateTask: %v", err)
|
||||
}
|
||||
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-update-existing-ready",
|
||||
TaskID: taskID,
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
Status: models.TaskEnvironmentStatusReady,
|
||||
}
|
||||
if err := repo.CreateTaskEnvironment(ctx, env); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
if err := repo.CreateRepository(ctx, &models.Repository{ID: taskID + "-repo", WorkspaceID: workspaceID, Name: taskID + "-repo"}); err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
if err := repo.CreateTaskRepository(ctx, &models.TaskRepository{TaskID: taskID, RepositoryID: taskID + "-repo", BaseBranch: "main"}); err != nil {
|
||||
t.Fatalf("CreateTaskRepository: %v", err)
|
||||
}
|
||||
env.WorkspacePath = "/workspace/task-update-existing-ready"
|
||||
if err := repo.UpdateTaskEnvironment(ctx, env); err != nil {
|
||||
t.Fatalf("UpdateTaskEnvironment on existing ready environment: %v", err)
|
||||
}
|
||||
|
||||
persisted, err := repo.GetTaskEnvironment(ctx, env.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskEnvironment: %v", err)
|
||||
}
|
||||
if persisted.Status != models.TaskEnvironmentStatusReady || persisted.WorkspacePath != env.WorkspacePath {
|
||||
t.Fatalf("persisted environment = %+v, want ready with updated workspace path", persisted)
|
||||
}
|
||||
}
|
||||
|
||||
// Same defect, direct-create path: CreateTaskEnvironment must not persist a
|
||||
// ready environment with zero repo rows for a repo-backed task either.
|
||||
func TestCreateTaskEnvironmentRefusesReadyWithEmptyInventoryForRepoBackedTask(t *testing.T) {
|
||||
repo := newRepoForEntityTests(t)
|
||||
ctx := context.Background()
|
||||
seedRepoBackedTask(t, repo, "workspace-create-empty-inventory", "task-create-empty-inventory")
|
||||
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-create-empty-inventory",
|
||||
TaskID: "task-create-empty-inventory",
|
||||
ExecutorType: string(models.ExecutorTypeSSH),
|
||||
Status: models.TaskEnvironmentStatusReady,
|
||||
}
|
||||
if err := repo.CreateTaskEnvironment(ctx, env); err == nil {
|
||||
t.Fatal("CreateTaskEnvironment published ready with empty inventory for a repo-backed task, want error")
|
||||
}
|
||||
|
||||
if _, err := repo.GetTaskEnvironment(ctx, env.ID); !errors.Is(err, ErrTaskEnvironmentNotFound) {
|
||||
t.Fatalf("GetTaskEnvironment after refused create = %v, want ErrTaskEnvironmentNotFound", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -182,6 +182,9 @@ func TestAddBranchToTask_RejectsNonWorktreeExecutor(t *testing.T) {
|
||||
Status: "ready",
|
||||
CreatedAt: now,
|
||||
UpdatedAt: now,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{ID: "env-1-repo-0", RepositoryID: "repo-1", CreatedAt: now},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
@@ -231,6 +234,9 @@ func TestAddBranchToTask_AllowsWorktreeExecutor(t *testing.T) {
|
||||
Status: "ready",
|
||||
CreatedAt: now,
|
||||
UpdatedAt: now,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{ID: "env-1-repo-0", RepositoryID: "repo-1", CreatedAt: now},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
@@ -591,6 +597,9 @@ func TestAddBranchToTask_RejectsNonWorktreeExecutor_NoOrphanRepo(t *testing.T) {
|
||||
Status: "ready",
|
||||
CreatedAt: now,
|
||||
UpdatedAt: now,
|
||||
Repos: []*models.TaskEnvironmentRepo{
|
||||
{ID: "env-1-repo-0", RepositoryID: "repo-primary", CreatedAt: now},
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
@@ -693,14 +702,32 @@ func (m *stubMaterializer) MaterializeBranch(_ context.Context, _ string, _ stri
|
||||
|
||||
// seedWorktreeTaskEnv attaches a worktree-executor task_environments row so
|
||||
// requireWorktreeExecutorForBranchAdd permits the call and taskAlreadyLaunched
|
||||
// reports the task as live.
|
||||
// reports the task as live. The environment carries inventory rows mirroring
|
||||
// the task's existing task_repositories, since CreateTaskEnvironment now
|
||||
// requires a ready environment to carry inventory for a repo-backed task.
|
||||
func seedWorktreeTaskEnv(t *testing.T, repo interface {
|
||||
CreateTaskEnvironment(ctx context.Context, env *models.TaskEnvironment) error
|
||||
ListTaskRepositories(ctx context.Context, taskID string) ([]*models.TaskRepository, error)
|
||||
}, taskID, id string,
|
||||
) {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC()
|
||||
if err := repo.CreateTaskEnvironment(context.Background(), &models.TaskEnvironment{
|
||||
taskRepos, err := repo.ListTaskRepositories(ctx, taskID)
|
||||
if err != nil {
|
||||
t.Fatalf("ListTaskRepositories: %v", err)
|
||||
}
|
||||
envRepos := make([]*models.TaskEnvironmentRepo, len(taskRepos))
|
||||
for i, tr := range taskRepos {
|
||||
envRepos[i] = &models.TaskEnvironmentRepo{
|
||||
ID: fmt.Sprintf("%s-repo-%d", id, i),
|
||||
WorktreeID: fmt.Sprintf("%s-worktree-%d", id, i),
|
||||
RepositoryID: tr.RepositoryID,
|
||||
BranchSlug: tr.CheckoutBranch,
|
||||
CreatedAt: now,
|
||||
}
|
||||
}
|
||||
if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
|
||||
ID: id,
|
||||
TaskID: taskID,
|
||||
ExecutorType: string(models.ExecutorTypeWorktree),
|
||||
@@ -708,6 +735,7 @@ func seedWorktreeTaskEnv(t *testing.T, repo interface {
|
||||
Status: "ready",
|
||||
CreatedAt: now,
|
||||
UpdatedAt: now,
|
||||
Repos: envRepos,
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
|
||||
@@ -790,6 +790,22 @@ func createTestEnvironment(t *testing.T, repo *sqliterepo.Repository, envID, tas
|
||||
}
|
||||
}
|
||||
|
||||
// createTestEnvironmentWithRepos is like createTestEnvironment but seeds the
|
||||
// canonical repository inventory atomically with the ready environment.
|
||||
// CreateTaskEnvironment requires a ready environment for a repo-backed task
|
||||
// to carry its inventory up front, so tests whose task has task_repositories
|
||||
// rows must use this instead of a separate CreateTaskEnvironmentRepo call.
|
||||
func createTestEnvironmentWithRepos(t *testing.T, repo *sqliterepo.Repository, envID, taskID string, repos []*models.TaskEnvironmentRepo) {
|
||||
t.Helper()
|
||||
if err := repo.CreateTaskEnvironment(context.Background(), &models.TaskEnvironment{
|
||||
ID: envID, TaskID: taskID, ExecutorType: "worktree",
|
||||
WorkspacePath: "/tmp", Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: repos,
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment(%s): %v", envID, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetWorkspaceInfoForSession_BasicFields(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
@@ -913,13 +929,12 @@ func TestGetWorkspaceInfoForSession_ProjectsPersistedWorktreeIdentity(t *testing
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskSession: %v", err)
|
||||
}
|
||||
createTestEnvironment(t, repo, "env-recovery", "task-123")
|
||||
if err := repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{
|
||||
ID: "session-worktree-recovery", TaskEnvironmentID: "env-recovery", WorktreeID: "worktree-recovery", RepositoryID: "repo-recovery",
|
||||
BranchSlug: "feature-recovery", WorktreePath: "/tasks/task-recovery/api-feature-recovery", CreatedAt: now,
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironmentRepo: %v", err)
|
||||
}
|
||||
createTestEnvironmentWithRepos(t, repo, "env-recovery", "task-123", []*models.TaskEnvironmentRepo{
|
||||
{
|
||||
ID: "session-worktree-recovery", TaskEnvironmentID: "env-recovery", WorktreeID: "worktree-recovery", RepositoryID: "repo-recovery",
|
||||
BranchSlug: "feature-recovery", WorktreePath: "/tasks/task-recovery/api-feature-recovery", CreatedAt: now,
|
||||
},
|
||||
})
|
||||
|
||||
info, err := svc.GetWorkspaceInfoForSession(ctx, "task-123", "session-recovery")
|
||||
if err != nil {
|
||||
@@ -959,15 +974,10 @@ func TestGetWorkspaceInfoForSession_ProjectsDistinctWorktreesForSameRepositoryBr
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskSession: %v", err)
|
||||
}
|
||||
createTestEnvironment(t, repo, "env-multi-recovery", "task-123")
|
||||
for _, worktree := range []*models.TaskEnvironmentRepo{
|
||||
createTestEnvironmentWithRepos(t, repo, "env-multi-recovery", "task-123", []*models.TaskEnvironmentRepo{
|
||||
{ID: "session-worktree-one", TaskEnvironmentID: "env-multi-recovery", WorktreeID: "worktree-one", RepositoryID: "repo-multi-recovery", BranchSlug: "feature-one", CreatedAt: now},
|
||||
{ID: "session-worktree-two", TaskEnvironmentID: "env-multi-recovery", WorktreeID: "worktree-two", RepositoryID: "repo-multi-recovery", BranchSlug: "feature-two", CreatedAt: now},
|
||||
} {
|
||||
if err := repo.CreateTaskEnvironmentRepo(ctx, worktree); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironmentRepo %q: %v", worktree.BranchSlug, err)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
info, err := svc.GetWorkspaceInfoForSession(ctx, "task-123", "session-multi-recovery")
|
||||
if err != nil {
|
||||
@@ -1024,10 +1034,9 @@ func TestGetWorkspaceInfoForSession_UsesRepositoryDefaultBranchIdentity(t *testi
|
||||
if err := repo.CreateTaskSession(ctx, &models.TaskSession{ID: "session-default-branch", TaskID: "task-123", TaskEnvironmentID: "env-default-branch", State: models.TaskSessionStateCompleted, StartedAt: now, UpdatedAt: now}); err != nil {
|
||||
t.Fatalf("CreateTaskSession: %v", err)
|
||||
}
|
||||
createTestEnvironment(t, repo, "env-default-branch", "task-123")
|
||||
if err := repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{ID: "session-worktree-default-branch", TaskEnvironmentID: "env-default-branch", WorktreeID: "worktree-default-branch", RepositoryID: "repo-default-branch", BranchSlug: "main", CreatedAt: now}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironmentRepo: %v", err)
|
||||
}
|
||||
createTestEnvironmentWithRepos(t, repo, "env-default-branch", "task-123", []*models.TaskEnvironmentRepo{
|
||||
{ID: "session-worktree-default-branch", TaskEnvironmentID: "env-default-branch", WorktreeID: "worktree-default-branch", RepositoryID: "repo-default-branch", BranchSlug: "main", CreatedAt: now},
|
||||
})
|
||||
|
||||
info, err := svc.GetWorkspaceInfoForSession(ctx, "task-123", "session-default-branch")
|
||||
if err != nil {
|
||||
@@ -1063,15 +1072,14 @@ func TestGetWorkspaceInfoForSession_UsesHashDisambiguatedBranchIdentities(t *tes
|
||||
{RepositoryID: "repo-hash-branches", BaseBranch: "feature/a", DefaultBranch: "main", PRNumber: 101, Position: 0},
|
||||
{RepositoryID: "repo-hash-branches", BaseBranch: "feature-a", DefaultBranch: "main", PRNumber: 202, Position: 1},
|
||||
})
|
||||
createTestEnvironment(t, repo, "env-hash-branches", "task-123")
|
||||
envRepos := make([]*models.TaskEnvironmentRepo, 0, len(plans))
|
||||
for index, plan := range plans {
|
||||
if err := repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{
|
||||
envRepos = append(envRepos, &models.TaskEnvironmentRepo{
|
||||
ID: fmt.Sprintf("session-worktree-hash-%d", index), TaskEnvironmentID: "env-hash-branches", WorktreeID: fmt.Sprintf("worktree-hash-%d", index),
|
||||
RepositoryID: "repo-hash-branches", BranchSlug: plan.IdentitySlug, CreatedAt: now,
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironmentRepo %q: %v", plan.IdentitySlug, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
createTestEnvironmentWithRepos(t, repo, "env-hash-branches", "task-123", envRepos)
|
||||
|
||||
info, err := svc.GetWorkspaceInfoForSession(ctx, "task-123", "session-hash-branches")
|
||||
if err != nil {
|
||||
@@ -1110,10 +1118,9 @@ func TestGetWorkspaceInfoForSession_ProjectsOnlyLifecycleValidRepositories(t *te
|
||||
if err := repo.CreateTaskSession(ctx, &models.TaskSession{ID: "session-projection-validity", TaskID: "task-123", TaskEnvironmentID: "env-projection-validity", State: models.TaskSessionStateCompleted, StartedAt: now, UpdatedAt: now}); err != nil {
|
||||
t.Fatalf("CreateTaskSession: %v", err)
|
||||
}
|
||||
createTestEnvironment(t, repo, "env-projection-validity", "task-123")
|
||||
if err := repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{ID: "session-worktree-projection-validity", TaskEnvironmentID: "env-projection-validity", WorktreeID: "worktree-projection-validity", RepositoryID: "repo-provider-recovery", BranchSlug: "main", CreatedAt: now}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironmentRepo: %v", err)
|
||||
}
|
||||
createTestEnvironmentWithRepos(t, repo, "env-projection-validity", "task-123", []*models.TaskEnvironmentRepo{
|
||||
{ID: "session-worktree-projection-validity", TaskEnvironmentID: "env-projection-validity", WorktreeID: "worktree-projection-validity", RepositoryID: "repo-provider-recovery", BranchSlug: "main", CreatedAt: now},
|
||||
})
|
||||
|
||||
info, err := svc.GetWorkspaceInfoForSession(ctx, "task-123", "session-projection-validity")
|
||||
if err != nil {
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
/* eslint-disable max-lines -- pagination, scroll anchoring, and retry state share one boundary. */
|
||||
|
||||
import { useCallback, useEffect, useLayoutEffect, useRef } from "react";
|
||||
import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import { useAppStoreApi } from "@/components/state-provider";
|
||||
@@ -348,6 +350,8 @@ export function useAutoScroll(params: {
|
||||
const storeApi = useAppStoreApi();
|
||||
const isNearBottomRef = useRef(true);
|
||||
const prevIsWorkingRef = useRef(isWorking);
|
||||
const prevEnabledRef = useRef(enabled);
|
||||
const frozenScrollTopRef = useRef<number | null>(null);
|
||||
|
||||
const resyncIsNearBottom = useCallback(() => {
|
||||
const el = scrollRef.current;
|
||||
@@ -374,6 +378,7 @@ export function useAutoScroll(params: {
|
||||
* coalesced persistence of the scroll position. */
|
||||
const onScroll = () => {
|
||||
resyncIsNearBottom();
|
||||
if (!enabled) frozenScrollTopRef.current = el.scrollTop;
|
||||
coalescer.schedule();
|
||||
};
|
||||
el.addEventListener("scroll", onScroll, { passive: true });
|
||||
@@ -385,7 +390,29 @@ export function useAutoScroll(params: {
|
||||
// if a coalesced write above was still pending.
|
||||
coalescer.flush();
|
||||
};
|
||||
}, [scrollRef, sessionId, storeApi, resyncIsNearBottom]);
|
||||
}, [scrollRef, sessionId, storeApi, resyncIsNearBottom, enabled]);
|
||||
|
||||
// Own the disabled offset across every transcript layout update. Sending a
|
||||
// prompt can briefly shrink the scroll range before the new message row is
|
||||
// committed, which makes the browser clamp scrollTop even with
|
||||
// overflow-anchor disabled. Keep the pre-update offset and reapply it after
|
||||
// each message/working-state render so that transient clamp cannot move the
|
||||
// reader. Real user scroll events update the owned offset above.
|
||||
useLayoutEffect(() => {
|
||||
const el = scrollRef.current;
|
||||
if (!el) return;
|
||||
const wasEnabled = prevEnabledRef.current;
|
||||
prevEnabledRef.current = enabled;
|
||||
if (enabled) {
|
||||
frozenScrollTopRef.current = null;
|
||||
return;
|
||||
}
|
||||
if (wasEnabled || frozenScrollTopRef.current === null) {
|
||||
frozenScrollTopRef.current = el.scrollTop;
|
||||
return;
|
||||
}
|
||||
el.scrollTop = frozenScrollTopRef.current;
|
||||
}, [enabled, isWorking, messages, scrollRef]);
|
||||
|
||||
// When isWorking transitions to true, force scroll to bottom (unless
|
||||
// disabled, locked, or a layout rebuild scroll restore is pending).
|
||||
|
||||
@@ -112,11 +112,13 @@ function AutoScrollHarness({
|
||||
hasUnreadDivider,
|
||||
messages = TEST_MESSAGES,
|
||||
markRef,
|
||||
enabled = true,
|
||||
}: {
|
||||
isWorking: boolean;
|
||||
hasUnreadDivider: boolean;
|
||||
messages?: Message[];
|
||||
markRef?: { current?: () => void };
|
||||
enabled?: boolean;
|
||||
}) {
|
||||
const scrollRef = useRef<HTMLDivElement>(null);
|
||||
const { markNotNearBottom } = useAutoScroll({
|
||||
@@ -124,7 +126,7 @@ function AutoScrollHarness({
|
||||
messages,
|
||||
isWorking,
|
||||
sessionId: null,
|
||||
enabled: true,
|
||||
enabled,
|
||||
hasUnreadDivider,
|
||||
isProgrammaticScrollLocked: NEVER_LOCKED,
|
||||
});
|
||||
@@ -405,6 +407,33 @@ describe("useScrollToDividerOrBottom — anchored-bar offset", () => {
|
||||
expect(scrollContainer.scrollTop).toBe(123);
|
||||
});
|
||||
|
||||
it("restores the disabled offset after a transient layout clamp", () => {
|
||||
const { rerender } = render(
|
||||
<AutoScrollHarness isWorking={false} hasUnreadDivider={false} enabled />,
|
||||
);
|
||||
const scrollContainer = document.querySelector<HTMLElement>(
|
||||
'[data-testid="auto-scroll-container"]',
|
||||
);
|
||||
if (!scrollContainer) throw new Error("auto-scroll container did not render");
|
||||
setScrollMetrics(scrollContainer);
|
||||
scrollContainer.scrollTop = 600;
|
||||
|
||||
rerender(<AutoScrollHarness isWorking={false} hasUnreadDivider={false} enabled={false} />);
|
||||
// Model the browser temporarily reducing the maximum scroll offset while
|
||||
// the composer clears, before the appended transcript row is committed.
|
||||
scrollContainer.scrollTop = 568;
|
||||
rerender(
|
||||
<AutoScrollHarness
|
||||
isWorking
|
||||
hasUnreadDivider={false}
|
||||
enabled={false}
|
||||
messages={[...TEST_MESSAGES, {} as Message]}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(scrollContainer.scrollTop).toBe(600);
|
||||
});
|
||||
|
||||
it("never re-scrolls once the reader has started scrolling, even if the anchored bar's height changes afterward", () => {
|
||||
const { rerender, container } = render(<Harness itemCount={2} anchoredBarOffsetPx={0} />);
|
||||
const scrollContainer = container.querySelector<HTMLElement>(
|
||||
|
||||
@@ -254,7 +254,8 @@ describe("useSubtaskSubmit", () => {
|
||||
});
|
||||
|
||||
it("sends the auto-title contract without a title", async () => {
|
||||
const opts = makeSubmitOptions({ autoTitle: true });
|
||||
const onClose = vi.fn();
|
||||
const opts = makeSubmitOptions({ autoTitle: true, onClose });
|
||||
const { result } = renderHook(() => useSubtaskSubmit(opts));
|
||||
|
||||
await act(async () => {
|
||||
@@ -269,6 +270,9 @@ describe("useSubtaskSubmit", () => {
|
||||
expect(mockSetActiveTask).toHaveBeenCalledWith(CREATED_TASK_ID);
|
||||
expect(mockSetActiveSession).toHaveBeenCalledWith(CREATED_TASK_ID, CREATED_SESSION_ID);
|
||||
expect(mockReplaceTaskUrl).toHaveBeenCalledWith(CREATED_TASK_ID);
|
||||
expect(onClose.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
mockReplaceTaskUrl.mock.invocationCallOrder[0],
|
||||
);
|
||||
});
|
||||
|
||||
it("sends the autopilot creation flag for a subtask", async () => {
|
||||
|
||||
@@ -52,6 +52,7 @@ type CreateSubtaskArgs = {
|
||||
workspaceMode: SubtaskWorkspaceMode;
|
||||
isLocalExecutor: boolean;
|
||||
freshBranchEnabled: boolean;
|
||||
onClose: () => void;
|
||||
setActiveTask: (taskId: string) => void;
|
||||
setActiveSession: (taskId: string, sessionId: string) => void;
|
||||
};
|
||||
@@ -71,6 +72,7 @@ async function createSubtask({
|
||||
workspaceMode,
|
||||
isLocalExecutor,
|
||||
freshBranchEnabled,
|
||||
onClose,
|
||||
setActiveTask,
|
||||
setActiveSession,
|
||||
}: CreateSubtaskArgs) {
|
||||
@@ -104,6 +106,9 @@ async function createSubtask({
|
||||
workspace_mode: workspaceMode,
|
||||
autopilot: autopilot || undefined,
|
||||
});
|
||||
// Close the dialog before navigation. Navigation can remount the sidebar
|
||||
// that owns the dialog state, which makes a later close update a stale owner.
|
||||
onClose();
|
||||
const newSessionId = response.session_id ?? response.primary_session_id ?? null;
|
||||
if (newSessionId) {
|
||||
setActiveTask(response.id);
|
||||
@@ -171,10 +176,10 @@ export function useSubtaskSubmit(opts: UseSubtaskSubmitOpts) {
|
||||
workspaceMode,
|
||||
isLocalExecutor,
|
||||
freshBranchEnabled,
|
||||
onClose,
|
||||
setActiveTask,
|
||||
setActiveSession,
|
||||
});
|
||||
onClose();
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: t("task:failedToCreateSubtask"),
|
||||
|
||||
@@ -12,7 +12,8 @@
|
||||
// - Skip leading Git global options when finding the subcommand.
|
||||
// - If KANDEV_E2E_GIT_DELAY_FILE holds a positive ms value and the subcommand
|
||||
// is `fetch`/`pull`, sleep that long before running real git (simulates slow
|
||||
// network git so prepare-panel streaming stays observable).
|
||||
// network git so prepare-panel streaming stays observable). A JSON object
|
||||
// with `startedFile` and `releaseFile` provides a deterministic test gate.
|
||||
// - If the subcommand is `push` and KANDEV_E2E_GITLAB_PUSH_FILE matches the
|
||||
// repo's origin remote, record the push args and exit 0 without pushing.
|
||||
// - Otherwise exec the real git binary with the original args, restoring the
|
||||
@@ -73,9 +74,19 @@ function runRealGit(args, extraEnv) {
|
||||
function maybeDelay(subcommand) {
|
||||
if (subcommand !== "fetch" && subcommand !== "pull") return;
|
||||
const raw = readFileSafe(process.env.KANDEV_E2E_GIT_DELAY_FILE);
|
||||
if (!/^[0-9]+$/.test(raw)) return;
|
||||
const delayMs = Number(raw);
|
||||
if (delayMs > 0) sleepMs(delayMs);
|
||||
if (/^[0-9]+$/.test(raw)) {
|
||||
const delayMs = Number(raw);
|
||||
if (delayMs > 0) sleepMs(delayMs);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const gate = JSON.parse(raw);
|
||||
if (typeof gate.startedFile !== "string" || typeof gate.releaseFile !== "string") return;
|
||||
fs.writeFileSync(gate.startedFile, "started");
|
||||
while (!fs.existsSync(gate.releaseFile)) sleepMs(50);
|
||||
} catch {
|
||||
// Invalid or absent delay configuration means transparent passthrough.
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1299,20 +1299,12 @@ export class SessionPage {
|
||||
.waitFor({ state: "hidden", timeout });
|
||||
}
|
||||
|
||||
/**
|
||||
* Wait for the terminal shell to be connected (buffer has content from
|
||||
* the prompt), then type a command and press Enter.
|
||||
*/
|
||||
/** Wait for the terminal WebSocket to connect, then type a command and press Enter. */
|
||||
async typeInTerminal(command: string): Promise<void> {
|
||||
await this.expectTerminalConnected();
|
||||
await expect
|
||||
.poll(async () => (await this.readXtermBuffer("terminal-panel")).length > 0, {
|
||||
timeout: TERMINAL_READY_TIMEOUT,
|
||||
message: "Waiting for terminal shell to connect",
|
||||
})
|
||||
.toBe(true);
|
||||
|
||||
const xterm = this.activePanel("terminal-panel").locator(".xterm");
|
||||
await expect(xterm).toBeVisible();
|
||||
await xterm.click();
|
||||
await this.page.keyboard.type(command);
|
||||
await this.page.keyboard.press("Enter");
|
||||
|
||||
@@ -59,7 +59,7 @@ export async function seedForkPRComparisonTask(
|
||||
execFileSync(
|
||||
"git",
|
||||
["config", "--global", `url.file://${comparisonTargetPath}.insteadOf`, targetURL],
|
||||
{ env: gitEnv },
|
||||
{ cwd: backend.tmpDir, env: gitEnv },
|
||||
);
|
||||
|
||||
await apiClient.updateRepository(seedData.repositoryId, {
|
||||
@@ -70,24 +70,6 @@ export async function seedForkPRComparisonTask(
|
||||
provider_name: "widget-fork",
|
||||
});
|
||||
|
||||
const task = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Fork PR comparison target",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: "/e2e:simple-message",
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repositories: [
|
||||
{
|
||||
repository_id: seedData.repositoryId,
|
||||
base_branch: "main",
|
||||
checkout_branch: "feature/fork",
|
||||
},
|
||||
],
|
||||
},
|
||||
);
|
||||
|
||||
await apiClient.mockGitHubReset();
|
||||
await apiClient.mockGitHubSetUser("contributor");
|
||||
await apiClient.mockGitHubAddPRs([
|
||||
@@ -127,12 +109,33 @@ export async function seedForkPRComparisonTask(
|
||||
stats_available: true,
|
||||
},
|
||||
]);
|
||||
|
||||
const task = await apiClient.createTask(seedData.workspaceId, "Fork PR comparison target", {
|
||||
description: "/e2e:simple-message",
|
||||
agent_profile_id: seedData.agentProfileId,
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repositories: [
|
||||
{
|
||||
repository_id: seedData.repositoryId,
|
||||
base_branch: "main",
|
||||
checkout_branch: "feature/fork",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await apiClient.associateGitHubTaskPR({
|
||||
workspace_id: seedData.workspaceId,
|
||||
task_id: task.id,
|
||||
repository_id: seedData.repositoryId,
|
||||
pr_url: "https://github.com/upstream/widget/pull/1701",
|
||||
});
|
||||
await apiClient.launchSession({
|
||||
task_id: task.id,
|
||||
agent_profile_id: seedData.agentProfileId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
prompt: "/e2e:simple-message",
|
||||
});
|
||||
|
||||
return { task, headSHA };
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@ import { SessionPage } from "../../pages/session-page";
|
||||
import { seedForkPRComparisonTask } from "./fork-pr-comparison-target-helpers";
|
||||
|
||||
test.describe("Fork pull-request comparison target", () => {
|
||||
test.describe.configure({ retries: 1, timeout: 120_000 });
|
||||
test.describe.configure({ timeout: 120_000 });
|
||||
|
||||
test("uses the upstream target for one fork commit and three files", async ({
|
||||
testPage,
|
||||
|
||||
@@ -17,7 +17,7 @@ async function expandSection(toggle: Locator) {
|
||||
}
|
||||
|
||||
test.describe("Mobile fork pull-request comparison target", () => {
|
||||
test.describe.configure({ retries: 1, timeout: 120_000 });
|
||||
test.describe.configure({ timeout: 120_000 });
|
||||
|
||||
test("shows the same upstream target in the touch branch drawer", async ({
|
||||
testPage,
|
||||
|
||||
@@ -71,10 +71,14 @@ async function openTask(
|
||||
await session.waitForLoad();
|
||||
// The shell hydrates the workspace MR map once per document. A link created
|
||||
// immediately before navigation can miss that first snapshot even though
|
||||
// the task details already show the association. Reload once so the chip
|
||||
// observes the same persisted link as the rest of the task page.
|
||||
await testPage.reload();
|
||||
await session.waitForLoad();
|
||||
// the task details already show the association. Re-drive document
|
||||
// hydration until the linked-MR chip observes the persisted map; one fixed
|
||||
// reload can race the same snapshot again under CI load.
|
||||
await expect(async () => {
|
||||
await testPage.reload();
|
||||
await session.waitForLoad();
|
||||
await expect(session.mrStatusChip()).toBeVisible({ timeout: 5_000 });
|
||||
}).toPass({ timeout: 30_000 });
|
||||
}
|
||||
|
||||
test.describe("GitLab MR status chip", () => {
|
||||
|
||||
@@ -174,6 +174,15 @@ async function moveChangesToChatGroupAndFocusChat(testPage: Page) {
|
||||
}
|
||||
|
||||
test.describe("Changes panel focus behavior", () => {
|
||||
test.beforeEach(({ backend, seedData }) => {
|
||||
// Requesting seedData guarantees that the repository and its offline
|
||||
// origin exist before this hook runs.
|
||||
const git = new GitHelper(seedData.repositoryPath, makeGitEnv(backend.tmpDir));
|
||||
const seedCommit = git.exec("git rev-list --max-parents=0 HEAD").trim();
|
||||
git.exec(`git reset --hard ${seedCommit}`);
|
||||
git.exec("git clean -fd");
|
||||
});
|
||||
|
||||
/**
|
||||
* Verifies the changes panel does NOT steal focus from the chat tab
|
||||
* on page refresh when the task has existing git changes/commits.
|
||||
|
||||
@@ -95,6 +95,11 @@ export async function createKotlinTask(
|
||||
path.join(backend.tmpDir, "repos", options.repositoryDirectory ?? "e2e-repo"),
|
||||
makeGitEnv(backend.tmpDir),
|
||||
);
|
||||
// Task repositories use main unless a test explicitly supplies another
|
||||
// branch. A preceding Git test can leave the shared seed repository checked
|
||||
// out on a feature branch; committing the LSP fixture there makes the task's
|
||||
// main checkout legitimately omit the file.
|
||||
git.exec("git checkout -f main");
|
||||
for (const [index, filePath] of filePaths.entries()) {
|
||||
const isTypeScript = filePath.endsWith(".ts");
|
||||
const defaultContent = isTypeScript
|
||||
@@ -132,6 +137,13 @@ export async function createKotlinTask(
|
||||
);
|
||||
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
|
||||
|
||||
await expect
|
||||
.poll(async () => (await apiClient.getTaskEnvironment(task.id))?.status ?? null, {
|
||||
timeout: 45_000,
|
||||
message: "Kotlin task environment did not become ready",
|
||||
})
|
||||
.toBe("ready");
|
||||
|
||||
const session = new SessionPage(page);
|
||||
if (options.navigate !== false) {
|
||||
await page.goto(`/t/${task.id}`);
|
||||
@@ -259,16 +271,7 @@ export async function openDesktopFile(
|
||||
session: SessionPage,
|
||||
filePath: string,
|
||||
): Promise<void> {
|
||||
await session.clickTab("Files");
|
||||
await expect(session.files).toBeVisible({ timeout: 10_000 });
|
||||
const pathSegments = filePath.split("/");
|
||||
for (let index = 1; index < pathSegments.length; index++) {
|
||||
const ancestor = session.fileTreeNode(pathSegments.slice(0, index).join("/"));
|
||||
await expect(ancestor).toBeVisible({ timeout: 15_000 });
|
||||
if ((await ancestor.locator(".tabler-icon-chevron-right").count()) > 0) {
|
||||
await ancestor.click();
|
||||
}
|
||||
}
|
||||
const fileNode = session.fileTreeNode(filePath);
|
||||
// Git status updates can auto-activate the Changes panel just after the
|
||||
// file tree renders. Re-select Files and click only while the row is still
|
||||
@@ -281,6 +284,14 @@ export async function openDesktopFile(
|
||||
async () => {
|
||||
try {
|
||||
await session.clickTab("Files", { force: true });
|
||||
if (!(await session.files.isVisible())) return false;
|
||||
for (let index = 1; index < pathSegments.length; index++) {
|
||||
const ancestor = session.fileTreeNode(pathSegments.slice(0, index).join("/"));
|
||||
if (!(await ancestor.isVisible())) return false;
|
||||
if ((await ancestor.locator(".tabler-icon-chevron-right").count()) > 0) {
|
||||
await ancestor.click();
|
||||
}
|
||||
}
|
||||
if (!(await fileNode.isVisible())) return false;
|
||||
await fileNode.click({ timeout: 2_000 });
|
||||
return true;
|
||||
|
||||
@@ -32,8 +32,17 @@ test.describe("Mobile plugin task-list facet", () => {
|
||||
|
||||
await testPage.getByRole("button", { name: "Open menu" }).tap();
|
||||
const menu = testPage.getByRole("dialog", { name: "Menu" });
|
||||
await menu.getByTestId("mobile-tasks-list-group").tap();
|
||||
await testPage.getByRole("listbox").getByRole("option", { name: "Fixture tag" }).tap();
|
||||
const groupSelect = menu.getByTestId("mobile-tasks-list-group");
|
||||
await groupSelect.evaluate((element) => element.scrollIntoView({ block: "center" }));
|
||||
await expect(groupSelect).toBeInViewport();
|
||||
const listbox = testPage.getByRole("listbox");
|
||||
await expect(async () => {
|
||||
if (!(await listbox.isVisible())) await groupSelect.tap();
|
||||
const fixtureTag = listbox.getByRole("option", { name: "Fixture tag" });
|
||||
await expect(fixtureTag).toBeInViewport({ timeout: 1_000 });
|
||||
await fixtureTag.click();
|
||||
await expect(groupSelect).toContainText("Fixture tag", { timeout: 1_000 });
|
||||
}).toPass({ timeout: 10_000 });
|
||||
|
||||
const section = testPage.getByTestId("tasks-list-section");
|
||||
await expect(section).toHaveCount(1);
|
||||
|
||||
@@ -201,7 +201,9 @@ test.describe("PR switcher changes panel", () => {
|
||||
|
||||
// --- Switch to Task B ---
|
||||
await session.taskInSidebar("Dashboard Task").click();
|
||||
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
|
||||
await expect(testPage).toHaveURL((url) => url.pathname.includes(taskB.id), {
|
||||
timeout: 15_000,
|
||||
});
|
||||
await session.clickTab("Changes");
|
||||
|
||||
// Wait for PR data to load for Task B
|
||||
@@ -221,16 +223,23 @@ test.describe("PR switcher changes panel", () => {
|
||||
|
||||
// --- Switch to Task C (no PR) ---
|
||||
await session.taskInSidebar("No PR Task").click();
|
||||
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
|
||||
await expect(testPage).toHaveURL((url) => url.pathname.includes(taskC.id), {
|
||||
timeout: 15_000,
|
||||
});
|
||||
await session.clickTab("Changes");
|
||||
|
||||
// PR sections should NOT be visible for a task without a PR
|
||||
// PR-specific content must not leak from the previous task. The unified
|
||||
// commits section can still be present for ordinary local commits, even
|
||||
// when the task has no linked PR.
|
||||
await expect(session.prFilesSection()).not.toBeVisible({ timeout: 10_000 });
|
||||
await expect(session.commitsSection()).not.toBeVisible();
|
||||
await expect(session.changes.getByText("add dashboard component")).not.toBeVisible();
|
||||
await expect(session.changes.getByText("add api client")).not.toBeVisible();
|
||||
|
||||
// --- Switch back to Task A to confirm data reappears ---
|
||||
await session.taskInSidebar("Auth Fix Task").click();
|
||||
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
|
||||
await expect(testPage).toHaveURL((url) => url.pathname.includes(taskA.id), {
|
||||
timeout: 15_000,
|
||||
});
|
||||
await session.clickTab("Changes");
|
||||
|
||||
await expect(session.prFilesSection()).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
@@ -119,7 +119,12 @@ test.describe("Session layout", () => {
|
||||
await sessionB.showSessionContext(30_000);
|
||||
await sessionB.waitForChatIdle({ timeout: 30_000 });
|
||||
|
||||
// Task B should have default (non-maximized) layout
|
||||
// Git updates can legitimately focus Changes while the task settles. The
|
||||
// maximize invariant is about group visibility, so select Files before
|
||||
// asserting the default right-column panel.
|
||||
await sessionB.clickTab("Files");
|
||||
|
||||
// Task B should have default (non-maximized) layout.
|
||||
await sessionB.expectDefaultLayout();
|
||||
|
||||
// Navigate back to Task A's session URL — this triggers a full page load,
|
||||
|
||||
@@ -37,24 +37,24 @@ test.describe("Setup script progress UX", () => {
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
|
||||
// Slow `git fetch` (worktree sync step) to keep the prepare panel reliably
|
||||
// expanded while the page mounts and the WS subscribes — without this the
|
||||
// whole prepare can finish before the client is listening, so events are
|
||||
// missed and the running state is never observed. Same shim pattern as
|
||||
// `long-prepare-panels.spec.ts`. The final `prepare.completed` payload
|
||||
// carries all captured stdout, so the streaming assertion is still valid
|
||||
// even if the page races past the per-progress events.
|
||||
const delayFile = path.join(backend.tmpDir, "git-delay-ms");
|
||||
// Gate the preceding fetch until the browser subscribes, then hold the
|
||||
// setup script so its preparing state and streamed output stay observable.
|
||||
const gateID = Date.now();
|
||||
const gitGateFile = path.join(backend.tmpDir, "git-delay-ms");
|
||||
const gitStartedFile = path.join(backend.tmpDir, `git-started-${gateID}`);
|
||||
const gitReleaseFile = path.join(backend.tmpDir, `git-release-${gateID}`);
|
||||
const startedFile = path.join(backend.tmpDir, `setup-started-${gateID}`);
|
||||
const releaseFile = path.join(backend.tmpDir, `setup-release-${gateID}`);
|
||||
let profile: { id: string } | null = null;
|
||||
try {
|
||||
// Inside the try so the finally always cleans up — if profile creation
|
||||
// throws after the shim is written, leaving the delay file behind would
|
||||
// contaminate every other test in this worker.
|
||||
fs.writeFileSync(delayFile, "5000");
|
||||
|
||||
fs.writeFileSync(
|
||||
gitGateFile,
|
||||
JSON.stringify({ startedFile: gitStartedFile, releaseFile: gitReleaseFile }),
|
||||
);
|
||||
const setupScript = [
|
||||
"echo '[setup] installing deps'",
|
||||
"sleep 1",
|
||||
`touch '${startedFile}'`,
|
||||
`while [ ! -f '${releaseFile}' ]; do sleep 0.1; done`,
|
||||
"echo '[setup] ready'",
|
||||
].join("; ");
|
||||
profile = await createWorktreeProfileWithScript(
|
||||
@@ -80,6 +80,20 @@ test.describe("Setup script progress UX", () => {
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
|
||||
await expect
|
||||
.poll(() => fs.existsSync(gitStartedFile), {
|
||||
message: "repository preparation should reach its deterministic git gate",
|
||||
timeout: 45_000,
|
||||
})
|
||||
.toBe(true);
|
||||
fs.writeFileSync(gitReleaseFile, "release");
|
||||
await expect
|
||||
.poll(() => fs.existsSync(startedFile), {
|
||||
message: "setup script should reach its deterministic test gate",
|
||||
timeout: 45_000,
|
||||
})
|
||||
.toBe(true);
|
||||
|
||||
// Panel appears inline in the chat, expanded while running.
|
||||
const panel = testPage.getByTestId("prepare-progress-panel");
|
||||
await expect(panel).toBeVisible({ timeout: 15_000 });
|
||||
@@ -90,17 +104,25 @@ test.describe("Setup script progress UX", () => {
|
||||
// Setup script output reaches the expanded step list — either streamed
|
||||
// in real time or captured from the final `prepare.completed` payload.
|
||||
await expect(panel).toContainText("[setup] installing deps", { timeout: 30_000 });
|
||||
fs.writeFileSync(releaseFile, "release");
|
||||
|
||||
// On clean success the panel stays visible but auto-collapses.
|
||||
await expect(panel).toHaveAttribute("data-status", "completed", { timeout: 30_000 });
|
||||
await expect(panel).toHaveAttribute("data-expanded", "false");
|
||||
} finally {
|
||||
if (fs.existsSync(delayFile)) fs.unlinkSync(delayFile);
|
||||
// Always release a setup process that is still waiting before teardown.
|
||||
if (!fs.existsSync(gitReleaseFile)) fs.writeFileSync(gitReleaseFile, "release");
|
||||
if (!fs.existsSync(releaseFile)) fs.writeFileSync(releaseFile, "release");
|
||||
if (profile) {
|
||||
await apiClient.deleteExecutorProfile(profile.id).catch(() => {
|
||||
// Profile may already be deleted if the test tore down mid-run.
|
||||
});
|
||||
}
|
||||
if (fs.existsSync(gitGateFile)) fs.unlinkSync(gitGateFile);
|
||||
if (fs.existsSync(gitStartedFile)) fs.unlinkSync(gitStartedFile);
|
||||
if (fs.existsSync(gitReleaseFile)) fs.unlinkSync(gitReleaseFile);
|
||||
if (fs.existsSync(startedFile)) fs.unlinkSync(startedFile);
|
||||
if (fs.existsSync(releaseFile)) fs.unlinkSync(releaseFile);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -15,6 +15,10 @@ test.describe("Task creation with branch policies", () => {
|
||||
backend,
|
||||
seedData,
|
||||
}) => {
|
||||
execSync("git clean -fd", {
|
||||
cwd: seedData.repositoryPath,
|
||||
env: makeGitEnv(backend.tmpDir),
|
||||
});
|
||||
execSync("git branch -f develop", {
|
||||
cwd: seedData.repositoryPath,
|
||||
env: makeGitEnv(backend.tmpDir),
|
||||
@@ -44,6 +48,9 @@ test.describe("Task creation with branch policies", () => {
|
||||
await expect(dialog).toBeVisible();
|
||||
await dialog.getByTestId("executor-profile-selector").click();
|
||||
await testPage.getByRole("option", { name: new RegExp(localProfile.name) }).click();
|
||||
await expect(dialog.getByTestId("executor-profile-selector")).toContainText(
|
||||
localProfile.name,
|
||||
);
|
||||
await dialog.getByTestId("branch-chip-trigger").click();
|
||||
const option = testPage.getByRole("option", { name: new RegExp(policy.name) });
|
||||
await expect(option).toContainText("Policy");
|
||||
@@ -55,7 +62,9 @@ test.describe("Task creation with branch policies", () => {
|
||||
);
|
||||
await testPage.mouse.move(0, 0);
|
||||
await policyInfo.focus();
|
||||
await expect(testPage.getByRole("tooltip")).toContainText(
|
||||
await expect(policyInfo).toBeFocused();
|
||||
await expect(policyInfo).toHaveAttribute(
|
||||
"aria-label",
|
||||
"Base: main. Template: feature/{title}-{suffix}. Pull request target: develop.",
|
||||
);
|
||||
await option.click();
|
||||
@@ -70,7 +79,7 @@ test.describe("Task creation with branch policies", () => {
|
||||
await dialog.getByTestId("task-description-input").fill("Create from a branch policy");
|
||||
await dialog.getByTestId("submit-start-agent-chevron").click();
|
||||
await testPage.getByTestId("submit-create-without-agent").click();
|
||||
await expect(dialog).not.toBeVisible();
|
||||
await expect(dialog).not.toBeVisible({ timeout: 30_000 });
|
||||
|
||||
let created: { id: string; title: string } | undefined;
|
||||
await expect
|
||||
@@ -160,6 +169,9 @@ test.describe("Task creation with branch policies", () => {
|
||||
await expect(dialog).toBeVisible();
|
||||
await dialog.getByTestId("executor-profile-selector").click();
|
||||
await testPage.getByRole("option", { name: new RegExp(localProfile.name) }).click();
|
||||
await expect(dialog.getByTestId("executor-profile-selector")).toContainText(
|
||||
localProfile.name,
|
||||
);
|
||||
|
||||
await dialog.getByTestId("add-repository").click();
|
||||
const repositoryChips = dialog.getByTestId("repo-chip-trigger");
|
||||
|
||||
@@ -39,25 +39,39 @@ test.describe("Task branch policy selection on mobile", () => {
|
||||
await mobile.mobileFab.tap();
|
||||
const dialog = testPage.getByTestId("create-task-dialog");
|
||||
await expect(dialog).toBeVisible();
|
||||
await dialog.getByTestId("executor-profile-selector").tap();
|
||||
await testPage.getByRole("option", { name: new RegExp(localProfile.name) }).tap();
|
||||
await dialog.getByTestId("branch-chip-trigger").tap();
|
||||
const option = testPage.getByRole("option", { name: new RegExp(policy.name) });
|
||||
const executorSelector = dialog.getByTestId("executor-profile-selector");
|
||||
await expect(async () => {
|
||||
await executorSelector.tap();
|
||||
await testPage.getByRole("option", { name: new RegExp(localProfile.name) }).click();
|
||||
await expect(executorSelector).toContainText(localProfile.name, { timeout: 1_000 });
|
||||
}).toPass({ timeout: 10_000 });
|
||||
const branchTrigger = dialog.getByTestId("branch-chip-trigger");
|
||||
await branchTrigger.tap();
|
||||
const policyOption = () => testPage.getByRole("option", { name: new RegExp(policy.name) });
|
||||
const option = policyOption();
|
||||
await expect(option).toContainText("Policy");
|
||||
await expect(option).not.toHaveAttribute("aria-disabled", "true", { timeout: 30_000 });
|
||||
await expectPolicyOptionUsesOneLine(option, policy.name);
|
||||
await testPage.getByTestId(`branch-policy-option-info-${policy.id}`).tap();
|
||||
await expect(testPage.getByRole("dialog", { name: policy.name })).toContainText(
|
||||
"Base: main. Template: mobile/{title}-{suffix}. Pull request target: main.",
|
||||
);
|
||||
await testPage.keyboard.press("Escape");
|
||||
await dialog.getByTestId("branch-chip-trigger").tap();
|
||||
const reopenedOption = testPage.getByRole("option", { name: new RegExp(policy.name) });
|
||||
await reopenedOption.tap({ force: true });
|
||||
await expect(dialog.getByTestId("branch-chip-trigger")).toContainText(policy.name);
|
||||
await expect(async () => {
|
||||
if (!(await policyOption().isVisible())) await branchTrigger.tap();
|
||||
await policyOption().evaluate((element) => (element as HTMLElement).click());
|
||||
await expect(branchTrigger).toContainText(policy.name, { timeout: 1_000 });
|
||||
}).toPass({ timeout: 10_000 });
|
||||
await expect(dialog.getByTestId("fresh-branch-toggle")).toHaveAttribute(
|
||||
"aria-pressed",
|
||||
"true",
|
||||
);
|
||||
|
||||
await branchTrigger.tap();
|
||||
const policyDetails = testPage.getByRole("dialog", { name: policy.name });
|
||||
await expect(async () => {
|
||||
if (!(await policyOption().isVisible())) await branchTrigger.tap();
|
||||
await testPage.getByTestId(`branch-policy-option-info-${policy.id}`).dispatchEvent("click");
|
||||
await expect(policyDetails).toBeVisible({ timeout: 1_000 });
|
||||
}).toPass({ timeout: 10_000 });
|
||||
await expect(policyDetails).toContainText(
|
||||
"Base: main. Template: mobile/{title}-{suffix}. Pull request target: main.",
|
||||
);
|
||||
expect(
|
||||
await testPage.evaluate(() => document.documentElement.scrollWidth),
|
||||
).toBeLessThanOrEqual(390);
|
||||
|
||||
@@ -863,8 +863,14 @@ test.describe("Mobile sidebar task actions", () => {
|
||||
|
||||
const dialog = testPage.getByTestId("new-subtask-dialog");
|
||||
await dialog.getByTestId("subtask-workspace-mode-new").tap();
|
||||
await dialog.getByTestId("executor-profile-selector").tap();
|
||||
await testPage.getByRole("option", { name: new RegExp(localProfile.name) }).tap();
|
||||
const executorSelector = dialog.getByTestId("executor-profile-selector");
|
||||
await expect(async () => {
|
||||
await executorSelector.tap();
|
||||
await testPage
|
||||
.getByRole("option", { name: new RegExp(localProfile.name) })
|
||||
.tap({ force: true });
|
||||
await expect(executorSelector).toContainText(localProfile.name, { timeout: 1_000 });
|
||||
}).toPass({ timeout: 10_000 });
|
||||
await dialog.getByTestId("branch-chip-trigger").tap();
|
||||
await testPage.getByRole("option", { name: new RegExp(policy.name) }).tap({ force: true });
|
||||
await expect(dialog.getByTestId("fresh-branch-toggle")).toHaveAttribute(
|
||||
@@ -873,7 +879,10 @@ test.describe("Mobile sidebar task actions", () => {
|
||||
);
|
||||
await dialog.getByTestId("subtask-title-input").fill(childTitle);
|
||||
await dialog.getByTestId("subtask-prompt-input").fill("/e2e:simple-message");
|
||||
await dialog.getByRole("button", { name: "Create Subtask", exact: true }).tap();
|
||||
const createSubtask = dialog.getByRole("button", { name: "Create Subtask", exact: true });
|
||||
await expect(executorSelector).toContainText(localProfile.name);
|
||||
await expect(createSubtask).toBeEnabled();
|
||||
await createSubtask.tap();
|
||||
await expect(dialog).toBeHidden({ timeout: 30_000 });
|
||||
let childId: string | undefined;
|
||||
await expect
|
||||
|
||||
@@ -82,6 +82,11 @@ test.describe("Mobile task listing display preferences", () => {
|
||||
await testPage.getByRole("button", { name: "Open menu" }).click();
|
||||
const tasksMenu = testPage.getByRole("dialog", { name: "Menu" });
|
||||
await tasksMenu.getByText("Show task details", { exact: true }).click();
|
||||
await expect
|
||||
.poll(async () => (await apiClient.getUserSettings()).settings.tasks_list_show_details, {
|
||||
message: "task detail preference was not persisted",
|
||||
})
|
||||
.toBe(true);
|
||||
await testPage.keyboard.press("Escape");
|
||||
await expect(tasksMenu).toHaveCount(0);
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
createStandardProfile,
|
||||
} from "../../helpers/git-helper";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
import { waitForLatestSessionDone } from "../../helpers/session";
|
||||
|
||||
/**
|
||||
* Regression: switching from a task with env-scoped panels open (file-editor,
|
||||
@@ -36,12 +37,16 @@ async function setupTaskWithFilePanel(args: {
|
||||
makeGitEnv(args.backendTmpDir),
|
||||
);
|
||||
const filename = "leak-fixture.ts";
|
||||
// Local-executor specs can leave the worker-scoped seed clone on a task
|
||||
// branch. Make this fixture independent of file execution order.
|
||||
git.exec("git checkout main");
|
||||
git.createFile(filename, "// leak fixture\n");
|
||||
git.stageAll();
|
||||
git.commit("seed leak fixture");
|
||||
git.exec("git push origin main");
|
||||
|
||||
const profile = await createStandardProfile(args.apiClient, "panel-leak-source");
|
||||
await args.apiClient.createTaskWithAgent(
|
||||
const task = await args.apiClient.createTaskWithAgent(
|
||||
args.seedData.workspaceId,
|
||||
"Panel Leak Source",
|
||||
profile.id,
|
||||
@@ -52,6 +57,12 @@ async function setupTaskWithFilePanel(args: {
|
||||
repository_ids: [args.seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
await waitForLatestSessionDone(
|
||||
args.apiClient,
|
||||
task.id,
|
||||
1,
|
||||
"source task did not finish preparing its workspace",
|
||||
);
|
||||
|
||||
const session = await openTaskSession(args.testPage, "Panel Leak Source");
|
||||
await session.clickTab("Files");
|
||||
@@ -87,8 +98,6 @@ const NON_DEFAULT_ENV_SCOPED_COMPONENTS = [
|
||||
];
|
||||
|
||||
test.describe("Sessionless task switch — env-scoped panel cleanup", () => {
|
||||
test.describe.configure({ retries: 1 });
|
||||
|
||||
test("dropping into a sessionless task releases the previous task's env-scoped panels", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
|
||||
@@ -303,8 +303,12 @@ test.describe("Subtask basics", () => {
|
||||
await dialog.getByTestId("subtask-workspace-mode-new").click();
|
||||
await dialog.getByTestId("executor-profile-selector").click();
|
||||
await testPage.getByRole("option", { name: new RegExp(localProfile.name) }).click();
|
||||
await expect(dialog.getByTestId("executor-profile-selector")).toContainText(
|
||||
localProfile.name,
|
||||
);
|
||||
await dialog.getByTestId("branch-chip-trigger").click();
|
||||
await testPage.getByRole("option", { name: new RegExp(policy.name) }).click();
|
||||
await expect(dialog.getByTestId("branch-chip-trigger")).toContainText(policy.name);
|
||||
await expect(dialog.getByTestId("fresh-branch-toggle")).toHaveAttribute(
|
||||
"aria-pressed",
|
||||
"true",
|
||||
|
||||
Reference in New Issue
Block a user