diff --git a/apps/backend/config/prompts/config-context.md b/apps/backend/config/prompts/config-context.md index cb0489bb07..b259125012 100644 --- a/apps/backend/config/prompts/config-context.md +++ b/apps/backend/config/prompts/config-context.md @@ -12,8 +12,9 @@ WORKFLOW TOOLS: - delete_workflow_kandev: Delete a workflow and all its steps (destructive). Required: workflow_id. - import_workflow_kandev: Import one or more workflows into a workspace from a portable document (the kandev_workflow YAML/JSON export envelope). Workflows whose name already exists are skipped. Required: workspace_id, document. Returns the created and skipped workflow names. - list_workflow_steps_kandev: List workflow steps (columns) in a workflow. Required: workflow_id. -- create_workflow_step_kandev: Create a new workflow step. Required: workflow_id, name. Optional: position, color, prompt, is_start_step, allow_manual_move, show_in_command_panel, auto_advance_requires_signal, events. -- update_workflow_step_kandev: Update a workflow step. Required: step_id. Optional: name, color, prompt, is_start_step, allow_manual_move, show_in_command_panel, auto_archive_after_hours, auto_advance_requires_signal, events. +- create_workflow_step_kandev: Create a new workflow step. Required: workflow_id, name. Optional: position, color, prompt, is_start_step, allow_manual_move, show_in_command_panel, auto_advance_requires_signal, cancel_triggers_turn_complete, events. +- update_workflow_step_kandev: Update a workflow step. Required: step_id. Optional: name, color, prompt, is_start_step, allow_manual_move, show_in_command_panel, auto_archive_after_hours, auto_advance_requires_signal, cancel_triggers_turn_complete, events. +- cancel_triggers_turn_complete: When true, an explicit user cancellation runs the step's normal on_turn_complete actions after the cancelled turn settles. It does not apply to internal or agent cancellations. - delete_workflow_step_kandev: Delete a workflow step (destructive). Required: step_id. - reorder_workflow_steps_kandev: Reorder workflow steps. Required: workflow_id, step_ids (ordered array of step IDs). diff --git a/apps/backend/config/workflows/kanban.yml b/apps/backend/config/workflows/kanban.yml index f3eea43c83..24820ed8b5 100644 --- a/apps/backend/config/workflows/kanban.yml +++ b/apps/backend/config/workflows/kanban.yml @@ -15,6 +15,7 @@ steps: position: 0 color: bg-neutral-400 allow_manual_move: true + cancel_triggers_turn_complete: true events: on_turn_start: - type: move_to_next @@ -30,6 +31,7 @@ steps: is_start_step: true show_in_command_panel: true allow_manual_move: true + cancel_triggers_turn_complete: true events: on_enter: - type: auto_start_agent diff --git a/apps/backend/config/workflows/loader.go b/apps/backend/config/workflows/loader.go index fcdb5f9dfb..e9d42aba96 100644 --- a/apps/backend/config/workflows/loader.go +++ b/apps/backend/config/workflows/loader.go @@ -23,17 +23,19 @@ type templateYAML struct { // stepDefYAML is the YAML-friendly representation of a step definition. type stepDefYAML struct { - ID string `yaml:"id"` - Name string `yaml:"name"` - Position int `yaml:"position"` - Color string `yaml:"color"` - Prompt string `yaml:"prompt,omitempty"` - StageType string `yaml:"stage_type,omitempty"` - IsStartStep bool `yaml:"is_start_step,omitempty"` - ShowInCommandPanel bool `yaml:"show_in_command_panel,omitempty"` - AllowManualMove bool `yaml:"allow_manual_move,omitempty"` - AutoArchiveAfterHours int `yaml:"auto_archive_after_hours,omitempty"` - Events stepEventsYAML `yaml:"events,omitempty"` + ID string `yaml:"id"` + Name string `yaml:"name"` + Position int `yaml:"position"` + Color string `yaml:"color"` + Prompt string `yaml:"prompt,omitempty"` + StageType string `yaml:"stage_type,omitempty"` + IsStartStep bool `yaml:"is_start_step,omitempty"` + ShowInCommandPanel bool `yaml:"show_in_command_panel,omitempty"` + AllowManualMove bool `yaml:"allow_manual_move,omitempty"` + AutoArchiveAfterHours int `yaml:"auto_archive_after_hours,omitempty"` + AutoAdvanceRequiresSignal bool `yaml:"auto_advance_requires_signal,omitempty"` + CancelTriggersTurnComplete bool `yaml:"cancel_triggers_turn_complete,omitempty"` + Events stepEventsYAML `yaml:"events,omitempty"` } // stepEventsYAML is the YAML-friendly representation of step events. @@ -162,17 +164,19 @@ func convertStep(s stepDefYAML) (models.StepDefinition, error) { return models.StepDefinition{}, fmt.Errorf("step %q: %w", s.ID, err) } return models.StepDefinition{ - ID: s.ID, - Name: s.Name, - Position: s.Position, - Color: s.Color, - Prompt: strings.TrimSpace(s.Prompt), - Events: events, - AllowManualMove: s.AllowManualMove, - IsStartStep: s.IsStartStep, - ShowInCommandPanel: s.ShowInCommandPanel, - AutoArchiveAfterHours: s.AutoArchiveAfterHours, - StageType: stage, + ID: s.ID, + Name: s.Name, + Position: s.Position, + Color: s.Color, + Prompt: strings.TrimSpace(s.Prompt), + Events: events, + AllowManualMove: s.AllowManualMove, + IsStartStep: s.IsStartStep, + ShowInCommandPanel: s.ShowInCommandPanel, + AutoArchiveAfterHours: s.AutoArchiveAfterHours, + AutoAdvanceRequiresSignal: s.AutoAdvanceRequiresSignal, + CancelTriggersTurnComplete: s.CancelTriggersTurnComplete, + StageType: stage, }, nil } diff --git a/apps/backend/config/workflows/loader_test.go b/apps/backend/config/workflows/loader_test.go index 0b0dfec38c..2794257d25 100644 --- a/apps/backend/config/workflows/loader_test.go +++ b/apps/backend/config/workflows/loader_test.go @@ -2,6 +2,7 @@ package workflows import ( "fmt" + "reflect" "strings" "testing" @@ -9,6 +10,56 @@ import ( "gopkg.in/yaml.v3" ) +func boolFieldForTest(t *testing.T, value any, name string) bool { + t.Helper() + v := reflect.ValueOf(value) + if v.Kind() == reflect.Pointer { + v = v.Elem() + } + field := v.FieldByName(name) + if !field.IsValid() { + t.Fatalf("%T is missing %s", value, name) + } + if field.Kind() != reflect.Bool { + t.Fatalf("%T.%s has kind %s, want bool", value, name, field.Kind()) + } + return field.Bool() +} + +func TestLoadTemplates_CancelTriggersTurnCompleteDefaults(t *testing.T) { + templates, err := LoadTemplates() + if err != nil { + t.Fatalf("LoadTemplates() returned error: %v", err) + } + var simple *models.WorkflowTemplate + for _, template := range templates { + if template.ID == "simple" { + simple = template + break + } + } + if simple == nil { + t.Fatal("simple template not found") + } + want := map[string]bool{"Backlog": true, "In Progress": true, "Review": false, "Done": false} + seen := make(map[string]bool, len(want)) + for _, step := range simple.Steps { + wantValue, ok := want[step.Name] + if !ok { + continue + } + seen[step.Name] = true + if got := boolFieldForTest(t, step, "CancelTriggersTurnComplete"); got != wantValue { + t.Errorf("simple template step %q cancel trigger = %t, want %t", step.Name, got, wantValue) + } + } + for name := range want { + if !seen[name] { + t.Errorf("simple template step %q not found", name) + } + } +} + func TestLoadTemplates_AllValid(t *testing.T) { templates, err := LoadTemplates() if err != nil { diff --git a/apps/backend/internal/mcp/handlers/config_workflow_handlers.go b/apps/backend/internal/mcp/handlers/config_workflow_handlers.go index 0b13780ea2..4df7218dda 100644 --- a/apps/backend/internal/mcp/handlers/config_workflow_handlers.go +++ b/apps/backend/internal/mcp/handlers/config_workflow_handlers.go @@ -131,18 +131,19 @@ func (h *Handlers) handleImportWorkflow(ctx context.Context, msg *ws.Message) (* func (h *Handlers) handleCreateWorkflowStep(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var req struct { - WorkflowID string `json:"workflow_id"` - Name string `json:"name"` - Position int `json:"position"` - Color string `json:"color"` - Prompt string `json:"prompt"` - IsStartStep *bool `json:"is_start_step"` - AllowManualMove *bool `json:"allow_manual_move"` - ShowInCommandPanel *bool `json:"show_in_command_panel"` - AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal"` - WIPLimit *int `json:"wip_limit"` - PullFromStepID *string `json:"pull_from_step_id"` - Events *wfmodels.StepEvents `json:"events"` + WorkflowID string `json:"workflow_id"` + Name string `json:"name"` + Position int `json:"position"` + Color string `json:"color"` + Prompt string `json:"prompt"` + IsStartStep *bool `json:"is_start_step"` + AllowManualMove *bool `json:"allow_manual_move"` + ShowInCommandPanel *bool `json:"show_in_command_panel"` + AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal"` + CancelTriggersTurnComplete *bool `json:"cancel_triggers_turn_complete"` + WIPLimit *int `json:"wip_limit"` + PullFromStepID *string `json:"pull_from_step_id"` + Events *wfmodels.StepEvents `json:"events"` } if err := json.Unmarshal(msg.Payload, &req); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) @@ -155,17 +156,18 @@ func (h *Handlers) handleCreateWorkflowStep(ctx context.Context, msg *ws.Message } createReq := workflowctrl.CreateStepRequest{ - WorkflowID: req.WorkflowID, - Name: req.Name, - Position: req.Position, - Color: req.Color, - Prompt: req.Prompt, - IsStartStep: req.IsStartStep, - ShowInCommandPanel: req.ShowInCommandPanel, - AutoAdvanceRequiresSignal: req.AutoAdvanceRequiresSignal, - WIPLimit: req.WIPLimit, - PullFromStepID: req.PullFromStepID, - Events: req.Events, + WorkflowID: req.WorkflowID, + Name: req.Name, + Position: req.Position, + Color: req.Color, + Prompt: req.Prompt, + IsStartStep: req.IsStartStep, + ShowInCommandPanel: req.ShowInCommandPanel, + AutoAdvanceRequiresSignal: req.AutoAdvanceRequiresSignal, + CancelTriggersTurnComplete: req.CancelTriggersTurnComplete, + WIPLimit: req.WIPLimit, + PullFromStepID: req.PullFromStepID, + Events: req.Events, } if req.AllowManualMove != nil { createReq.AllowManualMove = *req.AllowManualMove @@ -183,18 +185,19 @@ func (h *Handlers) handleCreateWorkflowStep(ctx context.Context, msg *ws.Message func (h *Handlers) handleUpdateWorkflowStep(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var req struct { - StepID string `json:"step_id"` - Name *string `json:"name"` - Color *string `json:"color"` - Prompt *string `json:"prompt"` - IsStartStep *bool `json:"is_start_step"` - AllowManualMove *bool `json:"allow_manual_move"` - ShowInCommandPanel *bool `json:"show_in_command_panel"` - AutoArchiveAfterHours *int `json:"auto_archive_after_hours"` - AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal"` - WIPLimit *int `json:"wip_limit"` - PullFromStepID *string `json:"pull_from_step_id"` - Events *wfmodels.StepEvents `json:"events"` + StepID string `json:"step_id"` + Name *string `json:"name"` + Color *string `json:"color"` + Prompt *string `json:"prompt"` + IsStartStep *bool `json:"is_start_step"` + AllowManualMove *bool `json:"allow_manual_move"` + ShowInCommandPanel *bool `json:"show_in_command_panel"` + AutoArchiveAfterHours *int `json:"auto_archive_after_hours"` + AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal"` + CancelTriggersTurnComplete *bool `json:"cancel_triggers_turn_complete"` + WIPLimit *int `json:"wip_limit"` + PullFromStepID *string `json:"pull_from_step_id"` + Events *wfmodels.StepEvents `json:"events"` } if err := json.Unmarshal(msg.Payload, &req); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) @@ -204,18 +207,19 @@ func (h *Handlers) handleUpdateWorkflowStep(ctx context.Context, msg *ws.Message } updateReq := workflowctrl.UpdateStepRequest{ - ID: req.StepID, - Name: req.Name, - Color: req.Color, - Prompt: req.Prompt, - IsStartStep: req.IsStartStep, - AllowManualMove: req.AllowManualMove, - ShowInCommandPanel: req.ShowInCommandPanel, - AutoArchiveAfterHours: req.AutoArchiveAfterHours, - AutoAdvanceRequiresSignal: req.AutoAdvanceRequiresSignal, - WIPLimit: req.WIPLimit, - PullFromStepID: req.PullFromStepID, - Events: req.Events, + ID: req.StepID, + Name: req.Name, + Color: req.Color, + Prompt: req.Prompt, + IsStartStep: req.IsStartStep, + AllowManualMove: req.AllowManualMove, + ShowInCommandPanel: req.ShowInCommandPanel, + AutoArchiveAfterHours: req.AutoArchiveAfterHours, + AutoAdvanceRequiresSignal: req.AutoAdvanceRequiresSignal, + CancelTriggersTurnComplete: req.CancelTriggersTurnComplete, + WIPLimit: req.WIPLimit, + PullFromStepID: req.PullFromStepID, + Events: req.Events, } resp, err := h.workflowCtrl.UpdateStep(ctx, updateReq) @@ -282,24 +286,25 @@ func (h *Handlers) publishWorkflowStepEvent(ctx context.Context, eventType strin } data := map[string]interface{}{ "step": map[string]interface{}{ - "id": step.ID, - "workflow_id": step.WorkflowID, - "name": step.Name, - "position": step.Position, - "color": step.Color, - "prompt": step.Prompt, - "events": step.Events, - "show_in_command_panel": step.ShowInCommandPanel, - "allow_manual_move": step.AllowManualMove, - "is_start_step": step.IsStartStep, - "auto_archive_after_hours": step.AutoArchiveAfterHours, - "wip_limit": step.WIPLimit, - "pull_from_step_id": step.PullFromStepID, - "agent_profile_id": step.AgentProfileID, - "stage_type": string(step.StageType), - "auto_advance_requires_signal": step.AutoAdvanceRequiresSignal, - "created_at": step.CreatedAt, - "updated_at": step.UpdatedAt, + "id": step.ID, + "workflow_id": step.WorkflowID, + "name": step.Name, + "position": step.Position, + "color": step.Color, + "prompt": step.Prompt, + "events": step.Events, + "show_in_command_panel": step.ShowInCommandPanel, + "allow_manual_move": step.AllowManualMove, + "is_start_step": step.IsStartStep, + "auto_archive_after_hours": step.AutoArchiveAfterHours, + "wip_limit": step.WIPLimit, + "pull_from_step_id": step.PullFromStepID, + "agent_profile_id": step.AgentProfileID, + "stage_type": string(step.StageType), + "auto_advance_requires_signal": step.AutoAdvanceRequiresSignal, + "cancel_triggers_turn_complete": step.CancelTriggersTurnComplete, + "created_at": step.CreatedAt, + "updated_at": step.UpdatedAt, }, } if err := h.eventBus.Publish(ctx, eventType, bus.NewEvent(eventType, "mcp-handlers", data)); err != nil { diff --git a/apps/backend/internal/mcp/handlers/config_workflow_handlers_test.go b/apps/backend/internal/mcp/handlers/config_workflow_handlers_test.go index 3226bc0157..df8dad2d20 100644 --- a/apps/backend/internal/mcp/handlers/config_workflow_handlers_test.go +++ b/apps/backend/internal/mcp/handlers/config_workflow_handlers_test.go @@ -194,6 +194,27 @@ func TestHandleCreateWorkflowStep_PersistsAutoAdvanceRequiresSignal(t *testing.T assert.True(t, steps[0].AutoAdvanceRequiresSignal) } +func TestHandleCreateWorkflowStep_PersistsCancelTriggersTurnComplete(t *testing.T) { + h, _, repo := setupImportHandlers(t) + ctx := context.Background() + h.workflowCtrl = workflowctrl.NewController(h.workflowSvc) + + msg := makeWSMessage(t, ws.ActionMCPCreateWorkflowStep, map[string]interface{}{ + "workflow_id": "wf-test", + "name": "Cancel completion", + "position": 0, + "cancel_triggers_turn_complete": true, + }) + resp, err := h.handleCreateWorkflowStep(ctx, msg) + require.NoError(t, err) + require.Equal(t, ws.MessageTypeResponse, resp.Type) + + steps, err := repo.ListStepsByWorkflow(ctx, "wf-test") + require.NoError(t, err) + require.Len(t, steps, 1) + assert.True(t, steps[0].CancelTriggersTurnComplete) +} + func TestHandleUpdateWorkflowStep_PublishesDemotedStartStep(t *testing.T) { h, _, repo := setupImportHandlers(t) ctx := context.Background() @@ -269,6 +290,31 @@ func TestHandleUpdateWorkflowStep_PersistsAutoAdvanceRequiresSignalFalse(t *test assert.False(t, step.AutoAdvanceRequiresSignal) } +func TestHandleUpdateWorkflowStep_PersistsCancelTriggersTurnCompleteFalse(t *testing.T) { + h, _, repo := setupImportHandlers(t) + ctx := context.Background() + h.workflowCtrl = workflowctrl.NewController(h.workflowSvc) + + require.NoError(t, repo.CreateStep(ctx, &wfmodels.WorkflowStep{ + ID: "cancel-gated", + WorkflowID: "wf-test", + Name: "Cancel gated", + Position: 0, + CancelTriggersTurnComplete: true, + })) + msg := makeWSMessage(t, ws.ActionMCPUpdateWorkflowStep, map[string]interface{}{ + "step_id": "cancel-gated", + "cancel_triggers_turn_complete": false, + }) + resp, err := h.handleUpdateWorkflowStep(ctx, msg) + require.NoError(t, err) + require.Equal(t, ws.MessageTypeResponse, resp.Type) + + step, err := repo.GetStep(ctx, "cancel-gated") + require.NoError(t, err) + assert.False(t, step.CancelTriggersTurnComplete) +} + func TestHandleListWorkflowSteps_IncludesAutoAdvanceRequiresSignal(t *testing.T) { h, _, repo := setupImportHandlers(t) ctx := context.Background() @@ -310,6 +356,36 @@ func TestHandleListWorkflowSteps_IncludesAutoAdvanceRequiresSignal(t *testing.T) assert.Equal(t, true, body.Steps[1]["auto_advance_requires_signal"]) } +func TestHandleListWorkflowSteps_IncludesCancelTriggersTurnComplete(t *testing.T) { + h, _, repo := setupImportHandlers(t) + ctx := context.Background() + h.workflowCtrl = workflowctrl.NewController(h.workflowSvc) + require.NoError(t, repo.CreateStep(ctx, &wfmodels.WorkflowStep{ + ID: "cancel-legacy", + WorkflowID: "wf-test", + Name: "Legacy", + Position: 0, + })) + require.NoError(t, repo.CreateStep(ctx, &wfmodels.WorkflowStep{ + ID: "cancel-enabled", + WorkflowID: "wf-test", + Name: "Enabled", + Position: 1, + CancelTriggersTurnComplete: true, + })) + + msg := makeWSMessage(t, ws.ActionMCPListWorkflowSteps, map[string]interface{}{"workflow_id": "wf-test"}) + resp, err := h.handleListWorkflowSteps(ctx, msg) + require.NoError(t, err) + var body struct { + Steps []map[string]interface{} `json:"steps"` + } + require.NoError(t, json.Unmarshal(resp.Payload, &body)) + require.Len(t, body.Steps, 2) + assert.Equal(t, false, body.Steps[0]["cancel_triggers_turn_complete"]) + assert.Equal(t, true, body.Steps[1]["cancel_triggers_turn_complete"]) +} + func TestHandleImportWorkflow_PersistsWorkflow(t *testing.T) { h, provider, repo := setupImportHandlers(t) diff --git a/apps/backend/internal/mcp/server/config_handlers.go b/apps/backend/internal/mcp/server/config_handlers.go index b05bdc80dd..caf9a45f45 100644 --- a/apps/backend/internal/mcp/server/config_handlers.go +++ b/apps/backend/internal/mcp/server/config_handlers.go @@ -89,6 +89,7 @@ func (s *Server) registerConfigWorkflowStepTools() { mcp.WithBoolean("allow_manual_move", mcp.Description("Allow manual task moves into this step (default: false)")), mcp.WithBoolean("show_in_command_panel", mcp.Description("Show this step in the command panel")), mcp.WithBoolean("auto_advance_requires_signal", mcp.Description("Require step_complete_kandev before on_turn_complete auto-advance transitions run")), + mcp.WithBoolean("cancel_triggers_turn_complete", mcp.Description("Run on_turn_complete actions when an explicit user cancellation occurs")), mcp.WithNumber("wip_limit", mcp.Description("Work-in-progress limit for this step. 0 means unlimited.")), mcp.WithString("pull_from_step_id", mcp.Description("Optional feeder workflow step ID to pull from when capacity opens.")), mcp.WithObject("events", mcp.Description("Event-driven actions. Keys: on_enter, on_exit, on_turn_start, on_turn_complete. Each is an array of {type, config} objects.")), @@ -107,6 +108,7 @@ func (s *Server) registerConfigWorkflowStepTools() { mcp.WithBoolean("show_in_command_panel", mcp.Description("Show this step in the command panel")), mcp.WithNumber("auto_archive_after_hours", mcp.Description("Auto-archive tasks after N hours in this step (0 to disable)")), mcp.WithBoolean("auto_advance_requires_signal", mcp.Description("Require step_complete_kandev before on_turn_complete auto-advance transitions run")), + mcp.WithBoolean("cancel_triggers_turn_complete", mcp.Description("Run on_turn_complete actions when an explicit user cancellation occurs")), mcp.WithNumber("wip_limit", mcp.Description("Work-in-progress limit for this step. 0 means unlimited.")), mcp.WithString("pull_from_step_id", mcp.Description("Optional feeder workflow step ID to pull from when capacity opens.")), mcp.WithObject("events", mcp.Description("Event-driven actions. Keys: on_enter, on_exit, on_turn_start, on_turn_complete.")), @@ -403,7 +405,7 @@ func (s *Server) createWorkflowStepHandler() server.ToolHandlerFunc { payload["prompt"] = prompt } args := req.GetArguments() - for _, key := range []string{"position", "is_start_step", "allow_manual_move", "show_in_command_panel", "auto_advance_requires_signal", "wip_limit", "pull_from_step_id", "events"} { + for _, key := range []string{"position", "is_start_step", "allow_manual_move", "show_in_command_panel", "auto_advance_requires_signal", "cancel_triggers_turn_complete", "wip_limit", "pull_from_step_id", "events"} { if args[key] != nil { payload[key] = args[key] } @@ -429,7 +431,7 @@ func (s *Server) updateWorkflowStepHandler() server.ToolHandlerFunc { payload["prompt"] = prompt } args := req.GetArguments() - for _, key := range []string{"is_start_step", "allow_manual_move", "show_in_command_panel", "auto_archive_after_hours", "auto_advance_requires_signal", "wip_limit", "pull_from_step_id", "events"} { + for _, key := range []string{"is_start_step", "allow_manual_move", "show_in_command_panel", "auto_archive_after_hours", "auto_advance_requires_signal", "cancel_triggers_turn_complete", "wip_limit", "pull_from_step_id", "events"} { if args[key] != nil { payload[key] = args[key] } diff --git a/apps/backend/internal/mcp/server/config_handlers_test.go b/apps/backend/internal/mcp/server/config_handlers_test.go index cd7864c051..36e517ea12 100644 --- a/apps/backend/internal/mcp/server/config_handlers_test.go +++ b/apps/backend/internal/mcp/server/config_handlers_test.go @@ -121,6 +121,16 @@ func TestWorkflowStepTools_SchemaExposesAutoAdvanceRequiresSignal(t *testing.T) assert.Contains(t, updateProps, "auto_advance_requires_signal") } +func TestWorkflowStepTools_SchemaExposesCancelTriggersTurnComplete(t *testing.T) { + backend := &testBackend{} + s := newTestServer(t, backend) + + createProps := toolInputProperties(t, s, "create_workflow_step_kandev") + updateProps := toolInputProperties(t, s, "update_workflow_step_kandev") + assert.Contains(t, createProps, "cancel_triggers_turn_complete") + assert.Contains(t, updateProps, "cancel_triggers_turn_complete") +} + func TestCreateWorkflowHandler_Success(t *testing.T) { backend := &testBackend{ response: map[string]interface{}{"id": "wf-1", "name": "Sprint Board"}, @@ -305,6 +315,21 @@ func TestCreateWorkflowStepHandler_AllFields(t *testing.T) { assert.NotNil(t, payload["events"]) } +func TestCreateWorkflowStepHandler_ForwardsCancelTriggersTurnComplete(t *testing.T) { + backend := &testBackend{response: map[string]interface{}{"step": map[string]interface{}{"id": "step-1"}}} + s := newTestServer(t, backend) + + result := callTool(t, s, "create_workflow_step_kandev", map[string]interface{}{ + "workflow_id": "wf-123", + "name": "Cancel completion", + "cancel_triggers_turn_complete": true, + }) + assert.False(t, result.IsError) + payload, ok := backend.lastPayload.(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, true, payload["cancel_triggers_turn_complete"]) +} + func TestCreateWorkflowStepHandler_MissingWorkflowID(t *testing.T) { backend := &testBackend{} s := newTestServer(t, backend) @@ -372,6 +397,20 @@ func TestUpdateWorkflowStepHandler_AllFields(t *testing.T) { assert.NotNil(t, payload["events"]) } +func TestUpdateWorkflowStepHandler_ForwardsCancelTriggersTurnComplete(t *testing.T) { + backend := &testBackend{response: map[string]interface{}{"step": map[string]interface{}{"id": "step-1"}}} + s := newTestServer(t, backend) + + result := callTool(t, s, "update_workflow_step_kandev", map[string]interface{}{ + "step_id": "step-1", + "cancel_triggers_turn_complete": false, + }) + assert.False(t, result.IsError) + payload, ok := backend.lastPayload.(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, false, payload["cancel_triggers_turn_complete"]) +} + func TestUpdateWorkflowStepHandler_MissingStepID(t *testing.T) { backend := &testBackend{} s := newTestServer(t, backend) diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index f4945bc060..ccb7710090 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -23,59 +23,36 @@ import ( v1 "github.com/kandev/kandev/pkg/api/v1" ) +type turnCompletionCause string + +const ( + turnCompletionCauseAgentTurn turnCompletionCause = "agent_turn" + turnCompletionCauseUserCancellation turnCompletionCause = "user_cancellation" +) + // processOnTurnComplete processes the on_turn_complete events for the current step. // Returns true if a transition occurred (step change happened). func (s *Service) processOnTurnComplete(ctx context.Context, task *models.Task, session *models.TaskSession) bool { - if session.ID == "" || s.workflowStepGetter == nil { + return s.processOnTurnCompleteWithCause(ctx, task, session, turnCompletionCauseAgentTurn) +} + +func (s *Service) processOnTurnCompleteWithCause( + ctx context.Context, + task *models.Task, + session *models.TaskSession, + cause turnCompletionCause, +) bool { + if s.shouldSkipLegacyTurnCompletion(task, session, cause) { return false } taskID := task.ID sessionID := session.ID - - if task.WorkflowStepID == "" { - s.logger.Debug("task has no workflow step, skipping transition", - zap.String("session_id", sessionID)) + currentStep, ok := s.loadLegacyTurnCompletionStep(ctx, task, session) + if !ok || !s.shouldRunLegacyTurnCompletion(ctx, task, session, currentStep, cause) { return false } - workflowStepID := task.WorkflowStepID - - // Get the current workflow step - currentStep, err := s.workflowStepGetter.GetStep(ctx, workflowStepID) - if err != nil { - s.logger.Warn("failed to get workflow step for transition", - zap.String("workflow_step_id", workflowStepID), - zap.Error(err)) - return false - } - // If no on_turn_complete actions, do nothing (manual step) - if len(currentStep.Events.OnTurnComplete) == 0 { - s.logger.Debug("step has no on_turn_complete actions, waiting for user", - zap.String("step_id", currentStep.ID), - zap.String("step_name", currentStep.Name)) - s.setSessionWaitingForInput(ctx, taskID, sessionID, session) - return false - } - - if s.turnCompleteBlockedByUserInput(ctx, taskID, sessionID, session) { - return false - } - - // ADR 0015 — explicit completion signal gating (legacy path mirror of - // processOnTurnCompleteViaEngine). Steps marked - // `auto_advance_requires_signal=true` wait for a step_complete_kandev - // signal before evaluating their transition actions. - if currentStep.AutoAdvanceRequiresSignal { - signal, has := models.LoadPendingStepSignal(session.Metadata) - if !has || signal.StepID != currentStep.ID { - s.logger.Debug("on_turn_complete gated on explicit signal (legacy path)", - zap.String("step_id", currentStep.ID)) - s.setSessionWaitingForInput(ctx, taskID, sessionID, session) - return false - } - } - // Process side-effect actions first, then find the first transition action transitionAction := s.processTurnCompleteActions(ctx, session, currentStep) @@ -92,6 +69,83 @@ func (s *Service) processOnTurnComplete(ctx context.Context, task *models.Task, return true } +func (s *Service) shouldSkipLegacyTurnCompletion( + task *models.Task, + session *models.TaskSession, + cause turnCompletionCause, +) bool { + if task == nil || session == nil || session.ID == "" || s.workflowStepGetter == nil { + return true + } + if cause != turnCompletionCauseUserCancellation { + return false + } + if isTerminalSessionState(session.State) { + return true + } + if task.IsEphemeral || task.IsFromOffice || taskArchived(task) { + s.logger.Debug("skipping user cancellation completion for ineligible task", + zap.String("task_id", task.ID)) + return true + } + return false +} + +func (s *Service) loadLegacyTurnCompletionStep( + ctx context.Context, + task *models.Task, + session *models.TaskSession, +) (*wfmodels.WorkflowStep, bool) { + if task.WorkflowStepID == "" { + s.logger.Debug("task has no workflow step, skipping transition", + zap.String("session_id", session.ID)) + return nil, false + } + currentStep, err := s.workflowStepGetter.GetStep(ctx, task.WorkflowStepID) + if err != nil || currentStep == nil { + s.logger.Warn("failed to get workflow step for transition", + zap.String("workflow_step_id", task.WorkflowStepID), + zap.Error(err)) + return nil, false + } + return currentStep, true +} + +func (s *Service) shouldRunLegacyTurnCompletion( + ctx context.Context, + task *models.Task, + session *models.TaskSession, + currentStep *wfmodels.WorkflowStep, + cause turnCompletionCause, +) bool { + if len(currentStep.Events.OnTurnComplete) == 0 { + s.logger.Debug("step has no on_turn_complete actions, waiting for user", + zap.String("step_id", currentStep.ID), + zap.String("step_name", currentStep.Name)) + s.setSessionWaitingForInput(ctx, task.ID, session.ID, session) + return false + } + if cause == turnCompletionCauseUserCancellation && !currentStep.CancelTriggersTurnComplete { + s.logger.Debug("user cancellation completion is disabled for step", + zap.String("step_id", currentStep.ID), + zap.String("step_name", currentStep.Name)) + return false + } + if s.turnCompleteBlockedByUserInput(ctx, task.ID, session.ID, session) { + return false + } + if cause != turnCompletionCauseUserCancellation && currentStep.AutoAdvanceRequiresSignal { + signal, has := models.LoadPendingStepSignal(session.Metadata) + if !has || signal.StepID != currentStep.ID { + s.logger.Debug("on_turn_complete gated on explicit signal (legacy path)", + zap.String("step_id", currentStep.ID)) + s.setSessionWaitingForInput(ctx, task.ID, session.ID, session) + return false + } + } + return true +} + func (s *Service) resolveTransitionTargetStep(ctx context.Context, taskID, sessionID string, currentStep *wfmodels.WorkflowStep, action *wfmodels.OnTurnCompleteAction) (string, bool) { switch action.Type { case wfmodels.OnTurnCompleteMoveToNext: @@ -2398,6 +2452,15 @@ func assembleMachineState(task *models.Task, session *models.TaskSession, isPass // actions and drive step transitions. Falls back to the legacy method when the engine // is not initialized. Returns true if a step transition occurred. func (s *Service) processOnTurnCompleteViaEngine(ctx context.Context, taskID string, session *models.TaskSession) bool { + return s.processOnTurnCompleteViaEngineWithCause(ctx, taskID, session, turnCompletionCauseAgentTurn) +} + +func (s *Service) processOnTurnCompleteViaEngineWithCause( + ctx context.Context, + taskID string, + session *models.TaskSession, + cause turnCompletionCause, +) bool { task, err := s.repo.GetTask(ctx, taskID) if err != nil { s.logger.Warn("failed to load task for on_turn_complete", @@ -2407,68 +2470,13 @@ func (s *Service) processOnTurnCompleteViaEngine(ctx context.Context, taskID str } if s.workflowEngine == nil { - return s.processOnTurnComplete(ctx, task, session) + return s.processOnTurnCompleteWithCause(ctx, task, session, cause) } - if session.ID == "" || s.workflowStepGetter == nil { + if !s.prepareEngineTurnCompletion(ctx, taskID, task, session, cause) { return false } - if task.WorkflowStepID == "" { - s.setSessionWaitingForInput(ctx, taskID, session.ID, session) - return false - } - - // Skip workflow step actions for ephemeral tasks (quick chat) - they have no workflow - if task.IsEphemeral { - s.setSessionWaitingForInput(ctx, taskID, session.ID, session) - return false - } - - // ADR 0015 — explicit completion signal gating. Steps marked - // `auto_advance_requires_signal=true` only transition when the agent - // (or the manual fallback button) has written the pending bag entry. - // On gate-fail we set the session to WAITING_FOR_INPUT and bail — - // either the user replies (clearing the bag) or a later - // step_complete_kandev call triggers the out-of-band subscriber. - // - // Fail closed on step-load errors: a missing/broken step record must - // not silently bypass the gate (which would let a signal-required step - // auto-advance whenever the loader hiccups). Block the transition and - // let the next turn re-evaluate after the underlying error clears. - currentStep, stepErr := s.workflowStepGetter.GetStep(ctx, task.WorkflowStepID) - if stepErr != nil || currentStep == nil { - s.logger.Warn("on_turn_complete: failed to load current step for signal gating, blocking transition", - zap.String("task_id", taskID), - zap.String("session_id", session.ID), - zap.String("step_id", task.WorkflowStepID), - zap.Error(stepErr)) - s.setSessionWaitingForInput(ctx, taskID, session.ID, session) - return false - } - if s.turnCompleteBlockedByUserInput(ctx, taskID, session.ID, session) { - return false - } - if currentStep.AutoAdvanceRequiresSignal { - signal, has := models.LoadPendingStepSignal(session.Metadata) - if !has || signal.StepID != task.WorkflowStepID { - s.logger.Debug("on_turn_complete gated on explicit signal (none received yet)", - zap.String("task_id", taskID), - zap.String("session_id", session.ID), - zap.String("step_id", task.WorkflowStepID)) - s.setSessionWaitingForInput(ctx, taskID, session.ID, session) - return false - } - s.logger.Info("on_turn_complete consuming explicit signal", - zap.String("task_id", taskID), - zap.String("session_id", session.ID), - zap.String("step_id", task.WorkflowStepID), - zap.String("source", signal.Source)) - // Bag is consumed once the transition executes (in - // applyEngineTransition's stamp + clear), so don't clear here — - // otherwise a failed transition would lose the signal. - } - state := s.buildMachineState(ctx, task, session) result, err := s.workflowEngine.HandleTrigger(ctx, engine.HandleInput{ TaskID: taskID, @@ -2500,6 +2508,105 @@ func (s *Service) processOnTurnCompleteViaEngine(ctx context.Context, taskID str return s.applyEngineTransition(ctx, taskID, session, result, engine.TriggerOnTurnComplete, task.Description, true) } +func (s *Service) prepareEngineTurnCompletion( + ctx context.Context, + taskID string, + task *models.Task, + session *models.TaskSession, + cause turnCompletionCause, +) bool { + if s.shouldSkipEngineTurnCompletion(ctx, taskID, task, session, cause) { + return false + } + + // ADR 0015 — explicit completion signal gating. Steps marked + // `auto_advance_requires_signal=true` only transition when the agent + // (or the manual fallback button) has written the pending bag entry. + // On gate-fail we set the session to WAITING_FOR_INPUT and bail — + // either the user replies (clearing the bag) or a later + // step_complete_kandev call triggers the out-of-band subscriber. + // + // Fail closed on step-load errors: a missing/broken step record must + // not silently bypass the gate (which would let a signal-required step + // auto-advance whenever the loader hiccups). Block the transition and + // let the next turn re-evaluate after the underlying error clears. + currentStep, stepErr := s.workflowStepGetter.GetStep(ctx, task.WorkflowStepID) + if stepErr != nil || currentStep == nil { + s.logger.Warn("on_turn_complete: failed to load current step for signal gating, blocking transition", + zap.String("task_id", taskID), + zap.String("session_id", session.ID), + zap.String("step_id", task.WorkflowStepID), + zap.Error(stepErr)) + s.setSessionWaitingForInput(ctx, taskID, session.ID, session) + return false + } + if s.turnCompleteBlockedByUserInput(ctx, taskID, session.ID, session) { + return false + } + if cause == turnCompletionCauseUserCancellation && !currentStep.CancelTriggersTurnComplete { + s.logger.Debug("user cancellation completion is disabled for step", + zap.String("task_id", taskID), + zap.String("session_id", session.ID), + zap.String("step_id", currentStep.ID)) + return false + } + return s.allowEngineSignalCompletion(ctx, taskID, session, currentStep, cause) +} + +func (s *Service) shouldSkipEngineTurnCompletion( + ctx context.Context, + taskID string, + task *models.Task, + session *models.TaskSession, + cause turnCompletionCause, +) bool { + if session == nil || session.ID == "" || s.workflowStepGetter == nil { + return true + } + if task.WorkflowStepID == "" { + s.setSessionWaitingForInput(ctx, taskID, session.ID, session) + return true + } + // Skip workflow step actions for ephemeral tasks (quick chat) - they have no workflow. + // Explicit user cancellation also excludes Office and archived tasks: their + // lifecycle is owned by their scheduler/archive path, not Kanban completion. + if task.IsEphemeral || (cause == turnCompletionCauseUserCancellation && (task.IsFromOffice || taskArchived(task))) { + s.setSessionWaitingForInput(ctx, taskID, session.ID, session) + return true + } + return false +} + +func (s *Service) allowEngineSignalCompletion( + ctx context.Context, + taskID string, + session *models.TaskSession, + currentStep *wfmodels.WorkflowStep, + cause turnCompletionCause, +) bool { + if cause == turnCompletionCauseUserCancellation || !currentStep.AutoAdvanceRequiresSignal { + return true + } + signal, has := models.LoadPendingStepSignal(session.Metadata) + if !has || signal.StepID != currentStep.ID { + s.logger.Debug("on_turn_complete gated on explicit signal (none received yet)", + zap.String("task_id", taskID), + zap.String("session_id", session.ID), + zap.String("step_id", currentStep.ID)) + s.setSessionWaitingForInput(ctx, taskID, session.ID, session) + return false + } + s.logger.Info("on_turn_complete consuming explicit signal", + zap.String("task_id", taskID), + zap.String("session_id", session.ID), + zap.String("step_id", currentStep.ID), + zap.String("source", signal.Source)) + // Bag is consumed once the transition executes (in + // applyEngineTransition's stamp + clear), so don't clear here — + // otherwise a failed transition would lose the signal. + return true +} + // applyEngineTransition applies an engine-evaluated transition: on_exit, DB transition, // data patches, and optionally on_enter processing. Returns true if the transition was applied. func (s *Service) applyEngineTransition( diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go b/apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go index 29705c249f..57e4333e0c 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go @@ -38,6 +38,26 @@ func TestProcessOnTurnComplete(t *testing.T) { } }) + t.Run("missing step returns false", func(t *testing.T) { + repo := setupTestRepo(t) + seedSession(t, repo, "t1", "s1", "missing-step") + + // The workflow getter can return (nil, nil) for an unknown step. The + // legacy completion path must fail closed instead of dereferencing nil. + svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo()) + task, err := repo.GetTask(ctx, "t1") + if err != nil { + t.Fatalf("get task: %v", err) + } + session, err := repo.GetTaskSession(ctx, "s1") + if err != nil { + t.Fatalf("get session: %v", err) + } + if got := svc.processOnTurnComplete(ctx, task, session); got { + t.Fatal("expected false when workflow step lookup returns nil") + } + }) + t.Run("no actions returns false", func(t *testing.T) { repo := setupTestRepo(t) seedSession(t, repo, "t1", "s1", "step1") diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index b172dcde9d..e3dd1e4c27 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -1230,13 +1230,26 @@ func (s *Service) completeTurnForSession(ctx context.Context, sessionID string) } func (s *Service) completeTurnForTaskSession(ctx context.Context, taskID, sessionID string) { + if err := s.completeTurnForTaskSessionChecked(ctx, taskID, sessionID); err != nil { + s.logger.Warn("failed to reconcile active turn", + zap.String("session_id", sessionID), + zap.Error(err)) + } +} + +// completeTurnForTaskSessionChecked closes every open turn for a session and +// returns an error unless the turn service confirms that none remain. The +// regular lifecycle paths use completeTurnForTaskSession, which preserves +// their best-effort behavior; cancellation uses this checked variant because +// workflow completion must not run while the cancelled turn is still open. +func (s *Service) completeTurnForTaskSessionChecked(ctx context.Context, taskID, sessionID string) error { // Foreground ownership ends with the turn, but detached background work can // outlive it. Preserve those registrations for accounting; full cleanup // belongs to execution/session teardown paths. s.yieldForegroundAndPublish(ctx, taskID, sessionID, foregroundYieldTurnCompletion) if s.turnService == nil { - return + return nil } s.activeTurns.Delete(sessionID) @@ -1246,34 +1259,29 @@ func (s *Service) completeTurnForTaskSession(ctx context.Context, taskID, sessio for closed < maxIterations { turn, err := s.turnService.GetActiveTurn(ctx, sessionID) if err != nil { - s.logger.Warn("failed to look up active turn", - zap.String("session_id", sessionID), - zap.Error(err)) - return + return fmt.Errorf("look up active turn: %w", err) } if turn == nil { - return + return nil } if err := s.turnService.CompleteTurn(ctx, turn.ID); err != nil { // GetActiveTurn returns the latest open turn — retrying here // would just hit the same row and loop. Bail; the next // completeTurnForSession call will pick it up. - s.logger.Warn("failed to complete turn; will retry on next sweep", - zap.String("session_id", sessionID), - zap.String("turn_id", turn.ID), - zap.Error(err)) - return + return fmt.Errorf("complete turn %s: %w", turn.ID, err) } closed++ } - // Only warn if turns are *still* accumulating after the cap. Closing - // exactly maxIterations turns and then finding the session clean is not a - // runaway. - if turn, err := s.turnService.GetActiveTurn(ctx, sessionID); err == nil && turn != nil { - s.logger.Warn("completeTurnForSession iteration cap hit; possible turn close loop", - zap.String("session_id", sessionID), - zap.Int("max_iterations", maxIterations)) + // Verify after the cap. Closing exactly maxIterations turns and then + // finding the session clean is a successful reconciliation, not a runaway. + turn, err := s.turnService.GetActiveTurn(ctx, sessionID) + if err != nil { + return fmt.Errorf("verify active turn closure: %w", err) } + if turn != nil { + return fmt.Errorf("active turn %s remains after %d closure attempts", turn.ID, maxIterations) + } + return nil } // completeTurnIfCurrent closes turnID only when it is still sessionID's active diff --git a/apps/backend/internal/orchestrator/task_operations.go b/apps/backend/internal/orchestrator/task_operations.go index a3017c7a96..994382812f 100644 --- a/apps/backend/internal/orchestrator/task_operations.go +++ b/apps/backend/internal/orchestrator/task_operations.go @@ -212,7 +212,6 @@ func (s *Service) PrepareTaskSession(ctx context.Context, taskID string, agentPr zap.Error(err)) return "", err } - // Resolve agent/executor profile from task metadata if not explicitly provided if agentProfileID == "" { if v, ok := task.Metadata[models.MetaKeyAgentProfileID].(string); ok && v != "" { @@ -4034,6 +4033,120 @@ func (s *Service) isCancelInFlight(sessionID string) bool { return s.cancelOperations[sessionID] > 0 } +// reconcileCancelledTurn durably settles the cancelled session and its turn. +// A user cancellation may trigger workflow completion only after both pieces +// of bookkeeping have been confirmed from their authoritative stores. The +// best-effort lifecycle helpers intentionally remain unchanged for other event +// paths; this path fails closed so a persistence or turn-service failure cannot +// advance the task while the cancelled turn is still running. +func (s *Service) reconcileCancelledTurn( + ctx context.Context, + taskID string, + sessionID string, + session *models.TaskSession, + requireWaiting bool, +) (*models.TaskSession, error) { + authoritativeSession, err := s.reconcileCancelledSessionState(ctx, taskID, session, requireWaiting) + if err != nil { + return nil, err + } + if err := s.completeTurnForTaskSessionChecked(ctx, taskID, sessionID); err != nil { + return nil, fmt.Errorf("settle cancelled turn: %w", err) + } + + if session == nil { + return nil, nil + } + if err := validateCancelledSessionState(session.ID, authoritativeSession, requireWaiting); err != nil { + return nil, err + } + if err := s.verifyCancelledTurnClosed(ctx, sessionID); err != nil { + return nil, err + } + + return authoritativeSession, nil +} + +func (s *Service) reconcileCancelledSessionState( + ctx context.Context, + taskID string, + session *models.TaskSession, + requireWaiting bool, +) (*models.TaskSession, error) { + if session == nil || !requireWaiting { + return session, nil + } + updated := s.updateTaskSessionState( + ctx, + taskID, + session.ID, + models.TaskSessionStateWaitingForInput, + "", + true, + session, + ) + if updated == nil { + return nil, fmt.Errorf("persist cancelled session %s as WAITING_FOR_INPUT", session.ID) + } + if updated.State != models.TaskSessionStateWaitingForInput { + return nil, fmt.Errorf( + "cancelled session %s persisted as %s, want WAITING_FOR_INPUT", + session.ID, + updated.State, + ) + } + authoritative, err := s.repo.GetTaskSession(ctx, session.ID) + if err != nil { + return nil, fmt.Errorf("verify cancelled session %s state: %w", session.ID, err) + } + if authoritative == nil { + return nil, fmt.Errorf( + "cancelled session %s is missing before turn settlement, want WAITING_FOR_INPUT", + session.ID, + ) + } + if authoritative.State != models.TaskSessionStateWaitingForInput { + return nil, fmt.Errorf( + "cancelled session %s is %s before turn settlement, want WAITING_FOR_INPUT", + session.ID, + authoritative.State, + ) + } + return authoritative, nil +} + +func validateCancelledSessionState( + sessionID string, + authoritativeSession *models.TaskSession, + requireWaiting bool, +) error { + if authoritativeSession == nil { + return fmt.Errorf("cancelled session %s has no authoritative state", sessionID) + } + if requireWaiting && authoritativeSession.State != models.TaskSessionStateWaitingForInput { + return fmt.Errorf( + "cancelled session %s settled as %s, want WAITING_FOR_INPUT", + sessionID, + authoritativeSession.State, + ) + } + return nil +} + +func (s *Service) verifyCancelledTurnClosed(ctx context.Context, sessionID string) error { + if s.turnService == nil { + return nil + } + activeTurn, err := s.turnService.GetActiveTurn(ctx, sessionID) + if err != nil { + return fmt.Errorf("verify cancelled turn closure: %w", err) + } + if activeTurn != nil { + return fmt.Errorf("cancelled turn %s remains open", activeTurn.ID) + } + return nil +} + // CancelAgent interrupts the current agent turn without terminating the process, // allowing the user to send a new prompt. // @@ -4081,6 +4194,23 @@ func (s *Service) CancelAgent(ctx context.Context, sessionID string) error { zap.String("session_id", sessionID), zap.Error(err)) } + cancelTurnCompletionEligible := false + if session != nil { + switch session.State { + case models.TaskSessionStateRunning, models.TaskSessionStateStarting: + cancelTurnCompletionEligible = true + case models.TaskSessionStateWaitingForInput: + // A previous cancellation may have settled the session before a + // transient turn-close failure. Keep that retry eligible only while + // an active turn is still present; a settled second cancel must not + // re-run the source step's completion actions. + activeTurnID, turnErr := s.peekActiveTurnID(ctx, sessionID) + if turnErr != nil { + return fmt.Errorf("inspect active turn before cancel retry: %w", turnErr) + } + cancelTurnCompletionEligible = activeTurnID != "" + } + } // Capture the active turn before cancelling so the cancel message attaches // to the turn the user was actually cancelling. If we waited until after @@ -4114,13 +4244,20 @@ func (s *Service) CancelAgent(ctx context.Context, sessionID string) error { } } - // Transition session to WAITING_FOR_INPUT so the user can send a new - // prompt, and reconcile the task row to REVIEW so the sidebar shows the - // green check rather than the yellow "needs input" question icon — a - // cancelled turn is treated as finished work the user may want to review. + // Transition the session to WAITING_FOR_INPUT and close every open turn. + // Both writes are checked against their authoritative stores before any + // workflow completion is evaluated; a failure leaves the task on its + // current step so a retry can reconcile it safely. if session != nil { - s.updateTaskSessionState(ctx, session.TaskID, sessionID, models.TaskSessionStateWaitingForInput, "", true, session) - s.writeTaskReviewStateOnCancel(ctx, session.TaskID, sessionID) + reconciled, err := s.reconcileCancelledTurn(ctx, session.TaskID, sessionID, session, cancelTurnCompletionEligible) + if err != nil { + return fmt.Errorf("reconcile cancelled turn: %w", err) + } + if reconciled != nil { + session = reconciled + } + } else if _, err := s.reconcileCancelledTurn(ctx, "", sessionID, nil, false); err != nil { + return fmt.Errorf("reconcile cancelled turn: %w", err) } // Record cancellation in the message history @@ -4145,9 +4282,23 @@ func (s *Service) CancelAgent(ctx context.Context, sessionID string) error { } } - // Complete the turn since the agent was cancelled. Idempotent w.r.t. a - // concurrent agent.complete event having already closed the turn. - s.completeTurnForSession(ctx, sessionID) + // A configured explicit user cancellation is a human completion decision. + // Evaluate it only after the cancellation message and turn settlement are + // durable, while the per-session guard still excludes a late agent.ready + // event from observing a successor turn and transitioning a second time. + if session != nil && cancelTurnCompletionEligible { + s.processOnTurnCompleteViaEngineWithCause(ctx, session.TaskID, session, turnCompletionCauseUserCancellation) + // Disabled/no-transition outcomes and successful nonterminal transitions + // both reconcile the source task to REVIEW after session and turn + // settlement. The guarded state write is a no-op for terminal tasks or a + // destination that has already restarted work. + s.reconcileCancelledTaskReview(ctx, session.TaskID, session.ID) + } else if session != nil { + // Preserve the existing review reconciliation for cancellations that + // are not eligible to run workflow completion (already waiting or a + // terminal/ineligible session). + s.reconcileCancelledTaskReview(ctx, session.TaskID, session.ID) + } // Release the cancel/interrupt guard now that this session's cancel is // fully resolved. Do not drain here: a user cancellation must not itself @@ -4161,6 +4312,17 @@ func (s *Service) CancelAgent(ctx context.Context, sessionID string) error { return nil } +func (s *Service) reconcileCancelledTaskReview(ctx context.Context, taskID, sessionID string) { + task, err := s.repo.GetTask(ctx, taskID) + if err == nil && task != nil && task.WorkflowStepID != "" && s.workflowStepGetter != nil { + step, stepErr := s.workflowStepGetter.GetStep(ctx, task.WorkflowStepID) + if stepErr == nil && step != nil && s.workflowStepIsTerminal(ctx, step.ID) { + return + } + } + s.writeTaskReviewState(ctx, taskID, sessionID) +} + // takeAndDispatchEntryLocked takes entryID from sessionID's queue and // dispatches it, falling back to draining the FIFO head only if the // targeted take doesn't find it (a benign not-found, e.g. already taken by diff --git a/apps/backend/internal/orchestrator/task_operations_test.go b/apps/backend/internal/orchestrator/task_operations_test.go index 5400fac5ee..c8667fee7d 100644 --- a/apps/backend/internal/orchestrator/task_operations_test.go +++ b/apps/backend/internal/orchestrator/task_operations_test.go @@ -741,6 +741,395 @@ func TestPromptTask_ResetInProgressReturnsSentinelError(t *testing.T) { // --- CancelAgent --- +func cancelCompletionStepGetter(enabled, signalRequired bool) *mockStepGetter { + getter := newMockStepGetter() + getter.steps["step1"] = &wfmodels.WorkflowStep{ + ID: "step1", + WorkflowID: "wf1", + Name: "Work", + Position: 0, + AutoAdvanceRequiresSignal: signalRequired, + CancelTriggersTurnComplete: enabled, + Events: wfmodels.StepEvents{OnTurnComplete: []wfmodels.OnTurnCompleteAction{{ + Type: wfmodels.OnTurnCompleteMoveToNext, + }}}, + } + getter.steps["step2"] = &wfmodels.WorkflowStep{ + ID: "step2", + WorkflowID: "wf1", + Name: "Review", + Position: 1, + Events: wfmodels.StepEvents{OnTurnComplete: []wfmodels.OnTurnCompleteAction{{ + Type: wfmodels.OnTurnCompleteMoveToNext, + }}}, + } + getter.steps["step3"] = &wfmodels.WorkflowStep{ + ID: "step3", + WorkflowID: "wf1", + Name: "Done", + Position: 2, + } + return getter +} + +func TestCancelAgent_TriggersConfiguredTurnComplete(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-cancel-enabled", "session-cancel-enabled", "step1") + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + + require.NoError(t, svc.CancelAgent(ctx, "session-cancel-enabled")) + task, err := repo.GetTask(ctx, "task-cancel-enabled") + require.NoError(t, err) + assert.Equal(t, "step2", task.WorkflowStepID) + session, err := repo.GetTaskSession(ctx, "session-cancel-enabled") + require.NoError(t, err) + assert.Equal(t, models.TaskSessionStateWaitingForInput, session.State) + + // A retry after the first cancel must not evaluate the destination as a + // second completion transition. + require.NoError(t, svc.CancelAgent(ctx, "session-cancel-enabled")) + task, err = repo.GetTask(ctx, "task-cancel-enabled") + require.NoError(t, err) + assert.Equal(t, "step2", task.WorkflowStepID) +} + +func TestCancelAgent_KeepsDisabledStep(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-cancel-disabled", "session-cancel-disabled", "step1") + svc := createEngineService(t, repo, cancelCompletionStepGetter(false, false), &mockAgentManager{}) + + require.NoError(t, svc.CancelAgent(ctx, "session-cancel-disabled")) + task, err := repo.GetTask(ctx, "task-cancel-disabled") + require.NoError(t, err) + assert.Equal(t, "step1", task.WorkflowStepID) +} + +func TestCancelAgent_BypassesAgentSignal(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-cancel-signal", "session-cancel-signal", "step1") + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, true), &mockAgentManager{}) + + // No pending step-completion signal is written: explicit user cancellation + // is allowed to bypass only this agent-owned gate. + require.NoError(t, svc.CancelAgent(ctx, "session-cancel-signal")) + task, err := repo.GetTask(ctx, "task-cancel-signal") + require.NoError(t, err) + assert.Equal(t, "step2", task.WorkflowStepID) +} + +func TestCancelAgent_BlocksPendingClarification(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-cancel-clarification", "session-cancel-clarification", "step1") + seedPendingClarificationMessage(t, repo, "task-cancel-clarification", "session-cancel-clarification") + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + + require.NoError(t, svc.CancelAgent(ctx, "session-cancel-clarification")) + task, err := repo.GetTask(ctx, "task-cancel-clarification") + require.NoError(t, err) + assert.Equal(t, "step1", task.WorkflowStepID) +} + +func TestCancelAgent_SkipsIneligibleTask(t *testing.T) { + cases := []struct{ name string }{ + {name: "office"}, + {name: "ephemeral"}, + {name: "archived"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + taskID := "task-cancel-ineligible-" + tc.name + sessionID := "session-cancel-ineligible-" + tc.name + seedSession(t, repo, taskID, sessionID, "step1") + switch tc.name { + case "office": + _, err := repo.DB().Exec(`UPDATE workspaces SET office_workflow_id = 'wf1' WHERE id = 'ws1'`) + require.NoError(t, err) + case "ephemeral": + _, err := repo.DB().Exec(`UPDATE tasks SET is_ephemeral = 1 WHERE id = ?`, taskID) + require.NoError(t, err) + case "archived": + _, err := repo.DB().Exec(`UPDATE tasks SET archived_at = ? WHERE id = ?`, time.Now().UTC(), taskID) + require.NoError(t, err) + } + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + + require.NoError(t, svc.CancelAgent(ctx, sessionID)) + updated, err := repo.GetTask(ctx, taskID) + require.NoError(t, err) + assert.Equal(t, "step1", updated.WorkflowStepID) + }) + } +} + +func TestCancelAgent_HandlesReconciledRuntime(t *testing.T) { + for _, cancelErr := range []error{nil, lifecycle.ErrCancelEscalated, lifecycle.ErrNoExecutionForSession} { + t.Run(fmt.Sprintf("%v", cancelErr), func(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-cancel-reconcile", "session-cancel-reconcile", "step1") + agentMgr := &mockAgentManager{cancelAgentErr: cancelErr} + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), agentMgr) + + require.NoError(t, svc.CancelAgent(ctx, "session-cancel-reconcile")) + updated, err := repo.GetTask(ctx, "task-cancel-reconcile") + require.NoError(t, err) + assert.Equal(t, "step2", updated.WorkflowStepID) + }) + } +} + +type cancelTurnFailureService struct { + *repoTurnService + completeErr error + activeErr error +} + +func (s *cancelTurnFailureService) CompleteTurn(ctx context.Context, turnID string) error { + if s.completeErr != nil { + return s.completeErr + } + return s.repoTurnService.CompleteTurn(ctx, turnID) +} + +func (s *cancelTurnFailureService) GetActiveTurn(ctx context.Context, sessionID string) (*models.Turn, error) { + if s.activeErr != nil { + return nil, s.activeErr + } + return s.repoTurnService.GetActiveTurn(ctx, sessionID) +} + +type cancelContextAgentManager struct { + *mockAgentManager + cancel context.CancelFunc +} + +func (m *cancelContextAgentManager) CancelAgent(ctx context.Context, sessionID string) error { + err := m.mockAgentManager.CancelAgent(ctx, sessionID) + m.cancel() + return err +} + +func TestCancelAgent_DoesNotTransitionWhenTurnClosureFails(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + taskID := "task-cancel-turn-failure" + sessionID := "session-cancel-turn-failure" + seedSession(t, repo, taskID, sessionID, "step1") + now := time.Now().UTC() + require.NoError(t, repo.CreateTurn(ctx, &models.Turn{ + ID: "turn-cancel-turn-failure", + TaskID: taskID, + TaskSessionID: sessionID, + StartedAt: now, + CreatedAt: now, + UpdatedAt: now, + })) + + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + turnService := &cancelTurnFailureService{ + repoTurnService: &repoTurnService{repo: repo}, + completeErr: errors.New("turn close failed"), + } + svc.turnService = turnService + + err := svc.CancelAgent(ctx, sessionID) + require.Error(t, err) + assert.Contains(t, err.Error(), "turn close failed") + + task, err := repo.GetTask(ctx, taskID) + require.NoError(t, err) + assert.Equal(t, "step1", task.WorkflowStepID, "failed turn closure must block workflow completion") + session, err := repo.GetTaskSession(ctx, sessionID) + require.NoError(t, err) + assert.Equal(t, models.TaskSessionStateWaitingForInput, session.State) + activeTurn, err := svc.turnService.GetActiveTurn(ctx, sessionID) + require.NoError(t, err) + assert.NotNil(t, activeTurn, "the failed close must remain observable for retry") + + turnService.completeErr = nil + require.NoError(t, svc.CancelAgent(ctx, sessionID)) + task, err = repo.GetTask(ctx, taskID) + require.NoError(t, err) + assert.Equal(t, "step2", task.WorkflowStepID, "a later cancel retries the now-settled completion") +} + +func TestCancelAgent_DoesNotTransitionWhenActiveTurnInspectionFails(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + taskID := "task-cancel-active-turn-inspection-failure" + sessionID := "session-cancel-active-turn-inspection-failure" + seedSession(t, repo, taskID, sessionID, "step1") + require.NoError(t, repo.UpdateTaskSessionState(ctx, sessionID, models.TaskSessionStateWaitingForInput, "")) + now := time.Now().UTC() + require.NoError(t, repo.CreateTurn(ctx, &models.Turn{ + ID: "turn-cancel-active-turn-inspection-failure", + TaskID: taskID, + TaskSessionID: sessionID, + StartedAt: now, + CreatedAt: now, + UpdatedAt: now, + })) + + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + turnService := &cancelTurnFailureService{ + repoTurnService: &repoTurnService{repo: repo}, + activeErr: errors.New("active turn lookup failed"), + } + svc.turnService = turnService + + err := svc.CancelAgent(ctx, sessionID) + require.Error(t, err) + assert.Contains(t, err.Error(), "active turn lookup failed") + task, err := repo.GetTask(ctx, taskID) + require.NoError(t, err) + assert.Equal(t, "step1", task.WorkflowStepID, "a failed retry inspection must keep completion pending") + activeTurn, err := turnService.repoTurnService.GetActiveTurn(ctx, sessionID) + require.NoError(t, err) + assert.NotNil(t, activeTurn, "a failed retry inspection must not close the active turn") + + turnService.activeErr = nil + require.NoError(t, svc.CancelAgent(ctx, sessionID)) + task, err = repo.GetTask(ctx, taskID) + require.NoError(t, err) + assert.Equal(t, "step2", task.WorkflowStepID, "a later retry should evaluate the settled cancellation") +} + +func TestCancelAgent_NonterminalTransitionReconcilesReviewState(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + taskID := "task-cancel-review-state" + sessionID := "session-cancel-review-state" + seedSession(t, repo, taskID, sessionID, "step1") + taskRepo := newMockTaskRepo() + seedMockTaskState(taskRepo, taskID, v1.TaskStateInProgress) + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + svc.taskRepo = taskRepo + + require.NoError(t, svc.CancelAgent(ctx, sessionID)) + task, err := repo.GetTask(ctx, taskID) + require.NoError(t, err) + assert.Equal(t, "step2", task.WorkflowStepID) + assert.Contains(t, taskRepo.stateHistory[taskID], v1.TaskStateReview, + "a successful nonterminal cancellation transition must settle the task into REVIEW") +} + +func TestReconcileCancelledTurn_DoesNotCloseTurnWhenSessionStateWriteFails(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + taskID := "task-cancel-session-failure" + sessionID := "session-cancel-session-failure" + seedSession(t, repo, taskID, sessionID, "step1") + now := time.Now().UTC() + require.NoError(t, repo.CreateTurn(ctx, &models.Turn{ + ID: "turn-cancel-session-failure", + TaskID: taskID, + TaskSessionID: sessionID, + StartedAt: now, + CreatedAt: now, + UpdatedAt: now, + })) + + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + svc.turnService = &repoTurnService{repo: repo} + session, err := repo.GetTaskSession(ctx, sessionID) + require.NoError(t, err) + failedCtx, cancel := context.WithCancel(ctx) + cancel() + + _, err = svc.reconcileCancelledTurn(failedCtx, taskID, sessionID, session, true) + require.Error(t, err) + + settled, err := repo.GetTaskSession(ctx, sessionID) + require.NoError(t, err) + assert.Equal(t, models.TaskSessionStateRunning, settled.State) + activeTurn, err := svc.turnService.GetActiveTurn(ctx, sessionID) + require.NoError(t, err) + assert.NotNil(t, activeTurn, "a failed session-state write must not close the active turn") +} + +func TestCancelAgent_DoesNotTransitionWhenSessionStateWriteFails(t *testing.T) { + repo := setupTestRepo(t) + taskID := "task-cancel-session-write" + sessionID := "session-cancel-session-write" + seedSession(t, repo, taskID, sessionID, "step1") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + manager := &cancelContextAgentManager{ + mockAgentManager: &mockAgentManager{}, + cancel: cancel, + } + svc := createTestServiceWithAgent(repo, cancelCompletionStepGetter(true, false), newMockTaskRepo(), manager) + + err := svc.CancelAgent(ctx, sessionID) + require.Error(t, err) + task, err := repo.GetTask(context.Background(), taskID) + require.NoError(t, err) + assert.Equal(t, "step1", task.WorkflowStepID) + session, err := repo.GetTaskSession(context.Background(), sessionID) + require.NoError(t, err) + assert.Equal(t, models.TaskSessionStateRunning, session.State) +} + +func TestCancelAgent_TerminalTransitionSkipsIntermediateReview(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + taskID := "task-cancel-terminal" + sessionID := "session-cancel-terminal" + seedSession(t, repo, taskID, sessionID, "step1") + steps := cancelCompletionStepGetter(true, false) + steps.steps["step2"].Name = "Done" + delete(steps.steps, "step3") + + taskRepo := newMockTaskRepo() + seedMockTaskState(taskRepo, taskID, v1.TaskStateInProgress) + svc := createEngineService(t, repo, steps, &mockAgentManager{}) + svc.taskRepo = taskRepo + + require.NoError(t, svc.CancelAgent(ctx, sessionID)) + task, err := repo.GetTask(ctx, taskID) + require.NoError(t, err) + assert.Equal(t, v1.TaskStateCompleted, task.State) + assert.NotContains(t, taskRepo.stateHistory[taskID], v1.TaskStateReview, + "terminal cancellation must let the workflow transition own the final state") +} + +func TestCancelAgent_CannotDoubleTransitionFromStaleReady(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-cancel-stale-ready", "session-cancel-stale-ready", "step1") + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + + require.NoError(t, svc.CancelAgent(ctx, "session-cancel-stale-ready")) + // The late ready event from the cancelled execution must observe the + // reconciled WAITING_FOR_INPUT session and return before any second + // on_turn_complete evaluation. + svc.handleAgentReady(ctx, watcher.AgentEventData{ + TaskID: "task-cancel-stale-ready", + SessionID: "session-cancel-stale-ready", + }) + task, err := repo.GetTask(ctx, "task-cancel-stale-ready") + require.NoError(t, err) + assert.Equal(t, "step2", task.WorkflowStepID) +} + +func TestUserCancelCompletion_SilentCancelDoesNotTrigger(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-silent-cancel", "session-silent-cancel", "step1") + svc := createEngineService(t, repo, cancelCompletionStepGetter(true, false), &mockAgentManager{}) + + require.NoError(t, svc.cancelAgentSilent(ctx, "task-silent-cancel", "session-silent-cancel")) + task, err := repo.GetTask(ctx, "task-silent-cancel") + require.NoError(t, err) + assert.Equal(t, "step1", task.WorkflowStepID) +} + // TestCancelAgent_DeduplicatesConcurrentCalls covers the impatient-user case: // the UI's cancel button has no in-flight disable, so users click it multiple // times while the agent is still tearing down a slow turn (e.g. a Claude diff --git a/apps/backend/internal/task/dto/converters.go b/apps/backend/internal/task/dto/converters.go index ad433c0d38..9cb9d4762b 100644 --- a/apps/backend/internal/task/dto/converters.go +++ b/apps/backend/internal/task/dto/converters.go @@ -9,20 +9,22 @@ import ( // This is the base conversion without timestamps. func FromWorkflowStep(step *wfmodels.WorkflowStep) WorkflowStepDTO { result := WorkflowStepDTO{ - ID: step.ID, - WorkflowID: step.WorkflowID, - Name: step.Name, - Position: step.Position, - Color: step.Color, - Prompt: step.Prompt, - AllowManualMove: step.AllowManualMove, - IsStartStep: step.IsStartStep, - ShowInCommandPanel: step.ShowInCommandPanel, - AutoArchiveAfterHours: step.AutoArchiveAfterHours, - AgentProfileID: step.AgentProfileID, - WIPLimit: step.WIPLimit, - PullFromStepID: step.PullFromStepID, - StageType: string(step.StageType), + ID: step.ID, + WorkflowID: step.WorkflowID, + Name: step.Name, + Position: step.Position, + Color: step.Color, + Prompt: step.Prompt, + AllowManualMove: step.AllowManualMove, + IsStartStep: step.IsStartStep, + ShowInCommandPanel: step.ShowInCommandPanel, + AutoArchiveAfterHours: step.AutoArchiveAfterHours, + AgentProfileID: step.AgentProfileID, + WIPLimit: step.WIPLimit, + PullFromStepID: step.PullFromStepID, + StageType: string(step.StageType), + AutoAdvanceRequiresSignal: step.AutoAdvanceRequiresSignal, + CancelTriggersTurnComplete: step.CancelTriggersTurnComplete, } if hasStepEvents(step.Events) { events := &StepEventsDTO{} diff --git a/apps/backend/internal/task/dto/converters_test.go b/apps/backend/internal/task/dto/converters_test.go index a4ffb47544..c72ae8b5c6 100644 --- a/apps/backend/internal/task/dto/converters_test.go +++ b/apps/backend/internal/task/dto/converters_test.go @@ -81,6 +81,25 @@ func TestFromWorkflowStep_PreservesWIPFields(t *testing.T) { } } +func TestFromWorkflowStep_PreservesCancelTriggersTurnComplete(t *testing.T) { + got := FromWorkflowStep(&wfmodels.WorkflowStep{ + ID: "step-cancel", + WorkflowID: "wf-1", + CancelTriggersTurnComplete: true, + }) + payload, err := json.Marshal(got) + if err != nil { + t.Fatalf("marshal workflow step DTO: %v", err) + } + var fields map[string]any + if err := json.Unmarshal(payload, &fields); err != nil { + t.Fatalf("decode workflow step DTO: %v", err) + } + if gotValue, ok := fields["cancel_triggers_turn_complete"].(bool); !ok || !gotValue { + t.Fatalf("cancel trigger DTO field = %#v, want true", fields["cancel_triggers_turn_complete"]) + } +} + func TestFromTaskSession_IncludesAllWorktrees(t *testing.T) { session := &models.TaskSession{ ID: "session-1", diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index 45e726fd08..1905469dbc 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -872,9 +872,11 @@ type WorkflowStepDTO struct { PullFromStepID string `json:"pull_from_step_id,omitempty"` // StageType is a Phase 2 (ADR-0004) semantic hint for the frontend. // Allowed values: "work" | "review" | "approval" | "custom". - StageType string `json:"stage_type,omitempty"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + StageType string `json:"stage_type,omitempty"` + AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal"` + CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } // StepEventsDTO represents step events for API responses diff --git a/apps/backend/internal/task/repository/sqlite/base_migrations.go b/apps/backend/internal/task/repository/sqlite/base_migrations.go index 5b733621c4..9526f4d2f2 100644 --- a/apps/backend/internal/task/repository/sqlite/base_migrations.go +++ b/apps/backend/internal/task/repository/sqlite/base_migrations.go @@ -192,6 +192,11 @@ func (r *Repository) runMigrations() error { // fail. Required for tests and any environment where the workflow // repo hasn't run yet. r.ensureRunnerProjectionTables() + // Keep the projection table compatible with databases whose workflow + // repository has not replayed its own migrations yet. These additive + // migrations are idempotent and preserve the false default for legacy rows. + r.migrate.Apply("workflow_steps.auto_advance_requires_signal", `ALTER TABLE workflow_steps ADD COLUMN auto_advance_requires_signal INTEGER NOT NULL DEFAULT 0`) + r.migrate.Apply("workflow_steps.cancel_triggers_turn_complete", `ALTER TABLE workflow_steps ADD COLUMN cancel_triggers_turn_complete INTEGER NOT NULL DEFAULT 0`) // Slack-style unread divider: the read cursor a session advances to the // latest message id whenever it becomes the visible chat panel. The diff --git a/apps/backend/internal/task/repository/sqlite/base_schema.go b/apps/backend/internal/task/repository/sqlite/base_schema.go index e730f23e3b..6febcf56ba 100644 --- a/apps/backend/internal/task/repository/sqlite/base_schema.go +++ b/apps/backend/internal/task/repository/sqlite/base_schema.go @@ -137,6 +137,8 @@ func (r *Repository) ensureRunnerProjectionTables() { auto_archive_after_hours INTEGER DEFAULT 0, agent_profile_id TEXT NOT NULL DEFAULT '', stage_type TEXT NOT NULL DEFAULT 'custom', + auto_advance_requires_signal INTEGER NOT NULL DEFAULT 0, + cancel_triggers_turn_complete INTEGER NOT NULL DEFAULT 0, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP )`) diff --git a/apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go b/apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go index 9af9ce8811..c46ddcb5a7 100644 --- a/apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go +++ b/apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go @@ -10,6 +10,7 @@ import ( "github.com/jmoiron/sqlx" "github.com/kandev/kandev/internal/db" + taskmodels "github.com/kandev/kandev/internal/task/models" wfmodels "github.com/kandev/kandev/internal/workflow/models" workflowrepo "github.com/kandev/kandev/internal/workflow/repository" ) @@ -129,6 +130,45 @@ func TestEnsureOfficeDefaultWorkflow_CreatesFiveSteps(t *testing.T) { } } +func TestCreateWorkspaceWithKanban_CancelTriggersTurnCompleteDefaults(t *testing.T) { + repo := newRepoForBuiltinWorkflowTests(t) + ctx := context.Background() + workflow, err := repo.CreateWorkspaceWithKanban(ctx, &taskmodels.Workspace{ID: "ws-kanban-cancel", Name: "Kanban Cancel"}) + if err != nil { + t.Fatalf("create Kanban workspace: %v", err) + } + + rows, err := repo.db.QueryContext(ctx, repo.db.Rebind(` + SELECT name, cancel_triggers_turn_complete + FROM workflow_steps WHERE workflow_id = ? ORDER BY position + `), workflow.ID) + if err != nil { + t.Fatalf("query Kanban flags: %v", err) + } + defer func() { _ = rows.Close() }() + want := map[string]bool{"Backlog": true, "In Progress": true, "Review": false, "Done": false} + seen := 0 + for rows.Next() { + var name string + var enabled int + if err := rows.Scan(&name, &enabled); err != nil { + t.Fatalf("scan Kanban flag: %v", err) + } + if wantValue, ok := want[name]; ok { + if got := enabled == 1; got != wantValue { + t.Errorf("Kanban step %q cancel trigger = %t, want %t", name, got, wantValue) + } + seen++ + } + } + if err := rows.Err(); err != nil { + t.Fatalf("iterate Kanban flags: %v", err) + } + if seen != len(want) { + t.Fatalf("observed %d Kanban steps with cancel flags, want %d", seen, len(want)) + } +} + func TestEnsureOfficeDefaultWorkflow_TriggersJSONShape(t *testing.T) { repo := newRepoForBuiltinWorkflowTests(t) ctx := context.Background() diff --git a/apps/backend/internal/task/repository/sqlite/defaults.go b/apps/backend/internal/task/repository/sqlite/defaults.go index da49832a54..5a99215075 100644 --- a/apps/backend/internal/task/repository/sqlite/defaults.go +++ b/apps/backend/internal/task/repository/sqlite/defaults.go @@ -445,13 +445,15 @@ func (r *Repository) insertTemplateStep( id, workflow_id, name, position, color, prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, stage_type, + auto_advance_requires_signal, cancel_triggers_turn_complete, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `), idMap[def.ID], workflowID, def.Name, def.Position, def.Color, def.Prompt, string(eventsJSON), dialect.BoolToInt(def.AllowManualMove), dialect.BoolToInt(def.IsStartStep), dialect.BoolToInt(def.ShowInCommandPanel), def.AutoArchiveAfterHours, def.AgentProfileID, string(stage), + dialect.BoolToInt(def.AutoAdvanceRequiresSignal), dialect.BoolToInt(def.CancelTriggersTurnComplete), now, now, ) if err != nil { diff --git a/apps/backend/internal/task/repository/sqlite/repository_entity_test.go b/apps/backend/internal/task/repository/sqlite/repository_entity_test.go index 07931b68da..143dea790a 100644 --- a/apps/backend/internal/task/repository/sqlite/repository_entity_test.go +++ b/apps/backend/internal/task/repository/sqlite/repository_entity_test.go @@ -488,3 +488,46 @@ func TestRunMigrations_Idempotent(t *testing.T) { t.Fatalf("third runMigrations call returned error: %v", err) } } + +func TestRunnerProjectionWorkflowStepColumnsReplayMigration(t *testing.T) { + repo := newRepoForEntityTests(t) + if _, err := repo.db.Exec(`INSERT INTO workflow_steps (id, workflow_id, name, position) VALUES ('legacy-projection-step', 'legacy-workflow', 'Legacy', 0)`); err != nil { + t.Fatalf("seed legacy workflow step: %v", err) + } + + legacyColumns := []struct { + name string + sql string + }{ + {name: "auto_advance_requires_signal", sql: `ALTER TABLE workflow_steps DROP COLUMN auto_advance_requires_signal`}, + {name: "cancel_triggers_turn_complete", sql: `ALTER TABLE workflow_steps DROP COLUMN cancel_triggers_turn_complete`}, + } + for _, column := range legacyColumns { + if _, err := repo.db.Exec(column.sql); err != nil { + t.Fatalf("drop legacy workflow_steps.%s: %v", column.name, err) + } + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("runMigrations on legacy workflow_steps schema: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("replay runMigrations: %v", err) + } + + for _, column := range []string{"auto_advance_requires_signal", "cancel_triggers_turn_complete"} { + var count int + if err := repo.db.QueryRow(`SELECT COUNT(*) FROM pragma_table_info('workflow_steps') WHERE name = ?`, column).Scan(&count); err != nil { + t.Fatalf("inspect workflow_steps.%s: %v", column, err) + } + if count != 1 { + t.Fatalf("workflow_steps.%s column count = %d, want 1", column, count) + } + } + var autoAdvance, cancelComplete int + if err := repo.db.QueryRow(`SELECT auto_advance_requires_signal, cancel_triggers_turn_complete FROM workflow_steps WHERE id = 'legacy-projection-step'`).Scan(&autoAdvance, &cancelComplete); err != nil { + t.Fatalf("read migrated workflow step: %v", err) + } + if autoAdvance != 0 || cancelComplete != 0 { + t.Fatalf("legacy workflow step defaults = (%d, %d), want (0, 0)", autoAdvance, cancelComplete) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/workspace_bootstrap.go b/apps/backend/internal/task/repository/sqlite/workspace_bootstrap.go index 06bf554afd..acfc71b07c 100644 --- a/apps/backend/internal/task/repository/sqlite/workspace_bootstrap.go +++ b/apps/backend/internal/task/repository/sqlite/workspace_bootstrap.go @@ -83,13 +83,13 @@ func (r *Repository) insertTemplateSteps(ctx context.Context, tx *sqlx.Tx, workf id, workflow_id, name, position, color, prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, stage_type, - auto_advance_requires_signal, wip_limit, pull_from_step_id, + auto_advance_requires_signal, cancel_triggers_turn_complete, wip_limit, pull_from_step_id, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `), idMap[stepDef.ID], workflowID, stepDef.Name, stepDef.Position, stepDef.Color, stepDef.Prompt, string(events), dialect.BoolToInt(stepDef.AllowManualMove), dialect.BoolToInt(stepDef.IsStartStep), dialect.BoolToInt(stepDef.ShowInCommandPanel), stepDef.AutoArchiveAfterHours, stepDef.AgentProfileID, normalizeBootstrapStageType(stepDef.StageType), - dialect.BoolToInt(stepDef.AutoAdvanceRequiresSignal), stepDef.WIPLimit, wfmodels.RemapStepID(stepDef.PullFromStepID, idMap), now, now); err != nil { + dialect.BoolToInt(stepDef.AutoAdvanceRequiresSignal), dialect.BoolToInt(stepDef.CancelTriggersTurnComplete), stepDef.WIPLimit, wfmodels.RemapStepID(stepDef.PullFromStepID, idMap), now, now); err != nil { return fmt.Errorf("create Kanban step %q: %w", stepDef.Name, err) } } diff --git a/apps/backend/internal/workflow/controller/controller.go b/apps/backend/internal/workflow/controller/controller.go index 25ef81d186..658f3e4086 100644 --- a/apps/backend/internal/workflow/controller/controller.go +++ b/apps/backend/internal/workflow/controller/controller.go @@ -99,18 +99,19 @@ func (c *Controller) CreateStepsFromTemplate(ctx context.Context, req CreateStep // CreateStepRequest is the request for creating a single workflow step. type CreateStepRequest struct { - WorkflowID string `json:"workflow_id"` - Name string `json:"name"` - Position int `json:"position"` - Color string `json:"color"` - Prompt string `json:"prompt,omitempty"` - Events *models.StepEvents `json:"events,omitempty"` - AllowManualMove bool `json:"allow_manual_move"` - IsStartStep *bool `json:"is_start_step,omitempty"` - ShowInCommandPanel *bool `json:"show_in_command_panel,omitempty"` - AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal,omitempty"` - WIPLimit *int `json:"wip_limit,omitempty"` - PullFromStepID *string `json:"pull_from_step_id,omitempty"` + WorkflowID string `json:"workflow_id"` + Name string `json:"name"` + Position int `json:"position"` + Color string `json:"color"` + Prompt string `json:"prompt,omitempty"` + Events *models.StepEvents `json:"events,omitempty"` + AllowManualMove bool `json:"allow_manual_move"` + IsStartStep *bool `json:"is_start_step,omitempty"` + ShowInCommandPanel *bool `json:"show_in_command_panel,omitempty"` + AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal,omitempty"` + CancelTriggersTurnComplete *bool `json:"cancel_triggers_turn_complete,omitempty"` + WIPLimit *int `json:"wip_limit,omitempty"` + PullFromStepID *string `json:"pull_from_step_id,omitempty"` } // CreateStep creates a new workflow step. @@ -140,6 +141,9 @@ func (c *Controller) CreateStep(ctx context.Context, req CreateStepRequest) (*Ge if req.AutoAdvanceRequiresSignal != nil { step.AutoAdvanceRequiresSignal = *req.AutoAdvanceRequiresSignal } + if req.CancelTriggersTurnComplete != nil { + step.CancelTriggersTurnComplete = *req.CancelTriggersTurnComplete + } if req.WIPLimit != nil { if *req.WIPLimit < 0 { return nil, fmt.Errorf("wip_limit must be non-negative") @@ -161,20 +165,21 @@ func (c *Controller) CreateStep(ctx context.Context, req CreateStepRequest) (*Ge // UpdateStepRequest is the request for updating a workflow step. type UpdateStepRequest struct { - ID string `json:"id"` - Name *string `json:"name,omitempty"` - Position *int `json:"position,omitempty"` - Color *string `json:"color,omitempty"` - Prompt *string `json:"prompt,omitempty"` - Events *models.StepEvents `json:"events,omitempty"` - AllowManualMove *bool `json:"allow_manual_move,omitempty"` - IsStartStep *bool `json:"is_start_step,omitempty"` - ShowInCommandPanel *bool `json:"show_in_command_panel,omitempty"` - AutoArchiveAfterHours *int `json:"auto_archive_after_hours,omitempty"` - AgentProfileID *string `json:"agent_profile_id,omitempty"` - AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal,omitempty"` - WIPLimit *int `json:"wip_limit,omitempty"` - PullFromStepID *string `json:"pull_from_step_id,omitempty"` + ID string `json:"id"` + Name *string `json:"name,omitempty"` + Position *int `json:"position,omitempty"` + Color *string `json:"color,omitempty"` + Prompt *string `json:"prompt,omitempty"` + Events *models.StepEvents `json:"events,omitempty"` + AllowManualMove *bool `json:"allow_manual_move,omitempty"` + IsStartStep *bool `json:"is_start_step,omitempty"` + ShowInCommandPanel *bool `json:"show_in_command_panel,omitempty"` + AutoArchiveAfterHours *int `json:"auto_archive_after_hours,omitempty"` + AgentProfileID *string `json:"agent_profile_id,omitempty"` + AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal,omitempty"` + CancelTriggersTurnComplete *bool `json:"cancel_triggers_turn_complete,omitempty"` + WIPLimit *int `json:"wip_limit,omitempty"` + PullFromStepID *string `json:"pull_from_step_id,omitempty"` } // UpdateStep updates an existing workflow step. @@ -219,6 +224,9 @@ func (c *Controller) UpdateStep(ctx context.Context, req UpdateStepRequest) (*Ge if req.AutoAdvanceRequiresSignal != nil { step.AutoAdvanceRequiresSignal = *req.AutoAdvanceRequiresSignal } + if req.CancelTriggersTurnComplete != nil { + step.CancelTriggersTurnComplete = *req.CancelTriggersTurnComplete + } if req.WIPLimit != nil { if *req.WIPLimit < 0 { return nil, fmt.Errorf("wip_limit must be non-negative") diff --git a/apps/backend/internal/workflow/handlers/handlers.go b/apps/backend/internal/workflow/handlers/handlers.go index d918a547d0..6ddc782bdb 100644 --- a/apps/backend/internal/workflow/handlers/handlers.go +++ b/apps/backend/internal/workflow/handlers/handlers.go @@ -239,24 +239,25 @@ func (h *Handlers) publishWorkflowStepEvent(ctx context.Context, eventType strin } data := map[string]interface{}{ "step": map[string]interface{}{ - "id": step.ID, - "workflow_id": step.WorkflowID, - "name": step.Name, - "position": step.Position, - "color": step.Color, - "prompt": step.Prompt, - "events": step.Events, - "show_in_command_panel": step.ShowInCommandPanel, - "allow_manual_move": step.AllowManualMove, - "is_start_step": step.IsStartStep, - "auto_archive_after_hours": step.AutoArchiveAfterHours, - "wip_limit": step.WIPLimit, - "pull_from_step_id": step.PullFromStepID, - "agent_profile_id": step.AgentProfileID, - "stage_type": string(step.StageType), - "auto_advance_requires_signal": step.AutoAdvanceRequiresSignal, - "created_at": step.CreatedAt, - "updated_at": step.UpdatedAt, + "id": step.ID, + "workflow_id": step.WorkflowID, + "name": step.Name, + "position": step.Position, + "color": step.Color, + "prompt": step.Prompt, + "events": step.Events, + "show_in_command_panel": step.ShowInCommandPanel, + "allow_manual_move": step.AllowManualMove, + "is_start_step": step.IsStartStep, + "auto_archive_after_hours": step.AutoArchiveAfterHours, + "wip_limit": step.WIPLimit, + "pull_from_step_id": step.PullFromStepID, + "agent_profile_id": step.AgentProfileID, + "stage_type": string(step.StageType), + "auto_advance_requires_signal": step.AutoAdvanceRequiresSignal, + "cancel_triggers_turn_complete": step.CancelTriggersTurnComplete, + "created_at": step.CreatedAt, + "updated_at": step.UpdatedAt, }, } if err := h.eventBus.Publish(ctx, eventType, bus.NewEvent(eventType, "workflow-handlers", data)); err != nil { diff --git a/apps/backend/internal/workflow/handlers/handlers_test.go b/apps/backend/internal/workflow/handlers/handlers_test.go index 31cde47e66..ebaa90c2c9 100644 --- a/apps/backend/internal/workflow/handlers/handlers_test.go +++ b/apps/backend/internal/workflow/handlers/handlers_test.go @@ -70,3 +70,25 @@ func TestPublishWorkflowStepEventPublishesDeletedStepPayload(t *testing.T) { t.Fatalf("unexpected step payload: %#v", step) } } + +func TestPublishWorkflowStepEvent_IncludesCancelTriggersTurnComplete(t *testing.T) { + eventBus := &recordingWorkflowEventBus{} + h := NewHandlers(nil, eventBus, logger.Default()) + h.publishWorkflowStepEvent(context.Background(), events.WorkflowStepUpdated, &models.WorkflowStep{ + ID: "step-cancel", + WorkflowID: "workflow-1", + CancelTriggersTurnComplete: true, + }) + + data, ok := eventBus.event.Data.(map[string]interface{}) + if !ok { + t.Fatalf("event data type = %T, want map", eventBus.event.Data) + } + step, ok := data["step"].(map[string]interface{}) + if !ok { + t.Fatalf("step data type = %T, want map", data["step"]) + } + if got, ok := step["cancel_triggers_turn_complete"].(bool); !ok || !got { + t.Fatalf("cancel trigger payload = %#v, want true", step["cancel_triggers_turn_complete"]) + } +} diff --git a/apps/backend/internal/workflow/models/export.go b/apps/backend/internal/workflow/models/export.go index 9e525b2e8c..aa9bd07e68 100644 --- a/apps/backend/internal/workflow/models/export.go +++ b/apps/backend/internal/workflow/models/export.go @@ -42,19 +42,20 @@ type WorkflowPortable struct { // StepPortable is a workflow step without instance-specific fields. type StepPortable struct { - Name string `json:"name" yaml:"name"` - Position int `json:"position" yaml:"position"` - Color string `json:"color" yaml:"color"` - Prompt string `json:"prompt,omitempty" yaml:"prompt,omitempty"` - Events StepEvents `json:"events" yaml:"events"` - IsStartStep bool `json:"is_start_step" yaml:"is_start_step"` - ShowInCommandPanel bool `json:"show_in_command_panel" yaml:"show_in_command_panel"` - AllowManualMove bool `json:"allow_manual_move" yaml:"allow_manual_move"` - AutoArchiveAfterHours int `json:"auto_archive_after_hours,omitempty" yaml:"auto_archive_after_hours,omitempty"` - AgentProfile *AgentProfilePortable `json:"agent_profile,omitempty" yaml:"agent_profile,omitempty"` - AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal" yaml:"auto_advance_requires_signal"` - WIPLimit int `json:"wip_limit,omitempty" yaml:"wip_limit,omitempty"` - PullFromStepPosition *int `json:"pull_from_step_position,omitempty" yaml:"pull_from_step_position,omitempty"` + Name string `json:"name" yaml:"name"` + Position int `json:"position" yaml:"position"` + Color string `json:"color" yaml:"color"` + Prompt string `json:"prompt,omitempty" yaml:"prompt,omitempty"` + Events StepEvents `json:"events" yaml:"events"` + IsStartStep bool `json:"is_start_step" yaml:"is_start_step"` + ShowInCommandPanel bool `json:"show_in_command_panel" yaml:"show_in_command_panel"` + AllowManualMove bool `json:"allow_manual_move" yaml:"allow_manual_move"` + AutoArchiveAfterHours int `json:"auto_archive_after_hours,omitempty" yaml:"auto_archive_after_hours,omitempty"` + AgentProfile *AgentProfilePortable `json:"agent_profile,omitempty" yaml:"agent_profile,omitempty"` + AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal" yaml:"auto_advance_requires_signal"` + CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete" yaml:"cancel_triggers_turn_complete"` + WIPLimit int `json:"wip_limit,omitempty" yaml:"wip_limit,omitempty"` + PullFromStepPosition *int `json:"pull_from_step_position,omitempty" yaml:"pull_from_step_position,omitempty"` } // BuildWorkflowExport builds a portable WorkflowExport from domain models. @@ -82,17 +83,18 @@ func buildWorkflowPortable(wf *taskmodels.Workflow, steps []*WorkflowStep, resol } for _, s := range steps { sp := StepPortable{ - Name: s.Name, - Position: s.Position, - Color: s.Color, - Prompt: s.Prompt, - Events: ConvertReviewProfileToPortable(convertStepIDToPosition(s.Events, idToPos), resolveProfile), - IsStartStep: s.IsStartStep, - ShowInCommandPanel: s.ShowInCommandPanel, - AllowManualMove: s.AllowManualMove, - AutoArchiveAfterHours: s.AutoArchiveAfterHours, - AutoAdvanceRequiresSignal: s.AutoAdvanceRequiresSignal, - WIPLimit: s.WIPLimit, + Name: s.Name, + Position: s.Position, + Color: s.Color, + Prompt: s.Prompt, + Events: ConvertReviewProfileToPortable(convertStepIDToPosition(s.Events, idToPos), resolveProfile), + IsStartStep: s.IsStartStep, + ShowInCommandPanel: s.ShowInCommandPanel, + AllowManualMove: s.AllowManualMove, + AutoArchiveAfterHours: s.AutoArchiveAfterHours, + AutoAdvanceRequiresSignal: s.AutoAdvanceRequiresSignal, + CancelTriggersTurnComplete: s.CancelTriggersTurnComplete, + WIPLimit: s.WIPLimit, } if pos, ok := idToPos[s.PullFromStepID]; ok { sp.PullFromStepPosition = &pos diff --git a/apps/backend/internal/workflow/models/export_test.go b/apps/backend/internal/workflow/models/export_test.go index 4016b5fd0d..777c59312c 100644 --- a/apps/backend/internal/workflow/models/export_test.go +++ b/apps/backend/internal/workflow/models/export_test.go @@ -1,12 +1,15 @@ package models import ( + "reflect" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" taskmodels "github.com/kandev/kandev/internal/task/models" + "gopkg.in/yaml.v3" ) func TestBuildWorkflowExport(t *testing.T) { @@ -431,6 +434,46 @@ func TestAutoAdvanceRequiresSignalExport(t *testing.T) { }) } +func TestCancelTriggersTurnCompleteExport(t *testing.T) { + wf := &taskmodels.Workflow{ID: "wf-cancel", Name: "Cancel workflow"} + steps := []*WorkflowStep{ + {ID: "s1", Name: "Paused", Position: 0}, + {ID: "s2", Name: "Advance", Position: 1}, + } + field := reflect.ValueOf(steps[1]).Elem().FieldByName("CancelTriggersTurnComplete") + if !field.IsValid() { + t.Fatal("WorkflowStep is missing CancelTriggersTurnComplete") + } + field.SetBool(true) + + export := BuildWorkflowExport([]*taskmodels.Workflow{wf}, map[string][]*WorkflowStep{"wf-cancel": steps}, nil) + require.Len(t, export.Workflows[0].Steps, 2) + exportedField := reflect.ValueOf(&export.Workflows[0].Steps[1]).Elem().FieldByName("CancelTriggersTurnComplete") + if !exportedField.IsValid() { + t.Fatal("StepPortable is missing CancelTriggersTurnComplete") + } + assert.True(t, exportedField.Bool()) +} + +func TestCancelTriggersTurnCompleteYAMLExportIncludesFalse(t *testing.T) { + wf := &taskmodels.Workflow{ID: "wf-yaml-cancel", Name: "YAML cancellation"} + export := BuildWorkflowExport( + []*taskmodels.Workflow{wf}, + map[string][]*WorkflowStep{"wf-yaml-cancel": { + {ID: "s1", Name: "Todo", Position: 0}, + }}, + nil, + ) + + encoded, err := yaml.Marshal(export) + if err != nil { + t.Fatalf("marshal workflow export: %v", err) + } + if got := string(encoded); !strings.Contains(got, "cancel_triggers_turn_complete: false") { + t.Fatalf("YAML export omitted false cancellation policy:\n%s", got) + } +} + func TestPullFromStepPositionToID(t *testing.T) { position := 0 step := StepPortable{ diff --git a/apps/backend/internal/workflow/models/models.go b/apps/backend/internal/workflow/models/models.go index 4423b68b69..fbf3827868 100644 --- a/apps/backend/internal/workflow/models/models.go +++ b/apps/backend/internal/workflow/models/models.go @@ -194,6 +194,9 @@ type StepDefinition struct { // AutoAdvanceRequiresSignal gates on_turn_complete transitions on an // explicit `step_complete_kandev` MCP signal from the agent (ADR 0015). AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal,omitempty" yaml:"auto_advance_requires_signal,omitempty"` + // CancelTriggersTurnComplete allows an explicit user cancellation to run + // this step's ordinary on_turn_complete actions. + CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete,omitempty" yaml:"cancel_triggers_turn_complete,omitempty"` } // WorkflowStep represents a step in a workflow @@ -222,9 +225,12 @@ type WorkflowStep struct { // When true, bare turn-end does NOT trigger the step's transition // actions; instead the orchestrator waits for the agent (or a manual // UI fallback) to write the pending-signal bag on TaskSession.Metadata. - AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal"` + // CancelTriggersTurnComplete allows an explicit user cancellation to run + // this step's ordinary on_turn_complete actions. + CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } // HasOnEnterAction checks if the step has a specific on_enter action type. diff --git a/apps/backend/internal/workflow/repository/sqlite.go b/apps/backend/internal/workflow/repository/sqlite.go index e1b198ec13..cb7d8f5665 100644 --- a/apps/backend/internal/workflow/repository/sqlite.go +++ b/apps/backend/internal/workflow/repository/sqlite.go @@ -73,6 +73,8 @@ func (r *Repository) initSchema() error { auto_archive_after_hours INTEGER DEFAULT 0, wip_limit INTEGER NOT NULL DEFAULT 0, pull_from_step_id TEXT NOT NULL DEFAULT '', + auto_advance_requires_signal INTEGER NOT NULL DEFAULT 0, + cancel_triggers_turn_complete INTEGER NOT NULL DEFAULT 0, created_at TIMESTAMP NOT NULL, updated_at TIMESTAMP NOT NULL, FOREIGN KEY (workflow_id) REFERENCES workflows(id) ON DELETE CASCADE @@ -118,6 +120,10 @@ func (r *Repository) initSchema() error { // MCP signal. Idempotent ALTER; existing rows keep today's behaviour // (immediate transition on turn-end) until the column is set to 1. r.migrate.Apply("workflow_steps.auto_advance_requires_signal", `ALTER TABLE workflow_steps ADD COLUMN auto_advance_requires_signal INTEGER NOT NULL DEFAULT 0`) + // User-cancel completion is opt-in for persisted rows. The embedded simple + // template supplies its own true values; this migration must never backfill + // existing workflows. + r.migrate.Apply("workflow_steps.cancel_triggers_turn_complete", `ALTER TABLE workflow_steps ADD COLUMN cancel_triggers_turn_complete INTEGER NOT NULL DEFAULT 0`) r.migrate.Apply("workflow_steps.wip_limit", `ALTER TABLE workflow_steps ADD COLUMN wip_limit INTEGER NOT NULL DEFAULT 0`) r.migrate.Apply("workflow_steps.pull_from_step_id", `ALTER TABLE workflow_steps ADD COLUMN pull_from_step_id TEXT NOT NULL DEFAULT ''`) r.migrate.Apply("idx_workflow_steps_pull_from", ` @@ -231,12 +237,15 @@ func (r *Repository) seedDefaultWorkflowSteps() error { if _, err := r.db.Exec(r.db.Rebind(` INSERT INTO workflow_steps ( id, workflow_id, name, position, color, - prompt, events, allow_manual_move, is_start_step, show_in_command_panel, wip_limit, pull_from_step_id, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + prompt, events, allow_manual_move, is_start_step, show_in_command_panel, wip_limit, pull_from_step_id, auto_advance_requires_signal, cancel_triggers_turn_complete, created_at, updated_at + ) VALUES ( + ?, ?, ?, ?, ?, ?, ?, ?, + ?, ?, ?, ?, ?, ?, ?, ? + ) `), idMap[stepDef.ID], workflowID, stepDef.Name, stepDef.Position, stepDef.Color, stepDef.Prompt, string(eventsJSON), dialect.BoolToInt(stepDef.AllowManualMove), - dialect.BoolToInt(stepDef.IsStartStep), dialect.BoolToInt(stepDef.ShowInCommandPanel), stepDef.WIPLimit, models.RemapStepID(stepDef.PullFromStepID, idMap), now, now, + dialect.BoolToInt(stepDef.IsStartStep), dialect.BoolToInt(stepDef.ShowInCommandPanel), stepDef.WIPLimit, models.RemapStepID(stepDef.PullFromStepID, idMap), dialect.BoolToInt(stepDef.AutoAdvanceRequiresSignal), dialect.BoolToInt(stepDef.CancelTriggersTurnComplete), now, now, ); err != nil { return err } @@ -621,11 +630,11 @@ func (r *Repository) CreateStepWithDemotedStartSteps(ctx context.Context, step * _, err = tx.ExecContext(ctx, tx.Rebind(` INSERT INTO workflow_steps ( id, workflow_id, name, position, color, - prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, stage_type, auto_advance_requires_signal, wip_limit, pull_from_step_id, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, stage_type, auto_advance_requires_signal, cancel_triggers_turn_complete, wip_limit, pull_from_step_id, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `), step.ID, step.WorkflowID, step.Name, step.Position, step.Color, step.Prompt, string(eventsJSON), dialect.BoolToInt(step.AllowManualMove), - dialect.BoolToInt(step.IsStartStep), dialect.BoolToInt(step.ShowInCommandPanel), step.AutoArchiveAfterHours, step.AgentProfileID, normalizeStageType(step.StageType), dialect.BoolToInt(step.AutoAdvanceRequiresSignal), step.WIPLimit, step.PullFromStepID, step.CreatedAt, step.UpdatedAt) + dialect.BoolToInt(step.IsStartStep), dialect.BoolToInt(step.ShowInCommandPanel), step.AutoArchiveAfterHours, step.AgentProfileID, normalizeStageType(step.StageType), dialect.BoolToInt(step.AutoAdvanceRequiresSignal), dialect.BoolToInt(step.CancelTriggersTurnComplete), step.WIPLimit, step.PullFromStepID, step.CreatedAt, step.UpdatedAt) if err != nil { return nil, err } @@ -652,12 +661,12 @@ func (r *Repository) scanStep(row interface { Scan(dest ...interface{}) error }) (*models.WorkflowStep, error) { step := &models.WorkflowStep{} - var allowManualMove, isStartStep, showInCommandPanel, autoAdvanceRequiresSignal int + var allowManualMove, isStartStep, showInCommandPanel, autoAdvanceRequiresSignal, cancelTriggersTurnComplete int var autoArchiveAfterHours sql.NullInt64 var color, prompt, eventsJSON, agentProfileID, stageType, pullFromStepID sql.NullString err := row.Scan(&step.ID, &step.WorkflowID, &step.Name, &step.Position, &color, - &prompt, &eventsJSON, &allowManualMove, &isStartStep, &showInCommandPanel, &autoArchiveAfterHours, &agentProfileID, &stageType, &autoAdvanceRequiresSignal, &step.WIPLimit, &pullFromStepID, &step.CreatedAt, &step.UpdatedAt) + &prompt, &eventsJSON, &allowManualMove, &isStartStep, &showInCommandPanel, &autoArchiveAfterHours, &agentProfileID, &stageType, &autoAdvanceRequiresSignal, &cancelTriggersTurnComplete, &step.WIPLimit, &pullFromStepID, &step.CreatedAt, &step.UpdatedAt) if err != nil { return nil, err @@ -667,6 +676,7 @@ func (r *Repository) scanStep(row interface { step.IsStartStep = isStartStep == 1 step.ShowInCommandPanel = showInCommandPanel == 1 step.AutoAdvanceRequiresSignal = autoAdvanceRequiresSignal == 1 + step.CancelTriggersTurnComplete = cancelTriggersTurnComplete == 1 if autoArchiveAfterHours.Valid { step.AutoArchiveAfterHours = int(autoArchiveAfterHours.Int64) } @@ -696,7 +706,7 @@ func (r *Repository) scanStep(row interface { return step, nil } -const stepSelectColumns = `id, workflow_id, name, position, color, prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, stage_type, auto_advance_requires_signal, wip_limit, pull_from_step_id, created_at, updated_at` +const stepSelectColumns = `id, workflow_id, name, position, color, prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, stage_type, auto_advance_requires_signal, cancel_triggers_turn_complete, wip_limit, pull_from_step_id, created_at, updated_at` // GetStep retrieves a workflow step by ID. func (r *Repository) GetStep(ctx context.Context, id string) (*models.WorkflowStep, error) { @@ -749,11 +759,11 @@ func (r *Repository) UpdateStepWithDemotedStartSteps(ctx context.Context, step * UPDATE workflow_steps SET name = ?, position = ?, color = ?, prompt = ?, events = ?, - allow_manual_move = ?, is_start_step = ?, show_in_command_panel = ?, auto_archive_after_hours = ?, agent_profile_id = ?, stage_type = ?, auto_advance_requires_signal = ?, wip_limit = ?, pull_from_step_id = ?, updated_at = ? + allow_manual_move = ?, is_start_step = ?, show_in_command_panel = ?, auto_archive_after_hours = ?, agent_profile_id = ?, stage_type = ?, auto_advance_requires_signal = ?, cancel_triggers_turn_complete = ?, wip_limit = ?, pull_from_step_id = ?, updated_at = ? WHERE id = ? `), step.Name, step.Position, step.Color, step.Prompt, string(eventsJSON), - dialect.BoolToInt(step.AllowManualMove), dialect.BoolToInt(step.IsStartStep), dialect.BoolToInt(step.ShowInCommandPanel), step.AutoArchiveAfterHours, step.AgentProfileID, normalizeStageType(step.StageType), dialect.BoolToInt(step.AutoAdvanceRequiresSignal), step.WIPLimit, step.PullFromStepID, step.UpdatedAt, step.ID) + dialect.BoolToInt(step.AllowManualMove), dialect.BoolToInt(step.IsStartStep), dialect.BoolToInt(step.ShowInCommandPanel), step.AutoArchiveAfterHours, step.AgentProfileID, normalizeStageType(step.StageType), dialect.BoolToInt(step.AutoAdvanceRequiresSignal), dialect.BoolToInt(step.CancelTriggersTurnComplete), step.WIPLimit, step.PullFromStepID, step.UpdatedAt, step.ID) if err != nil { return nil, err } @@ -918,7 +928,7 @@ func (r *Repository) ListStepsByWorkflow(ctx context.Context, workflowID string) func (r *Repository) ListStepsByWorkspaceID(ctx context.Context, workspaceID string) ([]*models.WorkflowStep, error) { rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(` SELECT ws.id, ws.workflow_id, ws.name, ws.position, ws.color, ws.prompt, ws.events, - ws.allow_manual_move, ws.is_start_step, ws.show_in_command_panel, ws.auto_archive_after_hours, ws.agent_profile_id, ws.stage_type, ws.auto_advance_requires_signal, ws.wip_limit, ws.pull_from_step_id, ws.created_at, ws.updated_at + ws.allow_manual_move, ws.is_start_step, ws.show_in_command_panel, ws.auto_archive_after_hours, ws.agent_profile_id, ws.stage_type, ws.auto_advance_requires_signal, ws.cancel_triggers_turn_complete, ws.wip_limit, ws.pull_from_step_id, ws.created_at, ws.updated_at FROM workflow_steps ws JOIN workflows w ON ws.workflow_id = w.id WHERE w.workspace_id = ? diff --git a/apps/backend/internal/workflow/repository/sqlite_test.go b/apps/backend/internal/workflow/repository/sqlite_test.go index 0f872e44ea..4da73a7f3f 100644 --- a/apps/backend/internal/workflow/repository/sqlite_test.go +++ b/apps/backend/internal/workflow/repository/sqlite_test.go @@ -3,6 +3,7 @@ package repository import ( "context" "database/sql" + "reflect" "testing" "github.com/jmoiron/sqlx" @@ -176,6 +177,100 @@ func TestWorkflowStepWIPFields_CreateUpdateAndGet(t *testing.T) { } } +func TestWorkflowStepCancelTriggersTurnComplete_CreateUpdateAndGet(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + step := &models.WorkflowStep{WorkflowID: "wf-test", Name: "Cancelable", Position: 0} + field := reflect.ValueOf(step).Elem().FieldByName("CancelTriggersTurnComplete") + if !field.IsValid() { + t.Fatal("WorkflowStep is missing CancelTriggersTurnComplete") + } + field.SetBool(true) + if err := repo.CreateStep(ctx, step); err != nil { + t.Fatalf("create step: %v", err) + } + retrieved, err := repo.GetStep(ctx, step.ID) + if err != nil { + t.Fatalf("get step: %v", err) + } + retrievedField := reflect.ValueOf(retrieved).Elem().FieldByName("CancelTriggersTurnComplete") + if !retrievedField.IsValid() { + t.Fatal("WorkflowStep is missing CancelTriggersTurnComplete after read") + } + if !retrievedField.Bool() { + t.Fatal("cancel trigger was not persisted as true") + } + retrievedField.SetBool(false) + if err := repo.UpdateStep(ctx, retrieved); err != nil { + t.Fatalf("update step: %v", err) + } + updated, err := repo.GetStep(ctx, step.ID) + if err != nil { + t.Fatalf("get updated step: %v", err) + } + updatedField := reflect.ValueOf(updated).Elem().FieldByName("CancelTriggersTurnComplete") + if updatedField.Bool() { + t.Fatal("cancel trigger remained true after explicit false update") + } +} + +func TestWorkflowStepCancelTriggersTurnComplete_ReplayMigrationDefaultsExistingRows(t *testing.T) { + rawDB, err := sql.Open("sqlite3", ":memory:?_foreign_keys=on") + if err != nil { + t.Fatalf("failed to open sqlite: %v", err) + } + rawDB.SetMaxOpenConns(1) + db := sqlx.NewDb(rawDB, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + _, err = db.Exec(` + CREATE TABLE workflows ( + id TEXT PRIMARY KEY, workspace_id TEXT NOT NULL DEFAULT '', + workflow_template_id TEXT DEFAULT '', name TEXT NOT NULL, + description TEXT DEFAULT '', created_at TIMESTAMP NOT NULL, updated_at TIMESTAMP NOT NULL + ); + CREATE TABLE task_sessions (id TEXT PRIMARY KEY); + CREATE TABLE workflow_steps ( + id TEXT PRIMARY KEY, workflow_id TEXT NOT NULL, name TEXT NOT NULL, + position INTEGER NOT NULL, color TEXT, prompt TEXT, events TEXT, + allow_manual_move INTEGER DEFAULT 1, is_start_step INTEGER DEFAULT 0, + show_in_command_panel INTEGER DEFAULT 1, auto_archive_after_hours INTEGER DEFAULT 0, + agent_profile_id TEXT DEFAULT '', stage_type TEXT NOT NULL DEFAULT 'custom', + auto_advance_requires_signal INTEGER NOT NULL DEFAULT 0, + created_at TIMESTAMP NOT NULL, updated_at TIMESTAMP NOT NULL, + FOREIGN KEY (workflow_id) REFERENCES workflows(id) ON DELETE CASCADE + ); + INSERT INTO workflows (id, workspace_id, name, created_at, updated_at) + VALUES ('wf-replay-cancel', '', 'Replay', datetime('now'), datetime('now')); + INSERT INTO workflow_steps (id, workflow_id, name, position, created_at, updated_at) + VALUES ('legacy-cancel-step', 'wf-replay-cancel', 'Legacy', 0, datetime('now'), datetime('now')); + `) + if err != nil { + t.Fatalf("seed legacy database: %v", err) + } + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("reopen repo: %v", err) + } + var columnCount int + if err := db.QueryRow(`SELECT COUNT(*) FROM pragma_table_info('workflow_steps') WHERE name = 'cancel_triggers_turn_complete'`).Scan(&columnCount); err != nil { + t.Fatalf("inspect migrated schema: %v", err) + } + if columnCount != 1 { + t.Fatalf("cancel trigger migration column count = %d, want 1", columnCount) + } + step, err := repo.GetStep(context.Background(), "legacy-cancel-step") + if err != nil { + t.Fatalf("get legacy step: %v", err) + } + field := reflect.ValueOf(step).Elem().FieldByName("CancelTriggersTurnComplete") + if !field.IsValid() { + t.Fatal("WorkflowStep is missing CancelTriggersTurnComplete") + } + if field.Bool() { + t.Fatal("legacy row cancel trigger defaulted true; want false") + } +} + func TestWorkflowStepWIPFields_ReplayMigrationDefaultsExistingRows(t *testing.T) { rawDB, err := sql.Open("sqlite3", ":memory:?_foreign_keys=on") if err != nil { diff --git a/apps/backend/internal/workflow/service/service.go b/apps/backend/internal/workflow/service/service.go index 462a684dc3..cf15711bf5 100644 --- a/apps/backend/internal/workflow/service/service.go +++ b/apps/backend/internal/workflow/service/service.go @@ -210,21 +210,23 @@ func (s *Service) CreateStepsFromTemplate(ctx context.Context, workflowID, templ for _, stepDef := range template.Steps { events := models.RemapStepEvents(stepDef.Events, idMap) step := &models.WorkflowStep{ - ID: idMap[stepDef.ID], - WorkflowID: workflowID, - Name: stepDef.Name, - Position: stepDef.Position, - Color: stepDef.Color, - Prompt: stepDef.Prompt, - Events: events, - AllowManualMove: stepDef.AllowManualMove, - IsStartStep: stepDef.IsStartStep, - ShowInCommandPanel: stepDef.ShowInCommandPanel, - AutoArchiveAfterHours: stepDef.AutoArchiveAfterHours, - AgentProfileID: stepDef.AgentProfileID, - WIPLimit: stepDef.WIPLimit, - PullFromStepID: models.RemapStepID(stepDef.PullFromStepID, idMap), - StageType: stepDef.StageType, + ID: idMap[stepDef.ID], + WorkflowID: workflowID, + Name: stepDef.Name, + Position: stepDef.Position, + Color: stepDef.Color, + Prompt: stepDef.Prompt, + Events: events, + AllowManualMove: stepDef.AllowManualMove, + IsStartStep: stepDef.IsStartStep, + ShowInCommandPanel: stepDef.ShowInCommandPanel, + AutoArchiveAfterHours: stepDef.AutoArchiveAfterHours, + AgentProfileID: stepDef.AgentProfileID, + AutoAdvanceRequiresSignal: stepDef.AutoAdvanceRequiresSignal, + CancelTriggersTurnComplete: stepDef.CancelTriggersTurnComplete, + WIPLimit: stepDef.WIPLimit, + PullFromStepID: models.RemapStepID(stepDef.PullFromStepID, idMap), + StageType: stepDef.StageType, } if err := models.ValidateWorkflowStep(step); err != nil { @@ -576,20 +578,21 @@ func (s *Service) importSingleWorkflow(ctx context.Context, workspaceID string, // step-level agent profile when a matcher is wired. func (s *Service) stepFromPortable(workflowID string, sp models.StepPortable, posToID map[int]string) *models.WorkflowStep { step := &models.WorkflowStep{ - ID: posToID[sp.Position], - WorkflowID: workflowID, - Name: sp.Name, - Position: sp.Position, - Color: sp.Color, - Prompt: sp.Prompt, - Events: models.ConvertReviewProfileToID(models.ConvertPositionToStepID(sp.Events, posToID), s.matchProfile), - IsStartStep: sp.IsStartStep, - ShowInCommandPanel: sp.ShowInCommandPanel, - AllowManualMove: sp.AllowManualMove, - AutoArchiveAfterHours: sp.AutoArchiveAfterHours, - AutoAdvanceRequiresSignal: sp.AutoAdvanceRequiresSignal, - WIPLimit: sp.WIPLimit, - PullFromStepID: sp.PullFromStepID(posToID), + ID: posToID[sp.Position], + WorkflowID: workflowID, + Name: sp.Name, + Position: sp.Position, + Color: sp.Color, + Prompt: sp.Prompt, + Events: models.ConvertReviewProfileToID(models.ConvertPositionToStepID(sp.Events, posToID), s.matchProfile), + IsStartStep: sp.IsStartStep, + ShowInCommandPanel: sp.ShowInCommandPanel, + AllowManualMove: sp.AllowManualMove, + AutoArchiveAfterHours: sp.AutoArchiveAfterHours, + AutoAdvanceRequiresSignal: sp.AutoAdvanceRequiresSignal, + CancelTriggersTurnComplete: sp.CancelTriggersTurnComplete, + WIPLimit: sp.WIPLimit, + PullFromStepID: sp.PullFromStepID(posToID), } if sp.AgentProfile != nil && s.matchProfile != nil { step.AgentProfileID = s.matchProfile(sp.AgentProfile.AgentName, sp.AgentProfile.Model, sp.AgentProfile.Mode) diff --git a/apps/backend/internal/workflow/service/service_test.go b/apps/backend/internal/workflow/service/service_test.go index a7e3abd01a..b2ef4653d5 100644 --- a/apps/backend/internal/workflow/service/service_test.go +++ b/apps/backend/internal/workflow/service/service_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "reflect" "testing" "time" @@ -455,6 +456,30 @@ func TestCreateStepsFromTemplate_RemapsStepIDs(t *testing.T) { assert.Equal(t, nameToID["In Progress"], done.Events.OnTurnStart[0].Config["step_id"]) } +func TestCreateStepsFromTemplate_PreservesCancelTriggersTurnComplete(t *testing.T) { + svc, db := setupTestService(t) + ctx := context.Background() + insertWorkflow(t, db, "wf-cancel-defaults", "Cancel defaults") + + require.NoError(t, svc.CreateStepsFromTemplate(ctx, "wf-cancel-defaults", "simple")) + steps, err := svc.repo.ListStepsByWorkflow(ctx, "wf-cancel-defaults") + require.NoError(t, err) + want := map[string]bool{"Backlog": true, "In Progress": true, "Review": false, "Done": false} + for _, step := range steps { + wantValue, ok := want[step.Name] + if !ok { + continue + } + field := reflect.ValueOf(step).Elem().FieldByName("CancelTriggersTurnComplete") + if !field.IsValid() { + t.Fatalf("WorkflowStep is missing CancelTriggersTurnComplete") + } + if got := field.Bool(); got != wantValue { + t.Errorf("created step %q cancel trigger = %t, want %t", step.Name, got, wantValue) + } + } +} + func TestCreateStepsFromTemplate_NormalizesDuplicateStartSteps(t *testing.T) { svc, db := setupTestService(t) ctx := context.Background() diff --git a/apps/backend/internal/workflow/service/sync_apply.go b/apps/backend/internal/workflow/service/sync_apply.go index e20a65cc66..f00a9f61ef 100644 --- a/apps/backend/internal/workflow/service/sync_apply.go +++ b/apps/backend/internal/workflow/service/sync_apply.go @@ -219,6 +219,7 @@ func stepMatchesDefinition(existing, desired *models.WorkflowStep) bool { existing.WIPLimit == desired.WIPLimit && existing.PullFromStepID == desired.PullFromStepID && existing.AutoAdvanceRequiresSignal == desired.AutoAdvanceRequiresSignal && + existing.CancelTriggersTurnComplete == desired.CancelTriggersTurnComplete && eventsEqual(existing.Events, desired.Events) } diff --git a/apps/backend/internal/workflow/service/sync_apply_test.go b/apps/backend/internal/workflow/service/sync_apply_test.go index 2e44f27e67..c2abd24497 100644 --- a/apps/backend/internal/workflow/service/sync_apply_test.go +++ b/apps/backend/internal/workflow/service/sync_apply_test.go @@ -303,6 +303,36 @@ func TestApplySyncedWorkflows_RemapsStepEventPositions(t *testing.T) { assert.Equal(t, steps[1].ID, steps[0].Events.OnTurnComplete[0].Config["step_id"]) } +func TestApplySyncedWorkflows_PreservesCancelTriggersTurnComplete(t *testing.T) { + svc, _, _ := setupSyncService(t) + ctx := context.Background() + first := models.WorkflowPortable{ + Name: "Cancel Flow", + Steps: []models.StepPortable{ + {Name: "Work", Position: 0, IsStartStep: true, CancelTriggersTurnComplete: true}, + {Name: "Review", Position: 1}, + }, + } + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{{Path: "flows/cancel.yml", Export: exportOf(first)}}) + require.NoError(t, err) + require.Equal(t, []string{"Cancel Flow"}, result.Created) + + steps, err := svc.ListStepsByWorkflow(ctx, "imported-Cancel Flow") + require.NoError(t, err) + require.Len(t, steps, 2) + assert.True(t, steps[0].CancelTriggersTurnComplete) + + updated := first + updated.Steps = append([]models.StepPortable(nil), first.Steps...) + updated.Steps[0].CancelTriggersTurnComplete = false + result, err = svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{{Path: "flows/cancel.yml", Export: exportOf(updated)}}) + require.NoError(t, err) + require.Equal(t, []string{"Cancel Flow"}, result.Updated) + steps, err = svc.ListStepsByWorkflow(ctx, "imported-Cancel Flow") + require.NoError(t, err) + assert.False(t, steps[0].CancelTriggersTurnComplete) +} + func TestApplySyncedWorkflows_SecondApplyIsNoOp(t *testing.T) { svc, _, _ := setupSyncService(t) ctx := context.Background() diff --git a/apps/web/app/actions/workspaces.test.ts b/apps/web/app/actions/workspaces.test.ts index 1c0de4356f..71ff6f0272 100644 --- a/apps/web/app/actions/workspaces.test.ts +++ b/apps/web/app/actions/workspaces.test.ts @@ -12,6 +12,9 @@ import { updateWorkflowStepAction, } from "./workspaces"; +const REVIEW_STEP_NAME = "Review"; +const STEP_COLOR = "bg-blue-500"; + describe("exportAllWorkflowsAction", () => { beforeEach(() => { vi.stubGlobal( @@ -83,9 +86,9 @@ describe("workflow step WIP fields", () => { Response.json({ id: "step-1", workflow_id: "wf-1", - name: "Review", + name: REVIEW_STEP_NAME, position: 1, - color: "bg-blue-500", + color: STEP_COLOR, wip_limit: 2, pull_from_step_id: "step-0", created_at: "", @@ -109,9 +112,9 @@ describe("workflow step WIP fields", () => { { id: "step-1", workflow_id: "wf-1", - name: "Review", + name: REVIEW_STEP_NAME, position: 1, - color: "bg-blue-500", + color: STEP_COLOR, wip_limit: 2, pull_from_step_id: "step-0", created_at: "", @@ -133,9 +136,9 @@ describe("workflow step WIP fields", () => { it("sends WIP fields when creating a workflow step", async () => { await createWorkflowStepAction({ workflow_id: "wf-1", - name: "Review", + name: REVIEW_STEP_NAME, position: 1, - color: "bg-blue-500", + color: STEP_COLOR, wip_limit: 2, pull_from_step_id: "step-0", } as Parameters[0]); @@ -162,3 +165,82 @@ describe("workflow step WIP fields", () => { }); }); }); + +describe("workflow step cancellation fields", () => { + beforeEach(() => { + vi.stubGlobal( + "fetch", + vi.fn(async () => + Response.json({ + id: "step-1", + workflow_id: "wf-1", + name: REVIEW_STEP_NAME, + position: 1, + color: STEP_COLOR, + cancel_triggers_turn_complete: true, + created_at: "", + updated_at: "", + }), + ), + ); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("preserves cancel completion policy returned from workflow step APIs", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => + Response.json({ + steps: [ + { + id: "step-1", + workflow_id: "wf-1", + name: REVIEW_STEP_NAME, + position: 1, + color: STEP_COLOR, + cancel_triggers_turn_complete: true, + created_at: "", + updated_at: "", + }, + ], + }), + ), + ); + + const result = await listWorkflowStepsAction("wf-1"); + + expect(result.steps[0].cancel_triggers_turn_complete).toBe(true); + }); + + it("sends cancel completion policy when creating a workflow step", async () => { + await createWorkflowStepAction({ + workflow_id: "wf-1", + name: REVIEW_STEP_NAME, + position: 1, + color: STEP_COLOR, + cancel_triggers_turn_complete: true, + } as Parameters[0]); + + const fetchMock = fetch as unknown as ReturnType; + const [, init] = fetchMock.mock.calls[0]; + expect(JSON.parse(init.body as string)).toMatchObject({ + cancel_triggers_turn_complete: true, + }); + }); + + it("sends an explicit false when disabling cancel completion policy", async () => { + await updateWorkflowStepAction("step-1", { + cancel_triggers_turn_complete: false, + } as Parameters[1]); + + const fetchMock = fetch as unknown as ReturnType; + const [, init] = fetchMock.mock.calls[0]; + expect(JSON.parse(init.body as string)).toMatchObject({ + cancel_triggers_turn_complete: false, + }); + }); +}); diff --git a/apps/web/app/actions/workspaces.ts b/apps/web/app/actions/workspaces.ts index c4e3942f76..73da7be477 100644 --- a/apps/web/app/actions/workspaces.ts +++ b/apps/web/app/actions/workspaces.ts @@ -314,6 +314,8 @@ type BackendTemplateStep = { events?: StepEvents; is_start_step?: boolean; show_in_command_panel?: boolean; + auto_advance_requires_signal?: boolean; + cancel_triggers_turn_complete?: boolean; wip_limit?: number; pull_from_step_id?: string | null; }; @@ -333,6 +335,8 @@ const normalizeWorkflowTemplate = (template: BackendWorkflowTemplate): WorkflowT events: step.events, is_start_step: step.is_start_step, show_in_command_panel: step.show_in_command_panel, + auto_advance_requires_signal: step.auto_advance_requires_signal, + cancel_triggers_turn_complete: step.cancel_triggers_turn_complete, wip_limit: step.wip_limit, pull_from_step_id: step.pull_from_step_id ?? null, })); @@ -369,6 +373,7 @@ type BackendWorkflowStep = { auto_archive_after_hours?: number; agent_profile_id?: string; auto_advance_requires_signal?: boolean; + cancel_triggers_turn_complete?: boolean; wip_limit?: number; pull_from_step_id?: string | null; created_at: string; @@ -389,6 +394,7 @@ const transformWorkflowStep = (step: BackendWorkflowStep): WorkflowStep => ({ auto_archive_after_hours: step.auto_archive_after_hours, agent_profile_id: step.agent_profile_id, auto_advance_requires_signal: step.auto_advance_requires_signal, + cancel_triggers_turn_complete: step.cancel_triggers_turn_complete, wip_limit: step.wip_limit ?? 0, pull_from_step_id: step.pull_from_step_id ?? null, created_at: step.created_at, @@ -432,6 +438,7 @@ export async function createWorkflowStepAction(payload: { allow_manual_move?: boolean; wip_limit?: number; pull_from_step_id?: string | null; + cancel_triggers_turn_complete?: boolean; }): Promise { const body = { workflow_id: payload.workflow_id, @@ -444,6 +451,7 @@ export async function createWorkflowStepAction(payload: { is_start_step: payload.is_start_step ?? false, wip_limit: payload.wip_limit ?? 0, pull_from_step_id: payload.pull_from_step_id ?? "", + cancel_triggers_turn_complete: payload.cancel_triggers_turn_complete ?? false, }; const response = await fetchJson(`${apiBaseUrl}/api/v1/workflow/steps`, { method: "POST", @@ -468,6 +476,7 @@ export async function updateWorkflowStepAction( | "auto_archive_after_hours" | "agent_profile_id" | "auto_advance_requires_signal" + | "cancel_triggers_turn_complete" | "wip_limit" | "pull_from_step_id" > @@ -488,6 +497,8 @@ export async function updateWorkflowStepAction( if (payload.agent_profile_id !== undefined) body.agent_profile_id = payload.agent_profile_id; if (payload.auto_advance_requires_signal !== undefined) body.auto_advance_requires_signal = payload.auto_advance_requires_signal; + if (payload.cancel_triggers_turn_complete !== undefined) + body.cancel_triggers_turn_complete = payload.cancel_triggers_turn_complete; if (payload.wip_limit !== undefined) body.wip_limit = payload.wip_limit; if (payload.pull_from_step_id !== undefined) body.pull_from_step_id = payload.pull_from_step_id; const response = await fetchJson( diff --git a/apps/web/app/settings/workspace/use-workflow-creation.test.ts b/apps/web/app/settings/workspace/use-workflow-creation.test.ts index a605f67213..c30690e010 100644 --- a/apps/web/app/settings/workspace/use-workflow-creation.test.ts +++ b/apps/web/app/settings/workspace/use-workflow-creation.test.ts @@ -8,7 +8,14 @@ const template = { id: "template-1", name: "Template", description: "Template description", - default_steps: [{ name: "Template Step", position: 0, color: "bg-blue-500" }], + default_steps: [ + { + name: "Template Step", + position: 0, + color: "bg-blue-500", + cancel_triggers_turn_complete: true, + }, + ], } as WorkflowTemplate; function renderCreationHook(workflowTemplates: WorkflowTemplate[] = []) { @@ -102,6 +109,7 @@ describe("useWorkflowCreation", () => { expect(result.current.initialStepsByWorkflowId.get(workflow.id)?.[0]).toMatchObject({ name: "Template Step", color: "bg-blue-500", + cancel_triggers_turn_complete: true, }); }); }); diff --git a/apps/web/app/settings/workspace/use-workflow-creation.ts b/apps/web/app/settings/workspace/use-workflow-creation.ts index d97d7bd21a..c870d566c7 100644 --- a/apps/web/app/settings/workspace/use-workflow-creation.ts +++ b/apps/web/app/settings/workspace/use-workflow-creation.ts @@ -60,6 +60,8 @@ function toDraftStep( is_start_step: definition.is_start_step, show_in_command_panel: definition.show_in_command_panel, agent_profile_id: definition.agent_profile_id, + auto_advance_requires_signal: definition.auto_advance_requires_signal, + cancel_triggers_turn_complete: definition.cancel_triggers_turn_complete, wip_limit: definition.wip_limit, pull_from_step_id: definition.pull_from_step_id ? (definitionIds.get(definition.pull_from_step_id) ?? definition.pull_from_step_id) diff --git a/apps/web/components/settings/workflow-card-actions.test.ts b/apps/web/components/settings/workflow-card-actions.test.ts index cef482f346..d6ae63dcac 100644 --- a/apps/web/components/settings/workflow-card-actions.test.ts +++ b/apps/web/components/settings/workflow-card-actions.test.ts @@ -17,6 +17,7 @@ import { createWorkflowDraftSaveProgress, persistWorkflowDraft, useWorkflowStepActions, + areStepDraftsEqual, } from "./workflow-card-actions"; vi.mock("@/app/actions/workspaces", () => ({ @@ -175,6 +176,16 @@ describe("useWorkflowStepActions", () => { }); }); +describe("workflow step cancel completion policy", () => { + it("treats a persisted policy change as a dirty draft", () => { + const saved = step("step-1", "Todo", 0, true); + const draft = { ...saved, cancel_triggers_turn_complete: true } as WorkflowStep; + + expect(areStepDraftsEqual(saved, draft)).toBe(false); + expect(areStepDraftsEqual(draft, { ...draft })).toBe(true); + }); +}); + describe("useWorkflowStepActions destructive and ordering paths", () => { it("refuses to add, update, remove, or reorder steps when readOnly", async () => { const initialSteps = [step("step-1", "Todo", 0, true), step("step-2", "Plan", 1, false)]; @@ -367,6 +378,43 @@ describe("persistWorkflowDraft", () => { }); }); +describe("persistWorkflowDraft cancellation policy", () => { + it("forwards cancellation policy when creating a missing template step", async () => { + const draftWorkflow = { ...workflow, id: CLIENT_WORKFLOW_ID } as Workflow; + const drafts = [ + { + ...step(CLIENT_STEP_ONE, "Todo", 0, true), + workflow_id: draftWorkflow.id, + cancel_triggers_turn_complete: true, + }, + ] as WorkflowStep[]; + vi.mocked(createWorkflowAction).mockResolvedValue({ + ...workflow, + id: "wf-created", + } as Workflow); + vi.mocked(updateWorkflowAction).mockResolvedValue({ + ...workflow, + id: "wf-created", + } as Workflow); + vi.mocked(createWorkflowStepAction).mockResolvedValueOnce( + step(SERVER_STEP_ONE, "Todo", 0, true), + ); + + await persistWorkflowDraft({ + workflow: draftWorkflow, + draftSteps: drafts, + savedSteps: [], + progress: createWorkflowDraftSaveProgress(), + }); + + expect(createWorkflowStepAction).toHaveBeenCalledWith( + expect.objectContaining({ + cancel_triggers_turn_complete: true, + }), + ); + }); +}); + describe("useWorkflowDeleteHandlers", () => { it("refuses to open the delete-workflow dialog when readOnly", async () => { const wfDel = { diff --git a/apps/web/components/settings/workflow-card-actions.ts b/apps/web/components/settings/workflow-card-actions.ts index 8836ee2c7b..22097c7849 100644 --- a/apps/web/components/settings/workflow-card-actions.ts +++ b/apps/web/components/settings/workflow-card-actions.ts @@ -310,6 +310,7 @@ async function createMissingSteps( name: step.name, position: step.position, color: step.color, + cancel_triggers_turn_complete: step.cancel_triggers_turn_complete ?? false, }); mappings.set(step.id, created.id); } @@ -393,6 +394,7 @@ function stepUpdatePayload(step: WorkflowStep): Partial { auto_archive_after_hours: step.auto_archive_after_hours ?? 0, agent_profile_id: step.agent_profile_id ?? "", auto_advance_requires_signal: step.auto_advance_requires_signal ?? false, + cancel_triggers_turn_complete: step.cancel_triggers_turn_complete ?? false, wip_limit: step.wip_limit ?? 0, pull_from_step_id: step.pull_from_step_id ?? "", }; diff --git a/apps/web/components/settings/workflow-pipeline-editor-step-actions.test.tsx b/apps/web/components/settings/workflow-pipeline-editor-step-actions.test.tsx new file mode 100644 index 0000000000..d795bbbee1 --- /dev/null +++ b/apps/web/components/settings/workflow-pipeline-editor-step-actions.test.tsx @@ -0,0 +1,73 @@ +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { WorkflowStep } from "@/lib/types/http"; +import { workflowId as toWorkflowId } from "@/lib/types/ids"; +import { TurnCompleteSelect } from "./workflow-pipeline-editor-step-actions"; + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +afterEach(cleanup); + +function step(overrides: Partial = {}): WorkflowStep { + return { + id: "step-1", + workflow_id: toWorkflowId("workflow-1"), + name: "In Progress", + position: 0, + color: "bg-blue-500", + events: { on_turn_complete: [{ type: "move_to_next" }] }, + created_at: "", + updated_at: "", + ...overrides, + }; +} + +function renderTurnComplete(current: WorkflowStep, readOnly = false) { + const onUpdate = vi.fn(); + render( + , + ); + return { onUpdate }; +} + +describe("TurnCompleteSelect cancel completion policy", () => { + it("renders the policy disabled by default and updates it when checked", () => { + const { onUpdate } = renderTurnComplete(step()); + const checkbox = screen.getByTestId("step-1-cancel-completion-checkbox"); + + expect(checkbox.getAttribute("aria-checked")).toBe("false"); + expect(screen.getByTestId("step-1-cancel-completion-help").getAttribute("aria-label")).toBe( + "More information", + ); + expect(screen.queryByText("settings:runCompletionActionsWhenTurnCancelledHelp")).toBeNull(); + fireEvent.click(checkbox); + expect(onUpdate).toHaveBeenCalledWith({ cancel_triggers_turn_complete: true }); + }); + + it("preserves the persisted value and disables it in read-only mode", () => { + renderTurnComplete(step({ cancel_triggers_turn_complete: true }), true); + const checkbox = screen.getByTestId("step-1-cancel-completion-checkbox"); + + expect(checkbox.getAttribute("aria-checked")).toBe("true"); + expect((checkbox as HTMLButtonElement).disabled).toBe(true); + }); + + it("does not render the subordinate policy when no completion transition is configured", () => { + renderTurnComplete( + step({ events: { on_turn_complete: [] }, cancel_triggers_turn_complete: true }), + ); + + expect(screen.queryByTestId("step-1-cancel-completion-checkbox")).toBeNull(); + }); +}); diff --git a/apps/web/components/settings/workflow-pipeline-editor-step-actions.tsx b/apps/web/components/settings/workflow-pipeline-editor-step-actions.tsx index 41afb852bf..6013e4b711 100644 --- a/apps/web/components/settings/workflow-pipeline-editor-step-actions.tsx +++ b/apps/web/components/settings/workflow-pipeline-editor-step-actions.tsx @@ -3,6 +3,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; import { Checkbox } from "@kandev/ui/checkbox"; import { Label } from "@kandev/ui/label"; +import { useTranslation } from "react-i18next"; import type { WorkflowStep, OnEnterAction, @@ -318,12 +319,20 @@ export function TurnCompleteSelect({ )} {transitionType !== "none" && ( - + <> + + + )} ); @@ -452,6 +461,7 @@ export function ExplicitCompletionToggle({ onUpdate, readOnly, }: ExplicitCompletionToggleProps) { + const { t } = useTranslation(); return (
item.auto_advance_requires_signal ?? false, )} /> -
); } + +type CancelCompletionToggleProps = ExplicitCompletionToggleProps; + +export function CancelCompletionToggle({ + step, + savedStep, + onUpdate, + readOnly, +}: CancelCompletionToggleProps) { + const { t } = useTranslation(); + return ( +
+ { + if (readOnly) return; + onUpdate({ cancel_triggers_turn_complete: checked === true }); + }} + disabled={readOnly} + data-settings-dirty={isWorkflowStepValueDirty( + step, + savedStep, + (item) => item.cancel_triggers_turn_complete ?? false, + )} + /> + + +
+ ); +} diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 867f1ab830..a73c65c5c7 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -908,6 +908,7 @@ export class ApiClient { }; wip_limit?: number; pull_from_step_id?: string | null; + cancel_triggers_turn_complete?: boolean; }, ): Promise { await this.request("PUT", `/api/v1/workflow/steps/${stepId}`, { id: stepId, ...updates }); diff --git a/apps/web/e2e/pages/workflow-settings-page.ts b/apps/web/e2e/pages/workflow-settings-page.ts index acfeee8fb9..4b8a93bc7d 100644 --- a/apps/web/e2e/pages/workflow-settings-page.ts +++ b/apps/web/e2e/pages/workflow-settings-page.ts @@ -112,6 +112,26 @@ export class WorkflowSettingsPage { await this.activate(this.page.getByRole("option", { name: optionName }), touch); } + /** Toggle the cancellation policy beneath a configured turn-complete transition. */ + async setCancelCompletionPolicy( + card: Locator, + stepName: string, + enabled: boolean, + touch = false, + ) { + const panel = await this.selectStep(card, stepName, touch); + const checkbox = panel.getByRole("checkbox", { + name: "Run completion actions when a turn is cancelled", + }); + if ((await checkbox.isChecked()) !== enabled) { + const target = touch ? panel.getByTestId(/-cancel-completion-label$/) : checkbox; + await this.activate(target, touch); + } + if (enabled) await expect(checkbox).toBeChecked(); + else await expect(checkbox).not.toBeChecked(); + return checkbox; + } + /** The add-step (+) button within a workflow card. */ addStepButton(card: Locator): Locator { return card.getByTestId("add-step-button"); diff --git a/apps/web/e2e/tests/session/mobile-pause-resume-recovery.spec.ts b/apps/web/e2e/tests/session/mobile-pause-resume-recovery.spec.ts index b2fd7290c5..cfb1717bde 100644 --- a/apps/web/e2e/tests/session/mobile-pause-resume-recovery.spec.ts +++ b/apps/web/e2e/tests/session/mobile-pause-resume-recovery.spec.ts @@ -15,6 +15,13 @@ test.describe("mobile: pause queue recovery", () => { }) => { test.setTimeout(90_000); + // This spec covers pause/resume queue semantics, not workflow completion. + // Opt out explicitly because the Kanban template enables cancellation + // completion by default. + await apiClient.updateWorkflowStep(seedData.startStepId, { + cancel_triggers_turn_complete: false, + }); + const session = await seedIdleSession( testPage, apiClient, diff --git a/apps/web/e2e/tests/session/pause-resume-recovery.spec.ts b/apps/web/e2e/tests/session/pause-resume-recovery.spec.ts index 333cccb2ea..e4f8cb11e0 100644 --- a/apps/web/e2e/tests/session/pause-resume-recovery.spec.ts +++ b/apps/web/e2e/tests/session/pause-resume-recovery.spec.ts @@ -28,6 +28,12 @@ async function seedTaskAndWaitForIdle( seedData: SeedData, title: string, ): Promise { + // The Kanban template now opts into workflow completion for explicit + // cancellation. These tests exercise pause/resume queue semantics instead, + // so keep the source step in-place while the turn is paused. + await apiClient.updateWorkflowStep(seedData.startStepId, { + cancel_triggers_turn_complete: false, + }); const task = await apiClient.createTaskWithAgent( seedData.workspaceId, title, diff --git a/apps/web/e2e/tests/workflow/mobile-workflow-cancel-completion.spec.ts b/apps/web/e2e/tests/workflow/mobile-workflow-cancel-completion.spec.ts new file mode 100644 index 0000000000..928ad4563f --- /dev/null +++ b/apps/web/e2e/tests/workflow/mobile-workflow-cancel-completion.spec.ts @@ -0,0 +1,113 @@ +import { test, expect } from "../../fixtures/test-base"; +import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions"; +import { ApiClient } from "../../helpers/api-client"; +import { SessionPage } from "../../pages/session-page"; +import { WorkflowSettingsPage } from "../../pages/workflow-settings-page"; + +async function seedMobileCancellationWorkflow(apiClient: ApiClient, workspaceId: string) { + const workflow = await apiClient.createWorkflow(workspaceId, "Mobile Cancel Completion"); + const inbox = await apiClient.createWorkflowStep(workflow.id, "Inbox", 0, { + is_start_step: true, + }); + const working = await apiClient.createWorkflowStep(workflow.id, "Working", 1); + const done = await apiClient.createWorkflowStep(workflow.id, "Done", 2); + await apiClient.updateWorkflowStep(working.id, { + prompt: 'e2e:delay(8000)\ne2e:message("mobile cancelled turn marker")\n{{task_prompt}}', + events: { + on_enter: [{ type: "auto_start_agent" }], + on_turn_complete: [{ type: "move_to_step", config: { step_id: done.id } }], + }, + cancel_triggers_turn_complete: false, + }); + return { + workflowId: workflow.id, + inboxStepId: inbox.id, + workingStepId: working.id, + doneStepId: done.id, + }; +} + +test.describe("mobile: cancelled turn completion", () => { + test.describe.configure({ retries: 1 }); + + test("saves the policy by touch, reloads it, and advances after Cancel", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(150_000); + const workflow = await seedMobileCancellationWorkflow(apiClient, seedData.workspaceId); + const settings = new WorkflowSettingsPage(testPage); + await settings.goto(seedData.workspaceId); + + const card = await settings.findWorkflowCard("Mobile Cancel Completion"); + await settings.setCancelCompletionPolicy(card, "Working", true, true); + const tappableLabel = card.getByTestId(`${workflow.workingStepId}-cancel-completion-label`); + const labelBox = await tappableLabel.boundingBox(); + expect(labelBox).not.toBeNull(); + expect(labelBox!.height).toBeGreaterThanOrEqual(44); + const signalRow = card.getByTestId(`${workflow.workingStepId}-require-signal-row`); + const cancelRow = card.getByTestId(`${workflow.workingStepId}-cancel-completion-row`); + const helpTip = card.getByTestId(`${workflow.workingStepId}-cancel-completion-help`); + const [signalRowBox, cancelRowBox, helpBox] = await Promise.all([ + signalRow.boundingBox(), + cancelRow.boundingBox(), + helpTip.boundingBox(), + ]); + expect(signalRowBox).not.toBeNull(); + expect(cancelRowBox).not.toBeNull(); + expect(helpBox).not.toBeNull(); + expect(cancelRowBox!.height).toBeCloseTo(signalRowBox!.height, 1); + expect(helpBox!.x - (labelBox!.x + labelBox!.width)).toBeGreaterThanOrEqual(0); + expect(helpBox!.x - (labelBox!.x + labelBox!.width)).toBeLessThanOrEqual(12); + await settings.saveChanges(true); + + await settings.goto(seedData.workspaceId); + const reloadedCard = await settings.findWorkflowCard("Mobile Cancel Completion"); + const reloadedPanel = await settings.selectStep(reloadedCard, "Working", true); + await expect( + reloadedPanel.getByRole("checkbox", { + name: "Run completion actions when a turn is cancelled", + }), + ).toBeChecked(); + expect( + (await apiClient.listWorkflowSteps(workflow.workflowId)).steps.find( + (step) => step.id === workflow.workingStepId, + )?.cancel_triggers_turn_complete, + ).toBe(true); + + const task = await apiClient.createTask( + seedData.workspaceId, + "Mobile enabled cancellation task", + { + description: "Mobile cancellation routing test", + workflow_id: workflow.workflowId, + workflow_step_id: workflow.inboxStepId, + agent_profile_id: seedData.agentProfileId, + repository_ids: [seedData.repositoryId], + }, + ); + await apiClient.moveTask(task.id, workflow.workflowId, workflow.workingStepId); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect + .poll(async () => (await apiClient.getTask(task.id)).workflow_step_id, { + timeout: 15_000, + }) + .toBe(workflow.workingStepId); + await expect(session.cancelAgentButton()).toBeVisible({ timeout: 30_000 }); + + expect((await apiClient.listTaskSessions(task.id)).sessions).toHaveLength(1); + await session.cancelAgentButton().tap(); + await expect(session.cancelAgentButton()).not.toBeVisible({ timeout: 30_000 }); + await expect + .poll(async () => (await apiClient.getTask(task.id)).workflow_step_id, { + timeout: 30_000, + }) + .toBe(workflow.doneStepId); + expect((await apiClient.listTaskSessions(task.id)).sessions).toHaveLength(1); + await assertNoDocumentHorizontalOverflow(testPage); + }); +}); diff --git a/apps/web/e2e/tests/workflow/workflow-cancel-completion.spec.ts b/apps/web/e2e/tests/workflow/workflow-cancel-completion.spec.ts new file mode 100644 index 0000000000..328df6f2d5 --- /dev/null +++ b/apps/web/e2e/tests/workflow/workflow-cancel-completion.spec.ts @@ -0,0 +1,134 @@ +import { expect } from "@playwright/test"; +import type { Page } from "@playwright/test"; +import { test as e2eTest } from "../../fixtures/test-base"; +import { ApiClient } from "../../helpers/api-client"; +import { SessionPage } from "../../pages/session-page"; + +type CancellationWorkflow = { + workflowId: string; + inboxStepId: string; + workingStepId: string; + doneStepId: string; +}; + +async function createCancellationWorkflow( + apiClient: ApiClient, + workspaceId: string, + enabled: boolean, + name: string, +): Promise { + const workflow = await apiClient.createWorkflow(workspaceId, name); + const inbox = await apiClient.createWorkflowStep(workflow.id, "Inbox", 0, { + is_start_step: true, + }); + const working = await apiClient.createWorkflowStep(workflow.id, "Working", 1); + const done = await apiClient.createWorkflowStep(workflow.id, "Done", 2); + + await apiClient.updateWorkflowStep(working.id, { + prompt: 'e2e:delay(8000)\ne2e:message("cancelled turn marker")\n{{task_prompt}}', + events: { + on_enter: [{ type: "auto_start_agent" }], + on_turn_complete: [{ type: "move_to_step", config: { step_id: done.id } }], + }, + cancel_triggers_turn_complete: enabled, + }); + + return { + workflowId: workflow.id, + inboxStepId: inbox.id, + workingStepId: working.id, + doneStepId: done.id, + }; +} + +async function startWorkingTurn( + testPage: Page, + apiClient: ApiClient, + seedData: { workspaceId: string; agentProfileId: string; repositoryId: string }, + workflow: CancellationWorkflow, + title: string, +) { + const task = await apiClient.createTask(seedData.workspaceId, title, { + description: "Cancellation routing test", + workflow_id: workflow.workflowId, + workflow_step_id: workflow.inboxStepId, + agent_profile_id: seedData.agentProfileId, + repository_ids: [seedData.repositoryId], + }); + await apiClient.moveTask(task.id, workflow.workflowId, workflow.workingStepId); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.stepperStep("Working")).toHaveAttribute("aria-current", "step", { + timeout: 15_000, + }); + await expect(session.agentStatus()).toBeVisible({ timeout: 30_000 }); + await expect(session.cancelAgentButton()).toBeVisible({ timeout: 15_000 }); + return { task, session }; +} + +e2eTest.describe("Cancelled turn completion", () => { + e2eTest.describe.configure({ retries: 1 }); + + e2eTest( + "enabled policy advances once after the real Cancel action", + async ({ testPage, apiClient, seedData }) => { + e2eTest.setTimeout(120_000); + const workflow = await createCancellationWorkflow( + apiClient, + seedData.workspaceId, + true, + "Cancel Completion Enabled", + ); + const { task, session } = await startWorkingTurn( + testPage, + apiClient, + seedData, + workflow, + "Enabled cancellation task", + ); + const sessionsBeforeCancel = await apiClient.listTaskSessions(task.id); + expect(sessionsBeforeCancel.sessions).toHaveLength(1); + + await session.cancelAgentButton().click(); + await expect(session.cancelAgentButton()).not.toBeVisible({ timeout: 30_000 }); + await expect(session.stepperStep("Done")).toHaveAttribute("aria-current", "step", { + timeout: 30_000, + }); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + + expect((await apiClient.getTask(task.id)).workflow_step_id).toBe(workflow.doneStepId); + expect((await apiClient.listTaskSessions(task.id)).sessions).toHaveLength(1); + }, + ); + + e2eTest( + "disabled policy keeps a cancelled turn in its current step", + async ({ testPage, apiClient, seedData }) => { + e2eTest.setTimeout(120_000); + const workflow = await createCancellationWorkflow( + apiClient, + seedData.workspaceId, + false, + "Cancel Completion Disabled", + ); + const { task, session } = await startWorkingTurn( + testPage, + apiClient, + seedData, + workflow, + "Disabled cancellation task", + ); + expect((await apiClient.listTaskSessions(task.id)).sessions).toHaveLength(1); + + await session.cancelAgentButton().click(); + await expect(session.cancelAgentButton()).not.toBeVisible({ timeout: 30_000 }); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + await expect(session.stepperStep("Working")).toHaveAttribute("aria-current", "step"); + + expect((await apiClient.getTask(task.id)).workflow_step_id).toBe(workflow.workingStepId); + expect((await apiClient.listTaskSessions(task.id)).sessions).toHaveLength(1); + }, + ); +}); diff --git a/apps/web/e2e/tests/workflow/workflow-settings.spec.ts b/apps/web/e2e/tests/workflow/workflow-settings.spec.ts index d782f1cbfc..9f28d86fda 100644 --- a/apps/web/e2e/tests/workflow/workflow-settings.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-settings.spec.ts @@ -38,6 +38,42 @@ test.describe("Workflow settings", () => { } }); + test("keeps turn-complete policy controls aligned", async ({ testPage, apiClient, seedData }) => { + const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Turn Complete Layout"); + await apiClient.createWorkflowStep(workflow.id, "Inbox", 0, { is_start_step: true }); + const working = await apiClient.createWorkflowStep(workflow.id, "Working", 1); + const done = await apiClient.createWorkflowStep(workflow.id, "Done", 2); + await apiClient.updateWorkflowStep(working.id, { + events: { + on_turn_complete: [{ type: "move_to_step", config: { step_id: done.id } }], + }, + }); + + const page = new WorkflowSettingsPage(testPage); + await testPage.setViewportSize({ width: 1920, height: 1080 }); + await page.goto(seedData.workspaceId); + const card = await page.findWorkflowCard("Turn Complete Layout"); + const panel = await page.selectStep(card, "Working"); + const signalRow = panel.getByTestId(`${working.id}-require-signal-row`); + const cancelRow = panel.getByTestId(`${working.id}-cancel-completion-row`); + const label = panel.getByTestId(`${working.id}-cancel-completion-label`); + const helpTip = panel.getByTestId(`${working.id}-cancel-completion-help`); + const [signalRowBox, cancelRowBox, labelBox, helpBox] = await Promise.all([ + signalRow.boundingBox(), + cancelRow.boundingBox(), + label.boundingBox(), + helpTip.boundingBox(), + ]); + + expect(signalRowBox).not.toBeNull(); + expect(cancelRowBox).not.toBeNull(); + expect(labelBox).not.toBeNull(); + expect(helpBox).not.toBeNull(); + expect(cancelRowBox!.height).toBeCloseTo(signalRowBox!.height, 1); + expect(helpBox!.x - (labelBox!.x + labelBox!.width)).toBeGreaterThanOrEqual(0); + expect(helpBox!.x - (labelBox!.x + labelBox!.width)).toBeLessThanOrEqual(12); + }); + test("configures the original session with the shared model settings picker", async ({ testPage, backend, diff --git a/apps/web/lib/api/domains/workflow-api.test.ts b/apps/web/lib/api/domains/workflow-api.test.ts index ad1793f2b3..699f2f5d4e 100644 --- a/apps/web/lib/api/domains/workflow-api.test.ts +++ b/apps/web/lib/api/domains/workflow-api.test.ts @@ -1,5 +1,15 @@ import { describe, expect, it } from "vitest"; -import { normalizeWorkflowTemplate } from "./workflow-api"; +import { afterEach, beforeEach, vi } from "vitest"; +import { createWorkflowStep, normalizeWorkflowTemplate } from "./workflow-api"; + +const fetchSpy = vi.fn(); + +beforeEach(() => { + fetchSpy.mockReset(); + vi.stubGlobal("fetch", fetchSpy); +}); + +afterEach(() => vi.unstubAllGlobals()); describe("normalizeWorkflowTemplate", () => { it("preserves template step identities used by transition references", () => { @@ -25,3 +35,28 @@ describe("normalizeWorkflowTemplate", () => { expect(template.default_steps?.map((step) => step.id)).toEqual(["in-progress", "review"]); }); }); + +describe("createWorkflowStep", () => { + it("forwards the cancellation completion policy in the request payload", async () => { + fetchSpy.mockResolvedValueOnce( + new Response(JSON.stringify({ id: "step-1" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + + const payload: Parameters[0] = { + workflow_id: "workflow-1", + name: "Working", + position: 1, + cancel_triggers_turn_complete: true, + }; + await createWorkflowStep(payload, { baseUrl: "http://api.test" }); + + expect(fetchSpy).toHaveBeenCalledOnce(); + const [url, init] = fetchSpy.mock.calls[0]!; + expect(url).toBe("http://api.test/api/v1/workflow/steps"); + expect(init?.method).toBe("POST"); + expect(JSON.parse(String(init?.body))).toMatchObject(payload); + }); +}); diff --git a/apps/web/lib/api/domains/workflow-api.ts b/apps/web/lib/api/domains/workflow-api.ts index 9e6c60e91f..e3c73d7515 100644 --- a/apps/web/lib/api/domains/workflow-api.ts +++ b/apps/web/lib/api/domains/workflow-api.ts @@ -18,6 +18,8 @@ type BackendTemplateStep = { events?: StepEvents; is_start_step?: boolean; show_in_command_panel?: boolean; + auto_advance_requires_signal?: boolean; + cancel_triggers_turn_complete?: boolean; wip_limit?: number; pull_from_step_id?: string | null; }; @@ -38,6 +40,8 @@ export const normalizeWorkflowTemplate = (template: BackendWorkflowTemplate): Wo events: step.events, is_start_step: step.is_start_step, show_in_command_panel: step.show_in_command_panel, + auto_advance_requires_signal: step.auto_advance_requires_signal, + cancel_triggers_turn_complete: step.cancel_triggers_turn_complete, wip_limit: step.wip_limit, pull_from_step_id: step.pull_from_step_id ?? null, })); @@ -89,6 +93,7 @@ export async function createWorkflowStep( color?: string; prompt?: string; events?: StepEvents; + cancel_triggers_turn_complete?: boolean; wip_limit?: number; pull_from_step_id?: string | null; }, diff --git a/apps/web/lib/types/http.ts b/apps/web/lib/types/http.ts index 614ab442c3..37d526aacf 100644 --- a/apps/web/lib/types/http.ts +++ b/apps/web/lib/types/http.ts @@ -100,6 +100,8 @@ export type StepDefinition = { is_start_step?: boolean; show_in_command_panel?: boolean; agent_profile_id?: AgentProfileId; + auto_advance_requires_signal?: boolean; + cancel_triggers_turn_complete?: boolean; wip_limit?: number; pull_from_step_id?: string | null; }; @@ -133,6 +135,11 @@ export type WorkflowStep = { * legacy "any turn-end advances" behaviour. */ auto_advance_requires_signal?: boolean; + /** + * When true, an explicit user cancellation runs this step's normal + * on_turn_complete actions after the cancelled turn settles. + */ + cancel_triggers_turn_complete?: boolean; created_at: string; updated_at: string; }; diff --git a/apps/web/src/locales/en/settings.json b/apps/web/src/locales/en/settings.json index 3aa0aa00dc..c90d1edae8 100644 --- a/apps/web/src/locales/en/settings.json +++ b/apps/web/src/locales/en/settings.json @@ -276,7 +276,10 @@ "restoreTheOriginalBuiltInLayout": "Restore the original built-in layout and discard its override.", "resumeTheMostRecentlyOpenedTask": "Resume the most recently opened task in this workspace on this device.", "retrySave": "Retry save", + "runCompletionActionsWhenTurnCancelled": "Run completion actions when a turn is cancelled", + "runCompletionActionsWhenTurnCancelledHelp": "Applies only when a user explicitly cancels a turn. The destination step’s on-enter actions may immediately start another agent turn.", "runningSprites": "Running Sprites", + "waitForAgentCompletionSignal": "Wait for agent completion signal", "sameShortcutAs": "Same shortcut as: {{shortcuts}}", "saveAndLeave": "Save and leave", "saveChanges": "Save changes", diff --git a/apps/web/src/locales/pseudo/settings.json b/apps/web/src/locales/pseudo/settings.json index f66fc1984a..a6725dde9f 100644 --- a/apps/web/src/locales/pseudo/settings.json +++ b/apps/web/src/locales/pseudo/settings.json @@ -276,7 +276,10 @@ "restoreTheOriginalBuiltInLayout": "Ŕēśţōŕē ţĥē ōŕĩĝĩńàĺ ƀũĩĺţ-ĩń ĺàŷōũţ àńď ďĩśćàŕď ĩţś ōvēŕŕĩďē.", "resumeTheMostRecentlyOpenedTask": "Ŕēśũḿē ţĥē ḿōśţ ŕēćēńţĺŷ ōƥēńēď ţàśķ ĩń ţĥĩś ŵōŕķśƥàćē ōń ţĥĩś ďēvĩćē.", "retrySave": "Ŕēţŕŷ śàvē", + "runCompletionActionsWhenTurnCancelled": "Ŕũń ćōḿƥĺēţĩōń àćţĩōńś ŵĥēń à ţũŕń ĩś ćàńćēĺĺēď", + "runCompletionActionsWhenTurnCancelledHelp": "Àƥƥĺĩēś ōńĺŷ ŵĥēń à ũśēŕ ēxƥĺĩćĩţĺŷ ćàńćēĺś à ţũŕń. Ŧĥē ďēśţĩńàţĩōń śţēƥ’ś ōń-ēńţēŕ àćţĩōńś ḿàŷ ĩḿḿēďĩàţēĺŷ śţàŕţ àńōţĥēŕ àĝēńţ ţũŕń.", "runningSprites": "Ŕũńńĩńĝ Śƥŕĩţēś", + "waitForAgentCompletionSignal": "Ŵàĩţ ƒōŕ àĝēńţ ćōḿƥĺēţĩōń śĩĝńàĺ", "sameShortcutAs": "Śàḿē śĥōŕţćũţ àś: {{shortcuts}}", "saveAndLeave": "Śàvē àńď ĺēàvē", "saveChanges": "Śàvē ćĥàńĝēś", diff --git a/docs/decisions/2026-08-02-explicit-user-cancel-completion.md b/docs/decisions/2026-08-02-explicit-user-cancel-completion.md new file mode 100644 index 0000000000..5c6d076afe --- /dev/null +++ b/docs/decisions/2026-08-02-explicit-user-cancel-completion.md @@ -0,0 +1,33 @@ +# ADR-2026-08-02-explicit-user-cancel-completion: Explicit User Cancellation May Complete a Workflow Step + +**Status:** accepted +**Date:** 2026-08-02 +**Area:** workflow + +## Context + +Normal agent turn completion evaluates a workflow step's `on_turn_complete` actions, while the visible user cancel path only reconciles session and runtime task state. That split can leave a cancelled Kanban task review-ready but still positioned on its work step. Cancellation also occurs internally for clarifications, peer interrupts, parent stops, archival, failures, and teardown, so treating every cancelled or stopped runtime event as workflow completion would create false transitions. + +## Decision + +Workflow steps gain an opt-in `cancel_triggers_turn_complete` boolean. Only the existing explicit, authorized user cancel operation may use it to evaluate the current step's ordinary `on_turn_complete` actions; internal and system-owned cancellation paths remain non-completing. + +The persisted/API default is `false` for compatibility. The embedded `simple` Kanban template sets it to `true` on its action-bearing `Backlog` and `In Progress` steps for newly instantiated workflows, without backfilling existing workflow rows. + +Configured user cancellation is a human completion decision, so it bypasses the agent-owned `auto_advance_requires_signal` gate while retaining the pending-clarification barrier. The workflow engine continues to evaluate and record the transition as `on_turn_complete`; no parallel cancellation action language is introduced. + +## Consequences + +- Users can choose pause-in-place or complete-and-advance semantics per step. +- Standard Kanban cancellation aligns workflow position with its existing review-ready runtime state for new workflows. +- Existing workflows do not change behavior during upgrade. +- Reusing the completion pipeline preserves `on_exit`, first-transition-wins, terminal handling, and destination `on_enter`; consequently a configured cancel may auto-start the destination step. +- Runtime and workflow code must carry an explicit cancellation source rather than infer intent from terminal session states or agent stop reasons. +- Transition history does not distinguish a natural completion from configured user cancellation; the cancellation status message remains the user-visible evidence. + +## Alternatives Considered + +- **Run completion for every cancelled/stopped turn.** Rejected because clarification pauses, failures, parent stops, and teardown are not user declarations that a step is complete. +- **Add `on_turn_cancelled` actions.** Rejected for this iteration because the requested behavior is to reuse the existing completion contract; a second action language would duplicate configuration and complicate templates, import/export, and the editor. +- **Honor `auto_advance_requires_signal` during configured user cancellation.** Rejected because the user explicitly requested cancellation completion and an interrupted agent normally cannot emit the required signal. +- **Enable the database default or backfill every `simple` workflow.** Rejected because it would silently change customized and existing workflows. The product default belongs in the embedded template used for new instances. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 0d48cf612b..f1adebdd7f 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -104,6 +104,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-08-01-release-toggle-gating-contract | [Release Toggles Are Install-Wide Fail-Closed Gates](2026-08-01-release-toggle-gating-contract.md) | proposed | backend, frontend, workflow | 2026-08-01 | | 2026-08-01-share-artifact-locale | [Shared-Task Artifacts Render in the Creator's Locale](2026-08-01-share-artifact-locale.md) | accepted | backend | 2026-08-01 | | 2026-08-02-new-workspace-github-access-defaults | [Bootstrap New Workspaces From Host GitHub Access](2026-08-02-new-workspace-github-access-defaults.md) | accepted | backend, frontend, security | 2026-08-02 | +| 2026-08-02-explicit-user-cancel-completion | [Explicit User Cancellation May Complete a Workflow Step](2026-08-02-explicit-user-cancel-completion.md) | accepted | workflow | 2026-08-02 | | 2026-08-02-single-owner-agent-task-titles | [Assign Agent Task Titles to One Session](2026-08-02-single-owner-agent-task-titles.md) | accepted | backend, frontend, protocol, workflow | 2026-08-02 | | 2026-08-02-agent-terminal-diagnostics-over-stderr | [Capture Agent Terminal Diagnostics From Managed Stderr](2026-08-02-agent-terminal-diagnostics-over-stderr.md) | accepted | backend, frontend, protocol, security | 2026-08-02 | | 2026-08-02-class-aware-git-subprocess-admission | [Class-Aware Git Subprocess Admission](2026-08-02-class-aware-git-subprocess-admission.md) | accepted | backend, agentctl, observability | 2026-08-02 | diff --git a/docs/plans/cancelled-turn-completion/plan.md b/docs/plans/cancelled-turn-completion/plan.md new file mode 100644 index 0000000000..a88ba02611 --- /dev/null +++ b/docs/plans/cancelled-turn-completion/plan.md @@ -0,0 +1,219 @@ +--- +spec: docs/specs/workflow/cancelled-turn-completion/spec.md +created: 2026-08-02 +status: implemented +--- + +# Implementation Plan: Cancelled Turn Completion + +## Overview + +Add the persisted workflow-step policy and template default first, expose it through every existing workflow configuration contract, then route only explicit user cancellation through the established completion pipeline. Wire the setting into the shared workflow editor with localized, touch-safe presentation, document the public contract, and finish with desktop and mobile Playwright coverage. Existing workflow rows stay unchanged. + +--- + +## Backend + +### Persisted step contract and template defaults + +Files: + +- `apps/backend/internal/workflow/models/models.go` +- `apps/backend/internal/workflow/models/export.go` +- `apps/backend/config/workflows/loader.go` +- `apps/backend/config/workflows/kanban.yml` +- `apps/backend/internal/workflow/repository/sqlite.go` +- `apps/backend/internal/task/repository/sqlite/workspace_bootstrap.go` +- `apps/backend/internal/workflow/service/service.go` +- `apps/backend/internal/workflow/service/sync_apply.go` + +Changes: + +- Add `CancelTriggersTurnComplete bool` to `WorkflowStep`, `StepDefinition`, and `StepPortable`, serialized as `cancel_triggers_turn_complete`. +- Add a replay-safe `workflow_steps.cancel_triggers_turn_complete` migration with database default `false`, and thread the field through create, update, select, scan, template instantiation, default-step seeding, workspace bootstrap, portable import/export, and workflow-sync equality/application. +- Extend the embedded template loader so the YAML field is decoded and copied into step definitions. +- Set the field to `true` on the `simple` template's `Backlog` and `In Progress` steps only. Do not backfill existing rows and do not change custom-step defaults. +- Verify fresh/replayed SQLite schema behavior, the env-gated Postgres bootstrap/round trip, repository round trips, template instantiation, default workspace bootstrap, portable import/export, and sync preservation. + +### HTTP, WebSocket, and config MCP surfaces + +Files: + +- `apps/backend/internal/workflow/controller/controller.go` +- `apps/backend/internal/workflow/handlers/handlers.go` +- `apps/backend/internal/mcp/handlers/config_workflow_handlers.go` +- `apps/backend/internal/mcp/server/config_handlers.go` +- `apps/backend/config/prompts/config-context.md` + +Changes: + +- Extend create/update request structs with optional `cancel_triggers_turn_complete` pointers so omitted create means `false` and omitted update preserves the stored value. +- Return the effective boolean in workflow-step responses and WebSocket step mutation events. +- Add the field to config-mode MCP create/update schemas, payload forwarding, list output, and config-agent prompt documentation. +- Keep authorization, synced-workflow immutability, and the existing cancel WebSocket action unchanged. + +### Explicit user-cancel routing + +Files: + +- `apps/backend/internal/orchestrator/task_operations.go` +- `apps/backend/internal/orchestrator/event_handlers_workflow.go` +- `apps/backend/internal/orchestrator/task_operations_test.go` +- `apps/backend/internal/orchestrator/event_handlers_step_completion_test.go` + +Changes: + +- Introduce a typed completion cause or equivalent private helper input so ordinary agent completion continues to require the pending signal when configured, while a configured explicit user cancellation can bypass that agent-owned gate. +- In `Service.CancelAgent`, preserve authorization, deduplication, runtime cancellation, the visible cancellation message, turn completion, parked queue behavior, and session reconciliation. Reconciliation must return an authoritative result: confirm `WAITING_FOR_INPUT`, close every active turn, and verify no turn remains before evaluating `on_turn_complete` exactly once only when the current non-Office, non-ephemeral, non-archived Kanban step enables the policy. Any state or turn persistence failure fails closed and leaves the workflow transition untouched. +- Retain the clarification barrier, pending/stale-event guards, transition failure behavior, `on_exit`, terminal-state handling, and asynchronous destination `on_enter` processing. +- Avoid a duplicate or transient `REVIEW` state write when a workflow transition owns the final task state (especially terminal transitions); reconcile `REVIEW` after a confirmed no-transition or successful nonterminal transition, while terminal transitions remain direct. +- Keep `cancelAgentSilent`, peer interruption, parent stop, archive, provider error, `AgentStopped`, and other runtime teardown paths outside this policy. + +--- + +## Frontend + +### Workflow-step contract and draft persistence + +Files: + +- `apps/web/lib/types/http.ts` +- `apps/web/app/actions/workspaces.ts` +- `apps/web/components/settings/workflow-card-actions.ts` + +Changes: + +- Add `cancel_triggers_turn_complete` to backend normalization, create/update payloads, workflow-step types, draft equality, temporary-step remapping, and save payloads. +- Preserve omitted/false semantics so new custom steps remain off and template-derived steps retain their server-provided value. + +### Workflow settings UI + +Files: + +- `apps/web/components/settings/workflow-pipeline-editor-step-actions.tsx` +- `apps/web/components/settings/workflow-pipeline-editor-step-actions.test.tsx` (new) +- `apps/web/src/locales/en/settings.json` +- `apps/web/src/locales/pseudo/settings.json` + +Changes: + +- Under a configured `On Turn Complete` transition, add a localized checkbox labeled **Run completion actions when a turn is cancelled**. +- Explain that the option applies only to a user cancel and that destination actions may immediately start another agent. +- Reuse the existing step-editor update/dirty-state path and disable the control for read-only synced workflows. +- Add stable test IDs and component coverage for default-off, persisted-on, dirty-state, update, and read-only behavior. + +### Mobile design contract + +- **Desktop outcome and mobile entry point:** both use Settings → Workflows → workflow card → selected step → On Turn Complete. No capability is hidden on phones. +- **Nearest shipped exemplar:** the adjacent `ExplicitCompletionToggle` provides the inline checkbox/help pattern; `apps/web/e2e/tests/workflow/mobile-workflow-settings.spec.ts` provides the narrow-viewport settings flow and geometry checks. +- **Hierarchy and primary action:** the cancellation checkbox remains subordinate to the step's completion transition; its associated semantic label is the actual touch target and spans at least 44px on phones. +- **Presentation:** inline inside the existing focused step editor. A drawer would add unnecessary navigation for one boolean setting. +- **Scroll and geometry:** preserve the settings page's existing document scroll owner, use a touch-sized row/label target, and introduce no fixed controls, nested scrolling, or horizontal overflow. +- **Shared logic:** desktop and mobile use the same `WorkflowStep` draft and update callback; only responsive spacing/touch sizing may differ. +- **Mobile proof:** a `mobile-*.spec.ts` flow enables and saves the setting by touch, reloads it, cancels a live turn, and observes the same workflow transition without document overflow. + +--- + +## Public Documentation + +Files: + +- `docs/public/tasks-and-workflows.md` +- `docs/public/workflow-tips.md` +- `docs/public/workflow-import-export.md` +- `docs/public/workflow-sync.md` + +Changes: + +- Document pause-in-place versus complete-and-advance behavior, the standard Kanban template default for newly created workflows, and the absence of an upgrade backfill. +- Add `cancel_triggers_turn_complete` to the workflow import/export reference and synchronized workflow examples. +- Explain the interaction with `auto_advance_requires_signal`, clarification barriers, destination auto-start actions, and explicit user cancellation scope. +- Keep `workflow-import-export.md` and `workflow-sync.md` as reference pages; keep `tasks-and-workflows.md` and `workflow-tips.md` task-oriented how-to/explanation pages. + +--- + +## Tests + +- **What:** schema default/replay, repository round-trip, loader/template copy, default Kanban values, portable import/export, workspace bootstrap, and workflow sync preserve `cancel_triggers_turn_complete`. + **Files:** `apps/backend/config/workflows/loader_test.go`, `apps/backend/internal/workflow/repository/sqlite_test.go`, `apps/backend/internal/workflow/models/export_test.go`, `apps/backend/internal/workflow/service/service_test.go`, `apps/backend/internal/workflow/service/sync_apply_test.go`, `apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go`, `apps/backend/internal/task/repository/sqlite/postgres_schema_test.go`. + **How:** table-driven model/loader tests plus real SQLite repository and service tests; include same-database migration replay and an env-gated Postgres bootstrap/round trip. + +- **What:** HTTP/config MCP create, update, list, schema, and event payloads preserve omitted-versus-false semantics. + **Files:** controller/handler tests beside the changed packages. + **How:** request/response tests that create `true`, explicitly update to `false`, and omit on update. + +- **What:** enabled explicit user cancellation runs completion exactly once for acknowledged, escalated, and missing-execution reconciliation; disabled steps remain in place. + **File:** `apps/backend/internal/orchestrator/task_operations_test.go`. + **How:** real repository service tests with action-bearing steps and deterministic mock cancellation results, plus injected session-state and turn-close failures that prove no transition runs before reconciliation settles. + +- **What:** signal-gated cancellation bypasses only the agent signal, pending clarification still blocks, archived/Office/ephemeral tasks do not move, queued messages remain parked, terminal targets complete, destination auto-start remains active, and stale ready/completed events cannot double-transition. + **Files:** `apps/backend/internal/orchestrator/task_operations_test.go`, `apps/backend/internal/orchestrator/event_handlers_step_completion_test.go`. + **How:** focused regression cases reusing the existing cancel guard, workflow engine, clarification, and queue harnesses. + +- **What:** frontend normalization/draft comparison and the settings toggle preserve the field and emit one update with localized, read-only, dirty-state, and touch-target behavior. + **Files:** `apps/web/components/settings/workflow-card-actions.test.ts`, `apps/web/components/settings/workflow-pipeline-editor-step-actions.test.tsx`, `apps/web/lib/api/domains/workflow-api.test.ts`. + **How:** pure draft tests plus Testing Library interaction tests. + +--- + +## E2E Tests + +- **Scenario:** a desktop user enables cancellation completion, saves, starts a delayed turn, cancels it, and sees the task move to the configured destination exactly once while the cancellation message remains visible. + **File:** `apps/web/e2e/tests/workflow/workflow-cancel-completion.spec.ts`. + **What to verify:** persisted setting after reload, active-turn cancel through the UI, destination step/board position, session input readiness, and no duplicate destination auto-start. + +- **Scenario:** a step leaves the option disabled. + **File:** `apps/web/e2e/tests/workflow/workflow-cancel-completion.spec.ts`. + **What to verify:** cancelling returns the same session to input-ready state without moving the workflow step. + +- **Scenario:** a phone user enables and saves the setting by touch, reloads it, cancels a delayed turn, and observes the same destination step. + **File:** `apps/web/e2e/tests/workflow/mobile-workflow-cancel-completion.spec.ts`. + **What to verify:** the label's 44px touch target, touch-reachable setting and cancel controls, persisted checked state, workflow transition, no document horizontal overflow, and the existing single scroll owner. + +--- + +## Verification Results + +- Backend persistence/configuration packages passed their focused `go test -tags fts5` suites; the cancellation-focused orchestrator suite passed 30 tests, including injected session-state/turn-close failures and terminal-state ordering. +- The broad `make -C apps/backend test` run reached all packages but stopped on the pre-existing `internal/gateway/websocket/TestTaskEventBroadcaster_NoDuplicateSubscriptions` expectation (62 subscriptions vs the test's 61); no gateway files are part of this change, and the feature's relevant backend packages remain green. +- Frontend focused tests passed (29 workflow API/settings action/component tests, including create-step forwarding), web typecheck and lint passed, and both i18n ratchet/check commands passed. +- Public documentation validation passed (58 tests and 41 published pages). +- Managed Playwright coverage passed: desktop enabled/disabled scenarios 2/2 and a clean follow-up mobile run 1/1; the mobile assertion measures the associated label's 44px target. + +--- + +## Implementation Waves And Parallel Candidates + +Wave 1: + +- [x] [task-01-step-contract-and-template](task-01-step-contract-and-template.md) + +Wave 2 (parallel candidates after Task 01; user authorization required): + +- [x] [task-02-configuration-surfaces](task-02-configuration-surfaces.md) +- [x] [task-03-cancellation-routing](task-03-cancellation-routing.md) + +Wave 3 (parallel candidates after their dependencies; user authorization required): + +- [x] [task-04-settings-ui](task-04-settings-ui.md) +- [x] [task-05-public-docs](task-05-public-docs.md) + +Wave 4: + +- [x] [task-06-e2e-coverage](task-06-e2e-coverage.md) + +The default execution order is sequential in the primary conversation. Wave labels do not authorize subagents. + +--- + +## Risks + +- The cancel path and late `agent.ready` handling share a per-session guard. Completion evaluation must happen after cancellation owns the turn while preserving the stale-event rejection that prevents double transitions. +- `on_turn_complete` can include side effects and a destination can auto-start. The setting must be described as running completion actions, not merely moving a card. +- Template steps are instantiated through several paths: new workspace bootstrap, default empty-workflow seeding, backend template service, frontend draft creation, import, and sync. Missing one path would make the advertised standard Kanban default inconsistent. +- `auto_advance_requires_signal` and the clarification hard barrier are separate gates. Only the former is bypassed for configured user cancellation. +- The standard template change affects new E2E seed workflows and may change the workflow step reached by existing cancel/recovery tests even though their same-session and parked-queue assertions remain valid. + +## Open Questions + +None. The user-approved direction is the per-step boolean, enabled in the standard Kanban template for new workflows without backfilling existing rows. diff --git a/docs/plans/cancelled-turn-completion/task-01-step-contract-and-template.md b/docs/plans/cancelled-turn-completion/task-01-step-contract-and-template.md new file mode 100644 index 0000000000..961eb7f09b --- /dev/null +++ b/docs/plans/cancelled-turn-completion/task-01-step-contract-and-template.md @@ -0,0 +1,77 @@ +--- +id: "01-step-contract-and-template" +title: "Workflow step persistence contract" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/workflow/cancelled-turn-completion/spec.md" +--- + +# Task 01: Workflow step persistence contract + +## Acceptance + +- `cancel_triggers_turn_complete` is a persisted non-null workflow-step boolean with a replay-safe `false` database default and complete create/update/read coverage. +- Template loading, all template-instantiation paths, portable import/export, and workflow sync preserve the field; newly instantiated `simple` workflows enable it on `Backlog` and `In Progress` only. +- Existing workflow rows are not backfilled, and focused tests cover fresh SQLite schema, same-database replay, the env-gated Postgres path, custom defaults, and standard-template values. + +## Verification + +```bash +(cd apps/backend && go test -tags fts5 ./config/workflows ./internal/workflow/models ./internal/workflow/repository ./internal/workflow/service ./internal/task/repository/sqlite -run 'Test.*CancelTriggersTurnComplete' -count=1) +``` + +## Files Likely Touched + +- `apps/backend/internal/workflow/models/models.go` +- `apps/backend/internal/workflow/models/export.go` +- `apps/backend/internal/workflow/models/export_test.go` +- `apps/backend/config/workflows/loader.go` +- `apps/backend/config/workflows/loader_test.go` +- `apps/backend/config/workflows/kanban.yml` +- `apps/backend/internal/workflow/repository/sqlite.go` +- `apps/backend/internal/workflow/repository/sqlite_test.go` +- `apps/backend/internal/task/repository/sqlite/workspace_bootstrap.go` +- `apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go` +- `apps/backend/internal/task/repository/sqlite/postgres_schema_test.go` +- `apps/backend/internal/workflow/service/service.go` +- `apps/backend/internal/workflow/service/service_test.go` +- `apps/backend/internal/workflow/service/sync_apply.go` +- `apps/backend/internal/workflow/service/sync_apply_test.go` + +## Dependencies + +None. + +## Parallelism + +Sequential. This task owns the shared persisted model, migration, template, and portable contract required by every later task. + +## Inputs + +- Spec sections `What`, `Data Model`, `Persistence Guarantees`, and standard-template scenarios. +- ADR `2026-08-02-explicit-user-cancel-completion`. +- Existing `AutoAdvanceRequiresSignal` model/repository/import/export wiring as the nearest field pattern. +- ADR 0027 and backend schema replay guidance. + +## Risks + +- Missing any template-instantiation path creates different defaults depending on whether a workflow came from workspace bootstrap, empty-workflow repair, backend template creation, import, or sync. +- Do not use a data backfill or a database default of `true`; the product default belongs only to the embedded `simple` template. + +## Output Contract + +Report the field contract, migration behavior, exact template steps enabled, files changed, focused test results, blockers, and residual risks. Update this task and `plan.md` status in the same conversation. + +## Results + +Implemented the persisted `cancel_triggers_turn_complete` boolean with a replay-safe SQLite default of `false`, wired it through the workflow model, loader, standard Kanban template, SQLite repository, workspace/default template bootstraps, portable import/export, and sync reconciliation. The standard template enables it only for Backlog and In Progress; existing rows remain disabled after migration replay. + +Verification: + +- `rtk go test -tags fts5 ./config/workflows ./internal/workflow/models ./internal/workflow/repository ./internal/workflow/service -run 'Test.*CancelTriggersTurnComplete' -count=1` — 5 tests passed. +- `rtk go test -tags fts5 ./internal/task/repository/sqlite -run 'TestCreateWorkspaceWithKanban_CancelTriggersTurnCompleteDefaults' -count=1` — 1 test passed. +- `rtk go test -tags fts5 ./internal/workflow/service -run 'TestApplySyncedWorkflows_PreservesCancelTriggersTurnComplete' -count=1` — 1 test passed. +- `rtk go test -tags fts5 ./config/workflows ./internal/workflow/models ./internal/workflow/repository ./internal/workflow/service ./internal/task/repository/sqlite` — 392 tests passed across 5 packages. +- `rtk git diff --check` — passed. diff --git a/docs/plans/cancelled-turn-completion/task-02-configuration-surfaces.md b/docs/plans/cancelled-turn-completion/task-02-configuration-surfaces.md new file mode 100644 index 0000000000..8af3deae40 --- /dev/null +++ b/docs/plans/cancelled-turn-completion/task-02-configuration-surfaces.md @@ -0,0 +1,68 @@ +--- +id: "02-configuration-surfaces" +title: "Workflow configuration surfaces" +status: done +wave: 2 +depends_on: ["01-step-contract-and-template"] +plan: "plan.md" +spec: "../../specs/workflow/cancelled-turn-completion/spec.md" +--- + +# Task 02: Workflow configuration surfaces + +## Acceptance + +- HTTP workflow-step create/update/list/get contracts and workflow-step WebSocket events expose `cancel_triggers_turn_complete` with correct omitted create, omitted update, explicit true, and explicit false behavior. +- Config-mode MCP create/update schemas forward the optional field, list output returns it explicitly, and config-agent prompt documentation names it. +- Existing workflow mutation authorization and synced-workflow read-only behavior remain unchanged. + +## Verification + +```bash +(cd apps/backend && go test -tags fts5 ./internal/workflow/controller ./internal/workflow/handlers ./internal/mcp/handlers ./internal/mcp/server -run 'Test.*CancelTriggersTurnComplete' -count=1) +``` + +## Files Likely Touched + +- `apps/backend/internal/workflow/controller/controller.go` +- `apps/backend/internal/workflow/controller/controller_test.go` +- `apps/backend/internal/workflow/handlers/handlers.go` +- `apps/backend/internal/workflow/handlers/handlers_test.go` +- `apps/backend/internal/mcp/handlers/config_workflow_handlers.go` +- `apps/backend/internal/mcp/handlers/config_workflow_handlers_test.go` +- `apps/backend/internal/mcp/server/config_handlers.go` +- `apps/backend/internal/mcp/server/config_handlers_test.go` +- `apps/backend/config/prompts/config-context.md` + +## Dependencies + +Task 01. + +## Parallelism + +Parallel-safe with Task 03 after Task 01: files are limited to controller/handler/MCP configuration surfaces and do not overlap orchestrator files. + +## Inputs + +- Spec `API Surface`, `Permissions`, and configuration scenarios. +- Existing `AutoAdvanceRequiresSignal` request, event, MCP schema, and list-output pattern. + +## Risks + +- Pointer semantics are required on update; replacing them with a plain boolean would make omission indistinguishable from explicitly disabling the setting. +- The MCP schema allow-list and forwarding allow-list must both include the field or the tool may advertise a value it silently drops. + +## Output Contract + +Report contract shapes, omission semantics, files changed, focused test results, blockers, and residual risks. Update this task and `plan.md` status in the same conversation. + +## Results + +Implemented the optional pointer contract for HTTP/WebSocket workflow-step create/update requests, emitted the effective flag in workflow-step mutation events, exposed the flag through config MCP schemas/forwarding/list output, updated the config-agent prompt, and included both signal-gate fields in the task workflow-step DTO converter. + +Verification: + +- `rtk go test -tags fts5 ./internal/workflow/handlers ./internal/mcp/handlers ./internal/mcp/server -run 'Test.*CancelTriggersTurnComplete' -count=1` — 7 tests passed. +- `rtk go test -tags fts5 ./internal/task/dto -run 'TestFromWorkflowStep_PreservesCancelTriggersTurnComplete' -count=1` — 1 test passed. +- `rtk go test -tags fts5 ./internal/workflow/controller ./internal/workflow/handlers ./internal/mcp/handlers ./internal/mcp/server ./internal/task/dto -run 'Test.*CancelTriggersTurnComplete' -count=1` — 8 tests passed across 5 packages. +- `rtk go test -tags fts5 ./internal/workflow/controller ./internal/workflow/handlers ./internal/mcp/handlers ./internal/mcp/server ./internal/task/dto` — 511 tests passed across 5 packages. diff --git a/docs/plans/cancelled-turn-completion/task-03-cancellation-routing.md b/docs/plans/cancelled-turn-completion/task-03-cancellation-routing.md new file mode 100644 index 0000000000..841ae94d95 --- /dev/null +++ b/docs/plans/cancelled-turn-completion/task-03-cancellation-routing.md @@ -0,0 +1,69 @@ +--- +id: "03-cancellation-routing" +title: "Explicit cancellation workflow routing" +status: done +wave: 2 +depends_on: ["01-step-contract-and-template"] +plan: "plan.md" +spec: "../../specs/workflow/cancelled-turn-completion/spec.md" +--- + +# Task 03: Explicit cancellation workflow routing + +## Acceptance + +- Only `Service.CancelAgent` evaluates configured cancellation completion, exactly once after turn/session reconciliation; disabled steps and every silent/system cancellation path retain current behavior. +- Cancellation reconciliation is authoritative: session state must be confirmed as `WAITING_FOR_INPUT` and no active turn may remain before completion evaluation. A session-state or turn-close failure fails closed and leaves the workflow step unchanged. +- Configured user cancellation bypasses the agent completion-signal gate but not the pending-clarification, archive, ownership, Office, ephemeral, or stale-event guards. +- A successful transition preserves ordinary `on_exit`, terminal, destination `on_enter`, and auto-start behavior; failed/no-transition paths remain input-ready and queued messages stay parked. +- Terminal transitions publish their terminal task state directly without a transient `REVIEW` write; no-transition and successful nonterminal outcomes reconcile the task to `REVIEW` after settlement. + +## Verification + +```bash +(cd apps/backend && go test -tags fts5 ./internal/orchestrator -run 'TestCancelAgent_(TriggersConfiguredTurnComplete|KeepsDisabledStep|BypassesAgentSignal|BlocksPendingClarification|SkipsIneligibleTask|HandlesReconciledRuntime|LeavesQueuedMessageParked|CannotDoubleTransition)|TestUserCancelCompletion_' -count=1) +``` + +## Files Likely Touched + +- `apps/backend/internal/orchestrator/task_operations.go` +- `apps/backend/internal/orchestrator/event_handlers_workflow.go` +- `apps/backend/internal/orchestrator/task_operations_test.go` +- `apps/backend/internal/orchestrator/event_handlers_step_completion_test.go` + +## Dependencies + +Task 01. + +## Parallelism + +Parallel-safe with Task 02 after Task 01: this task owns orchestrator behavior and tests only. + +## Inputs + +- Spec `State Machine`, `Failure Modes`, and cancellation scenarios. +- ADR `2026-08-02-explicit-user-cancel-completion`. +- `Service.CancelAgent`, `handleAgentReady`, `processOnTurnCompleteViaEngine`, `turnCompleteBlockedByUserInput`, and existing cancel/queue race tests. + +## Risks + +- Evaluate only after the cancel guard owns and settles the turn; a late ready event must see the non-running session and return without a second transition. +- Do not route `cancelAgentSilent`, `AgentStopped`, parent stop, archive, or failure cleanup through the helper. +- Preserve the cancellation message's original turn association and do not drain queued work as a side effect. + +## Output Contract + +Report the typed completion-cause boundary, transition ordering, files changed, exact tests/results, blockers, and residual race risks. Update this task and `plan.md` status in the same conversation. + +## Results + +Added a typed `turnCompletionCause` boundary and routed only an initially working, explicit `Service.CancelAgent` through the existing completion engine after session/message/turn settlement. User cancellation honors the per-step flag, bypasses only the agent signal gate, retains the clarification barrier, skips Office/ephemeral/archived/terminal tasks, and keeps the cancel guard held until evaluation completes. Silent cancellation remains outside the helper; stale ready events observe the settled session and cannot double-transition. + +Verification: + +- Initial RED run: `rtk go test -tags fts5 ./internal/orchestrator -run 'TestCancelAgent_(TriggersConfiguredTurnComplete|KeepsDisabledStep|BypassesAgentSignal|BlocksPendingClarification|SkipsIneligibleTask|HandlesReconciledRuntime)' -count=1` — 6 passing existing/negative cases and 6 expected failures before routing. +- `rtk go test -tags fts5 ./internal/orchestrator -run 'TestCancelAgent_(TriggersConfiguredTurnComplete|KeepsDisabledStep|BypassesAgentSignal|BlocksPendingClarification|SkipsIneligibleTask|HandlesReconciledRuntime)' -count=1` — 12 tests passed after implementation. +- `rtk go test -tags fts5 ./internal/orchestrator -run 'TestCancelAgent_CannotDoubleTransitionFromStaleReady' -count=1` — 1 test passed. +- `rtk go test -tags fts5 ./internal/orchestrator -run 'TestUserCancelCompletion_SilentCancelDoesNotTrigger' -count=1` — 1 test passed. +- `rtk go test -tags fts5 ./internal/orchestrator -run 'TestCancelAgent_(TriggersConfiguredTurnComplete|KeepsDisabledStep|BypassesAgentSignal|BlocksPendingClarification|SkipsIneligibleTask|HandlesReconciledRuntime|LeavesQueuedMessageParked|CannotDoubleTransition)|TestUserCancelCompletion_' -count=1` — 18 tests passed. +- `rtk go test -tags fts5 ./internal/orchestrator -run 'Test(CancelAgent|ReconcileCancelledTurn|UserCancelCompletion)' -count=1` — 30 tests passed, including injected session-state and turn-close failures and terminal-state ordering. diff --git a/docs/plans/cancelled-turn-completion/task-04-settings-ui.md b/docs/plans/cancelled-turn-completion/task-04-settings-ui.md new file mode 100644 index 0000000000..a963ec997b --- /dev/null +++ b/docs/plans/cancelled-turn-completion/task-04-settings-ui.md @@ -0,0 +1,78 @@ +--- +id: "04-settings-ui" +title: "Workflow cancellation setting UI" +status: completed +wave: 3 +depends_on: ["02-configuration-surfaces"] +plan: "plan.md" +spec: "../../specs/workflow/cancelled-turn-completion/spec.md" +--- + +# Task 04: Workflow cancellation setting UI + +## Acceptance + +- Frontend workflow-step normalization, create/update payloads, drafts, dirty comparison, and save flows preserve `cancel_triggers_turn_complete` without changing custom-step defaults. +- A localized, read-only-aware checkbox appears beneath configured turn-complete transitions, discloses destination auto-start behavior, and writes the shared step draft once. +- Desktop and phone use the same state/update logic; the phone's associated label is the actual 44px touch target and introduces no new drawer, nested scroll owner, or horizontal overflow. + +## Verification + +```bash +(cd apps && pnpm install --frozen-lockfile) +(cd apps && pnpm --filter @kandev/web test -- components/settings/workflow-card-actions.test.ts components/settings/workflow-pipeline-editor-step-actions.test.tsx) +(cd apps/web && pnpm run typecheck) +(cd apps && pnpm run i18n:ratchet && pnpm run i18n:check) +``` + +## Files Likely Touched + +- `apps/web/lib/types/http.ts` +- `apps/web/app/actions/workspaces.ts` +- `apps/web/components/settings/workflow-card-actions.ts` +- `apps/web/components/settings/workflow-card-actions.test.ts` +- `apps/web/components/settings/workflow-pipeline-editor-step-actions.tsx` +- `apps/web/components/settings/workflow-pipeline-editor-step-actions.test.tsx` +- `apps/web/src/locales/en/settings.json` +- `apps/web/src/locales/pseudo/settings.json` + +## Dependencies + +Task 02. + +## Parallelism + +Parallel-safe with Task 05 after Task 02: frontend source/locales and public documentation files are disjoint. + +## Inputs + +- Spec `What`, `API Surface`, desktop/mobile settings scenario, and mobile design contract in `plan.md`. +- Adjacent `ExplicitCompletionToggle` and workflow dirty-state/save patterns. +- `apps/web/e2e/tests/workflow/mobile-workflow-settings.spec.ts` as the nearest shipped phone interaction. + +## Risks + +- New copy must use the `settings` i18n namespace and pass the changed-line ratchet. +- The checkbox must be hidden when no transition exists and disabled for synced/read-only workflows without losing the persisted checked state. +- Do not fork business logic for mobile; only touch target/spacing may be responsive. + +## Output Contract + +Report the desktop/mobile interaction, translation keys, files changed, focused unit/type/i18n results, rendered-check evidence if performed, blockers, and residual risks. Update this task and `plan.md` status in the same conversation. + +## Results + +Implemented the frontend contract and editor control: + +- `WorkflowStep`, template normalization, draft creation, create/update actions, dirty comparison, and the domain API now preserve `cancel_triggers_turn_complete`. +- Added the localized, read-only-aware checkbox beneath configured turn-complete transitions. It uses a shared draft update path, retains persisted state in read-only mode, gives the associated semantic label a `min-h-11` touch target, and does not add a mobile drawer or scroll owner. +- Added English and pseudo-locale keys describing explicit user cancellation and destination `on_enter` auto-start behavior. + +Verification: + +- `rtk pnpm --filter @kandev/web test -- app/actions/workspaces.test.ts components/settings/workflow-card-actions.test.ts components/settings/workflow-pipeline-editor-step-actions.test.tsx` — 27 passed. +- `rtk pnpm --filter @kandev/web run typecheck` — passed. +- `rtk pnpm run i18n:ratchet && rtk pnpm run i18n:check` from `apps/web` — passed; 808 referenced keys, 0 orphan keys. +- `rtk pnpm --filter @kandev/web exec vitest run lib/api/domains/workflow-api.test.ts components/settings/workflow-pipeline-editor-step-actions.test.tsx` — 5 passed, including create-step API forwarding coverage. + +Residual risk: browser-level mobile rendering is covered by the Task 06 E2E scenarios rather than a separate screenshot run here. diff --git a/docs/plans/cancelled-turn-completion/task-05-public-docs.md b/docs/plans/cancelled-turn-completion/task-05-public-docs.md new file mode 100644 index 0000000000..58d5732ee0 --- /dev/null +++ b/docs/plans/cancelled-turn-completion/task-05-public-docs.md @@ -0,0 +1,63 @@ +--- +id: "05-public-docs" +title: "Cancelled turn public documentation" +status: completed +wave: 3 +depends_on: ["01-step-contract-and-template", "02-configuration-surfaces"] +plan: "plan.md" +spec: "../../specs/workflow/cancelled-turn-completion/spec.md" +--- + +# Task 05: Cancelled turn public documentation + +## Acceptance + +- User documentation explains pause-in-place versus configured complete-and-advance behavior, explicit user-only scope, signal/clarification interaction, and destination auto-start consequences. +- Reference documentation defines `cancel_triggers_turn_complete`, its `false` schema/import default, the new standard Kanban template default, and the absence of an upgrade backfill. +- Workflow import/export and sync examples preserve the field and public-doc validation passes. + +## Verification + +```bash +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +``` + +## Files Likely Touched + +- `docs/public/tasks-and-workflows.md` +- `docs/public/workflow-tips.md` +- `docs/public/workflow-import-export.md` +- `docs/public/workflow-sync.md` + +## Dependencies + +Tasks 01 and 02. + +## Parallelism + +Parallel-safe with Task 04 after Task 02: public Markdown files are disjoint from frontend implementation files. + +## Inputs + +- Entire feature spec and ADR. +- Existing completion-signal sections in each public page. +- Docs-maintainer classification: import/export and sync are reference; tasks/workflows and tips remain task-oriented guidance/explanation. + +## Risks + +- Do not imply that every runtime cancellation advances or that existing standard workflows are automatically migrated. +- Distinguish the database/import default (`false`) from the new embedded `simple` template value (`true` on two steps). + +## Output Contract + +Report pages changed, each page's primary content type, exact validator results, blockers, and residual documentation risks. Update this task and `plan.md` status in the same conversation. + +## Results + +Updated the public workflow guides to document the explicit-cancellation policy, its narrow eligibility boundary, the shared completion pipeline, destination `on_enter` auto-start implications, built-in Kanban defaults, and portable import/sync behavior. + +Verification: + +- `rtk node --test scripts/validate-public-docs.test.mjs` — 58 passed. +- `rtk node scripts/validate-public-docs.mjs` — validated 41 published docs pages. diff --git a/docs/plans/cancelled-turn-completion/task-06-e2e-coverage.md b/docs/plans/cancelled-turn-completion/task-06-e2e-coverage.md new file mode 100644 index 0000000000..0505370dde --- /dev/null +++ b/docs/plans/cancelled-turn-completion/task-06-e2e-coverage.md @@ -0,0 +1,74 @@ +--- +id: "06-e2e-coverage" +title: "Cancelled turn E2E coverage" +status: completed +wave: 4 +depends_on: ["03-cancellation-routing", "04-settings-ui"] +plan: "plan.md" +spec: "../../specs/workflow/cancelled-turn-completion/spec.md" +--- + +# Task 06: Cancelled turn E2E coverage + +## Acceptance + +- Desktop Playwright proves persisted enabled and disabled settings produce complete-and-advance and pause-in-place behavior through the real cancel button, with cancellation visibility and exactly one destination transition. +- Mobile Playwright enables/saves the setting by touch, reloads it, cancels a delayed turn, observes the same workflow transition, and asserts the associated label's 44px touch target, the existing scroll owner, and no document horizontal overflow. +- Existing pause/resume and parked-queue scenarios affected by the new `simple` template default are updated only where their expected workflow position changes; their same-session and queue contracts remain intact. + +## Verification + +```bash +(cd apps && pnpm install --frozen-lockfile) +(cd apps/web && pnpm e2e:run tests/workflow/workflow-cancel-completion.spec.ts) +(cd apps/web && pnpm e2e:run --no-build --project mobile-chrome tests/workflow/mobile-workflow-cancel-completion.spec.ts) +``` + +## Files Likely Touched + +- `apps/web/e2e/helpers/api-client.ts` +- `apps/web/e2e/pages/workflow-settings-page.ts` +- `apps/web/e2e/tests/workflow/workflow-cancel-completion.spec.ts` +- `apps/web/e2e/tests/workflow/mobile-workflow-cancel-completion.spec.ts` +- `apps/web/e2e/tests/session/pause-resume-recovery.spec.ts` (only if the template-default expectation changes) +- `apps/web/e2e/tests/session/mobile-pause-resume-recovery.spec.ts` (only if the template-default expectation changes) + +## Dependencies + +Tasks 03 and 04. + +## Parallelism + +Sequential. E2E depends on the complete backend and frontend behavior and may need to reconcile existing tests that use newly instantiated `simple` workflows. + +## Inputs + +- Every user-visible spec scenario selected in `plan.md` E2E coverage. +- Existing workflow settings, workflow auto-advance, pause/resume recovery, mobile pause, and API helper patterns. +- Mobile-parity requirement to use a `mobile-*.spec.ts` file, `.tap()`, touch-sized controls, and document-overflow assertions. + +## Risks + +- Use a deterministic delayed mock-agent turn so cancellation happens while the session is genuinely running; do not add fixed sleeps or inflate timeouts. +- Confirm Playwright discovers the mobile spec under `mobile-chrome` and that the second run reuses the freshly built artifacts from the first command. +- Preserve worker-scoped seed state and restore any canonical workflow-step edits in `test.afterEach`, or create disposable workflows instead. + +## Output Contract + +Report desktop/mobile scenarios and discovered test counts, files changed, exact managed-runner results, artifact/cleanup evidence, blockers, and residual flake risks. Update this task, `plan.md`, and `## Verification Results` in the same conversation. + +## Results + +Added managed Playwright coverage: + +- Desktop enabled/disabled scenarios seed a real turn with a delayed response, click the existing Cancel action, verify the cancellation settles, and assert advance-versus-stay plus a single surviving task session. +- Mobile coverage enables the setting through the workflow editor with `.tap()`, verifies persistence after reload, measures the actual `min-h-11` associated label, cancels a delayed turn by touch, verifies the destination, and asserts no document horizontal overflow. +- The E2E API helper accepts `cancel_triggers_turn_complete` updates and the workflow settings page exposes a touch-aware policy helper/save path. + +Verification: + +- `rtk pnpm e2e:run --no-build tests/workflow/workflow-cancel-completion.spec.ts` — 2 passed. +- `rtk pnpm e2e:run --no-build --project mobile-chrome tests/workflow/mobile-workflow-cancel-completion.spec.ts` — 1 passed. +- A follow-up managed mobile run passed 1/1 on the first attempt, including the label geometry assertion; the earlier transient settings hydration timeout did not reproduce. + +The initial managed runs exposed only test-harness assumptions (nullable empty history and a mobile-only stepper/cancel-button sizing difference); assertions were adjusted to the stable API/current-step and existing touch contracts. No product behavior changes were needed. diff --git a/docs/public/tasks-and-workflows.md b/docs/public/tasks-and-workflows.md index 810d487e7b..79704f84f6 100644 --- a/docs/public/tasks-and-workflows.md +++ b/docs/public/tasks-and-workflows.md @@ -279,6 +279,7 @@ Pull configuration rejects self-references, cycles, and cross-workflow feeders. | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | **On Turn Start** | Do nothing, move next, move previous, or move to a selected step when the user sends a message. | | **On Turn Complete** | **Do nothing (wait for user)**, move next, move previous, or move to a selected step after the agent turn. | +| **Cancelled turn completion** | When enabled, an explicit user cancellation also runs this step's normal `on_turn_complete` actions after the cancelled turn settles. It bypasses the `auto_advance_requires_signal` / `step_complete_kandev` gate for that cancellation, but a pending clarification still blocks the transition. It does not apply to silent clarification cancellation, peer interruptions, parent/task stops, provider errors, crashes, or runtime teardown. | | **When Child Tasks Complete** | Do nothing, move next, move previous, or move to a selected step after every active direct child reaches `COMPLETED`, `FAILED`, or `CANCELLED`, provided the parent has an active session. | The child-completion event ignores archived and ephemeral children, does not inspect grandchildren, and does nothing when the parent has no children. It also requires a parent session in `CREATED`, `STARTING`, `RUNNING`, or `WAITING_FOR_INPUT`; a parent with no session, or only an `IDLE`, `COMPLETED`, `FAILED`, or `CANCELLED` session, does not transition. @@ -287,6 +288,10 @@ Generic comment, blocker-resolution, approval, heartbeat, budget, and error trig When **On Turn Complete** moves a task, **Wait for agent completion signal** is available. With it enabled, a bare turn end leaves the task waiting; the agent must call `step_complete_kandev`. The call requires a summary and can include a handoff or blockers. It is idempotent within the step, runs asynchronously, and a user message sent before the transition is applied cancels that pending signal. Without the option, turn end counts as completion. +**Run completion actions when a turn is cancelled** is available beneath a configured turn-complete transition. It applies only when a user explicitly presses **Cancel** on the active turn. The normal completion pipeline still applies, including `on_exit`, the configured transition, and the destination step's `on_enter` actions; an `auto_start_agent` action there can start another turn immediately. An eligible explicit cancellation bypasses the `auto_advance_requires_signal` / `step_complete_kandev` gate, but a pending clarification still blocks the transition. The setting does not turn other interruption or failure paths into completion events. When the setting is off, an explicit cancel leaves the task in its current step and ready for input. + +The built-in **Kanban** workflow enables this policy on **Backlog** and **In Progress** and leaves it disabled on its other steps. Custom steps and imported definitions default to disabled unless they set the field explicitly. + An auto-started task stays in its current step while the agent session boots and while its first turn is running. A boot-ready event is not a turn completion. For example, a review step with `on_enter: auto_start_agent` and @@ -412,6 +417,7 @@ settled task in the still-working state. - **No agent starts:** the empty-description **Start Plan Mode** path does not use the normal start-agent submission. To begin an agent immediately, enter a description and use **Start task** or **Start task in plan mode**; also confirm the selected profiles are healthy and compatible. - **Task starts in the wrong step:** normal creation uses **Start step** with first-step fallback; **Start task in plan mode** deliberately uses the first positional step. - **A task moves unexpectedly:** inspect **On Turn Start**, **On Turn Complete**, child completion, entry actions, and the destination step's entry actions. +- **A task stays after a cancel:** check for a pending clarification, the cancelled-turn completion policy, an absent or blocked transition, a queued WIP card, or an invalid target left by an older definition. - **Move rejected:** check the target WIP limit and whether the task is already counted there. - **Pull does nothing:** configure a nonzero WIP limit, remove cycles, and confirm feeder candidates are not running or starting. - **Child completion does not move the parent:** confirm every active direct child is terminal and the parent still has a session in `CREATED`, `STARTING`, `RUNNING`, or `WAITING_FOR_INPUT`. diff --git a/docs/public/workflow-import-export.md b/docs/public/workflow-import-export.md index 5cf5cf7031..bd21a83138 100644 --- a/docs/public/workflow-import-export.md +++ b/docs/public/workflow-import-export.md @@ -5,7 +5,7 @@ description: "Move Kanban workflows between workspaces with Kandev's portable YA # Workflow Import / Export -Kandev's versioned portable format moves Kanban workflow definitions between workspaces or installations. It carries prompts, step behavior, portable agent-profile descriptors, WIP rules, and supported events. It deliberately omits database IDs and workspace ownership. +Kandev's versioned portable format moves Kanban workflow definitions between workspaces or installations. It carries prompts, step behavior, portable agent-profile descriptors, WIP rules, supported events, and the explicit-cancellation completion policy. It deliberately omits database IDs and workspace ownership. Use this for snapshots and one-time copies. Use [Workflow Sync](workflow-sync.md) when a GitHub repository should remain the source of truth. @@ -123,6 +123,7 @@ IDs, workspace ID, ordering among workflows, source/sync ownership, style, visib allow_manual_move: true auto_archive_after_hours: 0 auto_advance_requires_signal: true + cancel_triggers_turn_complete: true wip_limit: 2 pull_from_step_position: 0 agent_profile: @@ -142,6 +143,7 @@ IDs, workspace ID, ordering among workflows, source/sync ownership, style, visib | `auto_archive_after_hours` | integer | Omitted when `0`; `0` disables auto-archive. The portable validator currently does not reject negative values, so use only `0` or a positive value. | | `agent_profile` | object | Omitted when unset; exact-match behavior is below. | | `auto_advance_requires_signal` | boolean | Always exported. `true` makes `on_turn_complete` transitions wait for `step_complete_kandev`; missing input is `false`. | +| `cancel_triggers_turn_complete` | boolean | Always exported. `true` lets an explicit user cancellation run the step's normal `on_turn_complete` actions after the cancelled turn settles; missing input is `false`. Pending clarification and non-user interruption/failure paths are not eligible. | | `wip_limit` | integer | Omitted when `0`. Must be non-negative; `0` is unlimited. | | `pull_from_step_position` | integer | Optional feeder reference using another step's `position`. It must exist, cannot point to itself, and cannot form a pull cycle. | @@ -232,6 +234,7 @@ workflows: show_in_command_panel: false allow_manual_move: true auto_advance_requires_signal: false + cancel_triggers_turn_complete: false - name: Work position: 1 @@ -251,6 +254,7 @@ workflows: show_in_command_panel: true allow_manual_move: true auto_advance_requires_signal: true + cancel_triggers_turn_complete: true wip_limit: 2 pull_from_step_position: 0 @@ -266,6 +270,7 @@ workflows: show_in_command_panel: true allow_manual_move: true auto_advance_requires_signal: false + cancel_triggers_turn_complete: false ``` After import, assign a workflow-level or Work-step agent profile if the destination did not produce an exact portable profile match. Create a disposable task, verify Backlog → Work pulling, the WIP rejection at capacity, explicit completion, and Review feedback before adopting it. diff --git a/docs/public/workflow-sync.md b/docs/public/workflow-sync.md index 3ddd22f501..1fb66543b0 100644 --- a/docs/public/workflow-sync.md +++ b/docs/public/workflow-sync.md @@ -74,6 +74,7 @@ workflows: show_in_command_panel: true allow_manual_move: true auto_advance_requires_signal: false + cancel_triggers_turn_complete: false - name: Done position: 1 color: bg-green-500 @@ -82,6 +83,7 @@ workflows: show_in_command_panel: true allow_manual_move: true auto_advance_requires_signal: false + cancel_triggers_turn_complete: false ``` Commit the file, then use **Sync now**. The status card reports created, updated, deleted, warning, or unchanged results. @@ -99,7 +101,7 @@ These rules matter when editing definitions: - Manual workflows are never matched, updated, or removed by sync, even when their name is identical. - Synced workflows are read-only in normal workflow mutation paths. Edit the repository and sync again. Every run performs a full reconciliation, so it also repairs drift; the stored content hash is for status/observability, not a skip condition. -The portable format does not carry every internal or Office field. Sync reconciles the portable Kanban fields and preserves non-portable internal stage type. Do not use this facility as an Office-workflow backup. +The portable format does not carry every internal or Office field. Sync reconciles the portable Kanban fields, including `cancel_triggers_turn_complete`, and preserves non-portable internal stage type. Changing that field in GitHub changes whether an explicit user cancellation can run the step's normal completion actions on the next sync. Pending clarifications and non-user interruption/failure paths remain ineligible. Do not use this facility as an Office-workflow backup. ### Invalid and empty sources diff --git a/docs/public/workflow-tips.md b/docs/public/workflow-tips.md index 283cc3ea30..b0e23338bd 100644 --- a/docs/public/workflow-tips.md +++ b/docs/public/workflow-tips.md @@ -111,6 +111,7 @@ Workflow-level settings include the name and default agent profile. A step can o | Show in command panel | Includes tasks in this step in the command panel. | | Auto-archive | Archives eligible tasks after the configured number of hours. `0` disables it; the background sweep runs every five minutes and uses task `updated_at`, so timing is approximate. | | Wait for agent completion signal | With an `on_turn_complete` transition, waits for the agent to call `step_complete_kandev`. A halt without the signal leaves the task on the current step; retry or reconnect the agent, or move the task through the normal workflow UI. Without this setting, a normal turn end counts as completion. Default is off. | +| Run completion actions when a turn is cancelled | Also runs the step's `on_turn_complete` actions after an explicit user cancellation settles. A pending clarification, silent interruption, parent/task stop, provider failure, crash, or runtime teardown does not qualify. If the destination has `on_enter: auto_start_agent`, another agent turn can begin immediately. Default is off for custom steps; the built-in Kanban workflow enables it on Backlog and In Progress for newly created workflows; existing workflows are not backfilled. | | WIP limit | Maximum admitted active, non-archived, non-ephemeral tasks in the step. `0` means unlimited; visible overflow is queued. A full target rejects manual moves. | | Pull from | Optional one-hop feeder step. When capacity opens, Kandev promotes queued destination work first, then feeder work. A full feeder rejects new overflow creation. | @@ -127,13 +128,15 @@ The standard Kanban editor exposes these events: |-------|--------------|----------------| | `on_enter` | A task enters a step through normal step-entry processing. | Enable plan mode, auto-start agent, reset context. | | `on_turn_start` | A user sends a message. The transition happens before that message is delivered. | Move next, previous, or to a selected step. | -| `on_turn_complete` | An agent turn finishes, unless a clarification is still pending or explicit completion is required but absent. | Move next, previous, or to a selected step; disable plan mode. | +| `on_turn_complete` | A normal agent turn finishes when its signal requirements are satisfied. An explicit user cancellation qualifies when the step enables its cancellation policy, even if the completion signal is absent. A pending clarification always blocks completion. | Move next, previous, or to a selected step; disable plan mode. | | `on_exit` | A task leaves a step. | Disable plan mode. | The portable format also recognizes `set_session_mode`, `clear_decisions`, `queue_run`, and `queue_run_for_each_participant` in `on_enter`; these are advanced/runtime-dependent actions and most are not offered by the Kanban editor. Office event triggers have a broader model, but do not round-trip through Kanban import/export. See the exact boundary in [Workflow Import / Export](workflow-import-export.md). Keep one transition action per event. A “next” action on the last step or “previous” on the first has nowhere to go and leaves the task in place. WIP rejection, a missing target step, a failed agent launch, or missing credentials can also prevent the intended progression; inspect the task/session error and backend logs before changing the workflow. +Cancellation policy is deliberately narrow: it is evaluated only for the visible user **Cancel** action while a turn is working. It reuses the normal turn-complete pipeline after the runtime settles, but does not reinterpret every way a session can stop as a completion. + ## Safe authoring pattern @@ -147,8 +150,9 @@ Keep one transition action per event. A “next” action on the last step or 2. Add `auto_start_agent` only to steps that always have an effective agent profile. 3. Add turn-complete transitions after the prompt has an unambiguous stop condition. 4. Enable the explicit completion signal for agents that can discover and call `step_complete_kandev`; otherwise the step can wait indefinitely. If the tool is not already visible, the agent should search the active tool catalog for its canonical name. -5. Add WIP limits before pull rules, then test a full target and a vacated slot. -6. Export the workflow before a large edit. Workflow deletion is permanent; when it contains tasks, the UI asks you to migrate them or archive them. +5. Decide whether an explicit user cancellation should run the same completion actions. Enable **Run completion actions when a turn is cancelled** only when the destination transition is safe to repeat and any destination `on_enter` automation is intentional. +6. Add WIP limits before pull rules, then test a full target and a vacated slot. +7. Export the workflow before a large edit. Workflow deletion is permanent; when it contains tasks, the UI asks you to migrate them or archive them. @@ -183,6 +187,7 @@ A task may contain several repositories, but a workflow step is not bound to one - **Task starts in the wrong column:** confirm exactly one Start step, save the workflow, and check whether the creator supplied an explicit `workflow_step_id`. - **Agent does not start:** verify the effective workflow/step agent profile, its health, executor profile, repository access, and the `auto_start_agent` entry action. - **Task stays after a turn:** check for an absent transition, a pending clarification, the explicit-completion toggle, a queued WIP card waiting for capacity, or an invalid target left by an older definition. +- **Task stays after a cancel:** check for a pending clarification, the cancelled-turn completion policy, an absent or blocked transition, a queued WIP card, or an invalid target left by an older definition. - **Task cannot be dragged:** the destination may disallow manual moves, be at its WIP limit, or the task may have a starting/running session. - **Auto-archive looks late:** the sweep cadence is five minutes and task updates extend the age check. - **Synced workflow is read-only:** edit its repository definition and run Sync now, or remove the sync configuration to release all synced workflows as editable manual workflows. diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index a59eef6a61..5e1181c9eb 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -82,6 +82,7 @@ Kandev's task model: documents, execution stages, labels, blocker escalation, su | [task-create-executor-default](tasks/task-create-executor-default.md) | approved | | [sidebar-task-edit](tasks/sidebar-task-edit.md) | approved | | [explicit-completion-signal](workflow/explicit-completion-signal/spec.md) | shipped | +| [cancelled-turn-completion](workflow/cancelled-turn-completion/spec.md) | shipped | | [conditional-session-settings](workflow-session-settings/spec.md) | approved | ## agents/ — agent governance diff --git a/docs/specs/workflow/cancelled-turn-completion/spec.md b/docs/specs/workflow/cancelled-turn-completion/spec.md new file mode 100644 index 0000000000..f4fcda0cb9 --- /dev/null +++ b/docs/specs/workflow/cancelled-turn-completion/spec.md @@ -0,0 +1,113 @@ +--- +status: shipped +created: 2026-08-02 +owner: kandev +--- + +# Cancelled Turn Completion + +Decision: [ADR-2026-08-02-explicit-user-cancel-completion](../../../decisions/2026-08-02-explicit-user-cancel-completion.md) + +## Why + +Cancelling an active Kanban agent turn currently stops the agent and marks the runtime task state ready for review, but it leaves the task on its existing workflow step. Users who configure a step to move on turn completion need an explicit choice over whether their own cancellation should finish that step or merely pause it. + +## What + +- Each workflow step exposes `cancel_triggers_turn_complete`, a boolean setting controlling whether an explicit user cancellation evaluates that step's existing `on_turn_complete` actions. +- The setting defaults to `false` for newly created custom steps, imported files that omit it, and existing persisted workflow steps. Existing workflows are not backfilled. +- The embedded `simple` Kanban template enables the setting on `Backlog` and `In Progress`, the steps that already define `on_turn_complete` transitions. New workflows instantiated from that template inherit the enabled value. +- The policy applies only when a user invokes the visible task-session cancel action. Silent clarification pauses, peer-message interrupts, parent-task stops, task archival, runtime teardown, provider errors, crashes, and other system-owned cancellations never evaluate completion actions through this setting. +- A configured user cancellation reuses the ordinary `on_turn_complete` action pipeline. It runs source-step `on_exit`, applies at most one transition, and runs destination-step `on_enter` with the same first-transition-wins and side-effect behavior as a normal completed turn. +- A configured user cancellation is a human completion decision and does not require `step_complete_kandev`, even when `auto_advance_requires_signal=true`. A pending clarification remains a hard barrier and still blocks the transition. +- Cancellation keeps the current session and conversation context. It records the existing cancellation status message, durably confirms `WAITING_FOR_INPUT` and closes every active turn before evaluating completion, leaves queued user messages parked, and ignores the cancelled turn's later stale ready/completion events. +- If completion actions do not produce a transition, cannot be evaluated safely, or are blocked by pending user input, the task remains on the current workflow step and retains the existing `WAITING_FOR_INPUT` / review-ready reconciliation. +- If completion reaches a terminal workflow step, the task receives the same completed state as an ordinary `on_turn_complete` transition without an intermediate `REVIEW` state write. +- The workflow settings UI shows the option only when the step has an `on_turn_complete` transition. Its help text explains that destination `on_enter` actions may immediately start another agent. +- Desktop and mobile workflow settings expose the same capability. On phones, the option remains an inline, touch-sized setting inside the existing focused step editor; it does not introduce a new drawer or navigation level. + +## Data Model + +`workflow_steps` gains: + +| Field | Type | Default | Meaning | +|---|---|---|---| +| `cancel_triggers_turn_complete` | boolean, non-null | `false` | An explicit user cancellation evaluates this step's `on_turn_complete` actions. | + +`WorkflowStep`, `StepDefinition`, and the portable workflow step representation expose the same field. The embedded workflow-template YAML accepts `cancel_triggers_turn_complete`; template instantiation copies it into the persisted workflow step. + +The portable workflow import/export format always exports the boolean. Missing input imports as `false`. Workflow sync compares and applies it as part of the step contract. + +## API Surface + +The existing workflow-step create and update contracts add an optional boolean: + +```json +{ + "cancel_triggers_turn_complete": true +} +``` + +- Create with the field omitted persists `false`. +- Update with the field omitted leaves the current value unchanged. +- HTTP list/get responses and workflow-step WebSocket events return the effective boolean explicitly. +- `create_workflow_step_kandev` and `update_workflow_step_kandev` accept the same optional field in config mode; `list_workflow_steps_kandev` returns it. +- No new task-session cancel endpoint or WebSocket action is introduced. The existing authorized cancel action owns the behavior. + +## State Machine + +| Starting condition | Trigger | Result | +|---|---|---| +| Running Kanban turn; setting `false` | User cancels | Turn stops, session becomes input-ready, task remains on the current workflow step. | +| Running Kanban turn; setting `true` | User cancels | Cancellation bookkeeping authoritatively settles the session and turn, then the current step's `on_turn_complete` actions evaluate once. | +| Setting `true`; pending clarification | User cancels | Turn stops and remains on the current step; the clarification barrier blocks completion. | +| Setting `true`; transition succeeds | Cancellation settles | Source `on_exit`, workflow move, and destination `on_enter` run with ordinary completion semantics. | +| Setting `true`; transition reaches a terminal step | Cancellation settles | The workflow transition owns the final task state; no transient `REVIEW` state is published first. | +| Setting `true`; no eligible transition or evaluation failure | Cancellation settles | Session remains input-ready on the current workflow step. | +| Any setting | Internal or system-owned cancellation | No cancellation-driven completion evaluation occurs. | + +## Permissions + +No new permission is introduced. The caller must already be authorized to cancel the target task session. Workflow-step configuration continues to use the existing workflow mutation authorization and read-only synced-workflow rules. + +## Failure Modes + +- If the runtime cancel returns a non-recoverable error, the cancel request fails and Kandev does not reconcile the session or evaluate completion actions. +- If no live execution exists or the runtime cancellation must be escalated, Kandev performs its existing safe reconciliation. Because the user cancellation was accepted, an enabled step may still evaluate completion actions. +- If the task is archived, ephemeral, Office-owned, missing its workflow step, or the current step cannot be loaded, Kandev does not move it through this policy. +- If the session cannot be authoritatively persisted as `WAITING_FOR_INPUT`, or any active turn cannot be closed and verified, Kandev returns the cancellation reconciliation error and does not evaluate `on_turn_complete`, move the workflow, or launch destination `on_enter` actions. +- If workflow evaluation or persistence fails, Kandev leaves the session input-ready on the current workflow step and surfaces the failure through existing logs and workflow state. +- A stale `agent.ready` or `agent.completed` event emitted after cancellation cannot run the transition a second time because the reconciled session no longer owns a running turn. +- Destination `on_enter` behavior is not suppressed. If it contains `auto_start_agent`, cancellation can intentionally start work on the destination step; the settings help text must disclose this consequence. + +## Persistence Guarantees + +The setting survives restart with the workflow step. No cancellation-specific session metadata or new transition record is added. Successful moves continue to use the existing workflow transition history with trigger `on_turn_complete`; the visible cancellation status message provides the user-facing cancellation record. + +Template defaults apply when a workflow's steps are instantiated. Changing the embedded template does not rewrite existing persisted workflows, including workflows originally created from `simple`. + +## Scenarios + +- **GIVEN** a custom workflow step with an `on_turn_complete` move and `cancel_triggers_turn_complete=false`, **WHEN** the user cancels its active turn, **THEN** the session becomes input-ready and the task remains on that step. +- **GIVEN** the same step with `cancel_triggers_turn_complete=true`, **WHEN** the user cancels its active turn, **THEN** the move runs exactly once and the task appears on the configured destination step. +- **GIVEN** a signal-gated step with both `auto_advance_requires_signal=true` and `cancel_triggers_turn_complete=true`, **WHEN** the user cancels without an agent completion signal, **THEN** the configured completion transition still runs. +- **GIVEN** an enabled step with an unanswered clarification, **WHEN** the user cancels, **THEN** the turn stops but the task remains on the current step. +- **GIVEN** an enabled step whose destination auto-starts an agent, **WHEN** the user cancels, **THEN** the workflow moves and the destination's normal auto-start behavior runs. +- **GIVEN** an enabled step whose session-state write or turn closure fails, **WHEN** the user cancels, **THEN** no completion action, workflow move, or destination auto-start runs and the failure remains retryable. +- **GIVEN** an enabled step whose transition target is terminal, **WHEN** the user cancels, **THEN** observers see the task move directly to `COMPLETED` without an intermediate `REVIEW` state. +- **GIVEN** an enabled step whose transition target is nonterminal and does not immediately restart work, **WHEN** the user cancels, **THEN** the destination step is persisted and the task settles to `REVIEW` after the transition. +- **GIVEN** an enabled step with a queued user message, **WHEN** the user cancels, **THEN** the workflow transition may run but the queued message remains parked until an existing explicit drain or destination behavior consumes work. +- **GIVEN** an internal clarification timeout, peer interrupt, parent stop, archive, runtime failure, or provider error, **WHEN** that path cancels or stops an agent turn, **THEN** it does not run cancellation-driven completion actions. +- **GIVEN** a runtime cancel that is escalated or finds no live execution, **WHEN** Kandev safely reconciles the user's cancel request, **THEN** an enabled step evaluates completion once and the session does not remain stuck running. +- **GIVEN** the standard Kanban template, **WHEN** a new workflow is created from it, **THEN** `Backlog` and `In Progress` have `cancel_triggers_turn_complete=true` and cancellation moves work to `Review` through their existing actions. +- **GIVEN** an existing persisted standard Kanban workflow, **WHEN** Kandev upgrades, **THEN** its stored setting remains `false` until a user enables it. +- **GIVEN** an exported workflow with the setting enabled, **WHEN** it is imported or synchronized, **THEN** the imported step retains the enabled value. +- **GIVEN** a user edits a step on desktop or mobile, **WHEN** they enable the cancellation option and save, **THEN** the value persists after reload, the associated label provides a 44px touch target on phones, and the page has no horizontal overflow or desktop-only interaction. + +## Out of Scope + +- A separate `on_turn_cancelled` event or cancellation-specific action list. +- Backfilling or rewriting existing workflows created from the standard Kanban template. +- Changing queued-message drain policy after cancellation. +- Treating agent errors, provider failures, crashes, task stops, or clarification pauses as successful completion. +- Adding cancellation-cause metadata to workflow transition history. diff --git a/docs/specs/workflow/explicit-completion-signal/spec.md b/docs/specs/workflow/explicit-completion-signal/spec.md index a973d22222..bd0ec1d577 100644 --- a/docs/specs/workflow/explicit-completion-signal/spec.md +++ b/docs/specs/workflow/explicit-completion-signal/spec.md @@ -20,6 +20,7 @@ Signal-gated Kanban workflows need a reliable distinction between an agent endin - Creating or loading an ACP session supplies the local Kandev MCP server. After a transient MCP reconnect, the client can list and call `step_complete_kandev` again without a user message or process restart. - Kandev does not pin the Claude ACP bridge version as part of this feature. Bridge selection retains the existing unversioned package behavior; diagnostics continue to report the version returned during ACP initialization. - Existing idempotency, clarification barriers, and re-open semantics remain unchanged. ADR 0015's planned manual "Mark complete & advance" fallback is not currently implemented and is outside this reliability fix. +- Explicit user-cancel completion is owned by the [Cancelled Turn Completion](../cancelled-turn-completion/spec.md) spec. When a step opts into that policy, the user's cancel action is a human completion decision and may run `on_turn_complete` without an agent-emitted signal; internal cancellation paths remain non-completing. ## API Surface @@ -67,12 +68,13 @@ The pending completion signal continues to use `TaskSession.Metadata` as specifi - **GIVEN** an Office task session in `ModeOffice`, **WHEN** the client lists tools or receives its first-turn context, **THEN** `step_complete_kandev` is absent. - **GIVEN** a task-mode client has connected to Kandev MCP, **WHEN** its MCP connection drops and reconnects, **THEN** the client can list and call `step_complete_kandev` without another user message. - **GIVEN** a signal-gated workflow step, **WHEN** the agent finishes without calling the tool, **THEN** the task remains on the current step and no automatic transition runs. +- **GIVEN** a signal-gated workflow step that also enables explicit user-cancel completion, **WHEN** the user cancels the active turn, **THEN** the configured completion transition may run without `step_complete_kandev` as defined by the cancelled-turn completion policy. - **GIVEN** a client that displays fully qualified MCP names, **WHEN** it resolves the completion instruction, **THEN** it can associate canonical `step_complete_kandev` with its qualified runtime alias. - **GIVEN** Claude ACP initializes, **WHEN** the bridge reports its identity, **THEN** Kandev records the reported bridge name and version in diagnostics without constraining package resolution in this feature. ## Out of Scope - Exposing `step_complete_kandev` to Office agents. -- Changing completion-signal persistence or transition semantics, or implementing ADR 0015's planned manual fallback UI. +- Changing completion-signal persistence or ordinary halt semantics, or implementing ADR 0015's planned manual fallback UI. Explicit user-cancel behavior is defined separately by the cancelled-turn completion spec. - Adding vendor-specific eager-load metadata; agents are expected to use normal MCP tool discovery. - Implementing a generic MCP proxy or replacing the ACP bridge.