Skip to content

fix(server): report session log persistence and keep followers alive - #61831

Open
kitlangton wants to merge 2 commits into
devfrom
session-events-stream
Open

kitlangton wants to merge 2 commits into
devfrom
session-events-stream

Conversation

@kitlangton

@kitlangton kitlangton commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

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

  1. A client receives log.synced and cannot tell whether new event payloads will be available.
  2. A quiet followed log sends no keep-alives.

✅ After

  1. The marker includes persist, the current process's persistence setting. Older servers omit it, which means unknown. Enabling persistence does not recover previously discarded history.
  2. Followers receive SSE heartbeat comments every 15 seconds. These never advance the durable cursor, and SSE responses bypass compression.
- synced = { aggregateID, seq }
+ synced = { aggregateID, seq, persist }
- follow = encoded durable events
+ follow = encoded durable events + periodic heartbeat comments

📊 Proof

Case Before After
Persistence disabled ❌ Watermark alone ✅ Explicit persist: false
Quiet followed log ❌ No keep-alive ✅ Cursor-free comments
Client accepts gzip Compressed log body ✅ SSE remains uncompressed

The 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:

  1. Live status, permission/form requests and usage, with authoritative reconnect snapshots.
  2. A managed-server persistence setting and explicit history-gap reporting.

👀 Read: packages/server/src/handlers/session.ts adds comments after the standard SSE encoder, preserving its failure frames; packages/server/src/process.ts excludes 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 generate and bun run check:generated in both Protocol and Client — pass.
  • bun typecheck in Server and Client — pass; Core, Schema and Protocol typechecks also passed in the earlier root check.
  • Oxlint and Prettier on changed source/tests — pass.
  • Root bun run check: lint passed; typechecking stopped on an existing error in untouched packages/app/src/session/composer/region.tsx:338 (pinned does not exist).

Pre-push app typecheck fails on dev at packages/app/src/session/composer/region.tsx:338 (timeline.view.pinned), unrelated; this PR doesn't touch packages/app.

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72">
  <style>.pulse{animation:beat 3s ease-in-out infinite}@keyframes beat{0%,100%{opacity:.25}50%{opacity:1}}@media(prefers-reduced-motion:reduce){.pulse{animation:none}}</style>
  <g fill="none" stroke="currentColor"><rect x="8" y="20" width="52" height="32" rx="6" opacity=".5"/><path d="M60 36h120" opacity=".25"/><rect x="180" y="20" width="52" height="32" rx="6" opacity=".5"/></g>
  <g fill="currentColor" font-family="sans-serif" font-size="11" text-anchor="middle"><text x="34" y="40">log</text><text x="206" y="40">client</text><text x="120" y="62" opacity=".6">heartbeat</text><circle class="pulse" cx="120" cy="36" r="4"/></g>
</svg>

Added by the opencode bot (AI-generated).

Related: #42839

Public API: adds optional persist to the log.synced marker and SSE heartbeat comments on followed session logs; SSE responses are no longer compressed (backwards compatible)

@opencodejr opencodejr 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.

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.synced gets an optional persist: boolean. It's updated in the OpenAPI spec and the generated client EventLogSynced type; older servers omit it.
  • With follow=true, the response gets : heartbeat SSE comment lines every 15s and Cache-Control: no-cache, no-transform and X-Accel-Buffering: no headers. Standard SSE parsers ignore comment lines; a hand-written parser that doesn't handle them could break.
  • text/event-stream responses are no longer compressed by the server.

No blocking concerns.

Related: #42839

@opencodejr opencodejr 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.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant