Skip to content

Commit 5828cc9

Browse files
Fix route-mode defects found while re-porting movie-library (#380, #381, #383)
- #380: a `mcp.servers.<id>` block for a route-generated server augments it (env, args, targets, apps, transport: 'stdio') instead of failing AB4304/AB4322; redeclaring entry/command/url under an explicit generated mode is the new AB4340. - #381: bundle the TypeScript parser (devDependency, pinned 5.9.3) so an npm install of agent-bundle never links a `tsc` bin over the consumer's own TypeScript; packed-tarball proof added. The emitted chunk gets an import.meta.url-derived __filename/__dirname shim for the parser's eager getNodeSystem(). - #383: `Agent.Result metadata` projects to `CallToolResult._meta` (object only; a non-object fails closed with McpProjectionError('invalid-result-metadata')). - Generated tools advertise `outputSchema` only when `resultSchema` describes an object, so text-only routes need no `structuredContent`. - Document the `[<code>] <message>` error text form on the MCP wire.
1 parent 055edf1 commit 5828cc9

20 files changed

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

‎docs/diagnostics.md‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ gate a build, a validation, or a dev rebuild.
1414
| `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). |
1515
| `AB41xx` | Normalized model invariants (unknown targets, duplicate IDs and outputs). |
1616
| `AB42xx` | Hook configuration and native hook sources. |
17-
| `AB43xx` | MCP server and MCP App configuration. |
17+
| `AB43xx` | MCP server and MCP App configuration (`AB4340`: a declaration for a route-generated server redeclares `entry`/`command`/`url`; see below). |
1818
| `AB44xx` | Script configuration. |
1919
| `AB4500` | Registered config extensions (strict finite JSON). |
2020
| `AB46xx` | Assets and the generated-runtime floor. |
@@ -289,6 +289,20 @@ simply not been built yet is a validation **warning** that only
289289
| `AB4749` | error (build) | A payload directory overlaps the artifact `--output` root. |
290290
| `AB4750` | info | A payload is older than the newest project source file and may be stale; rerun the project's own build if so. |
291291

292+
## Config beside a route-generated MCP server (`AB4340`)
293+
294+
A `mcp.servers.<id>` block for a server the route graph compiles in
295+
`generated` mode augments that server (`env`, `args`, `targets`, `apps`,
296+
`transport: 'stdio'`) — see the precedence table in
297+
[Entry conventions](entry-conventions.md#config-beside-a-route-generated-mcp-server).
298+
The local-entry field rules apply to it unchanged (`AB4305`, `AB4308`–`AB4312`,
299+
`AB432x`), and it never triggers `AB4304` or `AB4322`: the route modules are
300+
its entry.
301+
302+
| Code | Severity | Trigger |
303+
| --- | --- | --- |
304+
| `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`. |
305+
292306
## Conventional host components: rules and commands (`AB4900`–`AB4906`, `AB4920`–`AB4926`)
293307

294308
Conventional `src/rules/*.mdc` documents compile to the Rule IR (closed

‎docs/entry-conventions.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,26 @@ entries carry `provenance.kind: 'conventional'` in the normalized model.
8787
Route and package entry conventions match `.ts` and `.tsx` files exactly;
8888
the state convention is specifically `src/state.ts`.
8989

90+
### Config beside a route-generated MCP server
91+
92+
A `mcp.servers.<server>` block whose `<server>` the route graph compiles in
93+
`generated` mode does not redeclare the server — its entry is the route
94+
modules — it **augments** it. This is the precedence table for one generated
95+
server (config wins, conventions fill):
96+
97+
| Field | Source of truth | Config declaration |
98+
| --- | --- | --- |
99+
| 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`. |
100+
| `env` | — | Applied verbatim beneath the injected plugin-root anchor (`AB4312` shape rules). |
101+
| `args` | The content-hashed entry path | Appended after the entry path (`AB4311` shape rules). |
102+
| `targets` | The project's selected targets | Replaces the default selection (`AB4305` shape rules). |
103+
| `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). |
104+
105+
Provenance stays `conventional` (the first route module) because the routes
106+
supply the entry; `inspect` shows the merged `env`, `args`, and `targets`.
107+
Setting `routes.servers.<server>` to `custom`, `command`, or `remote` turns the
108+
same block back into an ordinary server declaration and omits the routes.
109+
90110
### Generated state mounting
91111

92112
The compiler parses `src/state.ts` without executing it and requires one

‎docs/framework-mode.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,19 @@ stay `unknown`. Route-unit and CLI-dispatch tests inject fixture values through
106106
`renderRoute(id, { context: { providers: { library } } })`; the harness never
107107
executes provider modules on a test's behalf.
108108

109+
### What reaches the MCP wire
110+
111+
The final Agent Document of a tool route lowers to one `CallToolResult`:
112+
113+
| Route surface | Wire effect |
114+
| --- | --- |
115+
| `Agent.Text`, `Agent.Markdown`, `Agent.Context`, `Agent.Json` children | Ordered `content` text blocks (`Agent.Json` as its JSON text). |
116+
| `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. |
117+
| `Agent.Result value` | `structuredContent` when the value is a JSON object; a non-object value emits none and is never wrapped. |
118+
| `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. |
119+
| `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. |
120+
| `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. |
121+
109122
Everything else is power-tier reference: custom/remote server modes and
110123
collision recovery are in [Entry conventions](entry-conventions.md); accepted
111124
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‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Agent } from '@agent-bundle/runtime';
22
import { z } from 'zod';
33

44
export const config = {
5+
_meta: { ui: { resourceUri: 'ui://route-harness/panel.html' } },
56
description: 'Returns a closed-object report that rejects unknown serialized keys.',
67
title: 'Strict report',
78
};
@@ -16,8 +17,10 @@ export const resultSchema = z.strictObject({
1617
export default async function StrictReport({ input }: { readonly input: z.infer<typeof inputSchema> }) {
1718
const reportId = input.reportId ?? 'report-1';
1819
const value = { reportId, summary: `summary for ${reportId}` };
20+
// The MCP Apps convention stamps the App resource on every result as well
21+
// as on the listing; `metadata` is the result half (`CallToolResult._meta`).
1922
return (
20-
<Agent.Result value={value}>
23+
<Agent.Result metadata={{ ui: { resourceUri: 'ui://route-harness/panel.html' } }} value={value}>
2124
<Agent.Text>{value.summary}</Agent.Text>
2225
</Agent.Result>
2326
);

‎packages/agent-bundle/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@
102102
"ignore": "7.0.7",
103103
"jiti": "2.7.0",
104104
"open": "11.0.2",
105-
"typescript-5": "npm:typescript@5.6.1-rc",
106105
"ws": "8.21.3",
107106
"yaml": "2.9.0"
108107
},
@@ -112,6 +111,7 @@
112111
"@types/ws": "8.18.1",
113112
"effect-rstest": "https://pkg.pr.new/ScriptedAlchemy/effect-rstest@e5f8d5f",
114113
"react": "19.2.8",
114+
"typescript-5": "npm:typescript@5.9.3",
115115
"zod": "4.5.4"
116116
},
117117
"peerDependencies": {

‎packages/agent-bundle/rslib.config.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,37 @@
11
import { resolve } from 'node:path';
22

3-
import { defineConfig } from '@rslib/core';
3+
import { defineConfig, type Rspack, type rspack as RspackInstance } from '@rslib/core';
44
import { pluginPublint } from 'rsbuild-plugin-publint';
55
import packageManifest from './package.json' with { type: 'json' };
66

7+
const esmNodeGlobalsShim = [
8+
'// agent-bundle ESM shims for the bundled TypeScript parser',
9+
"const __filename = process.getBuiltinModule('node:url').fileURLToPath(import.meta.url);",
10+
"const __dirname = process.getBuiltinModule('node:path').dirname(__filename);",
11+
'',
12+
].join('\n');
13+
14+
/**
15+
* Prepends the `__filename`/`__dirname` shim to every emitted ESM chunk that
16+
* still references those CommonJS globals (the bundled TypeScript parser's
17+
* eager `getNodeSystem()`); every other chunk is left untouched.
18+
*/
19+
const esmNodeGlobalsPlugin = (rspack: typeof RspackInstance): Rspack.RspackPluginInstance => ({
20+
apply(compiler: Rspack.Compiler) {
21+
compiler.hooks.thisCompilation.tap('agent-bundle:esm-node-globals', (compilation: Rspack.Compilation) => {
22+
compilation.hooks.processAssets.tap({
23+
name: 'agent-bundle:esm-node-globals',
24+
stage: rspack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONS,
25+
}, (assets: Rspack.Assets) => {
26+
for (const [name, asset] of Object.entries(assets)) {
27+
if (!name.endsWith('.js') || !/\b__(?:filename|dirname)\b/u.test(asset.source().toString())) continue;
28+
compilation.updateAsset(name, new rspack.sources.ConcatSource(esmNodeGlobalsShim, asset));
29+
}
30+
});
31+
});
32+
},
33+
});
34+
735
export default defineConfig({
836
lib: [
937
{
@@ -25,6 +53,23 @@ export default defineConfig({
2553
// Suggestions stay informational; errors and warnings block publishing.
2654
plugins: [pluginPublint({ throwOn: 'warning' })],
2755
root: import.meta.dirname,
56+
tools: {
57+
// The TypeScript 5 parser is bundled (a devDependency, #381) so consumers
58+
// never receive its `tsc` bin beside their own TypeScript.
59+
rspack: (config, { rspack }) => {
60+
// Its `sys.tryEnableSourceMapsForHost` requires `source-map-support`
61+
// inside a try/catch for the tsc CLI only; the static route-config
62+
// extractor never reaches it.
63+
config.ignoreWarnings = [...(config.ignoreWarnings ?? []), /Can't resolve 'source-map-support'/u];
64+
// Its eager `getNodeSystem()` reads the CommonJS `__filename`/`__dirname`
65+
// globals, which the ESM output does not define and which Rspack's
66+
// `node-module` rewrite leaves untouched inside that module. The chunk
67+
// that carries them gets a module-scoped shim derived from its own
68+
// `import.meta.url` (`process.getBuiltinModule` is Node >= 22.3).
69+
config.node = { ...(typeof config.node === 'object' ? config.node : {}), __dirname: false, __filename: false };
70+
config.plugins = [...(config.plugins ?? []), esmNodeGlobalsPlugin(rspack)];
71+
},
72+
},
2873
source: {
2974
tsconfigPath: './tsconfig.build.json',
3075
define: {

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

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,10 +477,12 @@ const validateMcpApps = (
477477
loaded: LoadedConfig,
478478
seenApps: Map<string, string | undefined>,
479479
seenUris: Map<string, string>,
480+
options: { readonly generated?: boolean } = {},
480481
): Diagnostic[] => {
481482
if (server.apps === undefined) return [];
482483
const diagnostics: Diagnostic[] = [];
483-
const hasLocalEntry = server.entry !== undefined || (
484+
// A route-generated server always has a compiled local entry (#380).
485+
const hasLocalEntry = options.generated === true || server.entry !== undefined || (
484486
server.command === undefined && server.url === undefined &&
485487
conventionalMcpEntrySource(loaded.context.projectRoot, name) !== undefined
486488
);
@@ -750,6 +752,55 @@ const validateMcpServer = (
750752
return diagnostics;
751753
};
752754

755+
/**
756+
* The declaration a route-generated server accepts (#380). Config wins and
757+
* conventions fill: the `src/mcp/<name>/` route modules supply the entry, so
758+
* a `mcp.servers.<name>` block *augments* that server — `env`, `args`,
759+
* `targets`, `apps`, and `transport: 'stdio'` — and never redeclares it.
760+
* `entry`, `command`, and `url` are a precise error rather than a silently
761+
* ignored field: the route graph already compiles this server, so a second
762+
* entry claim has no reading the compiler could honor.
763+
*/
764+
const validateGeneratedMcpServerDeclaration = (
765+
name: string,
766+
value: unknown,
767+
loaded: LoadedConfig,
768+
): Diagnostic[] => {
769+
const diagnostics: Diagnostic[] = [];
770+
if (!nonemptyString(name)) {
771+
diagnostics.push(sourceDiagnostic('AB4302', 'MCP server names must be nonempty.', loaded.configPath));
772+
}
773+
if (!isRecord(value)) {
774+
diagnostics.push(sourceDiagnostic('AB4303', `MCP server ${JSON.stringify(name)} must be an object.`, loaded.configPath));
775+
return diagnostics;
776+
}
777+
const server = value as AgentBundleMcpServer;
778+
const claims = (['entry', 'command', 'url'] as const).filter((key) => server[key] !== undefined);
779+
if (claims.length > 0) {
780+
diagnostics.push({
781+
code: 'AB4340',
782+
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.`,
783+
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.`,
784+
severity: 'error',
785+
sourcePath: loaded.configPath,
786+
});
787+
return diagnostics;
788+
}
789+
diagnostics.push(...validateStringList(server.targets, 'targets', 'AB4305', loaded));
790+
if (server.transport !== undefined && server.transport !== 'stdio') {
791+
diagnostics.push(sourceDiagnostic('AB4308', `MCP server ${JSON.stringify(name)} entry must use stdio transport.`, loaded.configPath));
792+
}
793+
if (server.cwd !== undefined) {
794+
diagnostics.push(sourceDiagnostic('AB4309', `MCP server ${JSON.stringify(name)} local entry cannot set cwd.`, loaded.configPath));
795+
}
796+
if (server.headers !== undefined) {
797+
diagnostics.push(sourceDiagnostic('AB4310', `MCP server ${JSON.stringify(name)} stdio server cannot set headers.`, loaded.configPath));
798+
}
799+
diagnostics.push(...validateStringList(server.args, 'args', 'AB4311', loaded));
800+
diagnostics.push(...validateStringRecord(server.env, 'env', 'AB4312', loaded));
801+
return diagnostics;
802+
};
803+
753804
const validatePluginLogo = (
754805
loaded: LoadedConfig,
755806
pluginRecord: Record<string, unknown> | undefined,
@@ -840,6 +891,7 @@ const validateRuntime = (loaded: LoadedConfig): Diagnostic[] => {
840891

841892
const validateMcp = (
842893
loaded: LoadedConfig,
894+
discovered: DiscoveredProject,
843895
registry: NormalizationTargetRegistry,
844896
payloads: readonly DeclaredPayload[],
845897
): Diagnostic[] => {
@@ -851,12 +903,21 @@ const validateMcp = (
851903
if (!isRecord(mcp.servers)) {
852904
return [sourceDiagnostic('AB4301', 'MCP configuration must define a servers object.', loaded.configPath)];
853905
}
906+
// The same judgment normalization applies: a server the route graph
907+
// compiles in generated mode is declared by its route modules, and a config
908+
// block for it augments rather than redeclares (#380).
909+
const generated = new Set((discovered.routeGraph?.servers ?? [])
910+
.filter((server) => server.mode === 'generated' && server.routes.length > 0)
911+
.map((server) => server.name));
854912
const names = new Map<string, string | undefined>();
855913
const uris = new Map<string, string>();
856914
return Object.entries(mcp.servers).flatMap(([name, server]) => {
857-
const diagnostics = validateMcpServer(name, server, loaded, registry, payloads);
915+
const isGenerated = generated.has(name);
916+
const diagnostics = isGenerated
917+
? validateGeneratedMcpServerDeclaration(name, server, loaded)
918+
: validateMcpServer(name, server, loaded, registry, payloads);
858919
return isRecord(server)
859-
? [...diagnostics, ...validateMcpApps(name, server as AgentBundleMcpServer, loaded, names, uris)]
920+
? [...diagnostics, ...validateMcpApps(name, server as AgentBundleMcpServer, loaded, names, uris, { generated: isGenerated })]
860921
: diagnostics;
861922
});
862923
};
@@ -1855,7 +1916,7 @@ export const validateSource = (
18551916
diagnostics.push(...validateBin(loaded));
18561917
diagnostics.push(...validateHooks(loaded, registry, payloads));
18571918
diagnostics.push(...validateLib(loaded));
1858-
diagnostics.push(...validateMcp(loaded, registry, payloads));
1919+
diagnostics.push(...validateMcp(loaded, discovered, registry, payloads));
18591920
diagnostics.push(...validateOutput(loaded));
18601921
diagnostics.push(...validatePayload(loaded, registry, options?.payloadFreshness !== false));
18611922
diagnostics.push(...validateRuntime(loaded));

0 commit comments

Comments
 (0)