Preserve chat state - #2836
Conversation
… reset The chat sample kept its entire session in memory, so the transcript was lost whenever Android tore the app down. Configuration changes destroyed it immediately, and backgrounding the app lost it as soon as the process was reclaimed. The server cannot fill the gap: agent-server exposes no history RPC, so the client has to own its transcript. Hoist the session into a ViewModel so rotation, theme, font-scale and locale changes no longer tear down the socket, and mirror the transcript to SharedPreferences via ChatSessionStore so it also survives process death. The joined conversationId is stored alongside the messages and the restored transcript is dropped if the server hands back a different conversation, rather than showing history the agent has no memory of. Restored messages are always sealed as final, otherwise they render as "Responding..." forever and can be retargeted by later streaming updates. Writes are debounced, because the message list re-emits on every streamed chunk while SharedPreferences rewrites its whole file per commit. Because viewModelScope is cancelled before onCleared runs, the debounced writer is already dead by teardown, so onCleared also flushes synchronously - otherwise the last few hundred milliseconds are lost, as are the bubbles that disconnect() seals during teardown, which no writer could ever observe. Bound the stored data on both axes. Size is capped at the newest 200 messages, and messages older than 30 days are dropped on save and on load so they expire even while the app is not running. A load that drops anything rewrites the file immediately, so expired messages are erased rather than merely hidden. Exclude the transcript from Auto Backup, which previously copied it to the user's Google account by way of the untouched template rules, and add a confirmed "New chat" action that clears it from both the screen and disk. Voice input is now auto-sent on a final recognition result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
George Ng (GeorgeNgMsft)
left a comment
There was a problem hiding this comment.
Just to restate the current layering that I see :
ChatViewModel coordinates lifecycle + UX, which should look similar to the CLI/Shell reference flows
WebSocketManager is the Android agent-server RPC client
It would be worth matching the persistence semantics in AgentServer's existing canvases now (the additional functionality can be built out later on)
I think this should include :
- "New conversation/chat" flow alignment with existing flows
- "Conversation" naming alignment for better semantic representation
- Updating the conversation resume behavior (for efficiency)
Overall, I think this is great! The conversation/session naming gets a bit confusing so I just wanted to help draw the distinctions early.
Rework the mobile-2 persistence change around AgentServer's conversation semantics, per PR review feedback. Resume the saved conversation directly. The joined conversation id is now passed back into `joinConversation` as a connect option, so the client rejoins the exact conversation the restored transcript belongs to instead of joining the default one and reconciling afterwards. If the server answers "Conversation not found" the join retries once against the default and the orphaned transcript is dropped from screen and disk; every other error still surfaces as a connection error, so a transport or auth failure cannot silently move the user into a different conversation. This removes the whole `reconcileRestoredTranscript` round trip. Use conversation terminology. AgentServer reserves "session" for dispatcher runtime state - configuration, caches, agent state - while a conversation is the user-facing identity and chat history. `ChatSessionStore` becomes `ConversationStore`, `PersistedChatSession` becomes `PersistedConversation`, `ChatSessionSerializer` becomes `ConversationSerializer`, and `joinedConversationId` becomes `lastJoinedConversationId` to reflect that it outlives a disconnect. The `SharedPreferences` file name is deliberately left alone: it is pinned by name in `backup_rules.xml` and `data_extraction_rules.xml`, and it names the file already on devices, so renaming it would orphan stored transcripts for no benefit. The reason is now documented on the constant. Fix the meaning of "New chat". The action only ever cleared client-side history, so it is renamed to `clearChatHistory` and relabelled "Clear chat", matching `@clear` on the other canvases. The confirmation dialog no longer implies the conversation itself is affected. A true new-conversation flow (`createConversation` -> join -> persist -> `leaveConversation`) is left for the follow-up that adds `getDisplayHistory` backfill. Tests: 3 new cases pin the fallback predicate, including that a wrapped "Error: Conversation not found" does not trigger it. Full suite: 87 passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Integrates upstream microsoft#2845 (client-hosted Android agent) with the conversation-persistence work in this PR. Resolution notes: - WebSocketManager.connect() now takes both schemaContent (upstream) and resumeConversationId (this PR); the connect-time synchronized block seeds requestedConversationId, agentSchemaContent and resets isClientAgentRegistered. The "Conversation not found" fallback rejoin reuses the same onResult path, so registerClientAgent still runs after falling back to the default conversation. - MainActivity collects clientActionEvents in a plain lifecycleScope launch rather than repeatOnLifecycle(RESUMED). Upstream's executeAction holds a server RPC open until the completion callback fires, so gating on RESUMED would hang that RPC while backgrounded. launchExternalIntent already does its own RESUMED check and fails fast, matching upstream's behavior; the unbounded channel still buffers across configuration changes. - ClientAction.Alarm/.Timer carry upstream's completion callback. dispatchClientAction fails the completion when the channel is closed so the RPC can never leak. - Upstream's onDestroy teardown is intentionally not carried over: the socket is owned by the ViewModel and tearing it down in onDestroy would disconnect on every rotation. - README keeps both the renamed "Client-hosted Android agent" section and the rewritten "Conversation persistence" section.
lifecycleScope dispatches with Dispatchers.Main.immediate, so the clientActions collector starts running inline inside onCreate. An action buffered across a configuration change was therefore picked up while the new Activity was still CREATED, where launchExternalIntent's foreground guard refused it and told the agent the app was backgrounded - which was false, the app was in the foreground being recreated. Rotating the device with an alarm or timer in flight failed every time. Give the Activity a bounded grace period to reach RESUMED before dispatching. A genuinely backgrounded app still fails fast once the timeout elapses, so the agent's executeAction RPC is released promptly. Also answer the completion if the collector is cancelled while holding an action: it has already been taken off the channel, so no other Activity would ever see it and the RPC would hang.
Two windows where the persisted conversation id could go wrong: clearChatHistory removed the whole stored record, id included, and relied on the debounced writer to put the id back up to 400ms later. A force-stop in that window left nothing to resume, so the next launch silently landed in the default conversation - contradicting the documented promise that clearing is client-side only and the same conversation is resumed. It now writes an empty transcript that keeps the id. The not-found fallback cleared savedConversationId but left lastJoinedConversationId holding the deleted id until the fallback join landed. A debounced save or teardown flush in that window wrote the dead id back to disk, and a reconnect would try to resume it. The id is now dropped before the stale handler runs. The fallback's new id also only reached disk if the user happened to send a message afterwards, since the writer is driven by the message list. Persist it when the join lands instead.
George Ng (GeorgeNgMsft)
left a comment
There was a problem hiding this comment.
Looks good! Thanks for resolving the comments
|
Something worth noting, for the other clients, we fall back to a "named default" of "CLI" for CLI and "Shell" for Shell. It's a completely arbitrary behavior which is a bit different from the "default" being used here. It's worth noting just because it differs in behavior which may come into play when Android joins the multi-client scenario fully. It is probably a good idea to create an "Android" default conversation at this point so the behavior is unified. At some point we will probably change it though. |
Problem
The
mobile-2Android sample kept its connection and transcript inMainActivity. Activity recreation—such as rotation, theme, font-scale, or locale changes—therefore disconnected the WebSocket and discarded the visible chat. The transcript was also lost whenever Android reclaimed the app process.The Android client does not currently load AgentServer display history, so it needs a local conversation cache to restore the UI immediately after process death.
Changes
ChatViewModelso configuration changes preserve the WebSocket, messages, input state, and pending interactions.ConversationStore, backed bySharedPreferences, to persist:joinConversationduring reconnect.RESUMEDbefore dispatching foreground intents.Testing
Follow-up
A later PR can treat
ConversationStoreas an immediate/offline cache and usegetDisplayHistory(afterSeq?)to backfill updates produced by other clients sharing the same conversation.