fix(routes,mcp,package): movie-library port findings — config augments generated servers, result _meta, conditional outputSchema, no tsc shadowing

.changeset/movie-library-port-findings.md

@@ -0,0 +1,24 @@
+---
+"@agent-bundle/runtime": minor
+"agent-bundle": minor
+---
+
+Fixes found while re-porting a real external plugin onto route mode
+(#380, #381, #383):
+
+- A `mcp.servers.<id>` declaration for a route-generated server now
+  **augments** that server — `env`, `args`, `targets`, `apps`, and
+  `transport: 'stdio'` apply — instead of failing `AB4304`/`AB4322`. Redeclaring
+  `entry`, `command`, or `url` beside `routes.servers.<id>: 'generated'` is
+  the new precise `AB4340` error; without an explicit mode it stays `AB4800`.
+- `Agent.Result metadata` projects to `CallToolResult._meta` (an object,
+  JSON-snapshotted like `structuredContent`; a non-object fails the projection
+  closed with `McpProjectionError('invalid-result-metadata')`). The
+  `mcp-in-memory` harness result exposes `_meta`.
+- Generated tools advertise `outputSchema` only when the route's
+  `resultSchema` describes an object; text-only routes (for example
+  `resultSchema = z.undefined()`) advertise none and return no
+  `structuredContent`, as the MCP specification requires.
+- The `typescript-5` parser alias is bundled into the package instead of
+  shipped as a dependency, so `npm install agent-bundle` never links a `tsc`
+  bin over the consumer's own TypeScript.

docs/diagnostics.md

@@ -14,7 +14,7 @@ gate a build, a validation, or a dev rebuild.
 | `AB40xx` | Plugin metadata and Skill source validation (`AB4000`/`AB4001`: name/version; `AB4002`–`AB4007`: Skill fields; `AB4008`–`AB4011` and `AB4013`: release identity, see below; `AB4012`: declared `plugin.logo` is missing, not a file, or outside the project). |
 | `AB41xx` | Normalized model invariants (unknown targets, duplicate IDs and outputs). |
 | `AB42xx` | Hook configuration and native hook sources. |
-| `AB43xx` | MCP server and MCP App configuration. |
+| `AB43xx` | MCP server and MCP App configuration (`AB4340`: a declaration for a route-generated server redeclares `entry`/`command`/`url`; see below). |
 | `AB44xx` | Script configuration. |
 | `AB4500` | Registered config extensions (strict finite JSON). |
 | `AB46xx` | Assets and the generated-runtime floor. |
@@ -289,6 +289,20 @@ simply not been built yet is a validation **warning** that only
 | `AB4749` | error (build) | A payload directory overlaps the artifact `--output` root. |
 | `AB4750` | info | A payload is older than the newest project source file and may be stale; rerun the project's own build if so. |
 
+## Config beside a route-generated MCP server (`AB4340`)
+
+A `mcp.servers.<id>` block for a server the route graph compiles in
+`generated` mode augments that server (`env`, `args`, `targets`, `apps`,
+`transport: 'stdio'`) — see the precedence table in
+[Entry conventions](entry-conventions.md#config-beside-a-route-generated-mcp-server).
+The local-entry field rules apply to it unchanged (`AB4305`, `AB4308`–`AB4312`,
+`AB432x`), and it never triggers `AB4304` or `AB4322`: the route modules are
+its entry.
+
+| Code | Severity | Trigger |
+| --- | --- | --- |
+| `AB4340` | error | A declaration for a route-generated server sets `entry`, `command`, or `url` while `routes.servers.<id>` is `generated`. The routes already compile this server, so a second entry claim has no reading the compiler could honor. Remove the field to keep the generated server (the other fields still apply), or set the mode to `custom`, `command`, or `remote` to serve the declared entry and omit the routes. Without an explicit mode the same collision is `AB4800`. |
+
 ## Conventional host components: rules and commands (`AB4900`–`AB4906`, `AB4920`–`AB4926`)
 
 Conventional `src/rules/*.mdc` documents compile to the Rule IR (closed

docs/entry-conventions.md

@@ -87,6 +87,26 @@ entries carry `provenance.kind: 'conventional'` in the normalized model.
 Route and package entry conventions match `.ts` and `.tsx` files exactly;
 the state convention is specifically `src/state.ts`.
 
+### Config beside a route-generated MCP server
+
+A `mcp.servers.<server>` block whose `<server>` the route graph compiles in
+`generated` mode does not redeclare the server — its entry is the route
+modules — it **augments** it. This is the precedence table for one generated
+server (config wins, conventions fill):
+
+| Field | Source of truth | Config declaration |
+| --- | --- | --- |
+| Entry, transport (`stdio`), `cwd` (plugin root) | `src/mcp/<server>/{tools,resources,prompts}/*` and the generated stdio shell | `entry`, `command`, or `url` is `AB4340` under `routes.servers.<server>: 'generated'` and `AB4800` without an explicit mode; `transport: 'stdio'` is accepted, any other transport is `AB4308`; `cwd` is `AB4309`; `headers` is `AB4310`. |
+| `env` | — | Applied verbatim beneath the injected plugin-root anchor (`AB4312` shape rules). |
+| `args` | The content-hashed entry path | Appended after the entry path (`AB4311` shape rules). |
+| `targets` | The project's selected targets | Replaces the default selection (`AB4305` shape rules). |
+| `apps` | `src/mcp/<server>/apps/*` routes | Config-side Apps are compiled and registered on the generated server beside the route-declared ones (`AB432x` rules; `AB4334` checks App targets against the declared server targets). The route-declared Apps take part in the collision checks: reusing a route App's name is `AB4325`, reusing its `resourceUri` under another name is `AB4330`. |
+
+Provenance stays `conventional` (the first route module) because the routes
+supply the entry; `inspect` shows the merged `env`, `args`, and `targets`.
+Setting `routes.servers.<server>` to `custom`, `command`, or `remote` turns the
+same block back into an ordinary server declaration and omits the routes.
+
 ### Generated state mounting
 
 The compiler parses `src/state.ts` without executing it and requires one

docs/framework-mode.md

@@ -106,6 +106,19 @@ stay `unknown`. Route-unit and CLI-dispatch tests inject fixture values through
 `renderRoute(id, { context: { providers: { library } } })`; the harness never
 executes provider modules on a test's behalf.
 
+### What reaches the MCP wire
+
+The final Agent Document of a tool route lowers to one `CallToolResult`:
+
+| Route surface | Wire effect |
+| --- | --- |
+| `Agent.Text`, `Agent.Markdown`, `Agent.Context`, `Agent.Json` children | Ordered `content` text blocks (`Agent.Json` as its JSON text). |
+| `Agent.Image`, `Agent.Audio`, `Agent.Resource` | Native `image`, `audio`, and `resource_link` blocks; a host without that capability fails the projection closed unless a text fallback is selected. |
+| `Agent.Result value` | `structuredContent` when the value is a JSON object; a non-object value emits none and is never wrapped. |
+| `Agent.Result metadata` | `CallToolResult._meta`. It must be a JSON object (snapshotted through the same wire boundary as `structuredContent`); anything else fails the projection closed with `McpProjectionError('invalid-result-metadata')`. Listing-level `_meta` still comes from static `config._meta`, so the MCP Apps convention stamps `_meta.ui.resourceUri` on both halves. |
+| `Agent.Error code message` | `isError: true` plus one text block `[<code>] <message>`. The wire has no error-code field, so the code is deliberately kept in the text (the routed CLI prints the same `**[code]** message` form); choose codes that read well to the model. |
+| `resultSchema` | `outputSchema` in `tools/list` **only when the schema describes an object** (`z.object`, `z.record`, a discriminated union of objects). The MCP specification requires every result of a tool that declares `outputSchema` to carry `structuredContent`, so a text-only route declares `resultSchema = z.undefined()` (or any non-object schema), advertises no `outputSchema`, and returns no `structuredContent`. An object schema keeps the SDK's fail-closed output validation on every call. |
+
 Everything else is power-tier reference: custom/remote server modes and
 collision recovery are in [Entry conventions](entry-conventions.md); accepted
 static metadata, generated `.agent-bundle/routes.d.ts`, and diagnostics are in

packages/agent-bundle/fixtures/route-harness/src/mcp/harness/tools/strict-report.tsx

@@ -2,6 +2,7 @@ import { Agent } from '@agent-bundle/runtime';
 import { z } from 'zod';
 
 export const config = {
+  _meta: { ui: { resourceUri: 'ui://route-harness/panel.html' } },
   description: 'Returns a closed-object report that rejects unknown serialized keys.',
   title: 'Strict report',
 };
@@ -16,8 +17,10 @@ export const resultSchema = z.strictObject({
 export default async function StrictReport({ input }: { readonly input: z.infer<typeof inputSchema> }) {
   const reportId = input.reportId ?? 'report-1';
   const value = { reportId, summary: `summary for ${reportId}` };
+  // The MCP Apps convention stamps the App resource on every result as well
+  // as on the listing; `metadata` is the result half (`CallToolResult._meta`).
   return (
-    <Agent.Result value={value}>
+    <Agent.Result metadata={{ ui: { resourceUri: 'ui://route-harness/panel.html' } }} value={value}>
       <Agent.Text>{value.summary}</Agent.Text>
     </Agent.Result>
   );

packages/agent-bundle/package.json

@@ -102,7 +102,6 @@
     "ignore": "7.0.7",
     "jiti": "2.7.0",
     "open": "11.0.2",
-    "typescript-5": "npm:typescript@5.6.1-rc",
     "ws": "8.21.3",
     "yaml": "2.9.0"
   },
@@ -112,6 +111,7 @@
     "@types/ws": "8.18.1",
     "effect-rstest": "https://pkg.pr.new/ScriptedAlchemy/effect-rstest@e5f8d5f",
     "react": "19.2.8",
+    "typescript-5": "npm:typescript@5.9.3",
     "zod": "4.5.4"
   },
   "peerDependencies": {

packages/agent-bundle/rslib.config.ts

@@ -1,9 +1,37 @@
 import { resolve } from 'node:path';
 
-import { defineConfig } from '@rslib/core';
+import { defineConfig, type Rspack, type rspack as RspackInstance } from '@rslib/core';
 import { pluginPublint } from 'rsbuild-plugin-publint';
 import packageManifest from './package.json' with { type: 'json' };
 
+const esmNodeGlobalsShim = [
+  '// agent-bundle ESM shims for the bundled TypeScript parser',
+  "const __filename = process.getBuiltinModule('node:url').fileURLToPath(import.meta.url);",
+  "const __dirname = process.getBuiltinModule('node:path').dirname(__filename);",
+  '',
+].join('\n');
+
+/**
+ * Prepends the `__filename`/`__dirname` shim to every emitted ESM chunk that
+ * still references those CommonJS globals (the bundled TypeScript parser's
+ * eager `getNodeSystem()`); every other chunk is left untouched.
+ */
+const esmNodeGlobalsPlugin = (rspack: typeof RspackInstance): Rspack.RspackPluginInstance => ({
+  apply(compiler: Rspack.Compiler) {
+    compiler.hooks.thisCompilation.tap('agent-bundle:esm-node-globals', (compilation: Rspack.Compilation) => {
+      compilation.hooks.processAssets.tap({
+        name: 'agent-bundle:esm-node-globals',
+        stage: rspack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONS,
+      }, (assets: Rspack.Assets) => {
+        for (const [name, asset] of Object.entries(assets)) {
+          if (!name.endsWith('.js') || !/\b__(?:filename|dirname)\b/u.test(asset.source().toString())) continue;
+          compilation.updateAsset(name, new rspack.sources.ConcatSource(esmNodeGlobalsShim, asset));
+        }
+      });
+    });
+  },
+});
+
 export default defineConfig({
   lib: [
     {
@@ -25,6 +53,23 @@ export default defineConfig({
   // Suggestions stay informational; errors and warnings block publishing.
   plugins: [pluginPublint({ throwOn: 'warning' })],
   root: import.meta.dirname,
+  tools: {
+    // The TypeScript 5 parser is bundled (a devDependency, #381) so consumers
+    // never receive its `tsc` bin beside their own TypeScript.
+    rspack: (config, { rspack }) => {
+      // Its `sys.tryEnableSourceMapsForHost` requires `source-map-support`
+      // inside a try/catch for the tsc CLI only; the static route-config
+      // extractor never reaches it.
+      config.ignoreWarnings = [...(config.ignoreWarnings ?? []), /Can't resolve 'source-map-support'/u];
+      // Its eager `getNodeSystem()` reads the CommonJS `__filename`/`__dirname`
+      // globals, which the ESM output does not define and which Rspack's
+      // `node-module` rewrite leaves untouched inside that module. The chunk
+      // that carries them gets a module-scoped shim derived from its own
+      // `import.meta.url` (`process.getBuiltinModule` is Node >= 22.3).
+      config.node = { ...(typeof config.node === 'object' ? config.node : {}), __dirname: false, __filename: false };
+      config.plugins = [...(config.plugins ?? []), esmNodeGlobalsPlugin(rspack)];
+    },
+  },
   source: {
     tsconfigPath: './tsconfig.build.json',
     define: {

packages/agent-bundle/src/config/validate.ts

@@ -477,10 +477,12 @@ const validateMcpApps = (
   loaded: LoadedConfig,
   seenApps: Map<string, string | undefined>,
   seenUris: Map<string, string>,
+  options: { readonly generated?: boolean } = {},
 ): Diagnostic[] => {
   if (server.apps === undefined) return [];
   const diagnostics: Diagnostic[] = [];
-  const hasLocalEntry = server.entry !== undefined || (
+  // A route-generated server always has a compiled local entry (#380).
+  const hasLocalEntry = options.generated === true || server.entry !== undefined || (
     server.command === undefined && server.url === undefined &&
     conventionalMcpEntrySource(loaded.context.projectRoot, name) !== undefined
   );
@@ -750,6 +752,55 @@ const validateMcpServer = (
   return diagnostics;
 };
 
+/**
+ * The declaration a route-generated server accepts (#380). Config wins and
+ * conventions fill: the `src/mcp/<name>/` route modules supply the entry, so
+ * a `mcp.servers.<name>` block *augments* that server — `env`, `args`,
+ * `targets`, `apps`, and `transport: 'stdio'` — and never redeclares it.
+ * `entry`, `command`, and `url` are a precise error rather than a silently
+ * ignored field: the route graph already compiles this server, so a second
+ * entry claim has no reading the compiler could honor.
+ */
+const validateGeneratedMcpServerDeclaration = (
+  name: string,
+  value: unknown,
+  loaded: LoadedConfig,
+): Diagnostic[] => {
+  const diagnostics: Diagnostic[] = [];
+  if (!nonemptyString(name)) {
+    diagnostics.push(sourceDiagnostic('AB4302', 'MCP server names must be nonempty.', loaded.configPath));
+  }
+  if (!isRecord(value)) {
+    diagnostics.push(sourceDiagnostic('AB4303', `MCP server ${JSON.stringify(name)} must be an object.`, loaded.configPath));
+    return diagnostics;
+  }
+  const server = value as AgentBundleMcpServer;
+  const claims = (['entry', 'command', 'url'] as const).filter((key) => server[key] !== undefined);
+  if (claims.length > 0) {
+    diagnostics.push({
+      code: 'AB4340',
+      message: `MCP server ${JSON.stringify(name)} is compiled from src/mcp/${name}/ route modules, so its declaration cannot set ${claims.join(', ')}; a config declaration for a generated server only augments it.`,
+      recovery: `Remove ${claims.join(', ')} to keep the generated server (env, args, targets, and apps still apply), or set routes.servers.${name} to custom, command, or remote to serve the declared entry instead of the route modules.`,
+      severity: 'error',
+      sourcePath: loaded.configPath,
+    });
+    return diagnostics;
+  }
+  diagnostics.push(...validateStringList(server.targets, 'targets', 'AB4305', loaded));
+  if (server.transport !== undefined && server.transport !== 'stdio') {
+    diagnostics.push(sourceDiagnostic('AB4308', `MCP server ${JSON.stringify(name)} entry must use stdio transport.`, loaded.configPath));
+  }
+  if (server.cwd !== undefined) {
+    diagnostics.push(sourceDiagnostic('AB4309', `MCP server ${JSON.stringify(name)} local entry cannot set cwd.`, loaded.configPath));
+  }
+  if (server.headers !== undefined) {
+    diagnostics.push(sourceDiagnostic('AB4310', `MCP server ${JSON.stringify(name)} stdio server cannot set headers.`, loaded.configPath));
+  }
+  diagnostics.push(...validateStringList(server.args, 'args', 'AB4311', loaded));
+  diagnostics.push(...validateStringRecord(server.env, 'env', 'AB4312', loaded));
+  return diagnostics;
+};
+
 const validatePluginLogo = (
   loaded: LoadedConfig,
   pluginRecord: Record<string, unknown> | undefined,
@@ -840,6 +891,7 @@ const validateRuntime = (loaded: LoadedConfig): Diagnostic[] => {
 
 const validateMcp = (
   loaded: LoadedConfig,
+  discovered: DiscoveredProject,
   registry: NormalizationTargetRegistry,
   payloads: readonly DeclaredPayload[],
 ): Diagnostic[] => {
@@ -851,12 +903,35 @@ const validateMcp = (
   if (!isRecord(mcp.servers)) {
     return [sourceDiagnostic('AB4301', 'MCP configuration must define a servers object.', loaded.configPath)];
   }
+  // The same judgment normalization applies: a server the route graph
+  // compiles in generated mode is declared by its route modules, and a config
+  // block for it augments rather than redeclares (#380).
+  const generatedServers = (discovered.routeGraph?.servers ?? [])
+    .filter((server) => server.mode === 'generated' && server.routes.length > 0);
+  const generated = new Set(generatedServers.map((server) => server.name));
+  // Route-declared Apps (`src/mcp/<server>/apps/*`) take part in the same
+  // name and resourceUri collision checks as configured ones: a config App
+  // that reuses a route App's name is AB4325 (a route module is never an
+  // identical config declaration) and one that reuses its resourceUri under
+  // another name is AB4330, instead of both Apps reaching the generated server.
   const names = new Map<string, string | undefined>();
   const uris = new Map<string, string>();
+  for (const server of generatedServers) {
+    for (const route of server.routes) {
+      if (route.kind !== 'app') continue;
+      const appName = route.id.slice(route.id.lastIndexOf('/') + 1);
+      if (!names.has(appName)) names.set(appName, undefined);
+      const resourceUri = route.config['resourceUri'];
+      if (typeof resourceUri === 'string' && !uris.has(resourceUri)) uris.set(resourceUri, appName);
+    }
+  }
   return Object.entries(mcp.servers).flatMap(([name, server]) => {
-    const diagnostics = validateMcpServer(name, server, loaded, registry, payloads);
+    const isGenerated = generated.has(name);
+    const diagnostics = isGenerated
+      ? validateGeneratedMcpServerDeclaration(name, server, loaded)
+      : validateMcpServer(name, server, loaded, registry, payloads);
     return isRecord(server)
-      ? [...diagnostics, ...validateMcpApps(name, server as AgentBundleMcpServer, loaded, names, uris)]
+      ? [...diagnostics, ...validateMcpApps(name, server as AgentBundleMcpServer, loaded, names, uris, { generated: isGenerated })]
       : diagnostics;
   });
 };
@@ -1855,7 +1930,7 @@ export const validateSource = (
   diagnostics.push(...validateBin(loaded));
   diagnostics.push(...validateHooks(loaded, registry, payloads));
   diagnostics.push(...validateLib(loaded));
-  diagnostics.push(...validateMcp(loaded, registry, payloads));
+  diagnostics.push(...validateMcp(loaded, discovered, registry, payloads));
   diagnostics.push(...validateOutput(loaded));
   diagnostics.push(...validatePayload(loaded, registry, options?.payloadFreshness !== false));
   diagnostics.push(...validateRuntime(loaded));