Trace client agent cleanup failures and pin the group's method set

android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AndroidDeviceAgent.kt

@@ -7,6 +7,24 @@ internal object AndroidDeviceAgent {
     const val NAME = "androidDevice"
     const val CHANNEL_NAME = "agent:$NAME"
     const val SCHEMA_ASSET = "typeagent/androidDeviceSchema.ts"
+
+    /**
+     * Methods this agent answers on its RPC channel, sent as `agentInterface`
+     * at registration.
+     *
+     * The server builds its proxy from this list and, when several devices host
+     * `androidDevice`, rejects one whose list differs from the others. So it has
+     * to describe what `handleAndroidDeviceInvoke` really dispatches: declaring
+     * a method the device cannot answer fails only later, at the call. Keeping
+     * one list for both the declaration and the dispatch guard is what stops the
+     * two from drifting - nothing else checks them against each other, and no CI
+     * job builds this module.
+     */
+    val SUPPORTED_METHODS = listOf("executeAction")
+
+    /** Whether [SUPPORTED_METHODS] covers an incoming RPC method. */
+    fun supports(methodName: String): Boolean = SUPPORTED_METHODS.contains(methodName)
+
     private const val AGENT_DESCRIPTION =
         "Acts on this Android device: sets alarms and countdown timers, shows the " +
             "alarm and timer lists, searches for nearby places, shows a place on the " +
@@ -34,11 +52,14 @@ internal object AndroidDeviceAgent {
             .put("actionDefaultEnabled", true)
             .put("schema", schema)
 
+        val agentInterface = JSONArray()
+        SUPPORTED_METHODS.forEach { agentInterface.put(it) }
+
         return JSONObject()
             .put("name", NAME)
             .put("conversationId", conversationId)
             .put("manifest", manifest)
-            .put("agentInterface", JSONArray().put("executeAction"))
+            .put("agentInterface", agentInterface)
             // Identifies this device so several devices can share one
             // `androidDevice` agent, and so a reconnect replaces this device
             // instead of adding another. `multiInstance` is the opt-in: without

android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt

@@ -748,7 +748,10 @@ class WebSocketManager internal constructor(
             Log.e(TAG, "Android agent invocation is missing callId.")
             return
         }
-        if (methodName != "executeAction") {
+        // The same list registration declares as agentInterface, so the guard
+        // and the declaration cannot drift apart. It has one entry today; a
+        // second would need its own dispatch below, not just a line in the list.
+        if (!AndroidDeviceAgent.supports(methodName)) {
             sendRpcError(
                 channelName,
                 callId,

android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AndroidDeviceAgentTest.kt

@@ -23,10 +23,16 @@ class AndroidDeviceAgentTest {
         assertEquals("instance-1", registration.getString("instanceId"))
         assertEquals("Pixel 8", registration.getString("displayName"))
         assertEquals(true, registration.getBoolean("multiInstance"))
-        assertEquals(
-            "executeAction",
-            registration.getJSONArray("agentInterface").getString(0)
-        )
+        // The declared set must be exactly what the RPC dispatcher answers: the
+        // server builds its proxy from this and, with several devices hosting
+        // the agent, rejects one that declares a different set.
+        val declared = registration.getJSONArray("agentInterface")
+        assertEquals(AndroidDeviceAgent.SUPPORTED_METHODS.size, declared.length())
+        for (index in AndroidDeviceAgent.SUPPORTED_METHODS.indices) {
+            assertEquals(AndroidDeviceAgent.SUPPORTED_METHODS[index], declared.getString(index))
+        }
+        assertTrue(AndroidDeviceAgent.supports("executeAction"))
+        assertFalse(AndroidDeviceAgent.supports("getDynamicDisplay"))
         assertEquals(
             "export type AndroidDeviceAction = never;",
             registration

ts/packages/agentServer/server/src/clientAgentRegistry.ts

@@ -7,6 +7,7 @@ import {
     AppAgentManifest,
     SessionContext,
 } from "@typeagent/agent-sdk";
+import type { AgentInterfaceFunctionName } from "@typeagent/agent-rpc/server";
 import { createHash } from "node:crypto";
 import { createLimiter } from "@typeagent/common-utils";
 import registerDebug from "debug";
@@ -36,6 +37,11 @@ export type ClientAgentGroup = {
     manifest: AppAgentManifest;
     /** Hash of the schema source; instances must agree on it. See {@link getManifestKey}. */
     manifestKey: string;
+    /**
+     * Normalized `agentInterface` of the instances currently in the group. See
+     * {@link getAgentInterfaceKey}.
+     */
+    agentInterfaceKey: string;
     /**
      * Whether the client that created this group opted in to sharing the name.
      * Off means a second, different client is rejected exactly as it was before
@@ -53,6 +59,13 @@ export type ClientAgentRegistration = {
     connectionId: string;
     appAgent: AppAgent;
     manifest: AppAgentManifest;
+    /**
+     * Methods the client implements, which the caller already used to build
+     * {@link ClientAgentRegistration.appAgent}. Required: `registerClientAgent`
+     * takes it as a required field and `createAgentRpcClient` cannot build a
+     * proxy without it, so a registration that reaches here always has one.
+     */
+    agentInterface: readonly AgentInterfaceFunctionName[];
     /** See {@link ClientAgentGroup.multiInstance}. Only read on the first registration. */
     multiInstance?: boolean;
 };
@@ -135,6 +148,27 @@ export function schemaMismatchMessage(name: string): string {
     return `Client agent '${name}' is already registered on this conversation with a different schema version. Update the app to the same version as the other device(s), or disconnect them first.`;
 }
 
+/**
+ * Normalized `agentInterface`, order-insensitive and de-duplicated so key order
+ * cannot cause a false mismatch (the same trap {@link getManifestKey} avoids
+ * for Android's `org.json.JSONObject`).
+ *
+ * The mux is built once, from the first instance's proxy, and
+ * {@link getManifestKey} only covers schema text -- two app versions can share
+ * a schema and still implement different methods. Without this, a device with a
+ * narrower interface joins a group created by a richer one and silently appears
+ * to support methods it does not; the call only fails once someone makes it.
+ */
+export function getAgentInterfaceKey(
+    agentInterface: readonly AgentInterfaceFunctionName[],
+): string {
+    return [...new Set(agentInterface)].sort().join("\u0000");
+}
+
+export function interfaceMismatchMessage(name: string): string {
+    return `Client agent '${name}' is already registered on this conversation by a device that implements a different set of methods. Update the app to the same version as the other device(s), or disconnect them first.`;
+}
+
 /**
  * Names the user can tell apart. Two phones of the same model both report
  * "Pixel 8", so duplicates get a numeric suffix in registration order.
@@ -481,6 +515,27 @@ function createMux(group: ClientAgentGroup, template: AppAgent): AppAgent {
     return mux as unknown as AppAgent;
 }
 
+/**
+ * Point the group's existing mux at a new method set, in place.
+ *
+ * The dispatcher was handed this exact object by `addDynamicAgent` and keeps
+ * that reference, checking each optional method on it at call time. Assigning a
+ * fresh object to `group.mux` would therefore leave the dispatcher on the old
+ * one, so the methods have to be swapped onto the object it already holds.
+ */
+function rebuildMux(group: ClientAgentGroup, template: AppAgent): void {
+    const next = methodsOf(createMux(group, template));
+    const current = methodsOf(group.mux);
+    for (const method of Object.keys(current)) {
+        if (next[method] === undefined) {
+            delete current[method];
+        }
+    }
+    for (const method of Object.keys(next)) {
+        current[method] = next[method];
+    }
+}
+
 /**
  * Bring a device that joined late up to the state the others are in. Failures
  * are traced, not thrown: one device must not fail another's registration.
@@ -530,6 +585,7 @@ export function createClientAgentGroup(
         name,
         manifest: registration.manifest,
         manifestKey: getManifestKey(registration.manifest),
+        agentInterfaceKey: getAgentInterfaceKey(registration.agentInterface),
         multiInstance: registration.multiInstance === true,
         instances: new Map([[instance.instanceId, instance]]),
         mux: undefined as unknown as AppAgent,
@@ -546,6 +602,25 @@ export function createClientAgentGroup(
     return group;
 }
 
+/**
+ * True when this registration takes over the group's only instance: either the
+ * same `instanceId` coming back, or the same connection re-registering under a
+ * new one (its old proxy died when the new one claimed the `agent:<name>`
+ * channel). Either way no other device is in the group, so the method set is
+ * this device's alone to change.
+ */
+function replacesSoleInstance(
+    group: ClientAgentGroup,
+    registration: ClientAgentRegistration,
+): boolean {
+    return (
+        group.instances.size === 1 &&
+        (group.instances.has(registration.instanceId) ||
+            findInstanceIdForConnection(group, registration.connectionId) !==
+                undefined)
+    );
+}
+
 /**
  * Add a device, or replace its proxy if the same `instanceId` is already
  * there. Replacing in place is what makes a reconnect work: the device keeps
@@ -561,6 +636,27 @@ export async function joinClientAgentGroup(
         throw new Error(schemaMismatchMessage(group.name));
     }
 
+    // Checked alongside the schema, and for a replacement too: the mux was
+    // built from the method set of whichever proxy created the group, so an
+    // instance that arrives with a different one would be routed calls it
+    // cannot answer.
+    const agentInterfaceKey = getAgentInterfaceKey(registration.agentInterface);
+    if (agentInterfaceKey !== group.agentInterfaceKey) {
+        // Unless this device is the whole group. A lone device that upgrades
+        // its app keeps its schema but can gain or lose methods, and rejecting
+        // that would break a reconnect that used to work - with a message
+        // telling the user to disconnect devices that do not exist. Nobody else
+        // is sharing the name, so adopt the new set and bring the mux with it.
+        if (!replacesSoleInstance(group, registration)) {
+            throw new Error(interfaceMismatchMessage(group.name));
+        }
+        group.agentInterfaceKey = agentInterfaceKey;
+        rebuildMux(group, registration.appAgent);
+        debugGroup(
+            `${group.name}: sole instance ${registration.instanceId} changed the method set; mux rebuilt`,
+        );
+    }
+
     const existing = group.instances.get(registration.instanceId);
     if (existing !== undefined) {
         existing.appAgent = registration.appAgent;

ts/packages/agentServer/server/src/connectionHandler.ts

@@ -24,6 +24,13 @@ import type { PortRegistrar } from "agent-dispatcher";
 import type { ConversationManager } from "./conversationManager.js";
 import { resolveTunnelUrlForDiscovery } from "./tunnelResolver.js";
 import { getSpeechToken } from "./speechToken.js";
+import registerDebug from "debug";
+
+// Disconnect cleanup is best effort, so a failure cannot be surfaced to anyone:
+// the socket it would be reported on is already gone. Without a trace, a client
+// agent left behind on the shared dispatcher only shows up much later as a
+// routing failure with nothing pointing back at the cause.
+const debugError = registerDebug("agent-server:connection:error");
 
 /**
  * Per-connection handler signature expected by transports (the WebSocket
@@ -609,6 +616,7 @@ export function createAgentServerConnectionHandler(
                         displayName,
                         connectionId,
                         param.multiInstance === true,
+                        agentInterface,
                     );
                 } catch (e) {
                     channelProvider.deleteChannel(`agent:${name}`);
@@ -683,8 +691,18 @@ export function createAgentServerConnectionHandler(
                         .removeClientAgent(conversationId, name, instanceId, {
                             ownerConnectionId: connectionId,
                         })
-                        .catch(() => {
-                            // Best effort on disconnect
+                        .catch((e) => {
+                            // Best effort on disconnect, but not silent: this
+                            // failing is how a client agent leaks onto the
+                            // shared dispatcher. Not retried on purpose --
+                            // removal is idempotent and ownership-checked, so a
+                            // second attempt could only race a reconnect that
+                            // has legitimately reclaimed the instance.
+                            debugError(
+                                `Failed to remove client agent "${name}" instance ${instanceId} (connection ${connectionId}) from conversation ${conversationId} on disconnect: ${
+                                    e instanceof Error ? e.message : String(e)
+                                }`,
+                            );
                         });
                 }
             }
@@ -695,8 +713,15 @@ export function createAgentServerConnectionHandler(
             ] of joinedConversations.entries()) {
                 conversationManager
                     .leaveConversation(conversationId, connectionId)
-                    .catch(() => {
-                        // Best effort on disconnect
+                    .catch((e) => {
+                        // Best effort on disconnect, but traced: a conversation
+                        // this connection never leaves keeps its dispatcher
+                        // alive and its idle timer from ever starting.
+                        debugError(
+                            `Failed to leave conversation ${conversationId} for connection ${connectionId} on disconnect: ${
+                                e instanceof Error ? e.message : String(e)
+                            }`,
+                        );
                     });
             }
             joinedConversations.clear();

ts/packages/agentServer/server/src/conversationManager.ts

@@ -24,6 +24,7 @@ import {
     ConversationSummaryResult,
 } from "agent-dispatcher";
 import type { AppAgent, AppAgentManifest } from "@typeagent/agent-sdk";
+import type { AgentInterfaceFunctionName } from "@typeagent/agent-rpc/server";
 import type {
     DisplayLogEntry,
     PendingInteractionRequest,
@@ -253,8 +254,9 @@ export type ConversationManager = {
      * same schema: the dynamic agent is added once and each client becomes an
      * instance behind it. Re-registering the same `instanceId` replaces its
      * proxy in place, which is how a reconnect recovers. Rejects when the
-     * schema differs, or when the instance is new and multi-instance support
-     * is switched off.
+     * schema differs, when the `agentInterface` differs from what the other
+     * devices implement, or when the instance is new and multi-instance
+     * support is switched off.
      */
     addClientAgent(
         conversationId: string,
@@ -265,6 +267,7 @@ export type ConversationManager = {
         displayName: string,
         connectionId: string,
         multiInstance: boolean,
+        agentInterface: readonly AgentInterfaceFunctionName[],
     ): Promise<void>;
     /**
      * Remove one instance added via {@link addClientAgent}. The dynamic agent
@@ -1220,6 +1223,7 @@ export async function createConversationManager(
             displayName: string,
             connectionId: string,
             multiInstance: boolean,
+            agentInterface: readonly AgentInterfaceFunctionName[],
         ): Promise<void> {
             const record = conversations.get(conversationId);
             if (record === undefined) {
@@ -1232,6 +1236,7 @@ export async function createConversationManager(
                 connectionId,
                 appAgent,
                 manifest,
+                agentInterface,
                 multiInstance,
             });
             debugConversation(

ts/packages/agentServer/server/test/clientAgentIntegration.spec.ts

@@ -16,6 +16,7 @@ import {
 } from "@typeagent/agent-rpc/channel";
 import type { AppAgent, AppAgentManifest } from "@typeagent/agent-sdk";
 import type { TypeAgentAction } from "@typeagent/agent-sdk";
+import type { AgentInterfaceFunctionName } from "@typeagent/agent-rpc/server";
 import type { ClientIO } from "@typeagent/dispatcher-rpc/types";
 import type { MacroManager } from "@typeagent/copilot-macros";
 import {
@@ -102,13 +103,15 @@ function createTestServer(): TestServer {
             displayName: string,
             connectionId: string,
             multiInstance: boolean,
+            agentInterface: readonly AgentInterfaceFunctionName[],
         ) {
             await registry.add(host, name, {
                 instanceId,
                 displayName,
                 connectionId,
                 appAgent,
                 manifest: agentManifest,
+                agentInterface,
                 multiInstance,
             });
         },