Skip to content

feat(appkit): plugin infrastructure — attachContext + PluginContext mediator#303

Open
MarioCadenas wants to merge 1 commit intoagent/v2/2-tool-primitivesfrom
agent/v2/3-plugin-infra
Open

feat(appkit): plugin infrastructure — attachContext + PluginContext mediator#303
MarioCadenas wants to merge 1 commit intoagent/v2/2-tool-primitivesfrom
agent/v2/3-plugin-infra

Conversation

@MarioCadenas
Copy link
Copy Markdown
Collaborator

@MarioCadenas MarioCadenas commented Apr 21, 2026

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:

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 MarioCadenas@users.noreply.github.com

PR Stack

  1. Shared agent types + LLM adapters — feat(appkit): shared agent types and LLM adapter implementations #301
  2. Tool primitives + ToolProvider surfaces — feat(appkit): tool primitives and ToolProvider surfaces on core plugins #302
  3. Plugin infrastructure (attachContext + PluginContext) (this PR)
  4. agents() plugin + createAgent(def) + markdown-driven agents — feat(appkit): agents() plugin, createAgent(def), and markdown-driven agents #304
  5. fromPlugin() DX + runAgent plugins arg + toolkit-resolver — feat(appkit): fromPlugin() DX, runAgent plugins arg, shared toolkit-resolver #305
  6. Reference app + dev-playground + docs — feat(appkit): reference agent-app, dev-playground chat UI, docs, and template #306

Demo

agent-demo.mp4

This was referenced Apr 21, 2026
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from b328cf2 to 5a7a4df Compare April 21, 2026 20:41
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from a5642df to e26795b Compare April 21, 2026 20:41
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from 5a7a4df to a384b1e Compare April 22, 2026 08:45
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from e26795b to d73e138 Compare April 22, 2026 08:45
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from a384b1e to 68e05d3 Compare April 22, 2026 09:24
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from d73e138 to 26f43e5 Compare April 22, 2026 09:24
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from 68e05d3 to b765708 Compare April 22, 2026 09:59
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from 26f43e5 to 2ffa31d Compare April 22, 2026 09:59
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from b765708 to 6712ce7 Compare April 22, 2026 10:21
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch 2 times, most recently from 3a4deb7 to ca9cfca Compare April 22, 2026 10:49
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from 6712ce7 to 7077eb0 Compare April 22, 2026 10:50
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from 863439e to cca914f Compare April 29, 2026 20:18
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from 7d7f66a to 22e267c Compare April 29, 2026 20:18
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from cca914f to a3d2cc6 Compare May 4, 2026 09:22
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch 2 times, most recently from 724e48a to 39e7091 Compare May 4, 2026 09:41
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch 2 times, most recently from f495962 to c15858b Compare May 4, 2026 11:19
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch 2 times, most recently from 0e06004 to a70bbcc Compare May 4, 2026 12:59
@MarioCadenas MarioCadenas requested a review from a team as a code owner May 4, 2026 12:59
@MarioCadenas MarioCadenas requested review from atilafassina and removed request for a team May 4, 2026 12:59
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch 3 times, most recently from aa95c27 to 71a986d Compare May 4, 2026 13:15
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from a70bbcc to 3343203 Compare May 4, 2026 16:36
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from 71a986d to 82d8ea0 Compare May 4, 2026 16:36
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from 3343203 to 85663cf Compare May 4, 2026 17:18
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch 2 times, most recently from a8418ea to c9c986d Compare May 4, 2026 17:32
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from a63b7a4 to 47c1c68 Compare May 5, 2026 10:09
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from c9c986d to 5fde8aa Compare May 5, 2026 10:09
@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch from 47c1c68 to 6f83621 Compare May 5, 2026 10:28
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch 2 times, most recently from 4a388b1 to a7ebc57 Compare May 5, 2026 10:40
Copy link
Copy Markdown
Member

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

LGTM but please verify the agentic review findings before merge 👍

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agentic review:

Findings

P1 — High

# File Issue Reviewer Conf. Route
1 plugin-context.ts:226 Lifecycle hook Set mutated during iteration. emitLifecycle iterates for (const fn of hooks) over a Set. If a callback calls context.onLifecycle() for the same event, the Set is mutated mid-iteration. Per ECMAScript spec, newly added Set entries during for...of WILL be visited — this can cause unexpected extra executions or infinite loops. adversarial, correctness 0.88 safe_auto -> review-fixer
2 plugin.ts:79 attachContext missing from EXCLUDED_FROM_PROXY. The EXCLUDED_FROM_PROXY set lists lifecycle methods that asUser() should not proxy. attachContext is absent, meaning plugin.asUser(req).attachContext(...) would run in user context and could mutate the plugin's telemetry, cache, and context bindings. Other lifecycle methods (setup, shutdown, injectRoutes) are already excluded. api-contract, adversarial 0.82 safe_auto -> review-fixer

P2 — Moderate

# File Issue Reviewer Conf. Route
3 plugin-context.ts:174,178 OTel span status uses magic numbers instead of SpanStatusCode enum. span.setStatus({ code: 0 }) and span.setStatus({ code: 2 }) use raw integers. The codebase pattern in TelemetryInterceptor uses SpanStatusCode.OK / SpanStatusCode.ERROR. Works by coincidence but fragile and inconsistent. reliability 0.95 safe_auto -> review-fixer
4 plugin-context.ts:119 getPlugins() returns mutable internal Map. Any caller can mutate the plugin registry (set, delete, clear). Server plugin iterates over it but external access could corrupt state. security 0.70 safe_auto -> review-fixer
5 plugin-context.ts:168 Unsafe (entry.plugin as any).asUser(req) cast bypasses type safety. ToolProvider doesn't declare asUser; it lives on the Plugin base class. The as any cast defeats TypeScript's ability to catch signature drift. The existing isToolProvider() guard could be extended post-asUser for runtime safety. kieran-ts, security 0.85 gated_auto -> downstream-resolver
6 plugin-context.ts:85 Double registerAsRouteTarget call silently overwrites. No guard prevents a second call. The first target's extensions registered after the second call would be lost. In practice only ServerPlugin calls this, but a defensive guard prevents silent misconfiguration. adversarial 0.75 manual -> human
7 plugin-context.ts:103 Tool provider name collision silently accepted. registerToolProvider uses Map.set() without checking for duplicates. A second plugin with the same name silently overwrites the first, making the first plugin's tools unreachable. adversarial 0.72 manual -> human
8 plugin-context.ts:161 Hardcoded 30s timeout in executeTool not configurable. Different tools have different performance profiles (ML inference vs. quick lookups). No way to override per-call or per-context. maintainability 0.75 advisory -> human
9 appkit.ts:46,85 Double context injection: constructor config + attachContext. Context is passed in extraData to the constructor (line 46) AND then via attachContext (line 85). The Plugin constructor extracts it at line 216. Both paths set the same context instance so it's not a bug, but the dual path is confusing and creates maintenance risk. maintainability, testing 0.70 advisory -> human
10 plugin.ts:216 Unsafe cast (config as Record<string, unknown>).context in Plugin constructor. BasePluginConfig doesn't declare a context field. The cast bypasses TypeScript to extract context from an enriched config object. Future config changes could silently break this. kieran-ts 0.92 gated_auto -> downstream-resolver

P3 — Low

# File Issue Reviewer Conf. Route
11 plugin-context.ts:18 ToolProviderEntry interface adds unnecessary indirection. Wraps plugin + name, but name is already the Map key. Could store ToolProvider directly and derive name from the key. maintainability 0.80 safe_auto -> review-fixer

Applied Fixes (Plan)

The following safe_auto findings should be fixed:

