th-13df6d: keep Rich Interactions alive across a reconnect

.changeset/reconnect-keeps-supports.md

@@ -0,0 +1,48 @@
+---
+"@smooai/smooth-operator": patch
+---
+
+fix(all five): a reconnect no longer silently turns Rich Interactions off
+
+`supports` — the client render-capability list that gates the **entire** Rich
+Interactions framework — was kept somewhere that a reconnect destroys, in every
+implementation.
+
+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** on a **new dispatcher**. So unless the client re-declared
+`supports` on every single reconnect, the server forgot the client could render
+cards at all and every interaction kind quietly fell back to conversational
+collection — no error, no event, nothing on the wire to notice. The parked-card
+flow (raise tool → `interaction_required` → `submit_interaction` → resume)
+simply stopped happening. Reconnects are routine (network blips, mobile
+backgrounding, deploys), so a shipped feature was degrading in the field with no
+signal.
+
+- **Rust** kept it in `Session.metadata.supports` — the per-pod session registry.
+- **Go** (`FrameDispatcher.supports`), **Python** (`_session_supports`) and
+  **.NET** (`_sessionSupports`) kept a per-connection map, also never pruned.
+- **TypeScript** already stored it through the `SessionStore`, but on the
+  **session** record, so a resumed session started empty just the same.
+
+The session was already the wrong home, and the repo had said so once: 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 lives on the
+**conversation** in all five, mirroring whatever conversation-scoped mechanism
+each store already had — Rust/Go/Python/TypeScript write `clientSupports` into
+`conversations.metadata_json`; .NET follows its own store's documented hold for
+`currentStepId`/`otpVerified` (session-row metadata) under the same key name.
+
+A list the frame **does** declare always wins, including `[]` — which is now how
+a text-only channel resuming a rich conversation opts out, and the opt-out is
+durable so the next reconnect that omits the key cannot resurrect the old
+capabilities. Because `[]` and an absent key now mean different things, each port
+had to stop collapsing them (`*[]string` in Go, `IReadOnlyList<string>?` in .NET,
+`undefined` vs `[]` across the TS store interface).
+
+The rule lives in the `supports` description in
+`spec/actions/create-conversation-session.schema.json` — the source of truth —
+and the TS/Go/Python/.NET wire types are regenerated from it rather than
+restating it by hand. Each implementation adds a reconnect test that drives a
+**second, fresh dispatcher over the same store**, and each was verified to fail
+against its own pre-fix code.

docs/Architecture/Rich Interactions.md

@@ -103,10 +103,41 @@ Intake = **collect** (who are you?), OTP = **verify** (prove it). Shared machine
 | Channel | `supports` | identity_intake behavior |
 | --- | --- | --- |
 | Chat widget (web) | `["identity_form"]` (registry-derived) | parked inline form card; server-validated; decline button |
-| SMS | — | conversational fallback: field-by-field ask, `submit_interaction` tool validates each value |
-| Voice | — | same fallback (spoken turn-by-turn) |
+| SMS | `[]` | conversational fallback: field-by-field ask, `submit_interaction` tool validates each value |
+| Voice | `[]` | same fallback (spoken turn-by-turn) |
 | Future rich client | declares the kinds its cards cover | rich per declared kind, fallback for the rest |
 
+### Capabilities survive a reconnect
+
+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 the declared list cannot live on the session — it is persisted on
+the **conversation** (`metadata.clientSupports`, written by
+`handle_create_session`), the same durability the workflow step pointer moved to
+for the same reason (th-c12df5: the session registry is per-pod and resets on
+reconnect/pod hop). A resume that **omits** `supports` inherits the conversation's
+last declared set, so Rich Interactions keep working across a network blip, a
+backgrounded mobile app, or a deploy.
+
+A frame that **does** declare wins, in both directions — which is why a text-only
+channel sends an explicit `[]` rather than omitting the key: omitting it on a
+resume would inherit the rich set. Both directions are pinned by
+`reconnect_resuming_a_conversation_keeps_the_declared_capabilities`
+(`rust/smooth-operator-server/tests/interactions.rs`).
+
+All five implementations hold to this. Before the fix each kept the set somewhere
+a reconnect destroys: Rust in `Session.metadata.supports` (the per-pod session
+registry), Go / Python / .NET in a per-connection map on the dispatcher (also
+never pruned), TypeScript on the store-backed **session** record — which a resume
+mints fresh. Every one of them silently degraded each kind to its conversational
+fallback after a reconnect: no error, no event, nothing to notice (th-13df6d).
+
+Rust, Go, Python and TypeScript store it as `clientSupports` on
+`conversations.metadata_json`; the .NET store follows its own documented hold for
+`currentStepId` / `otpVerified` (session-row metadata) under the same key name, so
+a database driven by both Rust and .NET would not share this one value — the same
+pre-existing divergence those two keys already have.
+
 ## What changes where
 
 | Layer | Change |

