refactor: centralize browser identifier sets across permissions and side-effects

assistant/src/__tests__/browser-identifier-parity-guard.test.ts

@@ -0,0 +1,80 @@
+/**
+ * Parity guard for browser identifier sets.
+ *
+ * Verifies that four independently-consumed browser identifier sources
+ * remain in sync:
+ *
+ *   1. Shared operation list        (BROWSER_OPERATIONS from browser/types.ts)
+ *   2. Shared browser_* tool names  (BROWSER_TOOL_NAMES from browser/operations.ts)
+ *   3. TOOLS.json tool names        (browser skill manifest)
+ *   4. CLI subcommand registrations (BROWSER_OPERATION_META from browser/operations.ts)
+ *
+ * Drift between any of these causes silent mismatches between the CLI,
+ * tool dispatch, permission defaults, and skill manifest. This guard
+ * catches additions or removals in one source that aren't mirrored in
+ * the others.
+ */
+
+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_OPERATIONS } from "../browser/types.js";
+
+// ── Helpers ──────────────────────────────────────────────────────────
+
+function sorted(arr: readonly string[]): string[] {
+  return [...arr].sort();
+}
+
+const TOOLS_JSON_PATH = resolve(
+  __dirname,
+  "../config/bundled-skills/browser/TOOLS.json",
+);
+
+function readToolsJsonNames(): string[] {
+  const raw = readFileSync(TOOLS_JSON_PATH, "utf-8");
+  const manifest = JSON.parse(raw) as {
+    tools: Array<{ name: string }>;
+  };
+  return manifest.tools.map((t) => t.name);
+}
+
+// ── Parity tests ─────────────────────────────────────────────────────
+
+describe("browser identifier parity guard", () => {
+  test("BROWSER_TOOL_NAMES matches BROWSER_OPERATIONS via browser_ prefix", () => {
+    const derivedToolNames = BROWSER_OPERATIONS.map((op) => `browser_${op}`);
+    expect(sorted(BROWSER_TOOL_NAMES)).toEqual(sorted(derivedToolNames));
+  });
+
+  test("TOOLS.json tool names match BROWSER_TOOL_NAMES", () => {
+    const toolsJsonNames = readToolsJsonNames();
+    expect(sorted(toolsJsonNames)).toEqual(sorted(BROWSER_TOOL_NAMES));
+  });
+
+  test("CLI subcommand operations match BROWSER_OPERATIONS", () => {
+    const metaOperations = BROWSER_OPERATION_META.map((m) => m.operation);
+    expect(sorted(metaOperations)).toEqual(sorted(BROWSER_OPERATIONS));
+  });
+
+  test("all four sources agree on the same count", () => {
+    const toolsJsonNames = readToolsJsonNames();
+    const metaOperations = BROWSER_OPERATION_META.map((m) => m.operation);
+
+    const counts = {
+      BROWSER_OPERATIONS: BROWSER_OPERATIONS.length,
+      BROWSER_TOOL_NAMES: BROWSER_TOOL_NAMES.length,
+      TOOLS_JSON: toolsJsonNames.length,
+      BROWSER_OPERATION_META: metaOperations.length,
+    };
+
+    // All counts must be identical.
+    const uniqueCounts = new Set(Object.values(counts));
+    expect(uniqueCounts.size).toBe(1);
+  });
+});

assistant/src/__tests__/browser-skill-endstate.test.ts

@@ -10,6 +10,7 @@ mock.module("../config/loader.js", () => ({
   getConfig: () => ({}),
 }));
 
