From 758f6bdf641397db7be313b211e03088e31cb8e9 Mon Sep 17 00:00:00 2001 From: saschabuehrle Date: Sun, 22 Mar 2026 10:34:17 +0100 Subject: [PATCH 1/2] fix(composer): expand template expressions in elicitation messages (fixes #4312) --- .../composer/elicitation_integration_test.go | 58 +++++++++++++++++++ pkg/vmcp/composer/workflow_engine.go | 15 +++++ 2 files changed, 73 insertions(+) diff --git a/pkg/vmcp/composer/elicitation_integration_test.go b/pkg/vmcp/composer/elicitation_integration_test.go index d4edd40c05..a1a431e9ea 100644 --- a/pkg/vmcp/composer/elicitation_integration_test.go +++ b/pkg/vmcp/composer/elicitation_integration_test.go @@ -527,3 +527,61 @@ func TestDefaultElicitationHandler_SDKErrorHandling(t *testing.T) { }) } } + +func TestWorkflowEngine_ElicitationMessageTemplateExpansion(t *testing.T) { + t.Parallel() + + te := newTestEngine(t) + mockSDK := mocks.NewMockSDKElicitationRequester(te.Ctrl) + + // Capture the elicitation request to verify the message was expanded + var capturedReq mcp.ElicitationRequest + mockSDK.EXPECT().RequestElicitation(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, req mcp.ElicitationRequest) (*mcp.ElicitationResult, error) { + capturedReq = req + return &mcp.ElicitationResult{ + ElicitationResponse: mcp.ElicitationResponse{ + Action: mcp.ElicitationResponseActionAccept, + Content: map[string]any{"confirmed": true}, + }, + }, nil + }, + ) + + handler := NewDefaultElicitationHandler(mockSDK) + stateStore := NewInMemoryStateStore(1*time.Minute, 1*time.Hour) + engine := NewWorkflowEngine(te.Router, te.Backend, handler, stateStore, nil, nil) + + workflow := &WorkflowDefinition{ + Name: "template-elicit", + Steps: []WorkflowStep{ + { + ID: "ask", + Type: StepTypeElicitation, + Elicitation: &ElicitationConfig{ + Message: "Deploy {{.params.repo}} to {{.params.env}}?", + Schema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "confirmed": map[string]any{"type": "boolean"}, + }, + }, + Timeout: 1 * time.Minute, + }, + }, + }, + } + + params := map[string]any{ + "repo": "acme/widget", + "env": "production", + } + + result, err := engine.ExecuteWorkflow(context.Background(), workflow, params) + require.NoError(t, err) + assert.Equal(t, WorkflowStatusCompleted, result.Status) + + // Verify that template placeholders were expanded in the message + assert.Equal(t, "Deploy acme/widget to production?", capturedReq.Params.Message, + "elicitation message should have template expressions expanded") +} diff --git a/pkg/vmcp/composer/workflow_engine.go b/pkg/vmcp/composer/workflow_engine.go index ecba1b7b10..3603d24e1e 100644 --- a/pkg/vmcp/composer/workflow_engine.go +++ b/pkg/vmcp/composer/workflow_engine.go @@ -634,6 +634,21 @@ func (e *workflowEngine) executeElicitationStep( return err } + // Expand template expressions in elicitation message (e.g. {{.params.owner}}) + if step.Elicitation.Message != "" { + wrapper := map[string]any{"message": step.Elicitation.Message} + expanded, expandErr := e.templateExpander.Expand(ctx, wrapper, workflowCtx) + if expandErr != nil { + err := fmt.Errorf("%w: failed to expand elicitation message for step %s: %v", + ErrTemplateExpansion, step.ID, expandErr) + workflowCtx.RecordStepFailure(step.ID, err) + return err + } + if msg, ok := expanded["message"].(string); ok { + step.Elicitation.Message = msg + } + } + // Request elicitation (synchronous - blocks until response or timeout) // Per MCP 2025-06-18: SDK handles JSON-RPC ID correlation internally response, err := e.elicitationHandler.RequestElicitation(ctx, workflowCtx.WorkflowID, step.ID, step.Elicitation) From 0b280a1df35ed0ba13aee87a1d770f1514c112a9 Mon Sep 17 00:00:00 2001 From: saschabuehrle Date: Tue, 24 Mar 2026 08:20:35 +0100 Subject: [PATCH 2/2] fix(composer): keep elicitation step immutable and table-drive test --- .../composer/elicitation_integration_test.go | 106 ++++++++++-------- pkg/vmcp/composer/workflow_engine.go | 10 +- 2 files changed, 68 insertions(+), 48 deletions(-) diff --git a/pkg/vmcp/composer/elicitation_integration_test.go b/pkg/vmcp/composer/elicitation_integration_test.go index a1a431e9ea..b7fe25af69 100644 --- a/pkg/vmcp/composer/elicitation_integration_test.go +++ b/pkg/vmcp/composer/elicitation_integration_test.go @@ -531,57 +531,75 @@ func TestDefaultElicitationHandler_SDKErrorHandling(t *testing.T) { func TestWorkflowEngine_ElicitationMessageTemplateExpansion(t *testing.T) { t.Parallel() - te := newTestEngine(t) - mockSDK := mocks.NewMockSDKElicitationRequester(te.Ctrl) + testCases := []struct { + name string + message string + params map[string]any + expectedMessage string + }{ + { + name: "expands template message", + message: "Deploy {{.params.repo}} to {{.params.env}}?", + params: map[string]any{"repo": "acme/widget", "env": "production"}, + expectedMessage: "Deploy acme/widget to production?", + }, + { + name: "passes through plain message", + message: "Deploy now?", + params: map[string]any{}, + expectedMessage: "Deploy now?", + }, + } - // Capture the elicitation request to verify the message was expanded - var capturedReq mcp.ElicitationRequest - mockSDK.EXPECT().RequestElicitation(gomock.Any(), gomock.Any()).DoAndReturn( - func(_ context.Context, req mcp.ElicitationRequest) (*mcp.ElicitationResult, error) { - capturedReq = req - return &mcp.ElicitationResult{ - ElicitationResponse: mcp.ElicitationResponse{ - Action: mcp.ElicitationResponseActionAccept, - Content: map[string]any{"confirmed": true}, + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + te := newTestEngine(t) + mockSDK := mocks.NewMockSDKElicitationRequester(te.Ctrl) + + var capturedReq mcp.ElicitationRequest + mockSDK.EXPECT().RequestElicitation(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, req mcp.ElicitationRequest) (*mcp.ElicitationResult, error) { + capturedReq = req + return &mcp.ElicitationResult{ + ElicitationResponse: mcp.ElicitationResponse{ + Action: mcp.ElicitationResponseActionAccept, + Content: map[string]any{"confirmed": true}, + }, + }, nil }, - }, nil - }, - ) + ) - handler := NewDefaultElicitationHandler(mockSDK) - stateStore := NewInMemoryStateStore(1*time.Minute, 1*time.Hour) - engine := NewWorkflowEngine(te.Router, te.Backend, handler, stateStore, nil, nil) + handler := NewDefaultElicitationHandler(mockSDK) + stateStore := NewInMemoryStateStore(1*time.Minute, 1*time.Hour) + engine := NewWorkflowEngine(te.Router, te.Backend, handler, stateStore, nil, nil) - workflow := &WorkflowDefinition{ - Name: "template-elicit", - Steps: []WorkflowStep{ - { - ID: "ask", - Type: StepTypeElicitation, - Elicitation: &ElicitationConfig{ - Message: "Deploy {{.params.repo}} to {{.params.env}}?", - Schema: map[string]any{ - "type": "object", - "properties": map[string]any{ - "confirmed": map[string]any{"type": "boolean"}, + workflow := &WorkflowDefinition{ + Name: "template-elicit", + Steps: []WorkflowStep{ + { + ID: "ask", + Type: StepTypeElicitation, + Elicitation: &ElicitationConfig{ + Message: tt.message, + Schema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "confirmed": map[string]any{"type": "boolean"}, + }, + }, + Timeout: 1 * time.Minute, }, }, - Timeout: 1 * time.Minute, }, - }, - }, - } + } - params := map[string]any{ - "repo": "acme/widget", - "env": "production", + result, err := engine.ExecuteWorkflow(context.Background(), workflow, tt.params) + require.NoError(t, err) + assert.Equal(t, WorkflowStatusCompleted, result.Status) + assert.Equal(t, tt.expectedMessage, capturedReq.Params.Message) + assert.Equal(t, tt.message, workflow.Steps[0].Elicitation.Message) + }) } - - result, err := engine.ExecuteWorkflow(context.Background(), workflow, params) - require.NoError(t, err) - assert.Equal(t, WorkflowStatusCompleted, result.Status) - - // Verify that template placeholders were expanded in the message - assert.Equal(t, "Deploy acme/widget to production?", capturedReq.Params.Message, - "elicitation message should have template expressions expanded") } diff --git a/pkg/vmcp/composer/workflow_engine.go b/pkg/vmcp/composer/workflow_engine.go index 3603d24e1e..f2867dcf37 100644 --- a/pkg/vmcp/composer/workflow_engine.go +++ b/pkg/vmcp/composer/workflow_engine.go @@ -635,8 +635,10 @@ func (e *workflowEngine) executeElicitationStep( } // Expand template expressions in elicitation message (e.g. {{.params.owner}}) - if step.Elicitation.Message != "" { - wrapper := map[string]any{"message": step.Elicitation.Message} + // without mutating the workflow step definition. + elicitationCfg := *step.Elicitation + if elicitationCfg.Message != "" { + wrapper := map[string]any{"message": elicitationCfg.Message} expanded, expandErr := e.templateExpander.Expand(ctx, wrapper, workflowCtx) if expandErr != nil { err := fmt.Errorf("%w: failed to expand elicitation message for step %s: %v", @@ -645,13 +647,13 @@ func (e *workflowEngine) executeElicitationStep( return err } if msg, ok := expanded["message"].(string); ok { - step.Elicitation.Message = msg + elicitationCfg.Message = msg } } // Request elicitation (synchronous - blocks until response or timeout) // Per MCP 2025-06-18: SDK handles JSON-RPC ID correlation internally - response, err := e.elicitationHandler.RequestElicitation(ctx, workflowCtx.WorkflowID, step.ID, step.Elicitation) + response, err := e.elicitationHandler.RequestElicitation(ctx, workflowCtx.WorkflowID, step.ID, &elicitationCfg) if err != nil { // Handle timeout if errors.Is(err, ErrElicitationTimeout) {