Fix missing realtime broadcasts for step:started and step:completed events #305

jumski · 2025-11-03T21:22:57Z

Summary

Fixes critical bug where step:started and step:completed realtime events were not being broadcast to clients due to PostgreSQL query planner optimizing away unused CTEs.

Bugs Fixed

1. Missing step:started broadcasts (Critical)

Problem: Clients never received step:started events when steps transitioned from Created to Started status.

Root Cause: The broadcast_events CTE in start_ready_steps() was not referenced by any subsequent CTEs or the final RETURN statement. PostgreSQL's query optimizer eliminated it as "dead code", preventing realtime.send() from executing.

Solution: Moved realtime.send() call directly into the RETURNING clause of the UPDATE statement that marks steps as Started. This ensures the broadcast executes atomically with the state change and cannot be optimized away.

2. Missing step:completed broadcasts for empty map steps (Critical)

Problem: Clients never received step:completed events for map steps that received empty arrays and completed immediately.

Root Cause: The broadcast_empty_completed CTE in start_ready_steps() suffered the same optimization issue - not referenced by subsequent operations.

Solution: Moved realtime.send() call into the RETURNING clause of the UPDATE that completes empty map steps.

3. Missing step:completed broadcasts in cascade completion (Critical)

Problem: Clients didn't receive events when taskless steps were cascade-completed in cascade_complete_taskless_steps().

Root Cause: Similar CTE optimization issue in the cascade completion logic.

Solution: Added realtime.send() directly in the RETURNING clause of the cascade completion UPDATE.

Technical Changes

SQL Schema Updates

start_ready_steps.sql:

Removed broadcast_events CTE (step:started)
Removed broadcast_empty_completed CTE (step:completed for empty maps)
Added realtime.send() in RETURNING clause of started_step_states CTE
Added realtime.send() in RETURNING clause of completed_empty_steps CTE

cascade_complete_taskless_steps.sql:

Added realtime.send() in RETURNING clause of cascade completion UPDATE
Broadcasts step:completed events atomically with state transition

complete_task.sql:

Added PERFORM statements for run:failed and step:failed event broadcasts
Ensures error events are reliably broadcast

Client-Side Improvements

New applySnapshot() methods:

Added to FlowRun and FlowStep classes
Applies database state directly without emitting events
Used for initial state hydration from start_flow_with_states() and get_run_with_states()

Why this matters: Previously, we used updateState() for initial state, which internally tried to emit events. This was wrong - the initial state from the database is a snapshot, not a series of events. The new applySnapshot() method makes this distinction clear.

Test Updates

Completely rewrote broadcast tests to understand the actual behavior:

Root steps vs Dependent steps:

Root steps start in the same transaction as start_flow() - already Started when the function returns
Can't observe their step:started broadcasts (would require listening before the transaction commits)
Tests now verify state directly instead of trying to catch impossible broadcasts

Dependent steps:

Start AFTER their dependencies complete (separate transaction)
CAN observe step:started broadcasts (the actual test case for the bug fix)
New test specifically verifies these broadcasts work correctly

Empty map steps:

Root empty maps complete in start_flow() transaction - verify state directly
Dependent empty maps complete after dependencies - verify broadcasts

Testing

All tests passing:

✅ Root steps verified via state (already Started when startFlow() returns)
✅ Dependent steps receive step:started broadcasts
✅ Dependent empty maps receive step:completed broadcasts (skip step:started)
✅ Cascade completion broadcasts step:completed events
✅ Event sequence validation (started → completed order)

Migration Notes

No breaking changes. This is a pure bug fix that makes the system work as originally intended.

Users who implemented workarounds for missing events can now remove them.

changeset-bot · 2025-11-03T21:23:00Z

🦋 Changeset detected

Latest commit: 962cd07

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@pgflow/client	Patch
@pgflow/core	Patch
pgflow	Patch
@pgflow/edge-worker	Patch
@pgflow/example-flows	Patch
@pgflow/dsl	Patch
@pgflow/website	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jumski · 2025-11-03T21:23:14Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

merge:queue - adds this PR to the back of the merge queue
hotfix:queue - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

nx-cloud · 2025-11-03T21:23:51Z

View your CI Pipeline Execution ↗ for commit 962cd07

Command	Status	Duration	Result
`nx affected -t build --configuration=production...`	✅ Succeeded	4s	View ↗
`nx affected -t lint typecheck test --parallel -...`	✅ Succeeded	6m 35s	View ↗
`nx affected -t test:e2e --parallel --base=6b7ac...`	✅ Succeeded	4m 53s	View ↗

☁️ Nx Cloud last updated this comment at 2025-11-03 22:44:17 UTC

graphite-app · 2025-11-03T22:15:27Z

pkgs/client/__tests__/integration/real-flow-execution.test.ts

