fix: expose signal-gated workflow steps in config MCP (#1629)

2026-07-09 08:08:53 +01:00
parent e1cf99f821
commit fd3cc64d12
8 changed files with 190 additions and 54 deletions
@@ -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).

@@ -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)
@@ -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)

@@ -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]
 			}
@@ -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"])
 }
@@ -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))
@@ -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"`
 }
@@ -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