Uh oh!
There was an error while loading. Please reload this page.
Support multiple android devices connected to Agent server - #2914
Open
jebrans wants to merge 11 commits into
Open
Support multiple android devices connected to Agent server#2914jebrans wants to merge 11 commits into
jebrans wants to merge 11 commits into
Conversation
Every Android device registered a client-hosted agent under the same fixed name `androidDevice`. Agent names are unique per conversation, so the second device failed with `App agent 'androidDevice' already exists`. Teardown had a matching defect: it was keyed by name with no reference count, so the first device to disconnect would have removed the agent for all of them. One logical agent now holds many device instances behind a multiplexer. The schema is registered with the dispatcher once; each device gets its own rpc proxy, and each call is routed to a single device - by default the one that made the request. The dynamic agent is removed only when the last device leaves. Server: - `clientAgentRegistry.ts` owns the groups: join-or-create, replace-in-place on a repeated `instanceId`, routing (requester, active device, only device, then ask), duplicate display-name disambiguation, and a schema-version key that hashes the schema text rather than the serialised manifest, so Android's unordered `JSONObject` keys cannot look like a version mismatch. - Group mutation is serialised per conversation. The dispatcher's command lock only wraps `addDynamicAgent`, not the lookup that decides to call it, so two devices registering at the same instant would otherwise both create a group. - Disconnect removes an instance only if it still names the disconnecting connection. A phone that sleeps reconnects and re-registers before its half-open socket is reaped; without the check the late disconnect would evict the live device. - An unregister with no `instanceId` resolves to the caller's own instance, so a client cannot drop another device's registration by name. - `TYPEAGENT_ALLOW_MULTIPLE_CLIENT_AGENT_INSTANCES=0` restores the old behaviour, including the original `already exists` message. SDK: `SessionContext.currentConnectionId` exposes the connection that made the request, threaded through agent-rpc's context params for out-of-process agents. Protocol: optional `instanceId` and `displayName` on registration, optional `instanceId` on unregistration, and `"android"` added to `clientType`. Every new field is optional, so a client that sends none still works as a single instance. Android sample: a `DeviceIdentity` seam backed by a `SharedPreferences` UUID (never a hardware id) with `Build.MODEL` as the default display name, injected into `WebSocketManager` so plain JVM unit tests keep working; `clientType` is now `"android"`; and the registration-collision recovery from PR 2891 is demoted to a compatibility shim for older servers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Readiness checks, completions and dynamic-schema calls run on the dispatcher's own schedule, so routing them through the ambiguous-target prompt would put a Which device? question in front of the user for something they never asked for. Restrict the prompt to executeAction, executeCommand, handleChoice and setup; everything else falls back to the most recently used device. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Routing had two fallbacks that guessed. After a 'Which device?' prompt the answer was remembered as the active device and reused silently for every later request, which is invisible state that sends an action to the wrong phone days later. Non-user-facing calls fell back to the most recently used device for the same reason. An action on the wrong phone is worse than one that does not run, because the user believes it ran. So routing is now certain or it fails: - the device that made the request, or - the only connected device, which keeps 'ask the shell, act on my one phone' working and cannot pick wrong, or - an error naming the connected devices. This deletes the prompt, the remembered active device and the reselect-on-disconnect path. Read-only calls (readiness, completion, dynamic schema) still take any instance: they change nothing on the device, and failing them would make the agent look broken for a question the user never asked. Also compacts the comments added on this branch. clientAgentRegistry.ts drops from 806 to 720 lines and from 135 to 100 comment lines, keeping the reasons that cannot be recovered from the code: the schema-text hash, the disconnect ownership check, why the lock exists, the per-instance context views, and why DeviceIdentity is injected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The shell registers a client-hosted agent the same way the Android app does (instance.ts:304), so grouping applied to it too: two shells on one conversation would both keep @shell instead of the second silently losing it. That may well be an improvement, but it is a behaviour change to a client this work was not about, and it needed its owner's sign-off before this could merge. Sharing a name is now opt-in per registration. The client that creates the registration decides, and a client that does not ask keeps exactly today's behaviour: a second, different client is rejected with the original 'App agent <name> already exists' error. The Android app opts in; the shell does not. The browser extension and VS Code are unaffected either way - their registerClientAgent is notSupported. The two fixes that are not behaviour changes still apply to every client: the disconnect ownership check, so a reconnecting client is not evicted by the late death of its old socket, and the per-conversation lock. The environment kill switch stays as a second, independent gate for the operator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Every existing case passed the boolean explicitly, so allowMultipleClientAgentInstances - the function the rollback plan depends on - was never executed by a test. A typo in the variable name or inverted '0'/'false' parsing would have shipped silently. Adds a parsed-value table (unset, empty, 1, true, yes, 0, false, FALSE, padded) and an end-to-end case: with the switch off a second device is rejected even though the client opted in, while a reconnect still replaces in place so a dropped socket does not strand the device. Also fixes a misleading trace. The rejection log named the group's multiInstance flag whichever gate refused, so a rejection caused by the environment switch read as a client that had not opted in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per-registration opt-in already isolates the change: only the Android app asks to share an agent name, and the group's creator decides, so the shell and every other client keep today's behaviour. The environment variable was a second gate on top of that with nothing left to protect - the app has not shipped, so there is no deployed APK to hold back without a rebuild. Removes the variable, its reader, the option that threaded it through addClientAgent and joinClientAgentGroup, and the eleven tests that covered it. The single-host case is still pinned by the opt-in test, which asserts the same 'already exists' text and that a known instanceId still reconnects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A connection hosts at most one instance of a given agent name: both would sit on the single agent:<name> channel, so a second one is always a dead proxy. Nothing enforced that. - joinClientAgentGroup now retires any instance already on the registering connection instead of adding a second. Requester routing scans in insertion order, so the stale one was picked ahead of the live device, and it outlived the connection because per-connection bookkeeping only tracks the newest id. Handing the slot over keeps the group non-empty, so the dynamic agent is not torn down and rebuilt. A client replacing itself is not a second device, so this skips the sharing opt-in. - unregisterClientAgent now drops the channel and the local entry only once the instance is actually gone. Naming another device's instance removes nothing, and clearing our own bookkeeping anyway stranded our instance past disconnect and killed our own live proxy. - removeClientAgent reports whether it removed anything. Also exclude the device id from Android backup and device transfer. Restoring it onto a second phone gave both the same instanceId, and the server would then route one phone's actions to the other. Drop an unused setDisplayName and the preference key only it wrote. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The lint ratchet counts a new file's violations as all-new, so the 11 no-explicit-any in clientAgentRegistry.ts failed the gate. The mux dispatches by method name, which AppAgent gives no indexed access to. Name that shape once as AppAgentMethod plus a methodsOf() view rather than casting each call site to any, and take arguments as unknown[] with a narrow cast where a context is actually read. Same for the manifest walker: SchemaBearingManifest describes the nesting it walks, so it no longer casts to Record<string, any> to reach two fields. Behaviour is unchanged; the registry and integration suites cover the dispatch paths this touches. Also document the four PR-only CI gates in CLAUDE.md. They only run on pull requests, so a change that builds and tests clean locally can still fail them, which is how these violations reached CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CLAUDE.md loads into every agent session in this repo, so anything added costs context on every task, not just the one that prompted it. The first pass over-corrected from a single PR: it grew the file by 22%, most of it serving cases that rarely come up. - Fold the Kotlin section into the gates section. android/ is touched in 20 of the last 1236 commits, which does not earn a section of its own with build commands and a style guide; the fact that nothing checks it is the part worth keeping. - Drop the "every added member needs a caller" section. It was stated as an absolute and is wrong for a published package, where exported API legitimately has no in-repo caller. It also largely repeated the existing "drop dead/unused parameters" and over-engineering guidance. What is left is one clause on the existing signatures bullet, with the public-API case called out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The PR added "android" to DispatcherConnectOptions.clientType but left the architecture doc listing only shell and extension. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Target design
Title: Let several devices register one client-hosted agent
Why
Every Android device registers a client-hosted agent under the same fixed name
androidDevice(AndroidDeviceAgent.NAME). Agent names are unique perconversation — so the second device is rejected in
appAgentManager.ts:1054:What this changes
One logical agent, many device instances behind a multiplexer, reference
counted. The schema is registered with the dispatcher exactly once; each device
keeps its own rpc proxy; each call goes to one device. This is the pattern the
browser agent already uses (
agentWebSocketServer.mts), not a new one.clientAgentRegistry.tsowns the groups. Registration is join-or-create:the first device creates the group and calls
addDynamicAgent, later deviceswith the same schema just add an instance.
behaviour. The shell registers a client-hosted agent through this same path
(
instance.ts:304) and expects to be the only host of@shell. It does notpass
multiInstance, so a second shell is still rejected with the originalerror, exactly as today. The Android app opts in. The browser extension and
VS Code never reach this code — their
registerClientAgentisnotSupported. The client that creates the registration decides, so nobodyregisters without opting in and keeps exactly today's behaviour; only the
Android app asks to share a name. A client that opts in cannot join a group
whose creator did not, so nobody can widen someone else's agent.
A reconnecting
instanceIdreplaces in place either way, since that pathfails today and so cannot regress anything.
DeviceIdentityseam backed by aSharedPreferencesUUID(never a hardware identifier, so no new permissions) with
Build.MODELas thedefault display name. It is injected rather than built from a
Contextbecause the sample's unit tests are plain JVM JUnit and
RegistrationCollisionRecoveryTestdrives a realWebSocketManager. Theparameter is required, not defaulted, so the production path cannot silently
fall back to a throwaway identity and make every reconnect look like a new
device.
clientTypewas"extension", copied from the browser extension. Nothingreads it today (
getBrowserControlForRequestis defined but never called), sothis is a correctness fix rather than a behaviour change. It ships with the
protocol change, since
"android"is rejected until the union is widened.Tests
29 new cases in the agent-server package.
clientAgentRegistry.spec.ts(27) covers the behaviour against the realregistry: a second device joins and the dispatcher still holds one agent; a
repeated
instanceIdreplaces rather than duplicates; a different schema isrejected with the version message while the same schema with reordered manifest
keys is accepted (the JSON key-order trap); device A disconnecting leaves B
working;
removeDynamicAgentfires exactly once and a double removal is inert;a name-only unregister removes nothing when the caller owns nothing; routing
picks the requester, falls back to the only device, and otherwise throws while
running nothing anywhere; asking from one device does not make it the target
for a later request from elsewhere (the guard against silent stickiness); two
devices sharing a display name are listed distinctly in the error; a read-only
call still runs when the target is ambiguous; overlapping requests from two
connections land on their own devices; two registrations issued before either
resolves produce one group and one
addDynamicAgent, plus the mirror add/removerace; a client that does not opt in staying the only host of its agent,
including a client that does opt in being unable to join its group — the shell
case, which also pins the original
already existstext and that a knowninstanceIdstill reconnects; and the sleeping-phone case, where the latedisconnect of a stale socket leaves the reconnected device registered.
clientAgentIntegration.spec.ts(2) drives two real client connections throughthe real connection handler and agent-rpc transport over an in-memory channel
pair: both register
androidDevice, each executes its own actions, A'sdisconnect leaves B working, and the last one out tears the dynamic agent down
once. The second case pins backward compatibility — a client that sends no
identity registers as a single instance keyed by its connection.
The routing tests use a
SessionContextwhosepopupQuestionthrows, so afuture change that reintroduces a prompt fails the suite rather than quietly
interrupting the user.
sessionContext.spec.tsgains one case:currentConnectionIdis the requestingconnection for a client request and
undefinedfor a server-initiated one.Android: 3 cases pinning that registration carries the device identity, that it
opts in with
multiInstance, and that the eviction call names no instance,since giving it one would reintroduce a way to drop a live registration.
Verification
pnpm run clean && pnpm run build— BUILD SUCCESSFUL.pnpm --filter agent-server test— 66 passed (48 before).pnpm --filter agent-dispatcher test:local—sessionContext.spec16 passed.pnpm run prettier:changed:fix— clean.:app:testDebugUnitTest— 171 tests, 0 failures (169 before).:app:assembleDebug— BUILD SUCCESSFUL.