Skip to content

fix(executor): restore delegated workflow execution - #6539

Merged
TheodoreSpeaks merged 3 commits into
stagingfrom
fix/subworkflow-executor-delegation
Aug 11, 2026
Merged

fix(executor): restore delegated workflow execution#6539
TheodoreSpeaks merged 3 commits into
stagingfrom
fix/subworkflow-executor-delegation

Conversation

@TheodoreSpeaks

@TheodoreSpeaks TheodoreSpeaks commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • preserve trusted delegation context across agent tools and nested workflows
  • run custom blocks under their source-owned execution context, trust only server-authored invocation scope, and fail fast when logging cannot start
  • resolve model-exposed workspace variables in custom tool arguments without leaking plaintext secrets

Type of Change

  • Bug fix

Testing

  • 641 focused tests passing, including the 150-test tool executor suite after custom-block hardening
  • Sim/auth type checks passing
  • Lint and all 24 CI audits passing

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 11, 2026 11:05am

Request Review

@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes span signed executor delegation, workflow read authorization, nested/custom-block execution identity, and secret rebinding for model-emitted tool args—security- and billing-sensitive surfaces with broad test coverage but non-trivial blast radius.

Overview
Restores trusted executor delegation across nested workflows, agent tools, and internal API reads by introducing ExecutorDelegationOrigin (canonical parent log identity) on execution context extensions. Regular child workflows keep that origin through deeper nesting; custom blocks switch delegation to the publisher-owned child execution after their log row starts, and fail before running if that logging session cannot open.

Workflow authorization now lets an active executor delegation read any workflow in the same workspace, not only the workflow id embedded in the token—aligned with loading child workflow definitions.

Agent / provider tool path gains safer handling when models echo {{ENV}} placeholders from the prompt into tool arguments: model-visible secret registries are attached per tool, resolveModelExposedEnvReferences rebinds only placeholders that were actually projected to the model, and provider runtime forwards the trusted executionContext into executeTool. Custom-block and workflow executor tools ignore forged _context and use server-resolved scope for invocation.

Executor-delegation protected tools mint tokens from executorDelegationOrigin when present (so nested runs still authenticate as the parent workflow).

Reviewed by Cursor Bugbot for commit c7ef54c. Bugbot is set up for automated code reviews on this repo. Configure here.

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR restores delegated execution across nested workflows and agent tools while preserving trusted execution identity and secret provenance.

  • Propagates canonical delegation origins through nested workflows and custom blocks.
  • Fails closed when custom-block execution logging cannot start.
  • Rebinds only model-exposed environment placeholders for tool execution.
  • Replaces the newly introduced Record<string, any> merge assertions with inferred Record<string, unknown> results.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/providers/utils.ts Safely resolves model-exposed environment references and fixes the previously reported merge-result typing issue.
apps/sim/executor/handlers/workflow/workflow-handler.ts Preserves canonical delegation identity for regular children and establishes source-owned identity for custom-block executions.
apps/sim/tools/index.ts Uses trusted execution scope for protected tools and overwrites model-supplied custom-block context fields.
apps/sim/executor/utils/resolved-secret-trace-registry.ts Adds scoped placeholder rebinding with collision detection and per-tool provenance tracking.
apps/sim/lib/workflows/application/authorization.ts Permits delegated workflow reads across workflows while retaining workspace and canonical-origin checks.

Reviews (3): Last reviewed commit: "fix(providers): keep tool parameters typ..." | Re-trigger Greptile

Comment thread apps/sim/providers/utils.ts Outdated
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@TheodoreSpeaks
TheodoreSpeaks merged commit 783e1b5 into staging Aug 11, 2026
30 checks passed
@TheodoreSpeaks
TheodoreSpeaks deleted the fix/subworkflow-executor-delegation branch August 11, 2026 11:13

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit c7ef54c. Configure here.

