Skip to content

Commit 60d91c4

Browse files
JAORMXclaude
andauthored
Expose content array alongside structuredContent in workflow templates (#4384)
Make embedded resource content accessible from composite tool templates ContentArrayToMap previously ignored EmbeddedResource content types, making it impossible for composite tool workflows to chain on embedded resource data (e.g., an SBOM returned by get_referrer_content). Handle ContentTypeResource in ContentArrayToMap with "resource", "resource_1", "resource_2" key pattern. Expose the content array as a separate template namespace ({{.steps.X.content.*}}) alongside the existing output namespace ({{.steps.X.output.*}}), keeping structuredContent clean for outputSchema validation. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 34dead0 commit 60d91c4

8 files changed

Lines changed: 217 additions & 33 deletions

File tree

pkg/vmcp/composer/composer.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"sync"
1414
"time"
1515

16+
"github.com/stacklok/toolhive/pkg/vmcp"
1617
"github.com/stacklok/toolhive/pkg/vmcp/config"
1718
)
1819

@@ -212,9 +213,14 @@ type StepResult struct {
212213
// Status is the step status.
213214
Status StepStatusType
214215

215-
// Output contains the step output data.
216+
// Output contains the step output data (from StructuredContent or ContentArrayToMap fallback).
216217
Output map[string]any
217218

219+
// Content holds the raw content array from the tool call result.
220+
// This is exposed separately in templates via {{.steps.stepID.content.*}} so that
221+
// structuredContent remains clean for outputSchema validation.
222+
Content []vmcp.Content
223+
218224
// Error contains error information if the step failed.
219225
Error error
220226

pkg/vmcp/composer/template_expander.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"time"
1313

1414
"github.com/stacklok/toolhive/pkg/templates"
15+
"github.com/stacklok/toolhive/pkg/vmcp/conversion"
1516
)
1617

1718
const (
@@ -163,7 +164,10 @@ func (e *defaultTemplateExpander) expandString(
163164
}
164165

165166
// buildStepsContext converts StepResult map to a template-friendly structure.
166-
// This provides access to step outputs via {{.steps.stepid.output.field}}.
167+
// This provides access to step outputs via:
168+
// - {{.steps.stepid.output.field}} for structuredContent fields
169+
// - {{.steps.stepid.content.text}} for text content from the content array
170+
// - {{.steps.stepid.content.resource}} for embedded resource content from the content array
167171
func (*defaultTemplateExpander) buildStepsContext(workflowCtx *WorkflowContext) map[string]any {
168172
// Acquire read lock to safely access Steps map during concurrent execution
169173
workflowCtx.mu.RLock()
@@ -173,8 +177,9 @@ func (*defaultTemplateExpander) buildStepsContext(workflowCtx *WorkflowContext)
173177

174178
for stepID, result := range workflowCtx.Steps {
175179
stepData := map[string]any{
176-
"status": string(result.Status),
177-
"output": result.Output,
180+
"status": string(result.Status),
181+
"output": result.Output,
182+
"content": conversion.ContentArrayToMap(result.Content),
178183
}
179184

180185
// Add error information if step failed

pkg/vmcp/composer/template_expander_test.go

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010

1111
"github.com/stretchr/testify/assert"
1212
"github.com/stretchr/testify/require"
13+
14+
"github.com/stacklok/toolhive/pkg/vmcp"
1315
)
1416

1517
func TestTemplateExpander_Expand(t *testing.T) {
@@ -74,6 +76,40 @@ func TestTemplateExpander_Expand(t *testing.T) {
7476
params: map[string]any{},
7577
expected: map[string]any{"val": "<no value>"},
7678
},
79+
{
80+
name: "step content resource substitution",
81+
data: map[string]any{"sbom": "{{.steps.fetch.content.resource}}"},
82+
steps: map[string]*StepResult{
83+
"fetch": {
84+
Status: StepStatusCompleted,
85+
Output: map[string]any{"format": "spdx"},
86+
Content: []vmcp.Content{
87+
{Type: vmcp.ContentTypeResource, Text: `{"spdxVersion":"SPDX-2.3"}`, URI: "file://sbom.json"},
88+
},
89+
},
90+
},
91+
expected: map[string]any{"sbom": `{"spdxVersion":"SPDX-2.3"}`},
92+
},
93+
{
94+
name: "step output and content are independent namespaces",
95+
data: map[string]any{
96+
"format": "{{.steps.fetch.output.format}}",
97+
"text": "{{.steps.fetch.content.text}}",
98+
},
99+
steps: map[string]*StepResult{
100+
"fetch": {
101+
Status: StepStatusCompleted,
102+
Output: map[string]any{"format": "spdx", "text": "structured text"},
103+
Content: []vmcp.Content{
104+
{Type: vmcp.ContentTypeText, Text: "content array text"},
105+
},
106+
},
107+
},
108+
expected: map[string]any{
109+
"format": "spdx",
110+
"text": "content array text",
111+
},
112+
},
77113
}
78114

79115
expander := NewTemplateExpander()
@@ -154,7 +190,7 @@ func TestWorkflowContext_Lifecycle(t *testing.T) {
154190
assert.Equal(t, StepStatusRunning, ctx.Steps["s1"].Status)
155191

156192
time.Sleep(10 * time.Millisecond)
157-
ctx.RecordStepSuccess("s1", map[string]any{"result": "ok"})
193+
ctx.RecordStepSuccess("s1", map[string]any{"result": "ok"}, nil)
158194
assert.Equal(t, StepStatusCompleted, ctx.Steps["s1"].Status)
159195
assert.Greater(t, ctx.Steps["s1"].Duration, time.Duration(0))
160196

@@ -185,12 +221,12 @@ func TestWorkflowContext_GetLastStepOutput(t *testing.T) {
185221
// Add steps with different completion times
186222
ctx.RecordStepStart("s1")
187223
time.Sleep(5 * time.Millisecond)
188-
ctx.RecordStepSuccess("s1", map[string]any{"order": 1})
224+
ctx.RecordStepSuccess("s1", map[string]any{"order": 1}, nil)
189225

190226
time.Sleep(5 * time.Millisecond)
191227
ctx.RecordStepStart("s2")
192228
time.Sleep(5 * time.Millisecond)
193-
ctx.RecordStepSuccess("s2", map[string]any{"order": 2})
229+
ctx.RecordStepSuccess("s2", map[string]any{"order": 2}, nil)
194230

195231
// Should return latest (s2)
196232
output := ctx.GetLastStepOutput()

pkg/vmcp/composer/workflow_context.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"time"
1111

1212
"github.com/google/uuid"
13+
14+
"github.com/stacklok/toolhive/pkg/vmcp"
1315
)
1416

1517
// workflowContextManager manages workflow execution contexts.
@@ -87,13 +89,15 @@ func (ctx *WorkflowContext) RecordStepStart(stepID string) {
8789

8890
// RecordStepSuccess records a successful step completion.
8991
// Thread-safe for concurrent step execution.
90-
func (ctx *WorkflowContext) RecordStepSuccess(stepID string, output map[string]any) {
92+
// The content parameter is optional (may be nil for non-tool steps like elicitation).
93+
func (ctx *WorkflowContext) RecordStepSuccess(stepID string, output map[string]any, content []vmcp.Content) {
9194
ctx.mu.Lock()
9295
defer ctx.mu.Unlock()
9396

9497
if result, exists := ctx.Steps[stepID]; exists {
9598
result.Status = StepStatusCompleted
9699
result.Output = output
100+
result.Content = content
97101
result.EndTime = time.Now()
98102
result.Duration = result.EndTime.Sub(result.StartTime)
99103
}
@@ -207,10 +211,16 @@ func (ctx *WorkflowContext) Clone() *WorkflowContext {
207211

208212
// Clone step results
209213
for stepID, result := range ctx.Steps {
214+
var contentCopy []vmcp.Content
215+
if result.Content != nil {
216+
contentCopy = make([]vmcp.Content, len(result.Content))
217+
copy(contentCopy, result.Content)
218+
}
210219
clone.Steps[stepID] = &StepResult{
211220
StepID: result.StepID,
212221
Status: result.Status,
213222
Output: cloneMap(result.Output),
223+
Content: contentCopy,
214224
Error: result.Error,
215225
StartTime: result.StartTime,
216226
EndTime: result.EndTime,

pkg/vmcp/composer/workflow_engine.go

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -428,24 +428,32 @@ func (e *workflowEngine) executeToolStep(
428428
}
429429

430430
// Call tool with retry logic
431-
output, retryCount, err := e.callToolWithRetry(ctx, target, step, expandedArgs, workflowCtx)
431+
result, retryCount, err := e.callToolWithRetry(ctx, target, step, expandedArgs, workflowCtx)
432432

433433
// Handle result
434434
if err != nil {
435435
return e.handleToolStepFailure(step, workflowCtx, retryCount, err)
436436
}
437437

438-
return e.handleToolStepSuccess(ctx, step, workflowCtx, output, retryCount)
438+
// Extract output map from result.
439+
// Prefer StructuredContent (already a map), fall back to Content array conversion.
440+
output := result.StructuredContent
441+
if output == nil {
442+
output = conversion.ContentArrayToMap(result.Content)
443+
}
444+
445+
return e.handleToolStepSuccess(ctx, step, workflowCtx, output, result.Content, retryCount)
439446
}
440447

441448
// callToolWithRetry calls a tool with retry logic using exponential backoff.
449+
// Returns the full ToolCallResult so callers can access both StructuredContent and Content.
442450
func (e *workflowEngine) callToolWithRetry(
443451
ctx context.Context,
444452
target *vmcp.BackendTarget,
445453
step *WorkflowStep,
446454
args map[string]any,
447455
_ *WorkflowContext,
448-
) (map[string]any, int, error) {
456+
) (*vmcp.ToolCallResult, int, error) {
449457
maxRetries, initialDelay := e.getRetryConfig(step)
450458

451459
// Configure exponential backoff
@@ -455,7 +463,7 @@ func (e *workflowEngine) callToolWithRetry(
455463
expBackoff.Reset()
456464

457465
attemptCount := 0
458-
operation := func() (map[string]any, error) {
466+
operation := func() (*vmcp.ToolCallResult, error) {
459467
attemptCount++
460468
// TODO: For composite tools, we may want to propagate metadata from the parent request
461469
result, err := e.backendClient.CallTool(ctx, target, step.Tool, args, nil)
@@ -482,30 +490,20 @@ func (e *workflowEngine) callToolWithRetry(
482490
return nil, fmt.Errorf("%w: %s", vmcp.ErrToolExecutionFailed, errorMsg)
483491
}
484492

485-
// Extract output map from result.
486-
// Workflow logic uses map[string]any for template variable substitution.
487-
// Prefer StructuredContent (already a map), fall back to Content array conversion.
488-
// The _meta field is not needed for workflow execution, so we don't extract it.
489-
if result.StructuredContent != nil {
490-
return result.StructuredContent, nil
491-
}
492-
493-
// Fallback: convert Content array to map for backward compatibility.
494-
// This happens when backends return only Content without StructuredContent.
495-
return conversion.ContentArrayToMap(result.Content), nil
493+
return result, nil
496494
}
497495

498496
// Execute with retry
499497
// Safe conversion: maxRetries is capped by maxRetryCount constant (10)
500-
output, err := backoff.Retry(ctx, operation,
498+
result, err := backoff.Retry(ctx, operation,
501499
backoff.WithBackOff(expBackoff),
502500
backoff.WithMaxTries(uint(maxRetries+1)), // #nosec G115 -- +1 because it includes the initial attempt
503501
backoff.WithNotify(func(_ error, duration time.Duration) {
504502
slog.Debug("retrying step", "step", step.ID, "after", duration)
505503
}),
506504
)
507505

508-
return output, attemptCount - 1, err // Return retry count (attempts - 1)
506+
return result, attemptCount - 1, err // Return retry count (attempts - 1)
509507
}
510508

511509
// extractErrorMessage extracts a user-friendly error message from a failed tool call result.
@@ -595,9 +593,10 @@ func (e *workflowEngine) handleToolStepSuccess(
595593
step *WorkflowStep,
596594
workflowCtx *WorkflowContext,
597595
output map[string]any,
596+
content []vmcp.Content,
598597
retryCount int,
599598
) error {
600-
workflowCtx.RecordStepSuccess(step.ID, output)
599+
workflowCtx.RecordStepSuccess(step.ID, output, content)
601600

602601
// Update retry count
603602
if result, exists := workflowCtx.GetStepResult(step.ID); exists {
@@ -698,7 +697,7 @@ func (*workflowEngine) handleElicitationAccept(
698697
"content": response.Content,
699698
}
700699

701-
workflowCtx.RecordStepSuccess(step.ID, output)
700+
workflowCtx.RecordStepSuccess(step.ID, output, nil)
702701
slog.Debug("step completed with user-provided data", "step", step.ID)
703702
return nil
704703
}
@@ -772,7 +771,7 @@ func (*workflowEngine) handleElicitationAction(
772771
"action": reason,
773772
"skipped": true,
774773
}
775-
workflowCtx.RecordStepSuccess(step.ID, output)
774+
workflowCtx.RecordStepSuccess(step.ID, output, nil)
776775
// Return a special error that the workflow engine can detect
777776
// For now, we'll just complete the step successfully
778777
return nil
@@ -795,7 +794,7 @@ func (*workflowEngine) handleElicitationAction(
795794
output := map[string]any{
796795
"action": reason,
797796
}
798-
workflowCtx.RecordStepSuccess(step.ID, output)
797+
workflowCtx.RecordStepSuccess(step.ID, output, nil)
799798
return nil
800799

801800
default:

pkg/vmcp/composer/workflow_engine_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,3 +846,64 @@ func TestWorkflowEngine_SessionEngine_ToolNotInList_ReturnsNilSchema(t *testing.
846846
require.NoError(t, err)
847847
assert.Equal(t, WorkflowStatusCompleted, result.Status)
848848
}
849+
850+
func TestWorkflowEngine_EmbeddedResourceAccessibleFromTemplate(t *testing.T) {
851+
t.Parallel()
852+
te := newTestEngine(t)
853+
854+
// Step 1: tool returns structuredContent (schema-conformant) + embedded resource in Content array.
855+
// Step 2: accesses structuredContent via .output and content array via .content.
856+
// This keeps structuredContent clean for outputSchema validation while making
857+
// content array data (text, resources) accessible through a separate namespace.
858+
def := simpleWorkflow("resource-chain",
859+
toolStep("fetch", "registry.get_referrer_content", map[string]any{
860+
"image": "ghcr.io/org/repo:latest",
861+
}),
862+
toolStepWithDeps("analyze", "sbom.analyze", map[string]any{
863+
"sbom_data": "{{.steps.fetch.content.resource}}",
864+
"format": "{{.steps.fetch.output.format}}",
865+
}, []string{"fetch"}),
866+
)
867+
868+
target := &vmcp.BackendTarget{
869+
WorkloadID: "test-backend",
870+
WorkloadName: "test",
871+
BaseURL: "http://test:8080",
872+
}
873+
te.Router.EXPECT().RouteTool(gomock.Any(), "registry.get_referrer_content").Return(target, nil)
874+
te.Backend.EXPECT().CallTool(gomock.Any(), target, "registry.get_referrer_content",
875+
map[string]any{"image": "ghcr.io/org/repo:latest"}, gomock.Any()).
876+
Return(&vmcp.ToolCallResult{
877+
StructuredContent: map[string]any{
878+
"contentType": "sbom",
879+
"format": "spdx",
880+
"size": float64(5347),
881+
},
882+
Content: []vmcp.Content{
883+
{Type: vmcp.ContentTypeText, Text: "summary of SBOM"},
884+
{Type: vmcp.ContentTypeResource, Text: `{"spdxVersion":"SPDX-2.3","name":"mypackage"}`, URI: "file://sbom.json"},
885+
},
886+
}, nil)
887+
888+
// Step 2: verify the template-expanded args pull from the right namespaces.
889+
te.Router.EXPECT().RouteTool(gomock.Any(), "sbom.analyze").Return(target, nil)
890+
te.Backend.EXPECT().CallTool(gomock.Any(), target, "sbom.analyze", gomock.Any(), gomock.Any()).
891+
DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, args map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) {
892+
// .content.resource comes from the Content array's embedded resource
893+
assert.Equal(t, `{"spdxVersion":"SPDX-2.3","name":"mypackage"}`, args["sbom_data"])
894+
// .output.format comes from structuredContent
895+
assert.Equal(t, "spdx", args["format"])
896+
return &vmcp.ToolCallResult{
897+
StructuredContent: map[string]any{"result": "analyzed"},
898+
Content: []vmcp.Content{},
899+
}, nil
900+
})
901+
902+
result, err := execute(t, te.Engine, def, nil)
903+
904+
require.NoError(t, err)
905+
assert.Equal(t, WorkflowStatusCompleted, result.Status)
906+
assert.Len(t, result.Steps, 2)
907+
assert.Equal(t, StepStatusCompleted, result.Steps["fetch"].Status)
908+
assert.Equal(t, StepStatusCompleted, result.Steps["analyze"].Status)
909+
}

0 commit comments

Comments
 (0)