diff --git a/knip.json b/knip.json index 9692fa3fb..67e825eee 100644 --- a/knip.json +++ b/knip.json @@ -19,15 +19,14 @@ "packages/appkit/src/plugin/index.ts", "packages/appkit/src/plugin/to-plugin.ts", "packages/appkit/src/plugins/agents/**", - "template/**", - "tools/**", - "docs/**", - ".github/scripts/**", + "packages/appkit/src/core/agent/index.ts", "packages/appkit/src/core/agent/tools/index.ts", - "packages/appkit/src/core/agent/from-plugin.ts", "packages/appkit/src/core/agent/load-agents.ts", "packages/appkit/src/connectors/mcp/index.ts", - "packages/appkit/src/plugin/to-plugin.ts" + "template/**", + "tools/**", + "docs/**", + ".github/scripts/**" ], "ignoreDependencies": ["json-schema-to-typescript"], "ignoreBinaries": ["tarball"] diff --git a/packages/appkit/src/beta.ts b/packages/appkit/src/beta.ts index 562cfd43a..3726ca178 100644 --- a/packages/appkit/src/beta.ts +++ b/packages/appkit/src/beta.ts @@ -52,8 +52,12 @@ export type { AgentDefinition, AgentsPluginConfig, AgentTool, + AgentTools, + AgentToolsFn, AutoInheritToolsConfig, BaseSystemPromptOption, + Plugins, + PluginToolkitProvider, PromptContext, RegisteredAgent, ResolvedToolEntry, diff --git a/packages/appkit/src/connectors/mcp/types.ts b/packages/appkit/src/connectors/mcp/types.ts index d74f0a46c..aeb61788e 100644 --- a/packages/appkit/src/connectors/mcp/types.ts +++ b/packages/appkit/src/connectors/mcp/types.ts @@ -1,7 +1,7 @@ /** * Input shape consumed by {@link AppKitMcpClient.connect}. Produced by the * agents plugin from user-facing `HostedTool` declarations (see - * `plugins/agents/tools/hosted-tools.ts`) and accepted directly by the + * `core/agent/tools/hosted-tools.ts`) and accepted directly by the * connector to keep its surface free of agent-layer concepts. */ export interface McpEndpointConfig { diff --git a/packages/appkit/src/core/agent/build-toolkit.ts b/packages/appkit/src/core/agent/build-toolkit.ts index 0140425d2..6e5a8ad19 100644 --- a/packages/appkit/src/core/agent/build-toolkit.ts +++ b/packages/appkit/src/core/agent/build-toolkit.ts @@ -1,4 +1,5 @@ import type { AgentToolDefinition } from "shared"; +import { applyToolkitOptions } from "./toolkit-options"; import type { ToolRegistry } from "./tools/define-tool"; import { toToolJSONSchema } from "./tools/json-schema"; import type { ToolkitEntry, ToolkitOptions } from "./types"; @@ -22,19 +23,11 @@ export function buildToolkitEntries( registry: ToolRegistry, opts: ToolkitOptions = {}, ): Record { - const prefix = opts.prefix ?? `${pluginName}.`; - const only = opts.only ? new Set(opts.only) : null; - const except = opts.except ? new Set(opts.except) : null; - const rename = opts.rename ?? {}; - const out: Record = {}; for (const [localName, entry] of Object.entries(registry)) { - if (only && !only.has(localName)) continue; - if (except?.has(localName)) continue; - - const keyAfterPrefix = `${prefix}${localName}`; - const key = rename[localName] ?? keyAfterPrefix; + const key = applyToolkitOptions(localName, pluginName, opts); + if (key === null) continue; const parameters = toToolJSONSchema( entry.schema, diff --git a/packages/appkit/src/core/agent/index.ts b/packages/appkit/src/core/agent/index.ts new file mode 100644 index 000000000..e35d483d4 --- /dev/null +++ b/packages/appkit/src/core/agent/index.ts @@ -0,0 +1,59 @@ +/** + * Agent runtime primitives. All framework-level agent types, tool helpers, + * and the standalone runner live here. The HTTP-facing `agents()` plugin in + * `plugins/agents/` consumes these but does not own them — peer plugins + * (analytics, files, genie, lakebase) can depend on this module without + * reaching across the sibling boundary. + */ +export { buildToolkitEntries } from "./build-toolkit"; +export { consumeAdapterStream } from "./consume-adapter-stream"; +export { createAgent } from "./create-agent"; +export { + agentIdFromMarkdownPath, + type LoadContext, + type LoadResult, + loadAgentFromFile, + loadAgentsFromDir, + parseFrontmatter, +} from "./load-agents"; +export { normalizeToolResult } from "./normalize-result"; +export { + type RunAgentInput, + type RunAgentResult, + runAgent, +} from "./run-agent"; +export { buildBaseSystemPrompt, composeSystemPrompt } from "./system-prompt"; +export { resolveToolkitFromProvider } from "./toolkit-resolver"; +export { + defineTool, + executeFromRegistry, + type FunctionTool, + functionToolToDefinition, + type HostedTool, + isFunctionTool, + isHostedTool, + mcpServer, + resolveHostedTools, + type ToolConfig, + type ToolEntry, + type ToolRegistry, + tool, + toolsFromRegistry, +} from "./tools"; +export { + type AgentDefinition, + type AgentsPluginConfig, + type AgentTool, + type AgentTools, + type AgentToolsFn, + type AutoInheritToolsConfig, + type BaseSystemPromptOption, + isToolkitEntry, + type Plugins, + type PluginToolkitProvider, + type PromptContext, + type RegisteredAgent, + type ResolvedToolEntry, + type ToolkitEntry, + type ToolkitOptions, +} from "./types"; diff --git a/packages/appkit/src/core/agent/plugins-map.ts b/packages/appkit/src/core/agent/plugins-map.ts new file mode 100644 index 000000000..181b2f7df --- /dev/null +++ b/packages/appkit/src/core/agent/plugins-map.ts @@ -0,0 +1,67 @@ +import type { Plugins, PluginToolkitProvider } from "./types"; + +/** + * Wrap a plain `Record` so that accessing an + * unknown plugin name throws with a named, actionable error instead of the + * default `TypeError: Cannot read properties of undefined (reading 'toolkit')` + * that surfaces from chained access on a missing key. + * + * The {@link Plugins} type is a `Record` + * without `noUncheckedIndexedAccess` workspace-wide, so unknown keys type as + * present at compile time but resolve to `undefined` at runtime. The proxy + * closes that gap with a runtime error that names the missing plugin and + * lists what's available — same shape as the agents plugin's pre-existing + * "plugin is not registered" errors. + * + * @param entries - the resolved plugin map; the proxy serves these directly + * for known keys and throws for any other string key. + * @param contextLabel - prefix included in the error message; differentiates + * the runtime context (e.g. `"runAgent: tools(plugins)"` vs + * `"AgentsPlugin: tools(plugins)"`) so users know which path to debug. + */ +export function createPluginsProxy( + entries: Record, + contextLabel: string, +): Plugins { + return new Proxy(entries, { + get(target, prop, receiver) { + // Symbols and well-known string accessors (e.g. `Symbol.toPrimitive`, + // `then` checked by Promise.resolve, `toJSON`) must pass through so + // host code that probes the object doesn't hit the missing-plugin + // error. Only named string-key access that misses is treated as a + // user error. + if (typeof prop !== "string") { + return Reflect.get(target, prop, receiver); + } + if (prop in target) { + return Reflect.get(target, prop, receiver); + } + // Probes from runtime tooling (e.g. utility inspection, JSON + // serialization, async-iterator detection) hit common property names + // that are clearly not plugin lookups. Pass those through silently. + if ( + prop === "then" || + prop === "toJSON" || + prop === "constructor" || + prop === "toString" || + prop === "valueOf" + ) { + return undefined; + } + const available = Object.keys(target).join(", ") || "(none)"; + throw new Error( + `${contextLabel} referenced plugin '${prop}', but it is not registered. ` + + `Available: ${available}.`, + ); + }, + has(target, prop) { + return Reflect.has(target, prop); + }, + ownKeys(target) { + return Reflect.ownKeys(target); + }, + getOwnPropertyDescriptor(target, prop) { + return Reflect.getOwnPropertyDescriptor(target, prop); + }, + }) as Plugins; +} diff --git a/packages/appkit/src/core/agent/run-agent.ts b/packages/appkit/src/core/agent/run-agent.ts index 9b5f57a2d..4dd4401ac 100644 --- a/packages/appkit/src/core/agent/run-agent.ts +++ b/packages/appkit/src/core/agent/run-agent.ts @@ -4,15 +4,26 @@ import type { AgentEvent, AgentToolDefinition, Message, + PluginConstructor, + PluginData, + ToolProvider, } from "shared"; import { consumeAdapterStream } from "./consume-adapter-stream"; +import { createPluginsProxy } from "./plugins-map"; +import { resolveToolkitFromProvider } from "./toolkit-resolver"; import { type FunctionTool, functionToolToDefinition, isFunctionTool, } from "./tools/function-tool"; import { isHostedTool } from "./tools/hosted-tools"; -import type { AgentDefinition, AgentTool, ToolkitEntry } from "./types"; +import type { + AgentDefinition, + AgentTool, + AgentTools, + Plugins, + PluginToolkitProvider, +} from "./types"; import { isToolkitEntry } from "./types"; export interface RunAgentInput { @@ -20,6 +31,14 @@ export interface RunAgentInput { messages: string | Message[]; /** Abort signal for cancellation. */ signal?: AbortSignal; + /** + * Optional plugin list. Required when `def.tools` is the function form + * `(plugins) => Record` and the function dereferences + * any plugins. `runAgent` constructs a fresh instance per plugin and + * dispatches tool calls against it as the service principal (no OBO — + * there is no HTTP request in standalone mode). + */ + plugins?: PluginData[]; } export interface RunAgentResult { @@ -34,21 +53,55 @@ export interface RunAgentResult { * inline tools, and drives the adapter's `run()` loop to completion. * * Limitations vs. running through the agents() plugin: - * - No OBO: there is no HTTP request, so plugin tools run as the service - * principal (when they work at all). - * - Plugin tools (`ToolkitEntry`) are not supported — they require a live - * `PluginContext` that only exists when registered in a `createApp` - * instance. This function throws a clear error if encountered. - * - Sub-agents (`agents: { ... }` on the def) are executed as nested - * `runAgent` calls with no shared thread state. + * - **No OBO and no approval gate** — there is no HTTP request, so plugin + * tools run as the service principal. The agents-plugin approval gate + * that prompts for human confirmation on `effect: "write" | "update" | + * "destructive"` tools is also absent. LLM-controlled tool arguments + * flow straight through to the SP. Treat standalone runAgent as a + * trusted-prompt environment (CI, batch eval, internal scripts) — not + * as an exposed user-facing surface. + * - **Hosted tools (MCP) are not supported** — they require a live MCP + * client that only exists inside the agents plugin's lifecycle. + * `runAgent` rejects them at index-build time with a clear error. + * - **Sub-agents** (`agents: { ... }` on the def) are executed as nested + * `runAgent` calls with no shared thread state. Plugin instances ARE + * shared across the recursion (same cache as the parent). + * - **Plugin tools** (used inside the function form via + * `plugins..toolkit(...)`) require passing `plugins: [...]` via + * `RunAgentInput`. Each plugin in that array is constructed once, + * `attachContext({})` and `await setup()` are called eagerly, and the + * resulting instance is shared across the top-level run and all + * sub-agent recursions. Plugins whose `setup()` requires runtime that + * only `createApp` provides (e.g. `WorkspaceClient`, `ServiceContext`, + * `PluginContext`) throw at standalone-init time with a clear "use + * createApp instead" message — not mid-stream. */ export async function runAgent( def: AgentDefinition, input: RunAgentInput, +): Promise { + // Single shared cache for the whole call graph: parent + every nested + // sub-agent dispatch share constructed plugin instances. Without this, + // each nested `runAgent` would build its own cache, re-instantiate every + // plugin, and silently diverge in-instance state between parent and child + // (e.g. query result caches, connection pools). + const providerCache = new Map(); + await initStandalonePlugins(input.plugins ?? [], providerCache); + return runAgentInternal(def, input, providerCache); +} + +async function runAgentInternal( + def: AgentDefinition, + input: RunAgentInput, + providerCache: Map, ): Promise { const adapter = await resolveAdapter(def); const messages = normalizeMessages(input.messages, def.instructions); - const toolIndex = buildStandaloneToolIndex(def); + const toolIndex = buildStandaloneToolIndex( + def, + input.plugins ?? [], + providerCache, + ); const tools = Array.from(toolIndex.values()).map((e) => e.def); const signal = input.signal; @@ -59,6 +112,13 @@ export async function runAgent( if (entry.kind === "function") { return entry.tool.execute(args as Record); } + if (entry.kind === "toolkit") { + return entry.provider.executeAgentTool( + entry.localName, + args as Record, + signal, + ); + } if (entry.kind === "subagent") { const subInput: RunAgentInput = { messages: @@ -68,13 +128,20 @@ export async function runAgent( ? (args as { input: string }).input : JSON.stringify(args), signal, + plugins: input.plugins, }; - const res = await runAgent(entry.agentDef, subInput); + // Reuse the same `providerCache` so sub-agent plugin tools dispatch + // through the same instances the parent constructed. + const res = await runAgentInternal( + entry.agentDef, + subInput, + providerCache, + ); return res.text; } throw new Error( `runAgent: tool "${name}" is a ${entry.kind} tool. ` + - "Plugin toolkits and MCP tools are only usable via createApp({ plugins: [..., agents(...)] }).", + "Hosted/MCP tools are only usable via createApp({ plugins: [..., agents(...)] }).", ); }; @@ -103,6 +170,73 @@ export async function runAgent( return { text, events }; } +/** + * Eagerly construct every plugin in `input.plugins`, run the standard + * AppKit lifecycle (`attachContext({})` + `await setup()`), and populate + * `cache`. Failures here surface BEFORE any adapter work — mid-stream + * `getWorkspaceClient is not initialised`-style errors become a clear + * startup failure naming the plugin and pointing the user at `createApp`. + * + * Plugins that don't need runtime context (no overridden `setup`, or one + * that doesn't dereference `createApp`-only state) initialise cleanly and + * standalone runAgent works as documented. Plugins like analytics/files + * that depend on `WorkspaceClient` will throw the underlying error wrapped + * with the migration hint. + */ +async function initStandalonePlugins( + plugins: PluginData[], + cache: Map, +): Promise { + for (const data of plugins) { + if (cache.has(data.name)) continue; + const instance = new data.plugin({ + ...(data.config ?? {}), + name: data.name, + }); + if (!isStandaloneToolProvider(instance)) { + throw new Error( + `runAgent: plugin '${data.name}' is not a ToolProvider ` + + "(missing getAgentTools/executeAgentTool). Only ToolProvider plugins " + + "are supported in standalone runAgent.", + ); + } + if ( + typeof (instance as { attachContext?: unknown }).attachContext === + "function" + ) { + try { + ( + instance as { attachContext: (deps: Record) => void } + ).attachContext({}); + } catch (err) { + throw new Error( + `runAgent: plugin '${data.name}' attachContext() failed in ` + + "standalone mode. This plugin probably depends on createApp's " + + "runtime (WorkspaceClient, ServiceContext, PluginContext). Run " + + "via createApp({ plugins: [..., agents(...)] }) instead. " + + `Cause: ${err instanceof Error ? err.message : String(err)}`, + { cause: err instanceof Error ? err : undefined }, + ); + } + } + if (typeof (instance as { setup?: unknown }).setup === "function") { + try { + await (instance as { setup: () => Promise | void }).setup(); + } catch (err) { + throw new Error( + `runAgent: plugin '${data.name}' setup() failed in standalone ` + + "mode. This plugin probably depends on createApp's runtime " + + "(WorkspaceClient, ServiceContext, PluginContext). Run via " + + "createApp({ plugins: [..., agents(...)] }) instead. " + + `Cause: ${err instanceof Error ? err.message : String(err)}`, + { cause: err instanceof Error ? err : undefined }, + ); + } + } + cache.set(data.name, instance); + } +} + async function resolveAdapter(def: AgentDefinition): Promise { const { model } = def; if (!model) { @@ -154,20 +288,32 @@ type StandaloneEntry = | { kind: "toolkit"; def: AgentToolDefinition; - entry: ToolkitEntry; + provider: ToolProvider; + pluginName: string; + localName: string; } | { kind: "hosted"; def: AgentToolDefinition; }; +/** + * Resolves `def.tools` (object or function form) and `def.agents` + * (sub-agents) into a flat dispatch index. The function form is invoked + * once per call against a {@link Plugins} map drawn from the shared + * `providerCache` populated by {@link initStandalonePlugins}. Missing + * references throw a named "not registered" error via the proxy. + */ function buildStandaloneToolIndex( def: AgentDefinition, + plugins: PluginData[], + providerCache: Map, ): Map { const index = new Map(); + const tools = resolveDefTools(def, plugins, providerCache); - for (const [key, tool] of Object.entries(def.tools ?? {})) { - index.set(key, classifyTool(key, tool)); + for (const [key, tool] of Object.entries(tools)) { + index.set(key, classifyTool(key, tool, providerCache)); } for (const [childKey, child] of Object.entries(def.agents ?? {})) { @@ -197,9 +343,75 @@ function buildStandaloneToolIndex( return index; } -function classifyTool(key: string, tool: AgentTool): StandaloneEntry { +/** + * Resolves `def.tools` to a plain record. The function form is invoked + * with a typed {@link Plugins} map drawn from the pre-populated + * `providerCache`; each `plugins.foo.toolkit(opts)` lookup hits the cache + * directly (no construction at toolkit-call time). + */ +function resolveDefTools( + def: AgentDefinition, + plugins: PluginData[], + providerCache: Map, +): AgentTools { + if (typeof def.tools !== "function") { + return def.tools ?? {}; + } + const pluginsMap = buildStandalonePluginsMap(plugins, providerCache); + try { + return def.tools(pluginsMap); + } catch (err) { + const name = def.name ?? ""; + throw new Error( + `runAgent: agent '${name}' tools(plugins) callback threw: ${ + err instanceof Error ? err.message : String(err) + }`, + { cause: err instanceof Error ? err : undefined }, + ); + } +} + +/** + * Builds the typed {@link Plugins} map passed to the function form of + * `def.tools` in standalone mode. Reads pre-constructed instances from + * `providerCache` (populated eagerly by {@link initStandalonePlugins}) + * and wraps the result in a Proxy so unknown plugin names produce a + * named "not registered, Available: ..." error instead of bubbling up a + * generic `TypeError: Cannot read properties of undefined`. + */ +function buildStandalonePluginsMap( + plugins: PluginData[], + providerCache: Map, +): Plugins { + const out: Record = {}; + for (const data of plugins) { + const provider = providerCache.get(data.name); + if (!provider) continue; // initStandalonePlugins should have set this + out[data.name] = { + toolkit: (opts) => resolveToolkitFromProvider(data.name, provider, opts), + }; + } + return createPluginsProxy(out, "runAgent: tools(plugins)"); +} + +function classifyTool( + key: string, + tool: AgentTool, + providerCache: Map, +): StandaloneEntry { if (isToolkitEntry(tool)) { - return { kind: "toolkit", def: { ...tool.def, name: key }, entry: tool }; + // Toolkit entries inside the function form's returned record carry the + // provider name they came from, so we can resolve the provider on + // demand and dispatch through it. The cache is shared with the + // pluginsMap path so the same instance is reused. + const provider = providerCacheLookup(tool.pluginName, providerCache); + return { + kind: "toolkit", + provider, + pluginName: tool.pluginName, + localName: tool.localName, + def: { ...tool.def, name: key }, + }; } if (isFunctionTool(tool)) { return { @@ -209,14 +421,47 @@ function classifyTool(key: string, tool: AgentTool): StandaloneEntry { }; } if (isHostedTool(tool)) { - return { - kind: "hosted", - def: { - name: key, - description: `Hosted tool: ${tool.type}`, - parameters: { type: "object", properties: {} }, - }, - }; + // Hosted tools (e.g. MCP `mcpServer(...)`) need a live MCP client that + // only exists inside the agents plugin's lifecycle. In standalone + // `runAgent` they would have errored at dispatch time with a confusing + // mid-conversation failure; reject them up front so misconfiguration + // surfaces before the adapter sees the tool list. + throw new Error( + `runAgent: tool "${key}" is a hosted tool (type="${tool.type}") which is only supported via createApp({ plugins: [..., agents(...)] }). Standalone runAgent has no MCP client.`, + ); } throw new Error(`runAgent: unrecognized tool shape at key "${key}"`); } + +function providerCacheLookup( + pluginName: string, + cache: Map, +): ToolProvider { + const cached = cache.get(pluginName); + if (cached) return cached; + const available = Array.from(cache.keys()).join(", ") || "(none)"; + throw new Error( + `runAgent: tool refers to plugin '${pluginName}', but no instance was ` + + "initialised for that name. Add it to RunAgentInput.plugins, or — if " + + "this came from a hand-rolled ToolkitEntry — go through " + + `plugins[name].toolkit() instead. Available: ${available}.`, + ); +} + +/** + * Lightweight `ToolProvider` shape check used by standalone `runAgent`. + * + * Distinct from `core/plugin-context.isToolProvider` which also requires + * `asUser` (request-scoped, only meaningful when running inside `createApp` + * with a live HTTP context). Standalone plugins are constructed without a + * `WorkspaceClient` and have no request to scope to, so checking only the + * two `ToolProvider` methods is the right narrowing here. + */ +function isStandaloneToolProvider(value: unknown): value is ToolProvider { + if (typeof value !== "object" || value === null) return false; + const obj = value as Record; + return ( + typeof obj.getAgentTools === "function" && + typeof obj.executeAgentTool === "function" + ); +} diff --git a/packages/appkit/src/core/agent/tests/create-agent.test.ts b/packages/appkit/src/core/agent/tests/create-agent.test.ts index a095439e5..46668e1a0 100644 --- a/packages/appkit/src/core/agent/tests/create-agent.test.ts +++ b/packages/appkit/src/core/agent/tests/create-agent.test.ts @@ -1,8 +1,8 @@ import { describe, expect, test } from "vitest"; import { z } from "zod"; -import { tool } from "../../../core/agent/tools/tool"; -import type { AgentDefinition } from "../../../core/agent/types"; import { createAgent } from "../create-agent"; +import { tool } from "../tools/tool"; +import type { AgentDefinition } from "../types"; describe("createAgent", () => { test("returns the definition unchanged for a simple agent", () => { @@ -28,7 +28,12 @@ describe("createAgent", () => { tools: { get_weather }, }); - expect(def.tools?.get_weather).toBe(get_weather); + // The object form preserves identity; we narrow with typeof to satisfy + // the dual ToolRecord | AgentToolsFn shape. + expect(typeof def.tools).toBe("object"); + if (typeof def.tools === "object") { + expect(def.tools.get_weather).toBe(get_weather); + } }); test("accepts sub-agents in a keyed record", () => { diff --git a/packages/appkit/src/plugins/agents/tests/mcp-server-helper.test.ts b/packages/appkit/src/core/agent/tests/mcp-server-helper.test.ts similarity index 100% rename from packages/appkit/src/plugins/agents/tests/mcp-server-helper.test.ts rename to packages/appkit/src/core/agent/tests/mcp-server-helper.test.ts diff --git a/packages/appkit/src/core/agent/tests/run-agent.test.ts b/packages/appkit/src/core/agent/tests/run-agent.test.ts index f94e8b7bd..4d60a8c96 100644 --- a/packages/appkit/src/core/agent/tests/run-agent.test.ts +++ b/packages/appkit/src/core/agent/tests/run-agent.test.ts @@ -3,13 +3,17 @@ import type { AgentEvent, AgentInput, AgentRunContext, + AgentToolDefinition, + PluginConstructor, + PluginData, + ToolProvider, } from "shared"; import { describe, expect, test, vi } from "vitest"; import { z } from "zod"; -import { tool } from "../../../core/agent/tools/tool"; -import type { ToolkitEntry } from "../../../core/agent/types"; import { createAgent } from "../create-agent"; import { runAgent } from "../run-agent"; +import { mcpServer } from "../tools/hosted-tools"; +import { tool } from "../tools/tool"; function scriptedAdapter(events: AgentEvent[]): AgentAdapter { return { @@ -84,17 +88,29 @@ describe("runAgent", () => { expect(weatherFn).toHaveBeenCalledWith({ city: "NYC" }); }); - test("throws a clear error when a ToolkitEntry is invoked", async () => { - const toolkitEntry: ToolkitEntry = { - __toolkitRef: true, - pluginName: "analytics", - localName: "query", - def: { - name: "analytics.query", - description: "SQL", - parameters: { type: "object", properties: {} }, - }, - }; + test("function-form tools(plugins) resolves toolkit() against RunAgentInput.plugins", async () => { + const pingExec = vi.fn(async () => "pong"); + class FakePlugin implements ToolProvider { + static manifest = { name: "ping" }; + static DEFAULT_CONFIG = {}; + name = "ping"; + constructor(public config: unknown) {} + async setup() {} + injectRoutes() {} + getEndpoints() { + return {}; + } + getAgentTools(): AgentToolDefinition[] { + return [ + { + name: "ping", + description: "ping", + parameters: { type: "object", properties: {} }, + }, + ]; + } + executeAgentTool = pingExec; + } let capturedCtx: AgentRunContext | null = null; const adapter: AgentAdapter = { @@ -107,14 +123,315 @@ describe("runAgent", () => { const def = createAgent({ instructions: "x", model: adapter, - tools: { "analytics.query": toolkitEntry }, + tools(plugins) { + return { ...plugins.ping.toolkit() }; + }, }); - await runAgent(def, { messages: "hi" }); + const pluginData: PluginData = { + plugin: FakePlugin as unknown as PluginConstructor, + config: {}, + name: "ping", + }; + + await runAgent(def, { messages: "hi", plugins: [pluginData] }); + expect(capturedCtx).not.toBeNull(); + // biome-ignore lint/style/noNonNullAssertion: asserted above + const result = await capturedCtx!.executeTool("ping.ping", {}); + expect(result).toBe("pong"); + expect(pingExec).toHaveBeenCalled(); + }); + + test("function-form throws with guidance when a referenced plugin is missing", async () => { + const adapter: AgentAdapter = { + async *run(_input, _context) { + yield { type: "message_delta", content: "" }; + }, + }; + + const def = createAgent({ + instructions: "x", + model: adapter, + tools(plugins) { + return { ...plugins.absent.toolkit() }; + }, + }); + + // No plugins passed → plugins.absent is undefined → toolkit() call throws. + await expect(runAgent(def, { messages: "hi" })).rejects.toThrow( + /tools\(plugins\) callback threw/, + ); + }); + + test("function-form rejects a plugin lacking ToolProvider methods", async () => { + class NotAToolProvider { + static manifest = { name: "noop" }; + static DEFAULT_CONFIG = {}; + name = "noop"; + constructor(public config: unknown) {} + async setup() {} + injectRoutes() {} + getEndpoints() { + return {}; + } + } + + const adapter: AgentAdapter = { + async *run(_input, _context) { + yield { type: "message_delta", content: "" }; + }, + }; + + const def = createAgent({ + instructions: "x", + model: adapter, + tools(plugins) { + return { ...plugins.noop.toolkit() }; + }, + }); + + const pluginData: PluginData = { + plugin: NotAToolProvider as unknown as PluginConstructor, + config: {}, + name: "noop", + }; + + await expect( + runAgent(def, { messages: "hi", plugins: [pluginData] }), + ).rejects.toThrow(/not a ToolProvider/); + }); + + test("rejects hosted/MCP tools at index-build time, not at dispatch", async () => { + // Pre-condition for #305 review finding #3: a hosted tool slipped into + // an agent def used with standalone runAgent must surface a clear error + // before the adapter sees the tool list — not later when the model + // emits a function_call mid-conversation. + const def = createAgent({ + instructions: "x", + // biome-ignore lint/suspicious/noExplicitAny: stub adapter — we never reach it + model: { async *run() {} } as any, + tools: { + analytics: mcpServer("analytics-mcp", "https://example.com/mcp"), + }, + }); + + await expect(runAgent(def, { messages: "hi" })).rejects.toThrow( + /hosted tool .* only supported via createApp/, + ); + }); + + test("recursively executes sub-agents declared on def.agents", async () => { + // Pre-condition for #305 review finding #8: parent's `agent-` tool + // call should kick a nested runAgent that returns the child's text + // output as the tool result. + const childAdapter: AgentAdapter = { + async *run(_input, _context) { + yield { type: "message_delta", content: "child says hi" }; + }, + }; + + let capturedCtx: AgentRunContext | null = null; + const parentAdapter: AgentAdapter = { + async *run(_input, context) { + capturedCtx = context; + yield { type: "message_delta", content: "" }; + }, + }; + + const parent = createAgent({ + instructions: "parent", + model: parentAdapter, + agents: { + helper: createAgent({ + instructions: "child", + model: childAdapter, + }), + }, + }); + + await runAgent(parent, { messages: "go" }); expect(capturedCtx).not.toBeNull(); + const result = + await // biome-ignore lint/style/noNonNullAssertion: asserted above + capturedCtx!.executeTool("agent-helper", { input: "say hi" }); + expect(result).toBe("child says hi"); + }); + + test("function-form invoked exactly once per runAgent call", async () => { + const toolsFn = vi.fn(() => ({})); + const adapter: AgentAdapter = { + async *run(_input, _context) { + yield { type: "message_delta", content: "" }; + }, + }; + const def = createAgent({ + instructions: "x", + model: adapter, + tools: toolsFn, + }); + await runAgent(def, { messages: "hi" }); + expect(toolsFn).toHaveBeenCalledTimes(1); + }); + + test("function-form names the missing plugin in the error (proxy)", async () => { + // Regression: previously `plugins.absent` was undefined, and accessing + // `.toolkit()` on it produced a generic "Cannot read properties of + // undefined" — no plugin name, no list of available names. The proxy + // now throws a clear "not registered. Available: ..." error. + const adapter: AgentAdapter = { + async *run(_input, _context) { + yield { type: "message_delta", content: "" }; + }, + }; + const def = createAgent({ + instructions: "x", + model: adapter, + tools(plugins) { + return { ...plugins.absent.toolkit() }; + }, + }); + await expect(runAgent(def, { messages: "hi" })).rejects.toThrow(/'absent'/); + await expect(runAgent(def, { messages: "hi" })).rejects.toThrow( + /not registered/, + ); + await expect(runAgent(def, { messages: "hi" })).rejects.toThrow( + /Available/, + ); + }); + + test("plugin setup() failure surfaces at runAgent entry, not mid-stream", async () => { + // Regression: previously plugins were constructed lazily during the + // function form's toolkit() call, and setup() was never invoked. That + // pushed any setup-time failure (e.g. `getWorkspaceClient is not + // initialised`) into mid-conversation tool dispatches with confusing + // stack traces. The fix: eagerly attachContext + setup at runAgent + // entry; failures wrap with a "use createApp instead" hint. + class BadSetupPlugin { + static manifest = { name: "bad" }; + static DEFAULT_CONFIG = {}; + name = "bad"; + constructor(public config: unknown) {} + async setup() { + throw new Error("WorkspaceClient not initialised"); + } + injectRoutes() {} + getEndpoints() { + return {}; + } + getAgentTools() { + return []; + } + async executeAgentTool() { + return null; + } + } + const adapter: AgentAdapter = { + async *run(_input, _context) { + // Should never reach the adapter. + yield { type: "message_delta", content: "this should not run" }; + }, + }; + const def = createAgent({ + instructions: "x", + model: adapter, + tools(plugins) { + return { ...plugins.bad.toolkit() }; + }, + }); + const pluginData: PluginData = { + plugin: BadSetupPlugin as unknown as PluginConstructor, + config: {}, + name: "bad", + }; + await expect( - // biome-ignore lint/style/noNonNullAssertion: asserted above - capturedCtx!.executeTool("analytics.query", {}), - ).rejects.toThrow(/only usable via createApp/); + runAgent(def, { messages: "hi", plugins: [pluginData] }), + ).rejects.toThrow(/setup\(\) failed in standalone mode/); + await expect( + runAgent(def, { messages: "hi", plugins: [pluginData] }), + ).rejects.toThrow(/createApp/); + }); + + test("sub-agent recursion shares the same plugin instance with the parent", async () => { + // Regression: providerCache used to be per-call inside + // buildStandaloneToolIndex, so each nested runAgent constructed fresh + // plugin instances and parent/child diverged in-instance state. + let constructorCount = 0; + class StatefulPlugin { + static manifest = { name: "stateful" }; + static DEFAULT_CONFIG = {}; + name = "stateful"; + readonly id = ++constructorCount; + constructor(public config: unknown) {} + async setup() {} + injectRoutes() {} + getEndpoints() { + return {}; + } + getAgentTools() { + return [ + { + name: "whoami", + description: "return instance id", + parameters: { type: "object", properties: {} }, + }, + ]; + } + async executeAgentTool() { + return String(this.id); + } + } + + const childAdapter: AgentAdapter = { + async *run(_input, context) { + const id = await context.executeTool("stateful.whoami", {}); + yield { type: "message_delta", content: `child-id=${id}` }; + }, + }; + const parentAdapter: AgentAdapter = { + async *run(_input, context) { + const myId = await context.executeTool("stateful.whoami", {}); + const childOut = await context.executeTool("agent-child", { + input: "go", + }); + yield { + type: "message_delta", + content: `parent-id=${myId};${childOut}`, + }; + }, + }; + + const child = createAgent({ + instructions: "child", + model: childAdapter, + tools(plugins) { + return { ...plugins.stateful.toolkit() }; + }, + }); + const parent = createAgent({ + instructions: "parent", + model: parentAdapter, + agents: { child }, + tools(plugins) { + return { ...plugins.stateful.toolkit() }; + }, + }); + + const pluginData: PluginData = { + plugin: StatefulPlugin as unknown as PluginConstructor, + config: {}, + name: "stateful", + }; + + constructorCount = 0; + const result = await runAgent(parent, { + messages: "go", + plugins: [pluginData], + }); + + // Plugin constructed exactly once across parent + child. + expect(constructorCount).toBe(1); + // Both parent and child reported the same instance id. + expect(result.text).toBe("parent-id=1;child-id=1"); }); }); diff --git a/packages/appkit/src/core/agent/tests/toolkit-options.test.ts b/packages/appkit/src/core/agent/tests/toolkit-options.test.ts new file mode 100644 index 000000000..763aa4f55 --- /dev/null +++ b/packages/appkit/src/core/agent/tests/toolkit-options.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, test } from "vitest"; +import { applyToolkitOptions } from "../toolkit-options"; + +describe("applyToolkitOptions", () => { + test("default prefix is `${pluginName}.`", () => { + expect(applyToolkitOptions("query", "analytics")).toBe("analytics.query"); + }); + + test("empty `prefix` drops the namespace", () => { + expect(applyToolkitOptions("query", "analytics", { prefix: "" })).toBe( + "query", + ); + }); + + test("explicit prefix overrides the default", () => { + expect(applyToolkitOptions("query", "analytics", { prefix: "sql_" })).toBe( + "sql_query", + ); + }); + + test("`only` allowlist filters out unmatched names", () => { + expect( + applyToolkitOptions("destructive", "analytics", { only: ["query"] }), + ).toBeNull(); + expect(applyToolkitOptions("query", "analytics", { only: ["query"] })).toBe( + "analytics.query", + ); + }); + + test("`except` denylist drops matched names", () => { + expect( + applyToolkitOptions("query", "analytics", { except: ["query"] }), + ).toBeNull(); + }); + + test("`rename` wins over the prefix path", () => { + expect( + applyToolkitOptions("query", "analytics", { + rename: { query: "sql_query" }, + }), + ).toBe("sql_query"); + }); + + test("`rename: { name: undefined }` falls through to the prefix path", () => { + // Regression: prior implementation used `Object.hasOwn(rename, name) ? + // rename[name] : ...` which returned `undefined` when the key was + // present-but-undefined (e.g. from a ternary that didn't fire). That + // produced a tool keyed literally `"undefined"` downstream. + expect( + applyToolkitOptions("query", "analytics", { + rename: { query: undefined as unknown as string }, + }), + ).toBe("analytics.query"); + }); + + test("`rename: { name: '' }` falls through (empty string is not a valid key)", () => { + expect( + applyToolkitOptions("query", "analytics", { + rename: { query: "" }, + }), + ).toBe("analytics.query"); + }); +}); diff --git a/packages/appkit/src/core/agent/toolkit-options.ts b/packages/appkit/src/core/agent/toolkit-options.ts new file mode 100644 index 000000000..6ac075425 --- /dev/null +++ b/packages/appkit/src/core/agent/toolkit-options.ts @@ -0,0 +1,36 @@ +import type { ToolkitOptions } from "./types"; + +/** + * Filter / prefix / rename a tool's local name into its toolkit key, or + * skip it entirely. Encapsulates the four-knob `ToolkitOptions` contract: + * + * - `only` — allowlist of local names; everything else is dropped. + * - `except` — denylist of local names. + * - `prefix` — string prepended to the local name; defaults to + * `${pluginName}.`. Pass `""` to drop the prefix entirely. + * - `rename` — per-tool exact remapping; wins over `prefix`. + * + * Returns the final key, or `null` when the tool is filtered out. + * + * Single source of truth so {@link buildToolkitEntries} (registry path) + * and {@link resolveToolkitFromProvider} (`getAgentTools()` fallback) stay + * in lockstep — bug fixes here apply to both. + */ +export function applyToolkitOptions( + localName: string, + pluginName: string, + opts: ToolkitOptions = {}, +): string | null { + // `only`/`except` take precedence: filter first, then derive the key. + if (opts.only && !opts.only.includes(localName)) return null; + if (opts.except?.includes(localName)) return null; + + // `rename` accepts string overrides; explicit `undefined` (e.g. from a + // ternary that didn't fire) and empty strings fall through to the prefix + // path so we never produce a tool keyed literally `"undefined"` or `""`. + const renamed = opts.rename?.[localName]; + if (typeof renamed === "string" && renamed.length > 0) return renamed; + + const prefix = opts.prefix ?? `${pluginName}.`; + return `${prefix}${localName}`; +} diff --git a/packages/appkit/src/core/agent/toolkit-resolver.ts b/packages/appkit/src/core/agent/toolkit-resolver.ts new file mode 100644 index 000000000..97d1d9b30 --- /dev/null +++ b/packages/appkit/src/core/agent/toolkit-resolver.ts @@ -0,0 +1,55 @@ +import type { ToolProvider } from "shared"; +import { applyToolkitOptions } from "./toolkit-options"; +import type { ToolkitEntry, ToolkitOptions } from "./types"; + +/** + * Internal interface: a `ToolProvider` that optionally exposes a typed + * `.toolkit(opts)` method. Core plugins (analytics, files, genie, lakebase) + * implement this; third-party `ToolProvider`s may not. + */ +type MaybeToolkitProvider = ToolProvider & { + toolkit?: (opts?: ToolkitOptions) => Record; +}; + +/** + * Resolve a plugin's tools into a keyed record of {@link ToolkitEntry} markers + * ready to be merged into an agent's tool index. + * + * Preferred path: call the plugin's own `.toolkit(opts)` method, which + * typically delegates to `buildToolkitEntries` with full `ToolkitOptions` + * support (prefix, only, except, rename). + * + * Fallback path: when the plugin doesn't expose `.toolkit()` (e.g. a + * third-party `ToolProvider` built with plain `toPlugin`), walk + * `getAgentTools()` and synthesize namespaced keys (`${pluginName}.${name}`) + * while still honoring `only` / `except` / `rename` / `prefix`. + * + * This helper is the single source of truth for "turn a provider into a + * toolkit entry record" and is used by `AgentsPlugin.buildToolIndex` + * (the `tools(plugins)` resolution pass and auto-inherit) and by the + * standalone `runAgent` executor. + */ +export function resolveToolkitFromProvider( + pluginName: string, + provider: ToolProvider, + opts?: ToolkitOptions, +): Record { + const withToolkit = provider as MaybeToolkitProvider; + if (typeof withToolkit.toolkit === "function") { + return withToolkit.toolkit(opts); + } + + const out: Record = {}; + for (const tool of provider.getAgentTools()) { + const key = applyToolkitOptions(tool.name, pluginName, opts); + if (key === null) continue; + + out[key] = { + __toolkitRef: true, + pluginName, + localName: tool.name, + def: { ...tool, name: key }, + }; + } + return out; +} diff --git a/packages/appkit/src/core/agent/tools/define-tool.ts b/packages/appkit/src/core/agent/tools/define-tool.ts index dc269ba61..b51792f4c 100644 --- a/packages/appkit/src/core/agent/tools/define-tool.ts +++ b/packages/appkit/src/core/agent/tools/define-tool.ts @@ -20,7 +20,8 @@ export interface ToolEntry { * consider it safe enough to appear in every agent's tool record without an * explicit `tools:` declaration. Destructive or privilege-sensitive tools * should leave this unset so that they only reach agents that wire them - * explicitly (via `tools:`, `toolkits:`, or `fromPlugin({ only: [...] })`). + * explicitly (via `tools:` object/function form, markdown `toolkits:`, or + * `plugins..toolkit({ only: [...] })`). */ autoInheritable?: boolean; handler: ( diff --git a/packages/appkit/src/core/agent/types.ts b/packages/appkit/src/core/agent/types.ts index 50e527523..4ff7d31ed 100644 --- a/packages/appkit/src/core/agent/types.ts +++ b/packages/appkit/src/core/agent/types.ts @@ -48,6 +48,43 @@ export interface ToolkitOptions { rename?: Record; } +/** + * Minimum shape every entry in the {@link Plugins} map must expose. Core + * plugins (analytics, files, genie, lakebase) implement this directly via + * their `.toolkit()` method. The agents plugin and standalone `runAgent` + * synthesize this shape for any registered plugin that doesn't implement + * `.toolkit()` directly (falling back to `getAgentTools()` walking). + */ +export interface PluginToolkitProvider { + toolkit(opts?: ToolkitOptions): Record; +} + +/** + * Plugin map passed to the function form of {@link AgentDefinition.tools}. + * Each entry exposes a `.toolkit(opts?)` method that returns a record of + * {@link ToolkitEntry} markers ready to be spread into a tool record. + * + * AppKit does not statically know which plugins the surrounding + * `createApp` will register, so this is a plain string-keyed record. + * Refer to plugins by the name used in `createApp({ plugins: [...] })`; + * unknown names resolve to `undefined` at runtime. + * + * @example + * ```ts + * const support = createAgent({ + * instructions: "...", + * tools(plugins) { + * return { + * get_weather: tool({ ... }), + * ...plugins.analytics.toolkit(), + * ...plugins.files.toolkit({ only: ["uploads.read"] }), + * }; + * }, + * }); + * ``` + */ +export type Plugins = Record; + /** * Context passed to `baseSystemPrompt` callbacks. */ @@ -62,6 +99,23 @@ export type BaseSystemPromptOption = | string | ((ctx: PromptContext) => string); +/** + * Per-agent tool record. String keys map to inline tools, toolkit entries, + * hosted tools, etc. + */ +export type AgentTools = Record; + +/** + * Function form of `AgentDefinition.tools`. Receives the typed + * {@link Plugins} map and returns a tool record. Invoked exactly once at + * setup (or once per `runAgent` call in standalone mode); the result is + * cached as the agent's resolved tool record. + * + * Use the function form when an agent needs tools from registered plugins. + * The bare object form is fine when an agent only uses inline tools. + */ +export type AgentToolsFn = (plugins: Plugins) => AgentTools; + export interface AgentDefinition { /** Filled in from the enclosing key when used in `agents: { foo: def }`. */ name?: string; @@ -73,8 +127,18 @@ export interface AgentDefinition { * falls back to the plugin's `defaultModel`. */ model?: AgentAdapter | Promise | string; - /** Per-agent tool record. Key is the LLM-visible tool-call name. */ - tools?: Record; + /** + * Per-agent tool record. Key is the LLM-visible tool-call name. + * + * Accepts either a plain record (for agents that only use inline tools) + * or a function `(plugins) => Record` that receives + * the typed {@link Plugins} map and returns a tool record (for agents + * that pull tools from registered plugins). + * + * The function is invoked once at agent setup; the result is cached. + * Don't put per-request logic in there. + */ + tools?: AgentTools | AgentToolsFn; /** Sub-agents, exposed as `agent-` tools on this agent. */ agents?: Record; /** Override the plugin's baseSystemPrompt for this agent only. */ @@ -97,7 +161,8 @@ export interface AgentDefinition { * with no explicit `tools:` declaration receive every registered ToolProvider * plugin tool whose author marked `autoInheritable: true`. Tools without that * flag — destructive, state-mutating, or privilege-sensitive — never spread - * automatically and must be wired via `tools:`, `toolkits:`, or `fromPlugin`. + * automatically and must be wired via `tools:` (object or function form) or + * markdown `toolkits:`. * * Defaults are `false` for both origins (safe-by-default): developers must * consciously opt an origin in to any auto-inherit behaviour. diff --git a/packages/appkit/src/plugin/index.ts b/packages/appkit/src/plugin/index.ts index 46a4eb94f..937652196 100644 --- a/packages/appkit/src/plugin/index.ts +++ b/packages/appkit/src/plugin/index.ts @@ -1,4 +1,4 @@ export type { ToPlugin } from "shared"; export type { ExecutionResult } from "./execution-result"; export { Plugin } from "./plugin"; -export { type NamedPluginFactory, toPlugin } from "./to-plugin"; +export { toPlugin } from "./to-plugin"; diff --git a/packages/appkit/src/plugin/to-plugin.ts b/packages/appkit/src/plugin/to-plugin.ts index c882f3000..785d96b9a 100644 --- a/packages/appkit/src/plugin/to-plugin.ts +++ b/packages/appkit/src/plugin/to-plugin.ts @@ -1,27 +1,15 @@ import type { PluginConstructor, PluginData, ToPlugin } from "shared"; -/** - * Factory function produced by {@link toPlugin}. Carries a static - * `pluginName` field so tooling (e.g. `fromPlugin`) can identify which - * plugin a factory references without constructing an instance. - */ -export type NamedPluginFactory = { - readonly pluginName: Name; -}; - /** * Wraps a plugin class so it can be passed to `createApp` with optional * config. Infers the config type from the constructor and the plugin name - * from the static `manifest.name` property, and stamps `pluginName` onto - * the returned factory function so `fromPlugin` can identify the plugin - * without needing to construct it. + * from the static `manifest.name` property. * * @internal */ export function toPlugin( plugin: T, -): ToPlugin[0], T["manifest"]["name"]> & - NamedPluginFactory { +): ToPlugin[0], T["manifest"]["name"]> { type Config = ConstructorParameters[0]; type Name = T["manifest"]["name"]; const pluginName = plugin.manifest.name as Name; @@ -32,10 +20,5 @@ export function toPlugin( config: config as Config, name: pluginName, }); - Object.defineProperty(factory, "pluginName", { - value: pluginName, - writable: false, - enumerable: true, - }); - return factory as ToPlugin & NamedPluginFactory; + return factory as ToPlugin; } diff --git a/packages/appkit/src/plugins/agents/agents.ts b/packages/appkit/src/plugins/agents/agents.ts index b0a64d3c2..3c20d6165 100644 --- a/packages/appkit/src/plugins/agents/agents.ts +++ b/packages/appkit/src/plugins/agents/agents.ts @@ -16,13 +16,16 @@ import type { ToolProvider, } from "shared"; import { AppKitMcpClient, buildMcpHostPolicy } from "../../connectors/mcp"; +import { getWorkspaceClient } from "../../context"; import { consumeAdapterStream } from "../../core/agent/consume-adapter-stream"; import { loadAgentsFromDir } from "../../core/agent/load-agents"; import { normalizeToolResult } from "../../core/agent/normalize-result"; +import { createPluginsProxy } from "../../core/agent/plugins-map"; import { buildBaseSystemPrompt, composeSystemPrompt, } from "../../core/agent/system-prompt"; +import { resolveToolkitFromProvider } from "../../core/agent/toolkit-resolver"; import { functionToolToDefinition, isFunctionTool, @@ -32,7 +35,10 @@ import { import type { AgentDefinition, AgentsPluginConfig, + AgentTools, BaseSystemPromptOption, + Plugins, + PluginToolkitProvider, PromptContext, RegisteredAgent, ResolvedToolEntry, @@ -134,6 +140,13 @@ export class AgentsPlugin extends Plugin implements ToolProvider { string, { controller: AbortController; userId: string } >(); + /** + * Per-user stream count, kept in sync with `activeStreams` so the + * concurrent-stream rate limit check is O(1) instead of O(n) over every + * active stream on every request. Mutated only via {@link trackStream} + * and {@link untrackStream}. + */ + private userStreamCounts = new Map(); private mcpClient: AppKitMcpClient | null = null; private threadStore; private approvalGate = new ToolApprovalGate(); @@ -190,13 +203,44 @@ export class AgentsPlugin extends Plugin implements ToolProvider { }; } - /** Count active streams owned by a given user. */ + /** Count active streams owned by a given user. O(1). */ private countUserStreams(userId: string): number { - let n = 0; - for (const entry of this.activeStreams.values()) { - if (entry.userId === userId) n++; + return this.userStreamCounts.get(userId) ?? 0; + } + + /** + * Register a stream for `userId` and bump the per-user counter. Paired + * with {@link untrackStream}; the two helpers are the only writers to + * `activeStreams` + `userStreamCounts`, so the counter cannot drift from + * the map. + */ + private trackStream( + requestId: string, + userId: string, + controller: AbortController, + ): void { + this.activeStreams.set(requestId, { controller, userId }); + this.userStreamCounts.set( + userId, + (this.userStreamCounts.get(userId) ?? 0) + 1, + ); + } + + /** + * Remove a stream from the active map and decrement the per-user + * counter. Idempotent — calling twice for the same `requestId` is a + * no-op (the second call sees no entry and returns early). + */ + private untrackStream(requestId: string): void { + const entry = this.activeStreams.get(requestId); + if (!entry) return; + this.activeStreams.delete(requestId); + const next = (this.userStreamCounts.get(entry.userId) ?? 1) - 1; + if (next <= 0) { + this.userStreamCounts.delete(entry.userId); + } else { + this.userStreamCounts.set(entry.userId, next); } - return n; } async setup() { @@ -385,7 +429,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { ); } catch (err) { throw new Error( - `Agent '${name}' has no model configured and no DATABRICKS_AGENT_ENDPOINT default available`, + `Agent '${name}' has no model configured and no DATABRICKS_SERVING_ENDPOINT_NAME default available`, { cause: err instanceof Error ? err : undefined }, ); } @@ -408,13 +452,16 @@ export class AgentsPlugin extends Plugin implements ToolProvider { src: AgentSource, ): Promise> { const index = new Map(); - const hasExplicitTools = def.tools && Object.keys(def.tools).length > 0; + const hasDeclaredTools = def.tools !== undefined; + const toolsRecord = this.resolveDefTools(agentName, def); const hasExplicitSubAgents = def.agents && Object.keys(def.agents).length > 0; const inheritDefaults = normalizeAutoInherit(this.config.autoInheritTools); + // Declaring `tools` (object or function, even an empty record) opts out + // of auto-inherit. Same rule for both forms — see plan decision (E1/I1). const shouldInherit = - !hasExplicitTools && + !hasDeclaredTools && !hasExplicitSubAgents && (src.origin === "file" ? inheritDefaults.file : inheritDefaults.code); @@ -450,7 +497,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { // 2. Explicit tools (toolkit entries, function tools, hosted tools) const hostedToCollect: import("../../core/agent/tools/hosted-tools").HostedTool[] = []; - for (const [key, tool] of Object.entries(def.tools ?? {})) { + for (const [key, tool] of Object.entries(toolsRecord)) { if (isToolkitEntry(tool)) { index.set(key, { source: "toolkit", @@ -484,6 +531,63 @@ export class AgentsPlugin extends Plugin implements ToolProvider { return index; } + /** + * Resolves an `AgentDefinition.tools` field to a plain tool record. The + * function form is invoked exactly once at agent setup with the typed + * {@link Plugins} map; the result replaces the function reference for the + * remainder of the registered agent's lifetime. + * + * Plain object form is returned as-is; an undefined `tools` returns an + * empty record. The function form is wrapped in a try/catch so a thrown + * callback fails registration with a useful message instead of leaking + * the raw stack. + */ + private resolveDefTools(agentName: string, def: AgentDefinition): AgentTools { + if (typeof def.tools !== "function") { + return def.tools ?? {}; + } + try { + return def.tools(this.buildPluginsMap()); + } catch (err) { + throw new Error( + `Agent '${agentName}': tools(plugins) callback threw: ${ + err instanceof Error ? err.message : String(err) + }`, + { cause: err instanceof Error ? err : undefined }, + ); + } + } + + /** + * Builds the typed {@link Plugins} map passed to the function form of + * `AgentDefinition.tools`. Each entry exposes the plugin instance directly + * (so user code can call typed instance methods including `.toolkit()`); + * plugins missing `.toolkit()` get a synthesized fallback that walks + * `getAgentTools()` via `resolveToolkitFromProvider`. + * + * Wrapped in {@link createPluginsProxy} so that accessing an unknown + * plugin name throws a named "not registered, Available: ..." error + * instead of bubbling up a generic `Cannot read properties of undefined` + * from the agent's `tools(plugins)` callback. + */ + private buildPluginsMap(): Plugins { + const out: Record = {}; + if (!this.context) { + return createPluginsProxy(out, `Agent '${this.name}': tools(plugins)`); + } + for (const { name, provider } of this.context.getToolProviders()) { + const direct = (provider as { toolkit?: unknown }).toolkit; + if (typeof direct === "function") { + out[name] = provider as unknown as PluginToolkitProvider; + } else { + out[name] = { + toolkit: (opts) => resolveToolkitFromProvider(name, provider, opts), + }; + } + } + return createPluginsProxy(out, `Agent '${this.name}': tools(plugins)`); + } + private async applyAutoInherit( agentName: string, index: Map, @@ -502,32 +606,19 @@ export class AgentsPlugin extends Plugin implements ToolProvider { provider, } of this.context.getToolProviders()) { if (pluginName === this.name) continue; - const withToolkit = provider as ToolProvider & { - toolkit?: (opts?: unknown) => Record; - }; - if (typeof withToolkit.toolkit === "function") { - const entries = withToolkit.toolkit() as Record; - for (const [key, maybeEntry] of Object.entries(entries)) { - if (!isToolkitEntry(maybeEntry)) continue; - if (maybeEntry.autoInheritable !== true) { - recordSkip(maybeEntry.pluginName, maybeEntry.localName); - continue; - } - index.set(key, { - source: "toolkit", - pluginName: maybeEntry.pluginName, - localName: maybeEntry.localName, - def: { ...maybeEntry.def, name: key }, - }); - inherited.push(key); + const entries = resolveToolkitFromProvider(pluginName, provider); + for (const [key, entry] of Object.entries(entries)) { + if (entry.autoInheritable !== true) { + recordSkip(entry.pluginName, entry.localName); + continue; } - continue; - } - // Fallback: providers without a toolkit() still expose getAgentTools(). - // These cannot be selectively opted in per tool, so we conservatively - // skip them during auto-inherit and require explicit `tools:` wiring. - for (const tool of provider.getAgentTools()) { - recordSkip(pluginName, tool.name); + index.set(key, { + source: "toolkit", + pluginName: entry.pluginName, + localName: entry.localName, + def: { ...entry.def, name: key }, + }); + inherited.push(key); } } @@ -720,22 +811,34 @@ export class AgentsPlugin extends Plugin implements ToolProvider { return; } - let thread = threadId ? await this.threadStore.get(threadId, userId) : null; - if (threadId && !thread) { - res.status(404).json({ error: `Thread ${threadId} not found` }); + // ThreadStore can throw on backing-storage failures (DB unreachable, + // permission errors, transient I/O). Without a try/catch the + // `async` Express handler bubbles the rejection without a response and + // the client connection hangs until the proxy times out. Surface the + // failure as a 500 so the SSE client falls back instead of waiting. + let thread: Thread; + try { + const existing = threadId + ? await this.threadStore.get(threadId, userId) + : null; + if (threadId && !existing) { + res.status(404).json({ error: `Thread ${threadId} not found` }); + return; + } + thread = existing ?? (await this.threadStore.create(userId)); + + const userMessage: Message = { + id: randomUUID(), + role: "user", + content: message, + createdAt: new Date(), + }; + await this.threadStore.addMessage(thread.id, userId, userMessage); + } catch (err) { + logger.error("threadStore failed in /chat: %O", err); + res.status(500).json({ error: "Thread operation failed" }); return; } - if (!thread) { - thread = await this.threadStore.create(userId); - } - - const userMessage: Message = { - id: randomUUID(), - role: "user", - content: message, - createdAt: new Date(), - }; - await this.threadStore.addMessage(thread.id, userId, userMessage); return this._streamAgent(req, res, registered, thread, userId); } @@ -770,30 +873,39 @@ export class AgentsPlugin extends Plugin implements ToolProvider { return; } - const thread = await this.threadStore.create(userId); + // Same rationale as `_handleChat`: surface threadStore failures as a + // 500 instead of letting the async handler hang the client connection. + let thread: Thread; + try { + thread = await this.threadStore.create(userId); - if (typeof input === "string") { - await this.threadStore.addMessage(thread.id, userId, { - id: randomUUID(), - role: "user", - content: input, - createdAt: new Date(), - }); - } else { - for (const item of input) { - const role = (item.role ?? "user") as Message["role"]; - const content = - typeof item.content === "string" - ? item.content - : JSON.stringify(item.content ?? ""); - if (!content) continue; + if (typeof input === "string") { await this.threadStore.addMessage(thread.id, userId, { id: randomUUID(), - role, - content, + role: "user", + content: input, createdAt: new Date(), }); + } else { + for (const item of input) { + const role = (item.role ?? "user") as Message["role"]; + const content = + typeof item.content === "string" + ? item.content + : JSON.stringify(item.content ?? ""); + if (!content) continue; + await this.threadStore.addMessage(thread.id, userId, { + id: randomUUID(), + role, + content, + createdAt: new Date(), + }); + } } + } catch (err) { + logger.error("threadStore failed in /invocations: %O", err); + res.status(500).json({ error: "Thread operation failed" }); + return; } return this._streamAgent(req, res, registered, thread, userId); @@ -809,7 +921,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { const abortController = new AbortController(); const signal = abortController.signal; const requestId = randomUUID(); - this.activeStreams.set(requestId, { controller: abortController, userId }); + this.trackStream(requestId, userId, abortController); const tools = Array.from(registered.toolIndex.values()).map((e) => e.def); const approvalPolicy = this.resolvedApprovalPolicy; @@ -919,7 +1031,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { // Any pending approval gates for this stream are auto-denied so the // adapter can unwind if it was still waiting. this.approvalGate.abortStream(requestId); - this.activeStreams.delete(requestId); + this.untrackStream(requestId); // Stateless agents (e.g. autocomplete) don't persist history; drop // the thread so `InMemoryThreadStore` doesn't accumulate one record // per request. Swallow delete errors — the stream has already @@ -1180,7 +1292,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { return; } entry.controller.abort("Cancelled by user"); - this.activeStreams.delete(streamId); + this.untrackStream(streamId); this.approvalGate.abortStream(streamId); res.json({ cancelled: true }); } diff --git a/packages/appkit/src/plugins/agents/index.ts b/packages/appkit/src/plugins/agents/index.ts index 0c8964109..869333afb 100644 --- a/packages/appkit/src/plugins/agents/index.ts +++ b/packages/appkit/src/plugins/agents/index.ts @@ -1,3 +1,6 @@ +// Re-exports of agent primitives that now live in core/agent/. Kept here so +// the public package barrel (`@databricks/appkit`) and any callers that +// already imported via `./plugins/agents` continue to resolve unchanged. export { buildToolkitEntries } from "../../core/agent/build-toolkit"; export { agentIdFromMarkdownPath, @@ -11,9 +14,13 @@ export { type AgentDefinition, type AgentsPluginConfig, type AgentTool, + type AgentTools, + type AgentToolsFn, type AutoInheritToolsConfig, type BaseSystemPromptOption, isToolkitEntry, + type Plugins, + type PluginToolkitProvider, type PromptContext, type RegisteredAgent, type ResolvedToolEntry, diff --git a/packages/appkit/src/plugins/agents/manifest.json b/packages/appkit/src/plugins/agents/manifest.json index a33f75ce4..922a10fee 100644 --- a/packages/appkit/src/plugins/agents/manifest.json +++ b/packages/appkit/src/plugins/agents/manifest.json @@ -12,12 +12,12 @@ "type": "serving_endpoint", "alias": "Model Serving (agents)", "resourceKey": "agents-serving-endpoint", - "description": "Databricks Model Serving endpoint for agents using workspace-hosted models (`DatabricksAdapter.fromModelServing`). Wire the same endpoint name AppKit reads from `DATABRICKS_AGENT_ENDPOINT` when no per-agent model is configured. Omit when agents use only external adapters.", + "description": "Databricks Model Serving endpoint for agents using workspace-hosted models (`DatabricksAdapter.fromModelServing`). Wire the same endpoint name AppKit reads from `DATABRICKS_SERVING_ENDPOINT_NAME` when no per-agent model is configured. The same env var the `serving` plugin reads — one value covers both. Omit when agents use only external adapters.", "permission": "CAN_QUERY", "fields": { "name": { - "env": "DATABRICKS_AGENT_ENDPOINT", - "description": "Endpoint name passed to Model Serving when agents default to `DatabricksAdapter.fromModelServing()`" + "env": "DATABRICKS_SERVING_ENDPOINT_NAME", + "description": "Endpoint name passed to Model Serving when agents default to `DatabricksAdapter.fromModelServing()`. Shared with the `serving` plugin." } } } diff --git a/packages/appkit/src/plugins/agents/tests/agents-plugin.test.ts b/packages/appkit/src/plugins/agents/tests/agents-plugin.test.ts index c42cc97f8..da8dd6bd6 100644 --- a/packages/appkit/src/plugins/agents/tests/agents-plugin.test.ts +++ b/packages/appkit/src/plugins/agents/tests/agents-plugin.test.ts @@ -16,6 +16,7 @@ import { defineTool, type ToolRegistry, } from "../../../core/agent/tools/define-tool"; +import { tool } from "../../../core/agent/tools/tool"; import type { AgentsPluginConfig, ToolkitEntry, @@ -370,4 +371,272 @@ describe("AgentsPlugin", () => { expect(isToolkitEntry({ foo: 1 })).toBe(false); expect(isToolkitEntry(null)).toBe(false); }); + + describe("function-form tools(plugins)", () => { + test("function form spreads tools from plugins..toolkit()", async () => { + const registry: ToolRegistry = { + query: defineTool({ + description: "q", + schema: z.object({ sql: z.string() }), + handler: () => "ok", + }), + }; + const ctx = fakeContext([ + { + name: "analytics", + provider: makeToolProvider("analytics", registry), + }, + ]); + + const plugin = instantiate( + { + dir: false, + agents: { + support: { + instructions: "...", + model: stubAdapter(), + tools(plugins) { + return { ...plugins.analytics.toolkit() }; + }, + }, + }, + }, + ctx, + ); + await plugin.setup(); + + const api = plugin.exports() as { + get: (name: string) => { toolIndex: Map } | null; + }; + const agent = api.get("support"); + expect(agent?.toolIndex.has("analytics.query")).toBe(true); + }); + + test("mixed inline tools + plugin toolkit() coexist in the function form", async () => { + const registry: ToolRegistry = { + query: defineTool({ + description: "q", + schema: z.object({ sql: z.string() }), + handler: () => "ok", + }), + }; + const ctx = fakeContext([ + { + name: "analytics", + provider: makeToolProvider("analytics", registry), + }, + ]); + + const plugin = instantiate( + { + dir: false, + agents: { + support: { + instructions: "...", + model: stubAdapter(), + tools(plugins) { + return { + ...plugins.analytics.toolkit(), + get_weather: tool({ + name: "get_weather", + description: "Weather", + schema: z.object({ city: z.string() }), + execute: async ({ city }) => `Sunny in ${city}`, + }), + }; + }, + }, + }, + }, + ctx, + ); + await plugin.setup(); + + const api = plugin.exports() as { + get: (name: string) => { toolIndex: Map } | null; + }; + const agent = api.get("support"); + expect(agent?.toolIndex.has("analytics.query")).toBe(true); + expect(agent?.toolIndex.has("get_weather")).toBe(true); + }); + + test("function-form callback that throws fails registration with a clear message", async () => { + const ctx = fakeContext([]); + const plugin = instantiate( + { + dir: false, + agents: { + support: { + instructions: "...", + model: stubAdapter(), + tools(plugins) { + // Calling .toolkit() on a missing plugin throws because + // plugins.analytics is undefined under the index signature. + return { ...plugins.analytics.toolkit() }; + }, + }, + }, + }, + ctx, + ); + await expect(plugin.setup()).rejects.toThrow( + /tools\(plugins\) callback threw/, + ); + }); + + test("function form opts out of auto-inherit even when other plugins are autoInheritable", async () => { + const analyticsReg: ToolRegistry = { + query: defineTool({ + description: "q", + schema: z.object({ sql: z.string() }), + handler: () => "ok", + }), + }; + const filesReg: ToolRegistry = { + list: defineTool({ + description: "l", + schema: z.object({}), + autoInheritable: true, + handler: () => [], + }), + }; + const ctx = fakeContext([ + { + name: "analytics", + provider: makeToolProvider("analytics", analyticsReg), + }, + { + name: "files", + provider: makeToolProvider("files", filesReg), + }, + ]); + + const plugin = instantiate( + { + dir: false, + autoInheritTools: { code: true }, + agents: { + support: { + instructions: "...", + model: stubAdapter(), + tools(plugins) { + return { ...plugins.analytics.toolkit() }; + }, + }, + }, + }, + ctx, + ); + await plugin.setup(); + + const api = plugin.exports() as { + get: (name: string) => { toolIndex: Map } | null; + }; + const agent = api.get("support"); + const toolNames = Array.from(agent?.toolIndex.keys() ?? []); + expect(toolNames.some((n) => n.startsWith("analytics."))).toBe(true); + // files is autoInheritable but the function form opted us out + expect(toolNames.some((n) => n.startsWith("files."))).toBe(false); + }); + + test("empty function-form record still opts out of auto-inherit", async () => { + const filesReg: ToolRegistry = { + list: defineTool({ + description: "l", + schema: z.object({}), + autoInheritable: true, + handler: () => [], + }), + }; + const ctx = fakeContext([ + { + name: "files", + provider: makeToolProvider("files", filesReg), + }, + ]); + + const plugin = instantiate( + { + dir: false, + autoInheritTools: { code: true }, + agents: { + support: { + instructions: "...", + model: stubAdapter(), + tools(_plugins) { + return {}; + }, + }, + }, + }, + ctx, + ); + await plugin.setup(); + + const api = plugin.exports() as { + get: (name: string) => { toolIndex: Map } | null; + }; + const agent = api.get("support"); + // Nothing inherited even though files.list is autoInheritable. + expect(agent?.toolIndex.size).toBe(0); + }); + + test("function form falls back to getAgentTools() for providers without toolkit()", async () => { + // Provider lacks .toolkit() — only getAgentTools/executeAgentTool. + const bareProvider: ToolProvider = { + getAgentTools: () => [ + { + name: "ping", + description: "ping", + parameters: { type: "object", properties: {} }, + }, + ], + executeAgentTool: vi.fn(async () => "pong"), + }; + const ctx = fakeContext([{ name: "bare", provider: bareProvider }]); + + const plugin = instantiate( + { + dir: false, + agents: { + support: { + instructions: "...", + model: stubAdapter(), + tools(plugins) { + return { ...plugins.bare.toolkit() }; + }, + }, + }, + }, + ctx, + ); + await plugin.setup(); + + const api = plugin.exports() as { + get: (name: string) => { toolIndex: Map } | null; + }; + const agent = api.get("support"); + expect(agent?.toolIndex.has("bare.ping")).toBe(true); + }); + + test("function form runs exactly once at setup", async () => { + const ctx = fakeContext([]); + const toolsFn = vi.fn(() => ({})); + const plugin = instantiate( + { + dir: false, + agents: { + support: { + instructions: "...", + model: stubAdapter(), + tools: toolsFn, + }, + }, + }, + ctx, + ); + await plugin.setup(); + expect(toolsFn).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/appkit/src/plugins/agents/tests/dos-limits.test.ts b/packages/appkit/src/plugins/agents/tests/dos-limits.test.ts index 25700396f..e2bbcbe99 100644 --- a/packages/appkit/src/plugins/agents/tests/dos-limits.test.ts +++ b/packages/appkit/src/plugins/agents/tests/dos-limits.test.ts @@ -149,10 +149,7 @@ describe("POST /chat — per-user concurrent-stream limit", () => { const plugin = seedPlugin(); for (let i = 0; i < 5; i++) { // biome-ignore lint/suspicious/noExplicitAny: seeding - (plugin as any).activeStreams.set(`s${i}`, { - controller: new AbortController(), - userId: "alice", - }); + (plugin as any).trackStream(`s${i}`, "alice", new AbortController()); } const { res, setHeader, json } = mockRes(); @@ -175,10 +172,7 @@ describe("POST /chat — per-user concurrent-stream limit", () => { const plugin = seedPlugin(); for (let i = 0; i < 5; i++) { // biome-ignore lint/suspicious/noExplicitAny: seeding - (plugin as any).activeStreams.set(`s${i}`, { - controller: new AbortController(), - userId: "alice", - }); + (plugin as any).trackStream(`s${i}`, "alice", new AbortController()); } // Carol's request must not see a 429 even though alice is at-limit. @@ -204,10 +198,7 @@ describe("POST /chat — per-user concurrent-stream limit", () => { }); for (let i = 0; i < 2; i++) { // biome-ignore lint/suspicious/noExplicitAny: seeding - (plugin as any).activeStreams.set(`s${i}`, { - controller: new AbortController(), - userId: "alice", - }); + (plugin as any).trackStream(`s${i}`, "alice", new AbortController()); } const { res } = mockRes(); @@ -220,16 +211,48 @@ describe("POST /chat — per-user concurrent-stream limit", () => { expect(res.status).toHaveBeenCalledWith(429); }); + test("trackStream/untrackStream keep userStreamCounts in sync with activeStreams (O(1) counter)", async () => { + // Regression for the agentic review finding: countUserStreams() used + // to walk every active stream on every chat request (O(n) over total + // concurrent streams). The new path keeps a per-user counter that + // must mirror the underlying map across track/untrack and across + // multiple users. + const plugin = seedPlugin(); + // biome-ignore lint/suspicious/noExplicitAny: private state probe + const t = plugin as any; + + expect(t.countUserStreams("alice")).toBe(0); + + t.trackStream("s1", "alice", new AbortController()); + t.trackStream("s2", "alice", new AbortController()); + t.trackStream("s3", "bob", new AbortController()); + + expect(t.countUserStreams("alice")).toBe(2); + expect(t.countUserStreams("bob")).toBe(1); + expect(t.activeStreams.size).toBe(3); + + t.untrackStream("s1"); + expect(t.countUserStreams("alice")).toBe(1); + expect(t.activeStreams.has("s1")).toBe(false); + + // Untrack the last stream for a user → counter map drops the key + // entirely (avoids unbounded growth across many distinct users). + t.untrackStream("s3"); + expect(t.countUserStreams("bob")).toBe(0); + expect(t.userStreamCounts.has("bob")).toBe(false); + + // Idempotent — untracking a missing stream is a no-op. + t.untrackStream("s1"); + expect(t.countUserStreams("alice")).toBe(1); + }); + test("/invocations also honours maxConcurrentStreamsPerUser (no bypass)", async () => { // Regression for the agentic review finding: /invocations skipped the // rate-limit gate that /chat enforces, letting clients bypass the cap. const plugin = seedPlugin(); for (let i = 0; i < 5; i++) { // biome-ignore lint/suspicious/noExplicitAny: seeding - (plugin as any).activeStreams.set(`s${i}`, { - controller: new AbortController(), - userId: "alice", - }); + (plugin as any).trackStream(`s${i}`, "alice", new AbortController()); } const { res, setHeader, json } = mockRes(); diff --git a/packages/appkit/src/plugins/agents/tests/route-handler-errors.test.ts b/packages/appkit/src/plugins/agents/tests/route-handler-errors.test.ts new file mode 100644 index 000000000..e654f5cad --- /dev/null +++ b/packages/appkit/src/plugins/agents/tests/route-handler-errors.test.ts @@ -0,0 +1,187 @@ +import type express from "express"; +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { CacheManager } from "../../../cache"; +import { AgentsPlugin } from "../agents"; + +/** + * Surface-level guarantees on the agents plugin's HTTP route handlers when + * downstream dependencies fail. Prior to PR #305 review finding #1+#2, + * `_handleChat` and `_handleInvocations` awaited `threadStore` without a + * try/catch — a backing-store failure (DB unreachable, permission error) + * would propagate the rejection without writing a response and the SSE + * client would hang until the upstream proxy timeout. + */ + +beforeEach(() => { + // biome-ignore lint/suspicious/noExplicitAny: test seam, mirrors other suites + (CacheManager as any).instance = { + get: vi.fn(), + set: vi.fn(), + getOrExecute: vi.fn(async (_k: unknown[], fn: () => Promise) => + fn(), + ), + generateKey: vi.fn(() => "test-key"), + }; +}); + +function mockReq(body: unknown, userId = "alice"): express.Request { + const headers: Record = { + "x-forwarded-user": userId, + "x-forwarded-access-token": "fake-token", + }; + return { + body, + headers, + header: (name: string) => headers[name.toLowerCase()], + } as unknown as express.Request; +} + +function mockRes() { + const json = vi.fn(); + const setHeader = vi.fn(); + let statusCode = 200; + const status = vi.fn((code: number) => { + statusCode = code; + return { json }; + }); + return { + res: { status, json, setHeader } as unknown as express.Response, + get statusCode() { + return statusCode; + }, + json, + }; +} + +function seedPlugin(): AgentsPlugin { + const plugin = new AgentsPlugin({ dir: false }); + // biome-ignore lint/suspicious/noExplicitAny: seed private state + (plugin as any).agents.set("default", { + name: "default", + instructions: "hi", + adapter: { async *run() {} }, + toolIndex: new Map(), + }); + // biome-ignore lint/suspicious/noExplicitAny: seed private state + (plugin as any).defaultAgentName = "default"; + return plugin; +} + +describe("POST /chat — threadStore failure", () => { + test("returns 500 when threadStore.get rejects (existing thread path)", async () => { + const plugin = seedPlugin(); + // biome-ignore lint/suspicious/noExplicitAny: stub threadStore for failure + (plugin as any).threadStore = { + get: vi.fn().mockRejectedValue(new Error("DB unreachable")), + create: vi.fn(), + addMessage: vi.fn(), + }; + + const { res, json } = mockRes(); + await ( + plugin as unknown as { + _handleChat: (r: express.Request, w: express.Response) => Promise; + } + )._handleChat(mockReq({ message: "hi", threadId: "t-1" }), res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: "Thread operation failed" }); + }); + + test("returns 500 when threadStore.create rejects (new-thread path)", async () => { + const plugin = seedPlugin(); + // biome-ignore lint/suspicious/noExplicitAny: stub + (plugin as any).threadStore = { + get: vi.fn().mockResolvedValue(null), + create: vi.fn().mockRejectedValue(new Error("Disk full")), + addMessage: vi.fn(), + }; + + const { res, json } = mockRes(); + await ( + plugin as unknown as { + _handleChat: (r: express.Request, w: express.Response) => Promise; + } + )._handleChat(mockReq({ message: "hi" }), res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: "Thread operation failed" }); + }); + + test("returns 500 when threadStore.addMessage rejects", async () => { + const plugin = seedPlugin(); + // biome-ignore lint/suspicious/noExplicitAny: stub + (plugin as any).threadStore = { + get: vi.fn().mockResolvedValue(null), + create: vi.fn().mockResolvedValue({ id: "t-new", messages: [] }), + addMessage: vi.fn().mockRejectedValue(new Error("Quota exceeded")), + }; + + const { res, json } = mockRes(); + await ( + plugin as unknown as { + _handleChat: (r: express.Request, w: express.Response) => Promise; + } + )._handleChat(mockReq({ message: "hi" }), res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: "Thread operation failed" }); + }); +}); + +describe("POST /invocations — threadStore failure", () => { + test("returns 500 when threadStore.create rejects", async () => { + const plugin = seedPlugin(); + // biome-ignore lint/suspicious/noExplicitAny: stub + (plugin as any).threadStore = { + create: vi.fn().mockRejectedValue(new Error("DB unreachable")), + addMessage: vi.fn(), + }; + + const { res, json } = mockRes(); + await ( + plugin as unknown as { + _handleInvocations: ( + r: express.Request, + w: express.Response, + ) => Promise; + } + )._handleInvocations(mockReq({ input: "hi" }), res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: "Thread operation failed" }); + }); + + test("returns 500 when threadStore.addMessage rejects mid-loop", async () => { + const plugin = seedPlugin(); + // biome-ignore lint/suspicious/noExplicitAny: stub + (plugin as any).threadStore = { + create: vi.fn().mockResolvedValue({ id: "t-new", messages: [] }), + addMessage: vi + .fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error("Conflict")), + }; + + const { res, json } = mockRes(); + await ( + plugin as unknown as { + _handleInvocations: ( + r: express.Request, + w: express.Response, + ) => Promise; + } + )._handleInvocations( + mockReq({ + input: [ + { role: "user", content: "first" }, + { role: "user", content: "second" }, + ], + }), + res, + ); + + expect(res.status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: "Thread operation failed" }); + }); +}); diff --git a/packages/appkit/src/plugins/analytics/analytics.ts b/packages/appkit/src/plugins/analytics/analytics.ts index a16d6303a..fdcb16b43 100644 --- a/packages/appkit/src/plugins/analytics/analytics.ts +++ b/packages/appkit/src/plugins/analytics/analytics.ts @@ -314,8 +314,8 @@ export class AnalyticsPlugin extends Plugin implements ToolProvider { * Returns the plugin's tools as a keyed record of `ToolkitEntry` markers. * Called by the agents plugin (via `resolveToolkitFromProvider`) to spread * a filtered, renamed view of the plugin's tools into an agent's tool - * index. Most callers should go through `fromPlugin(analytics, opts)` at - * module scope instead of reaching for this directly. + * index. Inside the function form of `AgentDefinition.tools`, callers + * reach this method via `plugins.analytics.toolkit(opts)`. */ toolkit(opts?: import("../../core/agent/types").ToolkitOptions) { return buildToolkitEntries(this.name, this.tools, opts);