From dee77fe060d1f85945df06ff50d3e0d374b9e02f Mon Sep 17 00:00:00 2001 From: ApolloBot Date: Sun, 24 May 2026 21:56:53 +0000 Subject: [PATCH 1/5] feat(plugin-api): export ToolDefinition for tool-file authoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The author-facing tool spec that plugin tool files and workspace tool files default-export. Lives at `@vellumai/plugin-api`'s public surface, type-only, structurally identical to the existing internal `PluginTool` shape so plugin authors can migrate by changing only the import name. This is the type the upcoming workspace-tool authoring guide (PR #31949) will reference instead of the daemon-internal `PluginTool` name, which was never publicly exported. Workspace tools and plugin tools share this surface but differ in their default risk floor: plugin tools default to \"medium\" (in-tree-vetted code), workspace tools default to \"high\" (operator-authored on-disk code). The default switch lives in each loader; this PR just adds the shared author-facing type. The name `ToolDefinition` collides with an internal type of the same name in `@vellumai/skill-host-contracts` that represents the JSON-schema bundle sent to providers. The doc comment on the new interface flags this — the imports are disjoint (different packages), so plugin authors always land on the right one. Tests cover full / empty / each risk-level literal and the narrow ToolContext exposure. --- .../plugin-api-tool-definition.test.ts | 96 +++++++++++++++++++ assistant/src/plugin-api/index.ts | 3 + assistant/src/plugin-api/types.ts | 68 +++++++++++++ 3 files changed, 167 insertions(+) create mode 100644 assistant/src/__tests__/plugin-api-tool-definition.test.ts 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..63e9f5bb6be --- /dev/null +++ b/assistant/src/__tests__/plugin-api-tool-definition.test.ts @@ -0,0 +1,96 @@ +/** + * 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. + * + * The shape is identical (structurally) to the existing internal + * `PluginTool`, so this also covers the migration path of plugin authors + * switching their imports from the legacy `PluginTool` name to the new + * `ToolDefinition` name. + */ + +import { describe, expect, test } from "bun:test"; + +import type { + ToolContext, + ToolDefinition, + 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: "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("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: "low" }; + const medium: ToolDefinition = { defaultRiskLevel: "medium" }; + const high: ToolDefinition = { defaultRiskLevel: "high" }; + + expect(low.defaultRiskLevel).toBe("low"); + expect(medium.defaultRiskLevel).toBe("medium"); + expect(high.defaultRiskLevel).toBe("high"); + }); + + test("execute receives the narrow 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 `ToolContext` (the narrow public one). 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/plugin-api/index.ts b/assistant/src/plugin-api/index.ts index 5db3ad9d45d..f571ad546a2 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,7 @@ export type { PluginLogger, PluginShutdownContext, ToolContext, + ToolDefinition, ToolExecutionResult, UserPromptSubmitContext, } from "./types.js"; diff --git a/assistant/src/plugin-api/types.ts b/assistant/src/plugin-api/types.ts index 3f50817492a..13e4ce3b147 100644 --- a/assistant/src/plugin-api/types.ts +++ b/assistant/src/plugin-api/types.ts @@ -39,6 +39,74 @@ export type { PluginToolExecutionResult as ToolExecutionResult, } from "../tools/types.js"; +// ─── Tool authoring surface ────────────────────────────────────────────────── + +import type { + PluginToolContext as PublicToolContext, + PluginToolExecutionResult as PublicToolExecutionResult, +} from "../tools/types.js"; + +/** + * Author-facing tool spec — what a plugin tool file or workspace tool file + * default-exports. + * + * `name` is omitted from this shape because the host derives it + * authoritatively from the tool's owning location: + * - Plugins: `tools/.{ts,js}` → name = basename + * - Workspace tools: `/tools/.{ts,js,json}` → name = filename stem + * + * Ownership stamps (`origin`, `ownerPluginId`, `ownerWorkspacePath`, etc.) + * are also set authoritatively by the host. Authors leave them blank. + * + * Every field on this interface is optional. The host fills the four + * normally-required slots (`description`, `defaultRiskLevel`, + * `input_schema`, `execute`) with documented defaults when omitted, so a + * misconfigured tool still loads cleanly and surfaces its problem at call + * time rather than blocking assistant boot. + * + * Default values differ by origin: + * - Plugin tools default `defaultRiskLevel` to `"medium"`. + * - Workspace tools default `defaultRiskLevel` to `"high"` — they run + * arbitrary on-disk code under the operator's workspace, so the floor + * is higher than for in-tree-vetted plugin code. + * + * (NOTE: there's an unrelated internal type also called `ToolDefinition` + * in `@vellumai/skill-host-contracts` that represents the JSON-schema + * bundle sent to LLM providers. That's the runtime/provider-side shape; + * this is the author-time shape. The names overlap but the imports are + * disjoint — plugin and workspace authors should always import this + * `ToolDefinition` from `@vellumai/plugin-api`.) + */ +export interface ToolDefinition { + /** + * Free-form description the model sees in its tool list. Defaults to + * an empty string when omitted. + */ + description?: string; + /** + * Default risk level applied when no path-based escalation matches. + * Defaults to `"medium"` for plugin tools and `"high"` for workspace + * tools. + */ + defaultRiskLevel?: "low" | "medium" | "high"; + /** + * JSON-Schema describing the tool's input arguments. Defaults to + * `{ type: "object", properties: {}, additionalProperties: false }` + * when omitted. + */ + input_schema?: object; + /** + * Execute handler. The host calls this with the model's input payload + * and a narrow `ToolContext`. Defaults to a stub that returns an error + * result when omitted (so the model sees a clear "not wired up" signal + * instead of the tool silently no-op'ing). + */ + execute?: ( + input: Record, + context: PublicToolContext, + ) => Promise; +} + // ─── Logger ────────────────────────────────────────────────────────────────── /** From d78513e79c53d03e1315db8951d8caa4e555cf11 Mon Sep 17 00:00:00 2001 From: ApolloBot Date: Mon, 25 May 2026 00:06:45 +0000 Subject: [PATCH 2/5] Consolidate PluginTool into ToolDefinition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Vargas's review on #31956: the previous `PluginTool` type in tools/types.ts was `Omit & { 4 optional fields }`, which retained ALL of Tool's internal stamps (origin, ownerPluginId, executionMode, executionTarget, owner* fields) on the author surface. Authors were told via docstring to leave them blank, but TypeScript didn't enforce it. The new `ToolDefinition` is the strict 4-field author surface — this commit unifies the two. Source-of-truth structure (mirrors the PluginToolContext/ToolContext pattern already used in this file): - `tools/types.ts` defines `PluginToolSpec` (strict author shape). All 4 fields optional. Defaults filled by the loader at registration. - `plugin-api/types.ts` re-exports as `ToolDefinition` for external authors. Docstring travels with the symbol via LSP. `LoadedPluginTool` is the post-defaults, ready-to-register shape. Overrides two fields from `Required`: - `defaultRiskLevel`: `RiskLevel` enum (vs the public string union), because `Tool.defaultRiskLevel` is the enum. - `execute`: rich `ToolContext`/`ToolExecutionResult` (vs the narrow public types), because `Tool.execute` is rich. The narrow→rich function cast happens once in `applyPluginToolDefaults`, the documented loader boundary. `applyPluginToolDefaults` also tightened: dropped the broad `...tool` spread so any extra runtime fields a JS-author or transpiled artifact might ship — origin, executionMode, owner stamps — get filtered out at the load boundary. TS authors are caught by the new narrow shape; this is the runtime backstop. Spoof test updated to cast through `unknown` — the narrow `LoadedPluginTool` no longer surfaces those fields, so we simulate a hostile/transpiled artifact arriving with spoofed fields baked in. The bootstrap-side defense is the second layer that must hold even when the type-level defense is bypassed. Typecheck, lint, all 21 affected tests: pass. --- .../plugin-api-tool-definition.test.ts | 11 +- .../plugin-tool-contribution.test.ts | 13 +- assistant/src/plugin-api/types.ts | 73 ++---------- .../src/plugins/external-plugin-loader.ts | 38 ++++-- assistant/src/plugins/types.ts | 7 +- assistant/src/tools/types.ts | 112 +++++++++++++----- 6 files changed, 137 insertions(+), 117 deletions(-) diff --git a/assistant/src/__tests__/plugin-api-tool-definition.test.ts b/assistant/src/__tests__/plugin-api-tool-definition.test.ts index 63e9f5bb6be..2193e92d476 100644 --- a/assistant/src/__tests__/plugin-api-tool-definition.test.ts +++ b/assistant/src/__tests__/plugin-api-tool-definition.test.ts @@ -7,10 +7,13 @@ * `assistant/src/plugin-api/types.ts`, this file fails to type-check and * the regression is caught at `tsc --noEmit` / `bun test` time. * - * The shape is identical (structurally) to the existing internal - * `PluginTool`, so this also covers the migration path of plugin authors - * switching their imports from the legacy `PluginTool` name to the new - * `ToolDefinition` name. + * The same shape lives internally as `PluginToolSpec` in + * `assistant/src/tools/types.ts` — the public `ToolDefinition` is just + * a re-export. This file asserts the public face stays stable; a sibling + * test in the daemon-internal suite would assert the internal face stays + * stable. (Today they're literally the same symbol, so one test covers + * both — but the separation gives future-us room to diverge them + * without losing test coverage.) */ import { describe, expect, test } from "bun:test"; diff --git a/assistant/src/__tests__/plugin-tool-contribution.test.ts b/assistant/src/__tests__/plugin-tool-contribution.test.ts index 9aa8c1f2055..f82d46101e1 100644 --- a/assistant/src/__tests__/plugin-tool-contribution.test.ts +++ b/assistant/src/__tests__/plugin-tool-contribution.test.ts @@ -320,12 +320,21 @@ 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 `LoadedPluginTool` shape no longer exposes these + // ownership fields, so TypeScript would prevent an honest author + // from setting them at compile time. The cast through `unknown` is + // deliberate: we're simulating a hostile or transpiled artifact that + // arrives at the registry with spoofed fields baked in. The + // bootstrap-side defense is the second layer that must hold even + // when the type-level defense is bypassed. + 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/plugin-api/types.ts b/assistant/src/plugin-api/types.ts index 13e4ce3b147..a351ea765f6 100644 --- a/assistant/src/plugin-api/types.ts +++ b/assistant/src/plugin-api/types.ts @@ -39,73 +39,14 @@ export type { PluginToolExecutionResult as ToolExecutionResult, } from "../tools/types.js"; -// ─── Tool authoring surface ────────────────────────────────────────────────── +// ─── Tool authoring surface (re-exported from daemon source-of-truth) ─────── +// +// `ToolDefinition` is `PluginToolSpec` from `assistant/src/tools/types.ts`, +// re-exported under its public name. The canonical docstring (covering +// defaults, naming-collision with `@vellumai/skill-host-contracts`, etc.) +// lives at the definition site and is preserved by LSP across the alias. -import type { - PluginToolContext as PublicToolContext, - PluginToolExecutionResult as PublicToolExecutionResult, -} from "../tools/types.js"; - -/** - * Author-facing tool spec — what a plugin tool file or workspace tool file - * default-exports. - * - * `name` is omitted from this shape because the host derives it - * authoritatively from the tool's owning location: - * - Plugins: `tools/.{ts,js}` → name = basename - * - Workspace tools: `/tools/.{ts,js,json}` → name = filename stem - * - * Ownership stamps (`origin`, `ownerPluginId`, `ownerWorkspacePath`, etc.) - * are also set authoritatively by the host. Authors leave them blank. - * - * Every field on this interface is optional. The host fills the four - * normally-required slots (`description`, `defaultRiskLevel`, - * `input_schema`, `execute`) with documented defaults when omitted, so a - * misconfigured tool still loads cleanly and surfaces its problem at call - * time rather than blocking assistant boot. - * - * Default values differ by origin: - * - Plugin tools default `defaultRiskLevel` to `"medium"`. - * - Workspace tools default `defaultRiskLevel` to `"high"` — they run - * arbitrary on-disk code under the operator's workspace, so the floor - * is higher than for in-tree-vetted plugin code. - * - * (NOTE: there's an unrelated internal type also called `ToolDefinition` - * in `@vellumai/skill-host-contracts` that represents the JSON-schema - * bundle sent to LLM providers. That's the runtime/provider-side shape; - * this is the author-time shape. The names overlap but the imports are - * disjoint — plugin and workspace authors should always import this - * `ToolDefinition` from `@vellumai/plugin-api`.) - */ -export interface ToolDefinition { - /** - * Free-form description the model sees in its tool list. Defaults to - * an empty string when omitted. - */ - description?: string; - /** - * Default risk level applied when no path-based escalation matches. - * Defaults to `"medium"` for plugin tools and `"high"` for workspace - * tools. - */ - defaultRiskLevel?: "low" | "medium" | "high"; - /** - * JSON-Schema describing the tool's input arguments. Defaults to - * `{ type: "object", properties: {}, additionalProperties: false }` - * when omitted. - */ - input_schema?: object; - /** - * Execute handler. The host calls this with the model's input payload - * and a narrow `ToolContext`. Defaults to a stub that returns an error - * result when omitted (so the model sees a clear "not wired up" signal - * instead of the tool silently no-op'ing). - */ - execute?: ( - input: Record, - context: PublicToolContext, - ) => Promise; -} +export type { PluginToolSpec as ToolDefinition } 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..7abb023bab2 100644 --- a/assistant/src/plugins/external-plugin-loader.ts +++ b/assistant/src/plugins/external-plugin-loader.ts @@ -51,7 +51,7 @@ import { z } from "zod"; import assistantPkg from "../../package.json" with { type: "json" }; import type { LoadedPluginTool, - PluginTool, + PluginToolSpec, RiskLevel, ToolExecutionResult, } from "../tools/types.js"; @@ -142,36 +142,48 @@ 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 PluginToolSpec} fields with + * documented defaults when the author omitted them. Returns a + * {@link LoadedPluginTool} that is safe to register. */ function applyPluginToolDefaults( - tool: PluginTool, + tool: PluginToolSpec, name: string, ): LoadedPluginTool { const description = typeof tool.description === "string" ? tool.description : PLUGIN_TOOL_DEFAULTS.description; - const defaultRiskLevel = + // Cast the public string-union to the internal `RiskLevel` enum. Runtime + // values are identical (`RiskLevel.Low === "low"` etc.) and the loader is + // the documented narrow→rich conversion boundary. + const defaultRiskLevel: RiskLevel = typeof tool.defaultRiskLevel === "string" - ? tool.defaultRiskLevel + ? (tool.defaultRiskLevel as RiskLevel) : 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 = + // Cast narrow author `execute` → rich internal `execute`. Safe at + // runtime because the rich `ToolContext` extends the narrow + // `PluginToolContext`: a function that only reads narrow fields works + // fine when the host hands it a rich context. The cast is one-way and + // happens once, here at the loader boundary. + const execute: LoadedPluginTool["execute"] = typeof tool.execute === "function" - ? tool.execute + ? (tool.execute as LoadedPluginTool["execute"]) : async (): Promise => ({ content: `plugin tool ${name} has no execute implementation`, isError: true, }); + // Construct the loaded shape explicitly (no spread of `tool`) so any + // extra fields the author tried to set on the imported runtime object + // — origin, ownerPluginId, executionMode, … — are dropped here at the + // load boundary rather than propagated into the registry. TypeScript + // would have caught these at compile time for TS authors, but JS + // authors and transpiled artifacts could still ship them at runtime. return { - ...tool, name, description, defaultRiskLevel, @@ -343,7 +355,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..fc4d579cbbf 100644 --- a/assistant/src/tools/types.ts +++ b/assistant/src/tools/types.ts @@ -379,50 +379,104 @@ 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 + * Strict author-facing tool spec. The narrow surface plugin and workspace + * tool authors implement. This is the canonical source-of-truth definition + * — `@vellumai/plugin-api` re-exports this type as `ToolDefinition` for + * external authors. + * + * Differs from the internal {@link Tool} shape in four ways: + * - Authors 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. + * - `name` is derived from the tool file's basename by the host loader. + * - `category` is registry-owned and stamped to `"plugin"` / + * `"workspace"` when the tool is registered. + * - All ownership stamps (`origin`, `ownerPluginId`, etc.) and host-level + * fields (`executionMode`, `executionTarget`, …) are NOT exposed on + * this shape. They are set authoritatively by the bootstrap and would + * otherwise tempt authors into trying to spoof ownership. + * + * Every field is optional. The loader fills the four normally-required + * slots (`description`, `defaultRiskLevel`, `input_schema`, `execute`) + * with documented defaults when an author omits them — see + * `applyPluginToolDefaults` in `external-plugin-loader.ts` and + * `WORKSPACE_TOOL_DEFAULTS` in `workspace-tools/loader.ts`. A nameless, + * body-less `export default {}` is a valid (if useless) tool; + * misconfigured tools surface at call time rather than blocking host + * boot. + * + * Default `defaultRiskLevel` differs by origin: + * - Plugin tools default to `"medium"`. + * - Workspace tools default to `"high"` — they run arbitrary on-disk + * code under the operator's workspace, so the floor is higher than + * for in-tree-vetted plugin code. * - * 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. + * (Note: there's an internal type also called `ToolDefinition` in + * `@vellumai/skill-host-contracts` that represents the JSON-schema + * bundle sent to LLM providers. That's the runtime/provider-side shape; + * this is the author-time shape. The names overlap but the imports are + * disjoint — plugin and workspace authors should always import + * `ToolDefinition` from `@vellumai/plugin-api`.) */ -export type PluginTool = Omit< - Tool, - "category" | "getDefinition" | "name" | "description" | "defaultRiskLevel" -> & { +export interface PluginToolSpec { + /** + * Free-form description the model sees in its tool list. Defaults to + * an empty string when omitted. + */ description?: string; - defaultRiskLevel?: RiskLevel; + /** + * Default risk level applied when no path-based escalation matches. + * Defaults to `"medium"` for plugin tools and `"high"` for workspace + * tools. + * + * Typed as a string union (not the `RiskLevel` enum) so external + * authors can write `defaultRiskLevel: "low"` without importing the + * host's enum. The values are identical at runtime + * (`RiskLevel.Low === "low"`); the loader casts to the enum at the + * registration boundary in `applyPluginToolDefaults`. + */ + defaultRiskLevel?: "low" | "medium" | "high"; + /** + * JSON-Schema describing the tool's input arguments. Defaults to + * `{ type: "object", properties: {}, additionalProperties: false }` + * when omitted. + */ input_schema?: object; + /** + * Execute handler. The host calls this with the model's input payload + * and a narrow `PluginToolContext`. Defaults to a stub that returns an + * error result when omitted (so the model sees a clear "not wired up" + * signal instead of the tool silently no-op'ing). + */ execute?: ( input: Record, - context: ToolContext, - ) => Promise; -}; + context: PluginToolContext, + ) => Promise; +} /** - * Plugin tool after the external loader has derived its registry name and + * Author-time tool spec after the host has derived its registry name and * filled defaults for any author-omitted fields. All four normally-required - * slots are guaranteed present. + * slots are guaranteed present. The internal stamping (origin, ownerPluginId, + * category, getDefinition) is added separately by `registerPluginTools`. + * + * Two fields are overridden from `Required` to use the + * internal rich types instead of the narrow public ones: + * - `defaultRiskLevel`: `RiskLevel` enum (vs `"low" | "medium" | "high"`), + * because downstream `Tool.defaultRiskLevel` is the enum. + * - `execute`: receives the rich `ToolContext` and returns the rich + * `ToolExecutionResult`, because `Tool.execute` is the rich variant. + * The narrow author function is widened via cast in + * `applyPluginToolDefaults` — safe at runtime because the rich types + * extend the narrow ones (a function that only reads narrow fields + * works fine when called with a rich object). */ -export type LoadedPluginTool = PluginTool & { +export type LoadedPluginTool = Required< + Omit +> & { name: string; - description: string; defaultRiskLevel: RiskLevel; - input_schema: object; execute: ( input: Record, context: ToolContext, From 6dca9acb8cfe33be4eba32d930b04b9eee008bc8 Mon Sep 17 00:00:00 2001 From: ApolloBot Date: Mon, 25 May 2026 00:56:21 +0000 Subject: [PATCH 3/5] Apply review feedback: ToolDefinition, RiskLevel, simpler LoadedPluginTool - Rename PluginToolSpec -> ToolDefinition (exported from plugin-api). - defaultRiskLevel uses the RiskLevel enum (was string union). - LoadedPluginTool reverted to a thin intersection: Omit & {required fields + rich execute}. The Omit drops the optional narrow execute so the rich override is the sole execute signature on the loaded shape. - Skill-host-contracts ToolDefinition aliased locally as ProviderToolSchema; the bulk re-export from tools/types is gone since no consumer used it (all go through providers/types). Two stragglers (daemon/meet-manifest-loader.ts, ipc/skill-routes/registries.ts) now import from providers/types for consistency. - plugin-api/types: drops the alias dance, re-exports ToolDefinition directly and exposes RiskLevel for authors. - Comments trimmed back toward one-liners; loader narrow-execute cast dropped (assignability holds via contravariance). --- .../plugin-api-tool-definition.test.ts | 35 +++--- .../plugin-tool-contribution.test.ts | 11 +- assistant/src/daemon/meet-manifest-loader.ts | 2 +- assistant/src/ipc/skill-routes/registries.ts | 2 +- assistant/src/plugin-api/index.ts | 1 + assistant/src/plugin-api/types.ts | 8 +- .../src/plugins/external-plugin-loader.ts | 31 ++--- assistant/src/tools/types.ts | 111 ++++-------------- 8 files changed, 56 insertions(+), 145 deletions(-) diff --git a/assistant/src/__tests__/plugin-api-tool-definition.test.ts b/assistant/src/__tests__/plugin-api-tool-definition.test.ts index 2193e92d476..41baeadb904 100644 --- a/assistant/src/__tests__/plugin-api-tool-definition.test.ts +++ b/assistant/src/__tests__/plugin-api-tool-definition.test.ts @@ -6,29 +6,22 @@ * 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. - * - * The same shape lives internally as `PluginToolSpec` in - * `assistant/src/tools/types.ts` — the public `ToolDefinition` is just - * a re-export. This file asserts the public face stays stable; a sibling - * test in the daemon-internal suite would assert the internal face stays - * stable. (Today they're literally the same symbol, so one test covers - * both — but the separation gives future-us room to diverge them - * without losing test coverage.) */ import { describe, expect, test } from "bun:test"; -import type { - ToolContext, - ToolDefinition, - ToolExecutionResult, +import { + RiskLevel, + type ToolContext, + type ToolDefinition, + type ToolExecutionResult, } from "../plugin-api/index.js"; -describe("ToolDefinition (public author-facing tool spec)", () => { +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: "low", + defaultRiskLevel: RiskLevel.Low, input_schema: { type: "object", properties: { @@ -52,7 +45,7 @@ describe("ToolDefinition (public author-facing tool spec)", () => { // 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("low"); + expect(tool.defaultRiskLevel).toBe(RiskLevel.Low); }); test("every field is optional — empty literal satisfies the interface", () => { @@ -61,13 +54,13 @@ describe("ToolDefinition (public author-facing tool spec)", () => { }); test("every author-facing risk level is permitted", () => { - const low: ToolDefinition = { defaultRiskLevel: "low" }; - const medium: ToolDefinition = { defaultRiskLevel: "medium" }; - const high: ToolDefinition = { defaultRiskLevel: "high" }; + const low: ToolDefinition = { defaultRiskLevel: RiskLevel.Low }; + const medium: ToolDefinition = { defaultRiskLevel: RiskLevel.Medium }; + const high: ToolDefinition = { defaultRiskLevel: RiskLevel.High }; - expect(low.defaultRiskLevel).toBe("low"); - expect(medium.defaultRiskLevel).toBe("medium"); - expect(high.defaultRiskLevel).toBe("high"); + expect(low.defaultRiskLevel).toBe(RiskLevel.Low); + expect(medium.defaultRiskLevel).toBe(RiskLevel.Medium); + expect(high.defaultRiskLevel).toBe(RiskLevel.High); }); test("execute receives the narrow public ToolContext", async () => { diff --git a/assistant/src/__tests__/plugin-tool-contribution.test.ts b/assistant/src/__tests__/plugin-tool-contribution.test.ts index f82d46101e1..bab19c1f0c5 100644 --- a/assistant/src/__tests__/plugin-tool-contribution.test.ts +++ b/assistant/src/__tests__/plugin-tool-contribution.test.ts @@ -321,13 +321,10 @@ describe("registerPluginTools / unregisterPluginTools helpers", () => { // stamped tool cannot leak across namespaces or spoof bundled-skill // auto-allow. // - // The narrow `LoadedPluginTool` shape no longer exposes these - // ownership fields, so TypeScript would prevent an honest author - // from setting them at compile time. The cast through `unknown` is - // deliberate: we're simulating a hostile or transpiled artifact that - // arrives at the registry with spoofed fields baked in. The - // bootstrap-side defense is the second layer that must hold even - // when the type-level defense is bypassed. + // 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", diff --git a/assistant/src/daemon/meet-manifest-loader.ts b/assistant/src/daemon/meet-manifest-loader.ts index 476a6aae936..384c47a6301 100644 --- a/assistant/src/daemon/meet-manifest-loader.ts +++ b/assistant/src/daemon/meet-manifest-loader.ts @@ -36,6 +36,7 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; +import type { ToolDefinition } from "../providers/types.js"; import type { SkillRoute } from "../runtime/skill-route-registry.js"; import { registerSkillRoute } from "../runtime/skill-route-registry.js"; import { getRepoSkillsDir } from "../skills/catalog-install.js"; @@ -44,7 +45,6 @@ import type { ExecutionTarget, Tool, ToolContext, - ToolDefinition, } from "../tools/types.js"; import { RiskLevel } from "../tools/types.js"; import { getLogger } from "../util/logger.js"; diff --git a/assistant/src/ipc/skill-routes/registries.ts b/assistant/src/ipc/skill-routes/registries.ts index 2dcf13a028b..e2cb6b5cf60 100644 --- a/assistant/src/ipc/skill-routes/registries.ts +++ b/assistant/src/ipc/skill-routes/registries.ts @@ -21,12 +21,12 @@ import { z } from "zod"; import type { MeetHostSupervisor } from "../../daemon/meet-host-supervisor.js"; import { registerShutdownHook } from "../../daemon/shutdown-registry.js"; +import type { ToolDefinition } from "../../providers/types.js"; import { registerSkillRoute } from "../../runtime/skill-route-registry.js"; import { registerSkillTools } from "../../tools/registry.js"; import type { ExecutionTarget, Tool, - ToolDefinition, } from "../../tools/types.js"; import { RiskLevel } from "../../tools/types.js"; import { getLogger } from "../../util/logger.js"; diff --git a/assistant/src/plugin-api/index.ts b/assistant/src/plugin-api/index.ts index f571ad546a2..3ed0b1d421e 100644 --- a/assistant/src/plugin-api/index.ts +++ b/assistant/src/plugin-api/index.ts @@ -47,3 +47,4 @@ export type { 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 a351ea765f6..831489a4c26 100644 --- a/assistant/src/plugin-api/types.ts +++ b/assistant/src/plugin-api/types.ts @@ -40,13 +40,9 @@ export type { } from "../tools/types.js"; // ─── Tool authoring surface (re-exported from daemon source-of-truth) ─────── -// -// `ToolDefinition` is `PluginToolSpec` from `assistant/src/tools/types.ts`, -// re-exported under its public name. The canonical docstring (covering -// defaults, naming-collision with `@vellumai/skill-host-contracts`, etc.) -// lives at the definition site and is preserved by LSP across the alias. -export type { PluginToolSpec as ToolDefinition } from "../tools/types.js"; +export type { ToolDefinition } 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 7abb023bab2..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, - PluginToolSpec, RiskLevel, + ToolDefinition, ToolExecutionResult, } from "../tools/types.js"; import { getLogger } from "../util/logger.js"; @@ -142,48 +142,35 @@ export const PLUGIN_TOOL_DEFAULTS = Object.freeze({ }); /** - * Fill the four normally-required {@link PluginToolSpec} fields with + * 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: PluginToolSpec, + tool: ToolDefinition, name: string, ): LoadedPluginTool { const description = typeof tool.description === "string" ? tool.description : PLUGIN_TOOL_DEFAULTS.description; - // Cast the public string-union to the internal `RiskLevel` enum. Runtime - // values are identical (`RiskLevel.Low === "low"` etc.) and the loader is - // the documented narrow→rich conversion boundary. - const defaultRiskLevel: RiskLevel = + const defaultRiskLevel = typeof tool.defaultRiskLevel === "string" - ? (tool.defaultRiskLevel as RiskLevel) + ? tool.defaultRiskLevel : PLUGIN_TOOL_DEFAULTS.defaultRiskLevel; const input_schema = tool.input_schema !== null && typeof tool.input_schema === "object" ? tool.input_schema : PLUGIN_TOOL_DEFAULTS.input_schema; - // Cast narrow author `execute` → rich internal `execute`. Safe at - // runtime because the rich `ToolContext` extends the narrow - // `PluginToolContext`: a function that only reads narrow fields works - // fine when the host hands it a rich context. The cast is one-way and - // happens once, here at the loader boundary. - const execute: LoadedPluginTool["execute"] = + const execute = typeof tool.execute === "function" - ? (tool.execute as LoadedPluginTool["execute"]) + ? tool.execute : async (): Promise => ({ content: `plugin tool ${name} has no execute implementation`, isError: true, }); - // Construct the loaded shape explicitly (no spread of `tool`) so any - // extra fields the author tried to set on the imported runtime object - // — origin, ownerPluginId, executionMode, … — are dropped here at the - // load boundary rather than propagated into the registry. TypeScript - // would have caught these at compile time for TS authors, but JS - // authors and transpiled artifacts could still ship them at runtime. return { + ...tool, name, description, defaultRiskLevel, @@ -355,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/tools/types.ts b/assistant/src/tools/types.ts index fc4d579cbbf..4209decad25 100644 --- a/assistant/src/tools/types.ts +++ b/assistant/src/tools/types.ts @@ -5,7 +5,9 @@ import type { ProxyApprovalCallback, RiskLevel, SensitiveOutputBinding, - ToolDefinition, + // The JSON-schema-bundle type sent to LLM providers; aliased locally so + // `ToolDefinition` is free for the author-facing tool spec defined below. + ToolDefinition as ProviderToolSchema, ToolExecutionErrorEvent, ToolExecutionStartEvent, ToolPermissionDeniedEvent, @@ -64,7 +66,6 @@ export type { ProxyEnvVars, SensitiveOutputBinding, SensitiveOutputKind, - ToolDefinition, ToolExecutionErrorEvent, ToolExecutionStartEvent, ToolPermissionDeniedEvent, @@ -371,7 +372,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,76 +380,22 @@ export interface Tool { } /** - * Strict author-facing tool spec. The narrow surface plugin and workspace - * tool authors implement. This is the canonical source-of-truth definition - * — `@vellumai/plugin-api` re-exports this type as `ToolDefinition` for - * external authors. + * Author-facing tool spec — the narrow surface plugin and workspace tool + * authors implement. Re-exported from `@vellumai/plugin-api`. Differs + * from the internal {@link Tool} shape: authors declare `input_schema` + * as a top-level field (the registration boundary synthesizes + * `getDefinition()` from `{name, description, input_schema}`), `name` is + * derived from the tool file's basename by the host loader, and + * `category` / ownership stamps are set authoritatively by the + * bootstrap. * - * Differs from the internal {@link Tool} shape in four ways: - * - Authors 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 host loader. - * - `category` is registry-owned and stamped to `"plugin"` / - * `"workspace"` when the tool is registered. - * - All ownership stamps (`origin`, `ownerPluginId`, etc.) and host-level - * fields (`executionMode`, `executionTarget`, …) are NOT exposed on - * this shape. They are set authoritatively by the bootstrap and would - * otherwise tempt authors into trying to spoof ownership. - * - * Every field is optional. The loader fills the four normally-required - * slots (`description`, `defaultRiskLevel`, `input_schema`, `execute`) - * with documented defaults when an author omits them — see - * `applyPluginToolDefaults` in `external-plugin-loader.ts` and - * `WORKSPACE_TOOL_DEFAULTS` in `workspace-tools/loader.ts`. A nameless, - * body-less `export default {}` is a valid (if useless) tool; - * misconfigured tools surface at call time rather than blocking host - * boot. - * - * Default `defaultRiskLevel` differs by origin: - * - Plugin tools default to `"medium"`. - * - Workspace tools default to `"high"` — they run arbitrary on-disk - * code under the operator's workspace, so the floor is higher than - * for in-tree-vetted plugin code. - * - * (Note: there's an internal type also called `ToolDefinition` in - * `@vellumai/skill-host-contracts` that represents the JSON-schema - * bundle sent to LLM providers. That's the runtime/provider-side shape; - * this is the author-time shape. The names overlap but the imports are - * disjoint — plugin and workspace authors should always import - * `ToolDefinition` from `@vellumai/plugin-api`.) + * Every author-visible field is optional; the loader fills documented + * defaults — see `applyPluginToolDefaults` in `external-plugin-loader.ts`. */ -export interface PluginToolSpec { - /** - * Free-form description the model sees in its tool list. Defaults to - * an empty string when omitted. - */ +export interface ToolDefinition { description?: string; - /** - * Default risk level applied when no path-based escalation matches. - * Defaults to `"medium"` for plugin tools and `"high"` for workspace - * tools. - * - * Typed as a string union (not the `RiskLevel` enum) so external - * authors can write `defaultRiskLevel: "low"` without importing the - * host's enum. The values are identical at runtime - * (`RiskLevel.Low === "low"`); the loader casts to the enum at the - * registration boundary in `applyPluginToolDefaults`. - */ - defaultRiskLevel?: "low" | "medium" | "high"; - /** - * JSON-Schema describing the tool's input arguments. Defaults to - * `{ type: "object", properties: {}, additionalProperties: false }` - * when omitted. - */ + defaultRiskLevel?: RiskLevel; input_schema?: object; - /** - * Execute handler. The host calls this with the model's input payload - * and a narrow `PluginToolContext`. Defaults to a stub that returns an - * error result when omitted (so the model sees a clear "not wired up" - * signal instead of the tool silently no-op'ing). - */ execute?: ( input: Record, context: PluginToolContext, @@ -456,27 +403,17 @@ export interface PluginToolSpec { } /** - * Author-time tool spec after the host has derived its registry name and - * filled defaults for any author-omitted fields. All four normally-required - * slots are guaranteed present. The internal stamping (origin, ownerPluginId, - * category, getDefinition) is added separately by `registerPluginTools`. - * - * Two fields are overridden from `Required` to use the - * internal rich types instead of the narrow public ones: - * - `defaultRiskLevel`: `RiskLevel` enum (vs `"low" | "medium" | "high"`), - * because downstream `Tool.defaultRiskLevel` is the enum. - * - `execute`: receives the rich `ToolContext` and returns the rich - * `ToolExecutionResult`, because `Tool.execute` is the rich variant. - * The narrow author function is widened via cast in - * `applyPluginToolDefaults` — safe at runtime because the rich types - * extend the narrow ones (a function that only reads narrow fields - * works fine when called with a rich object). + * Plugin tool after the external loader has derived its registry name and + * filled defaults for any author-omitted fields. `execute` is widened to the + * rich daemon-internal {@link ToolContext} / {@link ToolExecutionResult} at + * the loader boundary; the author's narrow function is assignable via + * contravariance and the runtime ctx already carries the rich fields. */ -export type LoadedPluginTool = Required< - Omit -> & { +export type LoadedPluginTool = Omit & { name: string; + description: string; defaultRiskLevel: RiskLevel; + input_schema: object; execute: ( input: Record, context: ToolContext, From 8236f27a5d139c61fe4478b608d2545dca7afb42 Mon Sep 17 00:00:00 2001 From: ApolloBot Date: Mon, 25 May 2026 09:49:30 +0000 Subject: [PATCH 4/5] Consolidate ToolContext/ToolExecutionResult; simplify LoadedPluginTool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Delete PluginToolContext (inline its fields into ToolContext) and PluginToolExecutionResult (inline into ToolExecutionResult). The narrow public/rich-internal split is gone; plugin authors see the same ToolContext/ToolExecutionResult shape the daemon uses internally. - ToolDefinition.execute now references the unified types directly. - LoadedPluginTool collapses to Required & { name: string }. - @vellumai/plugin-api re-exports ToolContext/ToolExecutionResult directly (no alias dance) — same names, same source. - Drop the direct skill-host-contracts ToolDefinition import in tools/types.ts; ProviderToolSchema is aliased via providers/types.ts for now (single-source-of-truth migration deferred to a follow-up). - Trim docstrings to one-liners. 30/30 plugin tests pass; typecheck/lint clean. --- .../plugin-api-tool-definition.test.ts | 4 +- assistant/src/plugin-api/types.ts | 43 ++-------- assistant/src/tools/types.ts | 80 +++---------------- 3 files changed, 21 insertions(+), 106 deletions(-) diff --git a/assistant/src/__tests__/plugin-api-tool-definition.test.ts b/assistant/src/__tests__/plugin-api-tool-definition.test.ts index 41baeadb904..d1abbceabcb 100644 --- a/assistant/src/__tests__/plugin-api-tool-definition.test.ts +++ b/assistant/src/__tests__/plugin-api-tool-definition.test.ts @@ -63,7 +63,7 @@ describe("ToolDefinition (public author-facing tool spec) ", () => { expect(high.defaultRiskLevel).toBe(RiskLevel.High); }); - test("execute receives the narrow public ToolContext", async () => { + 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 @@ -71,7 +71,7 @@ describe("ToolDefinition (public author-facing tool spec) ", () => { // lives in `tsc --noEmit` over this file. const tool: ToolDefinition = { async execute(_input, ctx) { - // `ctx` is `ToolContext` (the narrow public one). Touch + // `ctx` is the public `ToolContext`. Touch // commonly-needed fields to make sure they're present. const _conversationId = ctx.conversationId; const _workingDir = ctx.workingDir; diff --git a/assistant/src/plugin-api/types.ts b/assistant/src/plugin-api/types.ts index 831489a4c26..54d817d3c3e 100644 --- a/assistant/src/plugin-api/types.ts +++ b/assistant/src/plugin-api/types.ts @@ -1,47 +1,16 @@ /** - * 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"; - -// ─── Tool authoring surface (re-exported from daemon source-of-truth) ─────── - -export type { ToolDefinition } from "../tools/types.js"; export { RiskLevel } from "../tools/types.js"; // ─── Logger ────────────────────────────────────────────────────────────────── diff --git a/assistant/src/tools/types.ts b/assistant/src/tools/types.ts index 4209decad25..0f061a2bb63 100644 --- a/assistant/src/tools/types.ts +++ b/assistant/src/tools/types.ts @@ -5,9 +5,6 @@ import type { ProxyApprovalCallback, RiskLevel, SensitiveOutputBinding, - // The JSON-schema-bundle type sent to LLM providers; aliased locally so - // `ToolDefinition` is free for the author-facing tool spec defined below. - ToolDefinition as ProviderToolSchema, ToolExecutionErrorEvent, ToolExecutionStartEvent, ToolPermissionDeniedEvent, @@ -18,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([ @@ -77,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. */ @@ -106,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[]; @@ -212,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. */ @@ -236,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. */ @@ -380,42 +349,19 @@ export interface Tool { } /** - * Author-facing tool spec — the narrow surface plugin and workspace tool - * authors implement. Re-exported from `@vellumai/plugin-api`. Differs - * from the internal {@link Tool} shape: authors declare `input_schema` - * as a top-level field (the registration boundary synthesizes - * `getDefinition()` from `{name, description, input_schema}`), `name` is - * derived from the tool file's basename by the host loader, and - * `category` / ownership stamps are set authoritatively by the - * bootstrap. - * - * Every author-visible field is optional; the loader fills documented - * defaults — see `applyPluginToolDefaults` in `external-plugin-loader.ts`. + * 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 interface ToolDefinition { description?: string; defaultRiskLevel?: RiskLevel; input_schema?: object; execute?: ( - input: Record, - context: PluginToolContext, - ) => Promise; -} - -/** - * Plugin tool after the external loader has derived its registry name and - * filled defaults for any author-omitted fields. `execute` is widened to the - * rich daemon-internal {@link ToolContext} / {@link ToolExecutionResult} at - * the loader boundary; the author's narrow function is assignable via - * contravariance and the runtime ctx already carries the rich fields. - */ -export type LoadedPluginTool = Omit & { - 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 }; From fff69c41a001c83fb7d6a8bdd6e710ac10850ec6 Mon Sep 17 00:00:00 2001 From: ApolloBot Date: Mon, 25 May 2026 11:33:22 +0000 Subject: [PATCH 5/5] meet-manifest-loader/registries: source ToolDefinition from tools/types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch the two proxy-tool builders to import the author-facing ToolDefinition (from tools/types.ts) instead of the JSON-schema-bundle re-export in providers/types.ts. The local definition literal uses `satisfies ToolDefinition & { name: string }` so the literal narrowing is preserved for the getDefinition() closure return. Narrow scope per review feedback — the broader providers/types.ts migration is deferred to a follow-up. --- assistant/src/daemon/meet-manifest-loader.ts | 6 +++--- assistant/src/ipc/skill-routes/registries.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/assistant/src/daemon/meet-manifest-loader.ts b/assistant/src/daemon/meet-manifest-loader.ts index 384c47a6301..723df1f3718 100644 --- a/assistant/src/daemon/meet-manifest-loader.ts +++ b/assistant/src/daemon/meet-manifest-loader.ts @@ -36,7 +36,6 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; -import type { ToolDefinition } from "../providers/types.js"; import type { SkillRoute } from "../runtime/skill-route-registry.js"; import { registerSkillRoute } from "../runtime/skill-route-registry.js"; import { getRepoSkillsDir } from "../skills/catalog-install.js"; @@ -45,6 +44,7 @@ import type { ExecutionTarget, Tool, ToolContext, + ToolDefinition, } from "../tools/types.js"; import { RiskLevel } from "../tools/types.js"; import { getLogger } from "../util/logger.js"; @@ -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 e2cb6b5cf60..c1b66641c3e 100644 --- a/assistant/src/ipc/skill-routes/registries.ts +++ b/assistant/src/ipc/skill-routes/registries.ts @@ -21,12 +21,12 @@ import { z } from "zod"; import type { MeetHostSupervisor } from "../../daemon/meet-host-supervisor.js"; import { registerShutdownHook } from "../../daemon/shutdown-registry.js"; -import type { ToolDefinition } from "../../providers/types.js"; import { registerSkillRoute } from "../../runtime/skill-route-registry.js"; import { registerSkillTools } from "../../tools/registry.js"; import type { ExecutionTarget, Tool, + ToolDefinition, } from "../../tools/types.js"; import { RiskLevel } from "../../tools/types.js"; import { getLogger } from "../../util/logger.js"; @@ -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 {