diff --git a/assistant/src/__tests__/plugin-api-tool-definition.test.ts b/assistant/src/__tests__/plugin-api-tool-definition.test.ts new file mode 100644 index 00000000000..d1abbceabcb --- /dev/null +++ b/assistant/src/__tests__/plugin-api-tool-definition.test.ts @@ -0,0 +1,92 @@ +/** + * Shape tests for the public `ToolDefinition` author-facing tool spec. + * + * These tests don't exercise runtime behavior — they assert via + * `satisfies` that representative tool literals line up with the public + * interface. If a later PR breaks a field name or signature in + * `assistant/src/plugin-api/types.ts`, this file fails to type-check and + * the regression is caught at `tsc --noEmit` / `bun test` time. + */ + +import { describe, expect, test } from "bun:test"; + +import { + RiskLevel, + type ToolContext, + type ToolDefinition, + type ToolExecutionResult, +} from "../plugin-api/index.js"; + +describe("ToolDefinition (public author-facing tool spec) ", () => { + test("a fully-populated literal satisfies the interface", () => { + const tool = { + description: "Greet the model in a fixed language.", + defaultRiskLevel: RiskLevel.Low, + input_schema: { + type: "object", + properties: { + language: { type: "string" }, + }, + required: ["language"], + additionalProperties: false, + }, + async execute( + input: Record, + _ctx: ToolContext, + ): Promise { + return { + content: `hello, ${String(input.language)} speaker`, + isError: false, + }; + }, + } as const satisfies ToolDefinition; + + // `as const` propagates literal types and verifies type compatibility, + // but the runtime expectations below also smoke-check the structure + // for anyone reading the test without TS folded in. + expect(typeof tool.execute).toBe("function"); + expect(tool.defaultRiskLevel).toBe(RiskLevel.Low); + }); + + test("every field is optional — empty literal satisfies the interface", () => { + const tool: ToolDefinition = {}; + expect(tool).toEqual({}); + }); + + test("every author-facing risk level is permitted", () => { + const low: ToolDefinition = { defaultRiskLevel: RiskLevel.Low }; + const medium: ToolDefinition = { defaultRiskLevel: RiskLevel.Medium }; + const high: ToolDefinition = { defaultRiskLevel: RiskLevel.High }; + + expect(low.defaultRiskLevel).toBe(RiskLevel.Low); + expect(medium.defaultRiskLevel).toBe(RiskLevel.Medium); + expect(high.defaultRiskLevel).toBe(RiskLevel.High); + }); + + test("execute receives the public ToolContext", async () => { + // Type-only assertion: the execute signature uses the public + // ToolContext (narrow base). A daemon-internal field added to the + // rich ToolContext that doesn't exist on the narrow one must not be + // accessible here. We can't test this at runtime — the assertion + // lives in `tsc --noEmit` over this file. + const tool: ToolDefinition = { + async execute(_input, ctx) { + // `ctx` is the public `ToolContext`. Touch + // commonly-needed fields to make sure they're present. + const _conversationId = ctx.conversationId; + const _workingDir = ctx.workingDir; + const _signal = ctx.signal; + return { content: "ok", isError: false }; + }, + }; + const result = await tool.execute?.( + {}, + { + conversationId: "conv-abc", + workingDir: "/tmp", + signal: new AbortController().signal, + } as ToolContext, + ); + expect(result?.isError).toBe(false); + }); +}); diff --git a/assistant/src/__tests__/plugin-tool-contribution.test.ts b/assistant/src/__tests__/plugin-tool-contribution.test.ts index 9aa8c1f2055..bab19c1f0c5 100644 --- a/assistant/src/__tests__/plugin-tool-contribution.test.ts +++ b/assistant/src/__tests__/plugin-tool-contribution.test.ts @@ -320,12 +320,18 @@ describe("registerPluginTools / unregisterPluginTools helpers", () => { // ownerMcpServerId / ownerSkillBundled / ownerSkillVersionHash) so the // stamped tool cannot leak across namespaces or spoof bundled-skill // auto-allow. - const spoofed = makeFakeTool("pt_spoof", { + // + // The narrow `ToolDefinition` shape doesn't expose these ownership + // fields, so the cast through `unknown` simulates a hostile or + // transpiled artifact that arrives with spoofed fields baked in — + // the bootstrap-side defense is the second layer that must hold. + const spoofed = { + ...makeFakeTool("pt_spoof"), origin: "skill", ownerSkillId: "some-other-skill", ownerSkillBundled: true, ownerSkillVersionHash: "deadbeef", - }); + } as unknown as LoadedPluginTool; registerPluginTools("my-plugin", [spoofed]); const retrieved = getTool("pt_spoof"); expect(retrieved?.origin).toBe("plugin"); diff --git a/assistant/src/daemon/meet-manifest-loader.ts b/assistant/src/daemon/meet-manifest-loader.ts index 476a6aae936..723df1f3718 100644 --- a/assistant/src/daemon/meet-manifest-loader.ts +++ b/assistant/src/daemon/meet-manifest-loader.ts @@ -132,11 +132,11 @@ function buildProxyTool( supervisor: MeetHostSupervisor, manifestHash: string, ): Tool { - const definition: ToolDefinition = { + const definition = { name: entry.name, description: entry.description, input_schema: (entry.input_schema as object) ?? {}, - }; + } satisfies ToolDefinition & { name: string }; const risk = coerceRiskLevel(entry.risk, entry.name); return { name: entry.name, diff --git a/assistant/src/ipc/skill-routes/registries.ts b/assistant/src/ipc/skill-routes/registries.ts index 2dcf13a028b..c1b66641c3e 100644 --- a/assistant/src/ipc/skill-routes/registries.ts +++ b/assistant/src/ipc/skill-routes/registries.ts @@ -182,11 +182,11 @@ export function __getActiveSessionCountForTesting(): number { * exercised end-to-end. */ function buildProxyTool(manifest: ToolManifest): Tool { - const definition: ToolDefinition = { + const definition = { name: manifest.name, description: manifest.description, input_schema: manifest.input_schema as object, - }; + } satisfies ToolDefinition & { name: string }; // RiskLevel is a string enum whose values are "low" | "medium" | "high", // matching the schema above exactly — the cast is a no-op at runtime. return { diff --git a/assistant/src/plugin-api/index.ts b/assistant/src/plugin-api/index.ts index 5db3ad9d45d..3ed0b1d421e 100644 --- a/assistant/src/plugin-api/index.ts +++ b/assistant/src/plugin-api/index.ts @@ -25,6 +25,8 @@ * - {@link UserPromptSubmitContext} — passed to `user-prompt-submit` hook, * fired immediately before the agent loop receives a user's prompt * - {@link PluginLogger} — pino-compatible logger shape on the contexts + * - {@link ToolDefinition} — author-facing tool spec (default-export shape + * for both plugin tool files and workspace tool files) * - {@link ToolContext} — passed to a plugin tool's `execute` method * - {@link ToolExecutionResult} — return shape of a plugin tool's `execute` * @@ -41,6 +43,8 @@ export type { PluginLogger, PluginShutdownContext, ToolContext, + ToolDefinition, ToolExecutionResult, UserPromptSubmitContext, } from "./types.js"; +export { RiskLevel } from "./types.js"; diff --git a/assistant/src/plugin-api/types.ts b/assistant/src/plugin-api/types.ts index 3f50817492a..54d817d3c3e 100644 --- a/assistant/src/plugin-api/types.ts +++ b/assistant/src/plugin-api/types.ts @@ -1,43 +1,17 @@ /** - * Public plugin-API types. - * - * This module is the entry point plugin authors land on when they import - * from `@vellumai/plugin-api`. The shapes here are the canonical public - * contract — anything exported is part of the surface that semver gates. - * - * ## Tool-execution types - * - * `ToolContext` and `ToolExecutionResult` are re-exports of the narrow, - * stable bases defined alongside their daemon-internal counterparts in - * `assistant/src/tools/types.ts`. The daemon-internal `ToolContext` / - * `ToolExecutionResult` (with CES, trust classification, lifecycle - * events, sensitive-output bindings, risk metadata, etc.) `extends` - * the public bases, so the runtime can hand plugins the full value - * without a manual cast and tsc enforces the structural relationship. - * Plugin tools see the narrow surface only — they MUST NOT set fields - * that belong to the daemon-internal extension. - * - * ## Hook contexts - * - * The init / shutdown hook contexts are owned by this module directly. - * They have no daemon-internal extension today (the daemon constructs - * and hands them straight through), so there's nothing to inherit from. - * - * ## Compatibility - * - * Adding fields to any public shape is non-breaking. Renaming or - * removing fields is breaking and gated on a major bump of - * `@vellumai/plugin-api`. + * Public plugin-API types — the canonical contract for + * `@vellumai/plugin-api`. Adding fields is non-breaking; renaming / + * removing is breaking and gated on a major bump. */ import type { Message } from "../providers/types.js"; -// ─── Tool-execution types (re-exported from daemon source-of-truth) ────────── - export type { - PluginToolContext as ToolContext, - PluginToolExecutionResult as ToolExecutionResult, + ToolContext, + ToolDefinition, + ToolExecutionResult, } from "../tools/types.js"; +export { RiskLevel } from "../tools/types.js"; // ─── Logger ────────────────────────────────────────────────────────────────── diff --git a/assistant/src/plugins/external-plugin-loader.ts b/assistant/src/plugins/external-plugin-loader.ts index 6fa97cdea8f..a2997371713 100644 --- a/assistant/src/plugins/external-plugin-loader.ts +++ b/assistant/src/plugins/external-plugin-loader.ts @@ -51,8 +51,8 @@ import { z } from "zod"; import assistantPkg from "../../package.json" with { type: "json" }; import type { LoadedPluginTool, - PluginTool, RiskLevel, + ToolDefinition, ToolExecutionResult, } from "../tools/types.js"; import { getLogger } from "../util/logger.js"; @@ -142,12 +142,12 @@ export const PLUGIN_TOOL_DEFAULTS = Object.freeze({ }); /** - * Fill the four normally-required {@link PluginTool} fields with documented - * defaults when the author omitted them. Returns a {@link LoadedPluginTool} - * that is safe to register. + * Fill the four normally-required {@link ToolDefinition} fields with + * documented defaults when the author omitted them. Returns a + * {@link LoadedPluginTool} that is safe to register. */ function applyPluginToolDefaults( - tool: PluginTool, + tool: ToolDefinition, name: string, ): LoadedPluginTool { const description = @@ -159,8 +159,7 @@ function applyPluginToolDefaults( ? tool.defaultRiskLevel : PLUGIN_TOOL_DEFAULTS.defaultRiskLevel; const input_schema = - tool.input_schema !== null && - typeof tool.input_schema === "object" + tool.input_schema !== null && typeof tool.input_schema === "object" ? tool.input_schema : PLUGIN_TOOL_DEFAULTS.input_schema; const execute = @@ -343,7 +342,7 @@ async function buildPluginFromDir(pluginDir: string): Promise { for (const { name: toolName, path: toolPath } of listSurfaceDir( join(pluginDir, "tools"), )) { - const tool = await importDefault(toolPath); + const tool = await importDefault(toolPath); if (tool === null || typeof tool !== "object") { throw new Error( `external plugin ${name}: ${toolPath} default export must be an object`, diff --git a/assistant/src/plugins/types.ts b/assistant/src/plugins/types.ts index 10ed4e61984..d96081881b3 100644 --- a/assistant/src/plugins/types.ts +++ b/assistant/src/plugins/types.ts @@ -1010,9 +1010,10 @@ export interface Injector { /** * Tool registration contributed by a plugin. Uses the narrow * {@link LoadedPluginTool} shape. External plugin authors declare the - * nameless `PluginTool` file shape; the loader derives `name` from the - * `tools/.ts` basename before storing it on `plugin.tools`. Authors - * also leave category / ownership metadata to the bootstrap, which stamps + * nameless `ToolDefinition` (from `@vellumai/plugin-api`) file shape; + * the loader derives `name` from the `tools/.ts` basename before + * storing it on `plugin.tools`. Authors also leave category / ownership + * metadata to the bootstrap, which stamps * `category: "plugin"`, `origin: "plugin"`, and * `ownerPluginId: ` before handing the batch to * `registerPluginTools`. The registration boundary synthesizes diff --git a/assistant/src/tools/types.ts b/assistant/src/tools/types.ts index 01aa8b24c11..0f061a2bb63 100644 --- a/assistant/src/tools/types.ts +++ b/assistant/src/tools/types.ts @@ -5,7 +5,6 @@ import type { ProxyApprovalCallback, RiskLevel, SensitiveOutputBinding, - ToolDefinition, ToolExecutionErrorEvent, ToolExecutionStartEvent, ToolPermissionDeniedEvent, @@ -16,7 +15,10 @@ import type { InterfaceId } from "../channels/types.js"; import type { CesClient } from "../credential-execution/client.js"; import type { ToolActivityMetadata } from "../daemon/message-types/web-activity.js"; import type { SecretPromptResult } from "../permissions/secret-prompter.js"; -import type { ContentBlock } from "../providers/types.js"; +import type { + ContentBlock, + ToolDefinition as ProviderToolSchema, +} from "../providers/types.js"; import type { TrustClass } from "../runtime/actor-trust-resolver.js"; export const DISK_PRESSURE_CLEANUP_TOOL_NAMES: ReadonlySet = new Set([ @@ -64,7 +66,6 @@ export type { ProxyEnvVars, SensitiveOutputBinding, SensitiveOutputKind, - ToolDefinition, ToolExecutionErrorEvent, ToolExecutionStartEvent, ToolPermissionDeniedEvent, @@ -76,19 +77,7 @@ export { RiskLevel } from "@vellumai/skill-host-contracts"; // Assistant-side concrete overlays // --------------------------------------------------------------------------- -/** - * Public, narrow subset of {@link ToolExecutionResult} that plugin-authored - * tools are responsible for producing. Re-exported from - * `@vellumai/plugin-api` as `ToolExecutionResult` — the type name plugin - * authors actually import. The daemon-internal version below extends - * this and adds runtime-only fields (risk metadata, approval - * bookkeeping, sensitive-output bindings, etc.) that the executor - * populates around the call — plugins MUST NOT set those. - * - * Adding fields here is a non-breaking change; renaming or removing - * fields is breaking and gated on a major bump of `@vellumai/plugin-api`. - */ -export interface PluginToolExecutionResult { +export interface ToolExecutionResult { /** Textual result shown to the model in the tool-result block. Empty string is valid. */ content: string; /** When true, the agent loop treats `content` as an error and may surface it / retry. */ @@ -105,9 +94,6 @@ export interface PluginToolExecutionResult { * the LLM voluntarily end its turn. */ yieldToUser?: boolean; -} - -export interface ToolExecutionResult extends PluginToolExecutionResult { diff?: DiffInfo; /** Optional rich content blocks (e.g. images) to include alongside text in the tool result. */ contentBlocks?: ContentBlock[]; @@ -211,20 +197,7 @@ export type ToolLifecycleEventHandler = ( event: ToolLifecycleEvent, ) => void | Promise; -/** - * Public, narrow subset of {@link ToolContext} handed to plugin-authored - * tools. Re-exported from `@vellumai/plugin-api` as `ToolContext` — the - * type name plugin authors actually import. The daemon-internal version - * below extends this and adds host-only fields (CES client, trust class, - * lifecycle handlers, requester metadata, host-bash proxy, etc.). Plugin - * tools see this shape only — the runtime still hands them the full - * {@link ToolContext} value, but the structural extension here guarantees - * the assignment without a manual cast. - * - * Adding fields here is a non-breaking change; renaming or removing - * fields is breaking and gated on a major bump of `@vellumai/plugin-api`. - */ -export interface PluginToolContext { +export interface ToolContext { /** Identifier of the conversation this tool invocation belongs to. */ conversationId: string; /** Working directory the daemon was launched from. */ @@ -235,9 +208,6 @@ export interface PluginToolContext { signal?: AbortSignal; /** Optional incremental-output callback for streaming tools. Streaming tools should fall back to returning the full result in `content` when this is absent. */ onOutput?: (chunk: string) => void; -} - -export interface ToolContext extends PluginToolContext { /** Logical assistant scope for multi-assistant routing. */ assistantId?: string; /** When set, the tool execution is part of a task run. Used to retrieve ephemeral permission rules. */ @@ -371,7 +341,7 @@ export interface Tool { /** Declared execution target from the skill manifest. Used by resolveExecutionTarget * to accurately label lifecycle events for skill-provided tools. */ executionTarget?: ExecutionTarget; - getDefinition(): ToolDefinition; + getDefinition(): ProviderToolSchema; execute( input: Record, context: ToolContext, @@ -379,31 +349,11 @@ export interface Tool { } /** - * Plugin-facing tool shape. The narrow surface plugin authors implement; - * differs from {@link Tool} in four ways: - * - Plugins declare `input_schema` as a top-level field instead of - * implementing `getDefinition()`. The registration boundary synthesizes - * `getDefinition()` from `{name, description, input_schema}` before the - * tool enters the internal registry. - * - `name` is derived from the tool file's basename by the external plugin - * loader. - * - `category` is registry-owned and stamped to `"plugin"` when the tool is - * registered. - * - All ownership stamps (`origin`, `ownerPluginId`, etc.) are set - * authoritatively by the bootstrap; plugin authors leave them blank. - * - * Every author-visible field is optional. The loader fills the four - * normally-required slots (`description`, `defaultRiskLevel`, - * `input_schema`, `execute`) with documented defaults when a plugin omits - * them — see `applyPluginToolDefaults` in `external-plugin-loader.ts`. - * A nameless, body-less `export default {}` is a valid (if useless) tool; - * misconfigured tools surface at call time rather than blocking plugin - * load. + * Author-facing tool spec — re-exported from `@vellumai/plugin-api`. + * The loader fills documented defaults for omitted fields; see + * `applyPluginToolDefaults` in `external-plugin-loader.ts`. */ -export type PluginTool = Omit< - Tool, - "category" | "getDefinition" | "name" | "description" | "defaultRiskLevel" -> & { +export interface ToolDefinition { description?: string; defaultRiskLevel?: RiskLevel; input_schema?: object; @@ -411,20 +361,7 @@ export type PluginTool = Omit< input: Record, context: ToolContext, ) => Promise; -}; +} -/** - * Plugin tool after the external loader has derived its registry name and - * filled defaults for any author-omitted fields. All four normally-required - * slots are guaranteed present. - */ -export type LoadedPluginTool = PluginTool & { - name: string; - description: string; - defaultRiskLevel: RiskLevel; - input_schema: object; - execute: ( - input: Record, - context: ToolContext, - ) => Promise; -}; +/** Plugin tool after the external loader has derived its name and filled defaults. */ +export type LoadedPluginTool = Required & { name: string };