Skip to content

clients: decode interaction + preamble/reasoning frames, add submit_interaction - #505

Merged
brentrager merged 1 commit into
mainfrom
fix/client-interaction-frames
Aug 19, 2026
Merged

clients: decode interaction + preamble/reasoning frames, add submit_interaction#505
brentrager merged 1 commit into
mainfrom
fix/client-interaction-frames

Conversation

@brentrager

@brentrager brentrager commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Problem

The generated wire types for stream_preamble, stream_reasoning, interaction_required and interaction_invalid exist in every language, but the hand-maintained dispatch unions were never updated. Each client rejected those frames in its own guard and then discarded them in its dispatch loop:

client rejected at dropped at
Go ParseServerEventUnknownEventError client.go read loop continue // ignore malformed / unknown frames
Python is_server_event / parse_event raises client.py _handle_frame bare except
.NET ServerEventConverterJsonException SmoothAgentClient.HandleFrame catch-and-drop
TypeScript isServerEvent() (for stream_reasoning only) handleFrame early return

Impact. A session declaring a Rich Interaction (identity_form, choice_chips) parked a turn that Go/Python/.NET consumers never saw — the turn hung to the turn timeout, and forever on .NET (see the note at the bottom). stream_preamble and stream_reasoning are emitted by the production server today (rust/smooth-operator-server/src/protocol.rs:73, called from runner.rs:1133), so preamble and reasoning tokens were being discarded by all four clients.

Scope note: the original report named 3 frames and 3 clients. It is 4 frames and 4 clients — spec/events/ holds 17 schemas and TypeScript's union had 16.

The demo app was already silently dropping reasoning tokens. examples/web-chat/src/operator.ts:211 has a case 'stream_reasoning': that renders into a collapsible thinking pane. It iterates for await (const ev of turn) off the real TS client (line 297), and isServerEvent() filtered the frame out before it ever reached the turn — so that branch was dead code that could never fire. It is evidence the frame was expected to work all along, and it now actually fires: worth a glance from whoever owns the example that the rendering does what was intended.

What changed

  • Go — four event discriminators + As* accessors, ActionSubmitInteraction, Client.SubmitInteraction.
  • Python — four members on EventType and the ServerEvent union, submit_interaction on ActionType/ClientAction, SmoothAgentClient.submit_interaction, package re-exports.
  • .NETStreamPreambleEvent, StreamReasoningEvent, InteractionRequiredEvent, InteractionInvalidEvent, wired into ServerEventConverter.ByType and EventTypes.All; SubmitInteractionAction + SubmitInteractionAsync.
  • TypeScript — the missing stream_reasoning in EVENT_TYPES / ServerEvent / ServerEventByType.

Go, Python and .NET gain submit_interaction, mirroring typescript/src/client.ts:470 with the same values-or-declined wire shape. Without it a client can decode the park but cannot answer it, which was the actual user-facing goal. One verb serves every interaction kind, so adding a kind needs no new method.

A second drift site, found by the compiler

Adding stream_reasoning to the TS union turned src/validate.ts into a compile error: it keeps its own Record<EventType, string> event→schema map. Python has the same map untyped (_EVENT_SCHEMA_FILE / _ACTION_SCHEMA_REF in validate.py) and had drifted identically, silently — validate_event() returned Unknown event type for frames the spec defines. Both are corrected. Go and .NET validators take an explicit schema ref, so they have no such map.

.NET's EventTypes.All / ActionTypes.All were a third stale set, fixed and now guarded.

The guard encodes both tiers of the contract

