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
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ import { readFileSync } from "node:fs";
import { resolve } from "node:path";
import { describe, expect, test } from "bun:test";

import {
BROWSER_OPERATION_META,
BROWSER_TOOL_NAMES,
} from "../browser/operations.js";
import { BROWSER_TOOL_NAMES } from "../browser/identifiers.js";
import { BROWSER_OPERATION_META } from "../browser/operations.js";
import { BROWSER_OPERATIONS } from "../browser/types.js";

// ── Helpers ──────────────────────────────────────────────────────────
Expand Down
2 changes: 1 addition & 1 deletion assistant/src/__tests__/browser-skill-endstate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mock.module("../config/loader.js", () => ({
getConfig: () => ({}),
}));

import { BROWSER_TOOL_NAMES } from "../browser/operations.js";
import { BROWSER_TOOL_NAMES } from "../browser/identifiers.js";
import { _setOverridesForTesting } from "../config/assistant-feature-flags.js";
import {
projectSkillTools,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BROWSER_TOOL_NAMES } from "../../browser/operations.js";
import { BROWSER_TOOL_NAMES } from "../../browser/identifiers.js";
import type { Message } from "../../providers/types.js";

// Re-export BROWSER_TOOL_NAMES from the shared browser operations contract
Expand Down
64 changes: 64 additions & 0 deletions assistant/src/browser/identifiers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* Lightweight browser identifier helpers.
*
* This module exports browser tool-name constants and bidirectional
* name-mapping functions without importing any runtime browser
* dependencies (browser-execution, browser-manager, browser-mode).
*
* Policy and classification modules (permissions/defaults,
* workspace-policy, side-effects) should import from here instead
* of from `operations.ts` to avoid pulling the browser execution
* stack into non-browser codepaths.
*/

import { BROWSER_OPERATIONS, type BrowserOperation } from "./types.js";

// ── Tool name constants ──────────────────────────────────────────────

/**
* All `browser_*` tool names derived from operation identifiers.
*
* These names are the LLM-facing tool aliases registered by the browser
* skill wrappers. They are compatibility adapters: the canonical
* identifiers are the operation names in {@link BROWSER_OPERATIONS},
* and the `browser_*` prefix is a naming convention for the tool layer.
*
* Consumed by:
* - Permission default rules (permissions/defaults.ts)
* - Workspace policy classification (permissions/workspace-policy.ts)
* - Side-effect tool classification (tools/side-effects.ts)
* - Test harnesses and parity guards
*/
export const BROWSER_TOOL_NAMES: readonly string[] = BROWSER_OPERATIONS.map(
(op) => `browser_${op}`,
);

// ── Bidirectional name mapping ───────────────────────────────────────

/**
* Convert a `browser_*` tool name to its canonical operation ID.
* Returns `undefined` if the tool name does not match a known operation.
*/
export function browserToolNameToOperation(
toolName: string,
): BrowserOperation | undefined {
if (!toolName.startsWith("browser_")) return undefined;
const candidate = toolName.slice("browser_".length);
if ((BROWSER_OPERATIONS as readonly string[]).includes(candidate)) {
return candidate as BrowserOperation;
}
return undefined;
}

/**
* Convert a canonical operation ID to its `browser_*` tool name.
* Returns `undefined` if the operation is not a known identifier.
*/
export function browserOperationToToolName(
operation: string,
): string | undefined {
if ((BROWSER_OPERATIONS as readonly string[]).includes(operation)) {
return `browser_${operation}`;
}
return undefined;
}
78 changes: 22 additions & 56 deletions assistant/src/browser/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@
* this module has no dependency on skill registration files.
*
* Responsibilities:
* - Canonical operation <-> tool name mapping (bijective).
* - Dispatch to existing browser-execution.ts implementations.
* - Command-oriented metadata for CLI subcommand generation.
* - `wait_for_download` mode-constraint enforcement.
*
* Identifier-only exports (BROWSER_TOOL_NAMES, name mapping helpers)
* live in `browser/identifiers.ts` to avoid pulling the browser
* execution stack into policy/classification modules. This module
* re-exports them for backwards compatibility.
*/

