diff options
| author | amankumarsingh77 <amankumarsingh7702@gmail.com> | 2026-07-28 00:01:32 +0530 |
|---|---|---|
| committer | Aman Kumar <145339995+amankumarsingh77@users.noreply.github.com> | 2026-08-02 19:12:46 +0530 |
| commit | 5cfa7ccc8f4082db111bc0186c5fa3d44b767ad3 (patch) | |
| tree | 05720833665398725d960aad67496ff6159540eb /test | |
| parent | 753c2ad615fa9827e168be256e1a7c7384605855 (diff) | |
refactor(tools): declare concurrency in the tool definition
serialized: true was applied in registry.ts, away from the tools it
describes. Now a required concurrency field on each definition, with a
test pinning the serialized set so the classes cannot drift.
The lock check defaults to taking the mutex for any class that is not
explicitly "parallel", so a definition that bypasses defineBrowserTool at
runtime fails safe rather than racing on shared CDP state.
The test script gains --conditions=import: pi-coding-agent publishes no
"require" condition, so importing any tool definition from a CJS-loaded
test could not resolve it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Diffstat (limited to 'test')
| -rw-r--r-- | test/domains/registry-test.ts | 127 |
1 files changed, 127 insertions, 0 deletions
diff --git a/test/domains/registry-test.ts b/test/domains/registry-test.ts new file mode 100644 index 0000000..a35f485 --- /dev/null +++ b/test/domains/registry-test.ts @@ -0,0 +1,127 @@ +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import { Type } from "typebox"; +import type { ExtensionAPI, ToolDefinition } from "@mariozechner/pi-coding-agent"; +import type { TSchema } from "typebox"; +import { ALL_TOOLS } from "../../src/registry"; +import { type AnyBrowserToolDefinition, registerBrowserTool } from "../../src/util/tool"; +import type { BrowserClient } from "../../src/client"; +import { ok } from "../../src/util/result"; + +const SERIALIZED = new Set([ + "browser_click", + "browser_type", + "browser_fill", + "browser_select_option", + "browser_focus", + "browser_press_key", + "browser_dispatch_key", + "browser_scroll", + "browser_wait_for_load", + "browser_handle_dialog", + "browser_navigate", + "browser_open_urls", + "browser_go_back", + "browser_go_forward", + "browser_reload", + "browser_switch_tab", + "browser_new_tab", + "browser_close_tab", + "browser_upload_file", + "browser_download", + "browser_print_to_pdf", + "browser_viewport_resize", + "browser_drag_and_drop", + "browser_web_search", +]); + +describe("tool registry", () => { + test("every tool declares a concurrency class", () => { + for (const t of ALL_TOOLS) { + assert.ok( + t.concurrency === "serialized" || t.concurrency === "parallel", + `${t.name} has no concurrency class`, + ); + } + }); + + test("the serialized set is exactly the mutating tools", () => { + const actual = new Set( + ALL_TOOLS.filter((t) => t.concurrency === "serialized").map((t) => t.name), + ); + assert.deepEqual([...actual].sort(), [...SERIALIZED].sort()); + }); + + test("tool names are unique", () => { + const names = ALL_TOOLS.map((t) => t.name); + assert.equal(new Set(names).size, names.length); + }); + + test("every tool name uses the browser_ prefix", () => { + for (const t of ALL_TOOLS) assert.match(t.name, /^browser_/); + }); +}); + +type LockProbe = { + readonly acquires: ReadonlyArray<string>; + run: (def: AnyBrowserToolDefinition) => Promise<void>; +}; + +const lockProbe = (): LockProbe => { + const acquires: string[] = []; + const client = { + mutationMutex: () => ({ + acquire: async () => { + acquires.push("acquired"); + return () => {}; + }, + }), + } as unknown as BrowserClient; + return { + acquires, + run: async (def) => { + let registered: ToolDefinition<TSchema> | undefined; + const pi = { + registerTool: (td: ToolDefinition<TSchema>) => { + registered = td; + }, + } as unknown as ExtensionAPI; + registerBrowserTool(pi, client, def); + assert.ok(registered, "tool was not registered"); + await registered.execute?.("id", {}, undefined, undefined, undefined as never); + }, + }; +}; + +const probeTool = (concurrency: unknown): AnyBrowserToolDefinition => + ({ + name: "browser_probe", + label: "Probe", + description: "probe", + promptSnippet: "probe", + promptGuidelines: [], + parameters: Type.Object({}), + ensureAlive: false, + concurrency, + handler: async () => ok({ text: "done" }), + }) as AnyBrowserToolDefinition; + +describe("concurrency lane selection", () => { + test("a parallel tool skips the mutation mutex", async () => { + const probe = lockProbe(); + await probe.run(probeTool("parallel")); + assert.deepEqual([...probe.acquires], []); + }); + + test("a serialized tool takes the mutation mutex", async () => { + const probe = lockProbe(); + await probe.run(probeTool("serialized")); + assert.deepEqual([...probe.acquires], ["acquired"]); + }); + + test("a definition with no concurrency class takes the mutation mutex", async () => { + const probe = lockProbe(); + await probe.run(probeTool(undefined)); + assert.deepEqual([...probe.acquires], ["acquired"]); + }); +}); |
