fix(sse): add periodic heartbeats to web factories and agent streams

[jaredglaser]

Summary

• Both web SSE factories (createStatsSseHandler, createBroadcastSseHandler) now run a 25 s setInterval comment ping (: ping), guarded by the closed flag and cleared in teardown. A ping that hits a dead consumer (enqueue throws) triggers full teardown, so abandoned streams unsubscribe instead of lingering.
• The factory error paths (subscribe throws, loadSubscribe rejects) now route through teardown so the interval cannot leak.
• Added a shared startSseHeartbeat helper to agent/src/lib/sse-utils.ts and wired it into the agent stats, zfs, and containers-events streams (cleared on abort and in each stream's teardown/finally path). The logs route already had its own heartbeat.
• Heartbeat interval is injectable (heartbeatIntervalMs option/param, default 25 s) so tests run with millisecond intervals.
• Verified consumers are unaffected: worker collectors skip SSE messages without a data: line, and browser EventSource ignores comment frames natively.

Testing

• bun run typecheck and bun run typecheck:agent pass.
• New test file src/lib/sse/__tests__/create-stats-sse-handler.test.ts (headers, flush comment, data/error frames, subscribe failure, abort teardown, heartbeat ping, dead-consumer teardown, 401).
• Extended create-broadcast-sse-handler.test.ts with heartbeat ping, dead-consumer teardown, and interval-cleared-on-abort tests.
• Extended agent sse-utils.test.ts with startSseHeartbeat coverage (ping cadence, isClosed stop, enqueue-throw self-clear, stop function).
• Added one idle-heartbeat test each to agent stats.test.ts, zfs.test.ts, containers-events.test.ts.
• All affected files green under bun test --isolate (21 web + 68 agent tests).

Closes #233

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

@jaredglaser, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes and 2 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ba6f987b-cf6c-4999-87be-ef862f60aab6

📥 Commits

Reviewing files that changed from the base of the PR and between 5d66d9f and 492c674.

📒 Files selected for processing (12)

• agent/src/__tests__/containers-events.test.ts
• agent/src/__tests__/stats.test.ts
• agent/src/__tests__/zfs.test.ts
• agent/src/lib/__tests__/sse-utils.test.ts
• agent/src/lib/sse-utils.ts
• agent/src/routes/containers-events.ts
• agent/src/routes/stats.ts
• agent/src/routes/zfs.ts
• src/lib/sse/__tests__/create-broadcast-sse-handler.test.ts
• src/lib/sse/__tests__/create-stats-sse-handler.test.ts
• src/lib/sse/create-broadcast-sse-handler.ts
• src/lib/sse/create-stats-sse-handler.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/f7-sse-heartbeats

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jaredglaser]

Triage verdict: Close (obsolete) (confidence: high)

Closing this as overtaken by main, with one piece worth re-filing. src/lib/sse/create-sse-stream.ts is now the single owner of the SSE wire protocol and already implements the heartbeat (DEFAULT_HEARTBEAT_MS = 5000 at :13, setInterval at :89, cleared in teardown at :57-63), and both factories this PR edits were rewritten to delegate to it. main also carries the equivalent test at src/lib/sse/tests/create-stats-sse-handler.test.ts:172, and agent/src/routes/containers-events.ts:126-141 has its own 5s heartbeat. createStatsSseHandler gained a second channel argument on main, so the signature here no longer fits either. All 15 conflict hunks are in those four already-solved files. On the residual: agent/src/routes/stats.ts genuinely has no heartbeat and should get one, since a host with zero running containers produces a silent stream. agent/src/routes/zfs.ts does not need one though, because it pipes zpool iostat -v 1 and the loop at :44-64 enqueues every non-empty line, so that stream emits every second for the life of the process and cannot idle. One correction for the follow-up: 25s is too slow. Bun's default HTTP idleTimeout is 10s, which main notes explicitly at containers-events.ts:127-129 and is why it chose 5s, and the agent only sets idleTimeout: 0 inside the websocket block of Bun.serve. Please also use the same ':\n\n' comment frame both existing implementations emit rather than ': ping\n\n', so there is one spelling on the wire. Suggest retitling issue #233 to 'agent /stats/stream has no SSE heartbeat' rather than closing it, since its cited file and line locations no longer exist on main.

Specific items to fix