waleedlatif1 added a commit that referenced this pull request Aug 12, 2026
…policy (#6611)

A workflow attached as an Agent (or Pi) tool ran its entire child execution
with an empty environment-variable map and no block-output redaction policy.

Mechanism. `tools/index.ts` short-circuits `workflow_executor` into
`runWorkflowTool`, which builds its synthetic parent `ExecutionContext` with
`buildCustomBlockExecutionContext`. That builder was written for the
custom-block (deploy-as-block) path and hardcoded `environmentVariables: {}` —
safe there only because `WorkflowBlockHandler.executeCore` re-derives the
publisher's env inside `if (isCustomBlock)`. The workflow-tool path's synthetic
block carries `metadata.id: 'workflow_input'`, so `isCustomBlock` is false, the
re-derivation is skipped, and `{}` flows through `childEnvVarValues` into the
sub-Executor. `DAGExecutor` has no fallback and `EnvResolver` returns the raw
reference on a miss, so a child block field of `Bearer {{MY_API_KEY}}` was
transmitted to the third party verbatim and 401'd — silently, with the variable
name disclosed. The same builder never set `piiBlockOutputRedaction`, so
`block-executor`'s in-flight masking was disabled for every child block of orgs
that had explicitly enabled that stage. Both landed as unnoticed side effects of
#5273, whose stated goals were admission slots, log rows, cost roll-up and
structured errors; #6539 later patched a third dropped field on the same context
without noticing these two.

Fix. Thread both values through the runner `options` bag — never `params._context`,
which spreads model-reachable `contextParams._context` first and would let a model
inject its own env map or disable redaction. `executeTool` reads them off the
trusted `executionContext`, which also covers the Pi block, whose tool loop calls
`executeTool` with `executionContext: ctx` on the identical path.

`environmentVariables` is required rather than optional-with-a-default. Silent
omission is precisely the failure mode here and in #6539; making it required turns
the next caller's omission into a compile error. `runCustomBlockTool` now passes
`{}` explicitly, so that path is unchanged at runtime. `piiBlockOutputRedaction`
stays optional deliberately: `undefined` is its correct value for the many tenants
with no policy, whereas `{}` for env is a wrong identity rather than a default.
The builder's TSDoc states both halves of that asymmetry.

Identity semantics — this restores function but does not restore main's identity.
On main this tool was an HTTP hop into execution-core, which derived the env from
the CHILD workflow's owner, so the child got the child owner's personal env plus
the child workspace's env. Forwarding the caller's map gives the child the PARENT
CALLER's personal env: a different identity, not a subset. That is the deliberate
choice, because it is byte-identical to the long-standing canvas workflow block,
it is bounded to one workspace by `assertChildWorkflowInWorkspace` on this branch,
and it is the only variant consistent with the parent `resolvedSecretTraceRegistry`
this path already forwards. The narrow case that worked on main and still will not:
a same-workspace child owned by another member that relied on THAT member's
personal environment variable.

The `deployed_block_executor` call site deliberately gets neither value: custom
blocks skip the same-workspace assert and run cross-workspace under the
publisher's identity, so the consumer's env and redaction rules are the wrong
tenant's. A test pins that so a later refactor cannot unify the branches silently.

Tests. Three suites pin the fix itself (runner, builder, `executeTool` dispatch)
and go red without it. A fourth case in `workflow-handler.test.ts` pins the last
hop — `ctx.environmentVariables` -> `childEnvVarValues` -> the sub-Executor's
`envVarValues`, plus `piiBlockOutputRedaction` — on the NON-custom branch. That
hop is untouched staging code, so that case passes either way by construction; it
exists so a future change to the branch that distinguishes the two paths cannot
silently undo this fix downstream of the builder.

Out of scope, deliberately: `enforceCredentialAccess` is dropped by the same
synthetic context, but on main this path ran under an internal JWT with
`useAuthenticatedUserAsActor === false`, so forwarding the parent's value would
TIGHTEN behavior versus main and could break currently-working child runs
mid-release. It needs its own deliberate change — and it now compounds with this
one, since the child runs with the parent's decrypted env while credential-access
enforcement stays off. The `input` redaction stage (masking the LLM-authored
inputMapping) is also not restored — `ExecutionContext` has no field for it and
the canvas workflow block never had it either.

Re-enabling masking inside child runs is a live behavior change for affected
tenants: `redactObjectStrings` runs with `onFailure: 'throw'`, so a child agent
tool call that currently succeeds unmasked can now fail closed, which is main's
semantic restored. This belongs in the release note.
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.

1 participant