Skip to content

Add forEach step type for VMCP composite tool workflows#4375

Merged
JAORMX merged 2 commits intomainfrom
foreach-step-type
Mar 26, 2026
Merged

Add forEach step type for VMCP composite tool workflows#4375
JAORMX merged 2 commits intomainfrom
foreach-step-type

Conversation

@JAORMX
Copy link
Collaborator

@JAORMX JAORMX commented Mar 26, 2026

Summary

  • Workflows today require statically defined steps -- you cannot dynamically spawn steps based on a previous step's output. This makes patterns like "for each repository in an org, check for security advisories" impossible without hardcoding every item.
  • Add a third step type forEach that iterates over a collection produced by a previous step and executes an inner tool step for each item, with configurable parallelism and error handling.

Type of change

  • New feature

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)

Changes

File Change
pkg/vmcp/config/config.go Add 5 forEach fields to WorkflowStepConfig, update kubebuilder enum
pkg/vmcp/config/composite_validation.go Add ValidateForEachStep with inner step, collection, limits validation
pkg/vmcp/config/foreach_validation_test.go 12 validation test cases
pkg/vmcp/composer/composer.go Add StepTypeForEach, forEach fields on WorkflowStep, 2 new TemplateExpander methods
pkg/vmcp/composer/template_expander.go Unify expansion into single path with optional extraCtx for forEach
pkg/vmcp/composer/workflow_engine.go Add executeForEachStep with collection resolution, bounded parallel execution, result aggregation
pkg/vmcp/composer/foreach_test.go 10 execution test cases (basic, empty, abort, continue, max iterations, downstream, parallel, template context, string collection, invalid)
pkg/vmcp/server/workflow_converter.go Handle forEach type conversion, default empty Type to "tool"
pkg/vmcp/internal/compositetools/workflow_converter.go Check inner step tool accessibility for session filtering
docs/arch/10-virtual-mcp-architecture.md Add step type enumeration
docs/operator/composite-tools-quick-reference.md Add forEach to type enum, pattern example, validation rules
docs/operator/virtualmcpcompositetooldefinition-guide.md Add forEach step type reference section
docs/operator/advanced-workflow-patterns.md Add ForEach Iteration Patterns section

Does this introduce a user-facing change?

Yes. Users can now define forEach steps in composite tool workflows to iterate over collections from previous steps. Example:

steps:
  - id: list_repos
    type: tool
    tool: github.list_repos
    arguments:
      org: "{{.params.org}}"

  - id: check_advisories
    type: forEach
    collection: "{{json .steps.list_repos.output.repositories}}"
    itemVar: repo
    maxParallel: 5
    step:
      type: tool
      tool: github.list_security_advisories
      arguments:
        repo: "{{.forEach.repo.name}}"
    dependsOn: [list_repos]
    onError:
      action: continue

Special notes for reviewers

Key design decisions:

  • forEach is a DAG-opaque macro -- the DAG sees one node; internal parallelism is self-managed via errgroup + semaphore. This avoids rearchitecting the DAG executor.
  • Single inner step (tool type only). Multi-step iteration bodies and elicitation inner steps can be added later.
  • Nested forEach is explicitly rejected at validation time.
  • Template expander was refactored to eliminate code duplication: expandValueWithDepth and expandStringInternal now accept an optional extraCtx parameter, used by both normal and forEach expansion paths.

Safety limits:

  • maxIterations: default 100, hard cap 1000
  • maxParallel: default 10 (DAG default), hard cap 50
  • itemVar cannot be "index" (reserved key)

Interface change: Two methods added to TemplateExpander (ExpandString, ExpandWithForEach). ExpandString is a natural complement to Expand (string vs map). ExpandWithForEach cleanly separates the forEach concern without mutating shared WorkflowContext state. Both are backed by the same unified internal implementation.

Large PR Justification

  • The forEach step type spans validation, execution, template expansion, CRD schema, and documentation -- these are tightly coupled and would break if separated (e.g., validation without the execution path, or CRD changes without the converter).
  • ~400 lines are tests (foreach_test.go, foreach_validation_test.go).
  • ~200 lines are generated CRD YAML and documentation updates.
  • The core implementation logic is ~350 lines across 3 files.

Generated with Claude Code

@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Mar 26, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

@codecov
Copy link

codecov bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 75.83333% with 58 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.52%. Comparing base (60d91c4) to head (e581b46).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/vmcp/composer/workflow_engine.go 81.37% 21 Missing and 6 partials ⚠️
pkg/vmcp/server/workflow_converter.go 13.33% 12 Missing and 1 partial ⚠️
pkg/vmcp/config/composite_validation.go 79.66% 6 Missing and 6 partials ⚠️
...vmcp/internal/compositetools/workflow_converter.go 0.00% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4375      +/-   ##
==========================================
+ Coverage   69.42%   69.52%   +0.10%     
==========================================
  Files         485      485              
  Lines       49560    49789     +229     
==========================================
+ Hits        34405    34616     +211     
- Misses      12489    12494       +5     
- Partials     2666     2679      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JAORMX JAORMX force-pushed the foreach-step-type branch from 5b6ac44 to 13ae52c Compare March 26, 2026 09:51
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Mar 26, 2026
@JAORMX JAORMX force-pushed the foreach-step-type branch from 13ae52c to 9b60a62 Compare March 26, 2026 10:27
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Mar 26, 2026
@JAORMX JAORMX force-pushed the foreach-step-type branch from 9b60a62 to ee0ab74 Compare March 26, 2026 12:12
Workflows today require statically defined steps -- you cannot dynamically
spawn steps based on a previous step's output. This makes patterns like
"for each package in the image, query for vulnerabilities" impossible
without hardcoding every item.

Add a third step type `forEach` that iterates over a collection produced
by a previous step and executes an inner tool step for each item, with
configurable parallelism and error handling.

Key design decisions:
- forEach is a DAG-opaque macro: the DAG sees one node, internal
  parallelism is self-managed via errgroup + semaphore
- Single inner step (tool type only) keeps it simple; multi-step
  iteration bodies and elicitation inner steps can come later
- Nested forEach is explicitly rejected at validation time
- Collection resolves via Go template expansion then JSON parse,
  reusing existing template infrastructure
- Aggregated output has a well-known shape (iterations, count,
  completed, failed) so downstream templates have a predictable
  structure to reference

Safety limits:
- maxIterations: default 100, hard cap 1000
- maxParallel: default 10 (DAG default), hard cap 50
- itemVar cannot shadow the reserved "index" key
- Step and workflow timeouts apply as normal

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Mar 26, 2026
@JAORMX JAORMX force-pushed the foreach-step-type branch from ee0ab74 to 2cf35b8 Compare March 26, 2026 12:13
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Mar 26, 2026
Copy link
Contributor

@yrobla yrobla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings in executeForEachStep — one functional issue worth addressing before merge, one structural nit that can be a follow-up.

Write a "cancelled" sentinel to iteration results when errgroup
cancels the context, preventing zero-value entries from producing
misleading aggregated output. Add MaxParallel() accessor to
dagExecutor to avoid coupling forEach to DAG struct internals.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Mar 26, 2026
@JAORMX JAORMX merged commit 9781852 into main Mar 26, 2026
44 checks passed
@JAORMX JAORMX deleted the foreach-step-type branch March 26, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants