Skip to content

fix(terminal): terminal console update for child spans#4450

Open
icecrasher321 wants to merge 1 commit intostagingfrom
fix/terminal-logs
Open

fix(terminal): terminal console update for child spans#4450
icecrasher321 wants to merge 1 commit intostagingfrom
fix/terminal-logs

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

Summary

Fix terminal log races so child workflow errors, error-path executions, reconnect replays, and scoped cancellations preserve the correct final block status instead of showing stale canceled or duplicate rows.

Type of Change

  • Bug fix

Testing

Tested manually

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 5, 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 5, 2026 6:23pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 5, 2026

PR Summary

Medium Risk
Touches workflow execution/streaming lifecycle and terminal console reconciliation; regressions could affect log ordering, reconnect behavior, and when execution results return under load.

Overview
Improves terminal console correctness around workflow runs by scoping cancelRunningEntries to a specific executionId (and updating all call sites) so stale/parallel executions don’t incorrectly mark unrelated rows as canceled.

Fixes reconnect/log replay edge cases by deduping block start rows (matching iteration + parent-iteration identity) and by only clearing execution entries on reconnect when replay starts from eventId=0.

Enhances final log reconciliation to match entries by executionOrder, explicitly clear isCanceled, and propagate child workflow TraceSpan results into parent console rows before sweeping running entries. Also serializes Redis execution event writes to preserve event-id/write order and updates core execution to wait for lifecycle callbacks (e.g., block callbacks) to settle before returning the terminal result.

Reviewed by Cursor Bugbot for commit a300077. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Greptile Summary

This PR fixes a set of terminal console race conditions that caused child workflow errors, error-path blocks, reconnect replays, and scoped cancellations to display stale or duplicate rows. The core strategy is to scope cancelRunningEntries to a specific executionId, reconcile child workflow trace spans before sweeping running entries, serialize event-buffer writes via a promise queue, and await fire-and-forget lifecycle callbacks before returning the execution result.

  • Scoped cancellation: cancelRunningEntries now accepts an optional executionId so completing one execution no longer cancels running entries belonging to a concurrently active execution.
  • Child-span reconciliation: reconcileFinalBlockLogs now walks childTraceSpans and updates corresponding console entries for blocks inside child workflows, preventing them from being swept to canceled.
  • Event-buffer ordering + lifecycle sequencing: write() serializes through a writeQueue promise, and waitForLifecycleCallbacks() ensures all block-event callbacks complete before the final result is returned.

Confidence Score: 3/5

Safe to merge for simple workflows; child workflows that contain loop blocks will see all loop-iteration rows overwritten with the same span data

The new reconcileChildTraceSpans function calls updateConsole without iteration-discriminating fields (iterationCurrent, iterationContainerId), so for a child workflow that uses a loop, every per-iteration console row for the same block matches and gets stamped with the same final-span output. The TraceSpan type already carries iterationIndex and loopId that would fix this, but they are not forwarded. The remaining changes are correct and well-tested.

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts — specifically the reconcileChildTraceSpans function and its updateConsole calls

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts Adds child-span reconciliation and scoped cancelRunningEntries; reconcileChildTraceSpans omits iteration fields in updateConsole calls, breaking loop scenarios in child workflows
apps/sim/lib/execution/event-buffer.ts Serializes concurrent write() calls through a writeQueue promise chain, preventing out-of-order event-ID reservation during reconnect replay
apps/sim/lib/workflows/executor/execution-core.ts Decouples persistence from lifecycle callbacks; lifecycle callbacks are tracked and awaited via pendingLifecycleCallbacks before the execution result is returned
apps/sim/stores/terminal/console/store.ts cancelRunningEntries gains an optional executionId parameter to scope cancellation to a single execution, preventing stale cancellation of unrelated entries
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts All cancelRunningEntries callsites now pass the specific executionId; reconnect replay clears entries only when starting from eventId 0

Sequence Diagram

sequenceDiagram
    participant Executor
    participant ExecutionCore
    participant EventBuffer
    participant LifecycleCallbacks
    participant ConsoleStore

    Executor->>ExecutionCore: execute()
    loop Per block
        ExecutionCore->>EventBuffer: write(block:started) [queued via writeQueue]
        EventBuffer-->>ExecutionCore: eventId (ordered)
        ExecutionCore->>LifecycleCallbacks: trackLifecycleCallback(onBlockStart)
        ExecutionCore->>EventBuffer: write(block:completed) [queued]
        EventBuffer-->>ExecutionCore: eventId (ordered)
        ExecutionCore->>LifecycleCallbacks: trackLifecycleCallback(onBlockComplete)
    end
    Executor-->>ExecutionCore: ExecutionResult
    ExecutionCore->>LifecycleCallbacks: waitForLifecycleCallbacks()
    LifecycleCallbacks-->>ExecutionCore: all callbacks settled
    ExecutionCore->>ConsoleStore: reconcileFinalBlockLogs(finalBlockLogs)
    note over ConsoleStore: Updates running entries + child trace spans
    ExecutionCore->>ConsoleStore: cancelRunningEntries(workflowId, executionId)
    note over ConsoleStore: Only cancels entries for THIS executionId
    ExecutionCore-->>Executor: final result
Loading

Comments Outside Diff (1)

  1. apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts, line 483-505 (link)

    P1 Missing iteration discriminators in child-span updateConsole calls

    reconcileChildTraceSpans calls updateConsole(span.blockId, {...}, executionId) without including iteration-discriminating fields. The store's matchesEntryForUpdate narrows on iterationCurrent and iterationContainerId only when those keys are present in the update object — omitting them causes every console entry sharing the same (blockId, executionId, childWorkflowBlockId) to be updated. For a child workflow that contains a loop, all per-iteration rows for the same block will be overwritten with the final span's data rather than their own, silently producing stale output/status on every iteration except the last.

    TraceSpan already carries iterationIndex (→ iterationCurrent) and loopId / parallelId (→ iterationContainerId) that can be forwarded into the update to scope the match to the correct row.

Reviews (1): Last reviewed commit: "fix(terminal): terminal console update f..." | Re-trigger Greptile

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