Skip to content

fix(events): start every SSE response once its subscriptions are live - #8700

Merged
waleedlatif1 merged 4 commits into
stagingfrom
fix/sse-initial-flush
Oct 7, 2026
Merged

waleedlatif1 merged 4 commits into
stagingfrom
fix/sse-initial-flush

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Next sends a route's status and headers with the first body chunk (pipe-readable calls res.flushHeaders() on the first write, in next dev and next start alike). createSSEStream wrote nothing until an event or the 30 s heartbeat, so a client saw no response for up to 30 s after connecting.

  • In the desktop inbox E2E, the doorbell took 32-34 s to open in every run.
  • Sim desktop's doorbell aborts a response that has not started within 15 s and reconnects with backoff, so the stream would rarely stay open.
  • In the browser, EventSource.onopen fired up to 30 s late, and a rotation's replacement could open after the old stream's grace period closed it.
  • Events themselves were never held back: an event is a body chunk, so it started the response.

Changes:

  • Opening: createSSEStream writes a : connected comment once every subscription is live, which starts the response at once. Clients ignore comments.
  • Readiness: PubSubChannel.ready() settles once this process's Redis SUBSCRIBE on the current connection has completed. A dropped connection is not ready again until it has resubscribed, and a failed SUBSCRIBE leaves the channel not ready. The desktop doorbell, chat status and MCP event streams wait on their channels, so a client that reads its state on open (the desktop pulls its inbox) cannot miss an event published before the channel was listening.
  • Opening deadline: a stream opens at OPEN_DEADLINE_MS (5 s) when a subscription is still not live. A Redis outage then gives an open stream, which rides out the outage like any already-open stream, instead of a silent one. The deadline is shorter than the heartbeat interval, so a heartbeat can never start the response first.
  • Event gate: events wait for the opening too, so none can start the response early. Every event goes through one ordered write chain per stream. It is authorized as soon as it arrives (concurrent events still share one check) but written only after every event before it, so revalidated events keep their order.
  • Back-pressure: at most 16 events (MAX_UNDRAINED_CHUNKS) can wait before the stream opens, or behind an authorization. One more closes the stream (pending_backpressure), and the client reconnects and reconciles. This replaces the revalidated-only authorization_backpressure bound.
  • Closing before opening: a stream that closes before it opens settles its opening gate and clears its deadline timer. A subscription that never becomes ready therefore keeps nothing of the closed stream alive.
  • E2E: the desktop inbox E2E asserts that the doorbell opens within the desktop's 15 s handshake (after compiling the route with a refused request) and that a ring arrives within 2 s of the open.

Type of Change

  • Bug fix

Testing

  • sse-endpoint.test.ts, each red before its fix:
    • the response starts before the first heartbeat;
    • a stream opens only once its subscriptions are ready, holds events until then, and opens at the deadline;
    • revalidated events are written in arrival order;
    • a stream closed before it opens is no longer reachable from a never-ready subscription (checked with a weak reference and a forced GC);
    • its timers are cleared;
    • a revalidated event is delivered at once.
  • pubsub.test.ts covers per-connection readiness: subscribe, reconnect, a failed subscribe, and a subscribe answered after its connection closed.
  • The mothership and MCP event route tests check that each route waits on its channel before opening.
  • Desktop inbox E2E over real HTTP on a cold next dev app:
    • with the fix, 3/3 runs passed;
    • with the old sse-endpoint.ts, 2/2 runs failed, opening at the 30 s heartbeat (30045 and 30052 ms).
    • An earlier version that opened without waiting for the subscription lost the ring sent right after the open.
  • The full gate (lint, typecheck, audits, unit and integration tests) passes.

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)

Next sends a route's status and headers with the first body chunk, and
createSSEStream wrote nothing until an event or the 30 s heartbeat. A client
therefore saw no response for up to 30 s after connecting: the desktop inbox
doorbell E2E waited 32-34 s for its stream to open in every run, and Sim
desktop's doorbell gives up on a response that has not started within 15 s.