• 25s heartbeat exceeds Bun's 10s default HTTP idleTimeout, so it does not prevent the disconnect it exists to prevent
• Reintroduces three copies of the heartbeat constant where main has one owner in create-sse-stream.ts
• zfs.ts heartbeat guards nothing: zpool iostat -v 1 emits every second and the route enqueues every line
• createStatsSseHandler signature changed on main (second channel argument), so the overload here does not apply

Effort if pursued: n/a (recommend close). Follow-up for agent/src/routes/stats.ts is about 15 lines plus a test, under an hour.

Obsolescence: fully for the web half and the containers-events half; the zfs half is unnecessary. Proof: origin/main:src/lib/sse/create-sse-stream.ts:13,57-63,89; origin/main:src/lib/sse/create-broadcast-sse-handler.ts:1 and create-stats-sse-handler.ts:1,12,16 both delegate to createSseStream; origin/main:src/lib/sse/tests/create-stats-sse-handler.test.ts:172; origin/main:agent/src/routes/containers-events.ts:126-141; origin/main:agent/src/routes/zfs.ts:25-28,44-64 (zpool iostat -v 1, enqueues every line).

Sources checked

• origin/main:src/lib/sse/create-sse-stream.ts:13 DEFAULT_HEARTBEAT_MS = 5000, :89 setInterval, :57-63 teardown
• origin/main:src/lib/sse/create-stats-sse-handler.ts:16 (new second argument channel: StatsChannel)
• origin/main:src/lib/sse/__tests__/create-stats-sse-handler.test.ts:172 (heartbeat test already on main)
• origin/main:agent/src/routes/containers-events.ts:126-141 (5s heartbeat; comment notes Bun default idleTimeout is 10s)
• origin/main:agent/src/routes/zfs.ts:25-28,44-64 (zpool iostat -v 1 emits every second)
• git show origin/main:agent/src/routes/stats.ts | grep setInterval|heartbeat -> only an unrelated console.error
• origin/main:agent/src/index.ts (idleTimeout: 0 only inside the websocket block)
• git merge-tree -> 15 conflict hunks across 4 files, all in the obsolete half

Produced by a three-pass review (independent reviewer, adversarial re-reviewer, judge). Where the two passes disagreed, the disagreement was resolved by rechecking the code on main at 2672630.

---

Generated by Claude Code

[jaredglaser]

Closing as superseded by #368.

The heartbeat this PR adds now lives on main inside the shared SSE stream factories, on both sides of the web/agent split:

• src/lib/sse/create-sse-stream.ts:89 — heartbeatTimer = setInterval(() => write(() => ':\n\n'), heartbeatMs), cleared in teardown at line 59.
• agent/src/lib/sse-stream.ts:90 — the same, deliberately mirrored because the agent is not a workspace member and cannot import web code.

Both carry the injectable heartbeatMs option this PR introduced, and both route enqueue failure through teardown, so a ping to a dead consumer unsubscribes rather than lingering. That covers the behavior here without the per-route wiring, since every SSE route now builds its Response through the factory.

Nothing is lost by closing. If a gap turns up, the factory is the place to fix it once rather than per route.

---

Generated by Claude Code

[jaredglaser]

Amending my previous comment on one point. I wrote "nothing is lost by closing" as a bare assertion. The triage verdict above found a real residual, so it deserved evidence rather than a claim.

The residual it named was agent/src/routes/stats.ts having no heartbeat, on the grounds that a host with zero running containers produces a silent stream. That was true when the triage ran against main at 2672630. It is no longer true: #368 landed the agent-side createSseStream factory after that, and all four agent SSE routes now build their Response through it.

$ git show origin/main:agent/src/routes/stats.ts | grep -n "createSseStream"
3:import { createSseStream, type SseEmitter } from '../lib/sse-stream';
394:  return createSseStream(request, {

stats.ts, zfs.ts, logs.ts and containers-events.ts each reference it twice (import plus call site).

The triage's other correction also holds on main: it objected that this PR's 25s interval exceeds Bun's 10s default HTTP idleTimeout, so it would not prevent the disconnect it exists to prevent. agent/src/lib/sse-stream.ts:14 sets DEFAULT_HEARTBEAT_MS = 5000, as does the web factory. The wire format is ':\n\n' on both sides, not ': ping\n\n', so there is one spelling.

So the obsolescence is now complete rather than partial, including the half the triage wanted re-filed. Closing, and closing #233 with it.

---

Generated by Claude Code