Skip to content

fix(workflows): await execution log finalization - #6309

Merged
TheodoreSpeaks merged 2 commits into
stagingfrom
fix/workflow-column-execution-log
Aug 6, 2026
Merged

fix(workflows): await execution log finalization#6309
TheodoreSpeaks merged 2 commits into
stagingfrom
fix/workflow-column-execution-log

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • wait for post-execution logging before workflow execution returns or throws
  • preserve streaming ownership transfer and cover success and error lifecycle behavior

Type of Change

  • Bug fix

Testing

  • 39 affected tests passing
  • TypeScript type-check passing
  • lint and full CI audit suite 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 6, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 6, 2026 3:49am

Request Review

@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes core workflow return timing and error paths; behavior is well-tested but affects every non-streaming-transfer execution and how quickly HTTP/job callers observe completion.

Overview
Workflow execution now blocks on post-execution logging finalization before the executeWorkflow promise settles, so callers do not return (or throw) while execution logs are still being persisted.

A finally hook calls loggingSession.waitForPostExecution() unless post-execution ownership was transferred on a successful streaming run (skipLoggingComplete sets postExecutionOwnershipTransferred). On streaming failure, ownership is not transferred, so finalization is still awaited—matching callers that take over logging only after success.

Tests mock waitForPostExecution and cover resolve/reject ordering, streaming success transfer, and streaming error retention. Ship pre-check docs add check:tool-request-boundary to the parallel audit list (alongside existing tool-registry checks).

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

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes non-streaming workflow execution await post-execution logging finalization while preserving ownership transfer to successful streaming callers.

  • Tracks whether post-execution ownership was transferred before deciding whether to wait in finally.
  • Adds success, error, and streaming lifecycle tests.
  • Adds the tool-request boundary audit to the three ship command definitions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/workflows/executor/execute-workflow.ts Adds explicit logging-ownership tracking and awaits post-execution work on every path where ownership is not transferred.
apps/sim/lib/workflows/executor/execute-workflow.test.ts Covers delayed success and rejection settlement plus successful and failed streaming ownership behavior.
.agents/skills/ship/SKILL.md Adds the existing tool-request boundary check to the parallel ship audit list.
.claude/commands/ship.md Keeps the Claude ship command’s audit list synchronized.
.cursor/commands/ship.md Keeps the Cursor ship command’s audit list synchronized.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Execute workflow core] --> B{Execution succeeded?}
  B -- No --> C[Log and rethrow execution error]
  B -- Yes --> D[Handle pause state]
  D --> E{Streaming caller owns completion?}
  E -- Yes --> F[Return result with LoggingSession]
  E -- No --> G[Return ordinary result]
  C --> H[Await post-execution logging]
  G --> H
  F --> I[Streaming layer completes or fails LoggingSession]
Loading

Reviews (2): Last reviewed commit: "chore(ship): resolve staging drift" | Re-trigger Greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@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 5805929. Configure here.

@TheodoreSpeaks
TheodoreSpeaks merged commit 0ebbcc7 into staging Aug 6, 2026
30 checks passed
@TheodoreSpeaks
TheodoreSpeaks deleted the fix/workflow-column-execution-log branch August 6, 2026 04:38
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