From 32b81c2f2d39bab50eebf74abfb02e7dce8623b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 26 Aug 2026 07:11:38 +0000 Subject: [PATCH] fix: gate focus auto-start on task dependencies (#3045) --- .../internal/orchestrator/session_launch.go | 8 +- .../session_launch_dependency_test.go | 202 ++++++++++++++++++ .../workflow/workflow-automation.spec.ts | 26 +-- .../focus-autostart-dependency-gate/plan.md | 82 +++++++ .../task-01-gate-focus-autostart.md | 122 +++++++++++ .../tasks/system-design/task-dependencies.md | 9 +- 6 files changed, 429 insertions(+), 20 deletions(-) create mode 100644 apps/backend/internal/orchestrator/session_launch_dependency_test.go create mode 100644 docs/plans/focus-autostart-dependency-gate/plan.md create mode 100644 docs/plans/focus-autostart-dependency-gate/task-01-gate-focus-autostart.md diff --git a/apps/backend/internal/orchestrator/session_launch.go b/apps/backend/internal/orchestrator/session_launch.go index 62da2ad603..406c997ac0 100644 --- a/apps/backend/internal/orchestrator/session_launch.go +++ b/apps/backend/internal/orchestrator/session_launch.go @@ -240,10 +240,12 @@ func (s *Service) isPassthroughProfile(ctx context.Context, profileID string) bo // launchStart creates a new session and launches the agent. // If the request is an auto-start and the task's current workflow step does not -// have auto_start_agent, the request is downgraded to a prepare (workspace-only, -// no agent) to prevent unwanted auto-starts from the frontend's useAutoStartSession hook. +// have auto_start_agent, or the task has unresolved dependencies, the request +// is downgraded to a prepare (workspace-only, no agent) to prevent unwanted +// auto-starts from the frontend's useAutoStartSession hook. func (s *Service) launchStart(ctx context.Context, req *LaunchSessionRequest) (*LaunchSessionResponse, error) { - if req.AutoStart && s.shouldBlockAutoStart(ctx, req) { + if req.AutoStart && (s.shouldBlockAutoStart(ctx, req) || + s.dependencyBlocksAutoStart(ctx, req.TaskID, "session.launch")) { req.LaunchWorkspace = true return s.launchPrepare(ctx, req) } diff --git a/apps/backend/internal/orchestrator/session_launch_dependency_test.go b/apps/backend/internal/orchestrator/session_launch_dependency_test.go new file mode 100644 index 0000000000..de4f871dfe --- /dev/null +++ b/apps/backend/internal/orchestrator/session_launch_dependency_test.go @@ -0,0 +1,202 @@ +package orchestrator + +import ( + "context" + "errors" + "sync/atomic" + "testing" + "time" + + "github.com/kandev/kandev/internal/orchestrator/executor" + "github.com/kandev/kandev/internal/task/models" + taskservice "github.com/kandev/kandev/internal/task/service" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +type launchDependencyReader struct { + blocked bool + err error +} + +func (r *launchDependencyReader) DependencyGate(context.Context, string) (bool, string, error) { + return r.blocked, "pending", r.err +} + +func (r *launchDependencyReader) ListDependentTaskIDs(context.Context, string) ([]string, error) { + return nil, nil +} + +func (r *launchDependencyReader) ListPendingDependencyLaunches( + context.Context, +) ([]taskservice.PendingDependencyLaunch, error) { + return nil, nil +} + +// @covers AC-TASKS-TASK-DEPENDENCIES-001.1 +func TestLaunchSession_AutoStartDependencyGate(t *testing.T) { + tests := []struct { + name string + blocked bool + dependencyError error + wantPrepareOnly bool + }{ + { + name: "unresolved dependency prepares workspace only", + blocked: true, + wantPrepareOnly: true, + }, + { + name: "resolved dependency follows normal launch path", + }, + { + name: "dependency read error prepares workspace only", + dependencyError: errors.New("dependency store unavailable"), + wantPrepareOnly: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + const taskID = "task-focus-gate" + ctx := context.Background() + repo := setupTestRepo(t) + seedAutoStartGateTask(t, repo, taskID) + + taskRepo := newMockTaskRepo() + taskRepo.tasks[taskID] = &v1.Task{ + ID: taskID, + WorkspaceID: "ws1", + Title: "Focus gate task", + Description: "run the task", + State: v1.TaskStateCreated, + } + + var agentLaunches atomic.Int32 + workspaceLaunchDone := make(chan struct{}) + agentProcessStarted := make(chan struct{}) + agentManager := &mockAgentManager{ + launchAgentFunc: func( + _ context.Context, req *executor.LaunchAgentRequest, + ) (*executor.LaunchAgentResponse, error) { + if req.StartAgent { + agentLaunches.Add(1) + } else { + close(workspaceLaunchDone) + } + return &executor.LaunchAgentResponse{ + AgentExecutionID: "exec-focus-gate", + Status: v1.AgentStatusStarting, + }, nil + }, + startAgentProcessFunc: func(context.Context, string) error { + close(agentProcessStarted) + return nil + }, + } + svc := createTestServiceWithScheduler( + repo, newMockStepGetter(), taskRepo, agentManager, + ) + svc.SetTaskDependencyReader(&launchDependencyReader{ + blocked: tt.blocked, + err: tt.dependencyError, + }) + + response, err := svc.LaunchSession(ctx, &LaunchSessionRequest{ + TaskID: taskID, + Intent: IntentStart, + AgentProfileID: "profile-focus-gate", + AutoStart: true, + }) + if err != nil { + t.Fatalf("LaunchSession returned error: %v", err) + } + if response == nil { + t.Fatal("LaunchSession returned nil response") + } + + if tt.wantPrepareOnly { + if response.State != string(models.TaskSessionStateCreated) { + t.Fatalf("response state = %q, want CREATED", response.State) + } + if response.SessionID == "" { + t.Fatal("prepare-only response did not include a session ID") + } + if got := agentLaunches.Load(); got != 0 { + t.Fatalf("agent launches = %d, want 0", got) + } + waitForWorkspaceLaunch(t, workspaceLaunchDone) + assertTaskWasNotScheduled(t, repo, taskRepo, taskID) + return + } + + if got := agentLaunches.Load(); got != 1 { + t.Fatalf("agent launches = %d, want 1", got) + } + if response.AgentExecutionID == "" { + t.Fatal("normal launch response did not include an agent execution ID") + } + select { + case <-agentProcessStarted: + case <-time.After(2 * time.Second): + t.Fatal("normal launch did not start the agent process") + } + }) + } +} + +func seedAutoStartGateTask(t *testing.T, repo interface { + CreateWorkspace(context.Context, *models.Workspace) error + CreateWorkflow(context.Context, *models.Workflow) error + CreateTask(context.Context, *models.Task) error +}, taskID string) { + t.Helper() + ctx := context.Background() + now := time.Now().UTC() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ + ID: "ws1", Name: "Focus gate workspace", CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("create workspace: %v", err) + } + if err := repo.CreateWorkflow(ctx, &models.Workflow{ + ID: "wf1", WorkspaceID: "ws1", Name: "Focus gate workflow", CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("create workflow: %v", err) + } + if err := repo.CreateTask(ctx, &models.Task{ + ID: taskID, WorkspaceID: "ws1", WorkflowID: "wf1", + Title: "Focus gate task", Description: "run the task", State: v1.TaskStateCreated, + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("create task: %v", err) + } +} + +func waitForWorkspaceLaunch(t *testing.T, launched <-chan struct{}) { + t.Helper() + select { + case <-launched: + case <-time.After(2 * time.Second): + t.Fatal("workspace-only prepare did not launch workspace") + } +} + +func assertTaskWasNotScheduled(t *testing.T, repo interface { + GetTask(context.Context, string) (*models.Task, error) +}, taskRepo *mockTaskRepo, taskID string) { + t.Helper() + task, err := repo.GetTask(context.Background(), taskID) + if err != nil { + t.Fatalf("load task: %v", err) + } + if task.State == v1.TaskStateScheduling || task.State == v1.TaskStateInProgress { + t.Fatalf("task state = %q, must not be scheduled or in progress", task.State) + } + + taskRepo.mu.Lock() + defer taskRepo.mu.Unlock() + for _, state := range taskRepo.stateHistory[taskID] { + if state == v1.TaskStateScheduling || state == v1.TaskStateInProgress { + t.Fatalf("task state history contains %q", state) + } + } +} diff --git a/apps/web/e2e/tests/workflow/workflow-automation.spec.ts b/apps/web/e2e/tests/workflow/workflow-automation.spec.ts index 65dc1cf172..d197a31846 100644 --- a/apps/web/e2e/tests/workflow/workflow-automation.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-automation.spec.ts @@ -362,23 +362,23 @@ test.describe("Workflow automation", () => { * Kanban board workflow with on_turn_start: move_to_next on Backlog. * * Workflow: - * Backlog (on_turn_start: move_to_next, on_turn_complete: move_to_step("review")) - * In Progress (on_enter: auto_start_agent, on_turn_complete: move_to_step("review")) + * Backlog (on_turn_start: move_to_next, on_turn_complete: malformed move_to_step) + * In Progress (on_enter: auto_start_agent, on_turn_complete: malformed move_to_step) * Review (on_turn_start: move_to_previous) * Done (on_turn_start: move_to_step("in-progress")) * * No step has is_start_step, so the task lands at position 0 (Backlog). - * The on_turn_complete events use template-level step_id references ("review", - * "in-progress") which don't resolve to actual step UUIDs — they're no-ops. + * The on_turn_complete events use malformed step_position-only configs, which + * the runtime skips because move_to_step requires a step_id. * * Flow: * 1. Create task in Backlog (no start_agent) and navigate to the task page * 2. Send a chat message — only the WS message path runs on_turn_start via * dispatchPromptAsync, so the Backlog → In Progress move triggers here - * 3. Agent completes → on_turn_complete fires with move_to_step("review") - * which is gracefully skipped (invalid step_id) → task stays in In Progress + * 3. Agent completes → malformed on_turn_complete move_to_step is skipped + * → task stays in In Progress */ - test("kanban workflow: on_turn_start moves task, invalid step_id in on_turn_complete is skipped", async ({ + test("kanban workflow: on_turn_start moves task, malformed on_turn_complete is skipped", async ({ testPage, apiClient, seedData, @@ -396,18 +396,18 @@ test.describe("Workflow automation", () => { const inProgressStep = steps.find((s) => s.name === "In Progress")!; // Backlog: on_turn_start moves to next (In Progress), - // on_turn_complete uses template-level step_id (no-op in API-created steps) + // on_turn_complete has a malformed step_position-only config and is skipped. await apiClient.updateWorkflowStep(backlogStep.id, { events: { on_turn_start: [{ type: "move_to_next" }], - on_turn_complete: [{ type: "move_to_step", config: { step_id: "review" } }], + on_turn_complete: [{ type: "move_to_step", config: { step_position: 99 } }], }, }); - // In Progress: on_turn_complete with invalid step_id (should be skipped) + // In Progress: malformed on_turn_complete move_to_step (should be skipped) await apiClient.updateWorkflowStep(inProgressStep.id, { events: { - on_turn_complete: [{ type: "move_to_step", config: { step_id: "review" } }], + on_turn_complete: [{ type: "move_to_step", config: { step_position: 99 } }], }, }); @@ -441,13 +441,13 @@ test.describe("Workflow automation", () => { }); // Agent response confirms the mock agent ran; on_turn_complete then fires - // with move_to_step("review") — invalid step_id → gracefully skipped. + // with malformed move_to_step — gracefully skipped because step_id is missing. await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible({ timeout: 30_000, }); await expect(session.idleInput()).toBeVisible({ timeout: 15_000 }); - // Task stays in In Progress on the kanban board (invalid step_id is a no-op). + // Task stays in In Progress on the kanban board (malformed action is a no-op). const kanban = new KanbanPage(testPage); await kanban.goto(); const card = kanban.taskCardInColumn("Kanban Flow Task", inProgressStep.id); diff --git a/docs/plans/focus-autostart-dependency-gate/plan.md b/docs/plans/focus-autostart-dependency-gate/plan.md new file mode 100644 index 0000000000..0d9ad5b205 --- /dev/null +++ b/docs/plans/focus-autostart-dependency-gate/plan.md @@ -0,0 +1,82 @@ +--- +created: 2026-08-25 +status: done +requirements: + - REQ-TASKS-TASK-DEPENDENCIES-001 +system_design: + - ../../specs/tasks/system-design/task-dependencies.md +legacy_specs: [] +--- + +# Implementation Plan: Focus Auto-Start Dependency Gate + +## Overview + +Repair the `session.ensure` focus path so every `LaunchSession` request marked +as an automatic start observes the existing task-dependency gate. Add the +regression test first, then extend `launchStart`'s existing downgrade-to-prepare +condition without changing manual starts or dependency-resolution behavior. + +## Scope + +### In scope + +- Gate `LaunchSession` requests with `IntentStart` and `AutoStart: true` on + unresolved task dependencies. +- Fail closed when the dependency gate cannot be read. +- Preserve the workspace-only `CREATED` session used to inspect a blocked task. +- Preserve normal automatic starts after dependencies resolve. + +### Out of scope + +- Manual Start agent actions and direct manual `StartTask` calls. +- Existing dependency-resolution, deferred-launch, and WIP-promotion behavior. +- Frontend changes, API changes, persistence changes, and browser E2E coverage. + +## Technical approach + +Add the dependency predicate to the existing auto-start downgrade in +`apps/backend/internal/orchestrator/session_launch.go`. The request remains an +automatic request while `launchPrepare` runs, which suppresses passthrough +prepare upgrades and prevents a bounce back into `launchStart`. + +Add focused table-driven coverage in a new orchestrator test file. Use a +minimal configurable `TaskDependencyReader` fake and the established launch +counter/service wiring to prove blocked and read-error requests prepare only, +while an unblocked request reaches the agent launch path. + +## Tests + +- `AC-TASKS-TASK-DEPENDENCIES-001.1`: `TestLaunchSession_AutoStartDependencyGate` + covers an unresolved dependency, a resolved dependency, and a dependency-read + error at the `LaunchSession` boundary. +- The blocked and read-error cases assert a `CREATED` workspace-only session, + no agent launch, and no transition to `SCHEDULING` or `IN_PROGRESS`. +- The unblocked case asserts the normal launch path still reaches a running + agent session. + +## Work orders + +- [x] [Task 01: Gate focus-induced auto-start](task-01-gate-focus-autostart.md) + +## Verification results + +- Focused regression passed: + `go test -tags fts5 ./internal/orchestrator -run '^TestLaunchSession_AutoStartDependencyGate$' -count=1`. +- Focused race regression passed: + `go test -race -tags fts5 ./internal/orchestrator -run '^TestLaunchSession_AutoStartDependencyGate$' -count=1`. +- `make -C apps/backend lint` passed with zero issues. +- `gofmt -l` reported no changes for the modified Go files. +- `make -C apps/backend test` passed for `internal/orchestrator` and the other + packages reached before two unrelated environment-sensitive packages failed. + `internal/common/config` and `internal/launcher` selected the existing + `/root/.kandev/config.yaml` instead of each test's temporary home. + +## Risks + +- The downgrade must retain `AutoStart: true`; clearing it would allow a + passthrough prepare to upgrade back into an agent start. +- The dependency reader is optional in some test/service constructions, so the + nil-reader behavior must remain unchanged. +- Broad backend validation can expose unrelated repository failures; targeted + orchestrator evidence remains the regression proof. diff --git a/docs/plans/focus-autostart-dependency-gate/task-01-gate-focus-autostart.md b/docs/plans/focus-autostart-dependency-gate/task-01-gate-focus-autostart.md new file mode 100644 index 0000000000..c7e6f8dce2 --- /dev/null +++ b/docs/plans/focus-autostart-dependency-gate/task-01-gate-focus-autostart.md @@ -0,0 +1,122 @@ +--- +id: "01-gate-focus-autostart" +title: "Gate focus-induced auto-start" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-TASK-DEPENDENCIES-001 +acceptance_criteria: + - AC-TASKS-TASK-DEPENDENCIES-001.1 +system_design: + - ../../specs/tasks/system-design/task-dependencies.md +--- + +# Task 01: Gate Focus-Induced Auto-Start + +## Summary + +Make `launchStart` apply the task dependency gate to automatic starts before it +can call `startTask`. Blocked and unreadable dependency state downgrade to the +existing workspace-only prepare flow, while an unblocked task still starts. + +## In scope + +- Add focused `LaunchSession` regression coverage for blocked, unblocked, and + dependency-read-error cases. +- Extend `launchStart`'s `req.AutoStart` downgrade condition with + `dependencyBlocksAutoStart(ctx, req.TaskID, "session.launch")`. +- Preserve the request fields needed by `launchPrepare` and its passthrough + recursion guard. + +## Out of scope + +- Gating manual starts where `AutoStart` is false. +- Changing `autoStartTaskForStep`, dependency resolution, WIP admission, or the + frontend `session.ensure` message. +- Adding a browser test for a backend launch-admission regression. + +## Acceptance + +- A blocked automatic `IntentStart` creates only a `CREATED` workspace session, + never launches the agent, and never moves the task to `SCHEDULING` or + `IN_PROGRESS`. +- A dependency-read error has the same fail-closed prepare-only result. +- An unblocked automatic `IntentStart` follows the existing normal start path. + +## TDD sequence + +1. Add `TestLaunchSession_AutoStartDependencyGate` with a nearby + `@covers AC-TASKS-TASK-DEPENDENCIES-001.1` annotation and three cases: + unresolved, resolved, and read error. +2. Run only that test. Confirm the unresolved and read-error cases fail because + an agent starts or the task leaves `CREATED`; confirm the resolved positive + control reaches the existing launch path. +3. Add the minimum dependency predicate to `launchStart`'s current automatic + downgrade condition. +4. Rerun the focused test and keep all three cases green. Refactor only the test + setup if needed for clarity, then rerun it after the final production edit. + +## Verification + +```bash +cd apps/backend +go test -tags fts5 ./internal/orchestrator -run '^TestLaunchSession_AutoStartDependencyGate$' -count=1 +cd ../.. +make -C apps/backend test +make -C apps/backend lint +gofmt -l apps/backend/internal/orchestrator/session_launch.go apps/backend/internal/orchestrator/session_launch_dependency_test.go +git diff --cached --name-only | grep '\.go$' | xargs -r gofmt -l +``` + +If either formatting command reports a file, run `gofmt -w` on that exact file, +restage it, and repeat both checks before committing. + +## Files likely touched + +- `apps/backend/internal/orchestrator/session_launch.go` +- `apps/backend/internal/orchestrator/session_launch_dependency_test.go` + +## Dependencies + +None. + +## Risks + +- A test that observes only a session row could miss an agent launch; assert + both persisted state and the launch counter. +- The resolved positive control must use the same harness so a no-op setup + cannot make every case appear gated. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-TASKS-TASK-DEPENDENCIES-001` and + `AC-TASKS-TASK-DEPENDENCIES-001.1`. +- `docs/specs/tasks/system-design/task-dependencies.md`, especially automated + launch guards and fail-closed behavior. +- `apps/backend/internal/orchestrator/session_launch.go` and existing + orchestrator dependency/launch test helpers. + +## Results + +Implemented the dependency gate in `launchStart` and added +`TestLaunchSession_AutoStartDependencyGate`. The regression covers unresolved +dependencies, resolved dependencies, and dependency-read errors at the +`LaunchSession` boundary. Blocked and unreadable dependency state now keeps the +request on the workspace-only `CREATED` path; resolved state still launches. + +Validation: + +- Focused test passed. +- Focused race test passed. +- Backend lint passed with zero issues. +- `gofmt -l` reported no changes. +- The full backend target reached and passed `internal/orchestrator`, but the + target returned failure because unrelated config-discovery tests in + `internal/common/config` and `internal/launcher` read the existing + `/root/.kandev/config.yaml` instead of their temporary homes. diff --git a/docs/specs/tasks/system-design/task-dependencies.md b/docs/specs/tasks/system-design/task-dependencies.md index ddba583052..a1af4e5ffa 100644 --- a/docs/specs/tasks/system-design/task-dependencies.md +++ b/docs/specs/tasks/system-design/task-dependencies.md @@ -101,7 +101,7 @@ parallel ones. The reuse is part of the contract because the guarantees below |---|---| | `task_blockers` table + BFS cycle detection (`office/dashboard.AddTaskBlocker`) | The dependency edge store and cycle validator, promoted from Office-only to a core task relationship. | | Deferred launch intent (`tasks.metadata.deferred_launch`, claimed atomically) | Storage and idempotent consumption of "start automatically when unblocked". | -| The single auto-start chokepoint (`orchestrator.autoStartTaskForStep`) | Where the dependency gate lives and where dependency-triggered launches enter, so the existing auto-start claim guard prevents double launches. | +| Automated launch guards (`orchestrator.autoStartTaskForStep` and `orchestrator.launchStart`) | `autoStartTaskForStep` gates workflow, queue, watcher, and dependency-resolution starts before claims. `launchStart` gates `LaunchSession` auto-starts, including `session.ensure`, and downgrades blocked requests to workspace-only prepare. | | WIP admission and queue promotion ([`tasks/wip-limit-pull-system`](wip-limit-pull-system.md)) | Decides whether an unblocked, auto-start-intent task launches now or on promotion. | Two existing behaviors are intentionally *not* changed: @@ -419,8 +419,9 @@ Edges and blocking: Gating: - **GIVEN** B depends on an unfinished A and B's workflow step has - `on_enter: auto_start_agent`, **WHEN** B is moved into that step, **THEN** no - session is created and the skip is logged. + `on_enter: auto_start_agent`, **WHEN** B is moved into that step or its task + view sends `session.ensure`, **THEN** no agent starts. The move creates no + session; focus may create only a workspace-ready `CREATED` session. - **GIVEN** B depends on an unfinished A and B is queued for a WIP-limited auto-start step, **WHEN** capacity opens and B is promoted, **THEN** B is admitted, its queued badge clears, and no session is created. @@ -576,4 +577,4 @@ MCP: ## Implementation plan -See [`../../plans/task-dependencies/plan.md`](../../../plans/task-dependencies/plan.md). \ No newline at end of file +See [`../../plans/task-dependencies/plan.md`](../../../plans/task-dependencies/plan.md).