Skip to content

Add sandboxed workflow execution engine#13

Draft
sezallagwal wants to merge 2 commits into
feature/workflow-composer-entrypointfrom
feature/workflow-engine-execution
Draft

Add sandboxed workflow execution engine#13
sezallagwal wants to merge 2 commits into
feature/workflow-composer-entrypointfrom
feature/workflow-engine-execution

Conversation

@sezallagwal

Copy link
Copy Markdown
Collaborator

Summary

  • Add a sandboxed workflow execution engine that runs composed workflows step by step
  • Support API call steps with authenticated HTTP requests via a pluggable client
  • Add LLM sampling steps for AI-powered transformations via MCP server surface
  • Implement template resolution and expression evaluation with security sandboxing
  • Handle conditional branching, elicitation, and transform steps with dependency ordering
  • Add integration and security unit tests for the executor and sandbox

Depends on

#11

Add the Week 4 workflow engine as focused modules built on the existing
expression-security allowlist:

- templates.ts: VM-sandboxed expression and template resolution, gated by
  validateSafeExpression before any evaluation
- api-call.ts: api_call execution with forEach fan-out and outputPath extraction
- sampling.ts: sampling step execution with JSON-intent detection
- executor.ts: dependency-ordered orchestrator handling conditional, transform,
  and elicitation steps with branch-skip propagation

Includes sandbox security tests covering known escape patterns and an executor
integration test exercising data flow, branching, forEach, sampling, and errors.

@dhairyashiil dhairyashiil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi Sezal, thanks for the workflow execution engine. Could we fix two runtime correctness issues before merging this layer?

First, path parameters are not substituted into endpoint paths. For an endpoint like /api/apps/public/{app-id}/incoming, the engine keeps {app-id} in the path and appends app-id as a query param instead. A lot of Rocket.Chat endpoints use path params, so generated workflows will fail for those APIs.

Could we replace {param} placeholders from the resolved payload, and avoid sending those same values again as query/body fields?

Second, forEach currently uses Promise.allSettled, drops failed items as null, and still marks the step as success. For bulk actions like archive/delete/post message, this can report success even when some side effects failed.

Could we either fail the step when any item fails, or make partial success explicit with per-item errors and an intentional continueOnError style behavior?

These are important because this PR is the execution layer that later generated projects will trust.

@dhairyashiil dhairyashiil marked this pull request as draft July 1, 2026 09:04
Address review feedback on the execution engine:

- Substitute {param} placeholders in endpoint paths from the resolved payload and drop those keys so path values are no longer duplicated as query params or body fields. A missing required path param now raises a clear error instead of sending a literal {param} to the API.

- Replace silent forEach failure handling: by default any failed iteration fails the step (and the workflow); an opt-in continueOnError flag makes partial success explicit, keeping failed items as null and surfacing per-item errors on the step with a top-level 'partial' status.

- Add integration tests for path-param substitution (GET/POST, encoding, missing param) and both forEach failure policies.
@sezallagwal sezallagwal requested a review from dhairyashiil July 7, 2026 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants