From 27fc210f2a1e4dae8217092c8bfa9f5c5cc25f90 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 20 Apr 2026 12:15:29 +0200 Subject: [PATCH] feat(appkit): separate plugin context binding from construction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the telemetry/cache/context wiring out of the Plugin constructor into a new attachContext(deps) method called by AppKit after construction. Lets plugin factories eagerly construct instances at module top-level without depending on TelemetryManager/CacheManager being initialized yet. - Plugin.constructor stays pure by default but opportunistically binds telemetry+cache if the core services are already initialized (backward compat with existing tests that new PluginX(config) directly). - Plugin.attachContext(deps) is idempotent and does the full binding, including context. AppKit._createApp calls it after construction. - ServerPlugin overrides attachContext to register HTTP/Express instrumentations and become the route target (previously in its constructor). - BasePlugin.attachContext added to the shared interface as optional. Also defer ServerPlugin's start() to the 'setup:complete' lifecycle hook when running through AppKit. Previously start() ran inside setup() and extendRoutes() iterated context.getPlugins() synchronously — so any deferred-phase plugin declared after server() in the plugin array was invisible to the route-mount loop and its /api//* routes silently dropped. The fix also keeps the pre-AppKit path (direct new ServerPlugin(...) construction) working by falling back to immediate start() when this.context is undefined. No behavior change when used through createApp. Enables upcoming .toolkit() support on plugin factories where the factory eagerly constructs the plugin instance at call time. Signed-off-by: MarioCadenas --- packages/appkit/src/core/appkit.ts | 7 +++ packages/appkit/src/plugin/plugin.ts | 51 +++++++++++++++++++-- packages/appkit/src/plugins/server/index.ts | 20 +++++++- packages/shared/src/plugin.ts | 9 ++++ 4 files changed, 81 insertions(+), 6 deletions(-) diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index 9e00da1d..a0c2e566 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -78,6 +78,13 @@ 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); diff --git a/packages/appkit/src/plugin/plugin.ts b/packages/appkit/src/plugin/plugin.ts index 1581dc53..4c9a0e64 100644 --- a/packages/appkit/src/plugin/plugin.ts +++ b/packages/appkit/src/plugin/plugin.ts @@ -164,11 +164,11 @@ 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 */ @@ -195,15 +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/plugins/server/index.ts b/packages/appkit/src/plugins/server/index.ts index b6efd016..4c911b10 100644 --- a/packages/appkit/src/plugins/server/index.ts +++ b/packages/appkit/src/plugins/server/index.ts @@ -59,6 +59,10 @@ 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, @@ -68,9 +72,21 @@ export class ServerPlugin extends Plugin { /** 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. */ 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 */