Skip to content

feat(turn): SSE streaming response mode for POST /turn (ADR-0029) - #171

Merged
pdettori merged 8 commits into
rossoctl:mainfrom
pdettori:docs/turn-sse-streaming-spec
Aug 26, 2026
Merged

feat(turn): SSE streaming response mode for POST /turn (ADR-0029)#171
pdettori merged 8 commits into
rossoctl:mainfrom
pdettori:docs/turn-sse-streaming-spec

Conversation

@pdettori

Copy link
Copy Markdown
Member

Summary

Adds an SSE streaming response mode for POST /turn, selected purely by content
negotiation (Accept: text/event-stream). Implements the design in
docs/specs/2026-08-26-turn-sse-streaming-design.md and the decision recorded in
ADR-0029. Resolves #167.

Streaming is a representation of /turn chosen by the Accept header — not a new
/turn/stream route. When the client does not ask for the stream, the endpoint behaves
byte-for-byte as before.

What changed

  • @sh/harness/turn-stream — a TurnStreamFrame union (text / thinking /
    tool_use / tool_result / done / error) plus an sseExtension translator that
    renders each frame to the event: <type>\ndata: <JSON>\n\n wire form, with a byte-capped
    tool_result preview.
  • @sh/harness/run-turn — additive optional onEvent? / signal? inputs on the shared
    executeTurn core and a wireAbort bridge. runTurn (the sync path) is untouched.
  • @sh/knative-server — the Accept branch: streaming calls executeTurn directly and
    flushes SSE frames lazily (headers sent only once the first frame is ready, so an unknown
    sessionId or missing prompt still returns a real 404 / 400 with the same JSON body
    as the sync path); heartbeat keepalives; client disconnect aborts the in-flight turn via
    AbortControllersession.abort(), and the session persists to its last durable
    checkpoint so it resumes identically to a completed turn.
  • Docs — the curl -N example, the frame vocabulary, disconnect/resume semantics, tuning
    knobs, and the Knative autoscaling.knative.dev/target-burst-capacity: "0" anti-buffering
    note (the acceptance artifact owed by ADR-0029).
  • Gated smokedeploy/knative/turn-stream-smoke.sh (TURN_STREAM_LIVE_SMOKE=1) proves
    the stream actually streams (incremental inter-frame timing, not just final content),
    that the terminal done carries a sessionId, and that a follow-up turn on that
    sessionId resumes.

Back-compat

The default JSON /turn contract is unchanged. A golden test pins the sync body against a
frozen wire literal ({"sessionId":...,"response":...,"stopReason":...}) rather than
JSON.stringify(result), so an upstream key-order or serialization change is caught rather
than mirrored on both sides.

Testing

  • make test — full workspace suite green (knative-server 184/184).
  • make typecheck, make lint — clean.
  • Live streaming behavior covered by the gated smoke above (manual acceptance step; skips
    cleanly with the env var unset).

Scope (YAGNI)

No new route, no full-payload frames, no replay/backfill, no filter params, no auth change —
per ADR-0029 §6.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>

Design for a streaming response mode on POST /turn, selected by content
negotiation (Accept: text/event-stream), surfacing assistant-text/thinking
deltas and tool-call events live while keeping the non-streaming JSON
contract byte-for-byte unchanged. Realizes issue rossoctl#167.

Rides the shared executeTurn core (ADR-0028) via a new HTTP-agnostic
sseExtension sink + TurnStreamFrame union; server owns SSE transport with a
lazy header flush that preserves pre-turn 404/400 status parity. Client
disconnect wires to session.abort(); persistence/resume inherited unchanged.
Knative activator buffering mitigated (target-burst-capacity:"0") and
validated by an inter-frame-timing smoke.

- docs/specs/2026-08-26-turn-sse-streaming-design.md
- docs/adrs/0029-turn-sse-streaming.md (+ README index row)

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Add the neutral event seam between the turn core and any transport:
the TurnStreamFrame union, the Pi->frame sseExtension factory, fidelity-B
clip/previewCap, and terminalFrame derivation. Pure and HTTP-free.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
executeTurn gains two optional inputs: onEvent appends sseExtension to the
extension stack; signal wires client disconnect to session.abort() via the
pure wireAbort bridge. runTurn's signature and 404 contract are untouched.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Branch on Accept: text/event-stream after handleTurn's validation.
handleTurnStream lazily flushes SSE headers (404/400 parity preserved),
streams frames through executeTurn's onEvent sink, derives the terminal
frame from TurnResult, heartbeats, and aborts on client disconnect. The
sync JSON emission line is untouched (pinned by a golden byte test).

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
turn-stream-smoke.sh (TURN_STREAM_LIVE_SMOKE=1) curl -N's a streaming turn,
asserts event: text deltas arrive incrementally (anti-buffering, needs
target-burst-capacity: 0), the terminal done carries a sessionId, and a
follow-up turn on that sessionId resumes.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Add the Accept: text/event-stream representation of POST /turn beside the
sync example: the curl -N invocation, the frame vocabulary, disconnect/
resume behavior, tuning knobs, and the target-burst-capacity: 0 Knative
deployment note. Acceptance artifact owed by ADR-0029.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Assert the back-compat sync /turn body against a frozen literal instead of
JSON.stringify(result), so an upstream key-order or serialization change is
caught rather than mirrored on both sides. No behavior change. Addresses a
non-blocking whole-branch review observation.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Clean, well-scoped addition: SSE streaming for POST /turn via Accept: text/event-stream content negotiation, sharing the same turn engine as the sync path. The additive onEvent?/signal? inputs to executeTurn, lazy header flush (preserving 404/400 status parity), and disconnect→abort→resume wiring are sound. Findings below are all doc-accuracy and test-coverage refinements — no correctness or security issues.

