feat(host-proxy): cross-client preactivation for host_cu + host_app_control#30154
Conversation
…ost_cu + host_app_control Add `shouldAttachHostProxyForCapability` helper and generalize `preactivateHostProxySkills` to also fire when the source interface lacks native support but at least one connected client has the capability via `assistantEventHub.listClientsByCapability`. Excludes `chrome-extension` as a security boundary, mirroring the carve-out in `conversation-tool-setup.ts:isToolActiveForContext`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…oss-client turns Replace `supportsHostProxy(sourceInterface, capability)` at the proxy instantiation gates in `conversation-routes.ts` and `process-message.ts` with the new `shouldAttachHostProxyForCapability` helper. Without this, the proxies would be `undefined` on web/iOS turns even after the preactivation gate allows the skills through, causing tool calls to fail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The comment previously said host_cu and host_app_control cross-client preactivation was "a separate workstream." That work is now done — handled at the preactivation layer via `preactivateHostProxySkills`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add tests for `shouldAttachHostProxyForCapability` and `preactivateHostProxySkills` covering: - Native support (macOS → preactivate) - Cross-client available (web/ios + capable client → preactivate) - No capable client (web/ios + no client → no preactivation) - chrome-extension security boundary (client connected but excluded) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🚫 do-not-merge: Must merge after |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a3581b5aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!sourceInterface) return false; | ||
| if (supportsHostProxy(sourceInterface, capability)) return true; | ||
| if (sourceInterface === "chrome-extension") return false; | ||
| return assistantEventHub.listClientsByCapability(capability).length > 0; |
There was a problem hiding this comment.
shouldAttachHostProxyForCapability now enables host_cu/host_app_control whenever any client advertises the capability, but it has no actor-principal filter, and the new call sites in process-message.ts/conversation-routes.ts pass only sourceInterface. In a shared assistant, a web/iOS turn from user A will attach and expose these tools if only user B has a connected macOS client, which can route execution cross-user (e.g., untargeted host-proxy requests go to capability subscribers). This violates the same-actor boundary expected by other host-proxy paths and should be gated by source actor identity, not global capability presence.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 Cross-client CU/app-control from non-interactive interfaces (telegram, slack) is gated by isInteractive in drain paths but not create paths
In the drain paths, the isInteractive !== false guard at conversation-process.ts:443 and conversation-process.ts:884 prevents preactivation for non-interactive messages (telegram, slack, etc.). However, in the conversation-routes.ts create path (line 1437-1438), preactivateHostProxySkills is called without an isInteractive check. This means a telegram-sourced message going through the create path (when conversation is idle) would preactivate CU/app-control skills if a macOS client is connected, but a queued telegram message going through the drain path would NOT preactivate. This inconsistency is pre-existing (the drain path has always had the isInteractive gate) but becomes more relevant now that cross-client routing can activate these skills for non-desktop interfaces.
(Refers to lines 1434-1439)
Was this helpful? React with 👍 or 👎 to provide feedback.
| ): void { | ||
| if (!sourceInterface) return; | ||
| for (const { capability, skillId } of HOST_PROXY_SKILL_PREACTIVATIONS) { | ||
| if (supportsHostProxy(sourceInterface, capability)) { | ||
| if (shouldAttachHostProxyForCapability(capability, sourceInterface)) { | ||
| conversation.addPreactivatedSkillId(skillId); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Drain paths preactivate skills without instantiating proxies when a capable client connects mid-processing
The drain paths (drainSingleMessage and drainBatch in conversation-process.ts) only call preactivateHostProxySkills — they never instantiate HostCuProxy or HostAppControlProxy. Before this PR, preactivateHostProxySkills used the static supportsHostProxy() check, which only returned true for interfaces that natively support capabilities (e.g. macos), and the proxy was always already attached from the create path. After this PR, preactivateHostProxySkills internally calls shouldAttachHostProxyForCapability, which dynamically checks the event hub for connected capable clients. This breaks the assumption stated in the comment at conversation-process.ts:440-441 ("even though the underlying proxies … are still attached").
Scenario that triggers the bug
- Web client sends message A. No macOS client connected → no proxy instantiated. A starts processing.
- Web client sends message B while A processes → B queued. Still no macOS client, so proxy not instantiated at
conversation-routes.ts:1412. - macOS client connects to the event hub.
- A finishes. Drain processes B.
preactivateHostProxySkillsatconversation-process.ts:446detects the macOS client → preactivatescomputer-useandapp-controlskills.- LLM sees CU/app-control tools, invokes them.
surfaceProxyResolveratconversation-surfaces.ts:1910checksctx.hostCuProxy→ undefined → returns "Computer use is not available — no desktop client connected" even though a macOS client IS connected.
(Refers to lines 96-106)
Prompt for agents
The drain paths in conversation-process.ts (drainSingleMessage ~line 443, drainBatch ~line 884) call preactivateHostProxySkills but never instantiate HostCuProxy / HostAppControlProxy. Before this PR the preactivation function used the static supportsHostProxy() check so this was safe (proxies were always pre-attached for native-support interfaces). Now that preactivateHostProxySkills uses shouldAttachHostProxyForCapability (which dynamically queries the event hub for connected capable clients), the drain paths can preactivate skills for non-native interfaces (web, ios) without a corresponding proxy object on the conversation.
The fix should mirror the create-path pattern from conversation-routes.ts:1412-1433 and process-message.ts:159-178 in the drain paths: before calling preactivateHostProxySkills, check shouldAttachHostProxyForCapability for each capability, and instantiate the proxy if it's missing (conversation.hostCuProxy / conversation.hostAppControlProxy is undefined). This ensures skills are only preactivated when the proxy is available to service them.
Alternatively, preactivateHostProxySkills itself could be extended to also handle proxy instantiation, but that would change its interface contract (it would need access to the Conversation object, not just HostProxyPreactivationTarget).
Was this helpful? React with 👍 or 👎 to provide feedback.
Move the HostProxyCapability import after mock.module calls so all project imports form a single contiguous group, satisfying simple-import-sort. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
simple-import-sort requires alphabetical order within the import statement; type HostProxyPreactivationTarget must precede the function imports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ss-client connections Drain paths (drainSingleMessage, drainBatch) previously re-preactivated host-proxy skills but didn't ensure the backing proxies were instantiated. A macOS client connecting after a web turn was enqueued would cause preactivateHostProxySkills to mark skills active while hostCuProxy / hostAppControlProxy remained undefined — the LLM would see tools whose executor would immediately fail. Fix: add ensureHostProxiesForTurn to ProcessConversationContext and implement it on Conversation. Drain paths call it before preactivation so the proxy is always present when the skill is activated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Review findings — response + fixFinding 2 (Devin BUG): Drain paths preactivate skills without instantiating proxies ✅ FixedValid bug — fixed in commit Added Finding 1 (Codex P1) + Devin Finding 3 (isInteractive gate): By design / pre-existingSame-actor (Codex P1): Preactivation cross-actor means the LLM sees tools that may fire — but isInteractive gate inconsistency (Devin Finding 3): The Both non-actionable findings are pre-empted by design. This PR is implementation-complete and pending merge of |
Merge-ready ✅CI green (Lint, Test, Type Check all pass on latest push). All review findings addressed:
🚫 Merge blocker: |
#30163) * refactor(host-browser): remove dead WS resolveResult path The chrome extension migrated to HTTP POST in PR #29829. There is no /v1/browser-relay WebSocket endpoint in the current runtime — no WS handler calls HostBrowserProxy.resolveResult. Removing the public method eliminates the foot-gun of a future WS handler bypassing the kind-check and same-actor guard that the HTTP route enforces. The route now inlines pendingInteractions.resolve() + rpcResolve directly. Tests are updated to use a local resolveResult helper that mirrors the same call sequence. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(host-proxy-preactivation): add cross-client drain-path coverage PR #30154 enabled web/iOS turns to drive host_app_control and host_cu on a connected macOS client (cross-client routing). The existing drain-path tests cover macOS-native + chrome-extension + slack; this adds the missing cross-client cases: - web source + listClientsByCapability returns 1 macOS client → both app-control and computer-use skills re-added - web source + listClientsByCapability returns [] → neither re-added Introduces a per-test mockCapabilityClients variable (with afterEach reset) so hub state doesn't bleed across tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: credence-the-bot[bot] <test@test.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
What
Closes the LLM-exposure gap for
host_cuandhost_app_controlon web/iOS turns. The proxy execution path already handles cross-client routing (viaHostProxyBase.dispatchRequest+ auto-resolve inconversation-surfaces.ts); the only missing piece was the skill preactivation gate, which previously fired only when the source interface natively supported the capability.This PR generalizes
preactivateHostProxySkills(and proxy instantiation) to also fire when (a) the source interface doesn't natively support the capability AND (b) at least one connected client does. Mirrors the cross-client carve-out already in place forhost_bash/host_file/host_browseratconversation-tool-setup.ts:isToolActiveForContext.Why now
After PR #30066 (host_browser
target_client_id, May 9) closed Phase 4 of cross-client host proxy work, the remaining LLM-exposure gaps werehost_cuandhost_app_control. Investigation revealed the carve-out comment inconversation-tool-setup.tsdescribing them as a "separate workstream" overstated the work — proxy infrastructure already exists.Sequencing
credence/host-app-control-same-actor(the same-actor guard PR). Reason: that PR establishes the security boundary onhost_app_controlresult submissions; without it, exposing the skill cross-client would let any submitter resolve a result for the targeted client.Merge after the host_app_control same-actor guard PR (
credence/host-app-control-same-actor). Do not merge until that lands.Scope
host-proxy-preactivation.ts: addshouldAttachHostProxyForCapabilityhelper and extend the preactivation helper to consultassistantEventHub.listClientsByCapabilitywhen the source interface lacks native support.conversation-routes.ts+process-message.ts: same gate at proxy instantiation time (using the new helper).conversation-tool-setup.ts: update carve-out comment.Out of scope
host_app_control(its own PR —credence/host-app-control-same-actor)target_client_idLLM-facing param on skill tools (deferred — auto-resolve covers single-client case, ambiguous case errors with a useful message)Test command
Refs
/workspace/scratch/cross-client-skill-preactivation/DESIGN.mdcredence/host-app-control-same-actor(must merge first)