Repository navigation
fix(server): report session log persistence and keep followers alive - #61831
kitlangton wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Looks sound. I ran the PR's new server tests on dev: the marker has no persist, followed logs get no heartbeat headers, and they're gzip-compressed when the client accepts gzip. With this PR the server, core and schema suites pass, and the server type-check passes.
The compression change works because compression now runs after the log's heartbeat hook. The no-transform header and the text/event-stream check then skip it, so raw heartbeat bytes can't end up inside a gzipped body. Each SSE event is written as one complete chunk, so heartbeats can't land in the middle of an event.
Public API impact (experimental GET /api/experimental/session/:sessionID/log, backwards compatible):
EventLog.Synced/log.syncedgets an optionalpersist: boolean. It's updated in the OpenAPI spec and the generated clientEventLogSyncedtype; older servers omit it.- With
follow=true, the response gets: heartbeatSSE comment lines every 15s andCache-Control: no-cache, no-transformandX-Accel-Buffering: noheaders. Standard SSE parsers ignore comment lines; a hand-written parser that doesn't handle them could break. text/event-streamresponses are no longer compressed by the server.
No blocking concerns.
Related: #42839
There was a problem hiding this comment.
The new commits change only tests; production code is unchanged since my last review. I ran the reworked real-server test on dev: both persistence runs fail there and pass with this PR. The server process and sse-failure tests passed in five repeated runs; the core and server checks also pass.
The public API impact is unchanged: an optional persist on log.synced, heartbeat comments and no-cache headers on followed logs, and no compression for SSE responses. All are backwards compatible.
No new concerns.
Session-log clients can detect disabled persistence, and quiet followers receive keep-alives.
Programmatic clients and orchestrators currently receive a replay watermark even when event payloads are not being retained.
🔍 Origin: observed while following session logs with the default server configuration.
❌ Before
log.syncedand cannot tell whether new event payloads will be available.✅ After
persist, the current process's persistence setting. Older servers omit it, which means unknown. Enabling persistence does not recover previously discarded history.📊 Proof
persist: falseThe full affected test files pass 122 tests, without name filters. One parameterized real-server test covers default/explicit persistence, replay, unknown sessions, gzip, heartbeat timing and subsequent durable events. Disabled-persistence cursor assertions reuse the existing bus test. A separate unpatched run was not performed.
The simplify pass reduced the diff from +246/−24 across 13 files to +163/−21 across 11 files by removing the custom HTTP fixture and duplicate setup. Production code is unchanged by that pass.
This is the first slice; the log still carries durable events only. Follow-up slices:
👀 Read:
packages/server/src/handlers/session.tsadds comments after the standard SSE encoder, preserving its failure frames;packages/server/src/process.tsexcludes SSE before compression. Independent review caught the compression ordering, immediate first tick and missing OpenAPI regeneration; all are addressed.Observed with programmatic clients · base: dev, because these are shared core/server contracts · persistence remains opt-in
Checks run
packages/core:bun run script/test.ts test/session-log.test.ts test/bus.test.ts— pass.packages/server:bun test test/process.test.ts test/sse-failure.test.ts— pass.packages/schema:bun test test/event.test.ts— pass.packages/client:bun test test/effect.test.ts test/promise.test.ts— pass.packages/protocol:bun test test/experimental-identifiers.test.ts— pass.bun run generateandbun run check:generatedin both Protocol and Client — pass.bun typecheckin Server and Client — pass; Core, Schema and Protocol typechecks also passed in the earlier root check.bun run check: lint passed; typechecking stopped on an existing error in untouchedpackages/app/src/session/composer/region.tsx:338(pinneddoes not exist).Pre-push app typecheck fails on
devatpackages/app/src/session/composer/region.tsx:338(timeline.view.pinned), unrelated; this PR doesn't touchpackages/app.Visual
Added by the opencode bot (AI-generated).
Related: #42839
Public API: adds optional
persistto thelog.syncedmarker and SSE heartbeat comments on followed session logs; SSE responses are no longer compressed (backwards compatible)