import {
Expand Down Expand Up @@ -40,65 +44,26 @@ import {
type BrowserOperationMeta,
} from "./types.js";

// ── Tool name constants ──────────────────────────────────────────────
// ── Re-exports from identifiers.ts ───────────────────────────────────
//
// Identifier-only constants and helpers are defined in the lightweight
// `browser/identifiers.ts` module so that policy/classification
// consumers can import them without pulling in the browser execution
// stack. Re-exported here for backwards compatibility with existing
// callers that import from `browser/operations.js`.

export {
BROWSER_TOOL_NAMES,
browserOperationToToolName,
browserToolNameToOperation,
} from "./identifiers.js";

/**
* All canonical browser operation identifiers (re-exported from types).
*/
export const BROWSER_OPERATION_NAMES: readonly BrowserOperation[] =
BROWSER_OPERATIONS;

/**
* All `browser_*` tool names derived from operation identifiers.
*
* These names are the LLM-facing tool aliases registered by the browser
* skill wrappers. They are compatibility adapters: the canonical
* identifiers are the operation names in {@link BROWSER_OPERATIONS},
* and the `browser_*` prefix is a naming convention for the tool layer.
* When the `browser_*` tool wrappers are eventually removed, this
* derived list can be dropped — the CLI and operations layer only need
* {@link BROWSER_OPERATIONS} and {@link BROWSER_OPERATION_META}.
*
* Consumed by:
* - Permission default rules (permissions/defaults.ts)
* - Workspace policy classification (permissions/workspace-policy.ts)
* - Side-effect tool classification (tools/side-effects.ts)
* - Test harnesses and parity guards
*/
export const BROWSER_TOOL_NAMES: readonly string[] = BROWSER_OPERATIONS.map(
(op) => `browser_${op}`,
);

// ── Bidirectional name mapping ───────────────────────────────────────

/**
* Convert a `browser_*` tool name to its canonical operation ID.
* Returns `undefined` if the tool name does not match a known operation.
*/
export function browserToolNameToOperation(
toolName: string,
): BrowserOperation | undefined {
if (!toolName.startsWith("browser_")) return undefined;
const candidate = toolName.slice("browser_".length);
if ((BROWSER_OPERATIONS as readonly string[]).includes(candidate)) {
return candidate as BrowserOperation;
}
return undefined;
}

/**
* Convert a canonical operation ID to its `browser_*` tool name.
* Returns `undefined` if the operation is not a known identifier.
*/
export function browserOperationToToolName(
operation: string,
): string | undefined {
if ((BROWSER_OPERATIONS as readonly string[]).includes(operation)) {
return `browser_${operation}`;
}
return undefined;
}

// ── Dispatch handlers ────────────────────────────────────────────────

/**
Expand Down Expand Up @@ -219,9 +184,10 @@ export async function executeBrowserOperation(
* Metadata for every browser operation, describing fields, types, and
* constraints. Used by the CLI command builder to generate subcommands.
*
* The `browser_mode` and `activity` fields are omitted from per-operation
* metadata because they are not exposed through the CLI. Operations
* invoked via the CLI always use the default browser mode.
* The `browser_mode` field is handled as a shared parent-level option
* on the `assistant browser` command (--browser-mode), not as a
* per-operation field. The `activity` field is omitted because it is
* an internal execution concern, not a user-facing parameter.
*/
export const BROWSER_OPERATION_META: readonly BrowserOperationMeta[] = [
{
Expand Down
31 changes: 31 additions & 0 deletions assistant/src/cli/commands/__tests__/browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,37 @@ describe("IPC payload mapping", () => {
await runCommand(["browser", "snapshot"]);
expect(lastIpcCall!.params!.sessionId).toBe("default");
});

test("--browser-mode injects browser_mode into input", async () => {
await runCommand([
"browser",
"--browser-mode",
"local",
"navigate",
"--url",
"http://localhost:3000",
]);
expect(lastIpcCall!.params!.operation).toBe("navigate");
const input = lastIpcCall!.params!.input as Record<string, unknown>;
expect(input.browser_mode).toBe("local");
expect(input.url).toBe("http://localhost:3000");
});

test("--browser-mode is omitted from input when not specified", async () => {
await runCommand(["browser", "snapshot"]);
const input = lastIpcCall!.params!.input as Record<string, unknown>;
expect(input.browser_mode).toBeUndefined();
});

test("--browser-mode rejects invalid modes", async () => {
const { exitCode } = await runCommand([
"browser",
"--browser-mode",
"invalid",
"snapshot",
]);
expect(exitCode).not.toBe(0);
});
});

// ---------------------------------------------------------------------------
Expand Down
41 changes: 37 additions & 4 deletions assistant/src/cli/commands/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,22 @@ function buildSubcommand(parent: Command, meta: BrowserOperationMeta): void {
const parentOpts = parent.opts() as {
session?: string;
json?: boolean;
browserMode?: string;
};
const sessionId = parentOpts.session ?? "default";
const jsonMode = parentOpts.json ?? false;

// Map Commander camelCase options back to snake_case input keys,
// filtering out parent-level options (session, json) and screenshot
// ergonomics (output).
// filtering out parent-level options (session, json, browserMode)
// and screenshot ergonomics (output).
const input: Record<string, unknown> = {};
const excludeKeys = new Set(["session", "json", "output"]);
const excludeKeys = new Set(["session", "json", "output", "browserMode"]);

// Inject parent-level --browser-mode into the operation input so
// the backend receives the mode override for backend pinning.
if (parentOpts.browserMode) {
input.browser_mode = parentOpts.browserMode;
}

for (const [key, value] of Object.entries(opts)) {
if (excludeKeys.has(key)) continue;
Expand Down Expand Up @@ -243,6 +250,20 @@ function buildSubcommand(parent: Command, meta: BrowserOperationMeta): void {

// ── Registration ─────────────────────────────────────────────────────

/**
* Valid browser mode values for the --browser-mode option.
* Includes canonical values and compatibility aliases accepted by
* `normalizeBrowserMode` (cdp-debugger → cdp-inspect, playwright → local).
*/
const BROWSER_MODES = [
"auto",
"extension",
"cdp-inspect",
"cdp-debugger",
"local",
"playwright",
] as const;

export function registerBrowserCommand(program: Command): void {
const browser = program
.command("browser")
Expand All @@ -252,7 +273,13 @@ export function registerBrowserCommand(program: Command): void {
"Session ID to preserve browser state across invocations.",
"default",
)
.option("--json", "Output results as machine-readable JSON.");
.option("--json", "Output results as machine-readable JSON.")
.addOption(
new Option(
"--browser-mode <mode>",
"Browser backend to use. Overrides automatic selection.",
).choices([...BROWSER_MODES]),
);

browser.addHelpText(
"after",
Expand All @@ -265,13 +292,19 @@ The --session flag groups sequential commands so they share browser
state (same page, cookies, etc.). Different session IDs create
independent browser contexts.

The --browser-mode flag pins the browser backend for all operations
in the invocation. Valid modes: auto (default), extension, cdp-inspect,
local. Useful for debugging or when deterministic backend selection
is required.

Examples:
$ assistant browser navigate --url https://example.com
$ assistant browser snapshot
$ assistant browser click --selector "#login"
$ assistant browser type --text "hello" --element-id e14
$ assistant browser screenshot --output page.jpg
$ assistant browser --session myflow navigate --url https://example.com
$ assistant browser --browser-mode local navigate --url http://localhost:3000
$ assistant browser --json screenshot`,
);

Expand Down
6 changes: 2 additions & 4 deletions assistant/src/config/bundled-skills/browser/tools/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@
* can be removed without changing the CLI or the operations layer.
*/

import {
browserToolNameToOperation,
executeBrowserOperation,
} from "../../../../browser/operations.js";
import { browserToolNameToOperation } from "../../../../browser/identifiers.js";
import { executeBrowserOperation } from "../../../../browser/operations.js";
import type {
ToolContext,
ToolExecutionResult,
Expand Down
2 changes: 1 addition & 1 deletion assistant/src/permissions/defaults.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { join } from "node:path";

import { BROWSER_TOOL_NAMES } from "../browser/operations.js";
import { BROWSER_TOOL_NAMES } from "../browser/identifiers.js";
import { getIsContainerized } from "../config/env-registry.js";
import { getConfig } from "../config/loader.js";
import { getBundledSkillsDir } from "../config/skills.js";
Expand Down
2 changes: 1 addition & 1 deletion assistant/src/permissions/workspace-policy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { realpathSync } from "node:fs";
import { basename, dirname, normalize, resolve } from "node:path";

import { BROWSER_TOOL_NAMES } from "../browser/operations.js";
import { BROWSER_TOOL_NAMES } from "../browser/identifiers.js";

/**
* Resolve a path to its canonical form. When the target itself doesn't
Expand Down
2 changes: 1 addition & 1 deletion assistant/src/tools/side-effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// Used by private-conversation gating and permission simulation to decide
// whether a tool invocation requires explicit approval.

import { BROWSER_TOOL_NAMES } from "../browser/operations.js";
import { BROWSER_TOOL_NAMES } from "../browser/identifiers.js";

/**
* Browser tools that are read-only / observational and do NOT have
Expand Down
Loading