dotnet/server/integration-tests/SubmitInteractionTests.cs

@@ -39,16 +39,23 @@ public class SubmitInteractionTests
         }
         """;
 
-    private static WebApplication BuildApp()
+    /// <summary><paramref name="turns"/> scripted send_message turns, each "call request_choices, then
+    /// answer": a rich turn consumes the tool call when it parks and the text when the submit resumes
+    /// it; a fallback turn consumes both back-to-back. Either way one turn = one pair, so a test that
+    /// settles each turn before starting the next stays in step with the queue.</summary>
+    private static WebApplication BuildApp(int turns = 1)
     {
         var chat = new MockChatClient();
         var args = JsonSerializer.Deserialize<JsonElement>(ChoicesArgsJson);
-        chat.PushToolCall("call-1", "request_choices", new Dictionary<string, object?>
+        for (var i = 0; i < turns; i++)
         {
-            ["questions"] = args.GetProperty("questions"),
-            ["reason"] = args.GetProperty("reason"),
-        });
-        chat.PushText("Great — routing you to the right team now.");
+            chat.PushToolCall($"call-{i + 1}", "request_choices", new Dictionary<string, object?>
+            {
+                ["questions"] = args.GetProperty("questions"),
+                ["reason"] = args.GetProperty("reason"),
+            });
+            chat.PushText("Great — routing you to the right team now.");
+        }
 
         var builder = WebApplication.CreateBuilder();
         builder.WebHost.UseTestServer();
@@ -92,7 +99,14 @@ private static async Task<JsonObject> NextEventAsync(WebSocket socket)
         }
     }
 
-    private static async Task<string> CreateSessionAsync(WebSocket socket, string[]? supports = null)
+    private static async Task<string> CreateSessionAsync(WebSocket socket, string[]? supports = null) =>
+        (await CreateSessionDataAsync(socket, supports: supports))["sessionId"]!.GetValue<string>();
+
+    /// <summary>The create_conversation_session response's <c>data</c> (sessionId + conversationId).
+    /// A null <paramref name="supports"/> OMITS the key entirely; an empty array declares it explicitly
+    /// — the two are different frames and, on a resume, mean different things (inherit vs. opt out).
+    /// A non-null <paramref name="conversationId"/> makes this a resume, i.e. a reconnect.</summary>
+    private static async Task<JsonObject> CreateSessionDataAsync(WebSocket socket, string? conversationId = null, string[]? supports = null)
     {
         var frame = new JsonObject
         {
@@ -102,6 +116,10 @@ private static async Task<string> CreateSessionAsync(WebSocket socket, string[]?
             ["userName"] = "Alice",
             ["userEmail"] = "alice@example.com",
         };
+        if (conversationId is not null)
+        {
+            frame["conversationId"] = conversationId;
+        }
         if (supports is not null)
         {
             frame["supports"] = new JsonArray(supports.Select(s => (JsonNode)s).ToArray());
@@ -112,7 +130,7 @@ private static async Task<string> CreateSessionAsync(WebSocket socket, string[]?
             var ev = await NextEventAsync(socket);
             if (ev["type"]!.GetValue<string>() == "immediate_response")
             {
-                return ev["data"]!["sessionId"]!.GetValue<string>();
+                return ev["data"]!.AsObject();
             }
         }
     }
@@ -444,4 +462,114 @@ public async Task SubmitWithoutPending_IsACleanError()
         await socket.CloseAsync(WebSocketCloseStatus.NormalClosure, "done", CancellationToken.None);
         await app.StopAsync();
     }
+
+    /// <summary>Send one message and settle the raise: the <c>interaction_required</c> event when the
+    /// kind took the RICH (card) path, or <c>null</c> when the turn ran straight to
+    /// <c>eventual_response</c> on the conversational fallback. Returns the tool results seen either
+    /// way, so a fallback can be asserted on its directive rather than merely on the absence of a card.</summary>
+    private static async Task<(JsonObject? Card, List<JsonObject> ToolResults)> RunRaiseTurnAsync(WebSocket socket, string sessionId, string requestId)
+    {
+        await SendAsync(socket, new JsonObject
+        {
+            ["action"] = "send_message",
+            ["requestId"] = requestId,
+            ["sessionId"] = sessionId,
+            ["message"] = "help me choose",
+        });
+
+        var toolResults = new List<JsonObject>();
+        while (true)
+        {
+            var ev = await NextEventAsync(socket);
+            var type = ev["type"]!.GetValue<string>();
+            if (type == "stream_chunk" && ev["data"]?["state"]?["rawResponse"]?["toolResult"]?.AsObject() is { } tr)
+            {
+                toolResults.Add(tr);
+            }
+            else if (type == "interaction_required")
+            {
+                return (ev, toolResults);
+            }
+            else if (type == "eventual_response")
+            {
+                return (null, toolResults);
+            }
+        }
+    }
+
+    /// <summary>
+    /// A RECONNECT is a resume: the client re-opens the socket and re-issues
+    /// <c>create_conversation_session</c> with the same <c>conversationId</c>, which mints a NEW session
+    /// on a NEW <see cref="FrameDispatcher"/>. While the declared capabilities lived in that
+    /// dispatcher's per-connection map, every routine reconnect (network blip, mobile backgrounding,
+    /// deploy) silently turned the whole Rich Interactions framework off unless the client re-declared
+    /// <c>supports</c> every single time — no error, no event, nothing on the wire to notice. Each
+    /// connection below is a genuinely fresh dispatcher over the SAME singleton store. th-13df6d.
+    /// </summary>
+    [Fact]
+    public async Task Reconnect_ResumingAConversation_KeepsTheDeclaredCapabilities()
+    {
+        await using var app = BuildApp(turns: 3);
+        await app.StartAsync();
+        var server = app.GetTestServer();
+
+        // 1. First connection declares the capability, then drops (the blip).
+        string conversationId;
+        using (var first = await ConnectAsync(server))
+        {
+            var created = await CreateSessionDataAsync(first, supports: new[] { "choice_chips" });
+            conversationId = created["conversationId"]!.GetValue<string>();
+            await first.CloseAsync(WebSocketCloseStatus.NormalClosure, "blip", CancellationToken.None);
+        }
+
+        // 2. The reconnect: same conversation, `supports` OMITTED — exactly what a widget resuming from
+        //    its stored conversationId sends. This is where the feature used to go dark.
+        using (var reconnect = await ConnectAsync(server))
+        {
+            var data = await CreateSessionDataAsync(reconnect, conversationId: conversationId);
+            Assert.Equal(conversationId, data["conversationId"]!.GetValue<string>());
+            var sessionId = data["sessionId"]!.GetValue<string>();
+
+            var (card, _) = await RunRaiseTurnAsync(reconnect, sessionId, "r-reconnect");
+            Assert.True(card is not null, "a reconnect that omits 'supports' must inherit the conversation's declared capabilities");
+            Assert.Equal("choices", card!["data"]!["data"]!["kind"]!.GetValue<string>());
+
+            // Unpark so this turn is fully settled before the next one starts.
+            await SendAsync(reconnect, new JsonObject
+            {
+                ["action"] = "submit_interaction",
+                ["requestId"] = "r-reconnect",
+                ["sessionId"] = sessionId,
+                ["interactionId"] = card["data"]!["data"]!["interactionId"]!.GetValue<string>(),
+                ["values"] = JsonNode.Parse("""{ "answers": [ { "header": "Plan", "options": ["Pro"] }, { "header": "Topics", "options": ["Sales"] } ] }"""),
+            });
+            await ReadUntilAsync(reconnect, "eventual_response");
+            await reconnect.CloseAsync(WebSocketCloseStatus.NormalClosure, "done", CancellationToken.None);
+        }
+
+        // 3. A resume that DECLARES wins, including an explicit `[]` — how a text-only channel (SMS,
+        //    voice) resuming a rich conversation opts out instead of being handed cards it can't render.
+        using (var textOnly = await ConnectAsync(server))
+        {
+            var data = await CreateSessionDataAsync(textOnly, conversationId: conversationId, supports: Array.Empty<string>());
+            var (card, toolResults) = await RunRaiseTurnAsync(textOnly, data["sessionId"]!.GetValue<string>(), "r-text-only");
+            Assert.True(card is null, "an explicit empty 'supports' declares text-only and must never inherit");
+            Assert.Contains(toolResults, tr =>
+                tr["name"]!.GetValue<string>() == "request_choices"
+                && tr["result"]!.GetValue<string>().Contains("cannot display choice chips", StringComparison.Ordinal));
+            await textOnly.CloseAsync(WebSocketCloseStatus.NormalClosure, "done", CancellationToken.None);
+        }
+
+        // 4. ...and that opt-out is itself durable: the next reconnect that omits the key must inherit
+        //    the OPT-OUT, not resurrect the capability from a stale record.
+        using (var after = await ConnectAsync(server))
+        {
+            var data = await CreateSessionDataAsync(after, conversationId: conversationId);
+            var (card, _) = await RunRaiseTurnAsync(after, data["sessionId"]!.GetValue<string>(), "r-after-opt-out");
+            Assert.True(card is null, "the text-only declaration replaced the stored capabilities");
+            await after.CloseAsync(WebSocketCloseStatus.NormalClosure, "done", CancellationToken.None);
+        }
+
+        await app.StopAsync();
+    }
 }

dotnet/server/postgres/src/PostgresSessionStore.cs

@@ -16,9 +16,20 @@ namespace SmooAI.SmoothOperator.Server.Postgres;
 /// The interface stays CONVERSATION-keyed (<c>GetWorkflowStepAsync(conversationId)</c> and friends)
 /// while Rust/Go key the same metadata by session. That is a deliberate hold: this host's persisted
 /// workflow step and OTP bit survive a resume today, and flipping to session-keyed would silently
-/// require re-verification after every resume — a product decision, not a schema one. The DATA lives
-/// in the shared place under the shared keys either way; a conversation-keyed write touches every
-/// session row of the conversation and a read takes the most recent.
+/// require re-verification after every resume — a product decision, not a schema one. A
+/// conversation-keyed write touches every session row of the conversation and a read takes the most
+/// recent.
+///
+/// KNOWN DIVERGENCE — the data does NOT live in the same place as the other ports, only under the
+/// same key names. This store writes <c>conversation_sessions.metadata</c>; the Rust/Go/Python/
+/// TypeScript stores write <c>conversations.metadata_json</c> (Rust:
+/// <c>rust/adapters/postgres/src/lib.rs:618</c>). So one database driven by BOTH a .NET server and
+/// one of the others would not share <c>currentStepId</c>, <c>otpVerified</c>, or
+/// <c>clientSupports</c> — each would read its own table and see the other's writes as absent.
+/// Nothing enforces this at build time and no test pins it: you deploy ONE server implementation,
+/// so the mixed-driver case is theoretical. Do not "fix" it by moving one key — that would split
+/// this store's three keys across two tables, which is strictly worse. Unifying all three at once
+/// is tracked as its own piece of work (th-13df6d follow-up).
 /// </remarks>
 public sealed class PostgresSessionStore : ISessionStore, IAsyncDisposable
 {
@@ -335,6 +346,28 @@ ORDER BY sub.seq ASC
     public Task SetWorkflowStepAsync(string conversationId, string stepId, CancellationToken cancellationToken = default) =>
         MergeSessionMetadataAsync(conversationId, SessionMetadata.CurrentStepIdKey, stepId, cancellationToken);
 
+    public async Task<IReadOnlyList<string>> GetClientSupportsAsync(string conversationId, CancellationToken cancellationToken = default)
+    {
+        // `->>` renders the stored JSON array as its text (`["choice_chips"]`), so parse it back.
+        // Missing / unparseable ⇒ empty, i.e. the text-only behavior every kind already falls back to.
+        var value = await ReadSessionMetadataAsync(conversationId, SessionMetadata.ClientSupportsKey, cancellationToken).ConfigureAwait(false);
+        if (string.IsNullOrEmpty(value))
+        {
+            return Array.Empty<string>();
+        }
+        try
+        {
+            return JsonSerializer.Deserialize<string[]>(value) ?? Array.Empty<string>();
+        }
+        catch (JsonException)
+        {
+            return Array.Empty<string>();
+        }
+    }
+
+    public Task SetClientSupportsAsync(string conversationId, IReadOnlyList<string> supports, CancellationToken cancellationToken = default) =>
+        MergeSessionMetadataAsync(conversationId, SessionMetadata.ClientSupportsKey, supports.ToArray(), cancellationToken);
+
     public async Task<bool> GetSessionAuthenticatedAsync(string conversationId, CancellationToken cancellationToken = default)
     {
         var value = await ReadSessionMetadataAsync(conversationId, SessionMetadata.OtpVerifiedKey, cancellationToken).ConfigureAwait(false);
@@ -402,6 +435,11 @@ internal sealed class SessionMetadata
     internal const string OtpVerifiedKey = "otpVerified";
     internal const string CurrentStepIdKey = "currentStepId";
 
+    /// <summary>The conversation's last-declared render capabilities (a JSON array of strings). Named
+    /// to match the Rust reference's conversation-metadata key so a database driven by either server
+    /// reads the same record. th-13df6d.</summary>
+    internal const string ClientSupportsKey = "clientSupports";
+
     [System.Text.Json.Serialization.JsonPropertyName(ContactEmailKey)]
     [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull)]
     public string? ContactEmail { get; set; }