From fd3cc64d1219af70d11d5331ac2296923ab6f497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 9 Jul 2026 08:08:53 +0100 Subject: [PATCH] fix: expose signal-gated workflow steps in config MCP (#1629) --- apps/backend/config/prompts/config-context.md | 4 +- .../mcp/handlers/config_workflow_handlers.go | 74 ++++++++------- .../handlers/config_workflow_handlers_test.go | 91 +++++++++++++++++++ .../internal/mcp/server/config_handlers.go | 6 +- .../mcp/server/config_handlers_test.go | 59 +++++++++--- .../internal/sysprompt/sysprompt_test.go | 7 ++ .../internal/workflow/models/models.go | 2 +- ...icit-completion-signal-for-auto-advance.md | 1 + 8 files changed, 190 insertions(+), 54 deletions(-) diff --git a/apps/backend/config/prompts/config-context.md b/apps/backend/config/prompts/config-context.md index 6a9ff38cff..cb0489bb07 100644 --- a/apps/backend/config/prompts/config-context.md +++ b/apps/backend/config/prompts/config-context.md @@ -12,8 +12,8 @@ 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, 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, 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, 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. - 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/internal/mcp/handlers/config_workflow_handlers.go b/apps/backend/internal/mcp/handlers/config_workflow_handlers.go index 85b4b1ba57..741acc3360 100644 --- a/apps/backend/internal/mcp/handlers/config_workflow_handlers.go +++ b/apps/backend/internal/mcp/handlers/config_workflow_handlers.go @@ -131,15 +131,16 @@ 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"` - 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"` + 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) @@ -152,14 +153,15 @@ 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, - 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, + Events: req.Events, } if req.AllowManualMove != nil { createReq.AllowManualMove = *req.AllowManualMove @@ -177,15 +179,16 @@ 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"` - 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"` + 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) @@ -195,15 +198,16 @@ 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, - 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, + Events: req.Events, } resp, err := h.workflowCtrl.UpdateStep(ctx, updateReq) 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 be3af41d0f..3226bc0157 100644 --- a/apps/backend/internal/mcp/handlers/config_workflow_handlers_test.go +++ b/apps/backend/internal/mcp/handlers/config_workflow_handlers_test.go @@ -172,6 +172,28 @@ func TestHandleCreateWorkflowStep_PublishesDemotedStartStep(t *testing.T) { assert.True(t, (*published)[1].step["is_start_step"].(bool)) } +func TestHandleCreateWorkflowStep_PersistsAutoAdvanceRequiresSignal(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": "Signal gated", + "position": 0, + "auto_advance_requires_signal": 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].AutoAdvanceRequiresSignal) +} + func TestHandleUpdateWorkflowStep_PublishesDemotedStartStep(t *testing.T) { h, _, repo := setupImportHandlers(t) ctx := context.Background() @@ -219,6 +241,75 @@ func TestHandleUpdateWorkflowStep_PublishesDemotedStartStep(t *testing.T) { assert.True(t, (*published)[1].step["is_start_step"].(bool)) } +func TestHandleUpdateWorkflowStep_PersistsAutoAdvanceRequiresSignalFalse(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: "signal-gated", + WorkflowID: "wf-test", + Name: "Signal gated", + Position: 0, + ShowInCommandPanel: true, + AutoAdvanceRequiresSignal: true, + })) + + msg := makeWSMessage(t, ws.ActionMCPUpdateWorkflowStep, map[string]interface{}{ + "step_id": "signal-gated", + "auto_advance_requires_signal": false, + }) + + resp, err := h.handleUpdateWorkflowStep(ctx, msg) + require.NoError(t, err) + require.Equal(t, ws.MessageTypeResponse, resp.Type) + + step, err := repo.GetStep(ctx, "signal-gated") + require.NoError(t, err) + assert.False(t, step.AutoAdvanceRequiresSignal) +} + +func TestHandleListWorkflowSteps_IncludesAutoAdvanceRequiresSignal(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: "legacy", + WorkflowID: "wf-test", + Name: "Legacy", + Position: 0, + ShowInCommandPanel: true, + })) + require.NoError(t, repo.CreateStep(ctx, &wfmodels.WorkflowStep{ + ID: "signal-gated", + WorkflowID: "wf-test", + Name: "Signal gated", + Position: 1, + ShowInCommandPanel: true, + AutoAdvanceRequiresSignal: true, + })) + + msg := makeWSMessage(t, ws.ActionMCPListWorkflowSteps, map[string]interface{}{ + "workflow_id": "wf-test", + }) + + resp, err := h.handleListWorkflowSteps(ctx, msg) + require.NoError(t, err) + require.Equal(t, ws.MessageTypeResponse, resp.Type) + + var body struct { + Steps []map[string]interface{} `json:"steps"` + } + require.NoError(t, json.Unmarshal(resp.Payload, &body)) + require.Len(t, body.Steps, 2) + + assert.Contains(t, body.Steps[0], "auto_advance_requires_signal") + assert.Equal(t, false, body.Steps[0]["auto_advance_requires_signal"]) + assert.Contains(t, body.Steps[1], "auto_advance_requires_signal") + assert.Equal(t, true, body.Steps[1]["auto_advance_requires_signal"]) +} + 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 d60ed7954b..284394f21b 100644 --- a/apps/backend/internal/mcp/server/config_handlers.go +++ b/apps/backend/internal/mcp/server/config_handlers.go @@ -88,6 +88,7 @@ func (s *Server) registerConfigWorkflowStepTools() { mcp.WithBoolean("is_start_step", mcp.Description("Whether this is the start step")), 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.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.")), ), s.wrapHandler("create_workflow_step_kandev", s.createWorkflowStepHandler()), @@ -103,6 +104,7 @@ func (s *Server) registerConfigWorkflowStepTools() { mcp.WithBoolean("allow_manual_move", mcp.Description("Allow manual task moves into this step")), 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.WithObject("events", mcp.Description("Event-driven actions. Keys: on_enter, on_exit, on_turn_start, on_turn_complete.")), ), s.wrapHandler("update_workflow_step_kandev", s.updateWorkflowStepHandler()), @@ -397,7 +399,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", "events"} { + for _, key := range []string{"position", "is_start_step", "allow_manual_move", "show_in_command_panel", "auto_advance_requires_signal", "events"} { if args[key] != nil { payload[key] = args[key] } @@ -423,7 +425,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", "events"} { + for _, key := range []string{"is_start_step", "allow_manual_move", "show_in_command_panel", "auto_archive_after_hours", "auto_advance_requires_signal", "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 23541d6894..cd7864c051 100644 --- a/apps/backend/internal/mcp/server/config_handlers_test.go +++ b/apps/backend/internal/mcp/server/config_handlers_test.go @@ -61,6 +61,22 @@ func callTool(t *testing.T, s *Server, toolName string, args map[string]interfac return result } +func toolInputProperties(t *testing.T, s *Server, toolName string) map[string]interface{} { + t.Helper() + toolsMap := s.mcpServer.ListTools() + st, ok := toolsMap[toolName] + require.True(t, ok, "tool %q not registered", toolName) + + schema, err := json.Marshal(st.Tool.InputSchema) + require.NoError(t, err) + + var parsed map[string]interface{} + require.NoError(t, json.Unmarshal(schema, &parsed)) + props, ok := parsed["properties"].(map[string]interface{}) + require.True(t, ok, "schema should have properties") + return props +} + // --- Action constant tests --- func TestActionConstants_MatchWebSocketActions(t *testing.T) { @@ -94,6 +110,17 @@ func TestActionConstants_MatchWebSocketActions(t *testing.T) { // --- Workflow handler tests --- +func TestWorkflowStepTools_SchemaExposesAutoAdvanceRequiresSignal(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, "auto_advance_requires_signal") + assert.Contains(t, updateProps, "auto_advance_requires_signal") +} + func TestCreateWorkflowHandler_Success(t *testing.T) { backend := &testBackend{ response: map[string]interface{}{"id": "wf-1", "name": "Sprint Board"}, @@ -253,14 +280,15 @@ func TestCreateWorkflowStepHandler_AllFields(t *testing.T) { s := newTestServer(t, backend) result := callTool(t, s, "create_workflow_step_kandev", map[string]interface{}{ - "workflow_id": "wf-123", - "name": "Deploy", - "position": float64(0), - "color": "#22c55e", - "prompt": "Deploy prompt", - "is_start_step": true, - "allow_manual_move": true, - "show_in_command_panel": true, + "workflow_id": "wf-123", + "name": "Deploy", + "position": float64(0), + "color": "#22c55e", + "prompt": "Deploy prompt", + "is_start_step": true, + "allow_manual_move": true, + "show_in_command_panel": true, + "auto_advance_requires_signal": true, "events": map[string]interface{}{ "on_enter": []interface{}{map[string]interface{}{"type": "auto_start_agent"}}, }, @@ -273,6 +301,7 @@ func TestCreateWorkflowStepHandler_AllFields(t *testing.T) { require.True(t, ok) assert.Equal(t, true, payload["allow_manual_move"]) assert.Equal(t, true, payload["show_in_command_panel"]) + assert.Equal(t, true, payload["auto_advance_requires_signal"]) assert.NotNil(t, payload["events"]) } @@ -320,12 +349,13 @@ func TestUpdateWorkflowStepHandler_AllFields(t *testing.T) { s := newTestServer(t, backend) result := callTool(t, s, "update_workflow_step_kandev", map[string]interface{}{ - "step_id": "step-1", - "name": "In Review", - "color": "#3b82f6", - "allow_manual_move": true, - "show_in_command_panel": true, - "auto_archive_after_hours": float64(48), + "step_id": "step-1", + "name": "In Review", + "color": "#3b82f6", + "allow_manual_move": true, + "show_in_command_panel": true, + "auto_advance_requires_signal": false, + "auto_archive_after_hours": float64(48), "events": map[string]interface{}{ "on_enter": []interface{}{map[string]interface{}{"type": "enable_plan_mode"}}, }, @@ -337,6 +367,7 @@ func TestUpdateWorkflowStepHandler_AllFields(t *testing.T) { require.True(t, ok) assert.Equal(t, true, payload["allow_manual_move"]) assert.Equal(t, true, payload["show_in_command_panel"]) + assert.Equal(t, false, payload["auto_advance_requires_signal"]) assert.Equal(t, float64(48), payload["auto_archive_after_hours"]) assert.NotNil(t, payload["events"]) } diff --git a/apps/backend/internal/sysprompt/sysprompt_test.go b/apps/backend/internal/sysprompt/sysprompt_test.go index bbade2cd22..9807394b86 100644 --- a/apps/backend/internal/sysprompt/sysprompt_test.go +++ b/apps/backend/internal/sysprompt/sysprompt_test.go @@ -76,6 +76,13 @@ func TestFormatConfigContext_InjectsSessionID(t *testing.T) { assert.NotContains(t, result, "{session_id}") } +func TestConfigContext_DocumentsWorkflowStepSignalGate(t *testing.T) { + ctx := ConfigContext() + assert.Contains(t, ctx, "auto_advance_requires_signal") + assert.Contains(t, ctx, "create_workflow_step_kandev") + assert.Contains(t, ctx, "update_workflow_step_kandev") +} + func TestInjectConfigContext_WrapsInSystemTags(t *testing.T) { result := InjectConfigContext(testSessionID, testConfigPrompt) assert.True(t, strings.HasPrefix(result, TagStart)) diff --git a/apps/backend/internal/workflow/models/models.go b/apps/backend/internal/workflow/models/models.go index 6c6fa81cb6..be5e8ffec6 100644 --- a/apps/backend/internal/workflow/models/models.go +++ b/apps/backend/internal/workflow/models/models.go @@ -206,7 +206,7 @@ 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,omitempty"` + AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` } diff --git a/docs/decisions/0015-explicit-completion-signal-for-auto-advance.md b/docs/decisions/0015-explicit-completion-signal-for-auto-advance.md index 4924e1469e..2f59f6bfc7 100644 --- a/docs/decisions/0015-explicit-completion-signal-for-auto-advance.md +++ b/docs/decisions/0015-explicit-completion-signal-for-auto-advance.md @@ -80,6 +80,7 @@ Today, a workflow step's `on_turn_complete` actions (most commonly `move_to_next - **Per-step:** the existing `auto_advance` UI checkbox stays; a new sub-toggle "Wait for agent completion signal" appears underneath when an `on_turn_complete` transition is configured. Single switch; no profile/global flag. - **Workflow step model:** add `auto_advance_requires_signal bool` to `wfmodels.WorkflowStep`. Migration adds the column with default `false`; existing steps keep legacy "any turn-end advances" behaviour until the user opts a step in. +- **Config MCP:** `create_workflow_step_kandev` and `update_workflow_step_kandev` accept an optional `auto_advance_requires_signal` boolean with the same omitted-versus-false semantics as the REST step endpoints. `list_workflow_steps_kandev` returns the field explicitly for every step so config agents can audit which steps are signal-gated. ## Consequences