refactor(appkit): retire toPluginWithInstance; consolidate on fromPlugin + fix schema/routing bugs#300
Closed
MarioCadenas wants to merge 5 commits intoagent/5g-retire-deprecated-agentfrom
Closed
Conversation
With `fromPlugin` landed, there is no longer a reason for core plugin
factories to eagerly construct an instance at module scope just to
expose `.toolkit()` on the factory return value. `fromPlugin(factory)`
reads `factory.pluginName` synchronously and defers resolution to
`AgentsPlugin.setup()`, which calls `.toolkit()` as an instance method
on the real plugin.
Changes:
- Swap `toPluginWithInstance(X, ["toolkit"] as const)` to `toPlugin(X)`
in analytics, files, genie, and lakebase. `.toolkit()` remains as an
instance method on each Plugin class — it's still called by the
markdown loader, by `AgentsPlugin.applyAutoInherit`, and by the new
`resolveToolkitFromProvider` helper.
- Delete `toPluginWithInstance` from
`packages/appkit/src/plugin/to-plugin.ts` and its re-export from the
`plugin/` barrel. The `pluginName` stamp on `toPlugin` stays — that's
what `fromPlugin` reads.
- Revert the PR 5e plumbing it enabled: drop `instance?: BasePlugin`
from `OptionalConfigPluginDef` in `packages/shared/src/plugin.ts`,
undo the `preparePlugins` forwarding and the `pluginData.instance`
reuse branch in `packages/appkit/src/core/appkit.ts`, and delete the
instance-reuse test. Nothing produces an `instance` field anymore.
- Extract the duplicated provider-resolution logic (previously
copy-pasted between `AgentsPlugin` and `run-agent`) into a single
shared helper at
`packages/appkit/src/plugins/agents/toolkit-resolver.ts`. Both the
`fromPlugin` marker resolution, the markdown auto-inherit path, and
the standalone `runAgent` executor now go through it.
- Rewrite the `describe("toolkit()")` block in analytics.test.ts to
construct `AnalyticsPlugin` directly instead of reaching for the
factory return value; the behavior being tested lives on the class.
- Docs: drop the "Using `.toolkit()` directly (advanced)" subsection
from `agents.md` and the "before / after" duplication from the
migration guide. `fromPlugin` is now the only documented path.
No public-API regression for the new shape: every in-tree migration
already went through `fromPlugin` in PR 5f. Users who were still
spreading `.toolkit()` at module scope (not possible in-tree, but the
escape hatch was documented) need to switch to `fromPlugin(factory,
opts)` — same options, one less object-lifetime concern.
Net: ~370 lines removed, one canonical factory abstraction, one shared
resolver.
Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
Regenerates the generated typedoc markdown for `docs/docs/api/appkit/` so it matches the final state of the agent stack: - New pages for the surfaces added in PRs 5b–5h: `Function.fromPlugin`, `Function.isFromPluginMarker`, `Function.isToolkitEntry`, `Function.loadAgentFromFile`, `Function.loadAgentsFromDir`, `Function.runAgent`, `Interface.AgentDefinition`, `Interface.AgentsPluginConfig`, `Interface.FromPluginMarker`, `Interface.PromptContext`, `Interface.RunAgentInput`, `Interface.RunAgentResult`, `Interface.ToolkitEntry`, `Interface.ToolkitOptions`, `TypeAlias.AgentTools`, `TypeAlias.BaseSystemPromptOption`, `Variable.agents`. - Removed pages for the retired shortcut API: `Interface.AgentHandle`, `Interface.CreateAgentConfig`. - Refreshed sidebar, index, and a handful of pages whose rendered signatures changed as fields were added/removed upstream. These are derived artifacts — they don't carry narrative-appropriate home branches, so they land here at the tip, in sync with the final code state. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
Zod v4's `toJSONSchema()` emits a top-level `$schema` annotation (e.g. `"https://json-schema.org/draft/2020-12/schema"`) on every schema it produces. When an agent runs against a Databricks Model Serving endpoint backed by Google Gemini (e.g. `databricks-gemini-3-1-flash-lite`), Mosaic forwards tool schemas as Gemini `function_declarations[i].parameters`. That format accepts only a restricted JSON Schema subset and rejects any unrecognized top-level key, including `$schema`, with a 400: ``` Databricks API error (400): Invalid JSON payload received. Unknown name "$schema" at 'tools[0].function_declarations[0].parameters': Cannot find field. ``` Every chat turn that exposed tools failed with 9x (one per tool) field violations. The bug existed in all three call sites that materialize tool parameters from Zod: - `tool()` — the user-facing inline tool factory - `defineTool()` registry + `toolsFromRegistry()` — used by every plugin's `getAgentTools()` (analytics, files, genie, lakebase) - `buildToolkitEntries()` — the internal `.toolkit()` path used by markdown frontmatter resolution and auto-inherit Adds a small shared helper `toToolJSONSchema(schema)` at `packages/appkit/src/plugins/agents/tools/json-schema.ts` that wraps `toJSONSchema()` and strips the top-level `$schema` key. Swaps all three call sites to use it. The generated `parameters` record is otherwise unchanged (`type`, `properties`, `required`, `additionalProperties` all pass through), so no downstream semantics shift. Confirmed via runtime instrumentation that every outbound tool after the fix has `paramKeys: ["type", "properties", ...]` with `hasDollarSchema: false`, and that the autocomplete agent (which gets all 9 auto-inherited plugin tools under its markdown defaults) now streams completions without a 400. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
The file was committed with a malformed frontmatter block: the opening `---` was followed by `## endpoint: databricks-claude-sonnet-4-5` (a Markdown heading, not a YAML key) and the closing `---` was missing entirely, so `js-yaml` parsed no keys at all. Effect: the `assistant` agent still registered (via the body-only path and the plugin's adapter fallback), but `default: true` never took effect. `fileDefault` resolved to `null`, and the default agent was picked by insertion order instead of intent. Chat requests without an explicit `agent:` field landed wherever that fallback happened to point, producing confusing "which agent am I actually talking to" behavior. Rewrites the frontmatter as valid YAML with a closing `---`. Confirmed via runtime instrumentation that `loadAgents` now reports `fileDefault: "assistant"` and default-agent routing is stable. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
The working tree was persistently dirty across the stack because four independent generators / formatters kept rewriting committed content on every `pnpm dev`: - `template/appkit.plugins.json` is regenerated by `pnpm sync:template`, which scans `packages/appkit/src/plugins/*/manifest.json` for the canonical plugin name. The agents plugin's manifest declares `"name": "agent"` (singular, so routes mount at `/api/agent/*` to match the client's expectations), so the template's key should be `"agent"`. An earlier commit in the stack had it as `"agents"`, which `sync:template` corrected on every run. - `packages/appkit/src/registry/types.generated.ts` is regenerated by `tools/generate-registry-types.ts` (fires via `appkit#build:package` → `build:watch`). The committed version had stale formatting. - `packages/shared/src/schemas/plugin-manifest.generated.ts` is regenerated by `tools/generate-schema-types.ts` (same mechanism on the shared package). Same stale-formatting situation. - `run-agent.ts`, `agents.ts`, `from-plugin.ts`, `run-agent.test.ts` — biome format drift. A `pnpm format` run touched committed whitespace / line-wrap that no longer matched current rules. Committing all of these so the working tree settles. No semantic changes. Subsequent `pnpm dev` runs should leave the tree clean. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
This was referenced Apr 21, 2026
Collaborator
Author
|
Superseded by the v2 6-PR stack:
The v2 stack reorganizes the same work so no PR ships API that a later PR deletes. Start at #301 for the new entry point. Branches from this older stack are preserved unchanged. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Final cleanup layer. With
fromPluginas the canonical path for referencing plugin tools, there's no longer a reason for core plugin factories to eagerly construct an instance at module scope just to expose.toolkit()on the factory return value. This PR retires the eager pattern, consolidates the duplicated resolver logic, and bundles four runtime bug fixes surfaced during end-to-end testing of the full stack.Retire
toPluginWithInstancetoPluginWithInstance(X, ["toolkit"] as const)totoPlugin(X)in analytics, files, genie, lakebase..toolkit()remains as an instance method on each Plugin class — still called by the markdown loader, byAgentsPlugin.applyAutoInherit, and by the new shared resolver.toPluginWithInstancefrompackages/appkit/src/plugin/to-plugin.tsand its re-export. ThepluginNamestamp ontoPluginstays — that's whatfromPluginreads.instancefield anymore): dropinstance?: BasePluginfromOptionalConfigPluginDef, undo thepreparePluginsforwarding and thepluginData.instancereuse branch incore/appkit.ts, delete the instance-reuse test.describe("toolkit()")block inanalytics.test.tsto constructAnalyticsPlugindirectly.Extract shared toolkit resolver
New
packages/appkit/src/plugins/agents/toolkit-resolver.tscontainsresolveToolkitFromProvider— the single source of truth for "turn a ToolProvider into a keyed ToolkitEntry record". Used by:AgentsPlugin.buildToolIndex—fromPluginmarker resolutionAgentsPlugin.applyAutoInherit— markdown auto-inherit pathrunAgent's standalone executorPreviously this logic was copy-pasted across all three sites.
Bug fixes (bundled into this PR)
These surfaced during end-to-end testing of dev-playground and are included here rather than as a separate PR because they touch the same files:
fix(appkit): strip $schema from Zod-generated tool parameter schemas— Zod v4'stoJSONSchema()emits a top-level$schemakey. Databricks Mosaic serving forwards tool schemas to Gemini'sfunction_declarationsformat, which rejects unknown top-level keys. Every tool-using chat on a Gemini-backed endpoint was failing with a 400. New helpertoToolJSONSchema()atpackages/appkit/src/plugins/agents/tools/json-schema.ts; applied at the three materialization sites (tool(),toolsFromRegistry,buildToolkitEntries).fix(dev-playground): repair broken YAML frontmatter in assistant.md— missing closing---+## default: trueas a markdown heading →js-yamlparsed no keys →default: truenever took effect. Default-agent routing was picked by insertion order rather than intent. Rewritten as valid YAML.docs(appkit): regenerate typedoc API reference after stack cleanup— syncsdocs/docs/api/appkit/with the final surface (17 new pages forfromPlugin/runAgent/AgentDefinition/ etc., 2 deleted for the retired shortcut API).chore: sync generated files and refresh biome formatting— seeds the correct baseline fortemplate/appkit.plugins.json,packages/appkit/src/registry/types.generated.ts,packages/shared/src/schemas/plugin-manifest.generated.ts, and biome drift across 4 files. Subsequentpnpm devruns leave the tree clean.Net change
~297 lines removed (net), one canonical factory abstraction (
toPluginonly), one shared resolver, no deprecated surface left.PR Stack
agents()plugin +createAgent(def)+.toolkit()— feat(appkit): add agents() plugin, createAgent() factory, and .toolkit() #294agents()— feat(appkit): migrate agent-app and docs to the new agents() plugin #295preparePluginsforwards eager instance — refactor(appkit): forward eager plugin instance through preparePlugins #297fromPlugin()API — feat(appkit): add fromPlugin() for referencing plugin tools in code-defined agents #298agent()+createAgentApp— chore(appkit): remove deprecated agent() plugin and createAgentApp shortcut #299toPluginWithInstance+ bug fixes (this PR)Test plan
build:package+docs:build)autocompleteagent and the code-definedhelperagent; agent-appsupportagent routes analytics + files tool calls correctly