diff --git a/.changeset/interaction-required-before-chunk.md b/.changeset/interaction-required-before-chunk.md new file mode 100644 index 00000000..d7210ca6 --- /dev/null +++ b/.changeset/interaction-required-before-chunk.md @@ -0,0 +1,15 @@ +--- +'@smooai/smooth-operator': patch +--- + +go/typescript/python/dotnet-server: emit `interaction_required` **before** the raise tool's `toolCall` `stream_chunk`, matching the Rust reference. + +For a Rich Interaction, the Rust server emits the park event first; Go, TypeScript, Python and .NET all emitted the raw `toolCall` chunk first. A client that renders tool calls therefore showed "calling `request_identity_intake`…" before the card it was calling for ever appeared — framework internals leaking ahead of the semantic event. + +Ruled a port bug rather than a protocol variant, on the ports' own evidence: all five already defer the gated tool's chunk until after the prompt for the **other** park type (`hitl-write-confirmation`, which all five have always passed). The four were internally inconsistent between their two park paths while Rust was consistent across both. + +The fix reuses each port's existing write-confirmation mechanism rather than inventing a second one: suppress the chunk in the engine stream loop on a tool-name predicate, then re-emit it from the park path immediately after the park event. Go already deferred interaction raises but re-emitted at the top of the raise tool, ahead of the park — that emit moved. TypeScript, Python and .NET gained the predicate (`isInteractionRaise` / `_is_interaction_raise` / `IsInteractionRaise`) matched against the hosted kinds' `request_` names — deliberately **not** the generic `submit_interaction` tool, whose chunk has no park event to follow and would simply be dropped. Every non-park exit of the raise tool (parse error, conversational fallback) emits the chunk immediately, so `interaction-conversational-fallback` is unaffected. + +Verified against the shared conformance corpus, which pinned this order and marked the four as known divergences: `interaction-park-resume`, `interaction-declined`, `interaction-invalid-retryable` and `interaction-stale-id-rejected` now pass on all five servers, and their `knownDivergences` markers are removed. Rust is unchanged. + +`interaction-choices-park-resume` keeps a `["go"]` marker, re-pointed at a different bug that was only reachable once the ordering was fixed: `splitIntoChunks` in `smooth-operator-core` Go slices the mock reply by bytes, so the em-dash in that scenario's reply is split mid-rune and arrives as U+FFFD. It needs a one-line upstream fix (slice `[]rune`) and a core release. diff --git a/spec/conformance/scenarios/README.md b/spec/conformance/scenarios/README.md index 35db06f2..d6a3bf46 100644 --- a/spec/conformance/scenarios/README.md +++ b/spec/conformance/scenarios/README.md @@ -119,7 +119,8 @@ Implementation note per language, since `*testing.T` and panics do not catch ali Recorded here as facts, not as license to weaken the scenarios. **Do not "fix" a scenario to make a port pass.** -- **Park event vs. the raise tool's `stream_chunk` — Rust is 1 of 5, and Rust is right.** For a Rich Interaction, Rust emits `interaction_required` *before* the raise tool's `toolCall` chunk; Go, TypeScript, Python and .NET all emit the chunk first. **Ruled a port bug, not a protocol variant**, on three grounds: all five already defer the gated tool's chunk until after the prompt for the *other* park type — `hitl-write-confirmation`, a scenario **all five pass today** — so the four are internally inconsistent between their own two park paths while Rust is consistent; Rust is the designated reference and the ports mirror it; and semantically a client that renders tool calls would otherwise show "calling `request_identity_intake`…" before the card appears, leaking framework internals ahead of the semantic event. The four ports change, not these scenarios. +- **~~Park event vs. the raise tool's `stream_chunk` — Rust is 1 of 5~~ — FIXED (th-ef78d0).** For a Rich Interaction, Rust emitted `interaction_required` *before* the raise tool's `toolCall` chunk while Go, TypeScript, Python and .NET emitted the chunk first. **Ruled a port bug, not a protocol variant**, on three grounds: all five already defer the gated tool's chunk until after the prompt for the *other* park type — `hitl-write-confirmation`, a scenario all five passed throughout — so the four were internally inconsistent between their own two park paths while Rust was consistent; Rust is the designated reference; and semantically a client that renders tool calls would otherwise show "calling `request_identity_intake`…" before the card appears, leaking framework internals ahead of the semantic event. **The four ports changed, and the scenarios did not.** Each port already owned the mechanism — it is the same one its write-confirmation gate uses: suppress the chunk in the engine stream loop on a tool-name predicate, then re-emit it from the park path *after* the park event. Go needed only a move (its deferral existed but re-emitted at the top of the raise tool, ahead of the park); TypeScript, Python and .NET needed the predicate added (`isInteractionRaise` / `_is_interaction_raise` / `IsInteractionRaise`, matched against the hosted kinds' `request_` names — **not** the generic `submit_interaction` tool, whose chunk would then be dropped) plus the re-emit. Every non-park exit of the raise tool (parse error, conversational fallback) re-emits the chunk immediately, so `interaction-conversational-fallback` is unchanged. +- **A second bug hid behind the first, in Go only.** Fixing the order let `interaction-choices-park-resume` run past step 2 for the first time — and it still fails on Go, now for an unrelated reason: `splitIntoChunks` in `smooth-operator-core` Go (`go/core/llm_provider.go`) slices the mock reply by **bytes**, so the em-dash in "Pro it is — pulling that quote up." (36 bytes / 3 parts puts a boundary mid-rune) is split into invalid UTF-8 and each byte arrives as U+FFFD. `interaction-park-resume`'s reply is 33 bytes and its boundaries miss the rune, which is why only this one scenario trips. That scenario therefore keeps `"knownDivergences": ["go"]` with the reason rewritten to name the real bug; the fix is one line upstream (slice `[]rune`) plus a core release. This is the marker contract working as designed — a tracked bug with an expiry, re-pointed at the defect that is actually still there. - **~~A cancelled turn keeps running in Go and .NET~~ — FIXED (th-f2ac48, PR #514).** Recorded because it is what this scenario was built to catch, and because the fix is the corpus's first end-to-end proof of itself. Both ports used to leave the turn running after a `cancel`: the write-confirmation gate returned a deny instead of unwinding, the agent loop made one more model call, and the output was merely gagged (Go: `if turnCtx.Err() != nil { return }`). Cancellation was a mute button, not a stop button — real spend and real side-effect risk after a visitor hits Stop. Two independent proofs: re-running with one extra `mockLlmScript` entry made both pass (the entry was eaten by the cancelled turn), and `go test -race` reported a `DATA RACE` in core's `MockLlmProvider.ChatStream` where the cancelled turn's goroutine and the *next* turn's goroutine popped the same unguarded FIFO concurrently. That race was the one failure `knownDivergences` deliberately did **not** tolerate — it fires outside the runner's assertion path, and suppressing a data race is the opposite of what this corpus is for. The lesson if it recurs: do not "fix" it by guarding the mock's FIFO, which silences the evidence and leaves the bug. - **Ack payloads differ, so only `status` is asserted on a `submit_interaction` ack.** The five servers put different fields in `data` (Go omits `kind`/`values`; Python omits `kind`, and its decline ack omits `interactionId`/`declined`; .NET's decline ack omits `declined`). Asserting more would pin one language's shape rather than the protocol's. diff --git a/spec/conformance/scenarios/interaction-choices-park-resume.json b/spec/conformance/scenarios/interaction-choices-park-resume.json index 33972bb2..1982edb8 100644 --- a/spec/conformance/scenarios/interaction-choices-park-resume.json +++ b/spec/conformance/scenarios/interaction-choices-park-resume.json @@ -1,124 +1,44 @@ { - "name": "interaction-choices-park-resume", - "description": "The second Rich Interaction kind, `choices` (the AskUserQuestion-shaped multiple-choice ask), through the SAME generic envelope: `request_choices` parks behind the `choice_chips` capability, the server emits `interaction_required` with the normalized questions/options spec, and one `submit_interaction` resumes it. Adding a kind must require no new protocol action and no client release \u2014 this scenario is what proves that claim holds identically in every server, and that the kind catalog is not identity_intake-shaped by accident.", - "mockLlmScript": [ - { - "kind": "toolCall", - "name": "request_choices", - "arguments": "{\"questions\": [{\"question\": \"Which plan fits best?\", \"header\": \"Plan\", \"options\": [{\"label\": \"Starter\"}, {\"label\": \"Pro\"}]}], \"reason\": \"to route you to the right quote\"}" - }, - { - "kind": "text", - "text": "Pro it is \u2014 pulling that quote up." - } - ], - "steps": [ - { - "send": { - "action": "create_conversation_session", - "requestId": "r-create", - "agentId": "11111111-1111-1111-1111-111111111111", - "userName": "Alice", - "userEmail": "alice@example.com", - "supports": [ - "choice_chips" - ] - }, - "expect": [ - { - "type": "immediate_response", - "status": 200, - "capture": { - "sessionId": "data.sessionId" - } - } - ] - }, - { - "send": { - "action": "send_message", - "requestId": "r-msg", - "sessionId": "{{sessionId}}", - "message": "i want a quote", - "stream": true - }, - "expect": [ - { - "type": "immediate_response", - "status": 202 + "name": "interaction-choices-park-resume", + "description": "The second Rich Interaction kind, `choices` (the AskUserQuestion-shaped multiple-choice ask), through the SAME generic envelope: `request_choices` parks behind the `choice_chips` capability, the server emits `interaction_required` with the normalized questions/options spec, and one `submit_interaction` resumes it. Adding a kind must require no new protocol action and no client release — this scenario is what proves that claim holds identically in every server, and that the kind catalog is not identity_intake-shaped by accident.", + "knownDivergences": ["go"], + "knownDivergencesReason": "th-ef78d0 — the park ORDERING is fixed in all four ports; Go alone still fails this scenario, and on a DIFFERENT bug that only became reachable once the ordering was corrected: `splitIntoChunks` in smooth-operator-core Go (`go/core/llm_provider.go`) slices the mock reply by BYTES, so the em-dash in \"Pro it is — pulling that quote up.\" (36 bytes / 3 parts puts a boundary mid-rune) is split into invalid UTF-8 and each byte surfaces as U+FFFD. `interaction-park-resume`'s reply is 33 bytes and its boundaries miss the rune, which is why only this scenario trips. Fix is one line upstream (slice `[]rune`) plus a core release — not a protocol divergence, and NOT a reason to weaken this scenario.", + "mockLlmScript": [ + { "kind": "toolCall", "name": "request_choices", "arguments": "{\"questions\": [{\"question\": \"Which plan fits best?\", \"header\": \"Plan\", \"options\": [{\"label\": \"Starter\"}, {\"label\": \"Pro\"}]}], \"reason\": \"to route you to the right quote\"}" }, + { "kind": "text", "text": "Pro it is — pulling that quote up." } + ], + "steps": [ + { + "send": { "action": "create_conversation_session", "requestId": "r-create", "agentId": "11111111-1111-1111-1111-111111111111", "userName": "Alice", "userEmail": "alice@example.com", "supports": ["choice_chips"] }, + "expect": [ { "type": "immediate_response", "status": 200, "capture": { "sessionId": "data.sessionId" } } ] }, { - "type": "interaction_required", - "assert": { - "requestId": "r-msg", - "data.data.kind": "choices", - "data.data.reason": "to route you to the right quote", - "data.data.spec.questions.0.header": "Plan", - "data.data.spec.questions.0.question": "Which plan fits best?", - "data.data.spec.questions.0.options.1.label": "Pro" - }, - "capture": { - "interactionId": "data.data.interactionId" - } - }, - { - "type": "stream_chunk", - "assert": { - "data.state.rawResponse.toolCall.name": "request_choices" - } - } - ] - }, - { - "send": { - "action": "submit_interaction", - "requestId": "r-msg", - "sessionId": "{{sessionId}}", - "interactionId": "{{interactionId}}", - "kind": "choices", - "values": { - "answers": [ - { - "header": "Plan", - "options": [ - "Pro" - ] - } - ] - } - }, - "expect": [ - { - "type": "immediate_response", - "status": 200 - }, - { - "type": "stream_chunk", - "assert": { - "data.state.rawResponse.toolResult.name": "request_choices", - "data.state.rawResponse.toolResult.isError": false - } - }, - { - "type": "stream_token", - "repeat": true, - "accumulate": "token", - "assertAccumulated": "Pro it is \u2014 pulling that quote up." + "send": { "action": "send_message", "requestId": "r-msg", "sessionId": "{{sessionId}}", "message": "i want a quote", "stream": true }, + "expect": [ + { "type": "immediate_response", "status": 202 }, + { + "type": "interaction_required", + "assert": { + "requestId": "r-msg", + "data.data.kind": "choices", + "data.data.reason": "to route you to the right quote", + "data.data.spec.questions.0.header": "Plan", + "data.data.spec.questions.0.question": "Which plan fits best?", + "data.data.spec.questions.0.options.1.label": "Pro" + }, + "capture": { "interactionId": "data.data.interactionId" } + }, + { "type": "stream_chunk", "assert": { "data.state.rawResponse.toolCall.name": "request_choices" } } + ] }, { - "type": "eventual_response", - "status": 200, - "assert": { - "data.data.response.responseParts": [ - "Pro it is \u2014 pulling that quote up." + "send": { "action": "submit_interaction", "requestId": "r-msg", "sessionId": "{{sessionId}}", "interactionId": "{{interactionId}}", "kind": "choices", "values": { "answers": [{ "header": "Plan", "options": ["Pro"] }] } }, + "expect": [ + { "type": "immediate_response", "status": 200 }, + { "type": "stream_chunk", "assert": { "data.state.rawResponse.toolResult.name": "request_choices", "data.state.rawResponse.toolResult.isError": false } }, + { "type": "stream_token", "repeat": true, "accumulate": "token", "assertAccumulated": "Pro it is — pulling that quote up." }, + { "type": "eventual_response", "status": 200, "assert": { "data.data.response.responseParts": ["Pro it is — pulling that quote up."] } } ] - } } - ] - } - ], - "knownDivergences": [ - "go" - ], - "knownDivergencesReason": "th-CORE-runeslice \u2014 NOT an ordering divergence. Go's interaction_required ordering is FIXED (th-ef78d0); fixing it let this scenario run far enough to expose a second, previously-masked bug in smooth-operator-core: splitIntoChunks (go/core/llm_provider.go) slices the reply by BYTES, not runes. This scenario's reply contains an em-dash, and 36 bytes / 3 parts puts a chunk boundary mid-rune, so the pieces are invalid UTF-8 and each stray byte surfaces as U+FFFD. interaction-park-resume's reply is 33 bytes and its boundaries miss the rune, which is why only this scenario trips. Fix is one line upstream (slice []rune) in smooth-operator-core plus a Go module release. Do NOT weaken this scenario to make it pass." + ] } diff --git a/spec/conformance/scenarios/interaction-park-resume.json b/spec/conformance/scenarios/interaction-park-resume.json index 50f24c84..0c494077 100644 --- a/spec/conformance/scenarios/interaction-park-resume.json +++ b/spec/conformance/scenarios/interaction-park-resume.json @@ -1,6 +1,6 @@ { "name": "interaction-park-resume", - "description": "Rich Interactions, the happy path. A session that DECLARED the `identity_form` render capability in `supports` gets the real card: the agent's server-registered `request_identity_intake` raise tool PARKS the turn and the server emits `interaction_required` (kind `identity_intake`, a server-minted `interactionId`, the canonical `spec.fields`, and the agent's `reason`). A matching `submit_interaction` — same requestId, same interactionId — acks 200 and RESUMES the parked turn, which streams the wrap-up reply. The raise tools are server-registered in every server, so this needs no `server.*` directive; only the mock names the tool. ⚠️ The park event is asserted BEFORE the raise tool's `stream_chunk`, matching the Rust reference and the same ordering the corpus already pins for write-confirmation HITL (`hitl-write-confirmation`). Go / TypeScript / Python / .NET emit the chunk first; see the README's divergence note.", + "description": "Rich Interactions, the happy path. A session that DECLARED the `identity_form` render capability in `supports` gets the real card: the agent's server-registered `request_identity_intake` raise tool PARKS the turn and the server emits `interaction_required` (kind `identity_intake`, a server-minted `interactionId`, the canonical `spec.fields`, and the agent's `reason`). A matching `submit_interaction` — same requestId, same interactionId — acks 200 and RESUMES the parked turn, which streams the wrap-up reply. The raise tools are server-registered in every server, so this needs no `server.*` directive; only the mock names the tool. ⚠️ The park event is asserted BEFORE the raise tool's `stream_chunk`, matching the Rust reference and the same ordering the corpus already pins for write-confirmation HITL (`hitl-write-confirmation`). All five servers now produce this order (th-ef78d0 fixed the four ports); see the README's divergence note.", "mockLlmScript": [ { "kind": "toolCall", "name": "request_identity_intake", "arguments": "{\"fields\": [{\"key\": \"name\", \"required\": false}, {\"key\": \"email\", \"required\": true}], \"reason\": \"to send you the quote\"}" }, { "kind": "text", "text": "Thanks — I've got your details." }