Fix 1: Snapshot lifecycle hooks before iteration (Finding #1)

File: packages/appkit/src/core/plugin-context.ts:226

Change:

for (const fn of hooks) {

To:

for (const fn of [...hooks]) {

This creates an array snapshot so mutations during iteration don't affect the loop.

Fix 2: Add attachContext to EXCLUDED_FROM_PROXY (Finding #2)

File: packages/appkit/src/plugin/plugin.ts:79-92

Add "attachContext" to the Set:

const EXCLUDED_FROM_PROXY = new Set([
  // Lifecycle methods
  "setup",
  "shutdown",
  "attachContext",   // <-- add this
  "injectRoutes",
  ...
]);

Fix 3: Use SpanStatusCode enum (Finding #3)

File: packages/appkit/src/core/plugin-context.ts:174,178

Import SpanStatusCode from the telemetry module and replace magic numbers:

import { SpanStatusCode } from "@opentelemetry/api";
// ...
span.setStatus({ code: SpanStatusCode.OK });
// ...
span.setStatus({ code: SpanStatusCode.ERROR, message: ... });

Check how TelemetryInterceptor imports it — it uses import { SpanStatusCode } from "../../telemetry".

Fix 4: Return defensive copy from getPlugins() (Finding #4)

File: packages/appkit/src/core/plugin-context.ts:119-121

Change:

getPlugins(): Map<string, BasePlugin> {
  return this.plugins;
}

To:

getPlugins(): ReadonlyMap<string, BasePlugin> {
  return this.plugins;
}

TypeScript's ReadonlyMap prevents set/delete/clear at compile time. No runtime overhead.

Fix 5: Remove ToolProviderEntry indirection (Finding #11)

File: packages/appkit/src/core/plugin-context.ts

  • Remove the ToolProviderEntry interface (lines 18-21)
  • Change toolProviders to Map<string, BasePlugin & ToolProvider>
  • Update registerToolProvider to store the plugin directly
  • Update getToolProviders to map entries to { name, provider } from the Map

Residual Actionable Work

These findings should be addressed but require human judgment:

  1. Finding chore: tune tsdown configuration to reduce bundle size #5 — The as any cast in executeTool: Consider adding a HasAsUser interface or runtime check post-asUser using isToolProvider().
  2. Finding WIP: feat: add volume serving plugin #6 — Guard registerAsRouteTarget against double calls: Add if (this.routeTarget) throw new Error(...).
  3. Finding chore: add release to NPM workflow #7 — Warn on tool provider name collision: Add if (this.toolProviders.has(name)) logger.warn(...).
  4. Finding fix: npm publishing #10 — Unsafe config context extraction: Consider an InternalPluginConfig type or removing constructor context extraction in favor of attachContext-only path.

Testing Gaps

The following test coverage gaps were identified across reviewers:

  1. executeTool timeout/signal combination — No test verifies the 30s timeout fires, or that AbortSignal.any() correctly combines user signal + timeout signal.
  2. emitLifecycle warning for unapplied routes — No test for the setup:complete warning when routes are buffered but no server registered.
  3. Plugin.attachContext unit tests — No dedicated tests for attachContext config merging, idempotence, or telemetry binding.
  4. ServerPlugin.attachContext + registerAsRouteTarget — No test verifies attachContext triggers route target registration.
  5. AppKit ToolProvider auto-registration — No test verifies isToolProvider() check + registerToolProvider() in createAndRegisterPlugin.
  6. executeTool telemetry span assertions — Tests mock the span but never assert setStatus/recordException/end are called with correct values.
  7. Double registerAsRouteTarget behavior — No test for what happens when called twice.
  8. Lifecycle hook mutation during iteration — No test for hooks that register new hooks during execution.

Agent-Native Gaps

The agent-native reviewer identified that while PluginContext.executeTool() provides the right infrastructure for tool execution, it is not yet wired to AgentRunContext or exposed via HTTP endpoints for agent consumption. This is expected to be addressed in a subsequent PR in the agent/v2 series and is not a blocker for this infrastructure PR.

Learnings & Past Solutions

No relevant past solutions found in docs/solutions/. The learnings researcher confirmed the implementation patterns (route buffering, live registry, lifecycle hooks) are sound and well-tested.

Coverage

  • Suppressed: 0 findings below 0.60 confidence
  • Failed reviewers: 0
  • Untracked files: None
  • Project standards: Full compliance with CLAUDE.md (conventional commits, vitest, Biome, correct file placement, shared types)

Verdict: Ready with fixes

The architecture is sound. The PluginContext mediator is well-designed with proper separation of concerns. Five safe_auto fixes should be applied (lifecycle Set snapshot, EXCLUDED_FROM_PROXY, SpanStatusCode enum, ReadonlyMap, ToolProviderEntry cleanup). Four manual/gated_auto findings are recommended for human review. Eight testing gaps should be addressed before merge.

Fix order:

  1. Fix chore: rework TelemetryManager to use Node SDK #1 (P1 — Set mutation safety)
  2. Fix feat: reexport shadcn components and theme #2 (P1 — EXCLUDED_FROM_PROXY)
  3. Fix chore: fix ci runners #3 (P2 — SpanStatusCode)
  4. Fix feat: arrow stream #4 (P2 — ReadonlyMap)
  5. Fix chore: tune tsdown configuration to reduce bundle size #5 (P3 — ToolProviderEntry cleanup)

Verification

After applying fixes:

pnpm check:fix && pnpm -r typecheck && pnpm test

@MarioCadenas MarioCadenas force-pushed the agent/v2/2-tool-primitives branch 2 times, most recently from a2c725e to 8b67c5a Compare May 6, 2026 18:35
…nContext mediator

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.

`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`.

`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.

`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.

`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).

- 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 <MarioCadenas@users.noreply.github.com>
@MarioCadenas MarioCadenas force-pushed the agent/v2/3-plugin-infra branch from a7ebc57 to 91e66e1 Compare May 6, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants