fix(routes,mcp,package): movie-library port findings — config augments generated servers, result _meta, conditional outputSchema, no tsc shadowing - #405
Conversation
🦋 Changeset detectedLatest commit: 4cb9b5d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…, #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.
cd85b48 to
5828cc9
Compare
commit: |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5828cc96bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…r's route-declared Apps A config App beside a route-generated server now shares the AB4325 name and AB4330 resourceUri collision checks with the src/mcp/<server>/apps/* routes, so the same resource URI can no longer reach the generated server twice.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…fter #405 #405 projects Agent.Result metadata to the result _meta; with the route-harness layouts in place every harness tool now carries the root and server layout keys beside its own, so the in-memory pin asserts the merged object rather than the route-only one. The layout-free 'no metadata => no _meta' case stays pinned by mcp-projector.test.ts and generated-route-server.test.ts.
… Pages delivery (#384) ## Summary Lands the Rspress documentation site (`website/`, private workspace `@agent-bundle/docs`) designed in `docs/superpowers/specs/2026-09-02-agent-bundle-rspress-docsite-design.md` and planned in `docs/superpowers/plans/2026-09-02-agent-bundle-rspress-docsite.md` (both versioned here). Continues the prior `docs/rspress-website` branch (Tasks 1–5, rebased onto main) and completes Tasks 6–7 plus generated reference pages. - **Site**: Rspress 2.0.21 with `@rspress/plugin-typedoc` (11 public export entry modules, mirrored into `zh/`), `plugin-twoslash` (type hovers on guide samples), `plugin-llms` (`llms.txt`/`llms-full.txt` per locale + per-route Markdown), `plugin-sitemap`; dead-link, dead-anchor, dead-image, and language-parity checks on. Complete English + Simplified Chinese parity for Guide (start/authoring/development/distribution), Reference, Examples, Contributing, and home. - **Generated reference pages** (new `website/plugins/generated-reference.ts`, run in the `config` hook, gitignored output; the Reference overview links every generated page, so Rspress's dead-link check fails the build if generation is skipped): **Host capability matrix**, **Event and hook matrix**, **Notice delivery matrix** rendered from `packages/agent-bundle/src/adapters/capabilities/*.json`, and **Diagnostics reference** copied from `docs/diagnostics.md`. Source of truth stays in the repo; the docs cannot drift. - **Content**: documented event routes (`src/events/**`) and the full hook wire (stdin → IPC → warm runtime → projection → stdout, fail-closed semantics), verified against `events/ipc.ts`, `adapters/hook-contract.ts`, `events/projection.ts`; config reference now links every field to its TypeDoc-generated type (the config is a TS contract, not a Zod schema); fixed the hook entry shape (`handler`, not `entry`); family-parity homepage (Introduction / Quick start actions, nine linked feature cards) whose MDX body renders below the feature grid through a `Layout` override on the `afterFeatures` slot (`website/theme/index.tsx`): a write-vs-emit comparison with per-host output trees, a Describe → Develop → Prove → Ship walkthrough, a host table, and start-here links, authored per locale; Examples/Contributing nav entries. - **CI / delivery**: `.github/workflows/docs.yml` runs `pnpm docs:site:build` (typecheck + build with dead-link/anchor/image and language-parity checks; the standalone `verify-build.mjs` was dropped in favor of the built-in checks) on every PR and push to `main` (no path filter), and deploys `website/doc_build` to GitHub Pages from `main` only (Pages source set to GitHub Actions via API; target `https://scriptedalchemy.github.io/agent-bundle/`). `scripts/classify-docs-only.mjs` now treats `website/**` as docs-only (unit-tested), so website-only PRs skip the heavy `ci.yml` jobs. Root scripts: `docs:site:build`, `docs:site:dev`, `docs:site:preview`. READMEs link to the hosted docs. - **Accuracy fixes from review and browser acceptance**: host path-token table (Cursor and Codex do not accept `${PLUGIN_DATA}`), `pnpm example:*` prints the Workbench URL (`--open` launches the browser), zh Node-resolution order, zh `技能` → `Skill`, `AB4707`–`AB4709` descriptions, `AGENT_BUNDLE_WORKBENCH_API_PROXY` row, Development/Distribution overview titles no longer repeat their sidebar group, TypeDoc member titles emit unescaped underscores (API sidebar and prev/next labels showed `FOO\_BAR`), and over-wide code samples wrapped. - **Fix**: Twoslash could not resolve `zod` in package-entry samples on current main; the website package now declares it (`4.5.4`, matching the package). No package source is changed. The `packages/agent-bundle/README.md` link is a shipped-tarball change, so it carries one `agent-bundle` patch changeset (`.changeset/docs-site-readme-link.md`) per the policy `main` now enforces. ## Since consolidation - **Final-review corrections** (all verified against source, both locales): `reference/api.mdx` lists all eleven entry points and puts `build`/`validate`/`inspect`/`prepack` under `agent-bundle/api`; the hook `reason` rule follows `nativeHookWrapperSource` (denied `agentStop` requires a reason; `agentStart` cannot deny); `cursor` emits `.cursor-plugin/marketplace.json` when `marketplace: true` and `plugin` emits all three manifests; wrapper names carry a digest of their declaration, not a content hash; the diagnostics index gains `AB48xx`–`AB49xx`; `languageParity` now covers the homepages; `llms-full.txt` is emitted with `mdxToMd` (no raw theme imports/JSX); `docs.yml` lints website-only PRs and deploys on `workflow_dispatch` from `main`; the TypeScript 6 pin the docsite needs for `typedoc@0.28` is recorded in `docs/effect-conventions.md`. - **Review-thread fixes**: quick start installs only the hosts its configs build; the examples index counts all six examples and links the two advanced references; `dev.contracts` (`fixtures`, `server`, `AB7210`/`AB7211` gating) is in the configuration reference; the security page no longer denies the native eval harnesses; `agent-bundle.manifest.json` sits at the output root in the structure tree; the generated hosts page derives Claude's MCP path-token fields from its plugin path-substitution table; a duplicate changeset was removed. - **Artifact root**: `agent-bundle build` writes host artifacts to `artifact/` by default (it always runs the package build, which owns `dist/`), so the site now says `artifact/` everywhere it means host artifacts, explains the `artifact/`/`dist/` split in the configuration, CLI, and project-structure pages, and the CLI's `--output` help text no longer claims `default dist`. The Cursor and portable capability tables record the marketplace path and path tokens their adapters emit, so the generated host matrix stops rendering empty cells; the changeset covers both. - **`AGENTS.md`**: new "Documentation site" section — which user-facing changes must update `website/docs` in both locales, which pages are generated and must not be hand-edited, and `pnpm docs:site:build` as the gate. - **Merged `main`** (through #405, #395): the lockfile was regenerated on top of `main`'s; the generated diagnostics page now rewrites repo-relative `docs/*.md` links to GitHub URLs, since `docs/diagnostics.md` started linking to `entry-conventions.md`. ## Evidence - `pnpm docs:site:build` (website typecheck → `rspress build`): green (1424 sitemap pages on the merged head), language parity checked, no warnings. - `pnpm lint`: 0 errors / 0 warnings (1032 files). - `pnpm exec rstest --config rstest.unit.config.ts packages/agent-bundle/tests/classify-docs-only.test.ts`: 4/4 pass (new website-only and mixed website+source cases). - Root `pnpm build`, `pnpm typecheck`, `pnpm test:projection`: pass. `test:unit` (2665/2671) and `test:route-unit` (34/35) each had one timeout while running concurrently with the TypeDoc build; both tests pass in isolation. `test:integration:run` fails only `host-install-proof :: installs through Codex` against the locally installed real Codex CLI (`logo` field in the emitted manifest) — environment-specific and unrelated to this branch, which changes no package source. - Browser acceptance at 1440×900: crawled all 66 authored routes in both locales plus generated reference and API samples — every page renders with title, sidebar and outline; no raw MDX/JSX, `undefined` leaks, broken images, or non-`pre` overflow; Twoslash hover shows the real `defineConfig` signature; search returns prose and code-block hits; locale switch preserves the route both ways; dark mode clean. Earlier pass (Playwright, `rspress preview`): `/`, `/zh/`, `/reference/hosts`, `/reference/events`, `/guide/authoring/hooks`, `/api/` all render with no loading state and no broken images; nav shows Guide / Reference / Examples / Contributing / Type API + locale switch; LLM "Copy Markdown / Open in chat" actions present; per-route `.md` resolves under `/agent-bundle/`; `llms.txt` links use `https://scriptedalchemy.github.io/agent-bundle/`; `sitemap.xml` uses the same origin + base; Twoslash hover markup present in built HTML. ## Test plan - [ ] `Docs` workflow green on this PR (`pnpm docs:site:build`) - [ ] `CI` workflow: website+ci+scripts changes are not docs-only, so the full matrix runs and must be green - [ ] After merge: `Docs` deploy job publishes to https://scriptedalchemy.github.io/agent-bundle/ (Pages source already set to GitHub Actions)
Summary
Framework defects and wire deviations surfaced while re-porting the external
movie-libraryplugin onto route mode on currentmain.mcp.servers.<id>block whose<id>the route graph compiles ingeneratedmode now augments that server (env,args,targets,apps,transport: 'stdio') instead of failingAB4304/AB4322. Redeclaringentry/command/urlunder an explicitroutes.servers.<id>: 'generated'is the new preciseAB4340(message + recovery); without an explicit mode the same collision staysAB4800.validateMcpnow sees the discovered route graph and applies the same judgment normalization already did. Precedence table indocs/entry-conventions.md; code indocs/diagnostics.md.typescript-5alias dependency shadows the consumer'stscbin under npm (tsc 5.6.1-rc rejects ES2024 tsconfig) #381 —tscbin shadowing. Thetypescript-5parser alias moves fromdependenciestodevDependencies(pinnedtypescript@5.9.3) and is bundled into the package, sonpm install agent-bundlenever links atscbin over the consumer's own TypeScript. The bundled parser's eagergetNodeSystem()reads CommonJS__filename/__dirname, so the rslib build prepends animport.meta.url-derived shim to the chunk that references them. New packed-tarball proofpacked-consumer-typescript.test.tsinstalls the tarball beside the consumer's TypeScript and asserts.bin/tscis the consumer's, notypescript-5is hoisted, and the packed CLI still statically parses route config._meta.Agent.Result metadatanow projects toCallToolResult._meta(JSON-snapshotted likestructuredContent). MCP_metais an object, so a non-object fails the projection closed withMcpProjectionError('invalid-result-metadata'). Listing_meta(staticconfig._meta) is unchanged, so the MCP Apps convention stamps_meta.ui.resourceUrion both halves. Themcp-in-memoryharness result exposes_meta.outputSchema. Generated tools advertiseoutputSchemaonly whenresultSchemadescribes an object (object keywords, or aoneOf/anyOf/allOfof objects — the same judgment the SDK applies before stampingtype: "object"). Text-only routes (z.undefined(),z.string(), arrays) advertise none and return nostructuredContent, as the spec requires; object schemas keep the SDK's fail-closed output validation.[tool_error]prefix. Intentional:Agent.Error code messagelowers toisError: trueplus one text block[<code>] <message>because the wire has no error-code field, and the routed CLI prints the same**[code]** messageform. It was undocumented; it is now documented indocs/framework-mode.md("What reaches the MCP wire") rather than removed.Closes #380, closes #381, closes #383.
Tests
route-graph.test.ts: augmentation accepted end to end throughvalidate+inspect(env/args/targets/config App on the generated server);AB4340with message/recovery and noAB4304/AB4800; local-entry field rules (AB4308–AB4310) still apply to an augmenting block.generated-route-server.test.ts: builds an augmented generated server, checksmcp.jsonenv/args, then over real stdio assertstools/list_meta+ conditionaloutputSchema,tools/call_meta+structuredContentfor the object route and a bare text result for the text-only route, and reads the config-side App resource.mcp-projector.test.ts/mcp-in-memory.test.ts:_metaprojection, frozen snapshot, fail-closed on scalar and array metadata.mcp-server-runtime.test.ts(new):advertisedOutputSchemaon object/record/discriminated-union vs. undefined/void/string/number/array/mixed-union vs. opaque schema.packed-consumer-typescript.test.ts(new, packed pool): the agent-bundle'stypescript-5alias dependency shadows the consumer'stscbin under npm (tsc 5.6.1-rc rejects ES2024 tsconfig) #381 proof above.Verification
pnpm typecheck,pnpm lint,pnpm test:unit,pnpm test:route-unit,pnpm test:projection,pnpm build && pnpm test:integration:run(936 pass; three load-induced timeouts re-run green in isolation),pnpm test:packed(the new tsc proof passes;public-api-packednpm-install timeouts on a load-140 host —npm installof the tarball alone measured 27 s there — are load, CI is the arbiter),pnpm test:mcp-conformance.Not changed
movie-libraryitself (out of scope; a note will follow on its PR build(deps): bump react-router from 7.18.2 to 8.3.0 #5).outputSchemaopt-in beyond schema shape; a route that wants structured output declares an objectresultSchema.