- createSSEStream writes a `: connected` comment as soon as every
  subscription is live, which starts the response at once.
- PubSubChannel exposes ready(), settled when the process's Redis SUBSCRIBE
  completes, so a client that reads its state on open cannot miss an event
  published before the channel was listening. The desktop doorbell, chat
  status and MCP streams wait on their channels.
- The desktop inbox E2E asserts the doorbell opens within the desktop's 15 s
  handshake and that a ring arrives within 2 s of the open.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@vercel

vercel Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 7, 2026 1:37am UTC

Request Review

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 14 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/lib/events/sse-endpoint.ts Outdated
Comment thread apps/sim/lib/events/pubsub.ts Outdated
Comment thread apps/sim/lib/desktop/application/executor.ts
@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Changes when event streams announce they are ready to clients.

The PR appears safe to merge; no new actionable issue remains.

What we checked:

  • Closed streams drop queued events: cleanup() sets cleaned before settling the wait. Every later write goes through enqueue(), which returns without writing.
  • Later events cannot pass earlier ones: send() appends waiting events to writes. Its direct-write path is available only when no events are waiting.

Summary

Starts SSE responses once their subscriptions are live, with a five-second fallback during an outage.

  • Tracks Redis readiness separately for each connection.
  • Keeps events in arrival order and bounds waiting events.
  • Settles the opening wait when a stream closes and clears its timers.
  • Adds readiness, ordering, cleanup, and desktop HTTP checks.

All three supplied previous threads are unnumbered, so they have no previousFindings entries. Their fixes are present: events wait for opening, Redis readiness resets after disconnect, and cleanup settles the opening wait.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Create SSE stream] --> B[Wait for subscriptions]
  B --> C{First outcome}
  C -->|Subscriptions ready| D[Write connected comment]
  C -->|Five-second deadline| D
  C -->|Stream closes| E[Settle wait and clear timers]
  D --> F[Write events in arrival order]
  F --> G{Too many waiting events?}
  G -->|Yes| E
  G -->|No| F
Loading

Reviews (4) · Last reviewed commit: "fix(events): write each stream's events ..." · Reviewed by Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 14 files

Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/scripts/test-desktop-inbox-e2e.ts
Comment thread apps/sim/lib/events/sse-endpoint.ts Outdated
Comment thread apps/sim/lib/events/pubsub.ts
- Events wait for the opening gate too, so one published while another
  subscription is still connecting cannot start the response early.
- The gate opens at OPEN_DEADLINE_MS (5 s) when a subscription is not live,
  so a Redis outage degrades to an open stream instead of a silent one.
- PubSubChannel subscribes on every ready connection and is not ready again
  after a dropped one until it has resubscribed.
- The desktop inbox E2E compiles the doorbell route with a refused request
  before timing its open.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 15 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/lib/events/pubsub.ts
…swers a closed connection

A failed SUBSCRIBE no longer settles ready(): the stream's opening deadline
bounds the wait, and the next connection subscribes again. A subscribe
answered after its connection closed is ignored, so it cannot mark a
reconnected channel ready before that connection has subscribed.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 15 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/lib/events/pubsub.ts
… closed before it opens

- Every event goes through one ordered write chain: it is authorized as soon
  as it arrives, so concurrent events still share one check, but written only
  after every event before it. An event queued before the stream opened can
  no longer be overtaken by one that arrived during its authorization.
- Closing settles the opening gate as well as clearing its deadline, so a
  stream closed before it opens is not kept alive by a subscription that
  never becomes ready.
- Tests cover event order, retention after an early close, timer cleanup,
  and the ready wiring of the mothership and MCP event routes.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 16 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit da993c5 into staging Oct 7, 2026
36 of 37 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/sse-initial-flush branch October 7, 2026 02:14

This branch was previously deployed

1 inactive deployment
Preview — a13b9773 Deployed Oct 7, 2026 by vercel[bot]
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