Areas reviewed: TypeScript/Node (server + harness), Tests (vitest), Docs (ADR + spec), Shell (smoke script)
Agent/IDE config (.claude/.vscode): none
Supply-chain gate: clean
Commits: all signed-off ✓ CI: passing ✓

Assisted-By: Claude Code

- Bad `sessionId` + streaming `Accept` → real **404 JSON** (pre-first-frame regime), *not* an error
frame.
- Missing `prompt` + streaming `Accept` → **400** (pre-flight).
- **Disconnect/abort:** emit `req` `"close"` mid-turn → assert `session.abort()` fired (spy) and

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion — The disconnect test spec overclaims coverage. §5.2 says the test asserts session.abort() fired (spy) and backend.flush() ran, and §5.4 (line 276) cites "§5.2 flush-on-abort" as evidence for "Session persisted/resumable identically." But the actual test (server.test.ts:1115-1145) only asserts capturedSignal?.aborted === true — it cannot assert abort/flush because executeTurn is mocked in that suite. Suggest softening the spec to "asserts the abort signal propagates to executeTurn" so the doc matches the test, or moving the flush-on-abort claim to a test that exercises the real turn engine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in dd29f16 — §5.2 and the §5.4 table now say the mocked-executeTurn test only proves the AbortSignal reaches the turn core; the session.abort() → durable-resume contract is attributed to the §5.3 live-smoke follow-up turn instead.

});
});

describe("POST /turn — back-compat & streaming", () => {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion — The ADR's only net-new failure surface — a terminal error frame emitted after headers are flushed (server.ts:966-978, regime 3) — has no integration test. Every other regime is covered. Consider a case where the onEvent extension emits ≥1 frame and then executeTurn rejects: assert the response is 200 with a trailing event: error frame (not a 500 JSON body). This path also emits sessionId: "" for a fresh (no-sessionId) turn, which is worth pinning down since it affects client resume.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in dd29f16 — added a regime-3 integration test: executeTurn rejects after a frame has flushed, asserting the response stays 200 text/event-stream and degrades to a terminal event: error frame carrying the same facts, never a rewritten 500 JSON body.

const res = await request("POST", "/turn", { prompt: "Hi" });
expect(res.status).toBe(200);
// Frozen wire bytes (not JSON.stringify(result)): independent of how `result` is constructed,
// so an upstream key-order or serialization change is caught here (back-compat linchpin, ADR-0029).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit — This comment claims the golden assertion catches "an upstream key-order or serialization change." Since runTurn is mocked here, the test pins only the server's own emission line — an upstream TurnResult serialization change wouldn't be caught by it. Suggest narrowing the comment to "freezes the server's sync-response wire bytes."

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in dd29f16 — narrowed the comment to state plainly that with runTurn mocked the golden assertion pins only the server boundary's own sync-response emission bytes, not the upstream turn engine.

* Byte cap for tool_result previews (fidelity B). Read per call so an override takes effect without
* a restart; finite + non-negative or fall back to the default (mirrors server.ts intEnv).
*/
export function previewCap(override?: number): number {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpreviewCap()'s env-var branch (SH_TURN_STREAM_TOOL_RESULT_PREVIEW_BYTES) and the keepalive env parse have no direct test. Low risk, but a small unit test for the parse/default/clamp behavior would lock in the documented knobs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in dd29f16 — added a previewCap unit block covering override > env > default: finite/negative/NaN/Infinity override handling, valid and unparseable env values (0 valid), and override winning over env.

…st, doc accuracy

Addresses the four review comments on the SSE streaming /turn PR (all
doc-accuracy and test-coverage refinements; no behavior change):

- knative-server: add an integration test for regime 3 (the ADR's only
  net-new failure surface) — executeTurn rejecting AFTER a frame is on the
  wire degrades to a terminal `event: error` frame at 200, never a 500 JSON
  body; also pins the sessionId:"" wire shape for a fresh no-sessionId turn.
- knative-server: narrow the golden sync-JSON comment — with runTurn mocked
  it freezes the server's own emission bytes, not the upstream turn engine.
- harness: add a direct unit test for previewCap() override/env resolution,
  default, and clamp (SH_TURN_STREAM_TOOL_RESULT_PREVIEW_BYTES).
- spec: soften §5.2/§5.4 to match what the disconnect test actually asserts
  (the AbortSignal propagates to executeTurn); the real resume-after-abort
  contract is the §5.3 live smoke's follow-up turn, not this mocked unit.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
@pdettori
pdettori merged commit 16d8820 into rossoctl:main Aug 26, 2026
10 checks passed
@pdettori
pdettori deleted the docs/turn-sse-streaming-spec branch August 26, 2026 19:01
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.

feat: streaming responses for /turn (SSE) — surface tokens + tool calls live

1 participant