Skip to content
Merged
80 changes: 80 additions & 0 deletions assistant/src/__tests__/browser-identifier-parity-guard.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
41 changes: 12 additions & 29 deletions assistant/src/__tests__/browser-skill-endstate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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";

Expand All @@ -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);
}
});
Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}
});
Expand Down Expand Up @@ -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`,
);
Expand Down Expand Up @@ -217,9 +200,9 @@ describe("browser skill migration end-state", () => {
);
expect(fs.existsSync(execPath)).toBe(true);
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(
// browser_wait_for_download has no matching executeBrowser* function
// exported from browser-execution.ts — it is handled via operations.ts.
const TOOLS_WITH_EXECUTE_FN = BROWSER_TOOL_NAMES.filter(
(name) => name !== "browser_wait_for_download",
);
for (const name of TOOLS_WITH_EXECUTE_FN) {
Expand Down
24 changes: 4 additions & 20 deletions assistant/src/__tests__/test-support/browser-skill-harness.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
179 changes: 179 additions & 0 deletions assistant/src/browser/__tests__/operations.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import { describe, expect, test } from "bun:test";

import {
BROWSER_OPERATION_META,
BROWSER_OPERATION_NAMES,
BROWSER_TOOL_NAMES,
browserOperationToToolName,
browserToolNameToOperation,
executeBrowserOperation,
getBrowserOperationMeta,
} from "../operations.js";
import { BROWSER_OPERATIONS, type BrowserOperation } from "../types.js";

describe("browser operations contract", () => {
// ── Exactly 17 operations ──────────────────────────────────────────

test("defines exactly 17 operations", () => {
expect(BROWSER_OPERATION_NAMES).toHaveLength(17);
expect(BROWSER_TOOL_NAMES).toHaveLength(17);
});

// ── Bijective tool-name mapping ────────────────────────────────────

test("tool name -> operation is bijective", () => {
for (const op of BROWSER_OPERATIONS) {
const toolName = browserOperationToToolName(op);
expect(toolName).toBe(`browser_${op}`);
const roundTripped = browserToolNameToOperation(toolName!);
expect(roundTripped).toBe(op);
}
});

test("operation -> tool name is bijective", () => {
for (const toolName of BROWSER_TOOL_NAMES) {
const op = browserToolNameToOperation(toolName);
expect(op).toBeDefined();
const roundTripped = browserOperationToToolName(op!);
expect(roundTripped).toBe(toolName);
}
});

test("unknown tool name returns undefined", () => {
expect(browserToolNameToOperation("browser_unknown")).toBeUndefined();
expect(browserToolNameToOperation("not_browser")).toBeUndefined();
expect(browserToolNameToOperation("")).toBeUndefined();
});

test("unknown operation returns undefined", () => {
expect(browserOperationToToolName("unknown")).toBeUndefined();
expect(browserOperationToToolName("")).toBeUndefined();
});

// ── Every operation has metadata ───────────────────────────────────

test("every operation has command metadata", () => {
for (const op of BROWSER_OPERATIONS) {
const meta = getBrowserOperationMeta(op);
expect(meta).toBeDefined();
expect(meta!.operation).toBe(op);
expect(typeof meta!.description).toBe("string");
expect(meta!.description.length).toBeGreaterThan(0);
}
});

test("metadata count matches operation count", () => {
expect(BROWSER_OPERATION_META).toHaveLength(BROWSER_OPERATIONS.length);
});

test("metadata operations match BROWSER_OPERATIONS", () => {
const metaOps = BROWSER_OPERATION_META.map((m) => m.operation).sort();
const declaredOps = [...BROWSER_OPERATIONS].sort();
expect(metaOps).toEqual(declaredOps);
});

// ── Every operation has a dispatch handler ─────────────────────────

test("every operation has a dispatch handler (rejects unknown)", async () => {
// We verify dispatch handlers exist by calling executeBrowserOperation
// with an invalid operation, which should return an error for unknown
// operations. For known operations, the handler itself exists (it would
// attempt real browser work, which we do not test here).
const result = await executeBrowserOperation(
"nonexistent" as BrowserOperation,
{},
{
workingDir: "/tmp",
conversationId: "test",
trustClass: "guardian",
},
);
expect(result.isError).toBe(true);
expect(result.content).toContain("Unknown browser operation");
});

// ── wait_for_download mode constraints ─────────────────────────────

test("wait_for_download metadata restricts modes to auto and local", () => {
const meta = getBrowserOperationMeta("wait_for_download");
expect(meta).toBeDefined();
expect(meta!.allowedModes).toBeDefined();
expect(meta!.allowedModes).toContain("auto");
expect(meta!.allowedModes).toContain("local");
expect(meta!.allowedModes).toHaveLength(2);
});

test("wait_for_download rejects extension mode", async () => {
const result = await executeBrowserOperation(
"wait_for_download",
{ browser_mode: "extension" },
{
workingDir: "/tmp",
conversationId: "test",
trustClass: "guardian",
},
);
expect(result.isError).toBe(true);
expect(result.content).toContain("does not support browser_mode");
expect(result.content).toContain("extension");
});

test("wait_for_download rejects cdp-inspect mode", async () => {
const result = await executeBrowserOperation(
"wait_for_download",
{ browser_mode: "cdp-inspect" },
{
workingDir: "/tmp",
conversationId: "test",
trustClass: "guardian",
},
);
expect(result.isError).toBe(true);
expect(result.content).toContain("does not support browser_mode");
expect(result.content).toContain("cdp-inspect");
});

// ── Metadata field constraints ─────────────────────────────────────

test("all metadata fields have valid types", () => {
const validTypes = new Set(["string", "number", "boolean"]);
for (const meta of BROWSER_OPERATION_META) {
for (const field of meta.fields) {
expect(validTypes.has(field.type)).toBe(true);
expect(typeof field.name).toBe("string");
expect(field.name.length).toBeGreaterThan(0);
expect(typeof field.description).toBe("string");
expect(typeof field.required).toBe("boolean");
}
}
});

test("required fields appear before optional fields in metadata", () => {
for (const meta of BROWSER_OPERATION_META) {
let seenOptional = false;
for (const field of meta.fields) {
if (!field.required) {
seenOptional = true;
} else if (seenOptional) {
throw new Error(
`Operation "${meta.operation}": required field "${field.name}" appears after optional fields`,
);
}
}
}
});

// ── No TOOLS.json dependency ───────────────────────────────────────

test("operations module does not depend on TOOLS.json", async () => {
// Verify by checking that the operations module source does not
// reference TOOLS.json. This is a static analysis guard.
const { readFileSync } = await import("node:fs");
const source = readFileSync(
new URL("../operations.ts", import.meta.url),
"utf-8",
);
expect(source).not.toContain("TOOLS.json");
expect(source).not.toContain("bundled-skills");
});
});
Loading
Loading