Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions assistant/src/__tests__/plugin-bootstrap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ describe("plugin bootstrap", () => {
{
name: "gated-off-tool",
description: "should not be registered",
category: "test",
defaultRiskLevel: RiskLevel.Low,
executionTarget: "sandbox",
input_schema: { type: "object", properties: {}, required: [] },
Expand Down
9 changes: 5 additions & 4 deletions assistant/src/__tests__/plugin-tool-contribution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ import {
unregisterPluginTools,
} from "../tools/registry.js";
import type {
LoadedTool,
Tool,
ToolContext,
ToolExecutionResult,
} from "../tools/types.js";
Expand All @@ -84,14 +84,15 @@ const fakeCtx: DaemonContext = {

function makeFakeTool(
name: string,
extras: Partial<LoadedTool> = {},
): LoadedTool {
extras: Partial<Tool> = {},
): Tool {
return {
name,
description: `Fake ${name}`,
defaultRiskLevel: RiskLevel.Low,
executionTarget: "sandbox",
input_schema: { type: "object", properties: {}, required: [] },
category: "",
async execute(
_input: Record<string, unknown>,
_context: ToolContext,
Expand Down Expand Up @@ -336,7 +337,7 @@ describe("registerPluginTools / unregisterPluginTools helpers", () => {
...makeFakeTool("pt_spoof"),
origin: "skill",
owner: { kind: "skill", id: "some-other-skill" },
} as unknown as LoadedTool;
} as unknown as Tool;
registerPluginTools("my-plugin", [spoofed]);
expect(getTool("pt_spoof")).toBeDefined();
expect(getToolOwner("pt_spoof")).toEqual({
Expand Down
5 changes: 3 additions & 2 deletions assistant/src/__tests__/plugin-types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
type ToolResultTruncateResult,
type TurnContext,
} from "../plugins/types.js";
import type { LoadedTool } from "../tools/types.js";
import type { Tool } from "../tools/types.js";

const sampleTrust: TrustContext = {
sourceChannel: "vellum",
Expand Down Expand Up @@ -207,12 +207,13 @@ describe("plugin core types", () => {
},
};

const sampleTool: LoadedTool = {
const sampleTool: Tool = {
name: "sample-tool",
description: "Sample plugin tool",
defaultRiskLevel: RiskLevel.Low,
executionTarget: "sandbox",
input_schema: { type: "object", properties: {}, required: [] },
category: "",
async execute() {
return { content: "ok", isError: false };
},
Expand Down
4 changes: 2 additions & 2 deletions assistant/src/plugins/external-plugin-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { z } from "zod";

import assistantPkg from "../../package.json" with { type: "json" };
import { finalizeTool } from "../tools/tool-defaults.js";
import type { LoadedTool, ToolDefinition } from "../tools/types.js";
import type { Tool, ToolDefinition } from "../tools/types.js";
import { getLogger } from "../util/logger.js";
import { registerPlugin } from "./registry.js";
import type {
Expand Down Expand Up @@ -276,7 +276,7 @@ async function buildPluginFromDir(pluginDir: string): Promise<Plugin> {
const hooks = await loadHooks(pluginDir, name);
if (hooks !== undefined) plugin.hooks = hooks;

const tools: LoadedTool[] = [];
const tools: Tool[] = [];
for (const { name: toolName, path: toolPath } of listSurfaceDir(
join(pluginDir, "tools"),
)) {
Expand Down
6 changes: 3 additions & 3 deletions assistant/src/plugins/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import type {
} from "../providers/types.js";
import type { SkillRoute } from "../runtime/skill-route-registry.js";
import type {
LoadedTool,
Tool,
ToolContext,
ToolExecutionResult,
} from "../tools/types.js";
Expand Down Expand Up @@ -1157,10 +1157,10 @@ export interface Plugin {
* `@vellumai/plugin-api`); the loader derives `name` from the
* `tools/<name>.ts` basename and runs the definition through
* `finalizeTool` to fill omitted required fields, producing the
* `LoadedTool` values stored here. Category / ownership metadata is
* `Tool` values stored here. Category / ownership metadata is
* stamped by `registerPluginTools` at registration time.
*/
tools?: LoadedTool[];
tools?: Tool[];
/** HTTP route registrations served by the assistant. */
routes?: PluginRouteRegistration[];
/** Skill registrations loaded at startup. */
Expand Down
2 changes: 1 addition & 1 deletion assistant/src/tools/execution-target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface ManifestOverride {
* 3. Default sandbox.
*
* Called once per tool at load/construction time. The returned value is
* stamped onto every `LoadedTool`, so runtime reads are just a field read.
* stamped onto every `Tool`, so runtime reads are just a field read.
*/
export function resolveExecutionTarget(tool: {
name: string;
Expand Down
4 changes: 2 additions & 2 deletions assistant/src/tools/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { hostFileWriteTool } from "./host-filesystem/write.js";
import { hostShellTool } from "./host-terminal/host-shell.js";
import { toProviderSafeToolName } from "./provider-tool-name.js";
import { registerSystemTools } from "./system/register.js";
import type { LoadedTool, OwnerInfo, Tool } from "./types.js";
import type { OwnerInfo, Tool } from "./types.js";
import { allUiSurfaceTools } from "./ui-surface/definitions.js";
import { registerUiSurfaceTools } from "./ui-surface/registry.js";

Expand Down Expand Up @@ -235,7 +235,7 @@ export function registerSkillTools(skillId: string, newTools: Tool[]): Tool[] {
*/
export function registerPluginTools(
pluginName: string,
newTools: LoadedTool[],
newTools: Tool[],
): Tool[] {
const stamped: Tool[] = newTools.map((pluginTool) => {
const tool: Tool = {
Expand Down
19 changes: 13 additions & 6 deletions assistant/src/tools/tool-defaults.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
/**
* Single source of truth for the defaults applied when a `ToolDefinition`
* omits one of the normally-required fields, plus the `finalizeTool`
* helper that lifts a `ToolDefinition` into a `LoadedTool`.
* helper that lifts a `ToolDefinition` into a `Tool`.
*
* Plugins, external loaders, and any other registration boundary that
* accepts loose `ToolDefinition` objects from authors must run them
* through `finalizeTool` before handing the result to a `registerXxxTools`
* call. The registry types make this a hard rule: every registered tool
* is a `LoadedTool` (`Required<ToolDefinition> & { name }`).
* is a `Tool` (`Required<ToolDefinition> & { name }`).
*/

import type {
LoadedTool,
RiskLevel,
Tool,
ToolDefinition,
ToolExecutionResult,
} from "./types.js";
Expand All @@ -31,6 +31,10 @@ import type {
* - `executionTarget` defaults to `sandbox` — author-supplied tool code
* runs in the assistant container by default; opt in to `host` when
* the tool proxies work to the connected client.
* - `category` defaults to empty — Slack channel `allowedToolCategories`
* policy denies uncategorized tools when a category allow-list is set,
* which is the correct deny-by-default for tools the author didn't
* explicitly bucket.
*
* `execute` has no constant default because the default closure needs to
* close over the tool's name to produce a useful error message; see
Expand All @@ -45,12 +49,13 @@ export const TOOL_DEFAULTS = Object.freeze({
additionalProperties: false,
}) as object,
executionTarget: "sandbox" as const,
category: "",
});

/**
* Fill the four normally-required `ToolDefinition` fields with documented
* Fill the five normally-required `ToolDefinition` fields with documented
* defaults when the author omitted them, attach the registration-time
* `name`, and return a `LoadedTool` that is safe to hand to a
* `name`, and return a `Tool` that is safe to hand to a
* `registerXxxTools` call.
*
* The default `execute` returns an error result so the model sees a clear
Expand All @@ -61,7 +66,7 @@ export const TOOL_DEFAULTS = Object.freeze({
export function finalizeTool(
tool: ToolDefinition,
name: string,
): LoadedTool {
): Tool {
const description =
typeof tool.description === "string"
? tool.description
Expand All @@ -82,6 +87,7 @@ export function finalizeTool(
isError: true,
});
const executionTarget = tool.executionTarget ?? TOOL_DEFAULTS.executionTarget;
const category = tool.category ?? TOOL_DEFAULTS.category;
return {
...tool,
name,
Expand All @@ -90,5 +96,6 @@ export function finalizeTool(
input_schema,
executionTarget,
execute,
category,
};
}
8 changes: 3 additions & 5 deletions assistant/src/tools/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,12 @@ export interface ToolDefinition {
input: Record<string, unknown>,
context: ToolContext,
) => Promise<ToolExecutionResult>;
/** Tool category used for Slack channel `allowedToolCategories` enforcement. */
category?: string;
}

/** Tool after the loader has derived its name and filled defaults. */
export type LoadedTool = Required<ToolDefinition> & {
export type Tool = Required<ToolDefinition> & {
name: string;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the next PR, let's move this field into ToolDefinition too as optional, noting that we default to using the file name if not specified

};

Expand All @@ -355,7 +357,3 @@ export interface OwnerInfo {
/** ID of the owning extension (skill id / plugin name / MCP server id). */
id: string;
}

export interface Tool extends LoadedTool {
category: string;
}
Loading