From 14ca97c5dbba8345f357a2fe7204860bcca77bfc Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 21 Apr 2026 19:48:00 +0200 Subject: [PATCH] =?UTF-8?q?feat(appkit):=20plugin=20infrastructure=20?= =?UTF-8?q?=E2=80=94=20attachContext=20lifecycle=20+=20PluginContext=20med?= =?UTF-8?q?iator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third layer: the substrate every downstream PR relies on. No user- facing API changes here; the surface for this PR is the mediator pattern, lifecycle semantics, and factory stamping. ### Split Plugin construction from context binding `Plugin` constructors become pure — no `CacheManager.getInstanceSync()`, no `TelemetryManager.getProvider()`, no `PluginContext` wiring inside `constructor()`. That work moves to a new lifecycle method: ```ts interface BasePlugin { attachContext?(deps: { context?: unknown; telemetryConfig?: TelemetryOptions; }): void; } ``` `createApp` calls `attachContext()` on every plugin after all constructors have run, before `setup()`. This lets factories return `PluginData` tuples at module scope without pulling core services into the import graph — a prerequisite for later PRs that construct agent definitions before `createApp`. ### PluginContext mediator `packages/appkit/src/core/plugin-context.ts` — new class that mediates all inter-plugin communication: - **Route buffering**: `addRoute()` / `addMiddleware()` buffer until the server plugin calls `registerAsRouteTarget()`, then flush via `addExtension()`. Eliminates plugin-ordering fragility. - **ToolProvider registry**: `registerToolProvider(name, plugin)` + live `getToolProviders()`. Typed discovery of tool-exposing plugins. - **User-scoped tool execution**: `executeTool(req, pluginName, localName, args, signal?)` resolves the provider, wraps in `asUser(req)` for OBO, opens a telemetry span, applies a 30s timeout, dispatches, returns. - **Lifecycle hooks**: `onLifecycle('setup:complete' | 'server:ready' | 'shutdown', cb)` + `emitLifecycle(event)`. Callback errors don't block siblings. ### `toPlugin` stamps `pluginName` `packages/appkit/src/plugin/to-plugin.ts` — the factory now attaches a read-only `pluginName` property to the returned function. Later PRs' `fromPlugin(factory)` reads it to identify which plugin a factory refers to without needing to construct an instance. `NamedPluginFactory` type exported for consumers who want to type-constrain factories. ### Server plugin defers start to `setup:complete` `ServerPlugin.setup()` no longer calls `extendRoutes()` synchronously. It subscribes to the `setup:complete` lifecycle event via `PluginContext` and starts the HTTP server there. This ensures that any deferred-phase plugin (agents plugin in a later PR) has had a chance to register routes via `PluginContext.addRoute()` before the server binds. Removes the `plugins` field from `ServerConfig` (routes are now discovered via the context, not a config snapshot). ### Test plan - 25 new PluginContext tests (route buffering, tool provider registry, executeTool paths, lifecycle hooks, plugin metadata) - Updated AppKit lifecycle tests to inject `context` instead of `plugins` - Full appkit vitest suite: 1237 tests passing - Typecheck clean across all 8 workspace projects Signed-off-by: MarioCadenas --- packages/appkit/src/core/appkit.ts | 27 +- packages/appkit/src/core/plugin-context.ts | 287 ++++++++++++++++ .../appkit/src/core/tests/databricks.test.ts | 15 +- .../src/core/tests/plugin-context.test.ts | 325 ++++++++++++++++++ packages/appkit/src/plugin/index.ts | 2 +- packages/appkit/src/plugin/plugin.ts | 56 ++- packages/appkit/src/plugin/to-plugin.ts | 32 +- packages/appkit/src/plugins/server/index.ts | 41 ++- .../src/plugins/server/tests/server.test.ts | 42 ++- packages/appkit/src/plugins/server/types.ts | 2 - packages/shared/src/plugin.ts | 9 + 11 files changed, 799 insertions(+), 39 deletions(-) create mode 100644 packages/appkit/src/core/plugin-context.ts create mode 100644 packages/appkit/src/core/tests/plugin-context.test.ts diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index a2cba994..a0c2e566 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -13,14 +13,18 @@ import { ServiceContext } from "../context"; import { ResourceRegistry, ResourceType } from "../registry"; import type { TelemetryConfig } from "../telemetry"; import { TelemetryManager } from "../telemetry"; +import { isToolProvider, PluginContext } from "./plugin-context"; export class AppKit { #pluginInstances: Record = {}; #setupPromises: Promise[] = []; + #context: PluginContext; private constructor(config: { plugins: TPlugins }) { const { plugins, ...globalConfig } = config; + this.#context = new PluginContext(); + const pluginEntries = Object.entries(plugins); const corePlugins = pluginEntries.filter(([_, p]) => { @@ -35,20 +39,24 @@ export class AppKit { for (const [name, pluginData] of corePlugins) { if (pluginData) { - this.createAndRegisterPlugin(globalConfig, name, pluginData); + this.createAndRegisterPlugin(globalConfig, name, pluginData, { + context: this.#context, + }); } } for (const [name, pluginData] of normalPlugins) { if (pluginData) { - this.createAndRegisterPlugin(globalConfig, name, pluginData); + this.createAndRegisterPlugin(globalConfig, name, pluginData, { + context: this.#context, + }); } } for (const [name, pluginData] of deferredPlugins) { if (pluginData) { this.createAndRegisterPlugin(globalConfig, name, pluginData, { - plugins: this.#pluginInstances, + context: this.#context, }); } } @@ -70,8 +78,20 @@ export class AppKit { }; const pluginInstance = new Plugin(baseConfig); + if (typeof pluginInstance.attachContext === "function") { + pluginInstance.attachContext({ + context: this.#context, + telemetryConfig: baseConfig.telemetry, + }); + } + this.#pluginInstances[name] = pluginInstance; + this.#context.registerPlugin(name, pluginInstance); + if (isToolProvider(pluginInstance)) { + this.#context.registerToolProvider(name, pluginInstance); + } + this.#setupPromises.push(pluginInstance.setup()); const self = this; @@ -199,6 +219,7 @@ export class AppKit { const instance = new AppKit(mergedConfig); await Promise.all(instance.#setupPromises); + await instance.#context.emitLifecycle("setup:complete"); return instance as unknown as PluginMap; } diff --git a/packages/appkit/src/core/plugin-context.ts b/packages/appkit/src/core/plugin-context.ts new file mode 100644 index 00000000..c2801585 --- /dev/null +++ b/packages/appkit/src/core/plugin-context.ts @@ -0,0 +1,287 @@ +import type express from "express"; +import type { BasePlugin, ToolProvider } from "shared"; +import { createLogger } from "../logging/logger"; +import { TelemetryManager } from "../telemetry"; + +const logger = createLogger("plugin-context"); + +interface BufferedRoute { + method: string; + path: string; + handlers: express.RequestHandler[]; +} + +interface RouteTarget { + addExtension(fn: (app: express.Application) => void): void; +} + +interface ToolProviderEntry { + plugin: BasePlugin & ToolProvider; + name: string; +} + +type LifecycleEvent = "setup:complete" | "server:ready" | "shutdown"; + +/** + * Mediator for inter-plugin communication. + * + * Created by AppKit core and passed to every plugin. Plugins request + * capabilities from the context instead of holding direct references + * to sibling plugin instances. + * + * Capabilities: + * - Route mounting with buffering (order-independent) + * - Typed ToolProvider registry (live, not snapshot-based) + * - User-scoped tool execution with automatic telemetry + * - Lifecycle hooks for plugin coordination + */ +export class PluginContext { + private routeBuffer: BufferedRoute[] = []; + private routeTarget: RouteTarget | null = null; + private toolProviders = new Map(); + private plugins = new Map(); + private lifecycleHooks = new Map< + LifecycleEvent, + Set<() => void | Promise> + >(); + private telemetry = TelemetryManager.getProvider("plugin-context"); + + /** + * Register a route on the root Express application. + * + * If a route target (server plugin) has registered, the route is applied + * immediately. Otherwise it is buffered and flushed when a route target + * becomes available. + */ + addRoute( + method: string, + path: string, + ...handlers: express.RequestHandler[] + ): void { + if (this.routeTarget) { + this.applyRoute({ method, path, handlers }); + } else { + this.routeBuffer.push({ method, path, handlers }); + } + } + + /** + * Register middleware on the root Express application. + * + * Same buffering semantics as `addRoute`. + */ + addMiddleware(path: string, ...handlers: express.RequestHandler[]): void { + if (this.routeTarget) { + this.applyMiddleware(path, handlers); + } else { + this.routeBuffer.push({ method: "use", path, handlers }); + } + } + + /** + * Called by the server plugin to opt in as the route target. + * Flushes all buffered routes via the server's `addExtension`. + */ + registerAsRouteTarget(target: RouteTarget): void { + this.routeTarget = target; + + for (const route of this.routeBuffer) { + if (route.method === "use") { + this.applyMiddleware(route.path, route.handlers); + } else { + this.applyRoute(route); + } + } + this.routeBuffer = []; + } + + /** + * Register a plugin that implements the ToolProvider interface. + * Called by AppKit core after constructing each plugin. + */ + registerToolProvider(name: string, plugin: BasePlugin & ToolProvider): void { + this.toolProviders.set(name, { plugin, name }); + } + + /** + * Register a plugin instance. + * Called by AppKit core after constructing each plugin. + */ + registerPlugin(name: string, instance: BasePlugin): void { + this.plugins.set(name, instance); + } + + /** + * Returns all registered plugin instances keyed by name. + * Used by the server plugin for route injection, client config, + * and shutdown coordination. + */ + getPlugins(): Map { + return this.plugins; + } + + /** + * Returns all registered ToolProvider plugins. + * Always returns the current set — not a frozen snapshot. + */ + getToolProviders(): Array<{ name: string; provider: ToolProvider }> { + return Array.from(this.toolProviders.values()).map((entry) => ({ + name: entry.name, + provider: entry.plugin, + })); + } + + /** + * Execute a tool on a ToolProvider plugin with automatic user scoping + * and telemetry. + * + * The context: + * 1. Resolves the plugin by name + * 2. Calls `asUser(req)` for user-scoped execution + * 3. Wraps the call in a telemetry span with a 30s timeout + */ + async executeTool( + req: express.Request, + pluginName: string, + toolName: string, + args: unknown, + signal?: AbortSignal, + ): Promise { + const entry = this.toolProviders.get(pluginName); + if (!entry) { + throw new Error( + `PluginContext: unknown plugin "${pluginName}". Available: ${Array.from(this.toolProviders.keys()).join(", ")}`, + ); + } + + const tracer = this.telemetry.getTracer(); + const operationName = `executeTool:${pluginName}.${toolName}`; + + return tracer.startActiveSpan(operationName, async (span) => { + const timeout = 30_000; + const timeoutSignal = AbortSignal.timeout(timeout); + const combinedSignal = signal + ? AbortSignal.any([signal, timeoutSignal]) + : timeoutSignal; + + try { + const userPlugin = (entry.plugin as any).asUser(req); + const result = await (userPlugin as ToolProvider).executeAgentTool( + toolName, + args, + combinedSignal, + ); + span.setStatus({ code: 0 }); + return result; + } catch (error) { + span.setStatus({ + code: 2, + message: + error instanceof Error ? error.message : "Tool execution failed", + }); + span.recordException( + error instanceof Error ? error : new Error(String(error)), + ); + throw error; + } finally { + span.end(); + } + }); + } + + /** + * Register a lifecycle hook callback. + */ + onLifecycle(event: LifecycleEvent, fn: () => void | Promise): void { + let hooks = this.lifecycleHooks.get(event); + if (!hooks) { + hooks = new Set(); + this.lifecycleHooks.set(event, hooks); + } + hooks.add(fn); + } + + /** + * Emit a lifecycle event, calling all registered callbacks. + * Errors in individual callbacks are logged but do not prevent + * other callbacks from running. + * + * @internal Called by AppKit core only. + */ + async emitLifecycle(event: LifecycleEvent): Promise { + const hooks = this.lifecycleHooks.get(event); + if (!hooks) return; + + if ( + event === "setup:complete" && + this.routeBuffer.length > 0 && + !this.routeTarget + ) { + logger.warn( + "%d buffered routes were never applied — no server plugin registered as route target", + this.routeBuffer.length, + ); + } + + for (const fn of hooks) { + try { + await fn(); + } catch (error) { + logger.error("Lifecycle hook '%s' failed: %O", event, error); + } + } + } + + /** + * Returns all registered plugin names. + */ + getPluginNames(): string[] { + return Array.from(this.plugins.keys()); + } + + /** + * Check if a plugin with the given name is registered. + */ + hasPlugin(name: string): boolean { + return this.plugins.has(name); + } + + private applyRoute(route: BufferedRoute): void { + if (!this.routeTarget) return; + this.routeTarget.addExtension((app) => { + const method = route.method.toLowerCase() as keyof express.Application; + if (typeof app[method] === "function") { + (app[method] as (...a: unknown[]) => void)( + route.path, + ...route.handlers, + ); + } + }); + } + + private applyMiddleware( + path: string, + handlers: express.RequestHandler[], + ): void { + if (!this.routeTarget) return; + this.routeTarget.addExtension((app) => { + app.use(path, ...handlers); + }); + } +} + +/** + * Type guard: checks whether a plugin implements the ToolProvider interface. + */ +export function isToolProvider( + plugin: unknown, +): plugin is BasePlugin & ToolProvider { + return ( + typeof plugin === "object" && + plugin !== null && + "getAgentTools" in plugin && + typeof (plugin as ToolProvider).getAgentTools === "function" && + "executeAgentTool" in plugin && + typeof (plugin as ToolProvider).executeAgentTool === "function" + ); +} diff --git a/packages/appkit/src/core/tests/databricks.test.ts b/packages/appkit/src/core/tests/databricks.test.ts index c05345a6..9d3fe5f8 100644 --- a/packages/appkit/src/core/tests/databricks.test.ts +++ b/packages/appkit/src/core/tests/databricks.test.ts @@ -109,11 +109,11 @@ class DeferredTestPlugin implements BasePlugin { name = "deferredTest"; setupCalled = false; injectedConfig: any; - injectedPlugins: any; + injectedContext: any; constructor(config: any) { this.injectedConfig = config; - this.injectedPlugins = config.plugins; + this.injectedContext = config.context; } async setup() { @@ -130,7 +130,7 @@ class DeferredTestPlugin implements BasePlugin { return { setupCalled: this.setupCalled, injectedConfig: this.injectedConfig, - injectedPlugins: this.injectedPlugins, + injectedContext: this.injectedContext, }; } } @@ -276,7 +276,7 @@ describe("AppKit", () => { expect(setupOrder).toEqual(["core", "normal", "deferred"]); }); - test("should provide plugin instances to deferred plugins", async () => { + test("should provide PluginContext to deferred plugins", async () => { const pluginData = [ { plugin: CoreTestPlugin, config: {}, name: "coreTest" }, { plugin: DeferredTestPlugin, config: {}, name: "deferredTest" }, @@ -284,10 +284,9 @@ describe("AppKit", () => { const instance = (await createApp({ plugins: pluginData })) as any; - // Deferred plugins receive plugin instances (not SDKs) for internal use - expect(instance.deferredTest.injectedPlugins).toBeDefined(); - expect(instance.deferredTest.injectedPlugins.coreTest).toBeInstanceOf( - CoreTestPlugin, + expect(instance.deferredTest.injectedContext).toBeDefined(); + expect(instance.deferredTest.injectedContext.hasPlugin("coreTest")).toBe( + true, ); }); diff --git a/packages/appkit/src/core/tests/plugin-context.test.ts b/packages/appkit/src/core/tests/plugin-context.test.ts new file mode 100644 index 00000000..276c5502 --- /dev/null +++ b/packages/appkit/src/core/tests/plugin-context.test.ts @@ -0,0 +1,325 @@ +import type { AgentToolDefinition } from "shared"; +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { isToolProvider, PluginContext } from "../plugin-context"; + +vi.mock("../../telemetry", () => ({ + TelemetryManager: { + getProvider: () => ({ + getTracer: () => ({ + startActiveSpan: (_name: string, fn: (span: any) => any) => { + const span = { + setStatus: vi.fn(), + recordException: vi.fn(), + end: vi.fn(), + }; + return fn(span); + }, + }), + }), + }, +})); + +vi.mock("../../logging/logger", () => ({ + createLogger: () => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }), +})); + +function createMockToolProvider(tools: AgentToolDefinition[] = []) { + const mock = { + name: "mock-plugin", + setup: vi.fn().mockResolvedValue(undefined), + injectRoutes: vi.fn(), + getEndpoints: vi.fn().mockReturnValue({}), + getAgentTools: vi.fn().mockReturnValue(tools), + executeAgentTool: vi.fn().mockResolvedValue("tool-result"), + asUser: vi.fn().mockReturnThis(), + }; + return mock as any; +} + +describe("PluginContext", () => { + let ctx: PluginContext; + + beforeEach(() => { + ctx = new PluginContext(); + }); + + describe("route buffering", () => { + test("addRoute buffers when no route target exists", () => { + const handler = vi.fn(); + ctx.addRoute("post", "/invocations", handler); + + expect(ctx.getPluginNames()).toEqual([]); + }); + + test("flushRoutes applies buffered routes via addExtension", () => { + const handler = vi.fn(); + ctx.addRoute("post", "/invocations", handler); + + const addExtension = vi.fn(); + ctx.registerAsRouteTarget({ addExtension }); + + expect(addExtension).toHaveBeenCalledTimes(1); + const extensionFn = addExtension.mock.calls[0][0]; + + const mockApp = { post: vi.fn() }; + extensionFn(mockApp); + expect(mockApp.post).toHaveBeenCalledWith("/invocations", handler); + }); + + test("addRoute called after registerAsRouteTarget applies immediately", () => { + const addExtension = vi.fn(); + ctx.registerAsRouteTarget({ addExtension }); + + const handler = vi.fn(); + ctx.addRoute("get", "/health", handler); + + expect(addExtension).toHaveBeenCalledTimes(1); + const extensionFn = addExtension.mock.calls[0][0]; + + const mockApp = { get: vi.fn() }; + extensionFn(mockApp); + expect(mockApp.get).toHaveBeenCalledWith("/health", handler); + }); + + test("addRoute supports middleware chains", () => { + const auth = vi.fn(); + const handler = vi.fn(); + + const addExtension = vi.fn(); + ctx.registerAsRouteTarget({ addExtension }); + + ctx.addRoute("post", "/api", auth, handler); + + const extensionFn = addExtension.mock.calls[0][0]; + const mockApp = { post: vi.fn() }; + extensionFn(mockApp); + expect(mockApp.post).toHaveBeenCalledWith("/api", auth, handler); + }); + + test("addMiddleware buffers and applies via use()", () => { + const handler = vi.fn(); + ctx.addMiddleware("/api", handler); + + const addExtension = vi.fn(); + ctx.registerAsRouteTarget({ addExtension }); + + expect(addExtension).toHaveBeenCalledTimes(1); + const extensionFn = addExtension.mock.calls[0][0]; + + const mockApp = { use: vi.fn() }; + extensionFn(mockApp); + expect(mockApp.use).toHaveBeenCalledWith("/api", handler); + }); + + test("multiple buffered routes are all applied on registration", () => { + const h1 = vi.fn(); + const h2 = vi.fn(); + ctx.addRoute("post", "/a", h1); + ctx.addRoute("get", "/b", h2); + + const addExtension = vi.fn(); + ctx.registerAsRouteTarget({ addExtension }); + + expect(addExtension).toHaveBeenCalledTimes(2); + }); + }); + + describe("ToolProvider registry", () => { + test("registerToolProvider makes provider visible via getToolProviders", () => { + const provider = createMockToolProvider([ + { + name: "query", + description: "Run query", + parameters: { type: "object" }, + }, + ]); + + ctx.registerToolProvider("analytics", provider); + + const providers = ctx.getToolProviders(); + expect(providers).toHaveLength(1); + expect(providers[0].name).toBe("analytics"); + expect(providers[0].provider.getAgentTools()).toHaveLength(1); + }); + + test("getToolProviders returns all registered providers", () => { + ctx.registerToolProvider("analytics", createMockToolProvider()); + ctx.registerToolProvider("files", createMockToolProvider()); + ctx.registerToolProvider("genie", createMockToolProvider()); + + expect(ctx.getToolProviders()).toHaveLength(3); + }); + + test("getToolProviders returns current set, not snapshot", () => { + const before = ctx.getToolProviders(); + expect(before).toHaveLength(0); + + ctx.registerToolProvider("analytics", createMockToolProvider()); + + const after = ctx.getToolProviders(); + expect(after).toHaveLength(1); + }); + }); + + describe("executeTool", () => { + test("calls asUser(req).executeAgentTool on the correct plugin", async () => { + const provider = createMockToolProvider(); + ctx.registerToolProvider("analytics", provider); + + const mockReq = { headers: {} } as any; + await ctx.executeTool(mockReq, "analytics", "query", { sql: "SELECT 1" }); + + expect(provider.asUser).toHaveBeenCalledWith(mockReq); + expect(provider.executeAgentTool).toHaveBeenCalledWith( + "query", + { sql: "SELECT 1" }, + expect.any(Object), + ); + }); + + test("throws for unknown plugin name", async () => { + const mockReq = { headers: {} } as any; + + await expect( + ctx.executeTool(mockReq, "nonexistent", "query", {}), + ).rejects.toThrow('unknown plugin "nonexistent"'); + }); + + test("propagates tool execution errors", async () => { + const provider = createMockToolProvider(); + (provider.executeAgentTool as any).mockRejectedValue( + new Error("Query failed"), + ); + ctx.registerToolProvider("analytics", provider); + + const mockReq = { headers: {} } as any; + + await expect( + ctx.executeTool(mockReq, "analytics", "query", {}), + ).rejects.toThrow("Query failed"); + }); + + test("passes abort signal to executeAgentTool", async () => { + const provider = createMockToolProvider(); + ctx.registerToolProvider("analytics", provider); + + const controller = new AbortController(); + const mockReq = { headers: {} } as any; + + await ctx.executeTool( + mockReq, + "analytics", + "query", + {}, + controller.signal, + ); + + const callArgs = (provider.executeAgentTool as any).mock.calls[0]; + expect(callArgs[2]).toBeDefined(); + }); + }); + + describe("lifecycle hooks", () => { + test("onLifecycle registers callback, emitLifecycle invokes it", async () => { + const fn = vi.fn(); + ctx.onLifecycle("setup:complete", fn); + + await ctx.emitLifecycle("setup:complete"); + + expect(fn).toHaveBeenCalledTimes(1); + }); + + test("multiple callbacks for the same event all fire", async () => { + const fn1 = vi.fn(); + const fn2 = vi.fn(); + ctx.onLifecycle("setup:complete", fn1); + ctx.onLifecycle("setup:complete", fn2); + + await ctx.emitLifecycle("setup:complete"); + + expect(fn1).toHaveBeenCalledTimes(1); + expect(fn2).toHaveBeenCalledTimes(1); + }); + + test("callback error does not prevent other callbacks from running", async () => { + const fn1 = vi.fn().mockRejectedValue(new Error("fail")); + const fn2 = vi.fn(); + ctx.onLifecycle("shutdown", fn1); + ctx.onLifecycle("shutdown", fn2); + + await ctx.emitLifecycle("shutdown"); + + expect(fn1).toHaveBeenCalled(); + expect(fn2).toHaveBeenCalled(); + }); + + test("emitLifecycle with no registered hooks does nothing", async () => { + await expect(ctx.emitLifecycle("server:ready")).resolves.toBeUndefined(); + }); + }); + + describe("plugin metadata", () => { + const stubPlugin = { name: "stub" } as any; + + test("getPluginNames returns all registered names", () => { + ctx.registerPlugin("analytics", stubPlugin); + ctx.registerPlugin("server", stubPlugin); + ctx.registerPlugin("agent", stubPlugin); + + const names = ctx.getPluginNames(); + expect(names).toContain("analytics"); + expect(names).toContain("server"); + expect(names).toContain("agent"); + expect(names).toHaveLength(3); + }); + + test("hasPlugin returns true for registered plugins", () => { + ctx.registerPlugin("analytics", stubPlugin); + + expect(ctx.hasPlugin("analytics")).toBe(true); + expect(ctx.hasPlugin("nonexistent")).toBe(false); + }); + + test("getPlugins returns all registered instances", () => { + const p1 = { name: "analytics" } as any; + const p2 = { name: "server" } as any; + ctx.registerPlugin("analytics", p1); + ctx.registerPlugin("server", p2); + + const plugins = ctx.getPlugins(); + expect(plugins.size).toBe(2); + expect(plugins.get("analytics")).toBe(p1); + expect(plugins.get("server")).toBe(p2); + }); + }); +}); + +describe("isToolProvider", () => { + test("returns true for objects with getAgentTools and executeAgentTool", () => { + const provider = createMockToolProvider(); + expect(isToolProvider(provider)).toBe(true); + }); + + test("returns false for null", () => { + expect(isToolProvider(null)).toBe(false); + }); + + test("returns false for objects missing executeAgentTool", () => { + expect(isToolProvider({ getAgentTools: vi.fn() })).toBe(false); + }); + + test("returns false for objects missing getAgentTools", () => { + expect(isToolProvider({ executeAgentTool: vi.fn() })).toBe(false); + }); + + test("returns false for non-objects", () => { + expect(isToolProvider("string")).toBe(false); + expect(isToolProvider(42)).toBe(false); + expect(isToolProvider(undefined)).toBe(false); + }); +}); diff --git a/packages/appkit/src/plugin/index.ts b/packages/appkit/src/plugin/index.ts index 93765219..46a4eb94 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 { toPlugin } from "./to-plugin"; +export { type NamedPluginFactory, toPlugin } from "./to-plugin"; diff --git a/packages/appkit/src/plugin/plugin.ts b/packages/appkit/src/plugin/plugin.ts index 5173cb61..4c9a0e64 100644 --- a/packages/appkit/src/plugin/plugin.ts +++ b/packages/appkit/src/plugin/plugin.ts @@ -19,6 +19,7 @@ import { ServiceContext, type UserContext, } from "../context"; +import type { PluginContext } from "../core/plugin-context"; import { AppKitError, AuthenticationError } from "../errors"; import { createLogger } from "../logging/logger"; import { StreamManager } from "../stream"; @@ -163,11 +164,12 @@ export abstract class Plugin< > implements BasePlugin { protected isReady = false; - protected cache: CacheManager; + protected cache!: CacheManager; protected app: AppManager; protected devFileReader: DevFileReader; protected streamManager: StreamManager; - protected telemetry: ITelemetry; + protected telemetry!: ITelemetry; + protected context?: PluginContext; /** Registered endpoints for this plugin */ private registeredEndpoints: PluginEndpointMap = {}; @@ -193,12 +195,58 @@ export abstract class Plugin< config.name ?? (this.constructor as { manifest?: { name: string } }).manifest?.name ?? "plugin"; - this.telemetry = TelemetryManager.getProvider(this.name, config.telemetry); this.streamManager = new StreamManager(); - this.cache = CacheManager.getInstanceSync(); this.app = new AppManager(); this.devFileReader = DevFileReader.getInstance(); + this.context = (config as Record).context as + | PluginContext + | undefined; + + // Eagerly bind telemetry + cache if the core services have already been + // initialized (normal createApp path, or tests that mock CacheManager). + // If they haven't, we leave these undefined and rely on `attachContext` + // being called later — this lets factories eagerly construct plugin + // instances at module top-level before `createApp` has run. + this.tryAttachContext(); + } + + private tryAttachContext(): void { + try { + this.cache = CacheManager.getInstanceSync(); + } catch { + return; + } + this.telemetry = TelemetryManager.getProvider( + this.name, + this.config.telemetry, + ); + this.isReady = true; + } + /** + * Binds runtime dependencies (telemetry provider, cache, plugin context) to + * this plugin. Called by `AppKit._createApp` after construction and before + * `setup()`. Idempotent: safe to call if the constructor already bound them + * eagerly. Kept separate so factories can eagerly construct plugin instances + * without running this before `TelemetryManager.initialize()` / + * `CacheManager.getInstance()` have run. + */ + attachContext( + deps: { + context?: unknown; + telemetryConfig?: BasePluginConfig["telemetry"]; + } = {}, + ): void { + if (!this.cache) { + this.cache = CacheManager.getInstanceSync(); + } + this.telemetry = TelemetryManager.getProvider( + this.name, + deps.telemetryConfig ?? this.config.telemetry, + ); + if (deps.context !== undefined) { + this.context = deps.context as PluginContext; + } this.isReady = true; } diff --git a/packages/appkit/src/plugin/to-plugin.ts b/packages/appkit/src/plugin/to-plugin.ts index 77725027..c882f300 100644 --- a/packages/appkit/src/plugin/to-plugin.ts +++ b/packages/appkit/src/plugin/to-plugin.ts @@ -1,19 +1,41 @@ import type { PluginConstructor, PluginData, ToPlugin } from "shared"; /** - * Wraps a plugin class so it can be passed to createApp with optional config. - * Infers config type from the constructor and plugin name from the static `name` property. + * 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. * * @internal */ export function toPlugin( plugin: T, -): ToPlugin[0], T["manifest"]["name"]> { +): ToPlugin[0], T["manifest"]["name"]> & + NamedPluginFactory { type Config = ConstructorParameters[0]; type Name = T["manifest"]["name"]; - return (config: Config = {} as Config): PluginData => ({ + const pluginName = plugin.manifest.name as Name; + const factory = ( + config: Config = {} as Config, + ): PluginData => ({ plugin: plugin as T, config: config as Config, - name: plugin.manifest.name as Name, + name: pluginName, + }); + Object.defineProperty(factory, "pluginName", { + value: pluginName, + writable: false, + enumerable: true, }); + return factory as ToPlugin & NamedPluginFactory; } diff --git a/packages/appkit/src/plugins/server/index.ts b/packages/appkit/src/plugins/server/index.ts index e7b9b31a..4c911b10 100644 --- a/packages/appkit/src/plugins/server/index.ts +++ b/packages/appkit/src/plugins/server/index.ts @@ -59,17 +59,34 @@ export class ServerPlugin extends Plugin { this.serverApplication = express(); this.server = null; this.serverExtensions = []; + } + + attachContext(deps: Parameters[0] = {}): void { + super.attachContext(deps); this.telemetry.registerInstrumentations([ instrumentations.http, instrumentations.express, ]); + this.context?.registerAsRouteTarget(this); } /** Setup the server plugin. */ async setup() { - if (this.shouldAutoStart()) { - await this.start(); + if (!this.shouldAutoStart()) return; + if (this.context) { + // Defer the actual listen+extendRoutes to the `setup:complete` lifecycle + // hook. That way every plugin (including other deferred-phase plugins + // like `agents`) is already registered in PluginContext by the time + // extendRoutes() iterates. Otherwise plugins declared after server() + // in the plugin array would be silently dropped from /api/* mounts. + this.context.onLifecycle("setup:complete", async () => { + await this.start(); + }); + return; } + // No plugin context (e.g. tests constructing ServerPlugin directly) — + // start immediately. + await this.start(); } /** Get the server configuration. */ @@ -179,6 +196,16 @@ export class ServerPlugin extends Plugin { return this; } + /** + * Register a server extension from another plugin during setup. + * Unlike extend(), this does not guard on autoStart — it's designed + * for internal plugin-to-plugin coordination where extensions are + * registered before the server starts listening. + */ + addExtension(fn: (app: express.Application) => void) { + this.serverExtensions.push(fn); + } + /** * Setup the routes with the plugins. * @@ -193,14 +220,15 @@ export class ServerPlugin extends Plugin { const endpoints: PluginEndpoints = {}; const pluginConfigs: PluginClientConfigs = {}; - if (!this.config.plugins) return { endpoints, pluginConfigs }; + const plugins = this.context?.getPlugins(); + if (!plugins || plugins.size === 0) return { endpoints, pluginConfigs }; this.serverApplication.get("/health", (_, res) => { res.status(200).json({ status: "ok" }); }); this.registerEndpoint("health", "/health"); - for (const plugin of Object.values(this.config.plugins)) { + for (const plugin of plugins.values()) { if (EXCLUDED_PLUGINS.includes(plugin.name)) continue; if (plugin?.injectRoutes && typeof plugin.injectRoutes === "function") { @@ -349,8 +377,9 @@ export class ServerPlugin extends Plugin { } // 1. abort active operations from plugins - if (this.config.plugins) { - for (const plugin of Object.values(this.config.plugins)) { + const shutdownPlugins = this.context?.getPlugins(); + if (shutdownPlugins) { + for (const plugin of shutdownPlugins.values()) { if (plugin.abortActiveOperations) { try { plugin.abortActiveOperations(); diff --git a/packages/appkit/src/plugins/server/tests/server.test.ts b/packages/appkit/src/plugins/server/tests/server.test.ts index 22f18129..52d15845 100644 --- a/packages/appkit/src/plugins/server/tests/server.test.ts +++ b/packages/appkit/src/plugins/server/tests/server.test.ts @@ -1,4 +1,6 @@ +import type { BasePlugin } from "shared"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { PluginContext } from "../../../core/plugin-context"; // Use vi.hoisted for mocks that need to be available before module loading const { @@ -171,6 +173,14 @@ import { RemoteTunnelController } from "../remote-tunnel/remote-tunnel-controlle import { StaticServer } from "../static-server"; import { ViteDevServer } from "../vite-dev-server"; +function createContextWithPlugins(plugins: Record): PluginContext { + const ctx = new PluginContext(); + for (const [name, instance] of Object.entries(plugins)) { + ctx.registerPlugin(name, instance as BasePlugin); + } + return ctx; +} + describe("ServerPlugin", () => { let originalEnv: NodeJS.ProcessEnv; @@ -340,7 +350,7 @@ describe("ServerPlugin", () => { process.env.NODE_ENV = "production"; const injectRoutes = vi.fn(); - const plugins: any = { + const testPlugins: any = { "test-plugin": { name: "test-plugin", injectRoutes, @@ -348,7 +358,10 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ + autoStart: false, + context: createContextWithPlugins(testPlugins), + } as any); await plugin.start(); const routerFn = (express as any).Router as ReturnType; @@ -386,7 +399,10 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ + autoStart: false, + context: createContextWithPlugins(plugins), + } as any); await plugin.start(); expect(plugins["plugin-a"].clientConfig).toHaveBeenCalled(); @@ -413,7 +429,10 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ + autoStart: false, + context: createContextWithPlugins(plugins), + } as any); await plugin.start(); expect(plugins["plugin-null"].clientConfig).toHaveBeenCalled(); @@ -444,7 +463,10 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ + autoStart: false, + context: createContextWithPlugins(plugins), + } as any); await expect(plugin.start()).resolves.toBeDefined(); expect(mockLoggerError).toHaveBeenCalledWith( "Plugin '%s' clientConfig() failed, skipping its config: %O", @@ -608,19 +630,19 @@ describe("ServerPlugin", () => { const plugin = new ServerPlugin({ autoStart: false, - plugins: { + context: createContextWithPlugins({ ok: { name: "ok", abortActiveOperations: vi.fn(), - } as any, + }, bad: { name: "bad", abortActiveOperations: vi.fn(() => { throw new Error("boom"); }), - } as any, - }, - }); + }, + }), + } as any); // pretend started (plugin as any).server = mockHttpServer; diff --git a/packages/appkit/src/plugins/server/types.ts b/packages/appkit/src/plugins/server/types.ts index e187cacc..84a2327e 100644 --- a/packages/appkit/src/plugins/server/types.ts +++ b/packages/appkit/src/plugins/server/types.ts @@ -1,9 +1,7 @@ import type { BasePluginConfig } from "shared"; -import type { Plugin } from "../../plugin"; export interface ServerConfig extends BasePluginConfig { port?: number; - plugins?: Record; staticPath?: string; autoStart?: boolean; host?: string; diff --git a/packages/shared/src/plugin.ts b/packages/shared/src/plugin.ts index 9fa8066c..651840c7 100644 --- a/packages/shared/src/plugin.ts +++ b/packages/shared/src/plugin.ts @@ -26,6 +26,15 @@ export interface BasePlugin { exports?(): unknown; clientConfig?(): Record; + + /** + * Binds runtime dependencies (telemetry, cache, plugin context) after the + * plugin has been constructed. Called by the AppKit core before `setup()`. + */ + attachContext?(deps: { + context?: unknown; + telemetryConfig?: TelemetryOptions; + }): void; } /** Base configuration interface for AppKit plugins */