From 1ab987709a93f6b33b50f253b8d1c8022c390938 Mon Sep 17 00:00:00 2001 From: Vellum Assistant Date: Sun, 3 May 2026 04:28:58 +0000 Subject: [PATCH] chore(tools): delete unused app-control definitions.ts The 400-line tools/app-control/definitions.ts was referenced only by app-control-tool-schemas.test.ts. The production bundled-skill path uses TOOLS.json + bundled-tool-registry.ts. The hand-duplicated schemas in definitions.ts had no sync enforcement against TOOLS.json. Rewrite the schema test to validate TOOLS.json directly. The skill-proxy-bridge.ts helper is preserved (the bundled-skill stubs still use it). Part of plan: app-control-skill.md (fix round 1) --- .../app-control-tool-schemas.test.ts | 130 ++++-- .../src/tools/app-control/definitions.ts | 400 ------------------ 2 files changed, 87 insertions(+), 443 deletions(-) delete mode 100644 assistant/src/tools/app-control/definitions.ts diff --git a/assistant/src/__tests__/app-control-tool-schemas.test.ts b/assistant/src/__tests__/app-control-tool-schemas.test.ts index 8440ea916e3..229ba3c31c6 100644 --- a/assistant/src/__tests__/app-control-tool-schemas.test.ts +++ b/assistant/src/__tests__/app-control-tool-schemas.test.ts @@ -1,28 +1,20 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; import { describe, expect, test } from "bun:test"; -import { RiskLevel } from "../permissions/types.js"; -import { - appControlClickTool, - appControlComboTool, - appControlDragTool, - appControlObserveTool, - appControlPressTool, - appControlStartTool, - appControlStopTool, - appControlTools, - appControlTypeTool, -} from "../tools/app-control/definitions.js"; import { forwardAppControlProxyTool } from "../tools/app-control/skill-proxy-bridge.js"; -import type { Tool, ToolContext } from "../tools/types.js"; +import type { ToolContext } from "../tools/types.js"; // --------------------------------------------------------------------------- -// Helpers +// Load TOOLS.json (the production source of truth for app-control tool +// schemas, consumed by the bundled-skill registry). // --------------------------------------------------------------------------- interface JsonSchemaProp { type?: string; enum?: string[]; items?: { type?: string }; + description?: string; } interface JsonSchema { @@ -31,10 +23,44 @@ interface JsonSchema { properties?: Record; } -function schema(tool: Tool): JsonSchema { - return tool.getDefinition().input_schema as JsonSchema; +interface ToolEntry { + name: string; + description: string; + category: string; + risk: string; + input_schema: JsonSchema; + executor: string; + execution_target: string; +} + +interface ToolsJson { + version: number; + tools: ToolEntry[]; +} + +const TOOLS_JSON_PATH = join( + import.meta.dir, + "..", + "config", + "bundled-skills", + "app-control", + "TOOLS.json", +); + +const toolsJson: ToolsJson = JSON.parse(readFileSync(TOOLS_JSON_PATH, "utf-8")); + +function toolByName(name: string): ToolEntry { + const tool = toolsJson.tools.find((t) => t.name === name); + if (!tool) { + throw new Error(`tool ${name} not found in TOOLS.json`); + } + return tool; } +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + /** * Lightweight, schema-driven validator covering the cases this PR exercises: * - all `required` keys must be present @@ -116,45 +142,56 @@ const ctx: ToolContext = { // Aggregate invariants // --------------------------------------------------------------------------- -describe("app-control tool definitions (aggregate)", () => { - test("appControlTools contains exactly 8 tools", () => { - expect(appControlTools.length).toBe(8); +describe("app-control TOOLS.json (aggregate)", () => { + test("contains exactly 8 tools", () => { + expect(toolsJson.tools.length).toBe(8); }); - test("all tools have proxy execution mode", () => { - for (const tool of appControlTools) { - expect(tool.executionMode).toBe("proxy"); + test("all tools target host execution", () => { + for (const tool of toolsJson.tools) { + expect(tool.execution_target).toBe("host"); } }); test("all tools belong to the app-control category", () => { - for (const tool of appControlTools) { + for (const tool of toolsJson.tools) { expect(tool.category).toBe("app-control"); } }); test("all tools have unique names", () => { - const names = appControlTools.map((t) => t.name); + const names = toolsJson.tools.map((t) => t.name); expect(new Set(names).size).toBe(names.length); }); test("all tool names use the app_control_ prefix", () => { - for (const tool of appControlTools) { + for (const tool of toolsJson.tools) { expect(tool.name.startsWith("app_control_")).toBe(true); } }); test("all tools have non-empty descriptions", () => { - for (const tool of appControlTools) { + for (const tool of toolsJson.tools) { expect(tool.description.length).toBeGreaterThan(0); } }); - test("stub execute() throws for every tool", () => { - for (const tool of appControlTools) { - expect(() => tool.execute({}, ctx)).toThrow( - "app-control tool must be forwarded to the connected client", - ); + test("every tool declares an `app` schema property (required for all but stop)", () => { + for (const tool of toolsJson.tools) { + const props = tool.input_schema.properties ?? {}; + expect( + props.app, + `${tool.name} must declare an 'app' property`, + ).toBeDefined(); + expect(props.app.type).toBe("string"); + + if (tool.name === "app_control_stop") { + // stop is the terminal tool; `app` is optional. + expect(tool.input_schema.required ?? []).not.toContain("app"); + } else { + // every other tool requires `app`. + expect(tool.input_schema.required ?? []).toContain("app"); + } } }); }); @@ -164,7 +201,8 @@ describe("app-control tool definitions (aggregate)", () => { // --------------------------------------------------------------------------- describe("app_control_start", () => { - const s = schema(appControlStartTool); + const tool = toolByName("app_control_start"); + const s = tool.input_schema; test("well-formed input passes (with args)", () => { expect( @@ -188,13 +226,14 @@ describe("app_control_start", () => { expect(result.error).toContain("app"); }); - test("default risk level is Medium", () => { - expect(appControlStartTool.defaultRiskLevel).toBe(RiskLevel.Medium); + test("declares medium risk", () => { + expect(tool.risk).toBe("medium"); }); }); describe("app_control_observe", () => { - const s = schema(appControlObserveTool); + const tool = toolByName("app_control_observe"); + const s = tool.input_schema; test("well-formed input passes", () => { expect( @@ -208,13 +247,13 @@ describe("app_control_observe", () => { expect(result.error).toContain("app"); }); - test("default risk level is Low", () => { - expect(appControlObserveTool.defaultRiskLevel).toBe(RiskLevel.Low); + test("declares low risk", () => { + expect(tool.risk).toBe("low"); }); }); describe("app_control_press", () => { - const s = schema(appControlPressTool); + const s = toolByName("app_control_press").input_schema; test("well-formed input passes (with optional fields)", () => { expect( @@ -255,7 +294,7 @@ describe("app_control_press", () => { }); describe("app_control_combo", () => { - const s = schema(appControlComboTool); + const s = toolByName("app_control_combo").input_schema; test("well-formed input passes", () => { expect( @@ -288,7 +327,7 @@ describe("app_control_combo", () => { }); describe("app_control_type", () => { - const s = schema(appControlTypeTool); + const s = toolByName("app_control_type").input_schema; test("well-formed input passes", () => { expect( @@ -317,7 +356,7 @@ describe("app_control_type", () => { }); describe("app_control_click", () => { - const s = schema(appControlClickTool); + const s = toolByName("app_control_click").input_schema; test("well-formed input passes (defaults)", () => { expect( @@ -378,7 +417,7 @@ describe("app_control_click", () => { }); describe("app_control_drag", () => { - const s = schema(appControlDragTool); + const s = toolByName("app_control_drag").input_schema; test("well-formed input passes", () => { expect( @@ -429,10 +468,15 @@ describe("app_control_drag", () => { expect(result.ok).toBe(false); expect(result.error).toContain("button"); }); + + test("button enum is left/right/middle", () => { + const props = s.properties as Record; + expect(props.button.enum).toEqual(["left", "right", "middle"]); + }); }); describe("app_control_stop", () => { - const s = schema(appControlStopTool); + const s = toolByName("app_control_stop").input_schema; test("well-formed input passes (no app — terminal)", () => { expect(validate(s, { activity: "wrap up" }).ok).toBe(true); diff --git a/assistant/src/tools/app-control/definitions.ts b/assistant/src/tools/app-control/definitions.ts deleted file mode 100644 index b7a98a51de3..00000000000 --- a/assistant/src/tools/app-control/definitions.ts +++ /dev/null @@ -1,400 +0,0 @@ -/** - * App-control tool definitions. - * - * These tools target a specific application (by bundle ID or process name) on - * the desktop client (host machine). Each tool is a proxy: execution is - * forwarded to the connected client and never handled locally by the daemon. - * - * The eight tools mirror the input wire types declared in - * `daemon/message-types/host-app-control.ts`: - * start | observe | press | combo | type | click | drag | stop - * - * Distinct from the system-wide `computer_use_*` proxy tools — app-control - * scopes input/observation to a single targeted app window. - */ - -import { RiskLevel } from "../../permissions/types.js"; -import type { ToolDefinition } from "../../providers/types.js"; -import type { Tool, ToolExecutionResult } from "../types.js"; - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -function proxyExecute(): Promise { - throw new Error("app-control tool must be forwarded to the connected client"); -} - -const activityProperty = { - type: "string" as const, - description: - "Brief non-technical explanation of why this tool is being called", -}; - -const appProperty = { - type: "string" as const, - description: - "Bundle ID (preferred, e.g. 'com.apple.Safari') or process name of the target application", -}; - -const buttonEnum = ["left", "right", "middle"] as const; - -// --------------------------------------------------------------------------- -// start — launch (or focus) the target app, optionally with CLI args -// --------------------------------------------------------------------------- - -export const appControlStartTool: Tool = { - name: "app_control_start", - description: - "Start (launch or focus) the target application. Optionally pass command-line arguments. Begins an app-control session targeting this app.", - category: "app-control", - defaultRiskLevel: RiskLevel.Medium, - executionMode: "proxy", - - getDefinition(): ToolDefinition { - return { - name: this.name, - description: this.description, - input_schema: { - type: "object", - properties: { - app: appProperty, - args: { - type: "array", - items: { type: "string" }, - description: - "Optional command-line arguments to launch the app with", - }, - reasoning: { - type: "string", - description: "Explanation of why you are starting this app", - }, - activity: activityProperty, - }, - required: ["app", "reasoning"], - }, - }; - }, - - execute: proxyExecute, -}; - -// --------------------------------------------------------------------------- -// observe — capture window state of the target app -// --------------------------------------------------------------------------- - -export const appControlObserveTool: Tool = { - name: "app_control_observe", - description: - "Capture the current window state of the target application — returns lifecycle state (running/missing/minimized/occluded), an optional screenshot, and window bounds. Use this before issuing input actions, or to check progress without acting.", - category: "app-control", - defaultRiskLevel: RiskLevel.Low, - executionMode: "proxy", - - getDefinition(): ToolDefinition { - return { - name: this.name, - description: this.description, - input_schema: { - type: "object", - properties: { - app: appProperty, - activity: activityProperty, - }, - required: ["app", "activity"], - }, - }; - }, - - execute: proxyExecute, -}; - -// --------------------------------------------------------------------------- -// press — single key with optional modifiers and hold duration -// --------------------------------------------------------------------------- - -export const appControlPressTool: Tool = { - name: "app_control_press", - description: - "Press a single key in the target application, with optional modifiers (cmd/shift/option/ctrl) and a hold duration in milliseconds.", - category: "app-control", - defaultRiskLevel: RiskLevel.Low, - executionMode: "proxy", - - getDefinition(): ToolDefinition { - return { - name: this.name, - description: this.description, - input_schema: { - type: "object", - properties: { - app: appProperty, - key: { - type: "string", - description: 'Key identifier, e.g. "return", "a", "f12"', - }, - modifiers: { - type: "array", - items: { type: "string" }, - description: - 'Modifier list, e.g. ["cmd", "shift"]. Omit for no modifiers.', - }, - duration_ms: { - type: "integer", - description: "Hold duration in milliseconds", - }, - reasoning: { - type: "string", - description: "Explanation of why you are pressing this key", - }, - activity: activityProperty, - }, - required: ["app", "key", "reasoning"], - }, - }; - }, - - execute: proxyExecute, -}; - -// --------------------------------------------------------------------------- -// combo — multiple keys pressed simultaneously -// --------------------------------------------------------------------------- - -export const appControlComboTool: Tool = { - name: "app_control_combo", - description: - "Press multiple keys simultaneously in the target application (e.g. cmd+shift+4). Use for keyboard shortcuts where every key is held at once.", - category: "app-control", - defaultRiskLevel: RiskLevel.Low, - executionMode: "proxy", - - getDefinition(): ToolDefinition { - return { - name: this.name, - description: this.description, - input_schema: { - type: "object", - properties: { - app: appProperty, - keys: { - type: "array", - items: { type: "string" }, - description: - 'Sequence of keys pressed simultaneously, e.g. ["cmd", "shift", "4"]', - }, - duration_ms: { - type: "integer", - description: "Hold duration in milliseconds", - }, - reasoning: { - type: "string", - description: "Explanation of why you are pressing this combo", - }, - activity: activityProperty, - }, - required: ["app", "keys", "reasoning"], - }, - }; - }, - - execute: proxyExecute, -}; - -// --------------------------------------------------------------------------- -// type — type literal text into the target app -// --------------------------------------------------------------------------- - -export const appControlTypeTool: Tool = { - name: "app_control_type", - description: - "Type literal text into the target application at the current focus. Ensure the intended field is focused before calling.", - category: "app-control", - defaultRiskLevel: RiskLevel.Low, - executionMode: "proxy", - - getDefinition(): ToolDefinition { - return { - name: this.name, - description: this.description, - input_schema: { - type: "object", - properties: { - app: appProperty, - text: { - type: "string", - description: "The text to type", - }, - reasoning: { - type: "string", - description: "Explanation of what you are typing and why", - }, - activity: activityProperty, - }, - required: ["app", "text", "reasoning"], - }, - }; - }, - - execute: proxyExecute, -}; - -// --------------------------------------------------------------------------- -// click — click at window-relative (x, y) coordinates -// --------------------------------------------------------------------------- - -export const appControlClickTool: Tool = { - name: "app_control_click", - description: - "Click at the given (x, y) coordinates inside the target application's window. Defaults to a single left-click; pass `button` and/or `double` to vary.", - category: "app-control", - defaultRiskLevel: RiskLevel.Low, - executionMode: "proxy", - - getDefinition(): ToolDefinition { - return { - name: this.name, - description: this.description, - input_schema: { - type: "object", - properties: { - app: appProperty, - x: { - type: "integer", - description: "X coordinate (window-relative)", - }, - y: { - type: "integer", - description: "Y coordinate (window-relative)", - }, - button: { - type: "string", - enum: [...buttonEnum], - description: 'Mouse button (default: "left")', - }, - double: { - type: "boolean", - description: "When true, performs a double-click", - }, - reasoning: { - type: "string", - description: - "Explanation of what you see and why you are clicking here", - }, - activity: activityProperty, - }, - required: ["app", "x", "y", "reasoning"], - }, - }; - }, - - execute: proxyExecute, -}; - -// --------------------------------------------------------------------------- -// drag — drag from one coord to another inside the target app -// --------------------------------------------------------------------------- - -export const appControlDragTool: Tool = { - name: "app_control_drag", - description: - "Drag from (from_x, from_y) to (to_x, to_y) inside the target application's window. Defaults to left button.", - category: "app-control", - defaultRiskLevel: RiskLevel.Low, - executionMode: "proxy", - - getDefinition(): ToolDefinition { - return { - name: this.name, - description: this.description, - input_schema: { - type: "object", - properties: { - app: appProperty, - from_x: { - type: "integer", - description: "Source X coordinate (window-relative)", - }, - from_y: { - type: "integer", - description: "Source Y coordinate (window-relative)", - }, - to_x: { - type: "integer", - description: "Destination X coordinate (window-relative)", - }, - to_y: { - type: "integer", - description: "Destination Y coordinate (window-relative)", - }, - button: { - type: "string", - enum: [...buttonEnum], - description: 'Mouse button (default: "left")', - }, - reasoning: { - type: "string", - description: "Explanation of what you are dragging and why", - }, - activity: activityProperty, - }, - required: ["app", "from_x", "from_y", "to_x", "to_y", "reasoning"], - }, - }; - }, - - execute: proxyExecute, -}; - -// --------------------------------------------------------------------------- -// stop — terminal: end the app-control session -// --------------------------------------------------------------------------- - -export const appControlStopTool: Tool = { - name: "app_control_stop", - description: - "Stop the current app-control session. When `app` is omitted, stops whichever app currently holds the session. This is the terminal action for an app-control flow.", - category: "app-control", - defaultRiskLevel: RiskLevel.Low, - executionMode: "proxy", - - getDefinition(): ToolDefinition { - return { - name: this.name, - description: this.description, - input_schema: { - type: "object", - properties: { - app: { - type: "string", - description: - "Optional bundle ID or process name. When omitted, stops whichever app currently holds the session.", - }, - reason: { - type: "string", - description: "Free-form reason, surfaced for logging", - }, - activity: activityProperty, - }, - required: ["activity"], - }, - }; - }, - - execute: proxyExecute, -}; - -// --------------------------------------------------------------------------- -// All tools exported as array for convenience -// --------------------------------------------------------------------------- - -export const appControlTools: Tool[] = [ - appControlStartTool, - appControlObserveTool, - appControlPressTool, - appControlComboTool, - appControlTypeTool, - appControlClickTool, - appControlDragTool, - appControlStopTool, -];