The stream_reasoning schema says "Clients that do not recognize this event MUST ignore it." So there are two rules, and satisfying one naively breaks the other:

  1. Every event in spec/events/ must decode and be surfaced. The drift guard derives the expected set from spec/events/*.schema.json and spec/actions/*.schema.json at test time, never from a list maintained beside the union — a guard asserting against its own copy of the constant would lock the drift in. A new event schema that isn't wired up now fails the build.
  2. A genuinely unknown event must still be ignored gracefully. A future server sending something this client version predates has to be dropped quietly, not throw. That behaviour was already correct in all four clients and is untouched — but it was untested, so nothing stopped someone from "satisfying" rule 1 by turning the catch-all into an error path. Each client now asserts an unrecognised type is neither surfaced to consumers nor fatal to the turn.

Rule 2 is verified negatively: making Go's dispatch loop failAll on unknown frames makes TestUnknownEventIsIgnoredNotFatal fail.

Tests

The interaction fixtures (interaction_required_event, interaction_invalid_event, submit_interaction_request, choices_values) now go through each client's real frame dispatcher. The existing conformance tests validate those same fixtures against the schemas but never feed them to the dispatcher, which is precisely the blind spot that hid this bug. Outgoing submit_interaction frames are validated back against the spec schema, so the tests prove the client emits a frame the protocol actually allows.

stream_preamble / stream_reasoning have no conformance fixture, so those frames are built in-test and validated against their own schemas before dispatch — a frame the spec would reject proves nothing.

Each test was confirmed to fail without the fix, by un-wiring each dispatch map and re-running: Go 4 guard failures + dropped-frame assertions, Python 6 failed / 3 passed, .NET 5 failed / 3 passed, TypeScript 2 failed / 2 passed.

Deterministic, not timing-based

This box runs at load ~125 on 12 cores, so the dispatch assertions were rewritten to not race a clock. Go emits every frame plus the terminal, drains the turn's channel to close, and asserts on the collected slice — no timers in either the passing or the failing path, since the terminal always decodes and a dropped frame shows up as an absent entry rather than a hang. Python's and .NET's remaining waits are hang-detectors rather than assertions and moved from 2s/5s to 30s; observed wall time for the Python file alone already ranged 0.9s–3.7s under load.

Verified with go test -race -count=10 and 5 consecutive Python runs at load 125, all stable.

language result
Go go vet clean; 65 passed, -race -count=10 stable
Python 72 passed, 1 skipped (gated live e2e); ruff check clean
.NET 46 passed, 1 skipped (gated live e2e)
TypeScript 64 passed (10 files); tsc --noEmit clean

Left out (deliberately)

.NET has no turn timeout. SmoothAgentClientOptions exposes only RequestTimeout — there is no counterpart to TS's 120s, Go's DefaultTurnTimeout, or Python's turn_timeout=120.0. That is why a parked .NET turn hung forever rather than erroring. Not fixed here: it needs plumbing through the MessageTurn lifecycle and is a behavioural change independent of frame dispatch. Worth its own pearl.

No new conformance fixtures for stream_preamble / stream_reasoning — the spec-directory drift guard covers them without touching spec/conformance/fixtures.json, which other work is currently editing.

…nteraction (th-41e964)

The generated wire types for stream_preamble, stream_reasoning,
interaction_required and interaction_invalid exist in every language, but the
hand-maintained dispatch unions were never updated — so each client rejected
those frames in its own guard and then discarded them in its dispatch loop.
Go's ParseServerEvent returned UnknownEventError and the read loop continued,
Python's parse_event raised and _handle_frame swallowed it, and .NET's
ServerEventConverter threw a JsonException the client caught and dropped.

A session declaring a Rich Interaction parked a turn that Go/Python/.NET
consumers never saw: the turn hung to the turn timeout, and forever on .NET,
which has none. stream_preamble and stream_reasoning are emitted by the
production server today, so those tokens were being discarded by all four
clients — TypeScript included, which is why the shipped web-chat example has a
`case 'stream_reasoning'` that could never fire.

Go, Python and .NET also gain submit_interaction, mirroring the TypeScript
submitInteraction(): without it a client can decode the park but not answer it.
One verb serves every interaction kind, so adding a kind needs no new method.

TypeScript's and Python's validators keep a SECOND hand-maintained type→schema
map that had drifted the same way; both are corrected here.

Each language gains a drift guard that derives the expected discriminator set
from spec/events/*.schema.json and spec/actions/*.schema.json at test time
rather than from a list maintained beside the union — a guard asserting against
its own copy of the constant would lock the drift in instead of catching it.
The interaction fixtures now go through each client's real frame dispatcher,
which the existing schema-only conformance tests never did.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ac4b1a9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@smooai/smooth-operator Minor
@smooai/smooth-operator-web-chat-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@brentrager
brentrager merged commit 0f9e90f into main Aug 19, 2026
7 checks passed
@brentrager
brentrager deleted the fix/client-interaction-frames branch August 19, 2026 21:10
brentrager added a commit that referenced this pull request Aug 19, 2026
…-time tests (#508)

Test-only follow-up to #505 (shipped in 1.56.0). No changeset: nothing
publishable changes, only the tests around it.

The drift guard added in #505 has to encode BOTH tiers of the contract, or the
obvious way to satisfy it is to make unknown types an error — which would break
the other half. Per the stream_reasoning schema, a client that does not
recognize an event MUST ignore it, so a frame from a server newer than this
build has to be dropped quietly and leave the turn healthy. That behaviour was
already correct in all four clients and is untouched here; it was simply
untested, so nothing stopped a future "fix" from turning the catch-all into an
error path. Each client now asserts an unrecognised type is neither surfaced to
consumers nor fatal to the turn. Confirmed by making Go's dispatch loop fail the
turn on unknown frames: the new test catches it.

Go had no spec/actions guard, unlike the other three — and its ActionType
constants had the same drift that hid submit_interaction. Added, derived from
spec/actions/*.schema.json the same way.

The dispatch assertions were timing-based, which is a flake risk on a box
running at load 125. Go now emits every frame plus the terminal, drains the
turn's channel to close, and asserts on the collected slice — no timers at all,
in either the passing or the failing path, since the terminal always decodes and
a dropped frame shows up as an absent entry rather than a hang. Python's and
.NET's remaining waits are hang-detectors rather than assertions, so they move
from 2s/5s to 30s: observed wall time for the Python file already ranged 0.9s to
3.7s under load. Verified with go test -race -count=10 and 5 consecutive Python
runs at load 125.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
brentrager added a commit that referenced this pull request Aug 20, 2026
Mirrors the Rust reference in the preceding commit. Go, Python and .NET each
kept the declared render capabilities in a per-connection map on the dispatcher
(`FrameDispatcher.supports` / `_session_supports` / `_sessionSupports`), which a
reconnect wipes and which nothing ever pruned. TypeScript already routed the
value through its `SessionStore`, but stored it on the SESSION record — and a
resume mints a new session, so it started empty just the same.

Each port now persists it per CONVERSATION through the mechanism its own store
already used for conversation-scoped facts, rather than a fifth invented one:
Go adds `SetConversationSupports` beside `SetCurrentStep`, Python adds
`get/set_client_supports` beside the workflow-step pointer, TypeScript adds a
`convSupports` map beside `convOwner`/`convOrg`, .NET adds
`Get/SetClientSupportsAsync` beside `Get/SetWorkflowStepAsync`. Every
implementation of each store interface is updated, in-memory and Postgres.

The per-connection maps are DELETED rather than kept as caches, so there is one
source of truth and the leak goes with them.

Distinguishing an omitted `supports` from an explicit `[]` is load-bearing —
omitted inherits, `[]` replaces — and three of the four collapsed them. Go's
frame field becomes `*[]string` (json.Unmarshal gives nil for both), .NET's
`ParseSupports` returns `IReadOnlyList<string>?`, and the TS stores stopped
dropping an empty list on the floor.

Correction to the previous commit message: it claimed the four ports never parse
`supports` and host no interactions framework. That was true of the commit this
work branched from and is false on current main — #505/#509/#513/#520 landed the
framework in Go, TypeScript, Python and .NET in the meantime. The pearl's
original diagnosis was right; the rebase is what surfaced it.

Each port adds a reconnect test that drives a SECOND, FRESH dispatcher over the
same store — a single dispatcher would pass even with per-connection state — and
each was verified to fail against its own pre-fix code before being kept.

Verified: go build/vet/test + gofmt (go and go/server modules), ruff check +
format + pytest (389), tsc + vitest (390), dotnet build + test (642 across five
assemblies). Postgres-backed suites really ran; Docker was up.
brentrager added a commit that referenced this pull request Aug 21, 2026
* th-13df6d: keep Rich Interactions alive across a reconnect

`supports` — the client render-capability list that gates the entire Rich
Interactions framework — lived only on the session
(`Session.metadata.supports`, read by `AppState::session_capabilities`). A
reconnect IS a resume: the client re-opens the socket and re-issues
`create_conversation_session` with the same `conversationId`, which mints a NEW
session id. So unless the client re-declared `supports` every single time, the
server forgot it could render cards and every interaction kind quietly fell back
to conversational collection — no error, no event, nothing on the wire to
notice. Reconnects are routine (network blips, mobile backgrounding, deploys),
so a shipped feature was degrading in the field with no signal.

The session registry was already the wrong home, and this repo had said so once
before: th-c12df5 moved the workflow step pointer off it for exactly this reason
("this per-pod session map resets on reconnect/pod hop"). `supports` now rides
durable conversation metadata (`clientSupports`) the same way — same
read-modify-write shape as `persist_workflow_step`, same best-effort failure
mode — and a resume that omits the key inherits what the conversation last
declared.

A list the frame DOES declare always wins, including `[]`. That is now how a
text-only channel resuming a rich conversation opts out, so the spec's
`supports` description carries the rule and the generated TS/Go/Python/.NET
types are regenerated from it rather than restating it by hand. The inherit
direction is bounded anyway: a card a client cannot render times out
(`INTERACTION_TIMEOUT`) into the same conversational fallback the gate would
have chosen.

Verified across all five implementations first. The pearl's premise that the
four ports keep `supports` in a per-connection map does not hold — Go,
TypeScript, Python and .NET never parse `supports` at all and host no
interactions framework (`interaction_required` / `submit_interaction` /
`identity_form` appear only in their generated wire types), so there is nothing
there to persist yet. Rust is the only implementation with the behavior, so it
is the only one changed.

`reconnect_resuming_a_conversation_keeps_the_declared_capabilities` covers both
directions and fails on the pre-fix handler (verified by reverting).

* th-13df6d: move `supports` onto the conversation in all four ports

Mirrors the Rust reference in the preceding commit. Go, Python and .NET each
kept the declared render capabilities in a per-connection map on the dispatcher
(`FrameDispatcher.supports` / `_session_supports` / `_sessionSupports`), which a
reconnect wipes and which nothing ever pruned. TypeScript already routed the
value through its `SessionStore`, but stored it on the SESSION record — and a
resume mints a new session, so it started empty just the same.

Each port now persists it per CONVERSATION through the mechanism its own store
already used for conversation-scoped facts, rather than a fifth invented one:
Go adds `SetConversationSupports` beside `SetCurrentStep`, Python adds
`get/set_client_supports` beside the workflow-step pointer, TypeScript adds a
`convSupports` map beside `convOwner`/`convOrg`, .NET adds
`Get/SetClientSupportsAsync` beside `Get/SetWorkflowStepAsync`. Every
implementation of each store interface is updated, in-memory and Postgres.

The per-connection maps are DELETED rather than kept as caches, so there is one
source of truth and the leak goes with them.

Distinguishing an omitted `supports` from an explicit `[]` is load-bearing —
omitted inherits, `[]` replaces — and three of the four collapsed them. Go's
frame field becomes `*[]string` (json.Unmarshal gives nil for both), .NET's
`ParseSupports` returns `IReadOnlyList<string>?`, and the TS stores stopped
dropping an empty list on the floor.

Correction to the previous commit message: it claimed the four ports never parse
`supports` and host no interactions framework. That was true of the commit this
work branched from and is false on current main — #505/#509/#513/#520 landed the
framework in Go, TypeScript, Python and .NET in the meantime. The pearl's
original diagnosis was right; the rebase is what surfaced it.

Each port adds a reconnect test that drives a SECOND, FRESH dispatcher over the
same store — a single dispatcher would pass even with per-connection state — and
each was verified to fail against its own pre-fix code before being kept.

Verified: go build/vet/test + gofmt (go and go/server modules), ruff check +
format + pytest (389), tsc + vitest (390), dotnet build + test (642 across five
assemblies). Postgres-backed suites really ran; Docker was up.

* th-13df6d: don't float a reconnect to the top of the conversation sidebar

The TypeScript store's capability write also set conversations.updated_at.
That column is the sidebar's recency sort and is bumped when a MESSAGE lands
(appendMessage); a bare reconnect appends nothing, so this floated every
backgrounded tab to the top of the list on resume. The Go, Python and Rust
stores all leave it alone — a parity-relevant choice, now commented as one.

* th-13df6d: correct the .NET store's divergence note — the data is NOT in the shared place

The <remarks> claimed 'the DATA lives in the shared place under the shared keys
either way'. Only the key names are shared: this store writes
conversation_sessions.metadata, the other four write conversations.metadata_json
(rust/adapters/postgres/src/lib.rs:618). A note that asserts the divergence away
is worse than no note, since it is exactly what someone would rely on before
pointing both at one database.

States the real divergence, that nothing gates it (knownDivergences was retired
in #523, and a test pinning the current table would lock the drift in), and that
moving ONE key would split this store's three across two tables — strictly worse.
Unification tracked as th-52becd.
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