diff --git a/assistant/src/__tests__/conversation-app-control-instantiation.test.ts b/assistant/src/__tests__/conversation-app-control-instantiation.test.ts index b867f28daf2..8ea5992d265 100644 --- a/assistant/src/__tests__/conversation-app-control-instantiation.test.ts +++ b/assistant/src/__tests__/conversation-app-control-instantiation.test.ts @@ -192,6 +192,8 @@ mock.module("../skills/version-hash.js", () => ({ const { HostAppControlProxy } = await import("../daemon/host-app-control-proxy.js"); +const { preactivateHostProxySkills } = + await import("../daemon/host-proxy-preactivation.js"); const { projectSkillTools, resetSkillToolProjection } = await import("../daemon/conversation-skill-tools.js"); @@ -228,9 +230,11 @@ function makeFakeConversation(): FakeConversation { /** * Replica of the gating block from `prepareConversationForMessage` - * (process-message.ts) and `conversation-routes.ts`. Mirrors the production - * code exactly — when the diverged copies are merged into a shared helper, - * this test should be updated to call it directly. + * (process-message.ts) and `conversation-routes.ts`. The proxy-attachment + * step still lives inline at each call site (the proxy constructors take + * different argument shapes), but the preactivation step routes through the + * shared `preactivateHostProxySkills` helper exactly as the production code + * does. */ function applyAppControlInstantiation( conv: FakeConversation, @@ -240,12 +244,12 @@ function applyAppControlInstantiation( if (!conv.isProcessing() || !conv.hostAppControlProxy) { conv.setHostAppControlProxy(new HostAppControlProxy(conv.conversationId)); } - if (!conv.isProcessing()) { - conv.addPreactivatedSkillId("app-control"); - } } else if (!conv.isProcessing()) { conv.setHostAppControlProxy(undefined); } + if (!conv.isProcessing()) { + preactivateHostProxySkills(conv, interfaceId); + } } // --------------------------------------------------------------------------- diff --git a/assistant/src/daemon/conversation-process.ts b/assistant/src/daemon/conversation-process.ts index c3b82fb4922..61ae439befb 100644 --- a/assistant/src/daemon/conversation-process.ts +++ b/assistant/src/daemon/conversation-process.ts @@ -14,7 +14,6 @@ import { import { parseChannelId, parseInterfaceId, - supportsHostProxy, type TurnChannelContext, type TurnInterfaceContext, } from "../channels/types.js"; @@ -46,6 +45,7 @@ import { type SlashContext, } from "./conversation-slash.js"; import { getModelInfo } from "./handlers/config-model.js"; +import { preactivateHostProxySkills } from "./host-proxy-preactivation.js"; import type { ServerMessage, UsageStats, @@ -434,20 +434,10 @@ async function drainSingleMessage( if (next.isInteractive !== false) { const interfaceCtx = queuedInterfaceCtx ?? conversation.getTurnInterfaceContext(); - const sourceInterface = interfaceCtx?.userMessageInterface; - if (sourceInterface && supportsHostProxy(sourceInterface)) { - conversation.addPreactivatedSkillId("computer-use"); - } - // Gated on the `host_app_control` capability rather than the no-arg - // form so future host-proxy clients that opt into a subset can be - // selectively included. (chrome-extension supports `host_browser` - // but NOT `host_app_control`.) - if ( - sourceInterface && - supportsHostProxy(sourceInterface, "host_app_control") - ) { - conversation.addPreactivatedSkillId("app-control"); - } + preactivateHostProxySkills( + conversation, + interfaceCtx?.userMessageInterface, + ); } // Snapshot persona context at turn start so later tool turns can't pick up @@ -882,16 +872,10 @@ async function drainBatch( if (head.isInteractive !== false) { const interfaceCtx = queuedInterfaceCtx ?? conversation.getTurnInterfaceContext(); - const sourceInterface = interfaceCtx?.userMessageInterface; - if (sourceInterface && supportsHostProxy(sourceInterface)) { - conversation.addPreactivatedSkillId("computer-use"); - } - if ( - sourceInterface && - supportsHostProxy(sourceInterface, "host_app_control") - ) { - conversation.addPreactivatedSkillId("app-control"); - } + preactivateHostProxySkills( + conversation, + interfaceCtx?.userMessageInterface, + ); } // Snapshot persona context at turn start so later tool turns can't pick up diff --git a/assistant/src/daemon/host-proxy-preactivation.ts b/assistant/src/daemon/host-proxy-preactivation.ts new file mode 100644 index 00000000000..83acc88b176 --- /dev/null +++ b/assistant/src/daemon/host-proxy-preactivation.ts @@ -0,0 +1,82 @@ +/** + * Shared host-proxy skill preactivation registry. + * + * Several call sites need to mark host-proxy-backed skills as preactivated + * for a turn whenever the source interface supports the corresponding + * `HostProxyCapability`: + * + * - `runtime/routes/conversation-routes.ts` (create path, /v1/messages) + * - `daemon/process-message.ts` (create path, prepareConversationForMessage) + * - `daemon/conversation-process.ts` `drainSingleMessage` (re-add after dequeue) + * - `daemon/conversation-process.ts` `drainBatch` (re-add after dequeue) + * + * The create paths additionally instantiate the proxy itself; that + * instantiation logic is per-proxy-class and stays inline at each create + * site (constructors take different argument shapes — `HostCuProxy()` vs + * `HostAppControlProxy(conversationId)`). This module owns only the + * capability-to-skill mapping and the preactivation step. Adding a new + * host-proxy-backed skill is a one-line registry change here instead of + * touching all four call sites. + * + * Why a registry instead of repeated branches: each new host-proxy-backed + * skill that ships (e.g. a future `host_focus` capability with a `focus` + * skill) would otherwise add four near-identical `if (supportsHostProxy(...)) + * conversation.addPreactivatedSkillId("...")` blocks across these files. + * Centralizing the list makes the contract obvious and prevents drift + * where one call site re-adds a skill but another forgets to. + */ + +import type { HostProxyCapability, InterfaceId } from "../channels/types.js"; +import { supportsHostProxy } from "../channels/types.js"; + +/** + * Subset of Conversation/ProcessConversationContext that + * `preactivateHostProxySkills` needs. Both `Conversation` and + * `ProcessConversationContext` satisfy this structurally. + */ +export interface HostProxyPreactivationTarget { + addPreactivatedSkillId(id: string): void; +} + +/** + * Registry mapping each host-proxy capability to the skill that must be + * preactivated when that capability is supported by the source interface. + * + * Keep this list in sync with `HostProxyCapability` for any capability that + * has a corresponding bundled skill. + * + * Capabilities NOT listed here: + * - `host_bash`, `host_file` — these are surfaced as built-in tools rather + * than skills, so there is nothing to preactivate. + * - `host_browser` — the browser proxy is provisioned via the assistant + * event hub for chrome-extension and its skill projection is governed by + * a different code path (`host-browser-proxy.ts`). + */ +export const HOST_PROXY_SKILL_PREACTIVATIONS: ReadonlyArray<{ + capability: HostProxyCapability; + skillId: string; +}> = [ + { capability: "host_cu", skillId: "computer-use" }, + { capability: "host_app_control", skillId: "app-control" }, +]; + +/** + * Preactivate every host-proxy-backed skill that the given source interface + * supports. No-op when `sourceInterface` is undefined. + * + * Callers are responsible for any additional gating (e.g. only preactivating + * when the conversation is idle vs. when re-adding after dequeue), since + * those constraints differ across create vs. drain paths. This helper just + * iterates the registry and dispatches. + */ +export function preactivateHostProxySkills( + conversation: HostProxyPreactivationTarget, + sourceInterface: InterfaceId | undefined, +): void { + if (!sourceInterface) return; + for (const { capability, skillId } of HOST_PROXY_SKILL_PREACTIVATIONS) { + if (supportsHostProxy(sourceInterface, capability)) { + conversation.addPreactivatedSkillId(skillId); + } + } +} diff --git a/assistant/src/daemon/process-message.ts b/assistant/src/daemon/process-message.ts index 9f5050290ca..8011db81de5 100644 --- a/assistant/src/daemon/process-message.ts +++ b/assistant/src/daemon/process-message.ts @@ -47,6 +47,7 @@ import { import type { ConversationCreateOptions } from "./handlers/shared.js"; import { HostAppControlProxy } from "./host-app-control-proxy.js"; import { HostCuProxy } from "./host-cu-proxy.js"; +import { preactivateHostProxySkills } from "./host-proxy-preactivation.js"; const log = getLogger("process-message"); @@ -157,7 +158,6 @@ async function prepareConversationForMessage( if (!conversation.isProcessing() || !conversation.hostCuProxy) { conversation.setHostCuProxy(new HostCuProxy()); } - conversation.addPreactivatedSkillId("computer-use"); } else if (!conversation.isProcessing()) { conversation.setHostCuProxy(undefined); } @@ -171,10 +171,12 @@ async function prepareConversationForMessage( new HostAppControlProxy(conversationId), ); } - conversation.addPreactivatedSkillId("app-control"); } else if (!conversation.isProcessing()) { conversation.setHostAppControlProxy(undefined); } + // The early `isProcessing()` throw above guarantees the conversation is + // idle here, so preactivation is unconditional once the proxies are wired. + preactivateHostProxySkills(conversation, resolvedInterface); conversation.setCommandIntent(options?.commandIntent ?? null); conversation.setTurnChannelContext({ userMessageChannel: resolvedChannel, diff --git a/assistant/src/runtime/routes/conversation-routes.ts b/assistant/src/runtime/routes/conversation-routes.ts index eecedc68bb6..71188c722cf 100644 --- a/assistant/src/runtime/routes/conversation-routes.ts +++ b/assistant/src/runtime/routes/conversation-routes.ts @@ -46,6 +46,7 @@ import { import { renderHistoryContent } from "../../daemon/handlers/shared.js"; import { HostAppControlProxy } from "../../daemon/host-app-control-proxy.js"; import { HostCuProxy } from "../../daemon/host-cu-proxy.js"; +import { preactivateHostProxySkills } from "../../daemon/host-proxy-preactivation.js"; import type { ServerMessage } from "../../daemon/message-protocol.js"; import type { HostProxyTransportMetadata, @@ -1396,12 +1397,6 @@ export async function handleSendMessage( if (!conversation.isProcessing() || !conversation.hostCuProxy) { conversation.setHostCuProxy(new HostCuProxy()); } - // Only preactivate CU when the conversation is idle — if the conversation is - // processing, this message will be queued and preactivation is deferred - // to dequeue time in drainQueueImpl to avoid mutating in-flight turn state. - if (!conversation.isProcessing()) { - conversation.addPreactivatedSkillId("computer-use"); - } } else if (!conversation.isProcessing()) { conversation.setHostCuProxy(undefined); } @@ -1417,12 +1412,15 @@ export async function handleSendMessage( new HostAppControlProxy(mapping.conversationId), ); } - if (!conversation.isProcessing()) { - conversation.addPreactivatedSkillId("app-control"); - } } else if (!conversation.isProcessing()) { conversation.setHostAppControlProxy(undefined); } + // Only preactivate when the conversation is idle — if it's processing, + // this message will be queued and preactivation is deferred to dequeue + // time in drainQueueImpl to avoid mutating in-flight turn state. + if (!conversation.isProcessing()) { + preactivateHostProxySkills(conversation, sourceInterface); + } // Wire sendToClient to the SSE hub so all subsystems can reach the HTTP client. // hasNoClient must remain `!isInteractive` so downstream tool gating // (`isToolActiveForContext` for HOST_TOOL_NAMES, `createToolExecutor`'s