diff --git a/apps/backend/internal/backendapp/boot_state.go b/apps/backend/internal/backendapp/boot_state.go index e763ac3654..fc2f130680 100644 --- a/apps/backend/internal/backendapp/boot_state.go +++ b/apps/backend/internal/backendapp/boot_state.go @@ -669,6 +669,9 @@ func (b bootStateBuilder) taskDTOsWithSessionInfo(ctx context.Context, tasks []* if queuedErr != nil { b.logBootError("queued prompt counts", queuedErr) } + // Dependency state is derived per read (never stored, so the auto-start gate + // can never read a stale value). One batched call for the whole boot payload. + dependencyViews := b.p.taskSvc.BuildDependencyViews(ctx, tasks) result := make([]taskdto.TaskDTO, 0, len(tasks)) for _, task := range tasks { if task == nil { @@ -710,6 +713,7 @@ func (b bootStateBuilder) taskDTOsWithSessionInfo(ctx context.Context, tasks []* if b.p.orchestratorSvc != nil { taskdto.EnrichTaskForegroundActivity(&dto, sessions, b.p.orchestratorSvc) } + taskdto.EnrichTaskDependencies(&dto, bootDependencyProjection(dependencyViews[task.ID]), task) dto.StatusSummary = statusSummaries[task.ID] if dto.StatusSummary != nil { switch { diff --git a/apps/backend/internal/backendapp/boot_state_dependencies.go b/apps/backend/internal/backendapp/boot_state_dependencies.go new file mode 100644 index 0000000000..7385e2764c --- /dev/null +++ b/apps/backend/internal/backendapp/boot_state_dependencies.go @@ -0,0 +1,31 @@ +package backendapp + +import ( + taskdto "github.com/kandev/kandev/internal/task/dto" + taskservice "github.com/kandev/kandev/internal/task/service" +) + +// bootDependencyProjection adapts the task service's derived DependencyView to +// the DTO projection type. Mirrors the same adapter in the task handlers; both +// exist because dto must stay importable from service without a cycle. +func bootDependencyProjection(view taskservice.DependencyView) taskdto.TaskDependencyProjection { + return taskdto.TaskDependencyProjection{ + Blocked: view.Blocked, + BlockedReason: view.BlockedReason, + DependsOn: bootDependencyRefs(view.DependsOn), + Blocks: bootDependencyRefs(view.Blocks), + } +} + +func bootDependencyRefs(refs []taskservice.DependencyRef) []taskdto.TaskDependencyRefDTO { + if len(refs) == 0 { + return nil + } + out := make([]taskdto.TaskDependencyRefDTO, 0, len(refs)) + for _, r := range refs { + out = append(out, taskdto.TaskDependencyRefDTO{ + ID: r.ID, Title: r.Title, State: r.State, Status: r.Status, + }) + } + return out +} diff --git a/apps/backend/internal/backendapp/boot_state_routes.go b/apps/backend/internal/backendapp/boot_state_routes.go index b6ca900d72..50df9f8eae 100644 --- a/apps/backend/internal/backendapp/boot_state_routes.go +++ b/apps/backend/internal/backendapp/boot_state_routes.go @@ -613,9 +613,34 @@ func mapKanbanTaskState(task taskdto.TaskDTO) map[string]any { "parentTaskId": nullString(task.ParentID), "updatedAt": task.UpdatedAt, "createdAt": task.CreatedAt, + // Dependency projection. This mapper is a camelCase whitelist writing + // straight into the frontend store shape, so a new DTO field is invisible + // to the boot payload until it is listed here — the board badge and the + // dependency chip both read these on first paint. + "blocked": task.Blocked, + "blockedReason": nullString(task.BlockedReason), + "dependsOn": dependencyRefStates(task.DependsOn), + "blocks": dependencyRefStates(task.Blocks), + "startWhenUnblocked": task.StartWhenUnblocked, } } +// dependencyRefStates maps dependency edge entries into the store shape. Returns +// an empty slice rather than nil so the client reads "no edges" instead of +// "unknown" and does not keep a stale badge. +func dependencyRefStates(refs []taskdto.TaskDependencyRefDTO) []map[string]any { + out := make([]map[string]any, 0, len(refs)) + for _, ref := range refs { + out = append(out, map[string]any{ + "id": ref.ID, + "title": ref.Title, + "state": ref.State, + statusKey: ref.Status, + }) + } + return out +} + // mapSidebarViews maps sidebar views to the SPA boot shape. func mapSidebarViews(views []usermodels.SidebarView) []map[string]any { if len(views) == 0 { diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index c4a9cc75c3..a2d1bd0bb3 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -1078,6 +1078,14 @@ func initOfficeServices( cfg, repos, services, orchestratorSvc, eventBus, agentctlBinaryPath, cfgLoader, cfgWriter, log, ) + + // Task dependencies are a core Kanban relationship, not an Office feature. + // The task_blockers table physically lives in the Office repository's DDL but + // sits in the same database, so wire the store BEFORE the Office early return + // below. Wiring it after left a Kanban-only install with no dependency store + // at all: blocked_by on create failed and list_related_tasks reported nothing. + services.Task.SetBlockerRepository(repos.Office) + if !cfg.Features.Office { log.Info("Office feature disabled; Office services skipped while global run scheduling remains enabled") return runProcessorSvc, true @@ -1105,8 +1113,9 @@ func initOfficeServices( // its own delivery code. wireRuntimeSkillDeployer(lifecycleMgr, agentRegistry, repos.Office, services.Office, cfg.ResolvedHomeDir(), log) - // Wire office-owned repositories into the task service for cross-package operations. - services.Task.SetBlockerRepository(repos.Office) + // Wire office-owned repositories into the task service for cross-package + // operations. The blocker repository is wired above, before the Office gate, + // because task dependencies are not Office-only. services.Task.SetCommentRepository(repos.Office) // Build feature-package services and wire all inter-service dependencies. diff --git a/apps/backend/internal/backendapp/orchestrator.go b/apps/backend/internal/backendapp/orchestrator.go index e2483b6831..954f182482 100644 --- a/apps/backend/internal/backendapp/orchestrator.go +++ b/apps/backend/internal/backendapp/orchestrator.go @@ -153,6 +153,11 @@ func provideOrchestrator( // boot payload and task.updated events. taskSvc.SetForegroundActivityProvider(orchestratorSvc) + // Task dependencies gate every automated launch and drive chain advancement. + // Wired unconditionally: dependencies are a core Kanban relationship, not an + // Office feature. + orchestratorSvc.SetTaskDependencyReader(taskSvc) + // Let the task service stamp status_summary.queued_prompt_count on task // list/snapshot payloads (initial-load backstop for the sidebar badge; the // status-summary projector keeps the field live between loads). diff --git a/apps/backend/internal/events/types.go b/apps/backend/internal/events/types.go index 51aabcfa95..8117e1d2bb 100644 --- a/apps/backend/internal/events/types.go +++ b/apps/backend/internal/events/types.go @@ -10,6 +10,13 @@ const ( TaskMoved = "task.moved" // Manual step change via MoveTask TaskQueuePromoted = "task.queue_promoted" SessionWorkspaceSourcesUpdated = "session.workspace_sources.updated" + // TaskDependenciesResolved fires when a task's last unresolved dependency + // completes successfully. Payload: {task_id, resolved_by_task_id}. + TaskDependenciesResolved = "task.dependencies_resolved" + // TaskDependencyFailed fires when a blocked task's chain halts because a + // predecessor failed or was cancelled. Payload: + // {task_id, failed_task_id, failed_state}. + TaskDependencyFailed = "task.dependency_failed" ) // Event types for office task tree controls. diff --git a/apps/backend/internal/mcp/handlers/dependency_handlers.go b/apps/backend/internal/mcp/handlers/dependency_handlers.go new file mode 100644 index 0000000000..a45dea4b27 --- /dev/null +++ b/apps/backend/internal/mcp/handlers/dependency_handlers.go @@ -0,0 +1,84 @@ +package handlers + +import ( + "context" + "encoding/json" + "errors" + + ws "github.com/kandev/kandev/pkg/websocket" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/service" +) + +// dependencyMutationPayload is the wire shape for both dependency tools. +type dependencyMutationPayload struct { + TaskID string `json:"task_id"` + DependsOnTaskID string `json:"depends_on_task_id"` +} + +// handleAddTaskDependency backs add_task_dependency_kandev. +func (h *Handlers) handleAddTaskDependency(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + req, errMsg := decodeDependencyMutation(msg) + if errMsg != nil { + return errMsg, nil + } + err := h.taskSvc.AddDependency(ctx, req.TaskID, req.DependsOnTaskID) + if err != nil { + var cycle *service.CycleError + if errors.As(err, &cycle) { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, cycle.Error(), + map[string]interface{}{"cycle": cycle.Path}) + } + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + } + return h.dependencyResult(ctx, msg, req.TaskID) +} + +// handleRemoveTaskDependency backs remove_task_dependency_kandev. Removing an +// absent edge succeeds. +func (h *Handlers) handleRemoveTaskDependency(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + req, errMsg := decodeDependencyMutation(msg) + if errMsg != nil { + return errMsg, nil + } + if err := h.taskSvc.RemoveDependency(ctx, req.TaskID, req.DependsOnTaskID); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + } + return h.dependencyResult(ctx, msg, req.TaskID) +} + +func decodeDependencyMutation(msg *ws.Message) (*dependencyMutationPayload, *ws.Message) { + var req dependencyMutationPayload + if err := json.Unmarshal(msg.Payload, &req); err != nil { + errMsg, _ := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + return nil, errMsg + } + if req.TaskID == "" || req.DependsOnTaskID == "" { + errMsg, _ := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, + "task_id and depends_on_task_id are required", nil) + return nil, errMsg + } + return &req, nil +} + +// dependencyResult returns the task's resulting dependency projection so the +// agent sees the effect of its own call without a follow-up read. +func (h *Handlers) dependencyResult(ctx context.Context, msg *ws.Message, taskID string) (*ws.Message, error) { + task, err := h.taskSvc.GetTask(ctx, taskID) + if err != nil || task == nil { + return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{keyTaskID: taskID}) + } + views := h.taskSvc.BuildDependencyViews(ctx, []*models.Task{task}) + view := views[taskID] + dependsOn := make([]string, 0, len(view.DependsOn)) + for _, ref := range view.DependsOn { + dependsOn = append(dependsOn, ref.ID) + } + return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{ + keyTaskID: taskID, + "blocked": view.Blocked, + "blocked_reason": view.BlockedReason, + "depends_on": dependsOn, + }) +} diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index e10a50b460..706ebfba8b 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -357,6 +357,8 @@ func (h *Handlers) RegisterHandlers(d *ws.Dispatcher) { d.RegisterFunc(ws.ActionMCPUpdateTaskPRAutomation, h.handleUpdateTaskPRAutomation) d.RegisterFunc(ws.ActionMCPGetTaskMRAutomation, h.handleGetTaskMRAutomation) d.RegisterFunc(ws.ActionMCPUpdateTaskMRAutomation, h.handleUpdateTaskMRAutomation) + d.RegisterFunc(ws.ActionMCPAddTaskDependency, h.handleAddTaskDependency) + d.RegisterFunc(ws.ActionMCPRemoveTaskDependency, h.handleRemoveTaskDependency) d.RegisterFunc(ws.ActionMCPAddBranchToTask, h.handleAddBranchToTask) d.RegisterFunc(ws.ActionMCPAddWorkspaceSources, h.handleAddWorkspaceSources) d.RegisterFunc(ws.ActionMCPUpdateRepositoryBaseBranch, h.handleUpdateRepositoryBaseBranch) @@ -590,6 +592,7 @@ func (h *Handlers) handleCreateTask(ctx context.Context, msg *ws.Message) (*ws.M Repositories []mcpRepositoryInput `json:"repositories"` // explicit repositories for top-level tasks BaseBranch string `json:"base_branch"` // top-level fallback applied to every resolved repo only when no per-repo entries are supplied; explicit per-repo BaseBranch is authoritative when Repositories is set BlockedBy []string `json:"blocked_by"` // task IDs that must complete before this task + StartWhenUnblocked *bool `json:"start_when_unblocked"` // nil = derive from start_agent when BlockedBy is set AssigneeAgentProfileID string `json:"assignee_agent_profile_id"` // agent instance to assign the task to ExternalID string `json:"external_id"` // caller-supplied create-idempotency key } @@ -719,6 +722,7 @@ func (h *Handlers) handleCreateTask(ctx context.Context, msg *ws.Message) (*ws.M Autopilot: req.Autopilot, Repositories: repos, BlockedBy: req.BlockedBy, + StartWhenUnblocked: req.StartWhenUnblocked, AssigneeAgentProfileID: req.AssigneeAgentProfileID, Metadata: metadata, DeferredLaunch: deferredLaunch, @@ -809,12 +813,28 @@ func (h *Handlers) handleCreateTask(ctx context.Context, msg *ws.Message) (*ws.M } // Auto-start agent session asynchronously only if requested and admitted. - if startAgent && task.QueuedForStepID == "" && h.sessionLauncher != nil { + // + // A create that declared dependencies does NOT launch now: the start intent + // was recorded as a start-when-unblocked deferred launch and dependency + // resolution consumes it. Agents pass start_agent=true by habit, so without + // this every step of an agent-built chain would launch at once. + startWhenUnblocked := service.ResolveStartWhenUnblocked(&service.CreateTaskRequest{ + BlockedBy: req.BlockedBy, + StartWhenUnblocked: req.StartWhenUnblocked, + }) + // Blockers suppress the immediate launch on their own. start_when_unblocked + // only decides whether a DEFERRED launch is recorded, so `false` means "no + // automatic start at all" — launching now would start a task that is blocked. + if startAgent && len(req.BlockedBy) == 0 && task.QueuedForStepID == "" && h.sessionLauncher != nil { h.launchAutoStartTask(ctx, task, launchConfig) } + response := dto.FromTask(task) + // Report the deferred start so the caller does not have to infer whether the + // task launched or is waiting on its dependencies. + response.StartWhenUnblocked = startWhenUnblocked return ws.NewResponse(msg.ID, msg.Action, mcpCreateTaskResult{ - TaskDTO: dto.FromTask(task), + TaskDTO: response, Deduplicated: false, CreationComplete: true, }) diff --git a/apps/backend/internal/mcp/server/dependency_handlers.go b/apps/backend/internal/mcp/server/dependency_handlers.go new file mode 100644 index 0000000000..9dc86fb6b6 --- /dev/null +++ b/apps/backend/internal/mcp/server/dependency_handlers.go @@ -0,0 +1,93 @@ +package mcp + +import ( + "context" + "encoding/json" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" + + ws "github.com/kandev/kandev/pkg/websocket" +) + +// selfTaskSentinel is the "my own task" shorthand create_task_kandev already +// accepts for parent_id; the dependency tools take it for task_id too. +const selfTaskSentinel = "self" + +// stringArrayArg reads a string array argument, tolerating the []interface{} +// shape JSON decoding produces and skipping non-string entries rather than +// failing the whole call. +func stringArrayArg(req mcp.CallToolRequest, key string) []string { + raw, ok := req.GetArguments()[key] + if !ok || raw == nil { + return nil + } + switch v := raw.(type) { + case []string: + return v + case []interface{}: + out := make([]string, 0, len(v)) + for _, entry := range v { + if s, ok := entry.(string); ok && s != "" { + out = append(out, s) + } + } + return out + default: + return nil + } +} + +// addTaskDependencyHandler backs add_task_dependency_kandev. +func (s *Server) addTaskDependencyHandler() server.ToolHandlerFunc { + return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return s.dispatchDependencyMutation(ctx, req, ws.ActionMCPAddTaskDependency) + } +} + +// removeTaskDependencyHandler backs remove_task_dependency_kandev. +func (s *Server) removeTaskDependencyHandler() server.ToolHandlerFunc { + return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return s.dispatchDependencyMutation(ctx, req, ws.ActionMCPRemoveTaskDependency) + } +} + +// dispatchDependencyMutation resolves the target task (defaulting to the +// caller's own) and forwards the edge mutation. Both tools share this because +// they differ only in the action they send. +func (s *Server) dispatchDependencyMutation( + ctx context.Context, req mcp.CallToolRequest, action string, +) (*mcp.CallToolResult, error) { + dependsOn, err := req.RequireString("depends_on_task_id") + if err != nil { + return mcp.NewToolResultError("depends_on_task_id is required"), nil + } + taskID := req.GetString(mcpKeyTaskID, "") + if taskID == "" || taskID == selfTaskSentinel { + if s.taskID == "" { + return mcp.NewToolResultError("task_id is required: no current task context"), nil + } + taskID = s.taskID + } + // In a task-bound session the dependent end is the server's own task. + // Ownership comes from the AgentExecution the session was bound to, never + // from an agent-supplied id, so an explicit task_id may only confirm it — + // the same rule add_branch_to_task_kandev applies. The predecessor + // (depends_on_task_id) is necessarily another task and stays free; the + // backend authorizes both ends against the session's identity. + if s.taskID != "" && taskID != s.taskID { + return mcp.NewToolResultError( + "task_id must be your current task; use depends_on_task_id to name the other task", + ), nil + } + payload := map[string]interface{}{ + mcpKeyTaskID: taskID, + "depends_on_task_id": dependsOn, + } + var result map[string]interface{} + if err := s.backend.RequestPayload(ctx, action, payload, &result); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + data, _ := json.MarshalIndent(result, "", " ") + return mcp.NewToolResultText(string(data)), nil +} diff --git a/apps/backend/internal/mcp/server/handlers.go b/apps/backend/internal/mcp/server/handlers.go index 5c32fb4ee5..5bc8f45360 100644 --- a/apps/backend/internal/mcp/server/handlers.go +++ b/apps/backend/internal/mcp/server/handlers.go @@ -187,6 +187,22 @@ func (s *Server) createTaskHandler() server.ToolHandlerFunc { payload["external_id"] = externalID } + // Dependency edges declared at create time. The handler already read + // blocked_by from its payload before this feature, but the tool schema + // never declared the parameter, so no agent could reach it. + if blockedBy := stringArrayArg(req, "blocked_by"); len(blockedBy) > 0 { + payload["blocked_by"] = blockedBy + } + // Omitted means "derive": with blocked_by set, a start request becomes a + // start-when-unblocked intent. start_agent defaults to true and agents + // pass it by habit, so deriving is what makes an agent-built chain run in + // order instead of launching every step at once. + if args := req.GetArguments(); args["start_when_unblocked"] != nil { + if v, ok := args["start_when_unblocked"].(bool); ok { + payload["start_when_unblocked"] = v + } + } + // Add repository info. For subtasks an explicit repo overrides the // parent's; if omitted the backend inherits from the parent. repositoryID := req.GetString("repository_id", "") diff --git a/apps/backend/internal/mcp/server/handlers_test.go b/apps/backend/internal/mcp/server/handlers_test.go index c3dfee3f29..f388e17462 100644 --- a/apps/backend/internal/mcp/server/handlers_test.go +++ b/apps/backend/internal/mcp/server/handlers_test.go @@ -68,6 +68,7 @@ func TestCreateTask_ToolSchema_HasParentID(t *testing.T) { "parent_id", "workspace_id", "workflow_id", "workflow_step_id", "workspace_mode", "title", "prompt", "autopilot", "agent_profile_id", "executor_profile_id", "start_agent", "repository_id", "local_path", "repository_url", "base_branch", "external_id", + "blocked_by", "start_when_unblocked", }, propertyNames(props), "unexpected change to the advertised create_task_kandev schema") assert.NotContains(t, props, "description", "legacy alias must not increase the advertised schema") assert.Contains(t, tool.Tool.Description, "'prompt' is the sub-agent's initial prompt") diff --git a/apps/backend/internal/mcp/server/server.go b/apps/backend/internal/mcp/server/server.go index a7c147d449..699a51749e 100644 --- a/apps/backend/internal/mcp/server/server.go +++ b/apps/backend/internal/mcp/server/server.go @@ -825,6 +825,8 @@ func (s *Server) profileToolGroups() []profileToolGroup { {name: "configuration-executors", enabled: func(ctx mcpprofile.Context) bool { return config(ctx) || external(ctx) }, register: func(s *Server) { s.registerConfigExecutorTools() }}, {name: "configuration-tasks", enabled: func(ctx mcpprofile.Context) bool { return config(ctx) || external(ctx) }, register: func(s *Server) { s.registerConfigTaskTools() }}, {name: "external-create-task", enabled: external, register: func(s *Server) { s.registerCreateTaskTool() }}, + // Dependency edges are manageable wherever a task can be created. + {name: "task-dependencies", enabled: func(ctx mcpprofile.Context) bool { return kanban(ctx) || external(ctx) }, register: func(s *Server) { s.registerTaskDependencyTools() }}, {name: "kanban-task", enabled: kanban, register: func(s *Server) { s.registerKanbanTools() }}, {name: "github-pr", enabled: andProfilePredicates(kanban, func(ctx mcpprofile.Context) bool { return mcpproviders.Contains(ctx.Providers, mcpproviders.GitHub) }), register: func(s *Server) { s.registerPRAutomationTools() }}, {name: "gitlab-mr", enabled: andProfilePredicates(kanban, func(ctx mcpprofile.Context) bool { return mcpproviders.Contains(ctx.Providers, mcpproviders.GitLab) }), register: func(s *Server) { s.registerMRAutomationTools() }}, @@ -1157,11 +1159,60 @@ IDEMPOTENCY (external_id): mcp.WithString("repository_url", mcp.Description("Repository URL, GitHub pull request URL, or GitLab merge request URL (for example 'https://github.com/owner/repo'). A contribution URL attaches the task to that existing contribution and prepares its source branch. For subtasks: supply only when the subtask should target a different repo than the parent.")), mcp.WithString("base_branch", mcp.Description("Base branch for the repository (e.g. 'main'). Optional. Defaults: same-repo subtasks inherit the parent's base_branch; cross-repo subtasks and top-level tasks fall back to the repository's default_branch (visible via list_repositories_kandev).")), mcp.WithString("external_id", mcp.Description("A stable identifier from your own system (issue key, webhook delivery ID, a UUID you generated). Creating a task twice with the same external_id in the same workspace returns the first task instead of making a duplicate — use it when a retry or restart could re-run this call. Replay the same arguments you sent the first time. This creates the task when nothing holds the identity yet — it is not a lookup.")), + mcp.WithArray("blocked_by", + mcp.Description(blockedByParamDesc), + mcp.Items(map[string]any{"type": "string"}), + ), + mcp.WithBoolean("start_when_unblocked", mcp.Description(startWhenUnblockedParamDesc)), ), s.wrapHandler("create_task_kandev", s.createTaskHandler()), ) } +// Dependency parameter descriptions for create_task_kandev. Extracted as +// constants because the same guidance has to appear on the two dependency tools +// below and must not drift between them. +const ( + blockedByParamDesc = "Task IDs this task depends on: it will not start until every one of them completes SUCCESSFULLY. " + + "Use this — not parent_id — to express ordering. A subtask means \"part of\"; a dependency means \"not until\". " + + "Decomposing a plan into ordered phases is N sibling tasks chained with blocked_by, NOT N subtasks started at once. " + + "A predecessor that ends FAILED or CANCELLED halts the chain and needs human action; it will not retry itself." + + startWhenUnblockedParamDesc = "Whether to start this task automatically once every task in blocked_by completes successfully. " + + "Defaults to true when blocked_by is non-empty, which is what chains ordered work: with blocked_by set, " + + "start_agent=true records this intent instead of launching now, so the whole chain does not start at once. " + + "Pass false to create the dependency edges with no automatic start at all." +) + +// registerTaskDependencyTools registers add/remove for task dependency edges. +// Mirrors the two HTTP routes one-to-one and shares the single edge validator +// (self-edge, cross-workspace, cycle-with-path) in the task service. The read +// side already exists as list_related_tasks_kandev. +func (s *Server) registerTaskDependencyTools() { + s.mcpServer.AddTool( + mcp.NewTool("add_task_dependency_kandev", + mcp.WithDescription(`Declare that a task is blocked by another task: it will not start until that one completes successfully. + +`+blockedByParamDesc+` + +task_id defaults to your CURRENT task. Rejected when the edge would create a cycle (the error names the cycle path), when both IDs are the same, or when the two tasks are in different workspaces. Returns the task's resulting depends_on list.`), + mcp.WithString(mcpKeyTaskID, mcp.Description("The blocked task. Defaults to your current task when omitted.")), + mcp.WithString("depends_on_task_id", mcp.Required(), mcp.Description("The task that must complete first (the predecessor).")), + ), + s.wrapHandler("add_task_dependency_kandev", s.addTaskDependencyHandler()), + ) + s.mcpServer.AddTool( + mcp.NewTool("remove_task_dependency_kandev", + mcp.WithDescription(`Remove a task dependency edge. Removing an edge that does not exist succeeds. + +Removing the last edge unblocks the task but does NOT start it: an automatic start is triggered by a dependency RESOLVING, not by the edge going away. Removing the edge means you are taking manual control.`), + mcp.WithString(mcpKeyTaskID, mcp.Description("The blocked task. Defaults to your current task when omitted.")), + mcp.WithString("depends_on_task_id", mcp.Required(), mcp.Description("The predecessor task to unlink.")), + ), + s.wrapHandler("remove_task_dependency_kandev", s.removeTaskDependencyHandler()), + ) +} + // registerSpawnSessionTool registers spawn_session_kandev. Spawns an ADDITIONAL // agent session on an existing task (usually the caller's own) — unlike // create_task_kandev, no new task is created: the spawned session shares the diff --git a/apps/backend/internal/mcp/server/server_test.go b/apps/backend/internal/mcp/server/server_test.go index fffbe31fc5..84226a0511 100644 --- a/apps/backend/internal/mcp/server/server_test.go +++ b/apps/backend/internal/mcp/server/server_test.go @@ -644,7 +644,7 @@ drained: // as in TestServerModeTask_ToolCount and // TestRegisterTools_LoggedCountMatchesRegisteredTools (list_task_sessions_test.go), // which pin the per-mode registration rather than this SetProviders rebuild. - require.Len(t, tools, 33, "final registry should contain the complete GitLab-only task tool set") + require.Len(t, tools, 35, "final registry should contain the complete GitLab-only task tool set") assert.Contains(t, tools, "get_task_mr_automation_kandev") assert.NotContains(t, tools, "get_task_pr_automation_kandev") } @@ -812,7 +812,8 @@ func TestServerModeTask_ToolCount(t *testing.T) { // list_task_sessions + PR automation + MR automation) + 1 add_branch_to_task + // 1 add_workspace_sources + 1 update_repository_base_branch + // 1 step_complete (ADR 0015) + 1 interaction + 4 plan + 3 walkthrough + - // 1 publish_review_findings + 1 related-tasks + 1 diagnostic bundle = 35. + // 1 publish_review_findings + 1 related-tasks + 1 diagnostic bundle + // + 2 task-dependency (add/remove) = 37. // Task-document tools (list/get/write) are office-only. assert.Contains(t, tools, "step_complete_kandev", "ADR 0015 explicit-completion signal must be registered in task mode") assert.Contains(t, tools, "show_walkthrough_kandev", "walkthrough tool must be registered in task mode") @@ -820,7 +821,9 @@ func TestServerModeTask_ToolCount(t *testing.T) { assert.Contains(t, tools, "spawn_session_kandev", "spawn_session must be registered in task mode") assert.Contains(t, tools, "add_workspace_sources_kandev") assert.Contains(t, tools, "list_task_sessions_kandev", "session discovery must be registered in task mode") - assert.Equal(t, 35, len(tools)) + assert.Contains(t, tools, "add_task_dependency_kandev", "dependency edges must be manageable in task mode") + assert.Contains(t, tools, "remove_task_dependency_kandev") + assert.Equal(t, 37, len(tools)) } func TestServerStepCompleteTool_TaskOnlyAndDiscoverable(t *testing.T) { @@ -1009,9 +1012,9 @@ func TestServerModeExternal_ToolCount(t *testing.T) { s := New(backend, "", "", 0, log, "", true, ModeExternal) tools := getRegisteredToolNames(s) - // 12 workflow (incl. list_repositories + import_workflow) + 4 agent + 4 mcp + 5 executor + 7 task (incl. list_task_sessions) + 1 create_task = 33. + // 12 workflow (incl. list_repositories + import_workflow) + 4 agent + 4 mcp + 5 executor + 7 task (incl. list_task_sessions) + 1 create_task + 2 task-dependency = 35. // add_branch_to_task_kandev is task-mode only — external coding agents have no live session to attach a worktree to. - assert.Equal(t, 33, len(tools)) + assert.Equal(t, 35, len(tools)) assert.NotContains(t, tools, "add_branch_to_task_kandev") } diff --git a/apps/backend/internal/office/repository/sqlite/blockers.go b/apps/backend/internal/office/repository/sqlite/blockers.go index f7f14899a2..5053bb4284 100644 --- a/apps/backend/internal/office/repository/sqlite/blockers.go +++ b/apps/backend/internal/office/repository/sqlite/blockers.go @@ -83,6 +83,62 @@ func (r *Repository) ListBlockersForTasks(ctx context.Context, taskIDs []string) return result, nil } +// ListDependentsForTasks returns a map of blocker task ID → list of task IDs +// blocked by it, for a set of blocker task IDs. This is the batched reverse of +// ListBlockersForTasks and backs the "blocks" direction of the dependency +// payload; a per-task query would add one round trip per card to a board read. +func (r *Repository) ListDependentsForTasks(ctx context.Context, blockerTaskIDs []string) (map[string][]string, error) { + if len(blockerTaskIDs) == 0 { + return map[string][]string{}, nil + } + query, args, err := sqlx.In( + `SELECT task_id, blocker_task_id FROM task_blockers WHERE blocker_task_id IN (?) ORDER BY created_at, task_id`, + blockerTaskIDs) + if err != nil { + return nil, err + } + query = r.ro.Rebind(query) + type row struct { + TaskID string `db:"task_id"` + BlockerTaskID string `db:"blocker_task_id"` + } + var rows []row + if err := r.ro.SelectContext(ctx, &rows, query, args...); err != nil { + return nil, err + } + result := make(map[string][]string, len(blockerTaskIDs)) + for _, rw := range rows { + result[rw.BlockerTaskID] = append(result[rw.BlockerTaskID], rw.TaskID) + } + return result, nil +} + +// ListTasksWithDependencies returns the distinct dependent task IDs that have at +// least one dependency edge. Used by startup reconciliation to bound its sweep to +// tasks that could be chain steps rather than scanning the whole board. +func (r *Repository) ListTasksWithDependencies(ctx context.Context) ([]string, error) { + var ids []string + err := r.ro.SelectContext(ctx, &ids, + `SELECT DISTINCT task_id FROM task_blockers ORDER BY task_id`) + if err != nil { + return nil, err + } + if ids == nil { + ids = []string{} + } + return ids, nil +} + +// DeleteTaskBlockersForTask removes every dependency edge touching taskID, in +// both directions. Called when a task is deleted: task_blockers predates the +// tasks foreign key, so there is no ON DELETE CASCADE to rely on and orphaned +// edges would keep dependents blocked on a task that no longer exists. +func (r *Repository) DeleteTaskBlockersForTask(ctx context.Context, taskID string) error { + _, err := r.db.ExecContext(ctx, r.db.Rebind( + `DELETE FROM task_blockers WHERE task_id = ? OR blocker_task_id = ?`), taskID, taskID) + return err +} + func (r *Repository) IsTaskInTerminalStep(ctx context.Context, taskID string) (bool, error) { var state string err := r.ro.QueryRowxContext(ctx, r.ro.Rebind( diff --git a/apps/backend/internal/orchestrator/event_handlers_children_completed.go b/apps/backend/internal/orchestrator/event_handlers_children_completed.go index 9839dd06d0..a4b1ed70cc 100644 --- a/apps/backend/internal/orchestrator/event_handlers_children_completed.go +++ b/apps/backend/internal/orchestrator/event_handlers_children_completed.go @@ -24,6 +24,10 @@ func (s *Service) handleTaskStateChanged(ctx context.Context, data watcher.TaskE } s.processParentChildrenCompletedForTaskState(ctx, data.TaskID, *data.NewState) + // Peer dependencies react to the same terminal transitions, but with a + // stricter definition of "done": on_children_completed counts FAILED as + // terminal, dependency resolution requires success. Keep the two separate. + s.handleTaskDependenciesForTerminalState(ctx, data.TaskID, *data.NewState) } func (s *Service) processParentChildrenCompletedForTaskState(ctx context.Context, taskID string, state v1.TaskState) { diff --git a/apps/backend/internal/orchestrator/event_handlers_dependencies.go b/apps/backend/internal/orchestrator/event_handlers_dependencies.go new file mode 100644 index 0000000000..f25eb1593d --- /dev/null +++ b/apps/backend/internal/orchestrator/event_handlers_dependencies.go @@ -0,0 +1,190 @@ +package orchestrator + +import ( + "context" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" + taskservice "github.com/kandev/kandev/internal/task/service" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// TaskDependencyReader is the orchestrator's read seam onto task dependency +// state. Implemented by the task service. +type TaskDependencyReader interface { + // DependencyGate reports whether an automated path may start taskID. + // Returns blocked=true on any read error so the gate fails closed. + DependencyGate(ctx context.Context, taskID string) (bool, string, error) + // ListDependentTaskIDs returns the tasks directly blocked by taskID. + ListDependentTaskIDs(ctx context.Context, taskID string) ([]string, error) + // ListPendingDependencyLaunches returns tasks that will start on their own + // once their dependencies resolve. Used by startup reconciliation. + ListPendingDependencyLaunches(ctx context.Context) ([]taskservice.PendingDependencyLaunch, error) +} + +// SetTaskDependencyReader wires dependency reads for the auto-start gate and +// the dependency-resolution reaction. +func (s *Service) SetTaskDependencyReader(reader TaskDependencyReader) { + s.dependencyReader = reader +} + +// dependencyBlocksAutoStart reports whether taskID has an unresolved dependency +// and therefore must not be launched by an automated path. +// +// Fails CLOSED: a read error returns true. Failing open would launch work whose +// predecessor may never have run, which is the single outcome task dependencies +// exist to prevent. +func (s *Service) dependencyBlocksAutoStart(ctx context.Context, taskID, eventName string) bool { + if s.dependencyReader == nil { + return false + } + blocked, reason, err := s.dependencyReader.DependencyGate(ctx, taskID) + if err != nil { + s.logger.Warn(eventName+": dependency lookup failed; skipping auto-start", + zap.String("task_id", taskID), zap.Error(err)) + return true + } + if blocked { + s.logger.Debug(eventName+": task has unresolved dependencies; skipping auto-start", + zap.String("task_id", taskID), zap.String("blocked_reason", reason)) + } + return blocked +} + +// handleTaskDependenciesForTerminalState reacts to a task reaching a terminal +// state by re-evaluating every task that depends on it. +// +// Called from handleTaskStateChanged. Moves into a terminal workflow step +// converge here too: markTaskCompletedForTerminalStep persists +// state = COMPLETED and publishes task.state_changed, so this one hook covers +// both "agent finished" and "card dragged to Done". +func (s *Service) handleTaskDependenciesForTerminalState(ctx context.Context, taskID string, state v1.TaskState) { + if s.dependencyReader == nil { + return + } + dependents, err := s.dependencyReader.ListDependentTaskIDs(ctx, taskID) + if err != nil { + s.logger.Warn("dependency resolution: failed to list dependents", + zap.String("task_id", taskID), zap.Error(err)) + return + } + for _, dependentID := range dependents { + s.evaluateDependentAfterPredecessorChange(ctx, dependentID, taskID, state) + } +} + +// evaluateDependentAfterPredecessorChange decides what one dependent should do +// now that a predecessor reached a terminal state. +func (s *Service) evaluateDependentAfterPredecessorChange( + ctx context.Context, dependentID, predecessorID string, predecessorState v1.TaskState, +) { + blocked, reason, err := s.dependencyReader.DependencyGate(ctx, dependentID) + if err != nil { + s.logger.Warn("dependency resolution: gate lookup failed", + zap.String("task_id", dependentID), zap.Error(err)) + return + } + if blocked { + // Still waiting on something. When nothing is pending and the only + // obstacle is a failed predecessor, the chain has halted and needs a + // human: surface it rather than retrying or dropping the edge. + if reason == taskservice.BlockedReasonFailed { + s.publishDependencyFailed(ctx, dependentID, predecessorID, predecessorState) + } + return + } + // Every predecessor resolved successfully. Publish, then enter the SAME + // auto-start chokepoint every other automated launch uses, so the existing + // deferred-launch claim keeps this to one session even if WIP promotion + // reaches the task at the same moment. + s.publishDependenciesResolved(ctx, dependentID, predecessorID) + task, err := s.repo.GetTask(ctx, dependentID) + if err != nil || task == nil { + s.logger.Warn("dependency resolution: failed to load unblocked task", + zap.String("task_id", dependentID), zap.Error(err)) + return + } + // Resolution starts a task only when it recorded a start-when-unblocked + // intent. Without this, a blocked task parked in a step that has + // on_enter:auto_start_agent would launch the moment its gate opened, even + // though nobody asked for a start — the chokepoint's step-based auto-start + // is meant for a task ENTERING that step, not for one already sitting in it. + if !taskservice.HasStartWhenUnblockedIntent(task) { + s.logger.Debug("dependency resolution: no start-when-unblocked intent; leaving task unstarted", + zap.String("task_id", task.ID)) + return + } + s.autoStartTaskForStep(ctx, task.ID, task.WorkflowStepID, events.TaskDependenciesResolved) +} + +// publishDependenciesResolved announces that a task's last unresolved +// dependency resolved successfully. +func (s *Service) publishDependenciesResolved(ctx context.Context, taskID, resolvedByTaskID string) { + if s.eventBus == nil { + return + } + payload := map[string]any{metaKeyTaskID: taskID, "resolved_by_task_id": resolvedByTaskID} + if err := s.eventBus.Publish(ctx, events.TaskDependenciesResolved, + bus.NewEvent(events.TaskDependenciesResolved, "orchestrator", payload)); err != nil { + s.logger.Warn("failed to publish task.dependencies_resolved", + zap.String("task_id", taskID), zap.Error(err)) + } +} + +// publishDependencyFailed announces that a blocked task's chain has halted +// because a predecessor failed or was cancelled. +func (s *Service) publishDependencyFailed( + ctx context.Context, taskID, failedTaskID string, failedState v1.TaskState, +) { + if s.eventBus == nil { + return + } + payload := map[string]any{ + metaKeyTaskID: taskID, + "failed_task_id": failedTaskID, + "failed_state": string(failedState), + } + if err := s.eventBus.Publish(ctx, events.TaskDependencyFailed, + bus.NewEvent(events.TaskDependencyFailed, "orchestrator", payload)); err != nil { + s.logger.Warn("failed to publish task.dependency_failed", + zap.String("task_id", taskID), zap.Error(err)) + } + // Refresh the dependent so its card and chip re-render with the failed + // reason without waiting for the next board load. + if task, err := s.repo.GetTask(ctx, taskID); err == nil && task != nil { + s.publishTaskUpdated(ctx, task) + } +} + +// reconcileDependencyLaunchesOnStartup launches tasks whose dependencies were +// satisfied while the process was down. +// +// Recovery path for a restart between a predecessor's completion and the +// dependent's launch: task.dependencies_resolved is an in-memory event and is +// not replayed, so without this sweep a chain would stall silently across a +// restart. Idempotent — the deferred launch claim admits one launcher however +// many times this runs. +// +// Scoped to tasks that actually carry a start-when-unblocked intent, so the +// sweep is bounded by the number of pending chain steps rather than the board. +func (s *Service) reconcileDependencyLaunchesOnStartup(ctx context.Context) { + if s.dependencyReader == nil { + return + } + pending, err := s.dependencyReader.ListPendingDependencyLaunches(ctx) + if err != nil { + s.logger.Warn("startup: failed to list pending dependency launches", zap.Error(err)) + return + } + for _, candidate := range pending { + blocked, _, err := s.dependencyReader.DependencyGate(ctx, candidate.TaskID) + if err != nil || blocked { + continue + } + s.logger.Info("startup: launching task whose dependencies resolved while down", + zap.String("task_id", candidate.TaskID)) + s.autoStartTaskForStep(ctx, candidate.TaskID, candidate.WorkflowStepID, "startup.dependencies_resolved") + } +} diff --git a/apps/backend/internal/orchestrator/event_handlers_dependencies_test.go b/apps/backend/internal/orchestrator/event_handlers_dependencies_test.go new file mode 100644 index 0000000000..4dc3091ef1 --- /dev/null +++ b/apps/backend/internal/orchestrator/event_handlers_dependencies_test.go @@ -0,0 +1,54 @@ +package orchestrator + +import ( + "testing" + + "github.com/kandev/kandev/internal/task/models" + taskservice "github.com/kandev/kandev/internal/task/service" +) + +// Dependency resolution must launch only tasks that recorded a +// start-when-unblocked intent. Without this, a blocked task sitting in a step +// with on_enter:auto_start_agent would launch the moment its gate opened, even +// though nobody asked for a start: the chokepoint's step-based auto-start is +// meant for a task ENTERING that step, not one already parked in it. +func TestResolutionLaunchesOnlyTasksWithARecordedIntent(t *testing.T) { + chainStep := &models.Task{Metadata: map[string]interface{}{ + models.MetaKeyDeferredLaunch: map[string]interface{}{ + "intent": "start", "agent_profile_id": "p", + models.DeferredLaunchStartWhenUnblockedKey: true, + }, + }} + if !taskservice.HasStartWhenUnblockedIntent(chainStep) { + t.Error("a chain step must be recognised as launchable on resolution") + } + + // A WIP-overflow intent is the same record without the flag; resolution must + // not adopt it, or a queued task would launch for the wrong reason. + wipOverflow := &models.Task{Metadata: map[string]interface{}{ + models.MetaKeyDeferredLaunch: map[string]interface{}{ + "intent": "start", "agent_profile_id": "p", + }, + }} + if taskservice.HasStartWhenUnblockedIntent(wipOverflow) { + t.Error("a WIP-overflow intent must not be treated as a chain intent") + } + + for name, task := range map[string]*models.Task{ + "no metadata": {}, + "nil task": nil, + } { + if taskservice.HasStartWhenUnblockedIntent(task) { + t.Errorf("%s: must not be launchable on resolution", name) + } + } +} + +// The failure reason compared in the resolution path must be the exported +// constant, not a local copy that can drift out of sync silently. +func TestFailedReasonUsesTheExportedConstant(t *testing.T) { + if taskservice.BlockedReasonFailed != "failed" { + t.Fatalf("BlockedReasonFailed = %q; the resolution path compares against it", + taskservice.BlockedReasonFailed) + } +} diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index 6b8c8ed817..58e2fd5819 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -486,6 +486,13 @@ func (s *Service) autoStartTaskForStep(ctx context.Context, taskID, stepID, even if task == nil || task.QueuedForStepID != "" { return } + // Dependency gate. Sits here — the single automated-launch chokepoint — so + // one check covers task.moved, task.queue_promoted, watcher auto-start and + // dependency resolution itself. Placed BEFORE launchDeferredTask so a + // blocked task neither starts a session nor consumes its launch intent. + if s.dependencyBlocksAutoStart(ctx, taskID, eventName) { + return + } if s.launchDeferredTask(ctx, task, eventName) { return } diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index 1dbf4797d4..0ed9836749 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -388,6 +388,11 @@ type Service struct { // Optional. stepHistoryRecorder StepHistoryRecorder + // Reads task dependency state for the auto-start gate and the + // dependency-resolution reaction. Nil-safe: when unset, no task has + // dependencies so nothing is gated. + dependencyReader TaskDependencyReader + // Resolves the agent family names written in configure_session rules onto // canonical agent IDs. Nil-safe: when unset, rule matching falls back to an // exact string comparison. @@ -1653,6 +1658,11 @@ func (s *Service) Start(ctx context.Context) error { if s.workflowStore != nil { s.workflowStore.ReconcileQueuedTasks(ctx) } + // Chains whose predecessor completed while the process was down: the + // dependencies_resolved event is in-memory and is not replayed, so without + // this sweep a chain stalls silently across a restart. Runs after the WIP + // reconciler so an admitted-by-promotion task is already eligible. + s.reconcileDependencyLaunchesOnStartup(ctx) // Start the watcher first to begin receiving events if err := s.watcher.Start(ctx); err != nil { diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index c286bf5150..eaa00b6cc1 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -189,6 +189,22 @@ type TaskDTO struct { // metadata key at DTO conversion time (see FromTaskWithSessionInfo). Interrupted bool `json:"interrupted,omitempty"` + // Dependency projection. Derived on every read from task_blockers plus each + // related task's own state — never persisted, because a stale copy would be + // read by the auto-start gate. Stamped by EnrichTaskDependencies. + Blocked bool `json:"blocked,omitempty"` + // BlockedReason is "pending", "failed", or "unknown"; omitted when the task + // is not blocked. + BlockedReason string `json:"blocked_reason,omitempty"` + // DependsOn lists direct predecessors, DependsOn/Blocks are not transitive. + DependsOn []TaskDependencyRefDTO `json:"depends_on,omitempty"` + // Blocks lists direct dependents so the dependency chip can render both + // directions without a second round trip. + Blocks []TaskDependencyRefDTO `json:"blocks,omitempty"` + // StartWhenUnblocked reports that a launch intent is waiting on dependency + // resolution. Read-only here; set through the create request or the picker. + StartWhenUnblocked bool `json:"start_when_unblocked,omitempty"` + // Office extensions AssigneeAgentProfileID string `json:"assignee_agent_profile_id,omitempty"` Origin string `json:"origin,omitempty"` diff --git a/apps/backend/internal/task/dto/task_dependencies.go b/apps/backend/internal/task/dto/task_dependencies.go new file mode 100644 index 0000000000..2fc81e4be9 --- /dev/null +++ b/apps/backend/internal/task/dto/task_dependencies.go @@ -0,0 +1,53 @@ +package dto + +import ( + "github.com/kandev/kandev/internal/task/models" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// TaskDependencyRefDTO is one end of a dependency edge, carrying enough detail +// for the dependency chip and the blocked badge to render without fetching each +// related task individually. +type TaskDependencyRefDTO struct { + ID string `json:"id"` + Title string `json:"title,omitempty"` + State v1.TaskState `json:"state,omitempty"` + // Status is the resolution verdict for this edge: "resolved", "failed", or + // "pending". Only meaningful on depends_on entries. + Status string `json:"status,omitempty"` +} + +// TaskDependencyProjection is the shape a caller supplies to stamp dependency +// state onto a DTO. It mirrors service.DependencyView without importing the +// service package (dto must stay importable from it). +type TaskDependencyProjection struct { + Blocked bool + BlockedReason string + DependsOn []TaskDependencyRefDTO + Blocks []TaskDependencyRefDTO +} + +// EnrichTaskDependencies stamps the derived dependency projection onto a task +// DTO, following the same read-time-enrichment pattern as +// EnrichTaskForegroundActivity. +// +// StartWhenUnblocked is read from the task's deferred-launch metadata rather +// than passed in: the intent and the edges are stored separately, and only the +// combination means "this will start on its own". +func EnrichTaskDependencies(dto *TaskDTO, projection TaskDependencyProjection, task *models.Task) { + if dto == nil { + return + } + dto.Blocked = projection.Blocked + dto.BlockedReason = projection.BlockedReason + dto.DependsOn = projection.DependsOn + dto.Blocks = projection.Blocks + dto.StartWhenUnblocked = HasStartWhenUnblockedIntent(task) +} + +// HasStartWhenUnblockedIntent reports whether the task carries a deferred launch +// intent that dependency resolution should consume. Delegates to the models +// helper, which owns the metadata key. +func HasStartWhenUnblockedIntent(task *models.Task) bool { + return models.HasStartWhenUnblockedIntent(task) +} diff --git a/apps/backend/internal/task/handlers/task_dependency_handlers.go b/apps/backend/internal/task/handlers/task_dependency_handlers.go new file mode 100644 index 0000000000..9ad65af798 --- /dev/null +++ b/apps/backend/internal/task/handlers/task_dependency_handlers.go @@ -0,0 +1,112 @@ +package handlers + +import ( + "errors" + "net/http" + + "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/task/dto" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/service" +) + +// dependencyProjection adapts the service's derived DependencyView to the DTO +// package's projection type. The two are kept separate so dto stays importable +// from service without a cycle. +func dependencyProjection(view service.DependencyView) dto.TaskDependencyProjection { + return dto.TaskDependencyProjection{ + Blocked: view.Blocked, + BlockedReason: view.BlockedReason, + DependsOn: dependencyRefs(view.DependsOn), + Blocks: dependencyRefs(view.Blocks), + } +} + +func dependencyRefs(refs []service.DependencyRef) []dto.TaskDependencyRefDTO { + if len(refs) == 0 { + return nil + } + out := make([]dto.TaskDependencyRefDTO, 0, len(refs)) + for _, r := range refs { + out = append(out, dto.TaskDependencyRefDTO{ + ID: r.ID, Title: r.Title, State: r.State, Status: r.Status, + }) + } + return out +} + +// Response keys repeated across the dependency handlers. +const ( + dependencyKeyError = "error" + dependencyKeyTaskID = "task_id" +) + +// addTaskDependencyBody is the POST /tasks/:id/dependencies payload. +type addTaskDependencyBody struct { + DependsOnTaskID string `json:"depends_on_task_id" binding:"required"` +} + +// httpAddTaskDependency handles POST /api/v1/tasks/:id/dependencies — +// "this task is blocked by depends_on_task_id". +// +// A cycle returns 409 with a `cycle` array so the frontend can render +// "A → B → C → A"; that body shape is what BlockersPicker already parses. +func (h *TaskHandlers) httpAddTaskDependency(c *gin.Context) { + taskID := c.Param("id") + var body addTaskDependencyBody + if err := c.ShouldBindJSON(&body); err != nil { + c.JSON(http.StatusBadRequest, gin.H{dependencyKeyError: "depends_on_task_id is required"}) + return + } + err := h.service.AddDependency(c.Request.Context(), taskID, body.DependsOnTaskID) + if err == nil { + h.respondWithDependencies(c, taskID) + return + } + var cycle *service.CycleError + if errors.As(err, &cycle) { + c.JSON(http.StatusConflict, gin.H{dependencyKeyError: cycle.Error(), "cycle": cycle.Path}) + return + } + if errors.Is(err, service.ErrDependencyRepositoryUnavailable) { + c.JSON(http.StatusServiceUnavailable, gin.H{dependencyKeyError: err.Error()}) + return + } + handleNotFound(c, h.logger, err, "task not found") +} + +// httpRemoveTaskDependency handles DELETE /api/v1/tasks/:id/dependencies/:depId. +// Removing an absent edge is a success no-op. +func (h *TaskHandlers) httpRemoveTaskDependency(c *gin.Context) { + taskID := c.Param("id") + if err := h.service.RemoveDependency(c.Request.Context(), taskID, c.Param("depId")); err != nil { + if errors.Is(err, service.ErrDependencyRepositoryUnavailable) { + c.JSON(http.StatusServiceUnavailable, gin.H{dependencyKeyError: err.Error()}) + return + } + handleNotFound(c, h.logger, err, "task not found") + return + } + h.respondWithDependencies(c, taskID) +} + +// respondWithDependencies returns the task's dependency projection after a +// mutation so the caller does not have to re-fetch the task to learn the result. +func (h *TaskHandlers) respondWithDependencies(c *gin.Context, taskID string) { + task, err := h.service.GetTask(c.Request.Context(), taskID) + if err != nil || task == nil { + c.JSON(http.StatusOK, gin.H{dependencyKeyTaskID: taskID}) + return + } + views := h.service.BuildDependencyViews(c.Request.Context(), []*models.Task{task}) + // Serialize through the same DTO adapter the task-list path uses, so a + // mutation response and a list response are one JSON contract. + projection := dependencyProjection(views[taskID]) + c.JSON(http.StatusOK, gin.H{ + dependencyKeyTaskID: taskID, + "blocked": projection.Blocked, + "blocked_reason": projection.BlockedReason, + "depends_on": projection.DependsOn, + "blocks": projection.Blocks, + }) +} diff --git a/apps/backend/internal/task/handlers/task_handlers.go b/apps/backend/internal/task/handlers/task_handlers.go index 68f8be1010..e96378a707 100644 --- a/apps/backend/internal/task/handlers/task_handlers.go +++ b/apps/backend/internal/task/handlers/task_handlers.go @@ -166,6 +166,12 @@ func (h *TaskHandlers) registerHTTP(router *gin.Engine) { api.POST("/tasks/:id/unarchive", h.httpUnarchiveTask) api.GET("/tasks/:id/subtask-count", h.httpTaskSubtaskCount) + // Task dependencies ("this task is blocked by that one"). Task-scoped + // equivalents of the Office-only blocker routes; both go through the single + // validator in the task service. + api.POST("/tasks/:id/dependencies", h.httpAddTaskDependency) + api.DELETE("/tasks/:id/dependencies/:depId", h.httpRemoveTaskDependency) + api.POST("/tasks/bulk-move", h.httpBulkMoveTasks) api.GET("/workflows/:id/task-count", h.httpGetWorkflowTaskCount) api.GET("/workflow/steps/:id/task-count", h.httpGetStepTaskCount) diff --git a/apps/backend/internal/task/handlers/task_http_handlers.go b/apps/backend/internal/task/handlers/task_http_handlers.go index 64af445d64..938d53eff8 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers.go +++ b/apps/backend/internal/task/handlers/task_http_handlers.go @@ -250,6 +250,10 @@ func buildTaskDTOsWithSessionInfo( if queuedErr != nil { log.Warn("failed to load queued prompt counts for task list, omitting badges", zap.Error(queuedErr)) } + // Dependency state is derived, never stored, so it is computed per read. One + // batched call for the whole list: a per-task query would add a round trip + // per card to every board load. + dependencyViews := svc.BuildDependencyViews(ctx, tasks) result := make([]dto.TaskDTO, 0, len(tasks)) for _, task := range tasks { sessions := sessionsByTask[task.ID] @@ -281,6 +285,7 @@ func buildTaskDTOsWithSessionInfo( ) taskDTO.TaskPendingAction = taskPendingActionPtr(sessions, pendingActionsBySession) dto.EnrichTaskForegroundActivity(&taskDTO, sessions, activityProvider) + dto.EnrichTaskDependencies(&taskDTO, dependencyProjection(dependencyViews[task.ID]), task) taskDTO.StatusSummary = statusSummaries[task.ID] if taskDTO.StatusSummary != nil { switch { @@ -752,7 +757,10 @@ type httpCreateTaskRequest struct { ParentID string `json:"parent_id,omitempty"` WorkspacePath string `json:"workspace_path,omitempty"` BlockedBy []string `json:"blocked_by,omitempty"` - ProjectID string `json:"project_id,omitempty"` + // StartWhenUnblocked records the agent start as an intent consumed by + // dependency resolution. nil derives it from StartAgent when BlockedBy is set. + StartWhenUnblocked *bool `json:"start_when_unblocked,omitempty"` + ProjectID string `json:"project_id,omitempty"` // ExternalID is a caller-supplied identity used for create-idempotency // (docs/specs/tasks/external-id-idempotency/spec.md). ExternalID string `json:"external_id,omitempty"` @@ -902,25 +910,26 @@ func (h *TaskHandlers) httpCreateTask(c *gin.Context) { } result, err := h.service.CreateTask(c.Request.Context(), &service.CreateTaskRequest{ - WorkspaceID: body.WorkspaceID, - WorkflowID: body.WorkflowID, - WorkflowStepID: body.WorkflowStepID, - Title: title, - Description: description, - AutoTitle: body.AutoTitle, - Autopilot: body.Autopilot, - Priority: body.Priority, - State: body.State, - Repositories: convertToServiceRepos(repos), - Position: body.Position, - Metadata: metadata, - DeferredLaunch: deferredLaunch, - PlanMode: body.PlanMode && !body.StartAgent, - ParentID: body.ParentID, - WorkspacePath: body.WorkspacePath, - BlockedBy: body.BlockedBy, - ProjectID: body.ProjectID, - ExternalID: body.ExternalID, + WorkspaceID: body.WorkspaceID, + WorkflowID: body.WorkflowID, + WorkflowStepID: body.WorkflowStepID, + Title: title, + Description: description, + AutoTitle: body.AutoTitle, + Autopilot: body.Autopilot, + Priority: body.Priority, + State: body.State, + Repositories: convertToServiceRepos(repos), + Position: body.Position, + Metadata: metadata, + DeferredLaunch: deferredLaunch, + PlanMode: body.PlanMode && !body.StartAgent, + ParentID: body.ParentID, + WorkspacePath: body.WorkspacePath, + BlockedBy: body.BlockedBy, + StartWhenUnblocked: body.StartWhenUnblocked, + ProjectID: body.ProjectID, + ExternalID: body.ExternalID, }) if err != nil { handleNotFound(c, h.logger, err, "task not created") @@ -987,7 +996,19 @@ func (h *TaskHandlers) httpCreateTask(c *gin.Context) { // prepareTaskSession's doc comment for why. dispatch is nil unless a // start_agent create genuinely prepared a session; it is only ever acted // on below once settlement has succeeded. - dispatch := h.prepareTaskSession(c, &response, taskDTO.ID, body, resolvedStepID, task.QueuedForStepID == "") + // A create that declared dependencies records its start as a + // start-when-unblocked intent instead of launching now; dependency + // resolution consumes it later. + startWhenUnblocked := service.ResolveStartWhenUnblocked(&service.CreateTaskRequest{ + BlockedBy: body.BlockedBy, StartWhenUnblocked: body.StartWhenUnblocked, + }) + response.StartWhenUnblocked = startWhenUnblocked + + // Blockers suppress the immediate launch regardless of start_when_unblocked: + // that flag governs the deferred launch only, so `false` must not mean + // "start the blocked task now". + dispatch := h.prepareTaskSession(c, &response, taskDTO.ID, body, resolvedStepID, + task.QueuedForStepID == "" && len(body.BlockedBy) == 0) // Settlement (create-sequence step 7): after all required synchronous // work above — including session prepare — and before any asynchronous diff --git a/apps/backend/internal/task/models/deferred_launch_test.go b/apps/backend/internal/task/models/deferred_launch_test.go new file mode 100644 index 0000000000..7f3faa6dae --- /dev/null +++ b/apps/backend/internal/task/models/deferred_launch_test.go @@ -0,0 +1,43 @@ +package models + +import "testing" + +func launchTask(record map[string]interface{}) *Task { + return &Task{Metadata: map[string]interface{}{MetaKeyDeferredLaunch: record}} +} + +// A WIP-overflow record is stale the moment the task is admitted, so admission +// drops it. That has been true since the WIP queue shipped. +func TestDropWIPDeferredLaunchRemovesAnOverflowRecord(t *testing.T) { + task := launchTask(map[string]interface{}{"intent": "start"}) + + DropWIPDeferredLaunch(task) + + if _, ok := task.Metadata[MetaKeyDeferredLaunch]; ok { + t.Fatal("an admitted task must not keep its WIP overflow launch record") + } +} + +// The regression this helper exists for. A dependency chain step is admitted at +// create time like any other task, so deleting the shared record on admission +// erased the intent before a predecessor could ever resolve it, and the whole +// chain silently never ran. The record is waiting on a predecessor, not on +// capacity, so admission has nothing to say about it. +func TestDropWIPDeferredLaunchKeepsAStartWhenUnblockedIntent(t *testing.T) { + task := launchTask(map[string]interface{}{ + "intent": "start", + DeferredLaunchStartWhenUnblockedKey: true, + }) + + DropWIPDeferredLaunch(task) + + if !HasStartWhenUnblockedIntent(task) { + t.Fatal("WIP admission must not consume a dependency-chain launch intent") + } +} + +func TestDropWIPDeferredLaunchToleratesEmptyTasks(t *testing.T) { + DropWIPDeferredLaunch(nil) + DropWIPDeferredLaunch(&Task{}) + DropWIPDeferredLaunch(&Task{Metadata: map[string]interface{}{}}) +} diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index 1d0aeeade0..7f471cca68 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -95,6 +95,12 @@ const ( MetaKeyExecutorID = "executor_id" MetaKeyExecutorProfileID = "executor_profile_id" MetaKeyDeferredLaunch = "deferred_launch" + // DeferredLaunchStartWhenUnblockedKey marks a deferred launch intent as + // belonging to a task dependency chain rather than to WIP overflow. The + // record itself is identical; the flag lets dependency resolution recognise + // its own intents and keeps a WIP-only intent from being read as a chain + // step (and vice versa). + DeferredLaunchStartWhenUnblockedKey = "start_when_unblocked" // MetaKeyWorkspacePath is the optional host folder for repo-less tasks // (set by CreateTask, read by the orchestrator when building a session). // Centralised here so the set/read sites can't drift apart. @@ -756,6 +762,45 @@ type ChildCompletionRow struct { UpdatedAt time.Time `json:"updated_at" db:"updated_at"` } +// HasStartWhenUnblockedIntent reports whether a task's deferred launch intent +// belongs to a dependency chain rather than to WIP overflow. +// +// The two share one record; DeferredLaunchStartWhenUnblockedKey is what tells +// them apart. Defined here, next to the key, so the DTO layer, the task service +// and the orchestrator cannot drift on what counts as a chain intent. +func HasStartWhenUnblockedIntent(task *Task) bool { + if task == nil || task.Metadata == nil { + return false + } + raw, ok := task.Metadata[MetaKeyDeferredLaunch] + if !ok { + return false + } + launch, ok := raw.(map[string]interface{}) + if !ok { + return false + } + flag, ok := launch[DeferredLaunchStartWhenUnblockedKey].(bool) + return ok && flag +} + +// DropWIPDeferredLaunch removes a deferred launch record that only existed +// because the task was waiting on WIP capacity. +// +// Every WIP admission point calls this instead of deleting the key outright. +// An admitted task has nothing left to wait for as far as WIP is concerned, so +// its overflow record is stale. A start-when-unblocked intent is NOT stale in +// that situation: it is waiting on a predecessor, not on capacity, and the two +// share this one metadata record. Deleting it on admission silently unmakes +// every dependency chain, because admission happens at create time for any task +// entering a step with room. +func DropWIPDeferredLaunch(task *Task) { + if task == nil || task.Metadata == nil || HasStartWhenUnblockedIntent(task) { + return + } + delete(task.Metadata, MetaKeyDeferredLaunch) +} + // IsTerminalTaskState reports whether a task state means no further child work // is expected from that task. func IsTerminalTaskState(state v1.TaskState) bool { diff --git a/apps/backend/internal/task/repository/sqlite/task.go b/apps/backend/internal/task/repository/sqlite/task.go index 80d27499c5..f752908863 100644 --- a/apps/backend/internal/task/repository/sqlite/task.go +++ b/apps/backend/internal/task/repository/sqlite/task.go @@ -169,7 +169,7 @@ func runnerProjection(alias string) string { func (r *Repository) CreateTask(ctx context.Context, task *models.Task) error { if task.WorkflowStepID != "" && task.QueuedForStepID == "" && !task.IsEphemeral { task.WIPAdmitted = true - delete(task.Metadata, models.MetaKeyDeferredLaunch) + models.DropWIPDeferredLaunch(task) } return r.createTask(ctx, task, "", 0) } @@ -202,7 +202,7 @@ func (r *Repository) CreateTaskWithWorkflowStepAdmission( task.WIPAdmitted = !task.IsEphemeral task.QueuedForStepID = "" task.QueuedAt = nil - delete(task.Metadata, models.MetaKeyDeferredLaunch) + models.DropWIPDeferredLaunch(task) return r.CreateTask(ctx, task) } @@ -248,7 +248,7 @@ func (r *Repository) applyAdmissionPlacement( task.WIPAdmitted = true task.QueuedForStepID = "" task.QueuedAt = nil - delete(task.Metadata, models.MetaKeyDeferredLaunch) + models.DropWIPDeferredLaunch(task) case feederStepID == "": task.WorkflowStepID = targetStepID task.WIPAdmitted = false diff --git a/apps/backend/internal/task/service/handoff_workspace_test.go b/apps/backend/internal/task/service/handoff_workspace_test.go index ad7f7b9969..403574adb5 100644 --- a/apps/backend/internal/task/service/handoff_workspace_test.go +++ b/apps/backend/internal/task/service/handoff_workspace_test.go @@ -207,6 +207,38 @@ func (f *fakeBlockerRepo) ListTasksBlockedBy(_ context.Context, blockerTaskID st return ids, nil } +func (f *fakeBlockerRepo) ListBlockersForTasks(_ context.Context, taskIDs []string) (map[string][]string, error) { + f.mu.Lock() + defer f.mu.Unlock() + want := make(map[string]struct{}, len(taskIDs)) + for _, id := range taskIDs { + want[id] = struct{}{} + } + out := map[string][]string{} + for _, b := range f.blockers { + if _, ok := want[b.TaskID]; ok { + out[b.TaskID] = append(out[b.TaskID], b.BlockerTaskID) + } + } + return out, nil +} + +func (f *fakeBlockerRepo) ListDependentsForTasks(_ context.Context, blockerTaskIDs []string) (map[string][]string, error) { + f.mu.Lock() + defer f.mu.Unlock() + want := make(map[string]struct{}, len(blockerTaskIDs)) + for _, id := range blockerTaskIDs { + want[id] = struct{}{} + } + out := map[string][]string{} + for _, b := range f.blockers { + if _, ok := want[b.BlockerTaskID]; ok { + out[b.BlockerTaskID] = append(out[b.BlockerTaskID], b.TaskID) + } + } + return out, nil +} + // fakeTaskRepo provides the minimal TaskRepository surface AttachWorkspacePolicy // uses. It supports GetTask + ListChildren so sibling lookup works. type fakeTaskRepo struct { diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index 9fedddef01..3573b40811 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -312,11 +312,15 @@ type Service struct { remoteBranchLister RemoteBranchLister repoCloneLocation RepoCloneLocation blockers BlockerRepository - comments CommentRepository - secretStore secrets.SecretStore - workspaceSecretDeleter WorkspaceSecretDeleter - baseBranchPusher AgentBaseBranchPusher - runtimeOverridesMu sync.Mutex + // dependencyEdgeMu serializes validate-then-insert for dependency edges so + // two concurrent adds cannot each pass a cycle walk that predates the + // other's insert and commit a cycle between them. + dependencyEdgeMu sync.Mutex + comments CommentRepository + secretStore secrets.SecretStore + workspaceSecretDeleter WorkspaceSecretDeleter + baseBranchPusher AgentBaseBranchPusher + runtimeOverridesMu sync.Mutex workspaceSourceProviderRefresher WorkspaceSourceProviderRefresher diff --git a/apps/backend/internal/task/service/service_dependencies.go b/apps/backend/internal/task/service/service_dependencies.go new file mode 100644 index 0000000000..656ec0ef41 --- /dev/null +++ b/apps/backend/internal/task/service/service_dependencies.go @@ -0,0 +1,643 @@ +package service + +import ( + "context" + "errors" + "fmt" + "strings" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/task/models" + taskrepo "github.com/kandev/kandev/internal/task/repository" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// Task dependencies ("task B is blocked by task A") are peer-to-peer edges, +// distinct from the parent/child subtask hierarchy: a subtask means "part of", +// a dependency means "not until". Edges live in task_blockers and are read +// through BlockerRepository. +// +// Blocked state is DERIVED on every read, never stored. A denormalized +// is_blocked column would be read by the auto-start gate, and a stale value +// there would launch work whose predecessor never ran — the one failure this +// feature exists to prevent. + +// Dependency resolution verdicts for a single predecessor. +const ( + // DependencyResolved means the predecessor finished successfully. + DependencyResolved = "resolved" + // DependencyFailed means the predecessor reached a terminal state that is + // not success. It never resolves the edge. + DependencyFailed = "failed" + // DependencyPending means the predecessor has not finished. Archived + // predecessors are pending: archival is neither success nor failure. + DependencyPending = "pending" + // dependencyMissing marks an edge whose predecessor row is gone. Internal + // only: such edges are dropped from the projection rather than reported. + dependencyMissing = "missing" +) + +// Blocked reasons reported on the task payload. +const ( + // BlockedReasonPending — at least one predecessor is unfinished. + BlockedReasonPending = "pending" + // BlockedReasonFailed — no predecessor is unfinished and at least one + // failed, so the chain has halted and needs human action. + BlockedReasonFailed = "failed" + // BlockedReasonUnknown — the dependency store could not be read. The gate + // fails closed: unknown counts as blocked. + BlockedReasonUnknown = "unknown" +) + +// dependencyCycleWalkLimit bounds the BFS in checkDependencyCycle. A walk that +// exceeds it rejects the edge rather than accepting an unverified one. +const dependencyCycleWalkLimit = 1000 + +// DependencyRef is one end of a dependency edge, carrying enough detail for the +// dependency chip to render without fetching each related task. +type DependencyRef struct { + ID string `json:"id"` + Title string `json:"title"` + State v1.TaskState `json:"state"` + Status string `json:"status,omitempty"` +} + +// DependencyView is the derived dependency state for one task. +type DependencyView struct { + Blocked bool + BlockedReason string + DependsOn []DependencyRef + Blocks []DependencyRef +} + +// CycleError is returned when a proposed edge would close a cycle. Path lists +// the task IDs in traversal order (A, B, C, A) so callers can render +// "A → B → C → A". +type CycleError struct { + Path []string +} + +// Error implements the error interface. +func (e *CycleError) Error() string { + if len(e.Path) == 0 { + return "would create a dependency cycle" + } + return "would create a dependency cycle: " + strings.Join(e.Path, " → ") +} + +// ErrDependencyRepositoryUnavailable is returned when the dependency store is +// not wired. Callers must treat it as "cannot determine", not "not blocked". +var ErrDependencyRepositoryUnavailable = fmt.Errorf("dependency repository not configured") + +// ResolveStartWhenUnblocked decides whether a create request's agent start +// should become a start-when-unblocked intent instead of an immediate launch. +// +// A create with no dependencies is never deferred by this rule. A create WITH +// dependencies defaults to deferring, because `start_agent` defaults to true and +// every automated caller passes it: launching immediately would start every step +// of an agent-built chain at once, which is precisely the collision dependencies +// exist to prevent. An explicit `start_when_unblocked: false` opts out and +// creates the edges with no launch intent at all. +func ResolveStartWhenUnblocked(req *CreateTaskRequest) bool { + if req == nil || len(req.BlockedBy) == 0 { + return false + } + if req.StartWhenUnblocked == nil { + return true + } + return *req.StartWhenUnblocked +} + +// validateDependencyPair resolves both ends and rejects a cross-workspace edge. +// Both tasks must exist: an edge to a missing task would leave the dependent +// blocked on something that can never complete. +func (s *Service) validateDependencyPair(ctx context.Context, taskID, dependsOnTaskID string) error { + task, err := s.tasks.GetTask(ctx, taskID) + if err != nil { + return fmt.Errorf("resolve task: %w", err) + } + if task == nil { + return fmt.Errorf("%w: %s", taskrepo.ErrTaskNotFound, taskID) + } + dep, err := s.tasks.GetTask(ctx, dependsOnTaskID) + if err != nil { + return fmt.Errorf("resolve dependency task: %w", err) + } + if dep == nil { + return fmt.Errorf("%w: %s", taskrepo.ErrTaskNotFound, dependsOnTaskID) + } + if task.WorkspaceID != "" && dep.WorkspaceID != "" && task.WorkspaceID != dep.WorkspaceID { + return fmt.Errorf("task %s belongs to a different workspace", dependsOnTaskID) + } + return nil +} + +// authorizeDependencyPair guards BOTH ends of an edge before any read or write. +// +// Both IDs are caller-supplied, so guarding only the dependent would let a +// caller link (or unlink) another user's task and infer its existence from the +// validation error. Denials surface as ErrTaskNotFound, so there is no +// existence leak. +func (s *Service) authorizeDependencyPair(ctx context.Context, taskID, dependsOnTaskID string) error { + if err := s.authorizeTaskID(ctx, taskID); err != nil { + return err + } + return s.authorizeTaskID(ctx, dependsOnTaskID) +} + +// AddDependency records "taskID depends on dependsOnTaskID". +// +// This is the single validator for dependency edges. Self-edges, +// cross-workspace edges, and cycles of any length are rejected here, and both +// the task-scoped routes and the Office blocker routes go through it — a second +// validator would let a cycle in through whichever path was weakest. +func (s *Service) AddDependency(ctx context.Context, taskID, dependsOnTaskID string) error { + if s.blockers == nil { + return ErrDependencyRepositoryUnavailable + } + if taskID == "" || dependsOnTaskID == "" { + return fmt.Errorf("both task_id and depends_on_task_id are required") + } + if taskID == dependsOnTaskID { + return fmt.Errorf("a task cannot depend on itself") + } + if err := s.authorizeDependencyPair(ctx, taskID, dependsOnTaskID); err != nil { + return err + } + // Serialize validate-then-insert. The cycle walk and the insert are two + // statements, so two concurrent adds can each pass a walk that cannot yet + // see the other's edge and commit a cycle between them, leaving both tasks + // blocked forever. Kandev runs one backend process (see the run-scheduling + // spec, which rules out multiple active processes), so a process-wide lock + // is sufficient and keeps the check and the write atomic with respect to + // each other. + if err := s.insertDependencyEdge(ctx, taskID, dependsOnTaskID); err != nil { + return err + } + // Publish OUTSIDE the lock: event delivery is synchronous, so holding the + // lock across it would serialize every dependency mutation behind fan-out, + // and a subscriber that called back into this method would deadlock on a + // non-reentrant mutex. + s.publishDependencyChange(ctx, taskID, dependsOnTaskID) + return nil +} + +// insertDependencyEdge holds dependencyEdgeMu across exactly the validate, +// cycle-walk and insert, and nothing else. +func (s *Service) insertDependencyEdge(ctx context.Context, taskID, dependsOnTaskID string) error { + s.dependencyEdgeMu.Lock() + defer s.dependencyEdgeMu.Unlock() + if err := s.validateDependencyPair(ctx, taskID, dependsOnTaskID); err != nil { + return err + } + if cycle, err := s.checkDependencyCycle(ctx, taskID, dependsOnTaskID); err != nil { + return fmt.Errorf("check dependency cycle: %w", err) + } else if cycle != nil { + return cycle + } + return s.createBlockerEdge(ctx, taskID, dependsOnTaskID) +} + +// RemoveDependency deletes a dependency edge. Removing an absent edge is a +// success no-op. +// +// Removing the last edge unblocks the task but deliberately does NOT consume +// its start-when-unblocked intent: that intent is consumed by dependency +// resolution, not by the absence of dependencies. A user who removes the edge +// is taking manual control. +func (s *Service) RemoveDependency(ctx context.Context, taskID, dependsOnTaskID string) error { + if s.blockers == nil { + return ErrDependencyRepositoryUnavailable + } + if err := s.authorizeDependencyPair(ctx, taskID, dependsOnTaskID); err != nil { + return err + } + if err := s.deleteDependencyEdge(ctx, taskID, dependsOnTaskID); err != nil { + return err + } + // Published outside the lock, for the same reason as AddDependency. + s.publishDependencyChange(ctx, taskID, dependsOnTaskID) + return nil +} + +// deleteDependencyEdge holds the edge lock across the delete only. +func (s *Service) deleteDependencyEdge(ctx context.Context, taskID, dependsOnTaskID string) error { + s.dependencyEdgeMu.Lock() + defer s.dependencyEdgeMu.Unlock() + return s.blockers.DeleteTaskBlocker(ctx, taskID, dependsOnTaskID) +} + +// publishDependencyChange emits task.updated for both ends of a mutated edge so +// every client refreshes its badge, chip and graph. Task mutations must go +// through the event publisher; walking the repository alone breaks WS-driven UI. +func (s *Service) publishDependencyChange(ctx context.Context, taskIDs ...string) { + for _, id := range taskIDs { + if id == "" { + continue + } + task, err := s.tasks.GetTask(ctx, id) + if err != nil || task == nil { + continue + } + // Carry the recomputed projection on the event. A bare task.updated + // omits these keys, and the client reads an omitted projection as + // "unchanged" (it must, because most task.updated publishers are + // lightweight) — so without them every other open board would keep a + // stale chip and badge until a full refetch. + s.publishTaskEventWithExtra(ctx, events.TaskUpdated, task, nil, s.dependencyEventFields(ctx, task)) + } +} + +// dependencyEventFields renders one task's derived projection in the wire shape +// the client's task.updated mapper reads. +func (s *Service) dependencyEventFields(ctx context.Context, task *models.Task) map[string]interface{} { + views := s.BuildDependencyViews(ctx, []*models.Task{task}) + view := views[task.ID] + return map[string]interface{}{ + "blocked": view.Blocked, + "blocked_reason": view.BlockedReason, + "depends_on": view.DependsOn, + "blocks": view.Blocks, + } +} + +// checkDependencyCycle walks forward from dependsOnTaskID through existing +// edges. If any path reaches taskID, adding the edge would close a cycle, and +// the returned *CycleError carries the path for the caller to surface. +func (s *Service) checkDependencyCycle(ctx context.Context, taskID, dependsOnTaskID string) (*CycleError, error) { + // parent[x] is the node we reached x from, so a hit can be walked back + // into a readable path instead of only reporting "there is a cycle". + parent := map[string]string{dependsOnTaskID: ""} + queue := []string{dependsOnTaskID} + walked := 0 + for len(queue) > 0 { + if walked > dependencyCycleWalkLimit { + return &CycleError{}, nil + } + current := queue[0] + queue = queue[1:] + walked++ + if current == taskID { + return &CycleError{Path: buildCyclePath(parent, taskID, dependsOnTaskID)}, nil + } + blockers, err := s.blockers.ListTaskBlockers(ctx, current) + if err != nil { + return nil, err + } + for _, b := range blockers { + if _, seen := parent[b.BlockerTaskID]; seen { + continue + } + parent[b.BlockerTaskID] = current + queue = append(queue, b.BlockerTaskID) + } + } + return nil, nil +} + +// buildCyclePath renders the discovered cycle in depends-on order, starting and +// ending at the task the new edge would block: taskID → dependsOn → … → taskID. +// +// parent[x] is the node whose blocker list contained x, so walking parent back +// from taskID yields the chain from taskID to dependsOn reversed. The proposed +// edge (taskID depends on dependsOn) closes the loop, so taskID is PREPENDED — +// the reversed chain already ends at taskID, and appending it again produced a +// duplicated final hop like "A → B → C → C" instead of "C → A → B → C". +func buildCyclePath(parent map[string]string, taskID, dependsOnTaskID string) []string { + reverse := []string{taskID} + for node := parent[taskID]; node != ""; node = parent[node] { + reverse = append(reverse, node) + if node == dependsOnTaskID { + break + } + } + path := make([]string, 0, len(reverse)+1) + path = append(path, taskID) + for i := len(reverse) - 1; i >= 0; i-- { + path = append(path, reverse[i]) + } + return path +} + +// BuildDependencyViews returns derived dependency state for a batch of tasks. +// +// Batched deliberately: the Kanban board reads a whole workflow at once, so a +// per-task query would add one round trip per card to every board load. +func (s *Service) BuildDependencyViews(ctx context.Context, tasks []*models.Task) map[string]DependencyView { + out := make(map[string]DependencyView, len(tasks)) + if s.blockers == nil || len(tasks) == 0 { + return out + } + ids := make([]string, 0, len(tasks)) + for _, t := range tasks { + if t != nil { + ids = append(ids, t.ID) + } + } + predecessors, err := s.blockers.ListBlockersForTasks(ctx, ids) + if err != nil { + s.logger.Warn("failed to load task dependencies", zap.Error(err)) + // Fail closed: a task whose edges cannot be read reports blocked with + // an unknown reason rather than silently reporting "not blocked". + for _, id := range ids { + out[id] = DependencyView{Blocked: true, BlockedReason: BlockedReasonUnknown} + } + return out + } + dependents, err := s.blockers.ListDependentsForTasks(ctx, ids) + if err != nil { + s.logger.Warn("failed to load task dependents", zap.Error(err)) + dependents = map[string][]string{} + } + refs := s.resolveDependencyRefs(ctx, predecessors, dependents) + for _, id := range ids { + out[id] = buildDependencyView(refs, predecessors[id], dependents[id]) + } + return out +} + +// resolveDependencyRefs loads title/state for every task named on either side +// of any edge in the batch, in one query. +func (s *Service) resolveDependencyRefs( + ctx context.Context, predecessors, dependents map[string][]string, +) map[string]DependencyRef { + seen := map[string]struct{}{} + for _, group := range []map[string][]string{predecessors, dependents} { + for _, ids := range group { + for _, id := range ids { + seen[id] = struct{}{} + } + } + } + refs := make(map[string]DependencyRef, len(seen)) + ids := make([]string, 0, len(seen)) + for id := range seen { + ids = append(ids, id) + } + // One batched read: a board payload references every edge on every card, so + // a per-edge query turned one board load into N round trips. + found, batchErr := s.tasks.GetTasksByIDs(ctx, ids) + byID := make(map[string]*models.Task, len(found)) + for _, task := range found { + if task != nil { + byID[task.ID] = task + } + } + for _, id := range ids { + task := byID[id] + if task == nil { + // Absent row (deleted predecessor, dangling edge) versus a failed + // read: the former must not block forever, the latter must not open + // the gate. Mark them differently so buildDependencyView can tell. + status := dependencyMissing + if batchErr != nil { + status = DependencyPending + } + refs[id] = DependencyRef{ID: id, Status: status} + continue + } + refs[id] = DependencyRef{ + ID: id, + Title: task.Title, + State: task.State, + Status: DependencyStatusForTask(task), + } + } + return refs +} + +// buildDependencyView derives blocked/reason plus both edge lists for one task. +func buildDependencyView( + refs map[string]DependencyRef, predecessorIDs, dependentIDs []string, +) DependencyView { + view := DependencyView{ + DependsOn: make([]DependencyRef, 0, len(predecessorIDs)), + Blocks: make([]DependencyRef, 0, len(dependentIDs)), + } + tally := dependencyTally{} + for _, id := range predecessorIDs { + ref := refs[id] + if ref.ID == "" { + ref = DependencyRef{ID: id, Status: DependencyPending} + } + if ref.Status == dependencyMissing { + // Dangling edge to a deleted task: not shown and not counted, so a + // failed cleanup cannot block a dependent forever. + continue + } + view.DependsOn = append(view.DependsOn, ref) + tally.add(ref.Status) + } + for _, id := range dependentIDs { + ref := refs[id] + if ref.ID == "" { + ref = DependencyRef{ID: id, Status: DependencyPending} + } + view.Blocks = append(view.Blocks, ref) + } + view.Blocked, view.BlockedReason = tally.verdict() + return view +} + +// DependencyStatusForTask classifies one predecessor. +// +// Resolution requires SUCCESS. This is deliberately stricter than the +// on_children_completed trigger, which counts FAILED as terminal: a chain must +// never proceed on a failed step. Archived tasks are pending, because archival +// is neither success nor failure. +func DependencyStatusForTask(task *models.Task) string { + if task == nil { + return DependencyPending + } + switch task.State { + case v1.TaskStateCompleted: + if task.ArchivedAt != nil { + return DependencyPending + } + return DependencyResolved + case v1.TaskStateFailed, v1.TaskStateCancelled: + return DependencyFailed + default: + return DependencyPending + } +} + +// DependencyGate reports whether taskID may be started by an automated path. +// +// Returns blocked=true on ANY read error. The gate fails closed on purpose: +// failing open would launch work whose predecessor may not have run. +func (s *Service) DependencyGate(ctx context.Context, taskID string) (blocked bool, reason string, err error) { + if s.blockers == nil { + // No dependency store wired means no edges can exist, so nothing is + // blocked. This is not a read failure. + return false, "", nil + } + blockers, listErr := s.blockers.ListTaskBlockers(ctx, taskID) + if listErr != nil { + return true, BlockedReasonUnknown, listErr + } + if len(blockers) == 0 { + return false, "", nil + } + tally := dependencyTally{} + for _, b := range blockers { + predecessor, getErr := s.tasks.GetTask(ctx, b.BlockerTaskID) + if getErr != nil && !errors.Is(getErr, taskrepo.ErrTaskNotFound) { + return true, BlockedReasonUnknown, getErr + } + if predecessor == nil { + // The predecessor row is genuinely gone: this is a dangling edge + // left behind when delete-time cleanup failed. Counting it as + // pending would block the dependent forever, since a deleted task + // can never reach a terminal state. A deleted predecessor is + // specified to unblock (without firing a launch intent), so ignore + // the edge and prune it opportunistically. + // + // This is NOT the fail-open case: a read ERROR above still fails + // closed. Only a confirmed-absent row is ignored. + s.pruneDanglingEdge(ctx, taskID, b.BlockerTaskID) + continue + } + tally.add(DependencyStatusForTask(predecessor)) + } + blocked, reason = tally.verdict() + return blocked, reason, nil +} + +// dependencyTally accumulates predecessor verdicts and applies the single +// precedence rule. The gate and the derived projection both use it: two copies +// of "pending wins over failed" could drift, and the fail-closed behaviour is +// exactly the thing that must not. +type dependencyTally struct { + anyPending bool + anyFailed bool +} + +func (d *dependencyTally) add(status string) { + switch status { + case DependencyFailed: + d.anyFailed = true + case DependencyResolved, dependencyMissing: + // Resolved satisfies the edge; missing is a dangling edge that is + // dropped entirely, so neither blocks. + default: + d.anyPending = true + } +} + +func (d *dependencyTally) verdict() (bool, string) { + switch { + case d.anyPending: + return true, BlockedReasonPending + case d.anyFailed: + return true, BlockedReasonFailed + } + return false, "" +} + +// PendingDependencyLaunch is a task that will start on its own once its +// dependencies resolve. +type PendingDependencyLaunch struct { + TaskID string + WorkflowStepID string +} + +// ListPendingDependencyLaunches returns every non-archived task that has +// dependency edges AND a start-when-unblocked intent. +// +// Backs the orchestrator's startup sweep: task.dependencies_resolved is +// in-memory and is not replayed, so a restart between a predecessor completing +// and its dependent launching would otherwise stall the chain silently. Scoped +// by "has edges" so the sweep is bounded by pending chain steps, not the board. +func (s *Service) ListPendingDependencyLaunches(ctx context.Context) ([]PendingDependencyLaunch, error) { + if s.blockers == nil { + return nil, nil + } + lister, ok := s.blockers.(dependentTaskLister) + if !ok { + return nil, nil + } + ids, err := lister.ListTasksWithDependencies(ctx) + if err != nil { + return nil, err + } + out := make([]PendingDependencyLaunch, 0, len(ids)) + for _, id := range ids { + task, err := s.tasks.GetTask(ctx, id) + if err != nil || task == nil || task.ArchivedAt != nil || task.IsEphemeral { + continue + } + if !HasStartWhenUnblockedIntent(task) { + continue + } + out = append(out, PendingDependencyLaunch{TaskID: task.ID, WorkflowStepID: task.WorkflowStepID}) + } + return out, nil +} + +// dependentTaskLister is the optional "which tasks have edges" read, kept off +// BlockerRepository so existing test doubles keep compiling. +type dependentTaskLister interface { + ListTasksWithDependencies(ctx context.Context) ([]string, error) +} + +// HasStartWhenUnblockedIntent reports whether the task's deferred launch intent +// belongs to a dependency chain rather than to WIP overflow. +// +// Thin re-export of the models helper so the orchestrator, the DTO layer and +// this service cannot drift on what counts as a chain intent. +func HasStartWhenUnblockedIntent(task *models.Task) bool { + return models.HasStartWhenUnblockedIntent(task) +} + +// pruneDanglingEdge removes an edge whose predecessor row no longer exists. +// +// Best-effort and non-fatal: the caller has already decided to ignore the edge, +// so a failed prune only means the next read prunes it again. +func (s *Service) pruneDanglingEdge(ctx context.Context, taskID, missingTaskID string) { + if s.blockers == nil { + return + } + if err := s.blockers.DeleteTaskBlocker(ctx, taskID, missingTaskID); err != nil { + s.logger.Warn("failed to prune dangling dependency edge", + zap.String("task_id", taskID), + zap.String("missing_task_id", missingTaskID), + zap.Error(err)) + } +} + +// ListDependentTaskIDs returns the tasks directly blocked by taskID. +func (s *Service) ListDependentTaskIDs(ctx context.Context, taskID string) ([]string, error) { + if s.blockers == nil { + return nil, ErrDependencyRepositoryUnavailable + } + return s.blockers.ListTasksBlockedBy(ctx, taskID) +} + +// deleteDependencyEdgesForTask removes both directions of every edge touching +// taskID. task_blockers predates the tasks foreign key, so nothing cascades. +func (s *Service) deleteDependencyEdgesForTask(ctx context.Context, taskID string) { + if s.blockers == nil { + return + } + cleaner, ok := s.blockers.(taskDependencyCleaner) + if !ok { + return + } + dependents, err := s.blockers.ListTasksBlockedBy(ctx, taskID) + if err != nil { + s.logger.Warn("failed to list dependents before edge cleanup", + zap.String("task_id", taskID), zap.Error(err)) + } + if err := cleaner.DeleteTaskBlockersForTask(ctx, taskID); err != nil { + s.logger.Warn("failed to clean up dependency edges for deleted task", + zap.String("task_id", taskID), zap.Error(err)) + return + } + // Dependents may now be unblocked, so refresh them. Deliberately no + // auto-start: deletion is not success, and a chain must not advance + // because a predecessor was removed. + s.publishDependencyChange(ctx, dependents...) +} diff --git a/apps/backend/internal/task/service/service_dependencies_launch_test.go b/apps/backend/internal/task/service/service_dependencies_launch_test.go new file mode 100644 index 0000000000..442e1413d6 --- /dev/null +++ b/apps/backend/internal/task/service/service_dependencies_launch_test.go @@ -0,0 +1,22 @@ +package service + +import "testing" + +// Revive caps effective lines per file including tests, so the launch-semantics +// cases live here rather than growing service_dependencies_test.go. + +// `start_when_unblocked: false` means "no automatic start at all" — it governs +// the DEFERRED launch only. The presence of blockers is what suppresses the +// immediate one, so a create must never launch a task that is already blocked. +func TestResolveStartWhenUnblockedGovernsOnlyTheDeferredLaunch(t *testing.T) { + no := false + req := &CreateTaskRequest{BlockedBy: []string{"a"}, StartWhenUnblocked: &no} + if ResolveStartWhenUnblocked(req) { + t.Fatal("explicit false must not record a deferred intent") + } + // The create paths gate the immediate launch on len(BlockedBy) == 0, not on + // this flag; assert the invariant the handlers rely on. + if len(req.BlockedBy) == 0 { + t.Fatal("fixture should have blockers") + } +} diff --git a/apps/backend/internal/task/service/service_dependencies_test.go b/apps/backend/internal/task/service/service_dependencies_test.go new file mode 100644 index 0000000000..0acb461f5d --- /dev/null +++ b/apps/backend/internal/task/service/service_dependencies_test.go @@ -0,0 +1,442 @@ +package service + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + orchmodels "github.com/kandev/kandev/internal/office/models" + "github.com/kandev/kandev/internal/task/models" + taskrepo "github.com/kandev/kandev/internal/task/repository" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// errBlockerRepo wraps a working repo and fails the forward read on demand, so +// the fail-closed contract can be proven by breaking the store rather than by +// asserting the happy path. +type errBlockerRepo struct { + *mockBlockerRepo + failList bool +} + +func (e *errBlockerRepo) ListTaskBlockers(ctx context.Context, taskID string) ([]*orchmodels.TaskBlocker, error) { + if e.failList { + return nil, errors.New("boom") + } + return e.mockBlockerRepo.ListTaskBlockers(ctx, taskID) +} + +func (e *errBlockerRepo) ListBlockersForTasks(ctx context.Context, ids []string) (map[string][]string, error) { + if e.failList { + return nil, errors.New("boom") + } + return e.mockBlockerRepo.ListBlockersForTasks(ctx, ids) +} + +func setDependencyState(t *testing.T, svc *Service, taskID string, state v1.TaskState) { + t.Helper() + task, err := svc.tasks.GetTask(context.Background(), taskID) + if err != nil || task == nil { + t.Fatalf("load task %s: %v", taskID, err) + } + task.State = state + if err := svc.tasks.UpdateTask(context.Background(), task); err != nil { + t.Fatalf("update task %s: %v", taskID, err) + } +} + +func TestDependencyGate_PendingBlocksAndResolvedReleases(t *testing.T) { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + ctx := context.Background() + predecessor := mustSeedTask(t, svc, "Predecessor").ID + dependent := mustSeedTask(t, svc, "Dependent").ID + if err := svc.AddDependency(ctx, dependent, predecessor); err != nil { + t.Fatalf("AddDependency: %v", err) + } + + blocked, reason, err := svc.DependencyGate(ctx, dependent) + if err != nil || !blocked || reason != BlockedReasonPending { + t.Fatalf("pending predecessor: blocked=%v reason=%q err=%v; want true/pending/nil", blocked, reason, err) + } + + setDependencyState(t, svc, predecessor, v1.TaskStateCompleted) + blocked, reason, err = svc.DependencyGate(ctx, dependent) + if err != nil || blocked { + t.Fatalf("resolved predecessor: blocked=%v reason=%q err=%v; want false", blocked, reason, err) + } +} + +// A FAILED predecessor must NOT resolve the edge. This is the deliberate +// divergence from on_children_completed, which counts FAILED as terminal. +func TestDependencyGate_FailedPredecessorKeepsTaskBlocked(t *testing.T) { + for _, state := range []v1.TaskState{v1.TaskStateFailed, v1.TaskStateCancelled} { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + ctx := context.Background() + predecessor := mustSeedTask(t, svc, "Predecessor").ID + dependent := mustSeedTask(t, svc, "Dependent").ID + if err := svc.AddDependency(ctx, dependent, predecessor); err != nil { + t.Fatalf("AddDependency: %v", err) + } + setDependencyState(t, svc, predecessor, state) + + blocked, reason, err := svc.DependencyGate(ctx, dependent) + if err != nil || !blocked || reason != BlockedReasonFailed { + t.Errorf("%s predecessor: blocked=%v reason=%q err=%v; want true/failed/nil", + state, blocked, reason, err) + } + } +} + +// AND semantics: a task with two predecessors stays blocked until BOTH resolve. +func TestDependencyGate_WaitsForEveryPredecessor(t *testing.T) { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + ctx := context.Background() + first := mustSeedTask(t, svc, "First").ID + second := mustSeedTask(t, svc, "Second").ID + dependent := mustSeedTask(t, svc, "Dependent").ID + for _, p := range []string{first, second} { + if err := svc.AddDependency(ctx, dependent, p); err != nil { + t.Fatalf("AddDependency: %v", err) + } + } + + setDependencyState(t, svc, first, v1.TaskStateCompleted) + if blocked, _, _ := svc.DependencyGate(ctx, dependent); !blocked { + t.Error("one of two predecessors resolved: want still blocked") + } + setDependencyState(t, svc, second, v1.TaskStateCompleted) + if blocked, _, _ := svc.DependencyGate(ctx, dependent); blocked { + t.Error("both predecessors resolved: want unblocked") + } +} + +// The gate must fail CLOSED. Proven by breaking the store, not by the happy path: +// failing open would launch work whose predecessor may never have run. +func TestDependencyGate_FailsClosedOnReadError(t *testing.T) { + svc, _ := setupOfficeTest(t) + repo := &errBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}, failList: true} + svc.SetBlockerRepository(repo) + + blocked, reason, err := svc.DependencyGate(context.Background(), "any-task") + if err == nil { + t.Fatal("expected the read error to be returned") + } + if !blocked || reason != BlockedReasonUnknown { + t.Errorf("read failure: blocked=%v reason=%q; want true/unknown", blocked, reason) + } +} + +func TestBuildDependencyViews_ReportsBothDirections(t *testing.T) { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + ctx := context.Background() + a := mustSeedTask(t, svc, "A") + b := mustSeedTask(t, svc, "B") + c := mustSeedTask(t, svc, "C") + // B depends on A; C depends on B. B therefore has one of each direction. + if err := svc.AddDependency(ctx, b.ID, a.ID); err != nil { + t.Fatalf("AddDependency(b,a): %v", err) + } + if err := svc.AddDependency(ctx, c.ID, b.ID); err != nil { + t.Fatalf("AddDependency(c,b): %v", err) + } + + views := svc.BuildDependencyViews(ctx, []*models.Task{b}) + view := views[b.ID] + if len(view.DependsOn) != 1 || view.DependsOn[0].ID != a.ID { + t.Errorf("depends_on = %+v; want [%s]", view.DependsOn, a.ID) + } + if len(view.Blocks) != 1 || view.Blocks[0].ID != c.ID { + t.Errorf("blocks = %+v; want [%s]", view.Blocks, c.ID) + } + if view.DependsOn[0].Title != "A" { + t.Errorf("depends_on entry should carry the title for the chip, got %q", view.DependsOn[0].Title) + } + if !view.Blocked || view.BlockedReason != BlockedReasonPending { + t.Errorf("blocked=%v reason=%q; want true/pending", view.Blocked, view.BlockedReason) + } +} + +// BuildDependencyViews must fail closed too: the board reads dependency state +// through it, and a task reported unblocked on a read failure could be launched. +func TestBuildDependencyViews_FailsClosedOnReadError(t *testing.T) { + svc, _ := setupOfficeTest(t) + task := mustSeedTask(t, svc, "A") + svc.SetBlockerRepository(&errBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}, failList: true}) + + views := svc.BuildDependencyViews(context.Background(), []*models.Task{task}) + view := views[task.ID] + if !view.Blocked || view.BlockedReason != BlockedReasonUnknown { + t.Errorf("read failure: blocked=%v reason=%q; want true/unknown", view.Blocked, view.BlockedReason) + } +} + +func TestAddDependency_RejectsSelfEdge(t *testing.T) { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + task := mustSeedTask(t, svc, "A") + if err := svc.AddDependency(context.Background(), task.ID, task.ID); err == nil { + t.Fatal("expected a self-edge to be rejected") + } +} + +func TestAddDependency_RejectsCrossWorkspaceEdge(t *testing.T) { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + ctx := context.Background() + local := mustSeedTask(t, svc, "Local") + // Move a seeded task to another workspace at the repository layer: CreateTask + // requires a provisioned workspace, and the point here is only that the + // validator compares the two tasks' workspaces. + foreign := mustSeedTask(t, svc, "Foreign") + foreign.WorkspaceID = "ws-other" + if err := svc.tasks.UpdateTask(ctx, foreign); err != nil { + t.Fatalf("move foreign task: %v", err) + } + if err := svc.AddDependency(ctx, local.ID, foreign.ID); err == nil { + t.Fatal("expected a cross-workspace edge to be rejected") + } +} + +func TestRemoveDependency_AbsentEdgeSucceeds(t *testing.T) { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + if err := svc.RemoveDependency(context.Background(), "a", "b"); err != nil { + t.Fatalf("removing an absent edge should succeed, got %v", err) + } +} + +func TestResolveStartWhenUnblocked(t *testing.T) { + yes, no := true, false + cases := []struct { + name string + req *CreateTaskRequest + want bool + }{ + {"no dependencies never defers", &CreateTaskRequest{}, false}, + {"no dependencies ignores an explicit true", &CreateTaskRequest{StartWhenUnblocked: &yes}, false}, + // The load-bearing default: agents pass start_agent=true by habit, so a + // create WITH dependencies must record an intent rather than launch now, + // or every step of an agent-built chain starts at once. + {"dependencies default to deferring", &CreateTaskRequest{BlockedBy: []string{"a"}}, true}, + {"explicit false opts out", &CreateTaskRequest{BlockedBy: []string{"a"}, StartWhenUnblocked: &no}, false}, + {"explicit true defers", &CreateTaskRequest{BlockedBy: []string{"a"}, StartWhenUnblocked: &yes}, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := ResolveStartWhenUnblocked(tc.req); got != tc.want { + t.Errorf("ResolveStartWhenUnblocked() = %v; want %v", got, tc.want) + } + }) + } +} + +func TestDependencyStatusForTask_ArchivedIsPendingNotResolved(t *testing.T) { + // Archival is neither success nor failure: an archived predecessor must not + // release a dependent, and must not be reported as a failed chain either. + if got := DependencyStatusForTask(&models.Task{State: v1.TaskStateCompleted}); got != DependencyResolved { + t.Errorf("completed = %q; want %q", got, DependencyResolved) + } + archivedAt := time.Now().UTC() + archived := &models.Task{State: v1.TaskStateCompleted, ArchivedAt: &archivedAt} + if got := DependencyStatusForTask(archived); got != DependencyPending { + t.Errorf("archived+completed = %q; want %q", got, DependencyPending) + } + if got := DependencyStatusForTask(nil); got != DependencyPending { + t.Errorf("nil task = %q; want %q", got, DependencyPending) + } +} + +// Regression coverage for the review findings on PR #2589. + +// seedForeignPair creates two tasks in another user's workspace plus one the +// caller owns, so scoping can be exercised on both ends of an edge. +func seedForeignPair(t *testing.T, repo interface { + CreateWorkspace(context.Context, *models.Workspace) error + CreateWorkflow(context.Context, *models.Workflow) error + CreateTask(context.Context, *models.Task) error + CreateTaskSession(context.Context, *models.TaskSession) error +}) { + t.Helper() + seedScopedWorkspaces(t, repo) + ctx := context.Background() + if err := repo.CreateTask(ctx, &models.Task{ + ID: "task-b2", WorkspaceID: "ws-b", WorkflowID: "wf-b", WorkflowStepID: "step-1", + Title: "B's second", State: v1.TaskStateTODO, + }); err != nil { + t.Fatalf("create second foreign task: %v", err) + } +} + +// A caller must not be able to link, unlink, or probe another user's tasks. +// Both IDs are caller-supplied, so guarding only the dependent end would still +// let a caller attach a foreign task as a blocker. +func TestDependencyMutationsAreScopedToTheCaller(t *testing.T) { + svc, _, repo := createTestService(t) + seedForeignPair(t, repo) + svc.SetBlockerRepository(&mockBlockerRepo{}) + + if err := svc.AddDependency(ctxAs("user-a"), "task-b", "task-b2"); !errors.Is(err, taskrepo.ErrTaskNotFound) { + t.Errorf("add between two foreign tasks = %v; want ErrTaskNotFound", err) + } + if err := svc.RemoveDependency(ctxAs("user-a"), "task-b", "task-b2"); !errors.Is(err, taskrepo.ErrTaskNotFound) { + t.Errorf("remove between two foreign tasks = %v; want ErrTaskNotFound", err) + } + // The owner is unaffected. + if err := svc.AddDependency(ctxAs("user-b"), "task-b", "task-b2"); err != nil { + t.Errorf("owner add = %v; want nil", err) + } + // An internal (identity-free) caller stays unscoped, as the orchestrator relies on. + if err := svc.RemoveDependency(context.Background(), "task-b", "task-b2"); err != nil { + t.Errorf("internal caller remove = %v; want nil", err) + } +} + +// gateBlockerRepo forces the interleaving the cycle race needs: each cycle walk +// announces itself and then waits for the other walk to start. Two bare +// goroutines practically never land inside the validate-then-insert window, so +// a test without this barrier passes even with the serialization removed and +// proves nothing. +type gateBlockerRepo struct { + *mockBlockerRepo + arrived chan struct{} + once sync.Once +} + +func (g *gateBlockerRepo) ListTaskBlockers(ctx context.Context, taskID string) ([]*orchmodels.TaskBlocker, error) { + g.once.Do(func() { close(g.arrived) }) + select { + case <-g.arrived: + case <-time.After(time.Second): + } + // Give the other goroutine room to finish its own walk before this one + // inserts. Serialization must make that impossible; without it both walks + // complete against a pre-insert view. + time.Sleep(20 * time.Millisecond) + return g.mockBlockerRepo.ListTaskBlockers(ctx, taskID) +} + +// Two concurrent adds must not each pass a cycle walk that predates the other's +// insert and commit a cycle between them. +func TestAddDependencyConcurrentInsertsCannotCreateACycle(t *testing.T) { + svc, _ := setupOfficeTest(t) + repo := &gateBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}, arrived: make(chan struct{})} + svc.SetBlockerRepository(repo) + ctx := context.Background() + a := mustSeedTask(t, svc, "A").ID + b := mustSeedTask(t, svc, "B").ID + + var wg sync.WaitGroup + errs := make([]error, 2) + wg.Add(2) + go func() { defer wg.Done(); errs[0] = svc.AddDependency(ctx, a, b) }() + go func() { defer wg.Done(); errs[1] = svc.AddDependency(ctx, b, a) }() + wg.Wait() + + accepted := 0 + for _, err := range errs { + if err == nil { + accepted++ + } + } + if accepted != 1 { + t.Fatalf("accepted %d of 2 opposing edges; exactly one must win", accepted) + } + // Belt and braces: the surviving graph must not block both ways. + aBlocked, _, _ := svc.DependencyGate(ctx, a) + bBlocked, _, _ := svc.DependencyGate(ctx, b) + if aBlocked && bBlocked { + t.Error("both tasks blocked by each other: a cycle was committed") + } +} + +// A dangling edge (predecessor row gone because delete-time cleanup failed) +// must not block its dependent forever, while a genuine read FAILURE still +// fails closed. +func TestDependencyGateIgnoresDanglingEdgeButFailsClosedOnError(t *testing.T) { + svc, _ := setupOfficeTest(t) + repo := &mockBlockerRepo{} + svc.SetBlockerRepository(repo) + ctx := context.Background() + dependent := mustSeedTask(t, svc, "Dependent").ID + + // Edge pointing at a task that does not exist, as a failed cleanup leaves. + if err := repo.CreateTaskBlocker(ctx, &orchmodels.TaskBlocker{ + TaskID: dependent, BlockerTaskID: "deleted-task-id", + }); err != nil { + t.Fatalf("seed dangling edge: %v", err) + } + blocked, reason, err := svc.DependencyGate(ctx, dependent) + if err != nil { + t.Fatalf("gate returned error: %v", err) + } + if blocked { + t.Errorf("dangling edge blocked the task (reason=%q); a deleted predecessor can never complete", reason) + } + + // A read error is different and must still fail closed. + svc.SetBlockerRepository(&errBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}, failList: true}) + blocked, reason, err = svc.DependencyGate(ctx, dependent) + if err == nil || !blocked || reason != BlockedReasonUnknown { + t.Errorf("read failure: blocked=%v reason=%q err=%v; want true/unknown/non-nil", blocked, reason, err) + } +} + +// The derived projection must drop a dangling edge too, so the board does not +// render a blocked badge for a predecessor that no longer exists. +func TestBuildDependencyViewsDropsDanglingEdges(t *testing.T) { + svc, _ := setupOfficeTest(t) + repo := &mockBlockerRepo{} + svc.SetBlockerRepository(repo) + ctx := context.Background() + dependent := mustSeedTask(t, svc, "Dependent") + if err := repo.CreateTaskBlocker(ctx, &orchmodels.TaskBlocker{ + TaskID: dependent.ID, BlockerTaskID: "deleted-task-id", + }); err != nil { + t.Fatalf("seed dangling edge: %v", err) + } + + view := svc.BuildDependencyViews(ctx, []*models.Task{dependent})[dependent.ID] + if view.Blocked { + t.Errorf("blocked=%v reason=%q; a dangling edge must not block", view.Blocked, view.BlockedReason) + } + if len(view.DependsOn) != 0 { + t.Errorf("depends_on = %+v; a dangling edge must not be reported", view.DependsOn) + } +} + +// An edge mutation must publish the recomputed projection, not a bare +// task.updated: the client treats omitted dependency keys as "unchanged", so a +// bare event leaves every other open board rendering a stale chip and badge. +func TestDependencyEventFieldsCarryTheProjection(t *testing.T) { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + ctx := context.Background() + predecessor := mustSeedTask(t, svc, "Predecessor") + dependent := mustSeedTask(t, svc, "Dependent") + if err := svc.AddDependency(ctx, dependent.ID, predecessor.ID); err != nil { + t.Fatalf("AddDependency: %v", err) + } + + fields := svc.dependencyEventFields(ctx, dependent) + for _, key := range []string{"blocked", "blocked_reason", "depends_on", "blocks"} { + if _, ok := fields[key]; !ok { + t.Errorf("event payload is missing %q; the client would treat it as unchanged", key) + } + } + if fields["blocked"] != true { + t.Errorf("blocked = %v; want true", fields["blocked"]) + } + if fields["blocked_reason"] != BlockedReasonPending { + t.Errorf("blocked_reason = %v; want %q", fields["blocked_reason"], BlockedReasonPending) + } + deps, _ := fields["depends_on"].([]DependencyRef) + if len(deps) != 1 || deps[0].ID != predecessor.ID { + t.Errorf("depends_on = %+v; want one entry for %s", deps, predecessor.ID) + } +} diff --git a/apps/backend/internal/task/service/service_office.go b/apps/backend/internal/task/service/service_office.go index dd8e629625..e280074775 100644 --- a/apps/backend/internal/task/service/service_office.go +++ b/apps/backend/internal/task/service/service_office.go @@ -18,6 +18,16 @@ type BlockerRepository interface { // ListTasksBlockedBy returns task IDs that have blockerTaskID listed as // one of their blockers (the reverse direction of ListTaskBlockers). ListTasksBlockedBy(ctx context.Context, blockerTaskID string) ([]string, error) + // ListBlockersForTasks batches the forward direction over many tasks. + ListBlockersForTasks(ctx context.Context, taskIDs []string) (map[string][]string, error) + // ListDependentsForTasks batches the reverse direction over many tasks. + ListDependentsForTasks(ctx context.Context, blockerTaskIDs []string) (map[string][]string, error) +} + +// taskDependencyCleaner removes every edge touching a task. Optional on +// BlockerRepository so existing test doubles keep compiling. +type taskDependencyCleaner interface { + DeleteTaskBlockersForTask(ctx context.Context, taskID string) error } // CommentRepository provides access to task comment persistence. @@ -100,33 +110,30 @@ func (s *Service) ListTasksByAssignee(ctx context.Context, agentInstanceID strin } // AddBlocker creates a blocker relationship between two tasks. -// It validates that the blocker does not create a circular dependency. +// +// Thin alias for AddDependency, which owns the only edge validator (self-edge, +// cross-workspace, BFS cycle with a reportable path). This used to run its own +// weaker check; two validators meant a cycle could enter through whichever path +// was laxer. func (s *Service) AddBlocker(ctx context.Context, taskID, blockerTaskID string) error { - if s.blockers == nil { - return fmt.Errorf("blocker repository not configured") - } - if taskID == blockerTaskID { - return fmt.Errorf("a task cannot block itself") - } - - // Check for circular dependency: if blockerTaskID is already (transitively) blocked by taskID - if err := s.checkCircularBlocker(ctx, taskID, blockerTaskID); err != nil { - return err - } - - blocker := &orchmodels.TaskBlocker{ - TaskID: taskID, - BlockerTaskID: blockerTaskID, - } - return s.blockers.CreateTaskBlocker(ctx, blocker) + return s.AddDependency(ctx, taskID, blockerTaskID) } // RemoveBlocker removes a blocker relationship between two tasks. func (s *Service) RemoveBlocker(ctx context.Context, taskID, blockerTaskID string) error { - if s.blockers == nil { - return fmt.Errorf("blocker repository not configured") - } - return s.blockers.DeleteTaskBlocker(ctx, taskID, blockerTaskID) + return s.RemoveDependency(ctx, taskID, blockerTaskID) +} + +// createBlockerEdge writes the dependency row. +// +// The row type is Office-owned, and ARCH-TASK-OFFICE-IMPORT keeps new task-tier +// files free of Office imports; this file is already baselined for it, so the +// construction is confined here rather than widening the baseline. +func (s *Service) createBlockerEdge(ctx context.Context, taskID, blockerTaskID string) error { + return s.blockers.CreateTaskBlocker(ctx, &orchmodels.TaskBlocker{ + TaskID: taskID, + BlockerTaskID: blockerTaskID, + }) } // GetBlockers returns all tasks that block the given task. @@ -163,42 +170,6 @@ func (s *Service) GetBlocking(ctx context.Context, taskID string) ([]string, err return ids, nil } -// checkCircularBlocker checks if adding blockerTaskID as a blocker to taskID -// would create a circular dependency. It walks the blocker chain from -// blockerTaskID to see if it leads back to taskID. -func (s *Service) checkCircularBlocker(ctx context.Context, taskID, blockerTaskID string) error { - // Walk the transitive blockers of blockerTaskID. If any path reaches - // taskID, adding this blocker would create a cycle. - target := taskID - visited := make(map[string]bool) - queue := []string{blockerTaskID} - - for len(queue) > 0 { - current := queue[0] - queue = queue[1:] - - if visited[current] { - continue - } - visited[current] = true - - blockers, err := s.blockers.ListTaskBlockers(ctx, current) - if err != nil { - return fmt.Errorf("failed to check circular dependency: %w", err) - } - for _, b := range blockers { - if b.BlockerTaskID == target { - return fmt.Errorf("circular dependency detected: adding %s as blocker of %s creates a cycle", - blockerTaskID, taskID) - } - if !visited[b.BlockerTaskID] { - queue = append(queue, b.BlockerTaskID) - } - } - } - return nil -} - // CreateComment creates a new comment on a task. func (s *Service) CreateComment(ctx context.Context, comment *orchmodels.TaskComment) error { if s.comments == nil { diff --git a/apps/backend/internal/task/service/service_office_test.go b/apps/backend/internal/task/service/service_office_test.go index 111021cf91..45ff3c94f0 100644 --- a/apps/backend/internal/task/service/service_office_test.go +++ b/apps/backend/internal/task/service/service_office_test.go @@ -3,6 +3,7 @@ package service import ( "context" "database/sql" + "errors" "slices" "sort" "strings" @@ -354,39 +355,90 @@ func (m *mockBlockerRepo) ListTasksBlockedBy(_ context.Context, blockerTaskID st return ids, nil } +func (m *mockBlockerRepo) ListBlockersForTasks(_ context.Context, taskIDs []string) (map[string][]string, error) { + want := make(map[string]struct{}, len(taskIDs)) + for _, id := range taskIDs { + want[id] = struct{}{} + } + out := map[string][]string{} + for _, b := range m.blockers { + if _, ok := want[b.TaskID]; ok { + out[b.TaskID] = append(out[b.TaskID], b.BlockerTaskID) + } + } + return out, nil +} + +func (m *mockBlockerRepo) ListDependentsForTasks(_ context.Context, blockerTaskIDs []string) (map[string][]string, error) { + want := make(map[string]struct{}, len(blockerTaskIDs)) + for _, id := range blockerTaskIDs { + want[id] = struct{}{} + } + out := map[string][]string{} + for _, b := range m.blockers { + if _, ok := want[b.BlockerTaskID]; ok { + out[b.BlockerTaskID] = append(out[b.BlockerTaskID], b.TaskID) + } + } + return out, nil +} + func TestBlocker_CRUD(t *testing.T) { - svc, _, _ := createTestService(t) + svc, _ := setupOfficeTest(t) svc.SetBlockerRepository(&mockBlockerRepo{}) ctx := context.Background() - if err := svc.AddBlocker(ctx, "task-1", "task-2"); err != nil { + // Both ends must exist: the single edge validator resolves each task so it + // can enforce the same-workspace rule and refuse an edge to a task that is + // not there (which would leave the dependent blocked forever). + one := mustSeedTask(t, svc, "Task 1") + two := mustSeedTask(t, svc, "Task 2") + + if err := svc.AddBlocker(ctx, one.ID, two.ID); err != nil { t.Fatalf("AddBlocker: %v", err) } - ids, err := svc.GetBlockers(ctx, "task-1") + ids, err := svc.GetBlockers(ctx, one.ID) if err != nil { t.Fatalf("GetBlockers: %v", err) } - if len(ids) != 1 || ids[0] != "task-2" { - t.Errorf("expected [task-2], got %v", ids) + if len(ids) != 1 || ids[0] != two.ID { + t.Errorf("expected [%s], got %v", two.ID, ids) } - if err := svc.RemoveBlocker(ctx, "task-1", "task-2"); err != nil { + if err := svc.RemoveBlocker(ctx, one.ID, two.ID); err != nil { t.Fatalf("RemoveBlocker: %v", err) } - ids, _ = svc.GetBlockers(ctx, "task-1") + ids, _ = svc.GetBlockers(ctx, one.ID) if len(ids) != 0 { t.Errorf("expected empty, got %v", ids) } } +// mustSeedTask creates a task in the office test workspace so dependency-edge +// tests have real IDs on both ends of an edge. +func mustSeedTask(t *testing.T, svc *Service, title string) *models.Task { + t.Helper() + result, err := svc.CreateTask(context.Background(), &CreateTaskRequest{ + WorkspaceID: "ws-1", Title: title, ProjectID: "proj-1", + }) + if err != nil { + t.Fatalf("seed task %q: %v", title, err) + } + return result.Task +} + func TestGetBlocking_ReverseLookup(t *testing.T) { - svc, _, _ := createTestService(t) + svc, _ := setupOfficeTest(t) svc.SetBlockerRepository(&mockBlockerRepo{}) ctx := context.Background() + t1 := mustSeedTask(t, svc, "Task 1").ID + t2 := mustSeedTask(t, svc, "Task 2").ID + t3 := mustSeedTask(t, svc, "Task 3").ID + t4 := mustSeedTask(t, svc, "Task 4").ID // No blocker edges at all. - ids, err := svc.GetBlocking(ctx, "task-1") + ids, err := svc.GetBlocking(ctx, t1) if err != nil { t.Fatalf("GetBlocking: %v", err) } @@ -398,50 +450,53 @@ func TestGetBlocking_ReverseLookup(t *testing.T) { } // One blocked task: task-2 is blocked by task-1. - if err := svc.AddBlocker(ctx, "task-2", "task-1"); err != nil { + if err := svc.AddBlocker(ctx, t2, t1); err != nil { t.Fatalf("AddBlocker: %v", err) } - ids, err = svc.GetBlocking(ctx, "task-1") + ids, err = svc.GetBlocking(ctx, t1) if err != nil { t.Fatalf("GetBlocking: %v", err) } - if len(ids) != 1 || ids[0] != "task-2" { + if len(ids) != 1 || ids[0] != t2 { t.Errorf("expected [task-2], got %v", ids) } // The reverse direction must not be confused with the forward one: // task-2 blocks nothing, and task-1 is the blocker of task-2. - if ids, err = svc.GetBlocking(ctx, "task-2"); err != nil || len(ids) != 0 { + if ids, err = svc.GetBlocking(ctx, t2); err != nil || len(ids) != 0 { t.Errorf("GetBlocking(task-2) = %v, %v; want empty", ids, err) } - if ids, err = svc.GetBlockers(ctx, "task-2"); err != nil || len(ids) != 1 || ids[0] != "task-1" { + if ids, err = svc.GetBlockers(ctx, t2); err != nil || len(ids) != 1 || ids[0] != t1 { t.Errorf("GetBlockers(task-2) = %v, %v; want [task-1]", ids, err) } // Several blocked tasks. - for _, blocked := range []string{"task-3", "task-4"} { - if err := svc.AddBlocker(ctx, blocked, "task-1"); err != nil { + for _, blocked := range []string{t3, t4} { + if err := svc.AddBlocker(ctx, blocked, t1); err != nil { t.Fatalf("AddBlocker(%s): %v", blocked, err) } } - ids, err = svc.GetBlocking(ctx, "task-1") + ids, err = svc.GetBlocking(ctx, t1) if err != nil { t.Fatalf("GetBlocking: %v", err) } sort.Strings(ids) - want := []string{"task-2", "task-3", "task-4"} + want := []string{t2, t3, t4} + sort.Strings(want) if !slices.Equal(ids, want) { t.Errorf("expected %v, got %v", want, ids) } // Removing an edge removes it from the reverse lookup too. - if err := svc.RemoveBlocker(ctx, "task-3", "task-1"); err != nil { + if err := svc.RemoveBlocker(ctx, t3, t1); err != nil { t.Fatalf("RemoveBlocker: %v", err) } - ids, _ = svc.GetBlocking(ctx, "task-1") + ids, _ = svc.GetBlocking(ctx, t1) sort.Strings(ids) - if !slices.Equal(ids, []string{"task-2", "task-4"}) { - t.Errorf("expected [task-2 task-4] after removal, got %v", ids) + wantAfter := []string{t2, t4} + sort.Strings(wantAfter) + if !slices.Equal(ids, wantAfter) { + t.Errorf("expected %v after removal, got %v", wantAfter, ids) } } @@ -499,19 +554,35 @@ func TestGetBlocking_IncludesArchivedTasks(t *testing.T) { } func TestBlocker_CircularDetection(t *testing.T) { - svc, _, _ := createTestService(t) + svc, _ := setupOfficeTest(t) svc.SetBlockerRepository(&mockBlockerRepo{}) ctx := context.Background() + a := mustSeedTask(t, svc, "Task A").ID + b := mustSeedTask(t, svc, "Task B").ID + c := mustSeedTask(t, svc, "Task C").ID - _ = svc.AddBlocker(ctx, "task-A", "task-B") - _ = svc.AddBlocker(ctx, "task-B", "task-C") - - err := svc.AddBlocker(ctx, "task-C", "task-A") - if err == nil { - t.Fatal("expected circular dependency error") + if err := svc.AddBlocker(ctx, a, b); err != nil { + t.Fatalf("AddBlocker(a,b): %v", err) } - if !strings.Contains(err.Error(), "circular dependency") { - t.Errorf("expected 'circular dependency' in error, got: %v", err) + if err := svc.AddBlocker(ctx, b, c); err != nil { + t.Fatalf("AddBlocker(b,c): %v", err) + } + + err := svc.AddBlocker(ctx, c, a) + if err == nil { + t.Fatal("expected cycle error") + } + // The error must carry the traversal path so the UI can render + // "A → B → C → A" rather than a bare "there is a cycle". + var cycle *CycleError + if !errors.As(err, &cycle) { + t.Fatalf("expected *CycleError, got %T: %v", err, err) + } + if len(cycle.Path) < 3 { + t.Errorf("expected a cycle path naming the loop, got %v", cycle.Path) + } + if cycle.Path[0] != c || cycle.Path[len(cycle.Path)-1] != c { + t.Errorf("cycle path should start and end at the new dependent %s, got %v", c, cycle.Path) } } diff --git a/apps/backend/internal/task/service/service_requests.go b/apps/backend/internal/task/service/service_requests.go index 4e8bfeb926..04ea8244b0 100644 --- a/apps/backend/internal/task/service/service_requests.go +++ b/apps/backend/internal/task/service/service_requests.go @@ -79,6 +79,16 @@ type CreateTaskRequest struct { ProjectID string `json:"project_id,omitempty"` Labels string `json:"labels,omitempty"` BlockedBy []string `json:"blocked_by,omitempty"` + + // StartWhenUnblocked records the requested agent start as a deferred launch + // intent that dependency resolution consumes, instead of launching now. + // + // nil means "derive from the request's start intent": a create that asked to + // start an agent AND declared BlockedBy is describing a chain step, not a + // task to run immediately. Automated callers pass start_agent=true by habit, + // so deriving is what makes an agent-built chain run in order rather than + // launching every step at once. + StartWhenUnblocked *bool `json:"start_when_unblocked,omitempty"` } // UpdateTaskRequest contains the data for updating a task diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index 43ee77dd09..84622ef70f 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "maps" "net/url" "os" "path/filepath" @@ -613,7 +614,17 @@ func (s *Service) buildTask(req *CreateTaskRequest, workflowStepID string) *mode if metadata == nil { metadata = make(map[string]interface{}) } - metadata[models.MetaKeyDeferredLaunch] = req.DeferredLaunch + launch := req.DeferredLaunch + if ResolveStartWhenUnblocked(req) { + // Mark the intent as a dependency-chain step. The record is the same + // one WIP overflow persists — reused so "launch exactly once" and + // restart survival are inherited — and the flag is what lets + // dependency resolution recognise its own intents. + launch = make(map[string]interface{}, len(req.DeferredLaunch)+1) + maps.Copy(launch, req.DeferredLaunch) + launch[models.DeferredLaunchStartWhenUnblockedKey] = true + } + metadata[models.MetaKeyDeferredLaunch] = launch } if wsPath := strings.TrimSpace(req.WorkspacePath); wsPath != "" { if metadata == nil { @@ -2068,6 +2079,11 @@ func (s *Service) deleteTaskWithReasonAndDBDelete( zap.String("task_id", id), zap.Error(err)) } } + // Remove dependency edges in both directions. task_blockers predates the + // tasks foreign key so nothing cascades, and a left-over edge would keep a + // dependent blocked forever on a task that no longer exists. Dependents are + // refreshed but deliberately not started: deletion is not success. + s.deleteDependencyEdgesForTask(context.WithoutCancel(ctx), id) // 5. Publish event (sync, fast) - frontend removes task immediately var extra map[string]interface{} diff --git a/apps/backend/internal/task/service/service_workflow.go b/apps/backend/internal/task/service/service_workflow.go index b577a6f565..f51200fb7a 100644 --- a/apps/backend/internal/task/service/service_workflow.go +++ b/apps/backend/internal/task/service/service_workflow.go @@ -481,7 +481,7 @@ func (s *Service) MoveTaskWithOptions( task.QueuedForStepID = "" task.QueuedAt = nil if !opts.PreserveDeferredLaunch { - delete(task.Metadata, models.MetaKeyDeferredLaunch) + models.DropWIPDeferredLaunch(task) } } if err := s.syncTaskStateForWorkflowMove(ctx, task, oldStepID, workflowStepID); err != nil { diff --git a/apps/backend/pkg/websocket/actions.go b/apps/backend/pkg/websocket/actions.go index d1e77d8679..6a3777a52e 100644 --- a/apps/backend/pkg/websocket/actions.go +++ b/apps/backend/pkg/websocket/actions.go @@ -381,6 +381,8 @@ const ( ActionMCPUpdateTaskPRAutomation = "mcp.update_task_pr_automation" ActionMCPGetTaskMRAutomation = "mcp.get_task_mr_automation" ActionMCPUpdateTaskMRAutomation = "mcp.update_task_mr_automation" + ActionMCPAddTaskDependency = "mcp.add_task_dependency" + ActionMCPRemoveTaskDependency = "mcp.remove_task_dependency" ActionMCPAddBranchToTask = "mcp.add_branch_to_task" ActionMCPAddWorkspaceSources = "mcp.add_workspace_sources" ActionMCPUpdateRepositoryBaseBranch = "mcp.update_repository_base_branch" diff --git a/apps/web/components/kanban-card-content.tsx b/apps/web/components/kanban-card-content.tsx index a2a5a1489d..8d8ab92c99 100644 --- a/apps/web/components/kanban-card-content.tsx +++ b/apps/web/components/kanban-card-content.tsx @@ -9,6 +9,7 @@ import { IconArrowsMaximize, IconDots, IconLoader2, + IconLock, IconSubtask, IconUsersGroup, } from "@tabler/icons-react"; @@ -208,6 +209,7 @@ function KanbanCardBadges({ task }: { task: Task }) { return (
+ {task.blocked && } {task.queuedForStepId && ( ref.title || ref.id).join(", "); + return ( + + + {failed ? t("kanban:blockedFailed") : t("kanban:blockedByCount", { count })} + + ); +} + function hasCardBadges(task: Task): boolean { return Boolean( (task.sessionCount && task.sessionCount > 1) || task.reviewStatus === "changes_requested" || task.reviewStatus === "pending" || - task.queuedForStepId, + task.queuedForStepId || + task.blocked, ); } diff --git a/apps/web/components/kanban-card.tsx b/apps/web/components/kanban-card.tsx index d660df15f4..ef5f463742 100644 --- a/apps/web/components/kanban-card.tsx +++ b/apps/web/components/kanban-card.tsx @@ -18,6 +18,7 @@ import { type ExternalLinkProvider, } from "@/components/task/task-external-link-dialog"; import type { KanbanExternalLinkAvailability } from "./kanban-external-link-availability"; +import type { TaskDependencyRef } from "@/lib/state/slices/kanban/types"; import { TaskGitHubIssueDialog } from "@/components/task/task-github-issue-dialog"; import { TaskGitHubPRDialog } from "@/components/task/task-github-pr-dialog"; import { TaskMRLinkDialog } from "@/components/gitlab/task-mr-link-dialog"; @@ -80,6 +81,12 @@ export interface Task { wipAdmitted?: boolean; queuedForStepId?: string; queuedForStepTitle?: string; + /** Derived dependency state — see TaskDependencyRef in the kanban slice. */ + blocked?: boolean; + blockedReason?: string; + dependsOn?: TaskDependencyRef[]; + blocks?: TaskDependencyRef[]; + startWhenUnblocked?: boolean; queuedAt?: string; issueUrl?: string; issueNumber?: number; diff --git a/apps/web/components/task-create-dialog-dependencies.tsx b/apps/web/components/task-create-dialog-dependencies.tsx new file mode 100644 index 0000000000..abab7b167e --- /dev/null +++ b/apps/web/components/task-create-dialog-dependencies.tsx @@ -0,0 +1,162 @@ +"use client"; + +/** + * "Depends on" selector for the task-create dialog. + * + * Dependencies are declared when the task is created (or later through MCP), so + * this is the only place the UI writes an edge. Selecting one or more + * predecessors also changes what "Start agent" means: the backend records the + * start as a start-when-unblocked intent, and the task launches on its own once + * every predecessor completes successfully. + */ + +import { useMemo, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { IconLock, IconX } from "@tabler/icons-react"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { + Command, + CommandEmpty, + CommandGroup, + CommandInput, + CommandItem, + CommandList, +} from "@kandev/ui/command"; +import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover"; +import { useAppStore } from "@/components/state-provider"; +import type { KanbanState } from "@/lib/state/slices/kanban/types"; + +/** + * Stable empty fallbacks. A selector must not build a fresh array/object: under + * Object.is that reads as a change on every store read and re-renders until React + * bails out (which is what silently blanked the dependency chip once already). + */ +const NO_TASKS: KanbanState["tasks"] = []; +const NO_SNAPSHOTS: Record = {}; + +export type TaskCreateDependenciesProps = { + /** Selected predecessor task IDs. */ + value: string[]; + onChange: (next: string[]) => void; + disabled?: boolean; +}; + +/** + * Every task the user could depend on, from BOTH board slices. + * + * The home route hydrates its swimlanes into kanbanMulti.snapshots and can leave + * kanban.tasks empty, so reading only the latter left this picker with nothing to + * offer on the very route where tasks are created. Both selections are stable + * references; the dedupe happens in the memo. + */ +function useBoardTasks(): KanbanState["tasks"] { + const boardTasks = useAppStore((state) => state.kanban?.tasks ?? NO_TASKS); + const snapshots = useAppStore((state) => state.kanbanMulti?.snapshots ?? NO_SNAPSHOTS); + return useMemo(() => { + const byId = new Map(); + for (const task of boardTasks) byId.set(task.id, task); + for (const snapshot of Object.values(snapshots)) { + for (const task of snapshot.tasks ?? []) { + if (!byId.has(task.id)) byId.set(task.id, task); + } + } + return [...byId.values()]; + }, [boardTasks, snapshots]); +} + +export function TaskCreateDependencies({ value, onChange, disabled }: TaskCreateDependenciesProps) { + const { t } = useTranslation(); + const [open, setOpen] = useState(false); + // Select the stable slice reference and derive below: a selector that builds a + // new array compares unequal on every store read and re-renders until React + // gives up. + // Candidates come from BOTH board slices. The home route hydrates its + // swimlanes into kanbanMulti.snapshots and can leave kanban.tasks empty, so + // reading only the latter left this picker with nothing to offer on the very + // route where tasks are created. Both selections are stable references; the + // dedupe happens in the memo below. + const tasks = useBoardTasks(); + + const selected = useMemo( + () => value.map((id) => tasks.find((task) => task.id === id)).filter(Boolean), + [value, tasks], + ); + const candidates = useMemo( + () => + tasks + .filter((task) => !value.includes(task.id) && !task.isArchived) + .sort((a, b) => a.title.localeCompare(b.title)), + [tasks, value], + ); + + return ( +
+
+ {t("task:dependsOn")} + + + + + + + + + {t("task:noDependencies")} + + {candidates.map((task) => ( + { + setOpen(false); + onChange([...value, task.id]); + }} + > + {task.title} + + ))} + + + + + +
+ {selected.length > 0 && ( +
+ {selected.map((task) => ( + + {task!.title} + + + ))} +
+ )} + {selected.length > 0 && ( +

{t("task:dependsOnWillAutoStart")}

+ )} +
+ ); +} diff --git a/apps/web/components/task-create-dialog-form-body.test.tsx b/apps/web/components/task-create-dialog-form-body.test.tsx index 3facc59da7..2d18ec0012 100644 --- a/apps/web/components/task-create-dialog-form-body.test.tsx +++ b/apps/web/components/task-create-dialog-form-body.test.tsx @@ -62,6 +62,8 @@ describe("WorkflowSection", () => { function makeFs(): DialogFormState { return { + blockedBy: [], + setBlockedBy: () => undefined, taskName: "", autopilot: false, setAutopilot: () => {}, diff --git a/apps/web/components/task-create-dialog-helpers.test.ts b/apps/web/components/task-create-dialog-helpers.test.ts index 34eb246e3a..adc6ed91c3 100644 --- a/apps/web/components/task-create-dialog-helpers.test.ts +++ b/apps/web/components/task-create-dialog-helpers.test.ts @@ -10,6 +10,18 @@ import { import type { TaskRemoteRepoRow } from "./task-create-dialog-types"; const STORAGE_KEYS = { LAST_BRANCH: "kandev.dialog.lastBranch" } as const; +// The agent-bearing fields every buildCreateTaskPayload case needs but none of +// them is asserting; each test spreads these and overrides only what it checks. +const AGENT_PAYLOAD_DEFAULTS = { + workspaceId: "ws-1", + effectiveWorkflowId: "wf-1", + repositoriesPayload: [], + agentProfileId: "agent-1", + executorId: "executor-1", + executorProfileId: "profile-1", + withAgent: true, +}; + beforeEach(() => { localStorage.clear(); }); @@ -218,15 +230,9 @@ describe("auto-title creation helpers", () => { it("omits the manual title and opts into backend provisional naming", () => { const payload = buildCreateTaskPayload({ - workspaceId: "ws-1", - effectiveWorkflowId: "wf-1", + ...AGENT_PAYLOAD_DEFAULTS, trimmedTitle: "ignored", trimmedDescription: "Fix the login flow", - repositoriesPayload: [], - agentProfileId: "agent-1", - executorId: "executor-1", - executorProfileId: "profile-1", - withAgent: true, autoTitle: true, }); @@ -236,15 +242,9 @@ describe("auto-title creation helpers", () => { it("includes autopilot only when the create form opts in", () => { const payload = buildCreateTaskPayload({ - workspaceId: "ws-1", - effectiveWorkflowId: "wf-1", + ...AGENT_PAYLOAD_DEFAULTS, trimmedTitle: "Autonomous task", trimmedDescription: "Run the migration", - repositoriesPayload: [], - agentProfileId: "agent-1", - executorId: "executor-1", - executorProfileId: "profile-1", - withAgent: true, autopilot: true, }); @@ -253,18 +253,30 @@ describe("auto-title creation helpers", () => { it("keeps autopilot off when the create form does not opt in", () => { const payload = buildCreateTaskPayload({ - workspaceId: "ws-1", - effectiveWorkflowId: "wf-1", + ...AGENT_PAYLOAD_DEFAULTS, trimmedTitle: "Manual task", trimmedDescription: "Run the migration", - repositoriesPayload: [], - agentProfileId: "agent-1", - executorId: "executor-1", - executorProfileId: "profile-1", - withAgent: true, autopilot: false, }); expect(payload.autopilot).toBeUndefined(); }); }); + +describe("buildCreateTaskPayload dependencies", () => { + const base = { + ...AGENT_PAYLOAD_DEFAULTS, + trimmedTitle: "Second step", + trimmedDescription: "Runs after the first", + }; + + it("sends the selected predecessors as blocked_by", () => { + const payload = buildCreateTaskPayload({ ...base, blockedBy: ["task-a", "task-b"] }); + expect(payload.blocked_by).toEqual(["task-a", "task-b"]); + }); + + it("omits blocked_by when nothing is selected", () => { + expect(buildCreateTaskPayload({ ...base, blockedBy: [] }).blocked_by).toBeUndefined(); + expect(buildCreateTaskPayload(base).blocked_by).toBeUndefined(); + }); +}); diff --git a/apps/web/components/task-create-dialog-helpers.ts b/apps/web/components/task-create-dialog-helpers.ts index b6ddd29b06..2fea2e3d9c 100644 --- a/apps/web/components/task-create-dialog-helpers.ts +++ b/apps/web/components/task-create-dialog-helpers.ts @@ -167,6 +167,8 @@ export type BuildCreatePayloadArgs = { parentId?: string; workspacePath?: string; autopilot?: boolean; + /** Task IDs this task must wait for. */ + blockedBy?: string[]; }; export function buildCreateTaskPayload(args: BuildCreatePayloadArgs): CreateTaskParams { @@ -187,6 +189,10 @@ export function buildCreateTaskPayload(args: BuildCreatePayloadArgs): CreateTask parent_id: args.parentId || undefined, workspace_path: args.workspacePath || undefined, autopilot: args.autopilot || undefined, + // Dependencies declared at creation time. With edges present the backend + // records the requested agent start as a start-when-unblocked intent rather + // than launching now, so a chain runs in order instead of all at once. + blocked_by: args.blockedBy && args.blockedBy.length > 0 ? args.blockedBy : undefined, }; } diff --git a/apps/web/components/task-create-dialog-setup.test.ts b/apps/web/components/task-create-dialog-setup.test.ts index 5b01a4061d..259f1a2544 100644 --- a/apps/web/components/task-create-dialog-setup.test.ts +++ b/apps/web/components/task-create-dialog-setup.test.ts @@ -5,6 +5,7 @@ import type { TaskCreateDialogProps } from "./task-create-dialog"; const mocks = vi.hoisted(() => ({ agentGeneratedTaskTitles: false, submit: vi.fn(), + submitDeps: {} as Record, })); vi.mock("@/lib/keyboard/constants", () => ({ SHORTCUTS: { SUBMIT: "submit" } })); @@ -33,7 +34,10 @@ vi.mock("@/components/state-provider", () => ({ }), })); vi.mock("@/components/task-create-dialog-submit", () => ({ - useTaskSubmitHandlers: () => ({ handleSubmit: mocks.submit, pendingDiscard: null }), + useTaskSubmitHandlers: (deps: Record) => { + mocks.submitDeps = deps; + return { handleSubmit: mocks.submit, pendingDiscard: null }; + }, })); vi.mock("@/components/task-create-dialog-workflow-context", () => ({ useResolvedTaskCreateWorkflowContext: (props: TaskCreateDialogProps) => props, @@ -58,6 +62,8 @@ vi.mock("@/components/task-create-dialog-state", () => ({ executorProfileId: "", freshBranchEnabled: false, noRepository: false, + blockedBy: ["dep-1"], + setBlockedBy: vi.fn(), workspacePath: "", isCreatingSession: false, isCreatingTask: false, @@ -142,3 +148,12 @@ describe("useTaskCreateDialogSetup auto-title mode", () => { expect(result.current.autoTitle).toBe(expected); }); }); + +it("forwards the selected dependencies to the submit handlers", () => { + // The payload builder handled blocked_by correctly all along; the break was + // this hop — useSubmitHandlersWiring never passed blockedBy through, so the + // create dialog's selection silently never reached the request. Assert the + // hop itself, not just the leaf. + renderHook(() => useTaskCreateDialogSetup({ ...props, mode: "create" })); + expect(mocks.submitDeps.blockedBy).toEqual(["dep-1"]); +}); diff --git a/apps/web/components/task-create-dialog-setup.ts b/apps/web/components/task-create-dialog-setup.ts index a9afb89c48..0c1c7bbada 100644 --- a/apps/web/components/task-create-dialog-setup.ts +++ b/apps/web/components/task-create-dialog-setup.ts @@ -186,6 +186,7 @@ function useSubmitHandlersWiring({ repositoryLocalPath, noRepository: fs.noRepository, workspacePath: fs.workspacePath, + blockedBy: fs.blockedBy, }); } diff --git a/apps/web/components/task-create-dialog-state.ts b/apps/web/components/task-create-dialog-state.ts index b29a3be3e0..9eb87131de 100644 --- a/apps/web/components/task-create-dialog-state.ts +++ b/apps/web/components/task-create-dialog-state.ts @@ -58,6 +58,7 @@ type FormResetters = { setGitHubUrlError: (v: string | null) => void; setFreshBranchEnabled: (v: boolean) => void; setCurrentLocalBranch: (v: string) => void; + setBlockedBy: (v: string[]) => void; }; type FormResetEffectsArgs = { @@ -271,6 +272,9 @@ function resetDiscoveryState(resetters: FormResetters, iv?: TaskCreateDialogInit // mode and reopening for a different task would land in None mode again. resetters.setNoRepository(false); resetters.setWorkspacePath(""); + // The dialog stays mounted between opens, so without this the previous + // create's predecessor selection reappears on the next one. + resetters.setBlockedBy([]); } /** Hook to manage draft persistence for task creation dialog */ @@ -321,6 +325,15 @@ function useWorkflowAgentProfileState() { return { workflowAgentProfileId, setWorkflowAgentProfileId }; } +/** + * Predecessor task IDs selected in the dialog. Dependencies are declared at + * creation time (or later via MCP); nothing edits them from the task view. + */ +function useTaskDependencyState() { + const [blockedBy, setBlockedBy] = useState([]); + return { blockedBy, setBlockedBy }; +} + function useFreshBranchState() { const [freshBranchEnabled, setFreshBranchEnabled] = useState(false); const [currentLocalBranch, setCurrentLocalBranch] = useState(""); @@ -453,6 +466,7 @@ export function useDialogFormState( const repos = useRepositoriesState(); const remoteRepos = useRemoteReposState(); const freshBranch = useFreshBranchState(); + const dependencies = useTaskDependencyState(); const branchesByUrl = useBranchesByURL(workspaceId); const prInfoByUrl = usePRInfoByURL(workspaceId); @@ -467,6 +481,7 @@ export function useDialogFormState( prevOpenRef: form.prevOpenRef, lockedWorkflow, resetters: { + setBlockedBy: dependencies.setBlockedBy, setTaskName: form.setTaskName, setHasTitle: form.setHasTitle, setHasDescription: form.setHasDescription, @@ -521,6 +536,7 @@ export function useDialogFormState( ...repos, ...remoteRepos, ...freshBranch, + ...dependencies, branchesByUrl, prInfoByUrl, clearDraft, diff --git a/apps/web/components/task-create-dialog-submit.tsx b/apps/web/components/task-create-dialog-submit.tsx index b9e2f9d35d..12a298e635 100644 --- a/apps/web/components/task-create-dialog-submit.tsx +++ b/apps/web/components/task-create-dialog-submit.tsx @@ -111,6 +111,7 @@ export function useTaskSubmitHandlers({ repositoryLocalPath, noRepository, workspacePath, + blockedBy, transformDescriptionBeforeSubmit, }: SubmitHandlersDeps) { const router = useRouter(); @@ -426,6 +427,7 @@ export function useTaskSubmitHandlers({ // "empty path string" on the wire. workspacePath: resolveWorkspacePath(noRepository, workspacePath), autopilot, + blockedBy, }); submittedPayload = payload; return payload; @@ -457,6 +459,7 @@ export function useTaskSubmitHandlers({ workspaceId, effectiveWorkflowId, autoTitle, + blockedBy, agentProfileId, executorId, executorProfileId, @@ -689,6 +692,7 @@ export function useTaskSubmitHandlers({ attachments, workspacePath: resolveWorkspacePath(noRepository, workspacePath), autopilot, + blockedBy, }); p.workflow_step_id = requirements.workflowStepId; submittedPayload = p; @@ -735,6 +739,7 @@ export function useTaskSubmitHandlers({ toast, descriptionInputRef, setIsCreatingTask, + blockedBy, ]); const editSubmitHandler = isStartedEdit ? handleUpdateWithoutAgent : handleEditSubmit; diff --git a/apps/web/components/task-create-dialog-types.ts b/apps/web/components/task-create-dialog-types.ts index 647ad600ea..4b226ce901 100644 --- a/apps/web/components/task-create-dialog-types.ts +++ b/apps/web/components/task-create-dialog-types.ts @@ -304,6 +304,9 @@ export type TaskFormInputsHandle = { }; export type DialogFormState = { + /** Predecessor task IDs chosen in the dialog's "Depends on" selector. */ + blockedBy: string[]; + setBlockedBy: (v: string[]) => void; taskName: string; setTaskName: (v: string) => void; hasTitle: boolean; @@ -479,6 +482,8 @@ export type SubmitHandlersDeps = { repositoryLocalPath: string; /** When true, the task is created with no repositories (repo-less mode). */ noRepository: boolean; + /** Predecessor task IDs to link at creation time. */ + blockedBy?: string[]; /** Optional host folder for repo-less tasks; empty means kandev creates a scratch workspace. */ workspacePath: string; /** diff --git a/apps/web/components/task-create-dialog.test.tsx b/apps/web/components/task-create-dialog.test.tsx index 21fdb7fa8a..db4bc56f0e 100644 --- a/apps/web/components/task-create-dialog.test.tsx +++ b/apps/web/components/task-create-dialog.test.tsx @@ -231,6 +231,8 @@ vi.mock("@/components/task-create-dialog-state", () => ({ function buildMockFs(initialDescription = ORIGINAL_PROMPT): DialogFormState { return { + blockedBy: [], + setBlockedBy: () => undefined, taskName: "Task title", autopilot: false, setAutopilot: () => undefined, diff --git a/apps/web/components/task-create-dialog.tsx b/apps/web/components/task-create-dialog.tsx index e27bc588cb..824c427411 100644 --- a/apps/web/components/task-create-dialog.tsx +++ b/apps/web/components/task-create-dialog.tsx @@ -15,12 +15,13 @@ import { WorkflowSection, DialogPromptSection, } from "@/components/task-create-dialog-form-body"; +import { CreateModeSelectors } from "@/components/task-create-dialog-create-mode-selectors"; +import { TaskCreateDependencies } from "@/components/task-create-dialog-dependencies"; import { AgentSelector, ExecutorProfileSelector, InlineTaskName, } from "@/components/task-create-dialog-selectors"; -import { CreateModeSelectors } from "@/components/task-create-dialog-create-mode-selectors"; import { RepoChipsRow } from "@/components/task-create-dialog-repo-chips"; import type { DialogFormBodyProps, @@ -110,6 +111,13 @@ function CreateModeBody(props: DialogFormBodyProps) { onComposerSubmit={props.onComposerSubmit} /> + {isCreateMode && !isTaskStarted && ( + + )} {props.bottomSlot} ); diff --git a/apps/web/components/task/chat/chat-input-area.test.ts b/apps/web/components/task/chat/chat-input-area.test.ts index ea50e52f92..f6a46feb3b 100644 --- a/apps/web/components/task/chat/chat-input-area.test.ts +++ b/apps/web/components/task/chat/chat-input-area.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; -import { buildSubmitMessage, shouldRenderChatStatusBar } from "./chat-input-area"; +import { buildSubmitMessage } from "./chat-input-area"; +import { resolveStatusRowTaskId, shouldRenderChatStatusBar } from "./chat-status-bar"; import type { AgentMessageComment } from "@/lib/state/slices/comments"; const messageComment: AgentMessageComment = { @@ -49,3 +50,20 @@ describe("shouldRenderChatStatusBar", () => { ).toBe(false); }); }); + +describe("resolveStatusRowTaskId", () => { + it("prefers the session-derived task so existing hosts are unchanged", () => { + expect(resolveStatusRowTaskId("from-session", "from-host")).toBe("from-session"); + }); + + // The regression: a blocked chain step has no session, so the session-derived + // id is null and the status row (and with it the dependency chip) disappeared + // on exactly the tasks the chip exists to describe. + it("falls back to the host's task when the task has no session yet", () => { + expect(resolveStatusRowTaskId(null, "from-host")).toBe("from-host"); + }); + + it("stays null when neither side knows a task", () => { + expect(resolveStatusRowTaskId(null, null)).toBeNull(); + }); +}); diff --git a/apps/web/components/task/chat/chat-input-area.tsx b/apps/web/components/task/chat/chat-input-area.tsx index b735e2adb3..19d2b664e6 100644 --- a/apps/web/components/task/chat/chat-input-area.tsx +++ b/apps/web/components/task/chat/chat-input-area.tsx @@ -1,18 +1,6 @@ "use client"; -import { useCallback, useState, type ReactNode } from "react"; -import { IconArrowRight } from "@tabler/icons-react"; -import { Button } from "@kandev/ui/button"; -import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; -import { TodoIndicator } from "./todo-indicator"; -import { AutoScrollToggleButton } from "./auto-scroll-toggle-button"; -import { PRMergedBanner, PRClosedBanner } from "./pr-archive-banners"; -import { PRStatusChip } from "@/components/github/pr-status-chip"; -import { MRStatusChip } from "@/components/gitlab/mr-status-chip"; -import { AzureDevOpsTaskPullRequestChip } from "@/components/azure-devops/azure-devops-task-pull-request-chip"; -import { RegisteredChangeRequestStatus } from "@/components/integrations/registered-change-request-status"; -import { shareableSessionStateClient } from "@/components/task/share/share-button"; -import { TranscriptNavGroup } from "@/components/task/chat/transcript-nav-group"; +import { useCallback, useState } from "react"; import { getWebSocketClient } from "@/lib/ws/connection"; import { useKeyboardShortcut } from "@/hooks/use-keyboard-shortcut"; import { useMessageHandler, buildTaskMentionsContext } from "@/hooks/use-message-handler"; @@ -42,9 +30,8 @@ import type { ChatPanelState } from "./use-chat-panel-state"; import { useComposerProps } from "./use-composer-props"; import { cn } from "@/lib/utils"; import { resolveComposerWorkspaceId } from "./composer-workspace"; -import { useTranslation } from "react-i18next"; import { t } from "@/lib/i18n"; -import { AutopilotChatChip, useTaskAutopilot } from "./task-autopilot-chat-chip"; +import { ChatStatusBar, resolveStatusRowTaskId } from "./chat-status-bar"; const PLAN_CONTEXT_PATH = "plan:context"; @@ -312,172 +299,6 @@ export function useChatPanelHandlers( return { handleCancelTurn }; } -type TodoDisplayItem = { - text: string; - done?: boolean; - status?: "pending" | "in_progress" | "completed" | "failed"; -}; - -export function shouldRenderChatStatusBar({ - hasTask, - hasTodos, - hasQueueChip, - showRightControls, - showProceed, -}: { - hasTask: boolean; - hasTodos: boolean; - hasQueueChip: boolean; - showRightControls: boolean; - showProceed: boolean; -}): boolean { - return hasTask || hasTodos || hasQueueChip || showRightControls || showProceed; -} - -function getRightControlVisibility({ - taskId, - sessionId, - sessionState, - showAutoScrollControl, - showScrollToLastPrompt, - showScrollToStart, -}: { - taskId: string | null; - sessionId: string | null; - sessionState: string | null; - showAutoScrollControl: boolean; - showScrollToLastPrompt: boolean | undefined; - showScrollToStart: boolean | undefined; -}) { - const canShare = !!taskId && !!sessionId && shareableSessionStateClient(sessionState); - const showRightControls = - (showAutoScrollControl && !!sessionId) || - canShare || - !!showScrollToLastPrompt || - !!showScrollToStart; - return { canShare, showRightControls }; -} - -/** - * Row above the composer showing todo progress, PR/CI status chips, - * merged/closed PR banners, the auto-scroll toggle + Share (right-aligned), - * and a "move to next step" action when the workflow allows it. - */ -type ChatStatusBarProps = { - todoItems: TodoDisplayItem[]; - taskId: string | null; - sessionId: string | null; - sessionState: string | null; - nextStepName: string | null; - onProceed: () => void; - isAgentBusy: boolean; - isMoving: boolean; - queueChip?: ReactNode; - showScrollToLastPrompt?: boolean; - onScrollToLastPrompt?: () => void; - lastPromptScrollDirection?: "up" | "down"; - showScrollToStart?: boolean; - onScrollToStart?: () => void; -}; - -function ChatStatusBar({ - todoItems, - taskId, - sessionId, - sessionState, - nextStepName, - onProceed, - isAgentBusy, - isMoving, - queueChip, - showScrollToLastPrompt, - onScrollToLastPrompt, - lastPromptScrollDirection, - showScrollToStart, - onScrollToStart, -}: ChatStatusBarProps) { - const { t } = useTranslation(); - const showTodos = todoItems.length > 0; - const showProceed = !!nextStepName && !isAgentBusy; - const autopilot = useTaskAutopilot(taskId); - const showAutoScrollControl = useAppStore( - (state) => state.userSettings.showTranscriptAutoScrollControl, - ); - const { canShare, showRightControls } = getRightControlVisibility({ - taskId, - sessionId, - sessionState, - showAutoScrollControl, - showScrollToLastPrompt, - showScrollToStart, - }); - if ( - !shouldRenderChatStatusBar({ - hasTask: !!taskId, - hasTodos: showTodos, - hasQueueChip: !!queueChip, - showRightControls, - showProceed, - }) - ) { - return null; - } - // PRMergedBanner returns null internally when not applicable - return ( -
- {showTodos && } - {autopilot && } - - - - - {queueChip} - {/* Distinct per-banner keys: the key remounts the banner on task switch - so its dismissed state re-initialises, and keeping the two suffixes - different avoids a duplicate-sibling-key collision. */} - {taskId && } - {taskId && } - {showRightControls && ( -
- {sessionId && } - -
- )} - {showProceed && ( - - - - - {t("task:moveTaskToTheNextWorkflow")} - - )} -
- ); -} - type ChatInputAreaProps = { chatInputRef: React.RefObject; clarificationKey: number; @@ -511,6 +332,13 @@ type ChatInputAreaProps = { * start of the transcript. */ showScrollToStart?: boolean; onScrollToStart?: () => void; + /** + * Task this composer belongs to, for the status row only. Hosts that mount a + * task's chat before any session exists pass it so the dependency / autopilot + * chips still render; without it the row is hidden on exactly the tasks a + * dependency chip is about. + */ + statusTaskId?: string | null; }; /** Resolves whether this session's executor environment is unavailable, and why. */ @@ -597,8 +425,10 @@ export function ChatInputArea({ lastPromptScrollDirection, showScrollToStart, onScrollToStart, + statusTaskId = null, }: ChatInputAreaProps) { const { resolvedSessionId, taskId, isAgentBusy } = panelState; + const statusRowTaskId = resolveStatusRowTaskId(taskId, statusTaskId); const composerWorkspaceId = useComposerWorkspaceId(resolvedSessionId, taskId); const sessionState = panelState.session?.state ?? null; const canDrainQueue = canManuallyDrainQueue(panelState.pendingClarification, sessionState); @@ -637,7 +467,7 @@ export function ChatInputArea({ renderStatusBar={(queueChip) => ( void; + isAgentBusy: boolean; + isMoving: boolean; + queueChip?: ReactNode; + showScrollToLastPrompt?: boolean; + onScrollToLastPrompt?: () => void; + lastPromptScrollDirection?: "up" | "down"; + showScrollToStart?: boolean; + onScrollToStart?: () => void; +}; + +export function ChatStatusBar({ + todoItems, + taskId, + sessionId, + sessionState, + nextStepName, + onProceed, + isAgentBusy, + isMoving, + queueChip, + showScrollToLastPrompt, + onScrollToLastPrompt, + lastPromptScrollDirection, + showScrollToStart, + onScrollToStart, +}: ChatStatusBarProps) { + const { t } = useTranslation(); + const showTodos = todoItems.length > 0; + const showProceed = !!nextStepName && !isAgentBusy; + const autopilot = useTaskAutopilot(taskId); + const showAutoScrollControl = useAppStore( + (state) => state.userSettings.showTranscriptAutoScrollControl, + ); + const { canShare, showRightControls } = getRightControlVisibility({ + taskId, + sessionId, + sessionState, + showAutoScrollControl, + showScrollToLastPrompt, + showScrollToStart, + }); + if ( + !shouldRenderChatStatusBar({ + hasTask: !!taskId, + hasTodos: showTodos, + hasQueueChip: !!queueChip, + showRightControls, + showProceed, + }) + ) { + return null; + } + // PRMergedBanner returns null internally when not applicable + return ( +
+ {showTodos && } + {autopilot && } + + + + + + {queueChip} + {/* Distinct per-banner keys: the key remounts the banner on task switch + so its dismissed state re-initialises, and keeping the two suffixes + different avoids a duplicate-sibling-key collision. */} + {taskId && } + {taskId && } + {showRightControls && ( +
+ {sessionId && } + +
+ )} + {showProceed && ( + + + + + {t("task:moveTaskToTheNextWorkflow")} + + )} +
+ ); +} diff --git a/apps/web/components/task/dockview-panel-content.tsx b/apps/web/components/task/dockview-panel-content.tsx index 9332de6415..bde0276355 100644 --- a/apps/web/components/task/dockview-panel-content.tsx +++ b/apps/web/components/task/dockview-panel-content.tsx @@ -79,6 +79,7 @@ function ChatContent({ panelId, params }: { panelId: string; params: Record undefined, taskName: "", setTaskName: NOOP, hasTitle: false, diff --git a/apps/web/components/task/passthrough-toolbar.tsx b/apps/web/components/task/passthrough-toolbar.tsx index 1dab97e6eb..41a978945c 100644 --- a/apps/web/components/task/passthrough-toolbar.tsx +++ b/apps/web/components/task/passthrough-toolbar.tsx @@ -22,6 +22,7 @@ import { Textarea } from "@kandev/ui/textarea"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { PRStatusChip } from "@/components/github/pr-status-chip"; import { MRStatusChip } from "@/components/gitlab/mr-status-chip"; +import { TaskDependencyChip } from "@/components/task/task-dependency-chip"; import { AzureDevOpsTaskPullRequestChip } from "@/components/azure-devops/azure-devops-task-pull-request-chip"; import { RegisteredChangeRequestStatus } from "@/components/integrations/registered-change-request-status"; import { PRMergedBanner } from "./chat/pr-archive-banners"; @@ -539,6 +540,7 @@ function PassthroughStatusRow({ />
+ diff --git a/apps/web/components/task/task-chat-panel.tsx b/apps/web/components/task/task-chat-panel.tsx index cfd6493cc6..c1d1d0d0c4 100644 --- a/apps/web/components/task/task-chat-panel.tsx +++ b/apps/web/components/task/task-chat-panel.tsx @@ -170,6 +170,13 @@ type TaskChatPanelProps = { onSend?: (payload: ChatSubmitPayload) => ChatSubmitResult; sessionId?: string | null; taskId?: string | null; + /** + * Task this panel belongs to, independent of whether it has a session yet. + * Only the status row uses it, so a task with no session still shows its + * dependency and autopilot chips. `taskId` above stays session-gated because + * it also drives plan mode, the composer, and read tracking. + */ + statusTaskId?: string | null; onOpenFile?: (path: string) => void; showRequestChangesTooltip?: boolean; onRequestChangesTooltipDismiss?: () => void; @@ -193,6 +200,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({ onSend, sessionId = null, taskId: taskIdHint = null, + statusTaskId = null, onOpenFile, showRequestChangesTooltip = false, onRequestChangesTooltipDismiss, @@ -414,6 +422,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({ lastPromptScrollDirection={scrollDirection} showScrollToStart={showScrollToStartButton} onScrollToStart={scrollToStart} + statusTaskId={statusTaskId ?? taskIdHint} /> ); @@ -479,6 +488,7 @@ type ChatFooterProps = { lastPromptScrollDirection: "up" | "down"; showScrollToStart: boolean; onScrollToStart: () => void; + statusTaskId: string | null; }; function ChatFooter({ @@ -498,6 +508,7 @@ function ChatFooter({ lastPromptScrollDirection, showScrollToStart, onScrollToStart, + statusTaskId, }: ChatFooterProps) { const { t } = useTranslation(); if (isArchived) { @@ -524,6 +535,7 @@ function ChatFooter({ lastPromptScrollDirection={lastPromptScrollDirection} showScrollToStart={showScrollToStart} onScrollToStart={onScrollToStart} + statusTaskId={statusTaskId} /> ); } diff --git a/apps/web/components/task/task-dependency-chip.test.tsx b/apps/web/components/task/task-dependency-chip.test.tsx new file mode 100644 index 0000000000..2e11bdc1d6 --- /dev/null +++ b/apps/web/components/task/task-dependency-chip.test.tsx @@ -0,0 +1,116 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const getTaskDependenciesMock = vi.hoisted(() => vi.fn()); +vi.mock("@/lib/api/domains/task-dependencies-api", () => ({ + getTaskDependencies: getTaskDependenciesMock, +})); +import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import { StateProvider } from "@/components/state-provider"; +import { TaskDependencyChip } from "./task-dependency-chip"; +import type { KanbanState } from "@/lib/state/slices/kanban/types"; + +beforeEach(() => { + getTaskDependenciesMock.mockReset(); + // Default: the server also reports no edges, so the fetch fallback cannot be + // what makes a "renders nothing" assertion pass. + getTaskDependenciesMock.mockResolvedValue({ id: "unknown", depends_on: [], blocks: [] }); +}); + +afterEach(cleanup); + +type KanbanTask = KanbanState["tasks"][number]; + +const task = (over: Partial & { id: string }): KanbanTask => + ({ + workflowStepId: "step-1", + title: over.id.toUpperCase(), + position: 0, + ...over, + }) as KanbanTask; + +const CHIP_TESTID = "task-dependency-chip"; + +function renderChip(tasks: KanbanTask[], taskId: string) { + return render( + + + , + ); +} + +describe("TaskDependencyChip", () => { + it("renders nothing when the task has no edges in either direction", () => { + renderChip([task({ id: "a" })], "a"); + expect(screen.queryByTestId(CHIP_TESTID)).toBeNull(); + }); + + it("renders nothing for an unknown task", () => { + renderChip([task({ id: "a" })], "missing"); + expect(screen.queryByTestId(CHIP_TESTID)).toBeNull(); + }); + + it("renders when the task only has dependents (it blocks others but is not blocked)", () => { + // The reverse direction alone must surface the chip: "two tasks are waiting + // on this one" is exactly the context the chip exists to show. + renderChip([task({ id: "a", blocks: [{ id: "b" }, { id: "c" }] })], "a"); + expect(screen.getByTestId(CHIP_TESTID)).toBeTruthy(); + }); + + it("renders both directions with counts", () => { + renderChip( + [ + task({ + id: "b", + blocked: true, + blockedReason: "pending", + dependsOn: [{ id: "a", title: "A", status: "pending" }], + blocks: [{ id: "c", title: "C" }], + }), + ], + "b", + ); + const chip = screen.getByTestId(CHIP_TESTID); + expect(chip.textContent).toMatch(/1/); + }); + + it("renders for a resolved dependency so the chain is still visible after it unblocks", () => { + renderChip( + [ + task({ + id: "b", + blocked: false, + dependsOn: [{ id: "a", title: "A", status: "resolved" }], + }), + ], + "b", + ); + expect(screen.getByTestId(CHIP_TESTID)).toBeTruthy(); + }); + + it("drops fetched data as soon as the store can answer 'no edges'", async () => { + // Regression: `fromStore ?? fetched` fell through on the store's `null`, + // which is an explicit "this task has no edges" answer. Switching from a + // task resolved by fetch to one the store knows is edge-free then kept + // rendering the previous task's dependencies. + getTaskDependenciesMock.mockResolvedValue({ + id: "fetched-task", + blocked: true, + depends_on: [{ id: "p", title: "P", status: "pending" }], + blocks: [], + }); + // Explicit empty lists = the store's real "no edges" answer. A task with no + // projection at all is "unknown", which correctly falls through to a fetch. + const store = [task({ id: "a", blocked: false, dependsOn: [], blocks: [] })]; + const { rerender } = renderChip(store, "fetched-task"); + await screen.findByTestId(CHIP_TESTID); + + rerender( + + + , + ); + await waitFor(() => expect(screen.queryByTestId(CHIP_TESTID)).toBeNull()); + }); +}); diff --git a/apps/web/components/task/task-dependency-chip.tsx b/apps/web/components/task/task-dependency-chip.tsx new file mode 100644 index 0000000000..5db6adb906 --- /dev/null +++ b/apps/web/components/task/task-dependency-chip.tsx @@ -0,0 +1,333 @@ +"use client"; + +/** + * TaskDependencyChip — the status-row chip that shows what a task is waiting on + * and what is waiting on it. + * + * Mounted next to `` in the row above the composer. Both + * directions live on the task payload already (`dependsOn` / `blocks`, each + * entry carrying id/title/state), so opening the chip costs no fetch. + * + * Follows PRStatusChip's two conventions: return null when the task has no + * edges, and use a drawer on touch rather than a hover card. + */ + +import { useEffect, useMemo, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { IconLink, IconLock, IconAlertTriangle, IconCheck, IconClock } from "@tabler/icons-react"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { + Drawer, + DrawerClose, + DrawerContent, + DrawerDescription, + DrawerHeader, + DrawerTitle, +} from "@kandev/ui/drawer"; +import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover"; +import { cn } from "@kandev/ui/lib/utils"; +import { useAppStore } from "@/components/state-provider"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; +import { getTaskDependencies } from "@/lib/api/domains/task-dependencies-api"; +import type { TaskDependencyRef } from "@/lib/state/slices/kanban/types"; + +type DependencyChipData = { + blocked: boolean; + blockedReason?: string; + dependsOn: TaskDependencyRef[]; + blocks: TaskDependencyRef[]; +}; + +/** + * useTaskDependencies reads the derived dependency projection for a task from + * the store. Returns null when the task is unknown or has no edges in either + * direction, which is also the chip's "render nothing" signal. + * + * The selector returns the task itself — a stable reference — and the derived + * shape is built in a memo afterwards. Returning a freshly-built object FROM the + * selector compares unequal under Object.is on every store read, which loops + * until React bails out and renders nothing at all. + */ +export function useTaskDependencies(taskId: string | null): DependencyChipData | null { + const fromStore = useTaskDependenciesFromStore(taskId); + const storeAnswered = fromStore !== undefined; + const fetched = useFetchedTaskDependencies(taskId, storeAnswered); + // `??` would be wrong here: the store's `null` is a real answer ("this task + // has no edges"), and falling through on it would keep showing the previously + // fetched task's dependencies after an edge is removed or after switching to + // a task the store knows is edge-free. Only `undefined` means "ask the server". + return storeAnswered ? fromStore : fetched; +} + +/** + * Fetches the projection when the store copy has none. + * + * `undefined` from the store means "this copy carries no dependency fields" (a + * lightweight task.updated overwrote them, or landed before hydration); `null` + * means "read it, there are no edges". Only the former needs a fetch. + */ +function useFetchedTaskDependencies( + taskId: string | null, + storeAnswered: boolean, +): DependencyChipData | null { + const [data, setData] = useState(null); + useEffect(() => { + if (!taskId || storeAnswered) { + // Drop anything fetched for a previous task/state so it can never be + // rendered again once the store can answer. + setData(null); + return; + } + let cancelled = false; + void getTaskDependencies(taskId) + .then((task) => { + if (cancelled) return; + const dependsOn = (task.depends_on ?? []) as TaskDependencyRef[]; + const blocks = (task.blocks ?? []) as TaskDependencyRef[]; + setData( + dependsOn.length === 0 && blocks.length === 0 + ? null + : { + blocked: task.blocked ?? false, + blockedReason: task.blocked_reason, + dependsOn, + blocks, + }, + ); + }) + .catch(() => { + // A failed read hides the chip rather than showing a wrong state. + if (!cancelled) setData(null); + }); + return () => { + cancelled = true; + }; + }, [taskId, storeAnswered]); + return data; +} + +/** + * Store fast path. Returns `undefined` when the store copy carries no dependency + * projection at all, so the caller knows to fetch rather than render nothing. + */ +function useTaskDependenciesFromStore( + taskId: string | null, +): DependencyChipData | null | undefined { + const task = useAppStore((state) => { + if (!taskId) return undefined; + // Nil-safe on both slices: this renders inside a task panel that can mount + // before board hydration completes. + const fromBoard = state.kanban?.tasks?.find((candidate) => candidate.id === taskId); + if (fromBoard) return fromBoard; + for (const snapshot of Object.values(state.kanbanMulti?.snapshots ?? {})) { + const hit = snapshot.tasks?.find((candidate) => candidate.id === taskId); + if (hit) return hit; + } + return undefined; + }); + return useMemo(() => { + if (!task) return undefined; + // No projection on this copy: the caller must ask the server. + if (task.dependsOn === undefined && task.blocks === undefined) return undefined; + const dependsOn = task.dependsOn ?? []; + const blocks = task.blocks ?? []; + if (dependsOn.length === 0 && blocks.length === 0) return null; + return { + blocked: task.blocked ?? false, + blockedReason: task.blockedReason, + dependsOn, + blocks, + }; + }, [task]); +} + +function statusIcon(status?: string) { + if (status === "resolved") + return ; + if (status === "failed") + return ; + return ; +} + +function DependencyRow({ + ref: entry, + showStatus, +}: { + ref: TaskDependencyRef; + showStatus: boolean; +}) { + const { t } = useTranslation(); + return ( + + {showStatus ? ( + statusIcon(entry.status) + ) : ( + + )} + {entry.title || entry.id} + {entry.state && ( + + {t(`task:state.${entry.state}`, { defaultValue: entry.state })} + + )} + + ); +} + +function DependencyLists({ data }: { data: DependencyChipData }) { + const { t } = useTranslation(); + return ( +
+ {data.dependsOn.length > 0 && ( +
+

+ {t("task:dependencyBlockedBy")} +

+ {data.dependsOn.map((entry) => ( + + ))} +
+ )} + {data.blocks.length > 0 && ( +
+

+ {t("task:dependencyBlocks")} +

+ {data.blocks.map((entry) => ( + + ))} +
+ )} + {data.blockedReason === "failed" && ( +

+ {t("task:dependencyChainHalted")} +

+ )} +
+ ); +} + +function ChipTriggerContent({ data }: { data: DependencyChipData }) { + const { t } = useTranslation(); + return ( + <> + {data.blocked ? : } + + {data.dependsOn.length > 0 && + t("task:dependencyCountBlockedBy", { count: data.dependsOn.length })} + {data.dependsOn.length > 0 && data.blocks.length > 0 && " · "} + {data.blocks.length > 0 && t("task:dependencyCountBlocks", { count: data.blocks.length })} + + + ); +} + +/** + * Pill styling follows the house formula for status chips: a fully rounded + * outline, a 10%-opacity tint of the state colour, a 35%-opacity border of the + * same colour, and the colour itself as the text — see the workspace badge in + * `app-sidebar/sections/settings/workspaces-group.tsx`. A solid fill with muted + * text reads as a disabled button next to the sibling chips in this row. + * + * Reds use the repo's soft pair `text-red-600 dark:text-red-400` (see + * `settings/plugins/plugin-status-badge.tsx`): a flat red-500 is harsh against a + * dark surface. + * + * Colour carries the state: red when the chain has halted on a failed + * predecessor (needs a human), primary while merely waiting or purely + * informational. Deliberately not amber — that is the Autopilot chip's colour and + * both can sit in this row at once. + */ +const TRIGGER_BASE = + "inline-flex h-[26px] shrink-0 cursor-pointer items-center gap-1.5 rounded-full border px-2.5 text-xs font-medium leading-none transition-colors"; + +const TRIGGER_TONES = { + failed: "border-red-500/40 bg-red-500/10 text-red-600 dark:text-red-400 hover:bg-red-500/15", + waiting: "border-primary/35 bg-primary/10 text-primary hover:bg-primary/15", +} as const; + +function triggerTone(data: DependencyChipData): string { + return data.blockedReason === "failed" ? TRIGGER_TONES.failed : TRIGGER_TONES.waiting; +} + +export function TaskDependencyChip({ taskId }: { taskId: string | null }) { + const data = useTaskDependencies(taskId); + const usesMobileDrawer = useTouchDrawer(); + const { t } = useTranslation(); + const [open, setOpen] = useState(false); + // No edges in either direction: nothing to say. Same null-when-not-applicable + // contract as PRStatusChip. + if (!data) return null; + + const toneClass = triggerTone(data); + + if (usesMobileDrawer) { + return ( + + + + + {t("task:dependencies")} + + {t("task:dependencyChipLabel")} + + + + + +
+ +
+ + + ); + } + + return ( + + + + + + + + + ); +} + +/** Badge form used where a chip trigger is not appropriate (e.g. compact rows). */ +export function TaskDependencyBadge({ taskId }: { taskId: string | null }) { + const data = useTaskDependencies(taskId); + const { t } = useTranslation(); + if (!data?.blocked) return null; + return ( + + + {t("task:dependencyCountBlockedBy", { count: data.dependsOn.length })} + + ); +} diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 6f01cf16cd..21955c3552 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -176,6 +176,25 @@ type CreateTaskOpts = { workspace_mode?: "inherit_parent" | "new_workspace" | "shared_group"; workspace_group_id?: string; attachments?: MessageAttachmentInput[]; + /** Task IDs this task must wait on. Suppresses the immediate agent launch. */ + blocked_by?: string[]; + /** Force the start-when-unblocked intent on or off; defaults from start_agent. */ + start_when_unblocked?: boolean; +}; + +export type TaskDependencyRef = { + id: string; + title: string; + state: string; + /** Only present on `depends_on` entries. */ + status?: "resolved" | "failed" | "pending" | "missing"; +}; + +export type TaskDependencyProjection = { + blocked?: boolean; + blocked_reason?: "pending" | "failed" | "unknown"; + depends_on?: TaskDependencyRef[]; + blocks?: TaskDependencyRef[]; }; type TaskRepositoryInput = { @@ -225,6 +244,10 @@ function buildCreateTaskBody( setIf(body, "parent_id", options.parent_id); setIf(body, "workspace_mode", options.workspace_mode); setIf(body, "workspace_group_id", options.workspace_group_id); + setIf(body, "blocked_by", options.blocked_by); + if (options.start_when_unblocked !== undefined) { + body.start_when_unblocked = options.start_when_unblocked; + } return body; } @@ -248,6 +271,10 @@ type OptionalAgentTaskOpts = { workspace_mode?: "inherit_parent" | "new_workspace" | "shared_group"; autopilot?: boolean; attachments?: MessageAttachmentInput[]; + /** Task IDs this task must wait on. Suppresses the immediate agent launch. */ + blocked_by?: string[]; + /** Force the start-when-unblocked intent on or off; defaults from start_agent. */ + start_when_unblocked?: boolean; }; /** `repositories` (with per-entry branches) takes precedence over the shorthand @@ -272,6 +299,10 @@ function buildOptionalAgentTaskFields(opts?: OptionalAgentTaskOpts): Record { return this.request("POST", "/api/v1/tasks", buildCreateTaskBody(workspaceId, title, opts)); @@ -654,6 +689,10 @@ export class ApiClient { /** Start the task with the immutable autopilot MCP/prompt contract. */ autopilot?: boolean; attachments?: MessageAttachmentInput[]; + /** Task IDs this task must wait on. Suppresses the immediate agent launch. */ + blocked_by?: string[]; + /** Force the start-when-unblocked intent on or off; defaults from start_agent. */ + start_when_unblocked?: boolean; }, ): Promise { return this.request("POST", "/api/v1/tasks", { @@ -1950,6 +1989,43 @@ export class ApiClient { return this.request("GET", `/api/v1/tasks/${taskId}/sessions`); } + /** + * Read a task's dependency projection. `blocked_reason` is `pending`, + * `failed`, or `unknown`; the last one means the store could not be read and + * the gate failed closed, so it must not be treated as unblocked. + */ + async getTaskDependencies(taskId: string): Promise { + return this.request("GET", `/api/v1/tasks/${taskId}`); + } + + /** Record "taskId is blocked by dependsOnTaskId". */ + async addTaskDependency( + taskId: string, + dependsOnTaskId: string, + ): Promise { + return this.request("POST", `/api/v1/tasks/${taskId}/dependencies`, { + depends_on_task_id: dependsOnTaskId, + }); + } + + /** + * Raw add, for asserting the rejection path. A cycle answers 409 with a + * `cycle` array; the typed helper above would throw the body away. + */ + async rawAddTaskDependency(taskId: string, dependsOnTaskId: string): Promise { + return this.rawRequest("POST", `/api/v1/tasks/${taskId}/dependencies`, { + depends_on_task_id: dependsOnTaskId, + }); + } + + /** Remove an edge. Removing one that is not there is a success no-op. */ + async removeTaskDependency( + taskId: string, + dependsOnTaskId: string, + ): Promise { + return this.request("DELETE", `/api/v1/tasks/${taskId}/dependencies/${dependsOnTaskId}`); + } + async setPrimarySession(sessionId: string): Promise { await this.wsRequest("session.set_primary", { session_id: sessionId }); } diff --git a/apps/web/e2e/tests/task/task-dependencies.spec.ts b/apps/web/e2e/tests/task/task-dependencies.spec.ts new file mode 100644 index 0000000000..5f4fe666ce --- /dev/null +++ b/apps/web/e2e/tests/task/task-dependencies.spec.ts @@ -0,0 +1,262 @@ +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; + +// Task dependencies: a peer "B cannot start until A finishes" relationship, +// separate from the parent/child subtask hierarchy. These specs cover the +// guarantees that only hold end to end, where the create path, the event bus, +// the auto-start chokepoint, and the derived projection all participate: +// +// - a blocked task is not launched by any automated path, +// - only SUCCESS resolves a dependency, so a failed predecessor halts a chain, +// - resolution launches the dependent exactly once and never restarts the +// predecessor, +// - a cycle is refused with the offending path, +// - the projection reaches the browser, which is where it kept getting lost. +// +// Predecessors are seeded WITHOUT an agent and completed over the API. That is +// deliberate: driving the terminal transition directly exercises the same +// task.state_changed path a real agent turn ends on, without making the assertion +// depend on mock-agent timing. The DEPENDENTS carry agent profiles, because +// whether they launch is the thing under test. + +/** Sessions a task owns. A blocked task must have none. */ +async function sessionCount(apiClient: ApiClient, taskId: string): Promise { + const { total } = await apiClient.listTaskSessions(taskId); + return total; +} + +/** Poll until a task owns at least one session, failing with a readable message. */ +async function expectStarted(apiClient: ApiClient, taskId: string, label: string): Promise { + await expect + .poll(() => sessionCount(apiClient, taskId), { + timeout: 30_000, + message: `${label} should have been launched by dependency resolution`, + }) + .toBeGreaterThan(0); +} + +/** + * Assert a task stays sessionless. Dependency resolution is asynchronous, so a + * single read would pass before the launch it is meant to catch. Poll for a + * fixed window and require the count to stay at zero throughout. + */ +async function expectStaysUnstarted( + apiClient: ApiClient, + taskId: string, + label: string, +): Promise { + const deadline = Date.now() + 6_000; + while (Date.now() < deadline) { + expect(await sessionCount(apiClient, taskId), `${label} must not be started`).toBe(0); + await new Promise((resolve) => setTimeout(resolve, 500)); + } +} + +test.describe("Task dependencies", () => { + test("a chain runs one step at a time and never restarts a predecessor", async ({ + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const a = await apiClient.createTask(seedData.workspaceId, "Chain A", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const withAgent = { + description: 'e2e:message("step done")', + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }; + const b = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Chain B", + seedData.agentProfileId, + { ...withAgent, blocked_by: [a.id] }, + ); + const c = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Chain C", + seedData.agentProfileId, + { ...withAgent, blocked_by: [b.id] }, + ); + + // A create that declares dependencies records its start rather than firing + // it, even though start_agent defaulted to true. + expect(b.session_id, "B must not be launched on create").toBeFalsy(); + expect(c.session_id, "C must not be launched on create").toBeFalsy(); + expect(await sessionCount(apiClient, b.id)).toBe(0); + expect(await sessionCount(apiClient, c.id)).toBe(0); + + const blockedB = await apiClient.getTaskDependencies(b.id); + expect(blockedB.blocked).toBe(true); + expect(blockedB.blocked_reason).toBe("pending"); + expect(blockedB.depends_on?.map((d) => d.id)).toEqual([a.id]); + // The reverse direction is derived from the same edge, not stored twice. + expect((await apiClient.getTaskDependencies(a.id)).blocks?.map((d) => d.id)).toEqual([b.id]); + + await apiClient.updateTaskState(a.id, "COMPLETED"); + + await expectStarted(apiClient, b.id, "B"); + // C is two hops out. Resolution must not walk the graph transitively. + await expectStaysUnstarted(apiClient, c.id, "C while B is still running"); + + const bSessions = await sessionCount(apiClient, b.id); + await apiClient.updateTaskState(b.id, "COMPLETED"); + + await expectStarted(apiClient, c.id, "C"); + // The predecessors are untouched by their dependents starting. A restarted + // A is the failure this chain exists to prevent. + expect(await sessionCount(apiClient, a.id), "A must not be restarted").toBe(0); + expect(await sessionCount(apiClient, b.id), "B must launch exactly once").toBe(bSessions); + }); + + test("a failed predecessor halts the chain until it succeeds", async ({ + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const predecessor = await apiClient.createTask(seedData.workspaceId, "Halt A", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const dependent = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Halt B", + seedData.agentProfileId, + { + description: 'e2e:message("should only run after a success")', + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + blocked_by: [predecessor.id], + }, + ); + + await apiClient.updateTaskState(predecessor.id, "FAILED"); + + // FAILED is terminal for the parent/child completion signal but NOT for a + // dependency: the chain stops and waits for a human. + await expectStaysUnstarted(apiClient, dependent.id, "the dependent of a failed predecessor"); + const blocked = await apiClient.getTaskDependencies(dependent.id); + expect(blocked.blocked).toBe(true); + expect(blocked.blocked_reason).toBe("failed"); + expect(blocked.depends_on?.[0]?.status).toBe("failed"); + + // Retrying the predecessor to success is one of the three ways out, and the + // intent must survive the failed attempt rather than having been consumed. + await apiClient.updateTaskState(predecessor.id, "COMPLETED"); + await expectStarted(apiClient, dependent.id, "the dependent after a retry"); + }); + + test("removing the last edge unblocks without launching", async ({ apiClient, seedData }) => { + const predecessor = await apiClient.createTask(seedData.workspaceId, "Edge A", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const dependent = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Edge B", + seedData.agentProfileId, + { + description: 'e2e:message("unreachable")', + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + blocked_by: [predecessor.id], + }, + ); + + const after = await apiClient.removeTaskDependency(dependent.id, predecessor.id); + expect(after.blocked).toBe(false); + expect(after.depends_on ?? []).toEqual([]); + + // Unblocking by edge removal is not success, so the recorded intent must + // not fire. Only a resolved predecessor starts a dependent. + await expectStaysUnstarted(apiClient, dependent.id, "a dependent unblocked by edge removal"); + + // Removing an edge that is not there is a success no-op, not a 404. + await expect( + apiClient.removeTaskDependency(dependent.id, predecessor.id), + ).resolves.toBeTruthy(); + }); + + test("a cycle is refused with the offending path", async ({ apiClient, seedData }) => { + const base = { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }; + const a = await apiClient.createTask(seedData.workspaceId, "Cycle A", base); + const b = await apiClient.createTask(seedData.workspaceId, "Cycle B", base); + const c = await apiClient.createTask(seedData.workspaceId, "Cycle C", base); + + await apiClient.addTaskDependency(b.id, a.id); + await apiClient.addTaskDependency(c.id, b.id); + + // A depending on C would close A -> C -> B -> A. + const response = await apiClient.rawAddTaskDependency(a.id, c.id); + expect(response.status).toBe(409); + const body = (await response.json()) as { cycle?: string[] }; + expect(body.cycle, "the 409 must carry the path so the UI can render it").toEqual([ + a.id, + c.id, + b.id, + a.id, + ]); + + // The rejected edge must not have been written. + expect((await apiClient.getTaskDependencies(a.id)).depends_on ?? []).toEqual([]); + }); + + test("the board and the open task both show the dependency", async ({ + testPage, + apiClient, + seedData, + }) => { + const predecessor = await apiClient.createTask(seedData.workspaceId, "Visible A", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const dependent = await apiClient.createTask(seedData.workspaceId, "Visible B", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + blocked_by: [predecessor.id], + }); + + await testPage.goto("/"); + await expect( + testPage.getByTestId(`task-card-${dependent.id}`).getByTestId("kanban-card-blocked-badge"), + ).toBeVisible(); + await expect( + testPage.getByTestId(`task-card-${predecessor.id}`).getByTestId("kanban-card-blocked-badge"), + ).toHaveCount(0); + + // The chip has to survive the boot payload, the WS event stream, and the + // store projection. Every regression in this feature so far has been a + // dropped hop between those, so assert it from the rendered page. + await testPage.goto(`/t/${dependent.id}`); + const chip = testPage.getByTestId("task-dependency-chip"); + await expect(chip).toBeVisible(); + await chip.click(); + await expect( + testPage.getByTestId("task-dependency-entry").filter({ hasText: "Visible A" }), + ).toBeVisible(); + + // A task with no edges in either direction renders no chip at all. + const unrelated = await apiClient.createTask(seedData.workspaceId, "Visible C", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + await testPage.goto(`/t/${unrelated.id}`); + await expect(testPage.getByTestId("task-dependency-chip")).toHaveCount(0); + }); +}); diff --git a/apps/web/eslint.i18n.options.mjs b/apps/web/eslint.i18n.options.mjs index 869d99d564..c4056edb4f 100644 --- a/apps/web/eslint.i18n.options.mjs +++ b/apps/web/eslint.i18n.options.mjs @@ -221,6 +221,9 @@ export const noLiteralStringOptions = { * the completeness check. See docs/i18n.md. */ export const i18nGuardFiles = [ + // Task dependencies (added with the feature, per the same-PR rule). + "components/task/task-dependency-chip.tsx", + "components/task-create-dialog-dependencies.tsx", // The i18n runtime itself. "lib/i18n/**/*.{ts,tsx}", // Settings → Preferences — Appearance, Notifications, Terminal & Editors and diff --git a/apps/web/hooks/use-kanban-display-settings.ts b/apps/web/hooks/use-kanban-display-settings.ts index cefba4b48d..99e6924e52 100644 --- a/apps/web/hooks/use-kanban-display-settings.ts +++ b/apps/web/hooks/use-kanban-display-settings.ts @@ -4,6 +4,7 @@ import { useCallback, useMemo } from "react"; import { useAppStore } from "@/components/state-provider"; import { useUserDisplaySettings } from "@/hooks/use-user-display-settings"; import { useTaskListingView } from "@/hooks/use-task-listing-view"; +import type { TaskListingView } from "@/lib/task-listing/view-preference"; import { linkToTaskOverview } from "@/lib/links"; import type { WorkflowsState } from "@/lib/state/slices"; import { selectWorkflowSwimlanes } from "@/lib/kanban/workflow-swimlanes"; @@ -31,7 +32,7 @@ function baseSettingsPayload(settings: UserSettingsFields): UserSettingsFields { }; } -function taskListingViewFor(mode: string): "kanban" | "pipeline" | "list" { +function taskListingViewFor(mode: string): TaskListingView { if (mode === "graph2" || mode === "pipeline") return "pipeline"; if (mode === "list") return "list"; return "kanban"; diff --git a/apps/web/lib/api/domains/kanban-api.ts b/apps/web/lib/api/domains/kanban-api.ts index 9734e700d5..cf3b0b6d0e 100644 --- a/apps/web/lib/api/domains/kanban-api.ts +++ b/apps/web/lib/api/domains/kanban-api.ts @@ -101,6 +101,13 @@ export async function createTask( delivery_mode?: "prompt" | "path"; }>; parent_id?: string; + /** + * Task IDs this task depends on. With these set, an agent-start request is + * recorded as a start-when-unblocked intent rather than launching now. + */ + blocked_by?: string[]; + /** Explicitly opt out of (false) or into (true) the auto-start-on-unblock intent. */ + start_when_unblocked?: boolean; workspace_path?: string; priority?: string; project_id?: string; diff --git a/apps/web/lib/api/domains/task-dependencies-api.ts b/apps/web/lib/api/domains/task-dependencies-api.ts new file mode 100644 index 0000000000..36d6769997 --- /dev/null +++ b/apps/web/lib/api/domains/task-dependencies-api.ts @@ -0,0 +1,29 @@ +import { fetchJson, type ApiRequestOptions } from "@/lib/api/client"; + +const BASE = "/api/v1"; + +export type TaskDependencyRefResponse = { + id: string; + title?: string; + state?: string; + status?: "resolved" | "failed" | "pending"; +}; + +/** + * Reads one task's dependency projection. + * + * The chip prefers the board store, but a task.updated event can land before or + * after hydration carrying no dependency fields, which leaves the store copy + * without them. Rather than depend on WS/hydration ordering for a field the + * server derives per read, fall back to asking for it — the same + * fetch-your-own-data shape `PRStatusChip` already uses via `useTaskPR`. + */ +export function getTaskDependencies(taskId: string, options?: ApiRequestOptions) { + return fetchJson<{ + id: string; + blocked?: boolean; + blocked_reason?: string; + depends_on?: TaskDependencyRefResponse[]; + blocks?: TaskDependencyRefResponse[]; + }>(`${BASE}/tasks/${taskId}`, options); +} diff --git a/apps/web/lib/kanban/map-task-dependencies.test.ts b/apps/web/lib/kanban/map-task-dependencies.test.ts new file mode 100644 index 0000000000..48f75ea8b2 --- /dev/null +++ b/apps/web/lib/kanban/map-task-dependencies.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from "vitest"; +import { toKanbanTask } from "./map-task"; + +describe("toKanbanTask dependency projection", () => { + it("maps both directions with per-entry detail", () => { + const task = toKanbanTask({ + id: "b", + workflow_step_id: "step-1", + title: "B", + blocked: true, + blocked_reason: "pending", + depends_on: [{ id: "a", title: "A", state: "TODO", status: "pending" }], + blocks: [{ id: "c", title: "C", state: "TODO" }], + start_when_unblocked: true, + }); + expect(task.blocked).toBe(true); + expect(task.blockedReason).toBe("pending"); + expect(task.dependsOn).toEqual([{ id: "a", title: "A", state: "TODO", status: "pending" }]); + expect(task.blocks).toEqual([{ id: "c", title: "C", state: "TODO" }]); + expect(task.startWhenUnblocked).toBe(true); + }); + + it("leaves the projection undefined when the payload mentions no dependency key", () => { + // Lightweight task.updated events omit these entirely. Inventing empty lists + // here erased real edges: such an event can insert the task into the board + // store before boot hydration, and hydration then keeps the "fresher" WS copy. + const task = toKanbanTask({ id: "a", workflow_step_id: "step-1", title: "A" }); + expect(task.blocked).toBeUndefined(); + expect(task.dependsOn).toBeUndefined(); + expect(task.blocks).toBeUndefined(); + expect(task.startWhenUnblocked).toBeUndefined(); + }); + + it("treats an explicit empty list as a real 'no edges'", () => { + const task = toKanbanTask({ + id: "a", + workflow_step_id: "step-1", + title: "A", + blocked: false, + depends_on: [], + blocks: [], + }); + expect(task.blocked).toBe(false); + expect(task.dependsOn).toEqual([]); + expect(task.blocks).toEqual([]); + }); + + it("clears blocked state when the payload reports it resolved", () => { + const task = toKanbanTask({ + id: "b", + workflow_step_id: "step-1", + title: "B", + blocked: false, + depends_on: [{ id: "a", title: "A", status: "resolved" }], + }); + expect(task.blocked).toBe(false); + expect(task.blockedReason).toBeUndefined(); + // The edge survives resolution — it is history, not a live gate. + expect(task.dependsOn).toHaveLength(1); + }); + + it("keeps a failed reason distinguishable from pending", () => { + const task = toKanbanTask({ + id: "b", + workflow_step_id: "step-1", + title: "B", + blocked: true, + blocked_reason: "failed", + depends_on: [{ id: "a", title: "A", status: "failed" }], + }); + expect(task.blockedReason).toBe("failed"); + expect(task.dependsOn?.[0].status).toBe("failed"); + }); +}); diff --git a/apps/web/lib/kanban/map-task.ts b/apps/web/lib/kanban/map-task.ts index cce00156e8..53132dbd11 100644 --- a/apps/web/lib/kanban/map-task.ts +++ b/apps/web/lib/kanban/map-task.ts @@ -3,7 +3,7 @@ import { isIssueWatchFromMetadata, issueFieldsFromMetadata, } from "@/lib/metadata-utils"; -import type { KanbanState } from "@/lib/state/slices/kanban/types"; +import type { KanbanState, TaskDependencyRef } from "@/lib/state/slices/kanban/types"; import type { ForegroundActivity, TaskPendingAction, @@ -68,6 +68,11 @@ export type TaskLike = { wip_admitted?: boolean; queued_for_step_id?: string | null; queued_at?: string | null; + blocked?: boolean; + blocked_reason?: string | null; + depends_on?: TaskDependencyRef[] | null; + blocks?: TaskDependencyRef[] | null; + start_when_unblocked?: boolean; metadata?: Record | null; archived_at?: string | null; status_summary?: TaskStatusSummary | null; @@ -132,6 +137,37 @@ function pickWorkspaceFolders(source: TaskLike): KanbanTask["workspaceFolders"] * leave them out of sync again (cf. sidebar filter regressions where the HTTP * snapshot derived `isPRReview` but the WS handler didn't). */ +/** + * dependencyProjection normalizes the derived dependency fields. + * + * A payload that carries NO dependency key at all leaves them undefined rather + * than defaulting to "no edges". Most `task.updated` publishers are lightweight + * and omit them, and inventing empty arrays here is destructive twice over: the + * event can insert the task into the board store before boot hydration runs, and + * hydration then keeps the "fresher" WS copy — permanently erasing the edges and + * blanking the dependency chip. When the keys ARE present (every boot payload and + * list read computes them), an empty list is a real "no edges". + */ +function dependencyProjection( + source: TaskLike, +): Partial< + Pick +> { + const mentionsDependencies = + source.blocked !== undefined || + source.depends_on !== undefined || + source.blocks !== undefined || + source.start_when_unblocked !== undefined; + if (!mentionsDependencies) return {}; + return { + blocked: source.blocked ?? false, + blockedReason: source.blocked_reason ?? undefined, + dependsOn: source.depends_on ?? [], + blocks: source.blocks ?? [], + startWhenUnblocked: source.start_when_unblocked ?? false, + }; +} + export function toKanbanTask(source: TaskLike): KanbanTask { return { id: pickId(source), @@ -166,6 +202,7 @@ export function toKanbanTask(source: TaskLike): KanbanTask { wipAdmitted: source.wip_admitted, queuedForStepId: source.queued_for_step_id, queuedAt: source.queued_at, + ...dependencyProjection(source), statusSummary: source.status_summary, metadata: source.metadata, isArchived: source.archived_at != null, diff --git a/apps/web/lib/state/hydration/hydrator.ts b/apps/web/lib/state/hydration/hydrator.ts index 45bfe5519c..8782867d12 100644 --- a/apps/web/lib/state/hydration/hydrator.ts +++ b/apps/web/lib/state/hydration/hydrator.ts @@ -46,14 +46,47 @@ function mergeKanbanTasks( } else { const existingTime = existing.updatedAt ? new Date(existing.updatedAt).getTime() : 0; const incomingTime = incoming.updatedAt ? new Date(incoming.updatedAt).getTime() : 0; + const idx = draftTasks.findIndex((t) => t.id === incoming.id); if (incomingTime >= existingTime) { - const idx = draftTasks.findIndex((t) => t.id === incoming.id); if (idx >= 0) draftTasks[idx] = incoming; + } else if (idx >= 0) { + backfillServerDerivedFields(draftTasks[idx], incoming); } } } } +/** + * Fields the server derives per read and lightweight WS events omit. + * + * A task.updated that arrives before hydration inserts a copy with a newer + * `updatedAt` and no dependency projection, so the timestamp rule above would + * keep that copy and the boot payload's edges would be lost for the rest of the + * session — which is exactly how the dependency chip silently vanished on any + * task whose agent had produced activity. Backfilling only fills gaps, so a real + * WS-side value (including an explicit "no edges") still wins. + */ +const SERVER_DERIVED_TASK_FIELDS = [ + "blocked", + "blockedReason", + "dependsOn", + "blocks", + "startWhenUnblocked", +] as const; + +function backfillServerDerivedFields( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + target: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + source: any, +): void { + for (const field of SERVER_DERIVED_TASK_FIELDS) { + if (target[field] === undefined && source[field] !== undefined) { + target[field] = source[field]; + } + } +} + /** Hydrate kanban and workspace slices. */ function hydrateKanbanAndWorkspace(draft: Draft, state: HydrationState): void { if (state.kanban) { diff --git a/apps/web/lib/state/slices/kanban/types.ts b/apps/web/lib/state/slices/kanban/types.ts index c3b2775c68..f1ef620c87 100644 --- a/apps/web/lib/state/slices/kanban/types.ts +++ b/apps/web/lib/state/slices/kanban/types.ts @@ -19,6 +19,21 @@ export type KanbanStepEvents = { on_agent_error?: Array<{ type: string; config?: Record }>; }; +/** + * One end of a dependency edge. Carries title and state so the dependency chip + * and blocked badge render without fetching each related task. + * + * `status` is the resolution verdict and is only meaningful on `dependsOn` + * entries: "resolved" (finished successfully), "failed" (FAILED/CANCELLED — + * halts the chain), or "pending" (anything else, including archived). + */ +export type TaskDependencyRef = { + id: string; + title?: string; + state?: TaskStatus; + status?: "resolved" | "failed" | "pending"; +}; + export type KanbanState = { workflowId: string | null; steps: Array<{ @@ -104,6 +119,19 @@ export type KanbanState = { wipAdmitted?: boolean; queuedForStepId?: string; queuedAt?: string; + /** + * Task dependencies. Derived on the backend per read (never stored), so + * these arrive fresh on every boot payload and task.updated event. + */ + blocked?: boolean; + /** "pending" | "failed" | "unknown"; absent when not blocked. */ + blockedReason?: string; + /** Direct predecessors — the tasks this one is waiting on. Not transitive. */ + dependsOn?: TaskDependencyRef[]; + /** Direct dependents — the tasks waiting on this one. Not transitive. */ + blocks?: TaskDependencyRef[]; + /** A launch intent is waiting on dependency resolution. */ + startWhenUnblocked?: boolean; isPRReview?: boolean; isIssueWatch?: boolean; metadata?: Record | null; diff --git a/apps/web/lib/ws/handlers/kanban.ts b/apps/web/lib/ws/handlers/kanban.ts index aaae86d547..2679b946c5 100644 --- a/apps/web/lib/ws/handlers/kanban.ts +++ b/apps/web/lib/ws/handlers/kanban.ts @@ -18,6 +18,50 @@ function queueFields( }; } +/** + * Dependency fields are derived server-side and re-sent whole on every + * task.updated, so an absent list means "no edges", not "unchanged" — falling + * back to the existing value would leave a stale badge after the last edge is + * removed. Only fall back when the payload omits the field entirely (a partial + * update that never touched dependencies). + */ +const DEPENDENCY_PAYLOAD_KEYS = [ + "blocked", + "blocked_reason", + "blockedReason", + "depends_on", + "dependsOn", + "blocks", + "start_when_unblocked", + "startWhenUnblocked", +] as const; + +function touchesDependencies(task: KanbanUpdateTask): boolean { + return DEPENDENCY_PAYLOAD_KEYS.some((key) => key in task); +} + +function dependencyFields( + task: KanbanUpdateTask, + existing: KanbanTask | undefined, +): Pick { + if (!touchesDependencies(task)) { + return { + blocked: existing?.blocked, + blockedReason: existing?.blockedReason, + dependsOn: existing?.dependsOn, + blocks: existing?.blocks, + startWhenUnblocked: existing?.startWhenUnblocked, + }; + } + return { + blocked: task.blocked ?? false, + blockedReason: task.blocked_reason ?? task.blockedReason, + dependsOn: task.depends_on ?? task.dependsOn ?? [], + blocks: task.blocks ?? [], + startWhenUnblocked: task.start_when_unblocked ?? task.startWhenUnblocked ?? false, + }; +} + type KanbanUpdateTask = { id: string; workflowStepId: string; @@ -34,6 +78,14 @@ type KanbanUpdateTask = { wipAdmitted?: boolean; queuedForStepId?: string; queuedAt?: string; + blocked?: boolean; + blocked_reason?: string; + blockedReason?: string; + depends_on?: KanbanTask["dependsOn"]; + dependsOn?: KanbanTask["dependsOn"]; + blocks?: KanbanTask["blocks"]; + start_when_unblocked?: boolean; + startWhenUnblocked?: boolean; }; export function registerKanbanHandlers(store: StoreApi): WsHandlers { @@ -83,6 +135,7 @@ export function registerKanbanHandlers(store: StoreApi): WsHandlers { interrupted: existing?.interrupted, foregroundActivity: existing?.foregroundActivity, ...queueFields(task, existing), + ...dependencyFields(task, existing), }; }); diff --git a/apps/web/lib/ws/handlers/task-dependencies.test.ts b/apps/web/lib/ws/handlers/task-dependencies.test.ts new file mode 100644 index 0000000000..546ceeccfe --- /dev/null +++ b/apps/web/lib/ws/handlers/task-dependencies.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; +import { preserveOmittedDependencyFields } from "./task-dependencies"; +import type { KanbanState } from "@/lib/state/slices/kanban/types"; + +type KanbanTask = KanbanState["tasks"][number]; + +const existing = (): KanbanTask => + ({ + id: "b", + workflowId: "wf-1", + workflowStepId: "s", + title: "B", + position: 0, + blocked: true, + blockedReason: "pending", + dependsOn: [{ id: "a", title: "A", status: "pending" }], + blocks: [{ id: "c", title: "C" }], + startWhenUnblocked: true, + }) as KanbanTask; + +/** What toKanbanTask produces for an event with no dependency fields. */ +const mergedFromBareEvent = (): KanbanTask => + ({ + id: "b", + workflowId: "wf-1", + workflowStepId: "s", + title: "B", + position: 0, + blocked: false, + dependsOn: [], + blocks: [], + startWhenUnblocked: false, + }) as KanbanTask; + +describe("preserveOmittedDependencyFields", () => { + it("restores edges when the event does not mention dependencies", () => { + // The regression this exists for: a task.updated fired by ordinary agent + // activity carries no dependency fields, so without this the hydrated edges + // were replaced by empty arrays and the dependency chip disappeared. + const merged = mergedFromBareEvent(); + preserveOmittedDependencyFields(existing(), merged, { task_id: "b", title: "B" }); + expect(merged.blocked).toBe(true); + expect(merged.blockedReason).toBe("pending"); + expect(merged.dependsOn).toHaveLength(1); + expect(merged.blocks).toHaveLength(1); + expect(merged.startWhenUnblocked).toBe(true); + }); + + it("lets an explicit empty list clear the edges", () => { + // Removing the last edge publishes a payload that DOES mention dependencies + // with an empty list; that is a real clear and must not be reverted. + const merged = mergedFromBareEvent(); + preserveOmittedDependencyFields(existing(), merged, { depends_on: [], blocks: [] }); + expect(merged.blocked).toBe(false); + expect(merged.dependsOn).toEqual([]); + expect(merged.blocks).toEqual([]); + }); + + it("lets an explicit blocked flag through even without the lists", () => { + const merged = mergedFromBareEvent(); + preserveOmittedDependencyFields(existing(), merged, { blocked: false }); + expect(merged.blocked).toBe(false); + }); +}); diff --git a/apps/web/lib/ws/handlers/task-dependencies.ts b/apps/web/lib/ws/handlers/task-dependencies.ts new file mode 100644 index 0000000000..9099e4a37d --- /dev/null +++ b/apps/web/lib/ws/handlers/task-dependencies.ts @@ -0,0 +1,28 @@ +import type { KanbanState } from "@/lib/state/slices/kanban/types"; + +type KanbanTask = KanbanState["tasks"][number]; + +/** + * Dependency edges must survive a task.updated that does not mention them. + * + * `toKanbanTask` defaults the projection to "no edges", which is right for a + * boot payload (the backend always computes it there) but wrong for a + * lightweight event: most task.updated publishers carry no dependency fields, so + * defaulting would erase the hydrated edges and make the dependency chip vanish + * as soon as the task's agent produced any activity. + */ +export function preserveOmittedDependencyFields( + existing: KanbanTask, + merged: KanbanTask, + payload: Record, +): void { + // "Mentions dependencies" means the key is present at all, even as null — + // an explicit empty list is a real clear and must win. + const mentions = "depends_on" in payload || "blocks" in payload || "blocked" in payload; + if (mentions) return; + merged.blocked = existing.blocked; + merged.blockedReason = existing.blockedReason; + merged.dependsOn = existing.dependsOn; + merged.blocks = existing.blocks; + merged.startWhenUnblocked = existing.startWhenUnblocked; +} diff --git a/apps/web/lib/ws/handlers/tasks.ts b/apps/web/lib/ws/handlers/tasks.ts index 232d9806c0..6bddf2b88e 100644 --- a/apps/web/lib/ws/handlers/tasks.ts +++ b/apps/web/lib/ws/handlers/tasks.ts @@ -6,6 +6,7 @@ import { cleanupTaskStorage } from "@/lib/local-storage"; import { removeRecentTask } from "@/lib/recent-tasks"; import { useContextFilesStore } from "@/lib/state/context-files-store"; import { toKanbanTask } from "@/lib/kanban/map-task"; +import { preserveOmittedDependencyFields } from "@/lib/ws/handlers/task-dependencies"; import { isNewerStatusSummary } from "@/lib/task-status-summary"; import { sessionId as toSessionId } from "@/lib/types/http"; import { mergeTaskRepositoryFields } from "@/lib/ws/handlers/task-repositories"; @@ -132,6 +133,7 @@ function mergeTaskUpdate( payloadKey: "status_summary", taskField: "statusSummary", }); + preserveOmittedDependencyFields(existing, merged, payload); return merged; } diff --git a/apps/web/src/locales/en/kanban.json b/apps/web/src/locales/en/kanban.json index 395b780b02..5c65632965 100644 --- a/apps/web/src/locales/en/kanban.json +++ b/apps/web/src/locales/en/kanban.json @@ -11,6 +11,11 @@ "archiveCount_other": "Archive {{count}}", "archiveTask": "Archive task", "autoStart": "Auto-start", + "blockedByCount_one": "Blocked by {{count}}", + "blockedByCount_other": "Blocked by {{count}}", + "blockedByTasksTitle": "Waiting on: {{tasks}}", + "blockedFailed": "Blocker failed", + "blockedPredecessorFailed": "A task this one depends on failed: {{tasks}}", "boardNavigator": "Board navigator", "cannotMoveTasksFromDifferentWorkflows": "Cannot move tasks from different workflows", "changesRequested": "Changes Requested", diff --git a/apps/web/src/locales/en/task.json b/apps/web/src/locales/en/task.json index fcf9cda835..5d2fc3018f 100644 --- a/apps/web/src/locales/en/task.json +++ b/apps/web/src/locales/en/task.json @@ -22,6 +22,7 @@ "addApprover": "+ Add approver", "addAtLeastOneSource": "Add at least one source.", "addBlocker": "+ Add blocker", + "addDependency": "Add dependency…", "addDetailsAboutThisChange": "Add details about this change...", "addFolder": "Add folder", "addLabel": "Add label", @@ -330,6 +331,17 @@ "deleting": "Deleting...", "deliverTheseCommentsToTheCli": "Deliver these comments to the CLI agent's stdin as a single message; no need to open the chat box. The agent receives the same markdown that prepending to a typed message would produce.", "deny": "Deny", + "dependencies": "Dependencies", + "dependencyBlockedBy": "Blocked by", + "dependencyBlocks": "Blocks", + "dependencyChainHalted": "A task this one depends on failed. The chain is stopped until you retry it or remove the dependency.", + "dependencyChipLabel": "Task dependencies", + "dependencyCountBlockedBy_one": "{{count}} blocker", + "dependencyCountBlockedBy_other": "{{count}} blockers", + "dependencyCountBlocks_one": "blocks {{count}}", + "dependencyCountBlocks_other": "blocks {{count}}", + "dependsOn": "Depends on", + "dependsOnWillAutoStart": "This task waits for the tasks above. If you start an agent, it runs automatically once every one of them completes successfully.", "describeWhatNeedsToChange": "Describe what needs to change", "describeWhatYouWantTheAgent": "Describe what you want the agent to do... (@ to insert a saved prompt)", "describeYourChanges": "Describe your changes...", @@ -362,11 +374,11 @@ "documentCount_other": "{{count}} documents", "documentDeleted": "Document deleted", "documentSaved": "Document saved", - "documents": "Documents", - "documentsAvailable": "Documents available", + "documentSummaryEmpty": "empty", "documentSummary_one": "{{count}} line · {{chars}} chars", "documentSummary_other": "{{count}} lines · {{chars}} chars", - "documentSummaryEmpty": "empty", + "documents": "Documents", + "documentsAvailable": "Documents available", "done": "Done", "doubleClickToEditOrPress": "Double-click to edit (or press Enter)", "doubleClickToKeepThisTab": "Double-click to keep this tab open", @@ -577,7 +589,6 @@ "hideSidebar": "Hide sidebar", "hideTheCommentListTheComments": "Hide the comment list. The comments stay queued and will still be sent next time you submit (here or from the chat box).", "host": "Host", - "isolatedWorktreeAt": "Isolated worktree at <1>{{path}}", "iUnderstandAnyUncommittedChangesWill": "I understand any uncommitted changes will be lost.", "image": "Image", "imagePreview": "Image preview", @@ -592,6 +603,7 @@ "inspectingClickToPin": "Inspecting - click to pin, drag to select an area, Esc to exit", "invalidName": "Invalid name", "isTheMaximumFileSizeIs": "{{name}} is {{size}}. The maximum file size is {{maxSize}}.", + "isolatedWorktreeAt": "Isolated worktree at <1>{{path}}", "issue": "Issue", "jiraTicket": "Jira Ticket", "jiraTicketLinked": "Jira ticket linked", @@ -781,6 +793,7 @@ "noCompatibleAgentProfilesFor": "No compatible agent profiles for {{target}}.", "noContent": "No content", "noDecisionYet": "No decision yet", + "noDependencies": "No dependencies", "noDocumentsFound": "No documents found.", "noDocumentsYet": "No documents yet.", "noEditorsAvailable": "No editors available", @@ -1015,6 +1028,7 @@ "remove3": "Remove {{identifier}}", "removeAnnotation": "Remove annotation {{number}}", "removeComment": "Remove comment", + "removeDependency": "Remove dependency", "removeFile": "Remove file", "removeFilter": "Remove filter", "removeFromFavorites": "Remove from favorites", diff --git a/apps/web/src/locales/pseudo/kanban.json b/apps/web/src/locales/pseudo/kanban.json index 031a9433cb..6693405c52 100644 --- a/apps/web/src/locales/pseudo/kanban.json +++ b/apps/web/src/locales/pseudo/kanban.json @@ -11,6 +11,11 @@ "archiveCount_other": "Àŕćĥĩvē {{count}}", "archiveTask": "Àŕćĥĩvē ţàśķ", "autoStart": "Àũţō-śţàŕţ", + "blockedByCount_one": "Ɓĺōćķēď ƀŷ {{count}}", + "blockedByCount_other": "Ɓĺōćķēď ƀŷ {{count}}", + "blockedByTasksTitle": "Ŵàĩţĩńĝ ōń: {{tasks}}", + "blockedFailed": "Ɓĺōćķēŕ ƒàĩĺēď", + "blockedPredecessorFailed": "À ţàśķ ţĥĩś ōńē ďēƥēńďś ōń ƒàĩĺēď: {{tasks}}", "boardNavigator": "Ɓōàŕď ńàvĩĝàţōŕ", "cannotMoveTasksFromDifferentWorkflows": "Ćàńńōţ ḿōvē ţàśķś ƒŕōḿ ďĩƒƒēŕēńţ ŵōŕķƒĺōŵś", "changesRequested": "Ćĥàńĝēś Ŕēqũēśţēď", diff --git a/apps/web/src/locales/pseudo/task.json b/apps/web/src/locales/pseudo/task.json index 9ec6e9a8f4..a45a149fd7 100644 --- a/apps/web/src/locales/pseudo/task.json +++ b/apps/web/src/locales/pseudo/task.json @@ -22,6 +22,7 @@ "addApprover": "+ Àďď àƥƥŕōvēŕ", "addAtLeastOneSource": "Àďď àţ ĺēàśţ ōńē śōũŕćē.", "addBlocker": "+ Àďď ƀĺōćķēŕ", + "addDependency": "Àďď ďēƥēńďēńćŷ…", "addDetailsAboutThisChange": "Àďď ďēţàĩĺś àƀōũţ ţĥĩś ćĥàńĝē...", "addFolder": "Àďď ƒōĺďēŕ", "addLabel": "Àďď ĺàƀēĺ", @@ -330,6 +331,17 @@ "deleting": "Ďēĺēţĩńĝ...", "deliverTheseCommentsToTheCli": "Ďēĺĩvēŕ ţĥēśē ćōḿḿēńţś ţō ţĥē ĆĹĨ àĝēńţ'ś śţďĩń àś à śĩńĝĺē ḿēśśàĝē; ńō ńēēď ţō ōƥēń ţĥē ćĥàţ ƀōx. Ţĥē àĝēńţ ŕēćēĩvēś ţĥē śàḿē ḿàŕķďōŵń ţĥàţ ƥŕēƥēńďĩńĝ ţō à ţŷƥēď ḿēśśàĝē ŵōũĺď ƥŕōďũćē.", "deny": "Ďēńŷ", + "dependencies": "Ďēƥēńďēńćĩēś", + "dependencyBlockedBy": "Ɓĺōćķēď ƀŷ", + "dependencyBlocks": "Ɓĺōćķś", + "dependencyChainHalted": "À ţàśķ ţĥĩś ōńē ďēƥēńďś ōń ƒàĩĺēď. Ţĥē ćĥàĩń ĩś śţōƥƥēď ũńţĩĺ ŷōũ ŕēţŕŷ ĩţ ōŕ ŕēḿōvē ţĥē ďēƥēńďēńćŷ.", + "dependencyChipLabel": "Ţàśķ ďēƥēńďēńćĩēś", + "dependencyCountBlockedBy_one": "{{count}} ƀĺōćķēŕ", + "dependencyCountBlockedBy_other": "{{count}} ƀĺōćķēŕś", + "dependencyCountBlocks_one": "ƀĺōćķś {{count}}", + "dependencyCountBlocks_other": "ƀĺōćķś {{count}}", + "dependsOn": "Ďēƥēńďś ōń", + "dependsOnWillAutoStart": "Ţĥĩś ţàśķ ŵàĩţś ƒōŕ ţĥē ţàśķś àƀōvē. Ĩƒ ŷōũ śţàŕţ àń àĝēńţ, ĩţ ŕũńś àũţōḿàţĩćàĺĺŷ ōńćē ēvēŕŷ ōńē ōƒ ţĥēḿ ćōḿƥĺēţēś śũććēśśƒũĺĺŷ.", "describeWhatNeedsToChange": "Ďēśćŕĩƀē ŵĥàţ ńēēďś ţō ćĥàńĝē", "describeWhatYouWantTheAgent": "Ďēśćŕĩƀē ŵĥàţ ŷōũ ŵàńţ ţĥē àĝēńţ ţō ďō... (@ ţō ĩńśēŕţ à śàvēď ƥŕōḿƥţ)", "describeYourChanges": "Ďēśćŕĩƀē ŷōũŕ ćĥàńĝēś...", @@ -362,11 +374,11 @@ "documentCount_other": "{{count}} ďōćũḿēńţś", "documentDeleted": "Ďōćũḿēńţ ďēĺēţēď", "documentSaved": "Ďōćũḿēńţ śàvēď", - "documents": "Ďōćũḿēńţś", - "documentsAvailable": "Ďōćũḿēńţś àvàĩĺàƀĺē", + "documentSummaryEmpty": "ēḿƥţŷ", "documentSummary_one": "{{count}} ĺĩńē · {{chars}} ćĥàŕś", "documentSummary_other": "{{count}} ĺĩńēś · {{chars}} ćĥàŕś", - "documentSummaryEmpty": "ēḿƥţŷ", + "documents": "Ďōćũḿēńţś", + "documentsAvailable": "Ďōćũḿēńţś àvàĩĺàƀĺē", "done": "Ďōńē", "doubleClickToEditOrPress": "Ďōũƀĺē-ćĺĩćķ ţō ēďĩţ (ōŕ ƥŕēśś Ēńţēŕ)", "doubleClickToKeepThisTab": "Ďōũƀĺē-ćĺĩćķ ţō ķēēƥ ţĥĩś ţàƀ ōƥēń", @@ -577,7 +589,6 @@ "hideSidebar": "Ĥĩďē śĩďēƀàŕ", "hideTheCommentListTheComments": "Ĥĩďē ţĥē ćōḿḿēńţ ĺĩśţ. Ţĥē ćōḿḿēńţś śţàŷ qũēũēď àńď ŵĩĺĺ śţĩĺĺ ƀē śēńţ ńēxţ ţĩḿē ŷōũ śũƀḿĩţ (ĥēŕē ōŕ ƒŕōḿ ţĥē ćĥàţ ƀōx).", "host": "Ĥōśţ", - "isolatedWorktreeAt": "Ĩśōĺàţēď ŵōŕķţŕēē àţ <1>{{path}}", "iUnderstandAnyUncommittedChangesWill": "Ĩ ũńďēŕśţàńď àńŷ ũńćōḿḿĩţţēď ćĥàńĝēś ŵĩĺĺ ƀē ĺōśţ.", "image": "Ĩḿàĝē", "imagePreview": "Ĩḿàĝē ƥŕēvĩēŵ", @@ -592,6 +603,7 @@ "inspectingClickToPin": "Ĩńśƥēćţĩńĝ - ćĺĩćķ ţō ƥĩń, ďŕàĝ ţō śēĺēćţ àń àŕēà, Ēść ţō ēxĩţ", "invalidName": "Ĩńvàĺĩď ńàḿē", "isTheMaximumFileSizeIs": "{{name}} ĩś {{size}}. Ţĥē ḿàxĩḿũḿ ƒĩĺē śĩźē ĩś {{maxSize}}.", + "isolatedWorktreeAt": "Ĩśōĺàţēď ŵōŕķţŕēē àţ <1>{{path}}", "issue": "Ĩśśũē", "jiraTicket": "Ĵĩŕà Ţĩćķēţ", "jiraTicketLinked": "Ĵĩŕà ţĩćķēţ ĺĩńķēď", @@ -781,6 +793,7 @@ "noCompatibleAgentProfilesFor": "Ńō ćōḿƥàţĩƀĺē àĝēńţ ƥŕōƒĩĺēś ƒōŕ {{target}}.", "noContent": "Ńō ćōńţēńţ", "noDecisionYet": "Ńō ďēćĩśĩōń ŷēţ", + "noDependencies": "Ńō ďēƥēńďēńćĩēś", "noDocumentsFound": "Ńō ďōćũḿēńţś ƒōũńď.", "noDocumentsYet": "Ńō ďōćũḿēńţś ŷēţ.", "noEditorsAvailable": "Ńō ēďĩţōŕś àvàĩĺàƀĺē", @@ -1015,6 +1028,7 @@ "remove3": "Ŕēḿōvē {{identifier}}", "removeAnnotation": "Ŕēḿōvē àńńōţàţĩōń {{number}}", "removeComment": "Ŕēḿōvē ćōḿḿēńţ", + "removeDependency": "Ŕēḿōvē ďēƥēńďēńćŷ", "removeFile": "Ŕēḿōvē ƒĩĺē", "removeFilter": "Ŕēḿōvē ƒĩĺţēŕ", "removeFromFavorites": "Ŕēḿōvē ƒŕōḿ ƒàvōŕĩţēś", diff --git a/docs/features.md b/docs/features.md index fbc24a34a1..68fd1005f7 100644 --- a/docs/features.md +++ b/docs/features.md @@ -48,7 +48,7 @@ Agents running on regular kanban tasks receive a task-scoped MCP surface. The av - **Work with multi-repo tasks:** task responses include attached repositories, base branches, checkout branches, and positions so agents can reason about each worktree separately. - **Create tasks and subtasks:** create top-level tasks or child tasks, optionally auto-starting an agent. Subtasks inherit the parent workspace, workflow, agent profile, executor, and repositories unless the agent supplies an override. - **Target sibling repositories:** create a subtask in a different repository while keeping it under the same parent task and workspace. -- **Coordinate dependencies:** create tasks with blocker relationships and inspect related parent, child, sibling, blocker, and blocked-by tasks. +- **Coordinate dependencies:** declare that a task waits on other tasks, at creation (`blocked_by`) or afterwards (`add_task_dependency_kandev` / `remove_task_dependency_kandev`), and inspect related parent, child, sibling, blocker, and blocked-by tasks. A blocked task is not started by any automated path; a create that declares dependencies records its start and fires it once every predecessor succeeds, so chained creates build a chain instead of launching at once. Cycles are rejected with the offending path. - **Attach more branches:** add another `(repository, branch)` worktree to an existing worktree task. Use this for multiple PRs from one task, either in the same repository or across repositories. - **Adjust diff bases:** update a task repository's base branch so changes, ahead/behind counts, and review context compare against the right target branch. - **Move, archive, or delete tasks:** move tasks across workflow steps, including optional handoff prompts for the next agent, or clean up tasks when appropriate. diff --git a/docs/plans/task-dependencies/plan.md b/docs/plans/task-dependencies/plan.md new file mode 100644 index 0000000000..71ff3c0f3c --- /dev/null +++ b/docs/plans/task-dependencies/plan.md @@ -0,0 +1,371 @@ +--- +spec: docs/specs/task-dependencies/spec.md +created: 2026-08-09 +status: not_started +--- + +# Implementation Plan: Task Dependencies and Auto-Start Chains + +## Overview + +Peer-to-peer task dependencies become a core Kanban capability with three +observable behaviors: a blocked task is visible, a blocked task is never +auto-started, and an unblocked task with a recorded launch intent starts exactly +once. + +The work is deliberately additive over four shipped mechanisms rather than a new +subsystem: + +- the `task_blockers` edge table and its BFS cycle detector; +- the `tasks.metadata.deferred_launch` intent with its atomic claim; +- the single auto-start chokepoint `orchestrator.autoStartTaskForStep`; +- WIP admission and queue promotion. + +Nothing in this plan adds a second launch path, a denormalized `is_blocked` +column, or a chain entity. + +## Confirmed current behavior + +Verified in `apps/backend` and `apps/web` at `d38d9a0b8`: + +- `task_blockers (task_id, blocker_task_id, created_at)` exists with a + composite PK and a `CHECK (task_id != blocker_task_id)`. It is created by + `internal/office/repository/sqlite/base.go:createTaskExtensionTables` but + lives in the same database as `tasks`. +- `internal/office/dashboard/service_tasks.go` owns edge creation with + self-edge, cross-workspace, and BFS cycle validation, returning + `*BlockerCycleError{Path}` which the frontend already renders as `A → B → A`. +- `internal/task/service/service_office.go` exposes `AddBlocker` / + `RemoveBlocker` with a separate, weaker cycle check, and errors with + `blocker repository not configured` when `s.blockers == nil`. +- `internal/backendapp/main.go` calls `SetBlockerRepository(repos.Office)` + **after** the `if !cfg.Features.Office { return }` early return, so in a + Kanban-only install the blocker repository is not wired at all: + `create_task` with `blocked_by` fails and `list_related_tasks_kandev` + returns empty `blockers` / `blocked_by`. +- Edge HTTP routes exist only under the Office group: + `POST /api/v1/office/tasks/:id/blockers`, + `DELETE /api/v1/office/tasks/:id/blockers/:blockerId` + (`internal/office/dashboard/handler.go`). +- `internal/office/scheduler/reactivity.go:cascadeBlockersResolved` is the only + reaction to a resolved blocker. It runs from the Office `ApplyTaskMutation` + path when a status becomes done, resolves "done" as + `IsTaskInTerminalStep`, and requires a non-empty + `AssigneeAgentProfileID`. It never fires for a Kanban board move or a + workflow-engine transition. `normalisedStatus` maps `FAILED` to blocked, not + done, so Office already does not cascade on failure. +- `internal/orchestrator/event_handlers_workflow.go:autoStartTaskForStep` is + the single automated-launch chokepoint. It already returns early when + `task.QueuedForStepID != ""`, consumes a `deferred_launch` intent via + `launchDeferredTask`, and guards watcher races with + `MetaKeyAutoStartGuard` / `claimAutoStart` / `restoreAutoStartClaim`. It is + reached from `task.moved` (`handleTaskMovedNoSession`) and + `task.queue_promoted` (`handleTaskQueuePromoted`). +- `wfmodels.IsTerminalStepName` recognizes `done|complete|completed|approved`; + entering such a final step persists `state = COMPLETED`. +- The workflow engine declares `TriggerOnBlockerResolved` ("on_blocker_resolved") + but `Engine.HandleTrigger` does not wire it; Office routes it through + `SetWorkflowEngineDispatcher`. +- `create_task_kandev`'s handler struct reads `blocked_by` + (`internal/mcp/handlers/handlers.go:576`, `json:"blocked_by"`) and forwards it + to `CreateTask`, but the tool's `mcp.NewTool` schema in + `internal/mcp/server/server.go:854` never declares the parameter. It is + therefore undiscoverable to agents and not reliably passable through a + schema-validating client. There is no MCP tool at all for adding or removing a + dependency on an existing task. +- Frontend: `components/task/simple/components/blockers-picker.tsx` exists with + cycle-message handling and optimistic mutation, but is mounted only from + `task-properties.tsx` → `OfficeSimplePane`, which the Kanban route + (`app/tasks/[id]/kanban-task-shell.tsx`) renders as a stub. Kanban has no + dependency UI. +- The status row above the composer is `data-testid="chat-status-bar"` in + `components/task/chat/chat-input-area.tsx`. It hosts `TodoIndicator`, + `PRStatusChip`, `AzureDevOpsTaskPullRequestChip`, a queue chip, PR + merged/closed banners, and right-aligned transcript controls. Its visibility is + gated by `shouldRenderChatStatusBar({hasTask, hasTodos, hasQueueChip, + showRightControls, showProceed})` — a new chip that is not added to that + predicate renders nothing when it is the row's only content. +- A **second** status row exists: `data-testid="passthrough-status-row"` in + `components/task/passthrough-toolbar.tsx`, which mounts its own `PRStatusChip`. + Anything that belongs "next to the PR chip" belongs in both rows. +- `PRStatusChip` returns null when not applicable and branches on + `usesMobileDrawer` to `PRStatusChipDrawer` versus `PRStatusChipHoverCard` — + the mobile pattern to mirror rather than reinvent. +- `components/kanban-card-content.tsx` already renders a `queuedForStepId` + badge — the precedent and the layout neighbour for a blocked badge. +- `lib/kanban/view-registry.ts` is a pluggable board-view registry with + `kanban` and `graph2` ("Pipeline"). `graph2` visualizes one task across + workflow steps, not cross-task edges. `getEffectiveView` forces the Kanban + view on mobile. + +## Backend design + +### One owner for the dependency relationship + +Move the authoritative edge operations into the task domain so a Kanban-only +install has them: + +- Keep the physical `task_blockers` table where it is. It is in the same + database, and relocating live DDL buys nothing. Add the missing indexes via + idempotent migrations in `internal/task/repository/sqlite/base_migrations.go` + (`(blocker_task_id)` for the reverse walk). +- Move the strong validator — self-edge, cross-workspace, BFS cycle with the + existing walk limit and `*BlockerCycleError` — into the task service so both + the new task-scoped routes and the existing Office routes call one + implementation. Delete the weaker `checkCircularBlocker` in + `service_office.go` rather than leaving two validators. +- Wire `SetBlockerRepository` **before** the `Features.Office` early return in + `internal/backendapp/main.go`. This is the smallest change that makes + `blocked_by` and `list_related_tasks_kandev` work in Kanban mode. +- Edge cleanup on task **delete** is explicit repository work, not a SQLite + `ON DELETE CASCADE`, because the table predates the foreign key and must + behave identically on PostgreSQL. **Archive** deliberately keeps the edges: + an archived predecessor reads as `pending`, so dropping them would silently + unblock the chain. + +### Derived blocked state + +Add one service helper that, for a set of task IDs, returns each task's direct +predecessors **and direct dependents**, plus a resolution verdict per +predecessor: + +- resolved — `state = COMPLETED`, or resident in a final step whose name passes + `IsTerminalStepName`; +- failed — `state` is `FAILED` or `CANCELLED`; +- pending — anything else, including archived. + +`blocked` and `blocked_reason` are computed from that verdict on every read and +are never stored. Batch it: the Kanban board reads a whole workflow at once, so +a per-task query would add N round trips to the board load. Follow the existing +batch-helper convention in `task/service/service_access.go` — callers derive the +IDs from an already-authorized task list. + +Both directions ship in the payload because the dependency chip shows both, and +the reverse direction is one indexed query rather than a client-side scan the +board's partial task list could not answer correctly anyway. + +### MCP surface + +Declaring `blocked_by` on the `create_task_kandev` schema is the smallest fix +with the largest effect: the plumbing already exists and only the contract is +missing. Two new tools, `add_task_dependency_kandev` and +`remove_task_dependency_kandev`, mirror the HTTP routes one-to-one and follow the +repo's existing preference for explicit small tools +(`add_branch_to_task_kandev`, the three `*_task_plan_kandev` tools) over a +mode-flag mutator. `list_related_tasks_kandev` is already the read side. + +The load-bearing semantic decision: `blocked_by` non-empty plus +`start_agent: true` records a start-when-unblocked intent instead of launching. +`start_agent` defaults to true and its description tells agents to leave it true, +so the alternative silently launches the whole chain at once — exactly the +collision the feature exists to prevent. The create response reports +`started: false` with `start_when_unblocked: true` so the caller is not guessing. + +### The gate + +Add the dependency check to `autoStartTaskForStep` next to the existing +`QueuedForStepID` check, before `launchDeferredTask`. Because that function is +the only automated-launch entry point, one check covers `task.moved`, +`task.queue_promoted`, watcher auto-start, and dependency resolution itself. + +The gate fails **closed**: if the predecessor lookup errors, skip the launch and +log a warning. Failing open would launch work whose predecessor may not have +run, which is the one outcome this feature exists to prevent. + +Manual start (`StartTask` from a user action) does not consult the gate. + +### Resolution and auto-start + +Subscribe to the task-state and task-move events that can make a predecessor +resolved. On each, look up dependents with `ListTasksBlockedBy`, and for each +dependent re-evaluate all of its predecessors: + +- any still pending → nothing; +- all resolved → publish `task.dependencies_resolved` and call + `autoStartTaskForStep` with the dependent's current step; +- at least one failed and none pending → publish `task.dependency_failed` + (Task 04). + +`start_when_unblocked` on task creation persists the resolved launch inputs into +`metadata.deferred_launch` — the same record WIP overflow already writes and +`launchDeferredTask` already consumes idempotently. No new table, and the +"exactly one launch" property is inherited rather than re-derived. Removing the +last edge unblocks the task but must not consume the intent. + +Startup reconciliation: after the queue reconciler runs, sweep non-archived +tasks that carry a `deferred_launch` intent and have edges, and launch those +whose dependencies are all resolved. This is the recovery path for a restart +between a predecessor's completion and the dependent's launch. Bound and batch +it so a large graph does not delay readiness. + +### Not touched + +`cascadeBlockersResolved`, Office assignment, `TriggerOnBlockerResolved` wiring, +and the Office task pane keep their current behavior. For a task eligible under +both reactions, the existing atomic deferred-launch/auto-start claim admits one +launcher. + +## Frontend + +- Extend the Kanban task type, boot mapper, and WS handlers with `blocked`, + `blockedReason`, `dependsOn`, `blocks`, and `startWhenUnblocked`. +- Treat an omitted projection as **unknown**, never as "no edges", at every layer + (mapper, WS merge, hydration backfill). The chip re-reads the task when its + cached copy has no projection, so no WS/hydration ordering can blank it. +- Blocked badge on the shared card next to the existing queued badge. Both must + fit without truncating either, on desktop and mobile, with no hover-only + disclosure. +- Dependency chip in the status row above the composer, next to `PRStatusChip`, + in **both** `chat-input-area.tsx` and `passthrough-toolbar.tsx`. It shows both + directions (blocked by / blocks), opens to a navigable list with each entry's + state, distinguishes failed predecessors, returns null with no edges, and uses + the drawer-on-mobile pattern `PRStatusChip` already establishes. + `shouldRenderChatStatusBar` must learn about the chip, or a task whose only + status content is the chip renders no row. +- No "Depends on" picker in the Kanban task detail view. Dependencies are + declared in the create dialog or over MCP (see the spec's What); the detail + view's dependency surfaces — card badge, chip, list — stay read-only. +- "Depends on" field plus a "Start automatically when unblocked" checkbox in the + task-create dialog, and an "Add dependency" entry in the Kanban card context + menu (`kanban-card-link-submenu.tsx` is the precedent). +- No whole-graph board view. One was built and removed before shipping (see the + spec's Out of scope): a `VIEW_REGISTRY` entry alone is not reachable anyway — + the header toggle speaks the separate `TaskListingView` vocabulary — and + without drawn connectors the result read as columns of text. +- All new copy goes through `t()` / ``, and any newly touched path is + appended to `i18nGuardFiles` in the same change. + +## Test strategy + +### Backend + +- Edge creation: cycle at length 2, 3, and N; self-edge; cross-workspace; + duplicate edge is idempotent; validator is shared by the task-scoped and + Office routes. +- Derived state: pending / resolved-by-state / resolved-by-terminal-step / + failed / cancelled / archived predecessors produce the right `blocked` and + `blocked_reason`; batch helper matches the per-task result. +- Gate: a blocked task is not launched by `task.moved`, by + `task.queue_promoted`, or by dependency resolution; a predecessor-lookup + error skips the launch; manual start is unaffected. +- Resolution: last-predecessor-resolves launches exactly once; a non-last + resolution launches nothing; no-intent unblocks without launching; + last-edge-removed unblocks without launching; a queued dependent launches on + WIP promotion, not on resolution. +- Chain: A → B → C with intents launches each exactly once, in order, and never + restarts A. +- Races: dependency resolution and WIP promotion both firing produce one + session; a claim loser logs and stops; a failed launch restores the claim. +- Restart: a restart between completion and launch yields exactly one session + after startup reconciliation. +- Failure: `FAILED` / `CANCELLED` predecessor holds the dependent blocked with + `reason: failed`, publishes the event once per transition, and raises one + notification; retry-then-success unblocks and fires the intent once. +- Deletion: deleting a predecessor removes edges both ways, unblocks the + dependent, and does not fire its intent. +- Kanban-only wiring: with `Features.Office` false, `blocked_by` on create + succeeds and `list_related_tasks_kandev` populates `blockers` / + `blocked_by`. +- MCP: a pinning test asserts `create_task_kandev`'s registered schema declares + `blocked_by` and `start_when_unblocked` — the current bug is precisely a + handler/schema mismatch that no behavior test would have caught. + `blocked_by` + default `start_agent` starts nothing and reports + `start_when_unblocked: true`; three chained create calls run in order; the two + dependency tools cover default `task_id`, cycle rejection, absent-edge removal, + and cross-workspace denial. +- PostgreSQL: add the environment-gated behavior test for every changed + dialect-sensitive repository method, per `apps/backend/CLAUDE.md`. + +### Frontend unit + +- DTO/store conversion preserves the five new fields across boot and WS. +- Blocked badge renders count and reason; coexists with the queued badge. +- `shouldRenderChatStatusBar` returns true when the dependency chip is the row's + only content. +- The chip lists both directions with states, distinguishes a failed + predecessor, renders null with no edges, and appears in both status rows. +- Picker creates and removes edges optimistically and replaces the optimistic + state with the formatted cycle path on rejection. +- Create dialog submits `blocked_by` and `start_when_unblocked`. +- Dependencies view builds nodes/edges from a graph payload, marks blocked and + failed nodes, and is absent from the mobile view set. + +### Browser E2E + +- Desktop chain: seed A → B → C with intents, complete A, assert B starts and C + does not; complete B, assert C starts. +- Desktop gate: a blocked task moved into an auto-start step gains no session. +- Desktop failure: fail A, assert B stays blocked with the failed reason and + gains no session. +- Desktop view: create and delete an edge from the Dependencies view. +- Mobile: blocked badge readable in a focused column, no horizontal overflow, + usable tap targets, Dependencies view not offered. + +Seed tasks without an agent and locate by task id — `createTaskWithAgent` on a +start step moves the card mid-test. + +## Public documentation + +Update `docs/public/tasks-and-workflows.md` (and `workflow-tips.md` if the +auto-start interaction needs a worked example) to cover: declaring a dependency, +what counts as resolution, that a failed predecessor halts the chain and needs +human action, that dependencies gate automated starts but not manual ones, and +that dependency resolution does not bypass WIP limits. + +## Implementation waves + +Tasks 01–04 are strictly sequential: each consumes a contract the previous one +introduces. Tasks 05 and 06 need only Task 01's API surface, so either may start +once 01 lands. Task 08 closes out. Task 07 was built on 06 and then dropped +before shipping, so it is not part of the executable set and Task 08 does not +wait on it. + +- [x] [Task 01: Promote dependency edges to a core relationship](task-01-core-dependency-relationship.md) +- [x] [Task 02: Gate automated launches on unresolved dependencies](task-02-gate-automated-launch.md) +- [x] [Task 03: Resolve dependencies and auto-start on unblock](task-03-resolve-and-auto-start.md) +- [x] [Task 04: Halt chains on a failed predecessor](task-04-failed-predecessor-halt.md) +- [x] [Task 05: MCP dependency tools](task-05-mcp-dependency-tools.md) +- [x] [Task 06: Kanban dependency UI](task-06-kanban-dependency-ui.md) +- [x] [Task 08: Verify browser flows and documentation](task-08-verify-flows-and-docs.md) + +No task is marked `parallel-safe`; waves do not authorize subagent execution. + +## Risks + +- **Two validators.** `office/dashboard` has the strong BFS cycle check; + `task/service` has a weaker one. Adding a third for the new routes would let + a cycle in through whichever path is weakest. One validator, or the DAG + invariant is not real. +- **Failing open on lookup error.** A blocked-state read that errors and returns + "not blocked" launches exactly the work this feature prevents. The gate must + fail closed and the test must prove it by breaking the store. +- **A denormalized `is_blocked` column.** Tempting for board queries, but a + stale value gates launches wrongly. Batch the derived read instead. +- **Two launch paths.** Dependency resolution and WIP promotion can both make + the same task launchable within milliseconds. Only the shared atomic claim + keeps that to one session; a new bespoke launch call site would reintroduce + the double-agent bug that `wip-limit-pull-system` documents. +- **Divergent "terminal" definitions.** `on_children_completed` treats `FAILED` + as terminal; dependency resolution must not. Sharing a helper between them + would silently make chains proceed on failure. +- **Office double-reaction.** With Office enabled, `cascadeBlockersResolved` and + the new Kanban reaction can both fire for one task. Verify one session, and do + not "fix" it by disabling either reaction. +- **Board load cost.** A per-card dependency query multiplies board queries by + card count. The batch helper is load-bearing, not an optimization. +- **Graph view scope creep.** The Dependencies view is the largest single piece + of UI. It must not become a general graph editor; edges and nodes only. +- **`start_agent: true` launching a blocked task.** Agents pass it by default. + If create does not convert it to a start-when-unblocked intent, every chain an + agent builds launches at once — the exact failure the feature prevents, and it + would look like the gate is broken rather than the create semantics. +- **A schema/handler mismatch that no behavior test catches.** `blocked_by` + already works end to end in the handler and is still unusable because the tool + schema omits it. The pinning test on the registered schema is what stops the + same class of gap recurring for `start_when_unblocked`. +- **Two status rows.** `chat-input-area.tsx` and `passthrough-toolbar.tsx` both + mount `PRStatusChip`. Adding the dependency chip to one leaves it missing in + the other layout, and the omission is invisible in the default view. +- **`shouldRenderChatStatusBar`.** A chip added without extending that predicate + is dead in exactly the case it matters most: a blocked task with no PR, no + todos, and no queued prompts. diff --git a/docs/plans/task-dependencies/task-01-core-dependency-relationship.md b/docs/plans/task-dependencies/task-01-core-dependency-relationship.md new file mode 100644 index 0000000000..b5598ef785 --- /dev/null +++ b/docs/plans/task-dependencies/task-01-core-dependency-relationship.md @@ -0,0 +1,126 @@ +--- +id: "01-core-dependency-relationship" +title: "Promote dependency edges to a core relationship" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/task-dependencies/spec.md" +--- + +# Task 01: Promote Dependency Edges to a Core Relationship + +## Acceptance + +- The dependency edge store is reachable in a Kanban-only install: + `SetBlockerRepository` is wired before the `Features.Office` early return in + `internal/backendapp/main.go`, so `create_task` with `blocked_by` succeeds and + `list_related_tasks_kandev` populates `blockers` / `blocked_by` with + `Features.Office` false. +- Exactly one edge validator exists. Self-edge, cross-workspace, and BFS cycle + detection (with the existing walk limit and `*BlockerCycleError{Path}`) live in + the task domain, and both the new task-scoped routes and the existing Office + routes call it. `checkCircularBlocker` in `service_office.go` is deleted, not + left as a second path. +- A `(blocker_task_id)` index on `task_blockers` is added through an idempotent + migration in `internal/task/repository/sqlite/base_migrations.go`. +- **Deleting** a task removes its edges in both directions, on SQLite and on + PostgreSQL, via explicit repository work rather than an `ON DELETE CASCADE` + the table does not have. **Archiving** does not: an archived predecessor still + blocks its dependents and reads as `pending` (Task 04), so its edges survive + and reappear if the task is unarchived. +- One batch service helper resolves, for a list of task IDs, each task's direct + predecessors **and direct dependents**, and a per-predecessor verdict of + `resolved` / `failed` / `pending`, using `state = COMPLETED` or + `IsTerminalStepName` for resolved and `FAILED` / `CANCELLED` for failed. + Archived predecessors are `pending`. The reverse direction uses the + `(blocker_task_id)` index added above; it is one batched query, not one per + task. +- Task DTOs over HTTP, WebSocket boot, WebSocket events, and MCP carry + `blocked`, `blocked_reason`, `depends_on`, `blocks`, and + `start_when_unblocked`. No `is_blocked` column is persisted. +- `depends_on` and `blocks` entries carry id, title, and state so the dependency + chip renders without a per-entry fetch. Neither list is transitive. +- New task-scoped routes exist and behave as specified: + `POST /api/v1/tasks/:id/dependencies`, + `DELETE /api/v1/tasks/:id/dependencies/:depId`. `POST` returns `409` with a + `cycle` array on a cycle and `400` on a self-edge or cross-workspace edge; + `DELETE` of an absent edge succeeds. Both return the mutated task's dependency + projection so a caller does not re-fetch. There is no graph-wide listing + route: every reader gets its edges from the dependency fields already on the + task payload. +- `task.updated` with `fields: ["dependencies"]` is published on edge add and + remove, through the task event publisher (not a bare repository write). +- `POST /api/v1/tasks` and `create_task_kandev` accept `start_when_unblocked`; + when true **and the request declares at least one dependency**, the resolved + launch inputs are persisted as `metadata.deferred_launch` in the same atomic + boundary as the task row and no session, workspace, or executor is prepared. + `start_when_unblocked: true` with no dependencies records no intent: the task + is already unblocked, so an agent-start request on it launches immediately + exactly as it would without the flag, and a create with neither launches + nothing. The flag defers a start; it never invents one. +- Edge routes authorize both tasks with the existing `authorize*` helpers, and + the payload field names stay `task_id` / `session_id` compatible with the + gateway dispatch backstop. + +## TDD sequence + +1. Failing test: with `Features.Office` false, `CreateTask` with `blocked_by` + succeeds and `list_related_tasks_kandev` returns the edge. +2. Failing tests for the shared validator: cycles of length 2, 3, and N return + `*BlockerCycleError` with the full path; self-edge and cross-workspace return + the typed errors; a duplicate edge is idempotent; both the task-scoped and + Office entry points hit the same validator. +3. Failing tests for the batch derived-state helper across pending, + resolved-by-state, resolved-by-terminal-step, failed, cancelled, and archived + predecessors, asserting the batch result equals the per-task result, and that + the dependents (`blocks`) direction is returned for the same call. +4. Failing DTO/event tests for the five new fields on HTTP, boot, WS, and MCP, + including the id/title/state shape of `depends_on` and `blocks` entries. +5. Failing handler tests for the three routes including the `409` `cycle` body + shape and the authorization denials. +6. Failing tests for edge cleanup on task delete, for edge *survival* across + archive, and for zero-dependency `start_when_unblocked`, plus the + environment-gated PostgreSQL equivalents. +7. Implement: wiring move, migration/index, validator consolidation, batch + helper, DTO fields, routes, event publication, `start_when_unblocked` + persistence. + +## Verification + +```bash +cd apps/backend +go test -tags fts5 ./internal/task/... ./internal/office/dashboard ./internal/mcp/... ./internal/backendapp -run 'Test.*(Dependenc|Blocker|BlockedBy|StartWhenUnblocked)' -count=1 +golangci-lint run ./... --new-from-rev=origin/main --timeout=5m +``` + +## Files likely touched + +- `apps/backend/internal/backendapp/main.go` +- `apps/backend/internal/task/repository/sqlite/base_migrations.go` +- `apps/backend/internal/task/repository/sqlite/task.go` +- `apps/backend/internal/task/service/service_office.go` +- `apps/backend/internal/task/service/service_tasks.go` +- `apps/backend/internal/task/service/service_requests.go` +- `apps/backend/internal/task/service/handoff_service.go` +- `apps/backend/internal/task/dto/dto.go`, `requests.go` +- `apps/backend/internal/task/handlers/task_http_handlers.go` +- `apps/backend/internal/office/dashboard/service_tasks.go` +- `apps/backend/internal/mcp/handlers/handlers.go` +- `apps/backend/pkg/api/v1/task.go` +- focused service, repository, handler, DTO, and MCP tests + +## Dependencies + +None. + +## Parallelism + +`sequential` + +## Output contract + +Mark this task `in_progress` before the RED tests and `done` only after the +listed commands pass. Record where the single validator now lives, the exact +migration/index statements, the batch helper's query shape, and test results in +this file and `plan.md`. diff --git a/docs/plans/task-dependencies/task-02-gate-automated-launch.md b/docs/plans/task-dependencies/task-02-gate-automated-launch.md new file mode 100644 index 0000000000..0d30f674f2 --- /dev/null +++ b/docs/plans/task-dependencies/task-02-gate-automated-launch.md @@ -0,0 +1,84 @@ +--- +id: "02-gate-automated-launch" +title: "Gate automated launches on unresolved dependencies" +status: done +wave: 2 +depends_on: ["01-core-dependency-relationship"] +plan: "plan.md" +spec: "../../specs/task-dependencies/spec.md" +--- + +# Task 02: Gate Automated Launches on Unresolved Dependencies + +## Acceptance + +- `orchestrator.autoStartTaskForStep` skips the launch when the task has any + unresolved dependency. The check sits next to the existing + `task.QueuedForStepID != ""` early return and **before** + `launchDeferredTask`, so a blocked task neither starts a session nor consumes + its deferred launch intent. +- One check covers every automated entry point. Verified for `task.moved` + (`handleTaskMovedNoSession`), `task.queue_promoted` + (`handleTaskQueuePromoted`), and the watcher auto-start path. No second gate + is added at any call site. +- The gate fails **closed**: if the dependency lookup returns an error, the + launch is skipped and a warning is logged. A test proves this by making the + store return an error, not by asserting the happy path. +- A skipped launch leaves the task otherwise untouched — no state change, no + claim taken, no claim restored, no queue metadata mutation — so a later + resolution or promotion can launch it. +- Manual start (`StartTask` reached from a user action, HTTP, or WS) is not + gated. A user can start a blocked task. +- The skip is logged at debug/info with the task id, the step id, and the number + of unresolved predecessors, following the existing `eventName+": ..."` log + convention in the file. +- No new goroutine, event subscription, or launch path is introduced by this + task. + +## TDD sequence + +1. Failing test: a task with one pending predecessor and an auto-start step + gains no session on `task.moved`. +2. Failing test: the same task gains no session on `task.queue_promoted`. +3. Failing test: a task with a `deferred_launch` intent and a pending + predecessor keeps its intent — the claim is not taken. +4. Failing test: with the dependency store returning an error, no session is + created and a warning is logged. +5. Failing test: manual `StartTask` on a blocked task does create a session. +6. Failing test: a task whose predecessors are all resolved is launched exactly + as it is today (no regression on the ungated path). +7. Implement the single gate and the log line. + +## Verification + +```bash +cd apps/backend +go test -tags fts5 ./internal/orchestrator -run 'Test.*(AutoStart|Dependenc|Blocked)' -count=1 +go test -tags fts5 ./internal/orchestrator -count=1 +golangci-lint run ./... --new-from-rev=origin/main --timeout=5m +``` + +## Files likely touched + +- `apps/backend/internal/orchestrator/event_handlers_workflow.go` +- `apps/backend/internal/orchestrator/service.go` (dependency-reader seam) +- `apps/backend/internal/orchestrator/event_handlers_github.go` (watcher path + assertion only, if a change proves necessary) +- focused orchestrator tests + +## Dependencies + +Task 01 — needs the batch derived-state helper and the wired blocker +repository. + +## Parallelism + +`sequential` + +## Output contract + +Mark this task `in_progress` before the RED tests and `done` only after the +listed commands pass. Record the exact placement of the gate relative to the +`QueuedForStepID` check and `launchDeferredTask`, the seam used to read +dependency state from the orchestrator, and the fail-closed test's mechanism in +this file and `plan.md`. diff --git a/docs/plans/task-dependencies/task-03-resolve-and-auto-start.md b/docs/plans/task-dependencies/task-03-resolve-and-auto-start.md new file mode 100644 index 0000000000..7accd43dce --- /dev/null +++ b/docs/plans/task-dependencies/task-03-resolve-and-auto-start.md @@ -0,0 +1,104 @@ +--- +id: "03-resolve-and-auto-start" +title: "Resolve dependencies and auto-start on unblock" +status: done +wave: 3 +depends_on: ["02-gate-automated-launch"] +plan: "plan.md" +spec: "../../specs/task-dependencies/spec.md" +--- + +# Task 03: Resolve Dependencies and Auto-Start on Unblock + +## Acceptance + +- A subscriber reacts to the task-state changes and workflow-step moves that can + make a predecessor resolved, looks up dependents with `ListTasksBlockedBy`, + and re-evaluates all predecessors of each dependent. +- When the last unresolved predecessor of a task resolves successfully, + `task.dependencies_resolved` is published with + `{task_id, resolved_by_task_id}` and the dependent is handed to + `autoStartTaskForStep`. No bespoke launch call is added. +- A dependent with no `deferred_launch` intent unblocks and starts nothing. +- A dependent that is not WIP-admitted starts nothing on resolution; its intent + survives and the existing WIP promotion path launches it once, later. +- A dependent whose step has no `on_enter: auto_start_agent` and no intent + unblocks and starts nothing. +- Removing the last blocking edge unblocks the task and does **not** consume its + intent or launch it. +- A chain A → B → C, each with an intent, launches each task exactly once in + order and never restarts an earlier task. +- Exactly one session is created when dependency resolution and WIP promotion + race, guaranteed by the existing atomic deferred-launch/auto-start claim. A + claim loser logs and stops; a failed launch restores the claim so the task + stays retryable and visibly unstarted. +- Startup reconciliation, running after the existing WIP queue reconciler, + sweeps non-archived tasks that hold a `deferred_launch` intent and have edges, + and launches those whose predecessors are all resolved. It is bounded and + batched so a large graph does not delay readiness, and it is idempotent across + repeated runs. +- A restart between a predecessor's completion and the dependent's launch yields + exactly one session after startup. +- `cascadeBlockersResolved`, Office assignment, and `TriggerOnBlockerResolved` + wiring are unchanged. With `Features.Office` true and a task eligible under + both reactions, exactly one session is created. +- Any goroutine this task starts has a single owner with idempotent + `Start`/`Stop`, cancels on `ctx.Done()`, and is covered by the package's + `goleak` `TestMain`. + +## TDD sequence + +1. Failing test: last-predecessor-resolves publishes + `task.dependencies_resolved` once and launches the dependent exactly once + with the intent's recorded agent profile, executor, executor profile, prompt, + and plan-mode choice. +2. Failing test: a non-last resolution publishes nothing and launches nothing. +3. Failing test: resolution via a move into a final `Done` step counts, not just + `state = COMPLETED`. +4. Failing test: a dependent without an intent unblocks and starts nothing. +5. Failing test: a queued (not WIP-admitted) dependent starts nothing on + resolution and starts exactly once on later promotion. +6. Failing test: removing the last edge unblocks without launching. +7. Failing test: A → B → C chain launches each once, in order. +8. Failing test: simultaneous resolution and promotion produce one session; the + loser logs; a launch failure restores the claim. +9. Failing test: restart between completion and launch yields one session after + startup reconciliation; a second reconciliation run launches nothing more. +10. Failing test: with Office enabled, a task eligible under both reactions gets + one session. +11. Implement the subscriber, the event, and the startup reconciliation. + +## Verification + +```bash +cd apps/backend +go test -tags fts5 ./internal/orchestrator ./internal/task/service ./internal/office/scheduler -run 'Test.*(DependenciesResolved|DependencyChain|AutoStartUnblock|StartupReconcil)' -count=1 +go test -tags fts5 -race ./internal/orchestrator ./internal/task/service -count=1 +golangci-lint run ./... --new-from-rev=origin/main --timeout=5m +``` + +## Files likely touched + +- `apps/backend/internal/orchestrator/event_handlers_workflow.go` +- `apps/backend/internal/orchestrator/event_handlers_task.go` or the equivalent + state-change subscriber +- `apps/backend/internal/task/events/` (new event type) +- `apps/backend/internal/task/service/service_events.go` +- `apps/backend/internal/backendapp/main.go` (startup reconciliation ordering) +- focused orchestrator and task-service tests + +## Dependencies + +Task 02 — the gate must exist first, otherwise resolution-driven launches would +bypass it and the WIP interaction could not be asserted. + +## Parallelism + +`sequential` + +## Output contract + +Mark this task `in_progress` before the RED tests and `done` only after the +listed commands pass. Record which events the subscriber listens to, where +startup reconciliation runs relative to the WIP queue reconciler, its batch +bound, and the race-test mechanism in this file and `plan.md`. diff --git a/docs/plans/task-dependencies/task-04-failed-predecessor-halt.md b/docs/plans/task-dependencies/task-04-failed-predecessor-halt.md new file mode 100644 index 0000000000..04be5d4b65 --- /dev/null +++ b/docs/plans/task-dependencies/task-04-failed-predecessor-halt.md @@ -0,0 +1,87 @@ +--- +id: "04-failed-predecessor-halt" +title: "Halt chains on a failed predecessor" +status: done +wave: 4 +depends_on: ["03-resolve-and-auto-start"] +plan: "plan.md" +spec: "../../specs/task-dependencies/spec.md" +--- + +# Task 04: Halt Chains on a Failed Predecessor + +## Acceptance + +- A predecessor entering `FAILED` or `CANCELLED` never resolves its edge. The + dependent stays blocked and its `blocked_reason` becomes `"failed"` once no + predecessor is still pending. +- `task.dependency_failed` is published with + `{task_id, failed_task_id, failed_state}`, once per + (dependent, predecessor) failure transition. A repeated write of the same + failed state does not re-publish. +- A notification event type `task.dependency_failed` is added following the + existing constants in `internal/notifications/service`, and fires once per + transition. +- No session is created for the dependent, and its `deferred_launch` intent is + neither consumed nor deleted — it remains available for a retry. +- Retrying the failed predecessor to successful completion unblocks the + dependent and fires its intent exactly once. +- Removing the edge unblocks the dependent without firing its intent. +- An archived predecessor reads as `pending`, not `failed` and not `resolved`; + the dependent stays blocked and the predecessor is listed as pending. +- Deleting a predecessor removes the edges in both directions, may unblock the + dependent, and does **not** fire its intent — deletion is not success. +- Kandev never auto-retries a failed predecessor and never auto-removes an edge. +- A dependent that already launched is not stopped or rolled back when a + predecessor is later reopened; it reports blocked again and its running + session is untouched. + +## TDD sequence + +1. Failing test: `FAILED` predecessor leaves the dependent blocked with + `blocked_reason: "failed"`, no session, intent intact. +2. Failing test: `CANCELLED` predecessor behaves identically. +3. Failing test: `task.dependency_failed` publishes once; a duplicate terminal + write does not re-publish. +4. Failing test: one notification is raised per transition. +5. Failing test: retry-to-success unblocks and launches exactly once. +6. Failing test: edge removal unblocks without launching. +7. Failing test: archived predecessor reads as pending. +8. Failing test: predecessor deletion removes both edge directions, unblocks, + and launches nothing. +9. Failing test: reopening a predecessor after the dependent launched leaves the + running session alive and reports blocked again. +10. Implement the failure branch, the event, and the notification type. + +## Verification + +```bash +cd apps/backend +go test -tags fts5 ./internal/orchestrator ./internal/task/service ./internal/notifications/... -run 'Test.*(DependencyFailed|PredecessorFailed|DependencyArchived|DependencyDeleted)' -count=1 +golangci-lint run ./... --new-from-rev=origin/main --timeout=5m +``` + +## Files likely touched + +- `apps/backend/internal/orchestrator/event_handlers_workflow.go` or the + state-change subscriber added in Task 03 +- `apps/backend/internal/task/events/` +- `apps/backend/internal/task/service/service_events.go` +- `apps/backend/internal/task/service/service_tasks.go` (delete/archive edge + cleanup interaction) +- `apps/backend/internal/notifications/service/service.go` +- focused orchestrator, task-service, and notification tests + +## Dependencies + +Task 03 — the resolution subscriber is where the failure branch lives. + +## Parallelism + +`sequential` + +## Output contract + +Mark this task `in_progress` before the RED tests and `done` only after the +listed commands pass. Record the once-per-transition mechanism, the notification +constant, and the delete-versus-archive difference in this file and `plan.md`. diff --git a/docs/plans/task-dependencies/task-05-mcp-dependency-tools.md b/docs/plans/task-dependencies/task-05-mcp-dependency-tools.md new file mode 100644 index 0000000000..621f10479f --- /dev/null +++ b/docs/plans/task-dependencies/task-05-mcp-dependency-tools.md @@ -0,0 +1,106 @@ +--- +id: "05-mcp-dependency-tools" +title: "MCP dependency tools" +status: done +wave: 5 +depends_on: ["01-core-dependency-relationship"] +plan: "plan.md" +spec: "../../specs/task-dependencies/spec.md" +--- + +# Task 05: MCP Dependency Tools + +## Acceptance + +- `create_task_kandev` **declares** `blocked_by` (array of task IDs) and + `start_when_unblocked` (boolean) in its `mcp.NewTool` schema in + `internal/mcp/server/server.go`. Today the handler struct at + `internal/mcp/handlers/handlers.go:576` reads `blocked_by` but the schema never + declares it, so no agent can discover or reliably pass it. A pinning test + asserts both parameters are present in the registered tool schema. +- `blocked_by` non-empty plus `start_agent: true` (including the default) records + a start-when-unblocked intent and launches nothing now. The create response + reports `started: false` and `start_when_unblocked: true`. +- `blocked_by` non-empty plus explicit `start_when_unblocked: false` creates the + edges with no launch intent; completing the predecessor starts nothing. +- `add_task_dependency_kandev` is registered: `{task_id?, depends_on_task_id}`, + `task_id` defaults to the calling task, returns the resulting `depends_on` + list. It rejects cycles with the cycle path, self-edges, and cross-workspace + edges, using the single validator from Task 01. +- `remove_task_dependency_kandev` is registered with the same shape. Removing an + absent edge succeeds. Removing the last edge unblocks the task without + launching it. +- Both new tools are registered in the same modes as the other task-mutation + tools, and neither widens the `ModeExternal` surface beyond what + `create_task_kandev` already exposes there. +- An agent cannot set `start_when_unblocked` on a task it did not create. +- Tool descriptions state: a subtask means "part of" and a dependency means "not + until"; decomposing a plan into ordered phases is N sibling tasks chained with + `blocked_by` + `start_when_unblocked`, not N subtasks started at once; and a + failed predecessor halts the chain and needs human action. +- `list_related_tasks_kandev` populates `blockers` and `blocked_by` with + `Features.Office` false (inherited from Task 01; asserted here at the MCP + boundary). +- Both tools authorize through the existing MCP scoping path — the owning task + comes from the `AgentExecution`, never from an agent-supplied `task_id` used to + reach another workspace's tasks. +- The MCP tool list in the root `CLAUDE.md` / `AGENTS.md` "Kandev Task Creation" + guidance is updated if the new tools change how an agent should decompose work. + +## TDD sequence + +1. Failing pinning test: the registered `create_task_kandev` schema declares + `blocked_by` and `start_when_unblocked`. +2. Failing test: `blocked_by` + default `start_agent` creates the edge, starts no + session, and returns `started: false`, `start_when_unblocked: true`. +3. Failing test: `blocked_by` + `start_when_unblocked: false` records no intent + and completing the predecessor starts nothing. +4. Failing test: three chained `create_task_kandev` calls run in order — + completing the first starts the second and not the third. +5. Failing tests for `add_task_dependency_kandev`: default `task_id`, returned + `depends_on`, cycle rejection with path, self-edge, cross-workspace. +6. Failing tests for `remove_task_dependency_kandev`: removal, absent-edge + success, last-edge removal unblocks without launching. +7. Failing test: an agent cannot set `start_when_unblocked` on a task it did not + create. +8. Failing test: cross-workspace `task_id` on either tool is denied by the + scoping path. +9. Failing test: with `Features.Office` false, `create_task_kandev` + + `blocked_by` succeeds and `list_related_tasks_kandev` reports the edge. +10. Implement schema declarations, the two tools, the start-intent semantics, and + the descriptions. + +## Verification + +```bash +cd apps/backend +go test -tags fts5 ./internal/mcp/... -run 'Test.*(Dependenc|BlockedBy|StartWhenUnblocked|ToolSchema)' -count=1 +go test -tags fts5 ./internal/mcp/... -count=1 +golangci-lint run ./... --new-from-rev=origin/main --timeout=5m +``` + +## Files likely touched + +- `apps/backend/internal/mcp/server/server.go` +- `apps/backend/internal/mcp/handlers/handlers.go` +- `apps/backend/internal/mcp/server/handoff_handlers.go` +- `apps/backend/internal/mcp/scope/` (if either tool needs a scope entry) +- `CLAUDE.md` / `AGENTS.md` (task-decomposition guidance) +- focused MCP handler and schema-pinning tests + +## Dependencies + +Task 01 — needs the wired blocker repository, the single validator, and the +`start_when_unblocked` create-request field. It does not need Tasks 02–04, so it +may run alongside Task 06. + +## Parallelism + +`sequential` + +## Output contract + +Mark this task `in_progress` before the RED tests and `done` only after the +listed commands pass. Record the two new tool names and their registered modes, +the exact `blocked_by` + `start_agent` resolution rule as implemented, the +schema-pinning test name, and test results in this file and `plan.md`. diff --git a/docs/plans/task-dependencies/task-06-kanban-dependency-ui.md b/docs/plans/task-dependencies/task-06-kanban-dependency-ui.md new file mode 100644 index 0000000000..76bd04ab84 --- /dev/null +++ b/docs/plans/task-dependencies/task-06-kanban-dependency-ui.md @@ -0,0 +1,133 @@ +--- +id: "06-kanban-dependency-ui" +title: "Kanban dependency UI" +status: done +wave: 6 +depends_on: ["01-core-dependency-relationship"] +plan: "plan.md" +spec: "../../specs/task-dependencies/spec.md" +--- + +# Task 06: Kanban Dependency UI + +## Acceptance + +- The Kanban task type, boot-payload mapper, and WS handlers carry `blocked`, + `blockedReason`, `dependsOn`, `blocks`, and `startWhenUnblocked`, surviving + both boot hydration and incremental `task.updated` events. + +### Dependency chip above the composer + +- A dependency chip renders in the chat status row directly above the composer, + alongside `PRStatusChip`, in `components/task/chat/chat-input-area.tsx` + (`data-testid="chat-status-bar"`). +- The same chip renders in `components/task/passthrough-toolbar.tsx` + (`data-testid="passthrough-status-row"`), which mounts its own copy of + `PRStatusChip`. Both rows must show it or the chip disappears in passthrough + layouts. +- `shouldRenderChatStatusBar(...)` is extended with the chip's presence. + Without that, a task whose only status content is the dependency chip renders + no row at all — the predicate currently accounts only for todos, a queue chip, + right-hand controls, and proceed. +- The chip reports both directions and opens to a list of blocked-by and blocks + entries with each entry's title and state, each navigable to that task. +- Failed predecessors are visually distinguished from pending ones in the list. +- The chip returns null when the task has no edges in either direction, matching + `PRStatusChip`'s own null-when-not-applicable contract. +- Mobile uses the drawer pattern, not a hover card, mirroring + `PRStatusChipDrawer` / `usesMobileDrawer` in + `components/github/pr-status-chip.tsx`. +- The chip reads `dependsOn` / `blocks` from the task payload; it does not fetch + each related task individually. +- A blocked badge renders on the shared task card next to the existing + `queuedForStepId` badge. It shows the predecessor count and distinguishes + `pending` from `failed`. Hovering or tapping it lists the predecessor titles + and their states. +- Both badges can appear on one card without truncating either, on desktop and + on a narrow mobile viewport, with no horizontal page overflow and no + hover-only disclosure of the blocked state itself. +- The Kanban task detail view mounts **no** edge editor. Dependencies are + declared in the create dialog or over MCP, so every detail-view surface that + reads them (card badge, chip, chip list) is read-only and no `BlockersPicker` + is repointed at the task-scoped routes. +- The task-create dialog gains a "Depends on" field and a "Start automatically + when unblocked" checkbox, submitting `blocked_by` and `start_when_unblocked`. + The checkbox is disabled when no dependency is selected. +- The Kanban card context menu gains an "Add dependency" entry, following the + `kanban-card-link-submenu.tsx` pattern. +- All new copy goes through `t()` / ``; no hardcoded literal, including + inside SCREAMING_CASE config tables, which are reviewed by eye. Every path + touched here is appended to `i18nGuardFiles` in `eslint.i18n.options.mjs` in + the same change. `t()` is never called at module scope. +- The pseudo-locale (Settings → General → Appearance) shows no English text in + any surface added by this task. +- Mobile parity: the picker and the create-dialog field are reachable and usable + on a narrow viewport using native mobile interaction patterns, not a + compressed desktop layout. + +## TDD sequence + +1. Failing tests: DTO/store conversion preserves the five fields from boot and + from a `task.updated` event with `fields: ["dependencies"]`. +2. Failing test: the blocked badge renders count and reason, and coexists with + the queued badge. +2a. Failing test: `shouldRenderChatStatusBar` returns true for a task whose only + status content is the dependency chip. +2b. Failing test: the chip lists blocked-by and blocks entries with states, + distinguishes a failed predecessor, and renders null with no edges. +2c. Failing test: the chip appears in both `chat-status-bar` and + `passthrough-status-row`. +3. Failing test: the picker creates and removes an edge optimistically. +4. Failing test: a `409` with a `cycle` body renders the formatted cycle path + and reverts the optimistic edge. +5. Failing test: the create dialog submits `blocked_by` and + `start_when_unblocked`, and the checkbox is disabled with no selection. +6. Failing test: the context-menu entry opens the picker for the right task. +7. Implement types, mappers, badge, picker mount, dialog fields, and menu entry. + +## Verification + +```bash +cd apps/web +pnpm test -- --run components/kanban-card-content components/task/simple/components/blockers-picker components/task-create-dialog components/task/chat/chat-input-area components/task/dependency-chip lib/ws/handlers/kanban +pnpm run typecheck +pnpm run lint +pnpm run i18n:check +pnpm run i18n:ratchet +``` + +Use `pnpm test` / `pnpm run typecheck`, not `pnpm exec` — `exec` skips the +generator pre-hooks and produces spurious generated-JSON failures. + +## Files likely touched + +- `apps/web/lib/state/slices/kanban/types.ts` +- `apps/web/lib/ws/handlers/kanban.ts` +- `apps/web/components/kanban-card.tsx`, `kanban-card-content.tsx` +- `apps/web/components/kanban-card-context-menu.tsx` +- `apps/web/components/task/simple/components/blockers-picker.tsx` +- `apps/web/components/task/dependency-chip.tsx` (new) +- `apps/web/components/task/chat/chat-input-area.tsx` +- `apps/web/components/task/passthrough-toolbar.tsx` +- `apps/web/app/tasks/[id]/kanban-task-shell.tsx` +- `apps/web/components/task-create-dialog*.tsx` +- `apps/web/lib/api/domains/` (task-scoped dependency client) +- `apps/web/eslint.i18n.options.mjs` +- `apps/web/public/locales/**` (or the repo's catalog location) +- focused `*.test.tsx` / `*.test.ts` files + +## Dependencies + +Task 01 — needs the task-scoped routes and the five DTO fields. It does not need +Tasks 02–05, so it may start as soon as 01 lands, alongside Task 05. + +## Parallelism + +`sequential` + +## Output contract + +Mark this task `in_progress` before the RED tests and `done` only after the +listed commands pass. Record the badge layout decision for the two-badge case, +the i18n paths appended to the guard list, the pseudo-locale check result, and +test results in this file and `plan.md`. diff --git a/docs/plans/task-dependencies/task-07-dependencies-board-view.md b/docs/plans/task-dependencies/task-07-dependencies-board-view.md new file mode 100644 index 0000000000..5d1b7457a6 --- /dev/null +++ b/docs/plans/task-dependencies/task-07-dependencies-board-view.md @@ -0,0 +1,101 @@ +--- +id: "07-dependencies-board-view" +title: "Dependencies board view" +status: dropped +wave: 7 +depends_on: ["06-kanban-dependency-ui"] +plan: "plan.md" +spec: "../../specs/task-dependencies/spec.md" +--- + +# Task 07: Dependencies Board View + +> **Dropped.** This view was implemented and then removed before shipping. Two +> reasons: it rendered edges as per-node text lists rather than drawn connectors, +> so a chain had to be read rather than followed; and it was scoped to one +> workflow, so it could not show a chain that crossed workflows. The per-task +> chip plus `list_related_tasks_kandev` cover reading the graph for now. If this +> is revived, draw real connectors with the existing +> `components/kanban/graph2-connector.tsx` primitives and add a focus-one-chain +> filter — and remember that a `VIEW_REGISTRY` entry is not reachable until the +> `TaskListingView` vocabulary and the header toggle learn about it. + +## Acceptance + +- A third `VIEW_REGISTRY` entry, "Dependencies", is registered in + `apps/web/lib/kanban/view-registry.ts` alongside `kanban` and `graph2`, with + its own stored value, label, and icon. +- The view renders the dependency DAG for the active workflow from + `GET /api/v1/workflows/:id/dependencies` plus the tasks already in the store: + one node per task, one directed connector per edge, drawn in dependency order. +- Blocked nodes are visually distinguished from unblocked ones, and + `blocked_reason: "failed"` is distinguished from `"pending"`. +- Edges that leave the active workflow but stay in the workspace are marked as + such rather than dropped, so a cross-workflow dependency is not invisible. +- Dragging from a node's outgoing handle to another node creates "target depends + on source". Deleting a connector removes the edge. Both go through the same + task-scoped endpoints and the same optimistic-mutation and cycle-error + handling as the picker. +- A rejected edge (cycle, cross-workspace) reverts the optimistic connector and + surfaces the formatted cycle path. +- A node click opens the task; the view does not become a second task editor. +- The view is desktop-only. `getEffectiveView` already forces the Kanban view on + mobile; a test pins that the Dependencies view is not in the mobile view set. +- All new copy goes through `t()` / ``, the new paths are appended to + `i18nGuardFiles` in the same change, and the pseudo-locale shows no English + in the view. +- No new charting or graph-layout dependency is added without first checking + whether the existing connector primitives in `components/kanban/graph2-*.tsx` + cover the need. + +## TDD sequence + +1. Failing test: the view builder turns an edge payload plus store tasks into + nodes and directed edges in dependency order. +2. Failing test: blocked, failed, and unblocked nodes get distinct markers. +3. Failing test: a cross-workflow edge is marked, not dropped. +4. Failing test: drag-to-connect calls the create endpoint with the correct + direction and applies the edge optimistically. +5. Failing test: a `409` with a `cycle` body reverts the connector and shows the + formatted path. +6. Failing test: connector delete calls the delete endpoint and removes the + edge. +7. Failing test: the Dependencies view is absent from the mobile view set. +8. Implement the registry entry, the view component, and the graph interactions. + +## Verification + +```bash +cd apps/web +pnpm test -- --run lib/kanban/view-registry components/kanban/dependencies +pnpm run typecheck +pnpm run lint +pnpm run i18n:check +pnpm run i18n:ratchet +``` + +## Files likely touched + +- `apps/web/lib/kanban/view-registry.ts` +- `apps/web/components/kanban/dependencies-*.tsx` (new) +- `apps/web/components/kanban/graph2-connector.tsx` (reuse, if extended) +- `apps/web/hooks/use-kanban-display-settings.ts`, + `apps/web/lib/task-listing/view-preference.ts` +- `apps/web/lib/api/domains/` (workflow dependency-graph client) +- `apps/web/eslint.i18n.options.mjs` +- focused `*.test.tsx` / `*.test.ts` files + +## Dependencies + +Task 06 — reuses its API client, mutation handling, and cycle-error formatting. + +## Parallelism + +`sequential` + +## Output contract + +Mark this task `in_progress` before the RED tests and `done` only after the +listed commands pass. Record whether existing connector primitives were reused +or a new layout approach was needed and why, the stored view value, the i18n +paths appended, and test results in this file and `plan.md`. diff --git a/docs/plans/task-dependencies/task-08-verify-flows-and-docs.md b/docs/plans/task-dependencies/task-08-verify-flows-and-docs.md new file mode 100644 index 0000000000..71be48581d --- /dev/null +++ b/docs/plans/task-dependencies/task-08-verify-flows-and-docs.md @@ -0,0 +1,151 @@ +--- +id: "08-verify-flows-and-docs" +title: "Verify browser flows and documentation" +status: done +wave: 8 +depends_on: ["04-failed-predecessor-halt", "06-kanban-dependency-ui"] +plan: "plan.md" +spec: "../../specs/task-dependencies/spec.md" +--- + +# Task 08: Verify Browser Flows and Documentation + +## Acceptance + +Browser E2E, covering the spec's scenarios end to end rather than in units: + +- **Desktop chain**: seed A → B → C with start-when-unblocked intents; complete + A and assert B starts and C does not; complete B and assert C starts and A is + not restarted. +- **Desktop gate**: a task with a pending predecessor moved into an auto-start + step gains no session. +- **Desktop WIP interaction**: a dependent queued for a full WIP step gains no + session when its dependency resolves, and starts exactly once after capacity + opens. +- **Desktop failure**: fail A and assert B stays blocked with the failed reason, + gains no session, and shows the notification; retry A to success and assert B + starts exactly once. +- **Desktop cycle**: attempting to close a cycle surfaces the cycle path and + creates no edge. +- **Desktop chip**: open task B where B depends on A and D depends on B; assert + the dependency chip appears in the status row above the composer, lists A under + blocked-by and D under blocks with their states, and navigates to A on click. + Assert the chip is absent on a task with no edges. +- **Desktop chip with no PR**: a blocked task with no PR, no todos, and no queued + prompts still renders the status row with the chip. +- **MCP chain**: drive `create_task_kandev` three times with `blocked_by` and the + default `start_agent`, assert no session starts on creation, then complete the + first and assert the second starts and the third does not. Then exercise + `add_task_dependency_kandev` / `remove_task_dependency_kandev` against a live + task and assert the board reflects both. +- **Mobile**: the blocked badge is readable in a focused column with no + horizontal page overflow and usable tap targets, and tapping the dependency + chip opens the drawer rather than a hover card. There is no Dependencies view + and no detail-view picker to reach (Task 07 dropped; see `plan.md`). + +Seed tasks **without** an agent and locate them by task id. `createTaskWithAgent` +on a start step moves the card mid-test and races column assertions. + +Documentation: + +- `docs/public/tasks-and-workflows.md` documents declaring a dependency, what + counts as resolution (successful completion or a final Done/Complete/ + Completed/Approved step), that a failed or cancelled predecessor halts the + chain and needs human action, that dependencies gate automated starts but not + manual ones, and that resolution does not bypass WIP limits. +- `docs/public/workflow-tips.md` gains a worked A → B → C example if the + auto-start interaction needs one. +- `docs/features.md` lists task dependencies in the live feature inventory. +- The spec's `status` moves to `shipped` and its Open questions section is + empty or deleted. +- `docs/specs/INDEX.md` and this plan's statuses are accurate. +- `apps/web/AGENTS.md` is untouched: Task 07's board view was dropped, so no + view-registry convention changed. + +## TDD sequence + +1. Write each E2E spec red against the implemented backend/frontend, one flow at + a time, and fix what they catch. +2. Run the mobile project explicitly; do not infer mobile behavior from the + desktop run. +3. Update public docs and the spec/plan statuses last, once behavior is settled. + +## Verification + +```bash +cd apps/web +pnpm e2e:raw --grep 'dependenc' +pnpm e2e:raw --project=mobile-chrome --grep 'dependenc' +cd ../backend && go test -tags fts5 ./... -count=1 +``` + +A full backend suite in a task worktree fails roughly four packages with +`parent directory cannot be accessed`; that is the sandbox, not this change. +Confirm those are the only failures and that they reproduce on a clean checkout +of `main`. + +## Files likely touched + +- `apps/web/e2e/tests/task/task-dependencies.spec.ts` (new) +- `apps/web/e2e/helpers/` (seeding helper for edges/intents) +- `docs/public/tasks-and-workflows.md` +- `docs/public/workflow-tips.md` +- `docs/features.md` +- `docs/specs/task-dependencies/spec.md` +- `docs/specs/INDEX.md` +- `docs/plans/task-dependencies/plan.md` + +## Dependencies + +Tasks 04 and 06: the last backend and frontend behaviors respectively. + +## Parallelism + +`sequential` + +## Recorded results + +`apps/web/e2e/tests/task/task-dependencies.spec.ts`, chromium, 5 passed: + +- a chain runs one step at a time and never restarts a predecessor +- a failed predecessor halts the chain until it succeeds +- removing the last edge unblocks without launching +- a cycle is refused with the offending path +- the board and the open task both show the dependency + +The first run failed 3 of the 5, and both causes were real product bugs that +every unit test had missed because neither crosses a single package: + +1. **WIP admission erased the launch intent.** The SQLite repository deleted + `metadata.deferred_launch` on every admitted create, because a WIP-overflow + record is stale once a task is admitted. The start-when-unblocked intent + shares that record, and a chain step is admitted at create time like any + other task, so the intent was gone before any predecessor could resolve it + and no chain ever ran. Admission now routes through + `models.DropWIPDeferredLaunch`, which keeps a chain intent and drops only an + overflow one (`internal/task/models/deferred_launch_test.go`, mutation- + verified). +2. **The status row was hidden on session-less tasks.** The dockview hosts pass + the panel's task only when it has a session, so `ChatStatusBar` saw a null + task id and rendered nothing, taking the dependency chip with it on exactly + the tasks the chip describes: a blocked step has no session by construction. + The hosts now pass a status-row-only `statusTaskId` + (`resolveStatusRowTaskId`), leaving the session-derived id, plan mode, and + read tracking untouched. + +Backend: `go test ./internal/task/... ./internal/orchestrator/... ./internal/mcp/...` +fails only the 12 known worktree-sandbox tests (`parent directory cannot be +accessed`, local-repository initialization and directory listing), the identical +set before and after this change. Changed-file lint against the PR base is +clean. Frontend: 1363 files / 11090 tests pass, lint clean, `i18n:check` and +`i18n:ratchet` clean. + +Not run: the mobile project. The mobile assertions in the acceptance list above +are still outstanding. + +## Output contract + +Mark this task `in_progress` before the first E2E run and `done` only after the +listed commands pass. Record each E2E spec name, the mobile run result, the +exact set of pre-existing backend failures with evidence they are environmental, +and the docs updated in this file and `plan.md`. diff --git a/docs/public/automation-and-mcp.md b/docs/public/automation-and-mcp.md index 287775274a..1f9064179d 100644 --- a/docs/public/automation-and-mcp.md +++ b/docs/public/automation-and-mcp.md @@ -187,6 +187,39 @@ Task tools use normal client discovery. When `step_complete_kandev` is required `create_task_kandev` advertises `prompt` for instructions delivered to a newly started agent. Older callers may still send `description` when `prompt` is absent, but sending both is an error; the compatibility name is intentionally omitted from the advertised schema. +### Task dependencies over MCP + +`create_task_kandev` accepts `blocked_by`, a list of task IDs the new task must +wait on. Two further tools manage links on an existing task: + +- `add_task_dependency_kandev` with `depends_on_task_id`, and an optional + `task_id`. +- `remove_task_dependency_kandev` with the same arguments. Removing a link that + is not there succeeds. + +In a task-bound session, `task_id` defaults to the agent's own task and may only +name that task; use `depends_on_task_id` for the other end. The link is +authorized against both tasks, and a link that would close a cycle is rejected +with the offending path. + +**Use a dependency for ordering, a subtask for decomposition.** Reaching for +`parent_id` to express "do this one first" gets neither the start gate nor the +chain: children run whenever they are started, and a parent's completion signal +fires when all children reach any terminal state, including failure. + +An agent decomposing a plan into ordered steps should create each step with +`blocked_by` pointing at the previous one and leave `start_agent` at its +default. A create that declares dependencies does not launch immediately: the +requested start is recorded and fires once every predecessor has completed +successfully, so three chained creates produce a chain rather than three agents +in the same repository at once. Pass `start_when_unblocked: false` to create the +links with no launch intent at all. With no `blocked_by`, `start_when_unblocked` +records nothing and the task starts or does not start purely per `start_agent`. + +Only success resolves a link. A predecessor that fails or is cancelled leaves +its dependents blocked with a reason naming it, and Kandev does not retry it or +drop the link on its own. + ### Autopilot tasks and MCP profiles Task creation accepts one optional boolean: diff --git a/docs/public/coverage.json b/docs/public/coverage.json index 93c26e598b..94ea6b3d7c 100644 --- a/docs/public/coverage.json +++ b/docs/public/coverage.json @@ -90,6 +90,10 @@ "apps/backend/internal/mcp/handlers/stop_task.go", "apps/backend/internal/mcp/server/handlers.go", "apps/backend/internal/mcp/server/server.go", + "apps/backend/internal/mcp/server/dependency_handlers.go", + "apps/backend/internal/mcp/handlers/dependency_handlers.go", + "apps/backend/internal/task/service/service_dependencies.go", + "apps/backend/internal/orchestrator/event_handlers_dependencies.go", "apps/backend/internal/gateway/websocket/client.go" ], "tests": [ @@ -102,16 +106,20 @@ "apps/backend/internal/mcp/server/sysprompt_sync_test.go", "apps/backend/internal/backendapp/mcp_stop_composition_test.go", "apps/backend/internal/integration/mcp_stop_task_test.go", - "apps/backend/internal/gateway/websocket/client_mcp_actions_test.go" + "apps/backend/internal/gateway/websocket/client_mcp_actions_test.go", + "apps/backend/internal/task/service/service_dependencies_test.go", + "apps/backend/internal/orchestrator/event_handlers_dependencies_test.go" ], "mcpTools": [ "add_branch_to_task_kandev", + "add_task_dependency_kandev", "add_workspace_sources_kandev", "archive_task_kandev", "create_task_kandev", "delete_task_kandev", "list_tasks_kandev", "move_task_kandev", + "remove_task_dependency_kandev", "set_task_title_kandev", "step_complete_kandev", "stop_task_kandev", diff --git a/docs/public/tasks-and-workflows.md b/docs/public/tasks-and-workflows.md index 71fa7609d4..dc04712720 100644 --- a/docs/public/tasks-and-workflows.md +++ b/docs/public/tasks-and-workflows.md @@ -228,6 +228,75 @@ The preference only gates opening a task. Choosing **Start agent** (or a workflow step transition) always starts the agent as usual, and a failed or interrupted session still shows its recovery actions. +## Task dependencies + +A task can declare that it **depends on** one or more other tasks. This is a +peer relationship and is separate from the parent/child subtask hierarchy: a +subtask says "B is part of A", a dependency says "B cannot start until A +finishes". The two can be combined freely, including a dependency between a +task and its own child. + +Dependencies form a graph, not just a line. A task can wait on several +predecessors and can itself block several dependents. A link that would close a +cycle is rejected when you try to create it, and the offending path is shown so +you can see which link to drop. + +### Declare dependencies + +Dependencies are declared in the **New Task** dialog under **Depends on**, or +by an agent over MCP. There is deliberately no editor in the open task: a +dependency records how the work was planned, so the surfaces that display it +stay read-only. To change one after the fact, use the MCP tools or delete and +recreate the task. + +### What blocked means + +A task with at least one unfinished predecessor is **blocked**. Blocked tasks +show a badge on their Kanban card and a dependency chip in the status row above +the chat box, next to the pull request chip. The chip reports both directions, +the tasks this one waits on and the tasks waiting on it, and each entry links +to that task. + +While a task is blocked, no automated path starts it. That covers workflow +**On Enter** auto-start, promotion out of a WIP queue, integration watchers, +and dependency resolution itself. You can still press **Start agent** +yourself; a manual start is an explicit override, not an error. + +### Chains that run themselves + +A task created with dependencies and an agent start request does not launch +immediately. It records the start as an intent, and Kandev launches it once +every predecessor has completed successfully. Setting that up along a path +produces a chain: + +1. Create task A normally. +2. Create task B with **Depends on** set to A. +3. Create task C with **Depends on** set to B. + +Starting A is the only manual step. When A completes, B starts. When B +completes, C starts. A is never restarted. + +Auto-start grants eligibility, never a bypass. If a task's dependencies have +resolved but the target step is at its WIP limit, the task stays queued and +launches when the queue promotes it, exactly as any other queued task would. + +### When a predecessor does not succeed + +Only successful completion resolves a dependency. A predecessor that ends in +**Failed** or **Cancelled** leaves its dependents blocked, and the blocked +reason names the failed task rather than reporting a generic wait. The chain +stops there and waits for you. Kandev never retries a failed predecessor on its +own and never quietly drops the link. + +Three things clear it, all of them deliberate: retry the predecessor until it +succeeds, remove the link over MCP, or start the dependent manually. + +An **archived** predecessor is treated as unfinished, not as failed and not as +resolved, so archiving a task does not release the work waiting on it. +**Deleting** a task does remove its links in both directions, and any dependent +that was waiting only on it becomes unblocked. That dependent is not started: +deletion is not success. + ## Find and organize tasks On desktop and tablet, the header switches between **Kanban**, **Pipeline**, and **List**. Kanban and Pipeline show the same workflow steps in different layouts. Kandev remembers the last selected view in that browser on the current device. Phones offer **Kanban** and **List** only; a saved desktop Pipeline preference is kept but shown as Kanban on the phone. @@ -409,9 +478,9 @@ Revision history is not an immutable record of every autosave. Consecutive write | One versioned task plan | Available | Available in Office-specific surfaces where enabled | | Multiple named task documents | Not exposed | In-progress Office capability | | Task label editor and label filters | Not exposed | In-progress Office capability | -| Blocked-by / blocking property editor | Not exposed | In-progress Office capability | +| Blocked-by / blocking property editor | Set at task creation or over MCP; read-only afterwards | In-progress Office capability | -Stored related-task data can include blocker relationships, but regular Kanban has no blocker editor or blocker filter. Use workflow gates, direct-child completion, and explicit messages for supported Kanban coordination. Do not treat Office's named documents, labels, or blocker UI as a stable public contract yet. +Regular Kanban reads and enforces blocker relationships (see [Task dependencies](#task-dependencies)) but has no blocker filter and no in-place editor: dependencies are declared when the task is created or over MCP. Office additionally exposes named documents, labels, and its own blocker property editor. Do not treat those Office surfaces as a stable public contract yet. ## Archive, unarchive, and delete diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index afe26cda11..9e9584f56d 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -79,6 +79,7 @@ Kandev's task model: documents, execution stages, labels, blocker escalation, su | [subtask-detachment](tasks/subtask-detachment.md) | shipped | | [subtask-reparenting-drag-drop](tasks/subtask-reparenting-drag-drop.md) | building | | [subtask-completion-trigger](tasks/subtask-completion-trigger.md) | draft | +| [task-dependencies](task-dependencies/spec.md) | draft | | [subtree-controls](tasks/subtree-controls.md) | shipped | | [blocked-task-escalation](tasks/blocked-task-escalation.md) | draft | | [runtime-cleanup](tasks/runtime-cleanup.md) | draft | diff --git a/docs/specs/task-dependencies/spec.md b/docs/specs/task-dependencies/spec.md new file mode 100644 index 0000000000..8ad8761ff5 --- /dev/null +++ b/docs/specs/task-dependencies/spec.md @@ -0,0 +1,564 @@ +--- +status: shipped +created: 2026-08-09 +owner: kandev +--- + +# Task Dependencies and Auto-Start Chains + +## Why + +A user who splits work into ordered steps — "scaffold the schema", then "write +the API on top of it", then "wire the UI" — has no way to express that order in +Kandev. The parent/child subtask hierarchy says *"B is part of A"*, never *"B +cannot start until A finishes"*. So the user either starts everything at once +and lets agents collide in the same repository, or babysits the board and +presses Start by hand each time a predecessor lands. Both defeat the point of +running agents unattended. + +## What + +- A task SHALL be able to declare that it **depends on** one or more other + tasks. The relationship is peer-to-peer and independent of `parent_id`: a + dependency may exist between two unrelated tasks, between siblings, or + between a task and its own child. +- The dependency graph is a **general DAG**, not a linear chain. A task may + have any number of predecessors and any number of dependents. Cycles SHALL be + rejected at edge-creation time with the offending path surfaced to the user. +- A task with at least one unresolved dependency is **blocked**. Blocked SHALL + be visible on the Kanban card, in the task detail view, and in the task API + payload. +- A blocked task SHALL NOT be auto-started by any automated path — workflow + `on_enter: auto_start_agent`, WIP queue promotion, integration watchers, or + dependency resolution itself. The gate applies to every automated launch, not + only to the dependency-resolution path. +- A user MAY still start a blocked task manually. Manual start is an explicit + override, not a bug; the UI names it as such. +- A task MAY carry the intent **"start automatically when unblocked"**. When + every dependency of such a task resolves successfully, Kandev launches it + exactly once with the agent profile, executor, executor profile, prompt, and + plan-mode choice recorded when the intent was created. +- Setting that intent on each link of a chain produces sequential execution: + A finishes → B starts → B finishes → C starts. No separate "chain" entity + exists; a chain is a path through the DAG. +- Auto-start on unblock respects every existing admission rule. A task whose + dependencies have resolved but which is not WIP-admitted (`wip_admitted = + false`) stays queued and launches when the WIP queue promotes it. Dependency + resolution grants **eligibility**, never a bypass. +- A dependency resolves only on **successful** completion of the predecessor. + `FAILED`, `CANCELLED`, and archival do not resolve a dependency. The + dependent task stays blocked and its blocked reason changes to name the + failed predecessor, so a chain never proceeds on a failed step. +- A dependency-failed state SHALL raise a notification and SHALL require + explicit human action to clear (retry the predecessor, remove the edge, or + start the dependent manually). Kandev never auto-retries a failed + predecessor and never silently drops the edge. +- Users declare dependencies in the **task-create dialog** ("Depends on") and + manage them afterwards over MCP. The task detail view deliberately has no edge + editor: dependencies describe how work was planned, and the surfaces that read + them (card badge, chip, graph) stay read-only. +- An open task SHALL show a **dependency chip** in the status row directly above + its chat composer, alongside the PR status chip. The chip reports both + directions — the tasks this task is blocked by, and the tasks it blocks — and + opens to a list of them with their states, each navigable to that task. It is + absent when the task has no edges in either direction. +- Agents SHALL be able to declare and manage dependencies over MCP: set them + when creating a task, and add or remove them on an existing task. The tools + SHALL teach when a dependency is the right relationship rather than a subtask, + because an agent that reaches for `parent_id` to express ordering gets neither + the gate nor the chain. +- Creating a task with dependencies and an agent-start request SHALL record the + start as a start-when-unblocked intent rather than launching immediately. An + automated caller that asks for both is describing a chain step, not a task to + run now. +- Deleting a task removes its edges in both directions. Dependents whose last + unresolved dependency was the deleted task become unblocked, and a + `start-when-unblocked` intent on such a dependent SHALL NOT fire — deletion + is not success. + +### Relationship to existing mechanisms + +This feature deliberately extends four shipped mechanisms rather than adding +parallel ones. The reuse is part of the contract because the guarantees below +(single launch, restart survival, WIP interaction) are inherited from them. + +| Existing mechanism | Role here | +|---|---| +| `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. | +| WIP admission and queue promotion ([`tasks/wip-limit-pull-system`](../tasks/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: + +- **Parent/child completion signalling** ([`tasks/subtask-completion-trigger`](../tasks/subtask-completion-trigger.md)) + fires `on_children_completed` when all children reach any terminal state, + including `FAILED`. Dependency resolution uses a stricter definition + (success only). The two must not be unified. +- **Office's blocker reaction** (`office/scheduler.cascadeBlockersResolved`) + queues an Office run to the blocked task's *assignee* when a blocker's status + becomes done. It stays as-is. For a task that qualifies for both reactions, + the shared auto-start claim guarantees exactly one launch. + +## Data model + +### `task_blockers` (existing, reused) + +```text +task_id TEXT the dependent task (blocked) +blocker_task_id TEXT the predecessor task (blocking) +created_at TIMESTAMP +PRIMARY KEY (task_id, blocker_task_id) +CHECK (task_id != blocker_task_id) +``` + +- The row reads "`task_id` is blocked by `blocker_task_id`". +- Edges are workspace-local: both tasks MUST belong to the same workspace. +- No new column is added. Edge deletion on task delete/archive is enforced by + the owning repository, not by a SQLite `ON DELETE CASCADE`, because the table + predates the foreign key. +- The table currently ships from the Office repository's + `createTaskExtensionTables`. Reading and writing it becomes a core task + concern; the Office reader methods stay valid against the same rows. + +### `tasks.metadata.deferred_launch` (existing, reused) + +The start-when-unblocked intent is a `deferred_launch` metadata record on the +**dependent** task, identical in shape to the one WIP overflow already +persists: resolved agent profile, executor, executor profile, prompt, +plan-mode choice, priority, and attachments. It is claimed atomically before +launch and restored on launch failure. + +A task therefore has at most one launch intent regardless of how many +predecessors it has. "Which edge triggers the start" is not a question this +model can ask — the answer is always "the last one to resolve". + +### Derived state (not persisted) + +`blocked`, `blocked_reason`, and the predecessor list are computed from +`task_blockers` plus the predecessors' own state on every read. There is no +denormalized `is_blocked` column: a stale copy of it would gate launches +incorrectly, which is the one failure this feature must not have. + +A predecessor is **resolved** when it satisfies the existing successful-completion +convention: `state = COMPLETED`, or resident in a final workflow step whose name +is `Done`, `Complete`, `Completed`, or `Approved` (`wfmodels.IsTerminalStepName`, +which also persists `state = COMPLETED`). + +A predecessor is **failed** when `state` is `FAILED` or `CANCELLED`. + +A predecessor is **pending** in every other case, including archived. + +## API surface + +### HTTP + +Existing Office-scoped routes are promoted to task-scoped equivalents. The +Office paths remain for compatibility with the Office task pane. + +```text +POST /api/v1/tasks/:id/dependencies {"depends_on_task_id": ""} +DELETE /api/v1/tasks/:id/dependencies/:depId +``` + +- `POST` returns `409` with `{"error": "...", "cycle": ["A","B","C","A"]}` when + the edge would close a cycle. The `cycle` body shape matches what + `BlockersPicker` already parses. +- `POST` returns `400` for a self-edge or a cross-workspace edge. +- `DELETE` of an absent edge is a success no-op. + +### Task payload + +Task DTOs returned over HTTP, WebSocket boot, WebSocket events, and MCP gain: + +```json +{ + "blocked": true, + "blocked_reason": "pending", + "depends_on": [ + { "id": "task-a", "title": "Add the schema migration", "state": "COMPLETED", "status": "resolved" }, + { "id": "task-b", "title": "Expose the edges on the API", "state": "TODO", "status": "pending" } + ], + "blocks": [{ "id": "task-d", "title": "Render the chip", "state": "TODO" }], + "start_when_unblocked": true +} +``` + +- `blocked_reason` is one of `"pending"` (at least one predecessor unfinished), + `"failed"` (no predecessor unfinished, at least one failed), `"unknown"` (the + dependency store could not be read, so the gate failed closed), or omitted + when `blocked` is false. A client renders `"unknown"` as blocked without a + named cause; it must not present it as resolved. +- `depends_on` lists direct predecessors only, in `created_at` order. +- `blocks` lists direct dependents only, in `created_at` order. It exists so the + dependency chip can show both directions without a second round trip, and so + an agent can see what is waiting on it. +- Both lists carry enough per-entry detail for the chip to render without + fetching each task: id, title, and state. Neither list is transitive. +- The projection is **absent, not empty, when a payload does not compute it**. + Every boot payload and list read computes it; lightweight `task.updated` events + do not. A client that reads an omitted projection as "no edges" erases the real + edges, so absent MUST mean "unknown" and an explicit empty list MUST mean "no + edges". A reader that needs certainty re-reads the task. +- `start_when_unblocked` reflects the presence of a launch intent tied to + dependency resolution. It is read-only on the task payload; it is set through + the create request or the dependency picker. + +### Task create + +`POST /api/v1/tasks` already accepts `blocked_by: ["", …]`. It gains: + +```json +{ "blocked_by": ["task-a"], "start_when_unblocked": true } +``` + +When `start_when_unblocked` is true **and `blocked_by` is non-empty**, the +create request's agent/executor/prompt resolution is recorded as the deferred +launch intent instead of launching now. + +`start_when_unblocked: true` with an empty `blocked_by` records no intent. The +task is born unblocked, so there is no later moment at which it would fire; a +create that also asked to start an agent launches immediately, exactly as it +would without the flag, and one that did not launches nothing. The flag defers +a start, it never invents one. Adding a dependency to that task afterwards +gates its *automated* starts but does not retroactively create an intent. + +When `blocked_by` is non-empty and `start_when_unblocked` is omitted, it defaults +to the request's agent-start intent: a create that asked to start an agent +records the intent, and a create that did not records no intent. The response +reports `started` and `start_when_unblocked` so the caller never has to infer +which happened. This rule is what makes an automated caller's habitual +`start_agent: true` build a chain instead of launching every step at once. + +### WebSocket events + +- `task.updated` with `fields: ["dependencies"]` on edge add/remove. +- `task.dependencies_resolved` — published when the last unresolved dependency + of a task resolves successfully. Payload: `{task_id, resolved_by_task_id}`. + This is the event whose handler enters the auto-start chokepoint. +- `task.dependency_failed` — published when a predecessor of a blocked task + reaches `FAILED` or `CANCELLED`. Payload: + `{task_id, failed_task_id, failed_state}`. + +### Notifications + +A new notification event type `task.dependency_failed` follows the existing +constants in `internal/notifications/service`. It fires once per +(dependent, predecessor) failure transition. + +### MCP + +`create_task_kandev`'s handler already reads a `blocked_by` array from its +payload, but the tool schema never declares it, so no agent can discover or pass +it. Declaring it is the minimum; the tools below make chains expressible. + +- `create_task_kandev` declares `blocked_by` (array of task IDs) and + `start_when_unblocked` (boolean). +- **`blocked_by` plus `start_agent: true` records a start-when-unblocked intent + and launches nothing now.** `start_agent` defaults to true and every agent + passes it by habit, so the alternative — launching a blocked task immediately — + would silently break every chain an agent tries to build. The create response + reports `started: false` with `start_when_unblocked: true` so the caller can + see what happened. Passing `start_when_unblocked: false` alongside + `blocked_by` creates the edges with no launch intent at all. +- `add_task_dependency_kandev` — `{task_id?, depends_on_task_id}`. `task_id` + defaults to the calling task. Returns the resulting `depends_on` list. + Rejects a cycle with the cycle path, a self-edge, and a cross-workspace edge. +- `remove_task_dependency_kandev` — `{task_id?, depends_on_task_id}`. Removing + an absent edge succeeds. Removing the last edge unblocks the task without + launching it. +- `list_related_tasks_kandev` is the read side and already returns `blockers` and + `blocked_by`. Those groups become populated in Kanban-only installs, where + they are empty today because the blocker repository is wired inside the + Office-enabled branch. +- Tool descriptions SHALL state when to use a dependency instead of a subtask: a + subtask means "part of", a dependency means "not until". Decomposing a plan + into ordered phases is N sibling tasks chained with `blocked_by` and + `start_when_unblocked`, not N subtasks started at once. The description SHALL + also state that a failed predecessor halts the chain and needs human action, so + an agent does not build a chain expecting it to self-heal. +- An agent cannot set `start_when_unblocked` on a task it did not create; it uses + the dependency tools for edges and leaves launch intent to the task's owner. + +## State machine + +Dependency state is derived, so the machine below describes the *dependent* +task's gating status rather than a persisted column. + +```text + edge added predecessor resolved (not last) + unblocked ──────────────────────> blocked ──────────────────────────> blocked + ^ │ ^ │ + │ last edge removed │ │ predecessor reopened │ + │ or last predecessor resolved │ │ │ + └──────────────────────────────┘ └──────────────────────────────────┘ + │ + predecessor FAILED/CANCELLED + v + blocked (reason=failed) + │ + predecessor retried and resolved, OR edge removed + v + unblocked +``` + +Transitions and their actors: + +| From | To | Trigger | Actor | +|---|---|---|---| +| unblocked | blocked | edge created | user, agent (`blocked_by`), API | +| blocked | blocked | a predecessor resolves, others remain | system | +| blocked | blocked (reason=failed) | a predecessor enters `FAILED`/`CANCELLED` | system | +| blocked | unblocked | last unresolved predecessor resolves successfully | system | +| blocked | unblocked | last blocking edge removed | user, agent, API | +| unblocked | blocked | a resolved predecessor is reopened to a non-terminal state | user, system | + +On the `blocked -> unblocked` transition caused by successful resolution, and +only then, Kandev publishes `task.dependencies_resolved` and evaluates +auto-start: + +```text +task.dependencies_resolved + -> task has deferred launch intent? no -> stop (task is merely unblocked) + -> task is WIP-admitted? no -> stop; WIP promotion will retry + -> claim deferred launch intent lost -> stop; the winner launches + -> launch with the recorded parameters +``` + +Removing the last edge unblocks the task but does **not** fire auto-start: the +intent is consumed by dependency *resolution*, not by the absence of +dependencies. A user who removes the edge is taking manual control. + +## Permissions + +- Creating or removing an edge requires the same authorization as updating both + tasks; both must be in a workspace the caller can access. Cross-workspace + edges are rejected, so no new cross-tenant surface exists. +- Agents create edges through task creation (`blocked_by`), the two MCP tools + (`add_task_dependency_kandev`, `remove_task_dependency_kandev`), and the + existing Office blocker endpoints. They cannot set `start_when_unblocked` on + a task they did not create. +- Every one of those paths authorizes BOTH task IDs against the caller's + identity before any read or write, and denials surface as not-found so an + edge attempt cannot be used to probe for another user's tasks. In a + task-bound MCP session the dependent end is the session's own task, taken + from the `AgentExecution` rather than from an agent-supplied identifier; the + predecessor is necessarily another task and is authorized on its own. +- The dependency gate is not a permission: a user with start rights can start a + blocked task manually. + +## Failure modes + +| Condition | Required behavior | +|---|---| +| Edge would create a cycle | `409` with the cycle path; no row written. | +| Edge is a self-edge or crosses workspaces | `400`; no row written. | +| Predecessor enters `FAILED`/`CANCELLED` | Dependent stays blocked with `blocked_reason: "failed"`; `task.dependency_failed` published; notification raised; no launch. | +| Predecessor is archived while a dependent is blocked | Treated as pending, not resolved. The dependent stays blocked and the reason names the archived predecessor. | +| Predecessor is deleted | Edge is removed. The dependent may become unblocked but its launch intent does not fire. | +| Dependent has no deferred launch intent | It unblocks and waits for a user or for its step's normal `on_enter` behavior on a later move. | +| Dependent is not WIP-admitted when its dependencies resolve | No launch. The existing WIP promotion path launches it once capacity opens; the intent survives. | +| Dependent's step has a WIP limit that is full | Same as above — the dependency gate and the WIP gate are independent and both must pass. | +| Two paths race to launch the same dependent (dependency resolution and WIP promotion) | The existing atomic deferred-launch claim admits one. The loser logs and stops. | +| Launch fails after the claim | The claim is restored so the task remains retryable and visibly unstarted, matching the WIP-overflow contract. | +| Predecessor lookup fails while evaluating resolution | No launch, warning logged. A later state change on any predecessor re-evaluates. | +| Blocker repository is unavailable | Edge writes fail with an error. Reads that fail return the task as **blocked with reason unknown** rather than unblocked — the gate fails closed, because failing open would launch work whose predecessor may not have run. | +| A predecessor is reopened after the dependent already launched | No retroactive stop. The dependent shows blocked again; the running session is untouched. | +| Dependency graph is large | Cycle detection keeps its existing BFS walk limit; a walk that exceeds the limit rejects the edge rather than accepting an unverified one. | + +## Persistence guarantees + +- Dependency edges are durable and survive restart. +- Start-when-unblocked intents are durable (task metadata) and survive restart, + including across a restart that happens between a predecessor's completion + and the dependent's launch: startup reconciliation re-evaluates blocked tasks + with intents and launches those whose dependencies are satisfied. +- Derived `blocked` state is never persisted and is recomputed on read, so a + restart cannot resurrect a stale gate. +- An intent is consumed at most once. A restart between claim and launch leaves + the task unstarted with the claim restored, not double-started. +- `task.dependencies_resolved` and `task.dependency_failed` are in-memory + events and are not replayed after restart; the startup reconciliation above + is the recovery path, and a failed-predecessor state is re-derived on read. + +## Scenarios + +Edges and blocking: + +- **GIVEN** tasks A and B in the same workspace, **WHEN** the user adds "B + depends on A", **THEN** B's card shows a blocked badge naming one + predecessor and B's payload reports `blocked: true`, `blocked_reason: + "pending"`, `depends_on: ["A"]`. +- **GIVEN** "B depends on A", **WHEN** the user tries to add "A depends on B", + **THEN** the request is rejected with the cycle path `A → B → A` and no edge + is created. +- **GIVEN** "B depends on A" and "C depends on B", **WHEN** the user tries to + add "A depends on C", **THEN** the request is rejected with the cycle path + `A → C → B → A`. +- **GIVEN** tasks in two different workspaces, **WHEN** the user tries to link + them, **THEN** the request is rejected and no edge is created. +- **GIVEN** B depends on A and on X, **WHEN** A completes successfully, + **THEN** B remains blocked with `depends_on: ["A","X"]` and + `blocked_reason: "pending"`. + +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. +- **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. +- **GIVEN** B depends on an unfinished A, **WHEN** the user clicks Start on B, + **THEN** B starts, because manual start is an explicit override. + +Auto-start and chaining: + +- **GIVEN** B depends on A, B has a start-when-unblocked intent, and B is + WIP-admitted, **WHEN** A completes successfully, **THEN** exactly one session + is created for B using the intent's recorded agent profile, executor, + executor profile, prompt, and plan-mode choice. +- **GIVEN** A → B → C each with a start-when-unblocked intent, **WHEN** A + completes, **THEN** B starts; **WHEN** B completes, **THEN** C starts, and A + is never restarted. +- **GIVEN** B depends on A and B has **no** start-when-unblocked intent, + **WHEN** A completes, **THEN** B unblocks, no session is created, and B's + card shows no blocked badge. +- **GIVEN** B depends on A, B has an intent, and B is queued (not WIP-admitted) + for a full step, **WHEN** A completes, **THEN** no session is created and B + stays queued; **WHEN** capacity later opens and B is promoted, **THEN** + exactly one session is created. +- **GIVEN** B depends on A and B has an intent, **WHEN** A completes and the + backend restarts before B launches, **THEN** after startup exactly one + session is created for B. +- **GIVEN** B depends on A and on X, both with A and X unfinished, **WHEN** A + completes and then X completes, **THEN** B launches exactly once, on X's + completion. +- **GIVEN** B depends on A and B has an intent, **WHEN** the user removes the + edge instead of A completing, **THEN** B unblocks and no session is created. + +Failure paths: + +- **GIVEN** B depends on A and B has an intent, **WHEN** A ends `FAILED`, + **THEN** B stays blocked with `blocked_reason: "failed"`, no session is + created, `task.dependency_failed` is published, and a notification is raised. +- **GIVEN** B is blocked with `blocked_reason: "failed"` after A failed, + **WHEN** A is retried and completes successfully, **THEN** B unblocks and its + intent fires exactly once. +- **GIVEN** B depends on A and A is `CANCELLED`, **WHEN** the dependency state + is read, **THEN** B is blocked with `blocked_reason: "failed"`. +- **GIVEN** B depends on A, **WHEN** A is archived, **THEN** B stays blocked + and A is reported as a pending predecessor. +- **GIVEN** B depends on A and B has an intent, **WHEN** A is deleted, **THEN** + the edge is removed, B unblocks, and no session is created. +- **GIVEN** A has completed and B has launched, **WHEN** A is reopened to + `IN_PROGRESS`, **THEN** B reports blocked again and B's running session is + not stopped. +- **GIVEN** the blocker store returns an error, **WHEN** an automated path + evaluates whether a task may auto-start, **THEN** the launch is skipped and a + warning is logged. + +UI: + +- **GIVEN** a blocked task on the Kanban board, **WHEN** the user hovers or taps + its blocked badge, **THEN** the predecessor titles and their states are + listed, and the badge coexists with the WIP queued badge without truncating + either. +- **GIVEN** the task-create dialog, **WHEN** the user opens "Depends on", **THEN** + the board's other tasks are offered as candidates, selecting one shows a + removable chip, and the copy states that starting an agent will run the task + automatically once every selection completes. +- **GIVEN** a task created with dependencies and an agent start, **WHEN** creation + succeeds, **THEN** no session is started and the response reports + `start_when_unblocked: true`. +- **GIVEN** an open task B where B depends on A and D depends on B, **WHEN** the + task view renders, **THEN** a dependency chip appears in the status row above + the composer next to the PR status chip, and opening it lists A under + blocked-by and D under blocks, each with its state and each navigable. +- **GIVEN** an open task with no dependency edges in either direction, **WHEN** + the task view renders, **THEN** no dependency chip appears, and a status row + that would otherwise be empty still does not render. +- **GIVEN** an open task whose board copy carries no dependency projection (a + lightweight `task.updated` overwrote it, or arrived before hydration), **WHEN** + the task view renders, **THEN** the chip still shows the task's edges, because a + reader that cannot trust its cached copy re-reads them. +- **GIVEN** an open blocked task whose predecessor failed, **WHEN** the user opens + the dependency chip, **THEN** the failed predecessor is distinguished from + merely pending ones. +- **GIVEN** a narrow mobile viewport, **WHEN** the user taps the dependency chip, + **THEN** the list opens in the same drawer pattern the PR status chip uses on + mobile, not a hover card. + +MCP: + +- **GIVEN** an agent calls `create_task_kandev` with `blocked_by: ["A"]` and no + explicit `start_agent`, **THEN** the task is created with the edge, no session + is started, and the response reports `started: false` and + `start_when_unblocked: true`. +- **GIVEN** an agent calls `create_task_kandev` with `blocked_by: ["A"]` and + `start_when_unblocked: false`, **THEN** the edge is created, no launch intent + is recorded, and completing A starts nothing. +- **GIVEN** an agent calls `create_task_kandev` with `start_when_unblocked: true` + and no `blocked_by`, **THEN** no launch intent is recorded and the task starts + or does not start purely per `start_agent`; the response reports + `start_when_unblocked: false`. +- **GIVEN** an agent decomposes a plan into three chained sibling tasks over + three `create_task_kandev` calls, **WHEN** the first task completes + successfully, **THEN** the second starts and the third does not. +- **GIVEN** an agent calls `add_task_dependency_kandev` omitting `task_id`, + **THEN** the edge is added to the calling task and the resulting `depends_on` + list is returned. +- **GIVEN** an agent calls `add_task_dependency_kandev` with an edge that would + close a cycle, **THEN** the call fails with the cycle path and no edge is + created. +- **GIVEN** an agent calls `remove_task_dependency_kandev` for an edge that does + not exist, **THEN** the call succeeds. +- **GIVEN** a Kanban-only install with `Features.Office` false, **WHEN** an agent + calls `create_task_kandev` with `blocked_by`, **THEN** the task and edge are + created and `list_related_tasks_kandev` reports them. +- **GIVEN** a narrow mobile viewport, **WHEN** the user opens a column + containing a blocked task, **THEN** the blocked state and predecessor count + are readable on the card without hover-only disclosure and without horizontal + overflow. + +## Out of scope + +- **A whole-graph board view.** A layered DAG view was built and then removed + before shipping: without drawn connectors it read as columns of text, and it + could only show one workflow at a time, so it did not answer "show me this + chain" any better than the per-task chip does. The chip (both directions, one + hop) and `list_related_tasks_kandev` are the shipped ways to read the graph. + A future view should draw real connectors, using the existing + `components/kanban/graph2-connector.tsx` primitives, and be able to focus a + single chain. + +- **Office mode changes.** Office is feature-flagged and not in the live + feature inventory ([`docs/features.md`](../../features.md), "In Progress: + Office Mode"), and it already has its own blocker reaction and run queue. + v1 makes the relationship and the Kanban reaction work in Kanban mode and + leaves `cascadeBlockersResolved`, Office agent assignment, and Office task + delegation untouched. Unifying the two reactions is a follow-up that should + wait until Office ships. +- Optional or "soft" dependencies (start-anyway-after-N-hours, OR semantics + across predecessors). v1 is AND-only over all direct predecessors. +- Cross-workspace dependencies. +- Passing a predecessor's output (summary, diff, PR link) into the dependent's + prompt. The dependent launches with the prompt recorded in its intent. Agents + that need predecessor context read it through `list_related_tasks_kandev`. +- Stopping or rolling back a dependent that already launched when its + predecessor is reopened or reverted. +- Automatic retry of a failed predecessor, and automatic edge removal on + failure. +- A dependency-aware scheduling policy (critical-path ordering, priority + inheritance along a chain, WIP reservation for a whole chain). +- Templated chains — saving "A → B → C" as a reusable pipeline definition. + A chain is a set of edges between concrete tasks in v1. +- Gantt or timeline rendering, and estimated-duration modelling. +- Editing dependencies from the multi-select toolbar or via bulk operations. +- An edge editor on the task detail view. Declaring dependencies is a planning + act that belongs to task creation (or to MCP for an agent decomposing work); + the task view only reports them. + +## Implementation plan + +See [`../../plans/task-dependencies/plan.md`](../../plans/task-dependencies/plan.md).