+      // DEBUG: Check actual database state
+      const dbState = await sql`
+        SELECT step_slug, status, remaining_deps, initial_tasks
+        FROM pgflow.step_states
+        WHERE run_id = ${run.run_id}
+        ORDER BY step_slug
+      `;
+      console.log('DB State:', dbState);
+
+      // DEBUG: Check what get_run_with_states returns
+      const apiResponse = await sql`SELECT pgflow.get_run_with_states(${run.run_id})`;
+      console.log('API Response:', JSON.stringify(apiResponse[0].get_run_with_states, null, 2));
+
+      // DEBUG: Check client state
+      console.log('Client rootStep.status:', rootStep.status);
+      console.log('Client dependentStep.status:', dependentStep.status);


The debug logging statements should be removed before merging. While they're helpful during development for troubleshooting the realtime event issues, they'll clutter the test output in CI and for other developers. Consider either removing them or wrapping them in a conditional debug flag if this diagnostic information might be needed again in the future.

Suggested change

// DEBUG: Check actual database state

const dbState = await sql`

SELECT step_slug, status, remaining_deps, initial_tasks

FROM pgflow.step_states

WHERE run_id = ${run.run_id}

ORDER BY step_slug

`;

console.log('DB State:', dbState);

// DEBUG: Check what get_run_with_states returns

const apiResponse = await sql`SELECT pgflow.get_run_with_states(${run.run_id})`;

console.log('API Response:', JSON.stringify(apiResponse[0].get_run_with_states, null, 2));

// DEBUG: Check client state

console.log('Client rootStep.status:', rootStep.status);

console.log('Client dependentStep.status:', dependentStep.status);

// Check actual database state

const dbState = await sql`

SELECT step_slug, status, remaining_deps, initial_tasks

FROM pgflow.step_states

WHERE run_id = ${run.run_id}

ORDER BY step_slug

`;

// Check what get_run_with_states returns

const apiResponse = await sql`SELECT pgflow.get_run_with_states(${run.run_id})`;

Spotted by Graphite Agent

Is this helpful? React 👍 or 👎 to let us know.

pkgs/client/src/lib/FlowRun.ts

pkgs/client/src/lib/FlowStep.ts

…events for simplification - Eliminated broadcast_empty_completed CTE to streamline completed step event handling - Removed broadcast_events CTE for step:started events to improve query clarity - Replaced broadcast event logic with direct PERFORM statements for real-time event broadcasting - Enhanced real-time event broadcasting for step:started and step:completed events - Simplified event dispatching logic for better maintainability and performance

github-actions · 2025-11-03T22:44:28Z

🔍 Preview Deployment: Website

✅ Deployment successful!

🔗 Preview URL: https://pr-305.pgflow.pages.dev

📝 Details:

Branch: fix-cte-broadcast-sql
Commit: 5450224f46b5e4c329d763bf14c50418475c0c49
View Logs

_Last updated: _

graphite-app · 2025-11-03T23:28:08Z

Merge activity

