Skip to content

fix(server): bound cursorless SSE replay (#882) - #888

Open
blabbler78 wants to merge 1 commit into
open-mercato:mainfrom
blabbler78:fix/issue-882-bounded-sse-replay
Open

blabbler78 wants to merge 1 commit into
open-mercato:mainfrom
blabbler78:fix/issue-882-bounded-sse-replay

Conversation

@blabbler78

Copy link
Copy Markdown
Contributor

Closes #882

🎯 Goal

Keep long run streams usable in Safari and over tunnels by preventing a cursorless EventSource connection from replaying an unbounded multi-megabyte transcript.

🔍 Problem

The released 0.9.2 and 0.9.3 servers replay the complete NDJSON transcript on every connection. Measured runs returned between 3.49 MB and 28.6 MB per connection, and individual tool events reached 1.24 MB. Safari aborted during replay, immediately reconnected, and repeated the same transfer until the cockpit stopped accepting input.

🔍 Root Cause

PR #739 added progressive history, frame ids, Last-Event-ID, and cursor resume on main, but intentionally retained store.readEvents(id) for the protected no-query path. The history hook also uses that path as its compatibility fallback. Consequently the first cursorless connection still has no network or per-frame bound, and a single oversized persisted tool result can reproduce the reconnect amplification even after resume support ships.

What Changed

  • Added an SSE-only event projection that leaves normal frames byte-identical, truncates long string fields only when a frame exceeds 256 KiB, and falls back to a bounded envelope if an event still exceeds that limit. Persisted NDJSON and paged history payloads remain unchanged.
  • Limited the first cursorless replay to a contiguous recent tail with a conservative 1.5 MB wire budget. Reconnects continue to resume through the frame ids already added by feat(history): progressively load long sessions #739.
  • Added unit coverage for total replay and individual-frame bounds, plus a real route regression proving that old history is omitted from SSE while the persisted oversized event remains intact.
  • Updated the protected-contract inventory to document the intentional availability-first narrowing and the history-route migration path for consumers that need the complete archive.

🧪 Tests

  • Regression proof before implementation: run-event-replay.test.ts failed because the bounded replay implementation did not exist.
  • npm test --workspace @open-mercato/cezar -- --run src/server/run-event-replay.test.ts src/server/route-parity.test.ts — 12 tests passed.
  • npm run typecheck — passed for contract, API client, server, and web.
  • npm run test:unit — 35 passed, 1 platform-specific test skipped.
  • npm run build — passed; package check found 478 files and 85 web assets.
  • npm run test:package — 15 tests passed.
  • Two local full npm test runs reached 6,060/6,061 passing tests. The only failures were pre-existing parallel timing flakes in health-topic.test.ts / automations-gate.test.ts; both affected files pass together in isolation (31/31), and the base commit's exact GitHub CI is green: https://github.com/open-mercato/cezar/actions/runs/31816677977. PR CI remains authoritative for this head.

💥 Breaking Changes

The no-query /api/v1/runs/:id/events connection now receives a recent byte-bounded tail instead of the complete transcript. Consumers requiring the complete archive must use /api/v1/runs/:id/history. Event names, required event envelope fields, route aliases, persisted NDJSON, cursor semantics, sequence ids, and event ordering are unchanged. Oversized live SSE payloads can receive the same bounded wire projection while their full persisted form remains available through history.

@CLAassistant

CLAassistant commented Aug 14, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@blabbler78

Copy link
Copy Markdown
Contributor Author

🤖 om-open-pr completed: PR opened. Lock handed off to om-auto-review-pr — chain continues on this PR.

The configured labels (review, bug, needs-qa, priority-high, risk-high, and in-progress) could not be applied because the fork contributor account does not have label or assignment permission in the upstream repository.

@blabbler78

Copy link
Copy Markdown
Contributor Author

🤖 om-auto-fix-issue — run summary

Result

The cursorless per-run SSE replay is now bounded to a recent 1.5 MB tail, and individual SSE frames are bounded to 256 KiB. Ordinary events remain byte-identical; oversized string payloads are projected only on the SSE wire. Full persisted NDJSON and paged history responses remain unchanged.

Evidence

  • Added a fail-before-fix regression suite for both total replay size and single-event size.
  • The real route test verifies that early history is omitted from a cursorless stream, the newest event id is emitted, and the original oversized persisted event is untouched.
  • Typecheck, focused tests (12/12), unit tests (35 passed, 1 platform skip), build/package verification, and package tests (15/15) passed.
  • Full local Vitest reached 6,060/6,061 twice; the only failures were unrelated parallel timing flakes that pass in isolation. The base commit's exact CI is green, and this PR's exact-head CI is the final gate.

Contract note

This intentionally narrows the protected no-query full-replay behavior. Complete transcript consumers must use the paged history endpoint; route aliases, event names and envelope fields, sequence ids, cursor behavior, ordering, and on-disk data remain compatible.

@blabbler78
blabbler78 force-pushed the fix/issue-882-bounded-sse-replay branch from daec5a6 to 0f2f284 Compare August 14, 2026 16:18
@blabbler78

Copy link
Copy Markdown
Contributor Author

📏 Live tunnel verification

I activated the reversible patch against the installed 0.9.3 server and repeated the original long-run measurement through both localhost and the authenticated ngrok tunnel.

  • Cursorless replay: 1,528,380 bytes, down from approximately 29.7 MB for the same run.
  • Replay frames: 938 ids; largest serialized SSE line: 66,301 bytes.
  • Reconnect with the last emitted Last-Event-ID: 8,066 bytes (current run frame + keepalive), rather than another transcript replay.
  • Both tunnel requests returned HTTP 200 and the expected hotfix identification header.
  • Before restart, Safari reopened another stream for the active task roughly every 6–7 seconds. After restart, the ngrok inspector shows one Safari stream opened at 18:23:20 and no subsequent reconnects during the observation window.

This validates the two relevant protections under the reported conditions: a bounded first replay prevents the browser/tunnel abort, and sequence resume prevents amplification if the transport later reconnects. The persisted NDJSON files were not changed.

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.

SSE /runs/:id/events replays the whole transcript on every reconnect — long runs freeze the mobile cockpit and silently drop typed commands

2 participants