Skip to content

fix(console): match child-workflow inner blocks by instanceId when reconciling dropped SSE events#4575

Merged
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/fix-child-workflow-reconcile
May 13, 2026
Merged

fix(console): match child-workflow inner blocks by instanceId when reconciling dropped SSE events#4575
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/fix-child-workflow-reconcile

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Fix child-workflow inner blocks getting stuck isRunning: true (manifested as 34.57s wall-clock duration + value undefined on setProjects etc.) when a block:completed SSE event was dropped in transit
  • Root cause: reconcileChildTraceSpans matched entries against the parent's static nodeId, but inner-block entries store the per-invocation instanceId (set server-side via childWorkflowContext.parentBlockId). Strict equality always failed → rescue silently no-op'd → finishRunningEntries wall-clocked the row
  • Switch matcher to childWorkflowInstanceId throughout reconcileChildTraceSpans / spanConsoleIdentity / findConsoleEntryForSpan
  • Add logger.warn when a span finds no matching entry, so future misses are visible in CloudWatch
  • Close the test-coverage gap that let this ship: previous unit tests used a bare vi.fn() for updateConsole and only verified call args. Add an integration test file that exercises the real useTerminalConsoleStore and asserts entries actually flip end-to-end

Type of Change

  • Bug fix

Testing

  • New integration test seeds inner entries with the production shape (childWorkflowBlockId = instanceId) and confirms isRunning flips to false with the span's duration/output applied
  • Full apps/sim suite green: 382 files / 6229 tests
  • type-check clean

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
Copy link
Copy Markdown

vercel Bot commented May 13, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 13, 2026 0:26am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 13, 2026

PR Summary

Medium Risk
Changes how console entries are matched and updated when reconciling finalBlockLogs, which can affect correctness of terminal state updates for child workflows and nested spans. Risk is mitigated by expanded unit coverage plus a new integration test exercising the real store.

Overview
Fixes reconcileFinalBlockLogs child-workflow span reconciliation to match inner-block console rows by per-invocation instance id (using childWorkflowInstanceId as childWorkflowBlockId identity), including nested spans, loop/parallel iterations, and repeated invocations.

Updates and expands tests to reflect the new identity semantics and adds a new workflow-execution-utils.integration.test.ts that runs against the real useTerminalConsoleStore to assert end-to-end updates (success/error, duration, and output) are actually applied.

Reviewed by Cursor Bugbot for commit 5754939. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 13, 2026

Greptile Summary

This PR fixes a bug where child-workflow inner blocks could get stuck isRunning: true when a block:completed SSE event was dropped in transit. The root cause was that reconcileChildTraceSpans passed the parent's static blockId as the matching key, but inner-block console entries store the per-invocation instanceId as their childWorkflowBlockId — so the lookup always missed and the rescue silently no-op'd.

  • Core fix: removes the extra log.blockId argument from reconcileChildTraceSpans and threads childWorkflowInstanceId end-to-end through spanConsoleIdentity / findConsoleEntryForSpan, so the reconciliation now keys on the same instanceId that the server sets on block:started SSE events.
  • Test alignment: existing unit-test fixtures updated from childWorkflowBlockId: 'workflow-1' (static nodeId) to childWorkflowBlockId: 'child-inst-1' (instanceId), plus a new integration test file that exercises the real useTerminalConsoleStore end-to-end and catches identity-mismatch regressions.
  • Recursive depth: the fix correctly propagates through nested child workflows by forwarding matchingEntry?.childWorkflowInstanceId ?? childWorkflowInstanceId to recursive calls.

Confidence Score: 5/5

Safe to merge — the change is a targeted single-function fix with no side effects outside the reconciliation path, and is well-covered by both updated unit tests and a new real-store integration test.

The rename from static blockId to per-invocation instanceId is applied consistently in all three call sites (reconcileChildTraceSpans, spanConsoleIdentity, findConsoleEntryForSpan) and the recursive depth case. The integration test directly validates the store mutation end-to-end, covering the exact regression shape. No changed code touches auth, persistence, or network paths.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts Core logic fix: removes static blockId from reconcileChildTraceSpans and threads childWorkflowInstanceId throughout spanConsoleIdentity / findConsoleEntryForSpan; recursive child propagation also updated correctly.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.test.ts Unit test fixtures updated to use instanceId ('child-inst-1') instead of static nodeId ('workflow-1'); five new test cases cover the dropped-SSE scenario, multi-invocation disambiguation, parallel iterations, error propagation, and partial-iteration rescue.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.integration.test.ts New integration test file exercising the real useTerminalConsoleStore; covers four distinct scenarios including the core regression, multi-invocation targeting, error propagation, and loop-iteration matching.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[reconcileFinalBlockLogs] --> B{runningEntry found?}
    B -- yes --> C[updateConsole: flip isRunning to false]
    B -- no --> D[skip top-level update]
    C --> E{childWorkflowInstanceId present?}
    D --> E
    E -- no --> F[skip child reconcile]
    E -- yes --> G[reconcileChildTraceSpans using instanceId]
    G --> H[findConsoleEntryForSpan by instanceId plus span identity]
    H --> I[updateConsole span entry with childWorkflowBlockId equals instanceId]
    I --> J{span has children?}
    J -- yes --> K[recursive call using matchingEntry.childWorkflowInstanceId or parent instanceId]
    K --> G
    J -- no --> L[done]
Loading

Reviews (2): Last reviewed commit: "fix(console): drop noisy warn when recon..." | Re-trigger Greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 5754939. Configure here.

@waleedlatif1 waleedlatif1 merged commit d5c2ead into staging May 13, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/fix-child-workflow-reconcile branch May 13, 2026 00:41
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