+import { BROWSER_TOOL_NAMES } from "../browser/operations.js";
 import { _setOverridesForTesting } from "../config/assistant-feature-flags.js";
 import {
   projectSkillTools,
@@ -26,7 +27,6 @@ import { eagerModuleToolNames } from "../tools/tool-manifest.js";
 import {
   BROWSER_SKILL_ID,
   BROWSER_TOOL_COUNT,
-  BROWSER_TOOL_NAMES,
   buildSkillLoadHistory,
 } from "./test-support/browser-skill-harness.js";
 
@@ -44,37 +44,20 @@ describe("browser skill migration end-state", () => {
     await initializeTools();
   });
 
-  const BROWSER_TOOLS = [
-    "browser_navigate",
-    "browser_snapshot",
-    "browser_screenshot",
-    "browser_close",
-    "browser_attach",
-    "browser_detach",
-    "browser_click",
-    "browser_type",
-    "browser_press_key",
-    "browser_scroll",
-    "browser_select_option",
-    "browser_hover",
-    "browser_wait_for",
-    "browser_extract",
-    "browser_wait_for_download",
-    "browser_fill_credential",
-    "browser_status",
-  ] as const;
+  // Browser tool names sourced from the shared browser operations contract
+  // (BROWSER_TOOL_NAMES) — no independent list maintained here.
 
   // ── 1. Startup payload excludes browser tools ──────────────────────
 
   test("browser tools are NOT in startup core registry", () => {
     const toolNames = getAllTools().map((t) => t.name);
-    for (const name of BROWSER_TOOLS) {
+    for (const name of BROWSER_TOOL_NAMES) {
       expect(toolNames).not.toContain(name);
     }
   });
 
   test("browser tool names are NOT in eagerModuleToolNames", () => {
-    for (const name of BROWSER_TOOLS) {
+    for (const name of BROWSER_TOOL_NAMES) {
       expect(eagerModuleToolNames).not.toContain(name);
     }
   });
@@ -89,7 +72,7 @@ describe("browser skill migration end-state", () => {
     expect(definitions.length).toBeLessThanOrEqual(50);
 
     const defNames = definitions.map((d) => d.name);
-    for (const name of BROWSER_TOOLS) {
+    for (const name of BROWSER_TOOL_NAMES) {
       expect(defNames).not.toContain(name);
     }
 
@@ -122,9 +105,9 @@ describe("browser skill migration end-state", () => {
     );
     const manifest = JSON.parse(fs.readFileSync(toolsPath, "utf-8"));
     expect(manifest.version).toBe(1);
-    expect(manifest.tools).toHaveLength(BROWSER_TOOLS.length);
+    expect(manifest.tools).toHaveLength(BROWSER_TOOL_NAMES.length);
     const toolNames = manifest.tools.map((t: { name: string }) => t.name);
-    for (const name of BROWSER_TOOLS) {
+    for (const name of BROWSER_TOOL_NAMES) {
       expect(toolNames).toContain(name);
     }
   });
@@ -160,7 +143,7 @@ describe("browser skill migration end-state", () => {
 
   test("all browser tools have default allow rules", () => {
     const templates = getDefaultRuleTemplates();
-    for (const tool of BROWSER_TOOLS) {
+    for (const tool of BROWSER_TOOL_NAMES) {
       const rule = templates.find(
         (t) => t.id === `default:allow-${tool}-global`,
       );
@@ -219,7 +202,7 @@ describe("browser skill migration end-state", () => {
     const content = fs.readFileSync(execPath, "utf-8");
     // browser_wait_for_download uses a standalone wrapper that calls
     // browserManager.waitForDownload() directly — no execute* function.
-    const TOOLS_WITH_EXECUTE_FN = BROWSER_TOOLS.filter(
+    const TOOLS_WITH_EXECUTE_FN = BROWSER_TOOL_NAMES.filter(
       (name) => name !== "browser_wait_for_download",
     );
     for (const name of TOOLS_WITH_EXECUTE_FN) {

assistant/src/__tests__/test-support/browser-skill-harness.ts

@@ -1,25 +1,9 @@
+import { BROWSER_TOOL_NAMES } from "../../browser/operations.js";
 import type { Message } from "../../providers/types.js";
 
-/** The browser tool names provided by the browser skill. */
-export const BROWSER_TOOL_NAMES = [
-  "browser_navigate",
-  "browser_snapshot",
-  "browser_screenshot",
-  "browser_close",
-  "browser_attach",
-  "browser_detach",
-  "browser_click",
-  "browser_type",
-  "browser_press_key",
-  "browser_scroll",
-  "browser_select_option",
-  "browser_hover",
-  "browser_wait_for",
-  "browser_extract",
-  "browser_wait_for_download",
-  "browser_fill_credential",
-  "browser_status",
-] as const;
+// Re-export BROWSER_TOOL_NAMES from the shared browser operations contract
+// so existing test imports continue to work.
+export { BROWSER_TOOL_NAMES };
 
 /** Number of browser tools provided by the skill. */
 export const BROWSER_TOOL_COUNT = BROWSER_TOOL_NAMES.length;

assistant/src/browser/operations.ts

@@ -50,6 +50,20 @@ export const BROWSER_OPERATION_NAMES: readonly BrowserOperation[] =
 
 /**
  * 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}`,

assistant/src/config/bundled-skills/browser/tools/shared.ts

@@ -8,6 +8,13 @@
  * This keeps every wrapper as a thin adapter with no independent
  * execution logic — all behavior flows through the shared browser
  * operations contract.
+ *
+ * NOTE: The current `browser_*` skill tools are compatibility adapters
+ * over the canonical browser operations contract. They exist so the
+ * LLM-facing tool API remains stable while the CLI (`assistant browser`)
+ * consumes the same operations directly. Once the `browser_*` tool
+ * names are no longer referenced by clients or the LLM, these wrappers
+ * can be removed without changing the CLI or the operations layer.
  */
 
 import {

assistant/src/permissions/defaults.ts

@@ -1,5 +1,6 @@
 import { join } from "node:path";
 
+import { BROWSER_TOOL_NAMES } from "../browser/operations.js";
 import { getIsContainerized } from "../config/env-registry.js";
 import { getConfig } from "../config/loader.js";
 import { getBundledSkillsDir } from "../config/skills.js";
@@ -275,43 +276,20 @@ export function getDefaultRuleTemplates(): DefaultRuleTemplate[] {
   // Browser tools were previously core-registered with RiskLevel.Low (auto-allowed).
   // After migration to skill-provided tools, default allow rules preserve the
   // same frictionless UX so they don't trigger permission prompts.
+  //
   // browser_navigate candidates contain URLs with "/" (e.g.
   // "browser_navigate:https://example.com/path"), so it needs standalone
   // "**" globstar (same as host_bash / computer_use_*).  The tool field
   // already filters by tool name, so a prefix is unnecessary.
-  const browserNavigateRule: DefaultRuleTemplate = {
-    id: "default:allow-browser_navigate-global",
-    tool: "browser_navigate",
-    pattern: "**",
-    scope: "everywhere",
-    decision: "allow",
-    priority: 100,
-  };
-
-  const BROWSER_TOOLS_NO_SLASH = [
-    "browser_snapshot",
-    "browser_screenshot",
-    "browser_close",
-    "browser_attach",
-    "browser_detach",
-    "browser_click",
-    "browser_type",
-    "browser_press_key",
-    "browser_scroll",
-    "browser_select_option",
-    "browser_hover",
-    "browser_wait_for",
-    "browser_extract",
-    "browser_wait_for_download",
-    "browser_fill_credential",
-    "browser_status",
-  ] as const;
-
-  const browserToolRules: DefaultRuleTemplate[] = BROWSER_TOOLS_NO_SLASH.map(
+  // All other browser tools use the standard "tool:*" pattern.
+  //
+  // The canonical set of browser tool names is sourced from BROWSER_TOOL_NAMES
+  // (browser/operations.ts) — the single source of truth for browser identifiers.
+  const browserToolRules: DefaultRuleTemplate[] = BROWSER_TOOL_NAMES.map(
     (tool) => ({
       id: `default:allow-${tool}-global`,
       tool,
-      pattern: `${tool}:*`,
+      pattern: tool === "browser_navigate" ? "**" : `${tool}:*`,
       scope: "everywhere",
       decision: "allow" as const,
       priority: 100,
@@ -357,7 +335,6 @@ export function getDefaultRuleTemplates(): DefaultRuleTemplate[] {
     skillLoadDynamicRule,
     skillLoadRule,
     skillExecuteRule,
-    browserNavigateRule,
     ...browserToolRules,
     ...uiSurfaceRules,
     memoryRecallRule,

assistant/src/permissions/workspace-policy.ts

@@ -1,6 +1,8 @@
 import { realpathSync } from "node:fs";
 import { basename, dirname, normalize, resolve } from "node:path";
 
+import { BROWSER_TOOL_NAMES } from "../browser/operations.js";
+
 /**
  * Resolve a path to its canonical form. When the target itself doesn't
  * exist (e.g. a new file being written), walk up to the nearest existing
@@ -53,21 +55,13 @@ export function isPathWithinWorkspaceRoot(
 /** File-path tools whose workspace-scoped-ness depends on the file_path input. */
 const PATH_SCOPED_TOOLS = new Set(["file_read", "file_write", "file_edit"]);
 
-/** Network-accessing tools — never workspace-scoped. */
+/** Network-accessing tools — never workspace-scoped.
+ * Browser tool names are sourced from the shared browser operations contract
+ * (BROWSER_TOOL_NAMES) to avoid maintaining a separate browser tool list. */
 const NETWORK_TOOLS = new Set([
   "web_search",
   "web_fetch",
-  "browser_navigate",
-  "browser_click",
-  "browser_type",
-  "browser_scroll",
-  "browser_select_option",
-  "browser_hover",
-  "browser_screenshot",
-  "browser_close",
-  "browser_attach",
-  "browser_detach",
-  "browser_status",
+  ...BROWSER_TOOL_NAMES,
   "network_request",
 ]);
 

assistant/src/tools/side-effects.ts

@@ -4,6 +4,23 @@
 // 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";
+
+/**
+ * Browser tools that are read-only / observational and do NOT have
+ * side effects. These are excluded from the side-effect set.
+ * The mutating browser tools (navigate, click, type, etc.) are derived
+ * from BROWSER_TOOL_NAMES by subtracting this set.
+ */
+const BROWSER_READONLY_TOOLS = new Set([
+  "browser_snapshot",
+  "browser_screenshot",
+  "browser_extract",
+  "browser_wait_for",
+  "browser_wait_for_download",
+  "browser_status",
+]);
+
 const SIDE_EFFECT_TOOLS: ReadonlySet<string> = new Set([
   "file_write",
   "file_edit",
@@ -12,17 +29,7 @@ const SIDE_EFFECT_TOOLS: ReadonlySet<string> = new Set([
   "bash",
   "host_bash",
   "web_fetch",
-  "browser_navigate",
-  "browser_click",
-  "browser_type",
-  "browser_press_key",
-  "browser_scroll",
-  "browser_select_option",
-  "browser_hover",
-  "browser_close",
-  "browser_attach",
-  "browser_detach",
-  "browser_fill_credential",
+  ...BROWSER_TOOL_NAMES.filter((name) => !BROWSER_READONLY_TOOLS.has(name)),
   "document_create",
   "document_update",
   "schedule_create",