Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions assistant/src/__tests__/conversation-confirmation-signals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ mock.module("../memory/canonical-guardian-store.js", () => ({
// ---------------------------------------------------------------------------

import { Conversation } from "../daemon/conversation.js";
import { HostBashProxy } from "../daemon/host-bash-proxy.js";
import { HostBrowserProxy } from "../daemon/host-browser-proxy.js";

// ---------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -558,3 +560,77 @@ describe("sendToClient receives state signals", () => {
});
});
});

describe("restoreBrowserProxyAvailability", () => {
test("re-enables only the host browser proxy after clearProxyAvailability", () => {
const conversation = makeConversation();
const browserProxy = new HostBrowserProxy(() => {});
const bashProxy = new HostBashProxy(() => {});
conversation.setHostBrowserProxy(browserProxy);
conversation.setHostBashProxy(bashProxy);

// Mark as having a connected client (interactive desktop path).
conversation.updateClient(() => {}, false);
expect(browserProxy.isAvailable()).toBe(true);
expect(bashProxy.isAvailable()).toBe(true);

// The drain queue clears all proxies for non-interactive turns.
conversation.clearProxyAvailability();
expect(browserProxy.isAvailable()).toBe(false);
expect(bashProxy.isAvailable()).toBe(false);

// restoreBrowserProxyAvailability should bring back ONLY the browser proxy.
conversation.restoreBrowserProxyAvailability();
expect(browserProxy.isAvailable()).toBe(true);
expect(bashProxy.isAvailable()).toBe(false);
});

test("re-enables the browser proxy even when hasNoClient is true (chrome-extension)", () => {
// Regression: chrome-extension is non-interactive (hasNoClient stays
// true so host_bash/host_file tools remain gated), but we still need
// to provision the hostBrowserProxy so it can service CDP commands.
// The helper must NOT gate on hasNoClient.
const conversation = makeConversation();
const browserProxy = new HostBrowserProxy(() => {});
conversation.setHostBrowserProxy(browserProxy);

// updateClient with hasNoClient=true emulates the non-interactive
// chrome-extension turn. Host proxies start disabled because
// updateClient propagates hasNoClient through to updateSender.
conversation.updateClient(() => {}, true);
expect(browserProxy.isAvailable()).toBe(false);
expect(conversation["hasNoClient"]).toBe(true);

// The targeted helper bypasses the hasNoClient gate so the
// single-capability chrome-extension turn can drive the browser
// via CDP without flipping hasNoClient (which would also enable
// host_bash/host_file gating downstream).
conversation.restoreBrowserProxyAvailability();
expect(browserProxy.isAvailable()).toBe(true);
// hasNoClient itself MUST remain true so that
// isToolActiveForContext keeps host_bash/host_file/host_cu gated.
expect(conversation["hasNoClient"]).toBe(true);
});

test("leaves bash/file/cu proxies disabled when called for chrome-extension", () => {
// Regression: the targeted helper must not accidentally re-enable
// proxies other than host_browser, even when called from a path that
// owns multiple proxies (e.g. macOS holdover state with hasNoClient
// forced true for an explicit non-interactive run).
const conversation = makeConversation();
const browserProxy = new HostBrowserProxy(() => {});
const bashProxy = new HostBashProxy(() => {});
conversation.setHostBrowserProxy(browserProxy);
conversation.setHostBashProxy(bashProxy);

conversation.updateClient(() => {}, true);
expect(browserProxy.isAvailable()).toBe(false);
expect(bashProxy.isAvailable()).toBe(false);

conversation.restoreBrowserProxyAvailability();
expect(browserProxy.isAvailable()).toBe(true);
// Crucial: bash proxy stays disabled. The helper must touch ONLY the
// browser proxy.
expect(bashProxy.isAvailable()).toBe(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ function createFakeConversation(conversationId: string): Conversation {
setHostCuProxy(this: { hostCuProxy: unknown }, proxy: unknown) {
this.hostCuProxy = proxy;
},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
hasAnyPendingConfirmation: () => false,
hasPendingConfirmation: () => false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ describe("handleSendMessage canonical guardian reply interception", () => {
setHostBrowserProxy: () => {},
setHostFileProxy: () => {},
setHostCuProxy: () => {},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
} as unknown as import("../daemon/conversation.js").Conversation;

Expand Down Expand Up @@ -251,6 +252,7 @@ describe("handleSendMessage canonical guardian reply interception", () => {
setHostBrowserProxy: () => {},
setHostFileProxy: () => {},
setHostCuProxy: () => {},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
} as unknown as import("../daemon/conversation.js").Conversation;

Expand Down Expand Up @@ -325,6 +327,7 @@ describe("handleSendMessage canonical guardian reply interception", () => {
setHostBrowserProxy: () => {},
setHostFileProxy: () => {},
setHostCuProxy: () => {},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
} as unknown as import("../daemon/conversation.js").Conversation;

Expand Down Expand Up @@ -403,6 +406,7 @@ describe("handleSendMessage canonical guardian reply interception", () => {
setHostBrowserProxy: () => {},
setHostFileProxy: () => {},
setHostCuProxy: () => {},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
} as unknown as import("../daemon/conversation.js").Conversation;

Expand Down Expand Up @@ -477,6 +481,7 @@ describe("handleSendMessage canonical guardian reply interception", () => {
setHostBrowserProxy: () => {},
setHostFileProxy: () => {},
setHostCuProxy: () => {},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
} as unknown as import("../daemon/conversation.js").Conversation;

Expand Down Expand Up @@ -545,6 +550,7 @@ describe("handleSendMessage canonical guardian reply interception", () => {
setHostBrowserProxy: () => {},
setHostFileProxy: () => {},
setHostCuProxy: () => {},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
} as unknown as import("../daemon/conversation.js").Conversation;

Expand Down Expand Up @@ -615,6 +621,7 @@ describe("handleSendMessage canonical guardian reply interception", () => {
setHostBrowserProxy: () => {},
setHostFileProxy: () => {},
setHostCuProxy: () => {},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
} as unknown as import("../daemon/conversation.js").Conversation;

Expand Down Expand Up @@ -686,6 +693,7 @@ describe("handleSendMessage canonical guardian reply interception", () => {
setHostBrowserProxy: () => {},
setHostFileProxy: () => {},
setHostCuProxy: () => {},
restoreBrowserProxyAvailability: () => {},
addPreactivatedSkillId: () => {},
} as unknown as import("../daemon/conversation.js").Conversation;

Expand Down
2 changes: 2 additions & 0 deletions assistant/src/__tests__/gateway-only-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ const ALLOWLIST = new Set([
// --- Chrome extension (local relay communication, not gateway API consumption) ---
"clients/chrome-extension/background/worker.ts",
"clients/chrome-extension/popup/popup.ts",
// --- Chrome extension native messaging helper (local daemon pair endpoint, by design) ---
"clients/chrome-extension-native-host/src/index.ts",

// --- Documentation and comments that mention the port for explanatory purposes ---
"AGENTS.md", // documents the gateway-only rule itself
Expand Down
134 changes: 134 additions & 0 deletions assistant/src/channels/__tests__/types.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import { describe, expect, test } from "bun:test";

import {
INTERACTIVE_INTERFACES,
INTERFACE_IDS,
isInterfaceId,
supportsHostProxy,
} from "../types.js";

describe("INTERFACE_IDS", () => {
test("includes chrome-extension", () => {
expect(
(INTERFACE_IDS as readonly string[]).includes("chrome-extension"),
).toBe(true);
});

test("still includes macos and other existing interfaces", () => {
for (const id of [
"macos",
"ios",
"cli",
"telegram",
"phone",
"vellum",
"whatsapp",
"slack",
"email",
]) {
expect((INTERFACE_IDS as readonly string[]).includes(id)).toBe(true);
}
});
});

describe("INTERACTIVE_INTERFACES", () => {
test("does NOT include chrome-extension", () => {
// Chrome extensions don't render SSE-backed prompter UI, so they must
// stay out of the interactive set even though they have an InterfaceId.
expect(INTERACTIVE_INTERFACES.has("chrome-extension" as never)).toBe(false);
});

test("still includes macos", () => {
expect(INTERACTIVE_INTERFACES.has("macos")).toBe(true);
});
});

describe("isInterfaceId", () => {
test("returns true for chrome-extension", () => {
expect(isInterfaceId("chrome-extension")).toBe(true);
});

test("returns true for macos", () => {
expect(isInterfaceId("macos")).toBe(true);
});

test("returns false for unknown interface", () => {
expect(isInterfaceId("safari-extension")).toBe(false);
});
});

describe("supportsHostProxy", () => {
// ── macOS: supports every capability, and the no-arg form returns true. ──
test("macos returns true (no capability)", () => {
expect(supportsHostProxy("macos")).toBe(true);
});

test("macos returns true for host_bash", () => {
expect(supportsHostProxy("macos", "host_bash")).toBe(true);
});

test("macos returns true for host_file", () => {
expect(supportsHostProxy("macos", "host_file")).toBe(true);
});

test("macos returns true for host_cu", () => {
expect(supportsHostProxy("macos", "host_cu")).toBe(true);
});

test("macos returns true for host_browser", () => {
expect(supportsHostProxy("macos", "host_browser")).toBe(true);
});

// ── chrome-extension: only host_browser. ──
test("chrome-extension returns false (no capability)", () => {
// Chrome extension does not support "any host proxy at all" — it only
// supports host_browser, so the no-arg form must return false to keep
// existing call sites that guard desktop-only behavior unchanged.
expect(supportsHostProxy("chrome-extension")).toBe(false);
});

test("chrome-extension returns true for host_browser", () => {
expect(supportsHostProxy("chrome-extension", "host_browser")).toBe(true);
});

test("chrome-extension returns false for host_bash", () => {
expect(supportsHostProxy("chrome-extension", "host_bash")).toBe(false);
});

test("chrome-extension returns false for host_file", () => {
expect(supportsHostProxy("chrome-extension", "host_file")).toBe(false);
});

test("chrome-extension returns false for host_cu", () => {
expect(supportsHostProxy("chrome-extension", "host_cu")).toBe(false);
});

// ── Non-supporting interfaces: false in all forms. ──
test("cli returns false (no capability)", () => {
expect(supportsHostProxy("cli")).toBe(false);
});

test("cli returns false for host_bash", () => {
expect(supportsHostProxy("cli", "host_bash")).toBe(false);
});

test("cli returns false for host_browser", () => {
expect(supportsHostProxy("cli", "host_browser")).toBe(false);
});

test("telegram returns false (no capability)", () => {
expect(supportsHostProxy("telegram")).toBe(false);
});

test("telegram returns false for host_browser", () => {
expect(supportsHostProxy("telegram", "host_browser")).toBe(false);
});

test("vellum returns false (no capability)", () => {
expect(supportsHostProxy("vellum")).toBe(false);
});

test("email returns false for host_browser", () => {
expect(supportsHostProxy("email", "host_browser")).toBe(false);
});
});
35 changes: 32 additions & 3 deletions assistant/src/channels/types.ts
Comment thread
noanflaherty marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const INTERFACE_IDS = [
"whatsapp",
"slack",
"email",
"chrome-extension",
] as const;

export type InterfaceId = (typeof INTERFACE_IDS)[number];
Expand Down Expand Up @@ -90,9 +91,37 @@ export function isInteractiveInterface(id: InterfaceId): boolean {
return INTERACTIVE_INTERFACES.has(id);
}

/** Whether the interface supports host proxies (bash, file, computer-use). */
export function supportsHostProxy(id: InterfaceId): boolean {
return id === "macos";
/**
* Host proxy capabilities that an interface can support. The macOS client
* supports all four; the chrome-extension interface only supports
* host_browser (via the Chrome DevTools Protocol proxy).
*/
export type HostProxyCapability =
| "host_bash"
| "host_file"
| "host_cu"
| "host_browser";

/**
* Whether the interface supports a host proxy capability.
*
* The no-arg form `supportsHostProxy(id)` asks "does this interface support
* the full desktop host proxy set?" — it returns `true` only for macOS, which
* supports all four capabilities. It returns `false` for
* chrome-extension because chrome-extension only supports `host_browser`,
* and the no-arg form is the gate that legacy desktop-only call sites use
* (e.g. preactivating computer-use, restoring all four proxies in the drain
* queue). Callers that want to check a single capability — for example, to
* decide whether to keep `hostBrowserProxy` available for chrome-extension —
* should pass the capability explicitly: `supportsHostProxy(id, "host_browser")`.
*/
export function supportsHostProxy(
id: InterfaceId,
capability?: HostProxyCapability,
): boolean {
if (id === "macos") return true;
if (id === "chrome-extension" && capability === "host_browser") return true;
return false;
Comment thread
noanflaherty marked this conversation as resolved.
}

export interface TurnInterfaceContext {
Expand Down
21 changes: 20 additions & 1 deletion assistant/src/daemon/conversation-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ export interface ProcessConversationContext {
clearProxyAvailability(): void;
/** Restore host proxy availability based on whether a real client is connected. */
restoreProxyAvailability(): void;
/** Restore only the host browser proxy (used by chrome-extension drains). */
restoreBrowserProxyAvailability(): void;
emitActivityState(
phase:
| "idle"
Expand Down Expand Up @@ -311,10 +313,27 @@ export async function drainQueue(
// returns false and tool execution falls back to local.
if (next.isInteractive === false) {
conversation.clearProxyAvailability();
// chrome-extension is non-interactive (no SSE prompter UI) but DOES have
// a connected client that can service host_browser_request events. The
// unconditional clear above turned its hostBrowserProxy off; restore it
// here so the queued turn can still drive the browser via CDP.
const drainInterfaceCtx =
queuedInterfaceCtx ?? conversation.getTurnInterfaceContext();
const drainInterface = drainInterfaceCtx?.userMessageInterface;
if (
drainInterface &&
!supportsHostProxy(drainInterface) &&
supportsHostProxy(drainInterface, "host_browser")
) {
conversation.restoreBrowserProxyAvailability();
}
} else {
// Restore proxy availability only for desktop-originating turns (macos)
// in case a prior non-interactive drain disabled it. Non-desktop interactive
// interfaces (CLI, Vellum) should not re-enable desktop host proxies.
// interfaces (CLI, Vellum) should not re-enable desktop host proxies. The
// chrome-extension interface only supports host_browser, not the desktop
// proxies or computer-use, so it is excluded by the no-arg form of
// supportsHostProxy (which returns false for chrome-extension).
const interfaceCtx =
queuedInterfaceCtx ?? conversation.getTurnInterfaceContext();
const sourceInterface = interfaceCtx?.userMessageInterface;
Comment thread
noanflaherty marked this conversation as resolved.
Expand Down
Loading
Loading