Skip to content
111 changes: 86 additions & 25 deletions assistant/src/daemon/__tests__/conversation-tool-setup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,28 @@
* host_file_* are filtered out for chrome-extension regardless of the
* hasNoClient flag.
*
* Cross-client exception (Phase 1): host_bash is allowed for non-host-proxy
* interfaces (e.g. "web") when at least one host_bash-capable client is
* connected via the event hub. host_file_* and host_browser remain filtered
* regardless (Phase 2).
* Cross-client exception: tools whose capabilities are in
* CROSS_CLIENT_EXPOSED_CAPABILITIES (host_bash, host_file) are allowed for
* non-host-proxy interfaces (e.g. "web") when at least one capable client
* is connected via the event hub. host_browser is excluded (chrome-extension
* is its own executor; web turns have no CDP target model).
*/

import { beforeEach, describe, expect, mock, test } from "bun:test";

// ── Module-level mocks ─────────────────────────────────────────────

// Control how many host_bash-capable clients the hub reports.
let mockHostBashClientCount = 0;
// Control how many capable clients the hub reports per capability.
const mockClientCountByCapability = new Map<string, number>();

mock.module("../../runtime/assistant-event-hub.js", () => ({
assistantEventHub: {
listClientsByCapability: (cap: string) => {
if (cap === "host_bash") {
return Array.from({ length: mockHostBashClientCount }, (_, i) => ({
clientId: `mock-client-${i}`,
capabilities: ["host_bash"],
}));
}
return [];
const count = mockClientCountByCapability.get(cap) ?? 0;
return Array.from({ length: count }, (_, i) => ({
clientId: `mock-${cap}-client-${i}`,
capabilities: [cap],
}));
},
},
broadcastMessage: () => {},
Expand Down Expand Up @@ -72,7 +71,7 @@ function makeCtx(
}

beforeEach(() => {
mockHostBashClientCount = 0;
mockClientCountByCapability.clear();
});

describe("isToolActiveForContext — host tool capability gating", () => {
Expand Down Expand Up @@ -213,7 +212,7 @@ describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)
test("host_bash is active for web transport when a host_bash-capable client is connected", () => {
// Cross-client path: a web turn should see host_bash when a macOS client
// with host_bash capability is connected via the event hub.
mockHostBashClientCount = 1;
mockClientCountByCapability.set("host_bash", 1);
expect(
isToolActiveForContext(
"host_bash",
Expand All @@ -224,7 +223,7 @@ describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)

test("host_bash is NOT active for web transport when no capable client is connected", () => {
// No cross-client fallback: hub has no host_bash-capable subscribers.
mockHostBashClientCount = 0;
mockClientCountByCapability.set("host_bash", 0);
expect(
isToolActiveForContext(
"host_bash",
Expand All @@ -233,11 +232,11 @@ describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)
).toBe(false);
});

test("host_file_read is NOT active for web transport even when a capable client is connected (Phase 2 gate)", () => {
// The cross-client exception is scoped to host_bash only.
// host_file_* remain filtered for non-host-proxy interfaces regardless
// of connected clients until Phase 2 lands.
mockHostBashClientCount = 1;
test("host_file_read is NOT active for web transport when only a host_bash client is connected", () => {
// The cross-client exception is per-capability: a host_bash-capable
// client in the hub does not satisfy host_file's exposure check, since
// listClientsByCapability is queried with the tool's actual capability.
mockClientCountByCapability.set("host_bash", 1);
expect(
isToolActiveForContext(
"host_file_read",
Expand All @@ -249,7 +248,7 @@ describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)
test("host_bash for macos transport is unaffected by the cross-client exception", () => {
// macos natively supports host_bash via host proxy — the supportsHostProxy
// check passes, so the cross-client branch is never reached.
mockHostBashClientCount = 0;
mockClientCountByCapability.set("host_bash", 0);
expect(
isToolActiveForContext(
"host_bash",
Expand All @@ -262,7 +261,7 @@ describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)
// Even with a capable client in the hub, the macos SSE path takes
// precedence — it passes the supportsHostProxy check, bypasses the
// cross-client branch, and reaches the hasNoClient gate.
mockHostBashClientCount = 1;
mockClientCountByCapability.set("host_bash", 1);
expect(
isToolActiveForContext(
"host_bash",
Expand All @@ -275,7 +274,7 @@ describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)
// Security boundary: chrome-extension only gets host_browser. The
// cross-client exception explicitly excludes chrome-extension transport
// regardless of how many host_bash-capable clients are in the hub.
mockHostBashClientCount = 1;
mockClientCountByCapability.set("host_bash", 1);
expect(
isToolActiveForContext(
"host_bash",
Expand All @@ -287,7 +286,7 @@ describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)
test("host_bash is NOT active for web transport when hasNoClient is true (no approval UI)", () => {
// hasNoClient gate: no interactive approval UI available for this turn.
// Cross-client exception must not bypass this gate.
mockHostBashClientCount = 1;
mockClientCountByCapability.set("host_bash", 1);
expect(
isToolActiveForContext(
"host_bash",
Expand All @@ -297,6 +296,68 @@ describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)
});
});

describe("isToolActiveForContext — cross-client exposure for host_file_*", () => {
const HOST_FILE_TOOLS = [
"host_file_read",
"host_file_write",
"host_file_edit",
"host_file_transfer",
] as const;

for (const tool of HOST_FILE_TOOLS) {
test(`${tool} is exposed for web transport when a host_file client is connected`, () => {
mockClientCountByCapability.set("host_file", 1);
expect(
isToolActiveForContext(
tool,
makeCtx({ hasNoClient: false, transportInterface: "web" }),
),
).toBe(true);
});

test(`${tool} is NOT exposed for web when no host_file client is connected`, () => {
mockClientCountByCapability.set("host_file", 0);
expect(
isToolActiveForContext(
tool,
makeCtx({ hasNoClient: false, transportInterface: "web" }),
),
).toBe(false);
});

test(`${tool} is NOT exposed for chrome-extension (security boundary)`, () => {
mockClientCountByCapability.set("host_file", 1);
expect(
isToolActiveForContext(
tool,
makeCtx({ hasNoClient: true, transportInterface: "chrome-extension" }),
),
).toBe(false);
});

test(`${tool} is NOT exposed when hasNoClient is true (no approval UI)`, () => {
mockClientCountByCapability.set("host_file", 1);
expect(
isToolActiveForContext(
tool,
makeCtx({ hasNoClient: true, transportInterface: "web" }),
),
).toBe(false);
});
}

test("listClientsByCapability is queried with the actual capability, not host_bash (regression guard for D5 latent bug)", () => {
mockClientCountByCapability.set("host_bash", 0);
mockClientCountByCapability.set("host_file", 1);
expect(
isToolActiveForContext(
"host_file_transfer",
makeCtx({ hasNoClient: false, transportInterface: "web" }),
),
).toBe(true);
});
});

describe("HOST_TOOL_NAMES derivation", () => {
test("HOST_TOOL_NAMES is derived from HOST_TOOL_TO_CAPABILITY", () => {
// Sanity check: every tool in the names set has a capability mapping.
Expand Down
43 changes: 36 additions & 7 deletions assistant/src/daemon/conversation-tool-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,29 @@ export const HOST_TOOL_TO_CAPABILITY = new Map<string, HostProxyCapability>([
// Derived from HOST_TOOL_TO_CAPABILITY so the invariant "every host tool has
// a capability mapping" is a structural fact — no runtime assertion needed.
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).
* 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
* proxy's targetClientId path.
*
* Inclusions:
* - host_bash (Phase 1, PR #29322)
* - host_file (Phases 2 & 3, PRs #29398 + #29440)
*
* Exclusions:
* - host_browser: chrome-extension is its own executor; web turns don't
* have a CDP target model. Re-evaluate when host browser via macOS
* host proxy ships (PR #27489).
* - host_app_control, host_cu: not in HOST_TOOL_TO_CAPABILITY
* (skill-routed).
*/
const CROSS_CLIENT_EXPOSED_CAPABILITIES = new Set<HostProxyCapability>([
"host_bash",
"host_file",
Comment thread
credence-the-bot[bot] marked this conversation as resolved.
]);
const CLIENT_CAPABILITY_TOOL_NAMES = new Set(["app_open"]);
const PLATFORM_TOOL_NAMES = new Set(["request_system_permission"]);

Expand Down Expand Up @@ -384,16 +407,22 @@ export function isToolActiveForContext(
// Per-capability check is authoritative for structural support: if the
// transport cannot service this capability, the tool is filtered out.
if (transport && capability && !supportsHostProxy(transport, capability)) {
// Cross-client exception: allow host_bash for non-host-proxy interfaces when
// at least one capable client is connected via the event hub.
// Only applies to host_bash (not host_file, host_cu, host_browser — Phase 2).
// Excludes chrome-extension (security boundary: extension only gets host_browser)
// and hasNoClient turns (no interactive approval UI available).
// 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
// 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
// own executor and web turns don't have a CDP target model).
// chrome-extension transport is excluded as a security boundary
// (extension only gets host_browser); hasNoClient turns are excluded
// (no interactive approval UI available).
if (
capability === "host_bash" &&
capability &&
CROSS_CLIENT_EXPOSED_CAPABILITIES.has(capability) &&
transport !== "chrome-extension" &&
!ctx.hasNoClient &&
assistantEventHub.listClientsByCapability("host_bash").length > 0
assistantEventHub.listClientsByCapability(capability).length > 0
) {
return true;
}
Expand Down
130 changes: 130 additions & 0 deletions assistant/src/tools/host-filesystem/edit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import { mkdtempSync, realpathSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, mock, test } from "bun:test";

import type { ToolContext } from "../types.js";

// ---------------------------------------------------------------------------
// Singleton mocks — must precede the tool import so bun's module mock applies.
// ---------------------------------------------------------------------------

let mockProxyAvailable = false;

mock.module("../../daemon/host-file-proxy.js", () => ({
HostFileProxy: {
get instance() {
return {
isAvailable: () => mockProxyAvailable,
request: () => Promise.resolve({ content: "ok", isError: false }),
};
},
},
}));

mock.module("../../runtime/assistant-event-hub.js", () => ({
assistantEventHub: {
listClientsByCapability: () => [],
},
}));

const { hostFileEditTool } = await import("./edit.js");

const testDirs: string[] = [];

afterEach(() => {
mockProxyAvailable = false;
for (const dir of testDirs.splice(0)) {
rmSync(dir, { recursive: true, force: true });
}
});

function makeTempDir(): string {
const dir = realpathSync(mkdtempSync(join(tmpdir(), "host-edit-test-")));
testDirs.push(dir);
return dir;
}

function makeContext(
workingDir: string,
transportInterface: ToolContext["transportInterface"],
): ToolContext {
return {
workingDir,
conversationId: "test-conv",
trustClass: "guardian",
transportInterface,
};
}

describe("host_file_edit cross-client guards", () => {
test("returns 'no client' error on web transport when proxy unavailable and no targetClientId", async () => {
const workingDir = makeTempDir();
const result = await hostFileEditTool.execute(
{
path: "/some/host/path.txt",
old_string: "foo",
new_string: "bar",
},
makeContext(workingDir, "web"),
);
expect(result.isError).toBe(true);
expect(result.content).toContain(
"no client with host_file capability is connected",
);
});

test("returns 'specified client disconnected' error when targetClientId set but proxy unavailable on web", async () => {
const workingDir = makeTempDir();
const result = await hostFileEditTool.execute(
{
path: "/some/host/path.txt",
old_string: "foo",
new_string: "bar",
target_client_id: "abc-123",
},
makeContext(workingDir, "web"),
);
expect(result.isError).toBe(true);
expect(result.content).toContain(
'target client "abc-123" is no longer connected',
);
});

test("falls through to local fs on macos transport when proxy unavailable", async () => {
const workingDir = makeTempDir();
const result = await hostFileEditTool.execute(
{
path: "/nonexistent/x.txt",
old_string: "foo",
new_string: "bar",
},
makeContext(workingDir, "macos"),
);
// Proves the guard did NOT fire on macOS — instead we got the
// local FileSystemOps error path (file not found / IO error).
expect(result.isError).toBe(true);
expect(result.content.toLowerCase()).toMatch(/not found|enoent|editing/);
});

test("does NOT reject on macos transport with a stale target_client_id when proxy unavailable (regression: P2 fix)", async () => {
const workingDir = makeTempDir();
const result = await hostFileEditTool.execute(
{
path: "/nonexistent/x.txt",
old_string: "foo",
new_string: "bar",
target_client_id: "stale-mac",
},
makeContext(workingDir, "macos"),
);
// The disconnected-target guard is scoped to non-host-proxy transports
// (!supportsHostProxy). On macos, a stale target_client_id auto-filled
// from a prior cross-client turn must be silently ignored and the call
// must fall through to local FileSystemOps, NOT reject with "target
// client ... is no longer connected".
expect(result.isError).toBe(true);
expect(result.content).not.toContain("is no longer connected");
expect(result.content.toLowerCase()).toMatch(/not found|enoent|editing/);
});
});
Loading
Loading