Nov 3, 11:28 PM UTC: jumski added this pull request to the Graphite merge queue.
Nov 3, 11:28 PM UTC: CI is running for this pull request on a draft pull request (#307) due to your merge queue CI optimization settings.
Nov 3, 11:29 PM UTC: Merged by the Graphite merge queue via draft PR: #307.

…vents (#305) ## Summary Fixes critical bug where `step:started` and `step:completed` realtime events were not being broadcast to clients due to PostgreSQL query planner optimizing away unused CTEs. ## Bugs Fixed ### 1. Missing step:started broadcasts (Critical) **Problem:** Clients never received `step:started` events when steps transitioned from Created to Started status. **Root Cause:** The `broadcast_events` CTE in `start_ready_steps()` was not referenced by any subsequent CTEs or the final RETURN statement. PostgreSQL's query optimizer eliminated it as "dead code", preventing `realtime.send()` from executing. **Solution:** Moved `realtime.send()` call directly into the RETURNING clause of the UPDATE statement that marks steps as Started. This ensures the broadcast executes atomically with the state change and cannot be optimized away. ### 2. Missing step:completed broadcasts for empty map steps (Critical) **Problem:** Clients never received `step:completed` events for map steps that received empty arrays and completed immediately. **Root Cause:** The `broadcast_empty_completed` CTE in `start_ready_steps()` suffered the same optimization issue - not referenced by subsequent operations. **Solution:** Moved `realtime.send()` call into the RETURNING clause of the UPDATE that completes empty map steps. ### 3. Missing step:completed broadcasts in cascade completion (Critical) **Problem:** Clients didn't receive events when taskless steps were cascade-completed in `cascade_complete_taskless_steps()`. **Root Cause:** Similar CTE optimization issue in the cascade completion logic. **Solution:** Added `realtime.send()` directly in the RETURNING clause of the cascade completion UPDATE. ## Technical Changes ### SQL Schema Updates **`start_ready_steps.sql`:** - Removed `broadcast_events` CTE (step:started) - Removed `broadcast_empty_completed` CTE (step:completed for empty maps) - Added `realtime.send()` in RETURNING clause of `started_step_states` CTE - Added `realtime.send()` in RETURNING clause of `completed_empty_steps` CTE **`cascade_complete_taskless_steps.sql`:** - Added `realtime.send()` in RETURNING clause of cascade completion UPDATE - Broadcasts step:completed events atomically with state transition **`complete_task.sql`:** - Added PERFORM statements for run:failed and step:failed event broadcasts - Ensures error events are reliably broadcast ### Client-Side Improvements **New `applySnapshot()` methods:** - Added to `FlowRun` and `FlowStep` classes - Applies database state directly without emitting events - Used for initial state hydration from `start_flow_with_states()` and `get_run_with_states()` **Why this matters:** Previously, we used `updateState()` for initial state, which internally tried to emit events. This was wrong - the initial state from the database is a snapshot, not a series of events. The new `applySnapshot()` method makes this distinction clear. ### Test Updates Completely rewrote broadcast tests to understand the actual behavior: **Root steps vs Dependent steps:** - Root steps start in the same transaction as `start_flow()` - already Started when the function returns - Can't observe their step:started broadcasts (would require listening before the transaction commits) - Tests now verify state directly instead of trying to catch impossible broadcasts **Dependent steps:** - Start AFTER their dependencies complete (separate transaction) - CAN observe step:started broadcasts (the actual test case for the bug fix) - New test specifically verifies these broadcasts work correctly **Empty map steps:** - Root empty maps complete in `start_flow()` transaction - verify state directly - Dependent empty maps complete after dependencies - verify broadcasts ## Testing All tests passing: - ✅ Root steps verified via state (already Started when startFlow() returns) - ✅ Dependent steps receive step:started broadcasts - ✅ Dependent empty maps receive step:completed broadcasts (skip step:started) - ✅ Cascade completion broadcasts step:completed events - ✅ Event sequence validation (started → completed order) ## Migration Notes No breaking changes. This is a pure bug fix that makes the system work as originally intended. Users who implemented workarounds for missing events can now remove them.

jumski mentioned this pull request Nov 3, 2025

Critical: Fix CTE Broadcast Bug and Add Comprehensive Realtime Event Testing #304

Closed

9 tasks

jumski changed the title ~~refactor(schemas): remove broadcast empty completed and step started events for simplification~~ Fix missing realtime broadcasts for step:started and step:completed events Nov 3, 2025

jumski force-pushed the fix-cte-broadcast-sql branch 2 times, most recently from 9c48a1e to 560de7c Compare November 3, 2025 22:13

graphite-app bot reviewed Nov 3, 2025

View reviewed changes

pkgs/client/src/lib/FlowRun.ts Show resolved Hide resolved

pkgs/client/src/lib/FlowStep.ts Show resolved Hide resolved

jumski force-pushed the fix-cte-broadcast-sql branch 2 times, most recently from da943e2 to 10490fe Compare November 3, 2025 22:34

jumski force-pushed the fix-cte-broadcast-sql branch from 10490fe to 962cd07 Compare November 3, 2025 22:35

jumski temporarily deployed to preview November 3, 2025 22:43 — with GitHub Actions Inactive

jumski mentioned this pull request Nov 3, 2025

Fix missing realtime broadcasts for step:started and step:completed events #306

Closed

graphite-app bot closed this Nov 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix missing realtime broadcasts for step:started and step:completed events #305

Fix missing realtime broadcasts for step:started and step:completed events #305

Uh oh!

jumski commented Nov 3, 2025 •

edited

Loading

Uh oh!

changeset-bot bot commented Nov 3, 2025 •

edited

Loading

Uh oh!

jumski commented Nov 3, 2025 •

edited

Loading

Uh oh!

nx-cloud bot commented Nov 3, 2025 •

edited

Loading

Uh oh!

graphite-app bot Nov 3, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

graphite-app bot commented Nov 3, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix missing realtime broadcasts for step:started and step:completed events #305

Fix missing realtime broadcasts for step:started and step:completed events #305

Uh oh!

Conversation

jumski commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bugs Fixed

1. Missing step:started broadcasts (Critical)

2. Missing step:completed broadcasts for empty map steps (Critical)

3. Missing step:completed broadcasts in cascade completion (Critical)

Technical Changes

SQL Schema Updates

Client-Side Improvements

Test Updates

Testing

Migration Notes

Uh oh!

changeset-bot bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

jumski commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

nx-cloud bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graphite-app bot Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 3, 2025

🔍 Preview Deployment: Website

Uh oh!

graphite-app bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jumski commented Nov 3, 2025 •

edited

Loading

changeset-bot bot commented Nov 3, 2025 •

edited

Loading

jumski commented Nov 3, 2025 •

edited

Loading

nx-cloud bot commented Nov 3, 2025 •

edited

Loading

graphite-app bot commented Nov 3, 2025 •

edited

Loading