th-ef78d0: emit interaction_required before the raise tool's chunk (Go, Python, TS) - #520
Merged
Merged
Conversation
…unk (Go, Python, TS)
Preserved by the team lead at 3.2Gi free disk so the work survives a possible
ENOSPC. Go, Python and TypeScript are changed; .NET is NOT started. Markers in
spec/conformance/scenarios/ are untouched — CI will report which ports now pass
via the xpass check ("remove <lang> from knownDivergences — it now passes").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
CI confirmed Go, Python and TypeScript now pass all five interaction scenarios — each xpass'd with 'remove <lang> from knownDivergences — it now passes', which is the marker mechanism working as designed. .NET is now changed too, so all markers are dropped and CI adjudicates whether that claim holds: a real assertion failure (not an xpass) means .NET is not done and its marker goes back. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…FERENT bug I stripped this marker with a bulk script that only preserved 'dotnet'; that was wrong. The authoring agent had deliberately kept it and rewritten its reason: Go's ORDERING is fixed, and fixing it unmasked a byte-vs-rune slicing bug in smooth-operator-core's splitIntoChunks. Restoring the marker with that reason so the scenario keeps failing honestly against the real cause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
brentrager
marked this pull request as ready for review
August 20, 2026 00:30
brentrager
added a commit
that referenced
this pull request
Aug 20, 2026
) PR #520 landed the ordering fix itself but carried no changeset, so the fix would version and publish nowhere. Adds it, and cleans up two things that outlived the fix: - spec/conformance/scenarios/README.md still documented the divergence as live ("Rust is 1 of 5, and Rust is right") and interaction-park-resume's description still told readers the four ports emit the chunk first. Both now record it as FIXED, with the mechanism each port reused, and name the second bug the fix uncovered. - interaction-choices-park-resume.json had been rewritten by a JSON re-serializer (2-space indent, \u escapes), diverging from the corpus's formatting for a two-line change. Restored, marker and reason intact.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft — opened by the team lead to get CI running while disk is critically low (3.2Gi). The authoring agent is still deciding what is done vs mid-flight.
The divergence
Rust emits
interaction_requiredbefore the raise tool'stoolCallstream_chunk. Go, TypeScript, Python and .NET all emit the chunk first —expected interaction_required, got stream_chunk.Ruled in Rust's favour (recorded in
spec/conformance/scenarios/README.md), on three grounds:hitl-write-confirmationpath — all five pass that scenario — then do the opposite for interactions. Two park types, two orders, same server.toolCallchunk, or a client that renders tool calls shows "calling request_identity_intake…" before the card appears.State of this branch
Changed: Go, Python, TypeScript. .NET is NOT started.
knownDivergencesmarkers inspec/conformance/scenarios/are deliberately untouched, which makes CI the verifier: the corpus fails the build withremove <lang> from knownDivergences in <scenario> — it now passesfor each port that now behaves. Those failures are the success signal, and the fix is to drop that language from the marker list.Do not merge until the markers are reconciled against what CI reports.
Why a draft rather than local verification
The machine is at ~3Gi free with the shared cargo target at 70G and cleanup blocked, so local parity suites were not run. CI has its own disk and is the cheaper verifier here.
Relates to th-ef78d0. Corpus landed in #513.