Skip to content

Cover the SSE event dispatch table in use-sse - #959

Merged
selfcontained merged 1 commit into
mainfrom
agt_4060c0043635/job-test-enforcer-07bd2594
Aug 15, 2026
Merged

Cover the SSE event dispatch table in use-sse#959
selfcontained merged 1 commit into
mainfrom
agt_4060c0043635/job-test-enforcer-07bd2594

Conversation

@selfcontained

Copy link
Copy Markdown
Owner

Recurring Test Enforcer run for 2026-08-14.

Local suite: green on the first pass (27th consecutive)

Command Result
pnpm run check clean
pnpm run format clean on a warm tree — #942's .prettierignore fix still holding
pnpm run test server 161 files / 2733 tests · web 62 / 898 · extension 9 / 60 · scripts 4
pnpm run test:e2e 180 passed / 12 skipped
pnpm run test:e2e:live 11 passed in 21.4s, zero stray e2e-* tmux sessions (18th consecutive)
pnpm run finalize:web built

CI scan of the last 50 runs: 49 success. The single failure is the already-triaged Release-workflow guard (origin/main advanced after smoke tests) aborting v0.33.12 during a merge race — working as designed, not a test failure. No new flakes; the Brain flakes list stays empty.

No failures to triage, no flaky tests, and no product bugs found this run, so the effort went to the queued next_focus coverage gap.

What this adds

use-sse.ts has two halves. The fatal-EventSource reconnect state machine from #921 was already well covered — 8 tests spanning fatal vs. transient errors, exponential backoff, the STABLE_CONNECTION_MS health gate, connection-age vs. instance-age across an internal retry, hidden-tab cancellation, and unmount.

The other half, handleSSEMessage, was not covered at all. It is the wiring hub for the whole realtime UI: ~20 server push types, each routed into a specific react-query cache mutation or invalidation. Only snapshot was exercised, and only as a delivery vehicle for the backoff tests — nothing asserted what any event does to the cache.

23 tests now drive the real hook through the fake EventSource, so the routing itself is what gets pinned rather than any helper in isolation:

  • Cache writes — snapshot ordering and the state it does not carry (jobs, templates, brain, whiteboard, cached release info) plus the injection-hold reset that has to fail safe; upsert insertion order; delete leaving a defined empty list; terminal, injection-hold, and diff-stats keys.
  • Scoping — the exact: true flags that stop one agent's event from refetching every other agent's media or message list, and the review-detail invalidation predicate, verified against matching and non-matching keys.
  • Branching — the whiteboard agent-drew flag set only for agent strokes; stream start/stop touching only the streaming agent; a review with no attributed reviewer skipping the badge but still refreshing the lists.
  • Failure paths — an unparseable frame that must not wedge the stream, an unrecognized event type from a newer server, the notification ack firing only when something was actually shown, and its fire-and-forget rejection being handled.
  • Connection gate — the stream must not open before authentication, including via the foreground path, which reopens without re-running the effect and so has to re-check auth itself.

Mutation battery

50 mutants against use-sse.ts, 47 killed.

The first pass killed 45/49 and surfaced one real weakness: the foreground path re-checks auth, but the original gating test never fired visibilitychange while unauthenticated. Fixed and killed.

The remaining three survivors were diagnosed rather than papered over, and are provably unobservable:

  • patchAgentHasStream's hasStream !== hasStream guard and media.seen's !file.seen guard are each redundant with react-query's structural sharing.
  • applyReviewCreated's if (!reviewerAgentId) return only ever skips an element-identical rewrite.

An earlier draft asserted referential stability to chase the first two. Review pointed out that no change to use-sse.ts can make those assertions fail — only disabling structuralSharing on the QueryClient can — so they were testing react-query, not this hook. They were dropped, and the redundancy is recorded in the Brain backlog as a product-cleanup candidate instead.

Review

One general review agent inspected this run's diff and returned four items. Three applied:

  1. survives an ack request that rejects could never fail — the entire dispatch runs inside try/catch, so not.toThrow() passed with .catch() deleted. Rewritten to assert the rejection handler is attached; this kills a mutant that previously survived.
  2. The two identity-stability assertions removed, per above.
  3. Order-coupling relaxed on the four within-one-event invalidation assertions, since reordering invalidations inside a branch is a behavior-neutral refactor. toEqual is kept for the job/template/brain case, where the order is the test's own emit sequence and is what proves each event maps to its own collection.

Declined: deleting a pre-existing pure-helper test that the new integration test dominates. It is harmless, and removing existing passing coverage is out of scope for a test-health run.

🤖 Generated with Claude Code

The reconnect state machine added in #921 was already well covered, but
`handleSSEMessage` — the table that routes ~20 server push types into
react-query cache writes — was not. Only `snapshot` was exercised, and
only as a delivery vehicle for the backoff tests, with nothing asserted
about what it does to the cache. Every realtime update in the app flows
through this function.

Adds 23 tests driving the real hook through the fake EventSource, so the
routing itself is what gets pinned: which payload reaches which key, the
`exact` scoping that keeps one agent's event from refetching every other
agent's list, the review-detail invalidation predicate, the ack that must
only fire when a notification was actually shown, and the tolerance of an
unparseable or unrecognized frame. Also covers the connection gate — the
stream must not open before authentication, including on the foreground
path, which reopens without re-running the effect.

Mutation battery: 50 mutants, 47 killed. The three survivors are provably
unobservable rather than uncovered — `patchAgentHasStream`'s no-op guard
and `media.seen`'s `!file.seen` guard are each redundant with react-query
structural sharing, and `applyReviewCreated`'s null guard only ever skips
an element-identical rewrite. An earlier draft asserted referential
stability to chase the first two; those assertions were dropped because no
change to use-sse.ts can fail them, which made them a test of react-query
rather than of this hook.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@selfcontained
selfcontained merged commit c139a56 into main Aug 15, 2026
1 check passed
@selfcontained
selfcontained deleted the agt_4060c0043635/job-test-enforcer-07bd2594 branch August 15, 2026 02:40
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