-
Notifications
You must be signed in to change notification settings - Fork 14
feat(appkit): fromPlugin() DX, runAgent plugins arg, shared toolkit-resolver #305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
MarioCadenas
wants to merge
6
commits into
agent/v2/4-agents-plugin
Choose a base branch
from
agent/v2/5-fromplugin-runagent
base: agent/v2/4-agents-plugin
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
20ad673
feat(appkit): fromPlugin() DX, runAgent plugins arg, shared toolkit-r…
MarioCadenas ba823c3
fix(agents): update agents.ts imports to core/agent/ paths after Opti…
MarioCadenas 44f0425
refactor(appkit): move agent runtime to core/agent/
MarioCadenas 74d874b
refactor(appkit): split agents.ts helpers into separate modules
MarioCadenas e4f92d7
feat(appkit): unify on DATABRICKS_SERVING_ENDPOINT_NAME (SDK + templa…
MarioCadenas 63bdfc0
chore(appkit): drop duplicate prompt/registry-printer modules after r…
MarioCadenas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import type { NamedPluginFactory } from "../../plugin/to-plugin"; | ||
| import type { ToolkitOptions } from "./types"; | ||
|
|
||
| /** | ||
| * Symbol brand for the `fromPlugin` marker. Using a globally-interned symbol | ||
| * (`Symbol.for`) keeps the brand stable across module boundaries / bundle | ||
| * duplicates so `isFromPluginMarker` stays reliable. | ||
| */ | ||
| export const FROM_PLUGIN_MARKER = Symbol.for( | ||
| "@databricks/appkit.fromPluginMarker", | ||
| ); | ||
|
|
||
| /** | ||
| * A lazy reference to a plugin's tools, produced by {@link fromPlugin} and | ||
| * resolved to concrete `ToolkitEntry`s at `AgentsPlugin.setup()` time. | ||
| * | ||
| * The marker is spread under a unique symbol key so multiple calls to | ||
| * `fromPlugin` (even for the same plugin) coexist in an `AgentDefinition.tools` | ||
| * record without colliding. | ||
| */ | ||
| export interface FromPluginMarker { | ||
| readonly [FROM_PLUGIN_MARKER]: true; | ||
| readonly pluginName: string; | ||
| readonly opts: ToolkitOptions | undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Record shape returned by {@link fromPlugin} — a single symbol-keyed entry | ||
| * suitable for spreading into `AgentDefinition.tools`. | ||
| */ | ||
| export type FromPluginSpread = { readonly [key: symbol]: FromPluginMarker }; | ||
|
|
||
| /** | ||
| * Reference a plugin's tools inside an `AgentDefinition.tools` record without | ||
| * naming the plugin instance. The returned spread-friendly object carries a | ||
| * symbol-keyed marker that the agents plugin resolves against registered | ||
| * `ToolProvider`s at setup time. | ||
| * | ||
| * The factory argument must come from `toPlugin` (or any function that | ||
| * carries a `pluginName` field). `fromPlugin` reads `factory.pluginName` | ||
| * synchronously — it does not construct an instance. | ||
| * | ||
| * If the referenced plugin is also registered in `createApp({ plugins })`, the | ||
| * same runtime instance is used for dispatch. If the plugin is missing, | ||
| * `AgentsPlugin.setup()` throws with a clear `Available: …` listing. | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * import { analytics, createAgent, files, fromPlugin, tool } from "@databricks/appkit"; | ||
| * | ||
| * const support = createAgent({ | ||
| * instructions: "You help customers.", | ||
| * tools: { | ||
| * ...fromPlugin(analytics), | ||
| * ...fromPlugin(files, { only: ["uploads.read"] }), | ||
| * get_weather: tool({ ... }), | ||
| * }, | ||
| * }); | ||
| * ``` | ||
| * | ||
| * @param factory A plugin factory produced by `toPlugin`. Must expose a | ||
| * `pluginName` field. | ||
| * @param opts Optional toolkit scoping — `prefix`, `only`, `except`, `rename`. | ||
| * Same shape as the `.toolkit()` method. | ||
| */ | ||
| export function fromPlugin<F extends NamedPluginFactory>( | ||
| factory: F, | ||
| opts?: ToolkitOptions, | ||
| ): FromPluginSpread { | ||
| if ( | ||
| !factory || | ||
| typeof factory.pluginName !== "string" || | ||
| !factory.pluginName | ||
| ) { | ||
| throw new Error( | ||
| "fromPlugin(): factory is missing pluginName. Pass a factory created by toPlugin().", | ||
| ); | ||
| } | ||
| const pluginName = factory.pluginName; | ||
| const marker: FromPluginMarker = { | ||
| [FROM_PLUGIN_MARKER]: true, | ||
| pluginName, | ||
| opts, | ||
| }; | ||
| return { [Symbol(`fromPlugin:${pluginName}`)]: marker }; | ||
| } | ||
|
|
||
| /** | ||
| * Type guard for {@link FromPluginMarker}. | ||
| */ | ||
| export function isFromPluginMarker(value: unknown): value is FromPluginMarker { | ||
| return ( | ||
| typeof value === "object" && | ||
| value !== null && | ||
| (value as Record<symbol, unknown>)[FROM_PLUGIN_MARKER] === true | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| /** | ||
| * 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 { | ||
| FROM_PLUGIN_MARKER, | ||
| type FromPluginMarker, | ||
| type FromPluginSpread, | ||
| fromPlugin, | ||
| isFromPluginMarker, | ||
| } from "./from-plugin"; | ||
| 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 AutoInheritToolsConfig, | ||
| type BaseSystemPromptOption, | ||
| isToolkitEntry, | ||
| type PromptContext, | ||
| type RegisteredAgent, | ||
| type ResolvedToolEntry, | ||
| type ToolkitEntry, | ||
| type ToolkitOptions, | ||
| } from "./types"; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I still don't like the
fromPluginAPI. Asked an agent if that's just me and looks like the community usually takes a similar approach to the one I suggest 😄 See the report at the end of the comment.My suggestion is to: instead of
go with this:
What do you think?
The chicken-and-egg problem
The tension: agent definitions need plugin tools, but plugins need AppKit context (WorkspaceClient, PluginContext, OBO) to be functional. So at definition time, nothing is "ready."
Common JS patterns for this
1. Two-phase init (your preference)
Create a handle, wire references, then initialize. This is how Express, Fastify, and Koa work — you instantiate middleware/plugins, compose them, then call
listen():In AppKit terms, your preferred API would look like:
This is totally achievable —
analytics()already returns a data bag. It could also expose.toolkit()that returns the same lazy markersfromPlugincreates internally. Same resolution mechanism, more explicit surface.2. DI container / token-based (Angular, NestJS, InversifyJS)
Register by class or token, resolve lazily:
This is basically what
fromPlugindoes — reference-by-name, resolve-at-startup — but dressed up with decorators. Heavy for a non-decorator codebase.3. Builder / fluent API (Hono, tRPC)
Collect everything, resolve at
.build():Clean, but loses the "agents are plain objects" composability.
4. String references (Terraform, Spring, Django settings)
Just use names, resolve later. This is what markdown agents already do with
toolkits: [analytics]in frontmatter — it's the YAML equivalent offromPlugin(analytics).My honest assessment
fromPluginis pattern #2 disguised as a spread operator. It works, but has two DX friction points:It's an unusual pattern —
...fromPlugin(analytics)looks like it's doing something, but it's just creating a{ [Symbol]: { pluginName: "analytics" } }marker. The magic is hidden.It introduces a separate concept — you need to learn
fromPluginas a standalone primitive. In your preferred approach (chore: rework TelemetryManager to use Node SDK #1),.toolkit()is a method on the thing you already have — no new import, no new concept.The good news: these aren't mutually exclusive. The factory data bag returned by
analytics()could grow a.toolkit()method that produces the same markers.fromPlugincould remain as a lower-level escape hatch (e.g., for dynamic plugin names). This is actually a very small surface change —toPluginjust needs to stamp a.toolkit()method alongside the existing.pluginName.What the broader JS community leans toward
Explicit instance references (your preference) is the dominant pattern. Express, Fastify, Hono, tRPC, Drizzle, Prisma — almost nobody uses magic marker/symbol indirection. The standard expectation is: "I created it, I hold a reference to it, I pass that reference where it's needed."
The
fromPluginpattern is closer to how Terraform or Pulumi work (reference by logical name, resolve in a plan phase), which makes sense for infrastructure-as-code but feels foreign in a JS application SDK.