From ed58b3f139d502de19176192a0ed1a927fec883a Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" Date: Tue, 5 May 2026 07:47:49 +0000 Subject: [PATCH] fix(tool-projection): self-review cleanup for host_file_* exposure Phase 4 round 1: addresses minor integration-review gaps from the self-review pass on PR #29613. - Replace stale "iphone" references with the actual InterfaceId "ios" in 6 comment locations (conversation-tool-setup.ts, the 4 host_file_* executors). - Add an explanatory note in each host_file_* executor's disconnected-target guard pointing out the deliberate divergence from host_bash (host-shell.ts:239-247) and linking to the PR #29613 review for rationale, so future readers understand the asymmetry is intentional. - Align transfer.ts's "no client connected" guard message with the read/write/edit pattern by referring to "host_file" (the capability) rather than "host_file_transfer" (the tool name). - Add a stub mock for assistant-event-hub.js in transfer.test.ts so the multi-client guard test runs against an isolated stub rather than the live process-wide singleton, matching the read/write/edit test pattern. Co-Authored-By: Claude Opus 4.7 --- assistant/src/daemon/conversation-tool-setup.ts | 4 ++-- assistant/src/tools/host-filesystem/edit.ts | 5 ++++- assistant/src/tools/host-filesystem/read.ts | 5 ++++- assistant/src/tools/host-filesystem/transfer.test.ts | 9 +++++++++ assistant/src/tools/host-filesystem/transfer.ts | 7 +++++-- assistant/src/tools/host-filesystem/write.ts | 5 ++++- 6 files changed, 28 insertions(+), 7 deletions(-) diff --git a/assistant/src/daemon/conversation-tool-setup.ts b/assistant/src/daemon/conversation-tool-setup.ts index df13437aec5..208e6d3e041 100644 --- a/assistant/src/daemon/conversation-tool-setup.ts +++ b/assistant/src/daemon/conversation-tool-setup.ts @@ -352,7 +352,7 @@ export const HOST_TOOL_TO_CAPABILITY = new Map([ export const HOST_TOOL_NAMES = new Set(HOST_TOOL_TO_CAPABILITY.keys()); /** * Capabilities eligible for cross-client exposure on non-host-proxy - * transports (e.g. web, iphone routing to a connected macOS client). + * transports (e.g. web, ios routing to a connected macOS client). * Adding a capability here exposes ALL tools that map to it (per * HOST_TOOL_TO_CAPABILITY) on non-host-proxy transports — the daemon then * routes the actual invocation to the connected capable client via the @@ -409,7 +409,7 @@ export function isToolActiveForContext( if (transport && capability && !supportsHostProxy(transport, capability)) { // Cross-client exception: allow host tools whose capabilities have // cross-client routing infrastructure (Phases 1–3) to be exposed for - // non-host-proxy transports (e.g. "web", "iphone") when at least one + // non-host-proxy transports (e.g. "web", "ios") when at least one // capable client is connected via the event hub. Members of // CROSS_CLIENT_EXPOSED_CAPABILITIES (host_bash, host_file) qualify; // host_browser is intentionally excluded (chrome-extension is its diff --git a/assistant/src/tools/host-filesystem/edit.ts b/assistant/src/tools/host-filesystem/edit.ts index fc3f7240a40..e6093c6b4b1 100644 --- a/assistant/src/tools/host-filesystem/edit.ts +++ b/assistant/src/tools/host-filesystem/edit.ts @@ -131,8 +131,11 @@ class HostFileEditTool implements Tool { // and tool-execution). Scoped to !supportsHostProxy so macos turns — // where local-fs fallback IS the intended offline behavior — still fall // through if the LLM auto-fills a stale target_client_id from a prior - // cross-client turn. On web/iphone, the call must fail loudly rather + // cross-client turn. On web/ios, the call must fail loudly rather // than silently target the daemon container's filesystem. + // Note: this scoping deliberately differs from host_bash + // (host-shell.ts:239-247), which rejects unconditionally; see PR #29613 + // review discussion for rationale. if ( targetClientId != null && transportInterface != null && diff --git a/assistant/src/tools/host-filesystem/read.ts b/assistant/src/tools/host-filesystem/read.ts index ef7e96f3542..bc6c783c89e 100644 --- a/assistant/src/tools/host-filesystem/read.ts +++ b/assistant/src/tools/host-filesystem/read.ts @@ -102,8 +102,11 @@ class HostFileReadTool implements Tool { // and tool-execution). Scoped to !supportsHostProxy so macos turns — // where local-fs fallback IS the intended offline behavior — still fall // through if the LLM auto-fills a stale target_client_id from a prior - // cross-client turn. On web/iphone, the call must fail loudly rather + // cross-client turn. On web/ios, the call must fail loudly rather // than silently target the daemon container's filesystem. + // Note: this scoping deliberately differs from host_bash + // (host-shell.ts:239-247), which rejects unconditionally; see PR #29613 + // review discussion for rationale. if ( targetClientId != null && transportInterface != null && diff --git a/assistant/src/tools/host-filesystem/transfer.test.ts b/assistant/src/tools/host-filesystem/transfer.test.ts index a5e83140fdf..a2a6cf8e958 100644 --- a/assistant/src/tools/host-filesystem/transfer.test.ts +++ b/assistant/src/tools/host-filesystem/transfer.test.ts @@ -31,6 +31,15 @@ mock.module("../../daemon/host-transfer-proxy.js", () => ({ }, })); +// Mirror read/write/edit test files: stub the event hub so the multi-client +// guard at line ~100 of transfer.ts is exercised against an isolated stub +// rather than the live process-wide singleton. +mock.module("../../runtime/assistant-event-hub.js", () => ({ + assistantEventHub: { + listClientsByCapability: () => [], + }, +})); + const { hostFileTransferTool } = await import("./transfer.js"); const testDirs: string[] = []; diff --git a/assistant/src/tools/host-filesystem/transfer.ts b/assistant/src/tools/host-filesystem/transfer.ts index b263a1221ab..09c9f41cb60 100644 --- a/assistant/src/tools/host-filesystem/transfer.ts +++ b/assistant/src/tools/host-filesystem/transfer.ts @@ -119,7 +119,7 @@ class HostFileTransferTool implements Tool { ) { return { content: - "Error: no client with host_file capability is connected. Connect a macOS client to use host_file_transfer from a non-desktop interface.", + "Error: no client with host_file capability is connected. Connect a macOS client to use host_file from a non-desktop interface.", isError: true, }; } @@ -129,8 +129,11 @@ class HostFileTransferTool implements Tool { // and tool-execution). Scoped to !supportsHostProxy so macos turns — // where local-fs fallback IS the intended offline behavior — silently // ignore a stale target_client_id auto-filled from a prior cross-client - // turn. On web/iphone, the call must fail loudly rather than silently + // turn. On web/ios, the call must fail loudly rather than silently // target the daemon container's filesystem. + // Note: this scoping deliberately differs from host_bash + // (host-shell.ts:239-247), which rejects unconditionally; see PR #29613 + // review discussion for rationale. if ( targetClientId != null && context.transportInterface != null && diff --git a/assistant/src/tools/host-filesystem/write.ts b/assistant/src/tools/host-filesystem/write.ts index defbb969e71..bdb205d19c5 100644 --- a/assistant/src/tools/host-filesystem/write.ts +++ b/assistant/src/tools/host-filesystem/write.ts @@ -101,8 +101,11 @@ class HostFileWriteTool implements Tool { // and tool-execution). Scoped to !supportsHostProxy so macos turns — // where local-fs fallback IS the intended offline behavior — still fall // through if the LLM auto-fills a stale target_client_id from a prior - // cross-client turn. On web/iphone, the call must fail loudly rather + // cross-client turn. On web/ios, the call must fail loudly rather // than silently target the daemon container's filesystem. + // Note: this scoping deliberately differs from host_bash + // (host-shell.ts:239-247), which rejects unconditionally; see PR #29613 + // review discussion for rationale. if ( targetClientId != null && transportInterface != null &&