improvement(executor): support nested loops/parallels#3398
improvement(executor): support nested loops/parallels#3398waleedlatif1 merged 71 commits intostagingfrom
Conversation
…keyboard shortcuts, audit logs
…rects to rewrites
…stash, algolia tools; isolated-vm robustness improvements, tables backend (#3271) * feat(tools): advanced fields for youtube, vercel; added cloudflare and dataverse tools (#3257) * refactor(vercel): mark optional fields as advanced mode Move optional/power-user fields behind the advanced toggle: - List Deployments: project filter, target, state - Create Deployment: project ID override, redeploy from, target - List Projects: search - Create/Update Project: framework, build/output/install commands - Env Vars: variable type - Webhooks: project IDs filter - Checks: path, details URL - Team Members: role filter - All operations: team ID scope Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style(youtube): mark optional params as advanced mode Hide pagination, sort order, and filter fields behind the advanced toggle for a cleaner default UX across all YouTube operations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * added advanced fields for vercel and youtube, added cloudflare and dataverse block * addded desc for dataverse * add more tools * ack comment * more * ops --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * feat(tables): added tables (#2867) * updates * required * trashy table viewer * updates * updates * filtering ui * updates * updates * updates * one input mode * format * fix lints * improved errors * updates * updates * chages * doc strings * breaking down file * update comments with ai * updates * comments * changes * revert * updates * dedupe * updates * updates * updates * refactoring * renames & refactors * refactoring * updates * undo * update db * wand * updates * fix comments * fixes * simplify comments * u[dates * renames * better comments * validation * updates * updates * updates * fix sorting * fix appearnce * updating prompt to make it user sort * rm * updates * rename * comments * clean comments * simplicifcaiton * updates * updates * refactor * reduced type confusion * undo * rename * undo changes * undo * simplify * updates * updates * revert * updates * db updates * type fix * fix * fix error handling * updates * docs * docs * updates * rename * dedupe * revert * uncook * updates * fix * fix * fix * fix * prepare merge * readd migrations * add back missed code * migrate enrichment logic to general abstraction * address bugbot concerns * adhere to size limits for tables * remove conflicting migration * add back migrations * fix tables auth * fix permissive auth * fix lint * reran migrations * migrate to use tanstack query for all server state * update table-selector * update names * added tables to permission groups, updated subblock types --------- Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai> Co-authored-by: waleed <walif6@gmail.com> * fix(snapshot): changed insert to upsert when concurrent identical child workflows are running (#3259) * fix(snapshot): changed insert to upsert when concurrent identical child workflows are running * fixed ci tests failing * fix(workflows): disallow duplicate workflow names at the same folder level (#3260) * feat(tools): added redis, upstash, algolia, and revenuecat (#3261) * feat(tools): added redis, upstash, algolia, and revenuecat * ack comment * feat(models): add gemini-3.1-pro-preview and update gemini-3-pro thinking levels (#3263) * fix(audit-log): lazily resolve actor name/email when missing (#3262) * fix(blocks): move type coercions from tools.config.tool to tools.config.params (#3264) * fix(blocks): move type coercions from tools.config.tool to tools.config.params Number() coercions in tools.config.tool ran at serialization time before variable resolution, destroying dynamic references like <block.result.count> by converting them to NaN/null. Moved all coercions to tools.config.params which runs at execution time after variables are resolved. Fixed in 15 blocks: exa, arxiv, sentry, incidentio, wikipedia, ahrefs, posthog, elasticsearch, dropbox, hunter, lemlist, spotify, youtube, grafana, parallel. Also added mode: 'advanced' to optional exa fields. Closes #3258 * fix(blocks): address PR review — move remaining param mutations from tool() to params() - Moved field mappings from tool() to params() in grafana, posthog, lemlist, spotify, dropbox (same dynamic reference bug) - Fixed parallel.ts excerpts/full_content boolean logic - Fixed parallel.ts search_queries empty case (must set undefined) - Fixed elasticsearch.ts timeout not included when already ends with 's' - Restored dropbox.ts tool() switch for proper default fallback * fix(blocks): restore field renames to tool() for serialization-time validation Field renames (e.g. personalApiKey→apiKey) must be in tool() because validateRequiredFieldsBeforeExecution calls selectToolId()→tool() then checks renamed field names on params. Only type coercions (Number(), boolean) stay in params() to avoid destroying dynamic variable references. * improvement(resolver): resovled empty sentinel to not pass through unexecuted valid refs to text inputs (#3266) * fix(blocks): add required constraint for serviceDeskId in JSM block (#3268) * fix(blocks): add required constraint for serviceDeskId in JSM block * fix(blocks): rename custom field values to request field values in JSM create request * fix(trigger): add isolated-vm support to trigger.dev container builds (#3269) Scheduled workflow executions running in trigger.dev containers were failing to spawn isolated-vm workers because the native module wasn't available in the container. This caused loop condition evaluation to silently fail and exit after one iteration. - Add isolated-vm to build.external and additionalPackages in trigger config - Include isolated-vm-worker.cjs via additionalFiles for child process spawning - Add fallback path resolution for worker file in trigger.dev environment * fix(tables): hide tables from sidebar and block registry (#3270) * fix(tables): hide tables from sidebar and block registry * fix(trigger): add isolated-vm support to trigger.dev container builds (#3269) Scheduled workflow executions running in trigger.dev containers were failing to spawn isolated-vm workers because the native module wasn't available in the container. This caused loop condition evaluation to silently fail and exit after one iteration. - Add isolated-vm to build.external and additionalPackages in trigger config - Include isolated-vm-worker.cjs via additionalFiles for child process spawning - Add fallback path resolution for worker file in trigger.dev environment * lint * fix(trigger): update node version to align with main app (#3272) * fix(build): fix corrupted sticky disk cache on blacksmith (#3273) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Lakee Sivaraya <71339072+lakeesiv@users.noreply.github.com> Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai> Co-authored-by: Vikhyath Mondreti <vikhyathvikku@gmail.com>
… fixes, removed retired models, hex integration
…ogle tasks and bigquery integrations, workflow lock
Wire inner loop sentinel nodes into outer loop sentinel chains so that nested loops execute correctly. Resolves boundary-node detection to use effective sentinel IDs for nested loops, handles loop-exit edges from inner sentinel-end to outer sentinel-end, and recursively clears execution state for all nested loop scopes between iterations. NOTE: loop-in-loop nesting only; parallel nesting is not yet supported. Made-with: Cursor
…able resolution Introduce ParentIteration to track ancestor loop state, build a loopParentMap during DAG construction, and propagate parent iterations through block execution and child workflow contexts. Extend LoopResolver to support named loop references (e.g. <loop1.index>) and add output property resolution (<loop1.result>). Named references use the block's display name normalized to a tag-safe identifier, enabling blocks inside nested loops to reference any ancestor loop's iteration state. NOTE: loop-in-loop nesting only; parallel nesting is not yet supported. Made-with: Cursor
… and terminal display Thread parentIterations through SSE block-started, block-completed, and block-error events so the terminal can reconstruct nested loop hierarchies. Update the entry tree builder to recursively nest inner loop subflow nodes inside their parent iteration rows, using parentIterations depth-stripping to support arbitrary nesting depth. Display the block's store name for subflow container rows instead of the generic "Loop" / "Parallel" label. Made-with: Cursor
Remove the restriction that prevented subflow nodes from being dragged into other subflow containers, enabling loop-in-loop nesting on the canvas. Add cycle detection (isDescendantOf) to prevent a container from being placed inside one of its own descendants. Resize all ancestor containers when a nested child moves, collect descendant blocks when removing from a subflow so boundary edges are attributed correctly, and surface all ancestor loop tags in the tag dropdown for blocks inside nested loops. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR comprehensively implements support for nested loop/parallel execution across all four nesting combinations. The implementation is thorough with proper sentinel-based boundary resolution, subflow graph cloning, unified parent maps, and scope cleanup. All tests pass. Verified Fixes from Prior Review Rounds:
Two Remaining Findings:
Safe to merge with these two minor defensive hardening improvements. Confidence Score: 4/5
Last reviewed commit: 2a6b631 |
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Outdated
Show resolved
Hide resolved
|
@cursor review |
|
@greptile |
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/terminal/utils.ts
Outdated
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Outdated
Show resolved
Hide resolved
Additional Comments (1)
When a synthetic iteration group has no direct |
* feat(agent): add MCP server discovery mode for agent tool input * fix(tool-input): use type variant for MCP server tool count badge * fix(mcp-dynamic-args): align label styling with standard subblock labels * standardized inp format UI * feat(tool-input): replace MCP server inline expand with drill-down navigation * feat(tool-input): add chevron affordance and keyboard nav for MCP server drill-down * fix(tool-input): handle mcp-server type in refresh, validation, badges, and usage control * refactor(tool-validation): extract getMcpServerIssue, remove fake tool hack * lint * reorder dropdown * perf(agent): parallelize MCP server tool creation with Promise.all * fix(combobox): preserve cursor movement in search input, reset query on drilldown * fix(combobox): route ArrowRight through handleSelect, remove redundant type guards * fix(agent): rename mcpServers to mcpServerSelections to avoid shadowing DB import, route ArrowRight through handleSelect * docs: update google integration docs * fix(tool-input): reset drilldown state on tool selection to prevent stale view * perf(agent): parallelize MCP server discovery across multiple servers
…s anti-pattern (#3357) * improvement(tests): speed up unit tests by eliminating vi.resetModules anti-pattern - convert 51 test files from vi.resetModules/vi.doMock/dynamic import to vi.hoisted/vi.mock/static import - add global @sim/db mock to vitest.setup.ts - switch 4 test files from jsdom to node environment - remove all vi.importActual calls that loaded heavy modules (200+ block files) - remove slow mockConsoleLogger/mockAuth/setupCommonApiMocks helpers - reduce real setTimeout delays in engine tests - mock heavy transitive deps in diff-engine test test execution time: 34s -> 9s (3.9x faster) environment time: 2.5s -> 0.6s (4x faster) * docs(testing): update testing best practices with performance rules - document vi.hoisted + vi.mock + static import as the standard pattern - explicitly ban vi.resetModules, vi.doMock, vi.importActual, mockAuth, setupCommonApiMocks - document global mocks from vitest.setup.ts - add mock pattern reference for auth, hybrid auth, and database chains - add performance rules section covering heavy deps, jsdom vs node, real timers * fix(tests): fix 4 failing test files with missing mocks - socket/middleware/permissions: add vi.mock for @/lib/auth to prevent transitive getBaseUrl() call - workflow-handler: add vi.mock for @/executor/utils/http matching executor mock pattern - evaluator-handler: add db.query.account mock structure before vi.spyOn - router-handler: same db.query.account fix as evaluator * fix(tests): replace banned Function type with explicit callback signature
* feat(databricks): add Databricks integration with 8 tools
Add complete Databricks integration supporting SQL execution, job management,
run monitoring, and cluster listing via Personal Access Token authentication.
Tools: execute_sql, list_jobs, run_job, get_run, list_runs, cancel_run,
get_run_output, list_clusters
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(databricks): throw on invalid JSON params, fix boolean coercion, add expandTasks field
- Throw errors on invalid JSON in jobParameters/notebookParams instead of silently defaulting to {}
- Always set boolean params explicitly to prevent string 'false' being truthy
- Add missing expandTasks dropdown UI field for list_jobs operation
* fix(databricks): align tool inputs/outputs with official API spec
- execute_sql: fix wait_timeout default description (50s, not 10s)
- get_run: add queueDuration field, update lifecycle/result state enums
- get_run_output: fix notebook output size (5 MB not 1 MB), add logsTruncated field
- list_runs: add userCancelledOrTimedout to state, fix limit range (1-24), update state enums
- list_jobs: fix name filter description to "exact case-insensitive"
- list_clusters: add PIPELINE_MAINTENANCE to ClusterSource enum
* fix(databricks): regenerate docs to reflect API spec fixes
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
|
@cursor review |
|
@greptile |
...onents/panel/components/editor/components/sub-block/components/tag-dropdown/tag-dropdown.tsx
Show resolved
Hide resolved
Additional Comments (3)
The explicit The original guard was an intentional safety net for exactly this case — it ensured that regardless of what
For a loop with A secondary effect is that Consider tracking which clone IDs were created by each
The loop scan runs first, then the parallel scan iterates More importantly, even in the expected "clean" case, the ordering means for (const nodeId of config.nodes ?? []) {
if (dag.parallelConfigs.has(nodeId) || dag.loopConfigs.has(nodeId)) {
if (parentMap.has(nodeId)) {
logger.warn('Subflow has multiple parents in buildSubflowParentMap', {
nodeId,
existing: parentMap.get(nodeId),
incoming: { parentId: parallelId, parentType: 'parallel' },
})
}
parentMap.set(nodeId, { parentId: parallelId, parentType: 'parallel' })
}
} |
…of distributionItems The distributionItems fallback was never part of SerializedParallel — it only worked through any typing. Updated the test to use the real distribution property. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d update test findParallelBoundaryNodes now skips LOOP_CONTINUE back-edges when detecting terminal nodes, matching findLoopBoundaryNodes behavior. Without this, a nested loop's back-edge was incorrectly counted as a forward edge within the parallel, preventing terminal detection. Also updated parallel resolver test to use the real distribution property instead of the non-existent distributionItems fallback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lones When a parallel contains a nested loop, cloned loop scopes (__obranch-N) created by expandParallel were not being deleted, causing stale scopes to persist across outer loop iterations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ion, restore executionOrder - Remove unreachable `?? candidateIds[0]` fallbacks in loop/parallel resolvers - Remove arbitrary first-match fallback scan in findEffectiveContainerId - Fix edgeCrossesLoopBoundary to use innermost loop detection for nested loops - Add warning log for missing branch outputs in parallel aggregation - Restore executionOrder on WorkflowNodeMetadata and pipe through child workflow notification - Remove dead sim-drag-subflow classList.remove call - Clean up cloned loop subflowParentMap entries in deleteParallelScopeAndClones Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ed blocks findAncestorLoops only checked direct loop membership, missing blocks nested inside parallels within loops (and vice versa). Refactored to walk through both loop and parallel containers recursively, so a block inside a parallel inside a loop correctly sees the loop's contextual tags (index, currentItem) instead of the loop's output tags (results). Also fixed parallel ancestor detection to handle nested parallel-in-loop and loop-in-parallel scenarios, collecting all ancestor parallels instead of just the immediate containing one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ghting Remove !important z-index overrides that prevented nested subflows from being grabbed/dragged independently. Z-index is now managed by ReactFlow's elevateNodesOnSelect and per-node zIndex: depth props. Also adds execution status highlighting for nested subflows in both canvas and snapshot preview. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f8b6672 to
27ed713
Compare
|
@greptile |
|
@cursor review |
...orkspace/[workspaceId]/w/components/preview/components/preview-workflow/preview-workflow.tsx
Show resolved
Hide resolved
Prevents infinite recursion if subflowChildrenMap contains circular references by tracking visited nodes during traversal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@cursor review |
Additional Comments (2)
The
A valid workflow guarantees each container has exactly one parent, so this normally cannot happen. However, consider an edge case: if the same nested subflow ID is accidentally registered in both a loop config and a parallel config (e.g. during a migration or a copy-paste bug in the serializer), the parallel entry silently wins and A defensive guard would make the conflict visible: |
Summary
anytypes tounknownFixes #3301
Type of Change
Testing
Checklist