feat: expose ilchul cli with runctl internals#208
Conversation
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
![kapi-agent[bot]](https://avatars.githubusercontent.com/in/3661577?s=60&v=4){kind=small}
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
This PR’s original goal is to expose ilchul as the canonical public runtime CLI while preserving kapi compatibility and keeping internal implementation names product-neutral as runctl. The semantic scope is bounded: CLI bin wiring, runtime entrypoint/formatter renames, naming-policy/docs alignment, and regression tests for the public command surface. There was no prior kapi-agent review on this head, so no review/fix ping-pong was required.
What changed
Key paths inspected:
package.jsonbin/runctl.mjsbin/kapi.mjsscripts/setup-runctl-cli.mjssrc/cli/runctl-cli.tssrc/presentation/commands.ts- README and naming/audit docs
- CLI/bin/formatter tests
The package now exposes ilchul through the runtime bin shim, keeps kapi as a compatibility alias, and updates tests/docs from the prior public runctl examples to ilchul.
Why this is correct
The public CLI rename is implemented at the bin/package boundary without changing the underlying workflow/runtime contract. kapi explicitly sets the command-name environment to preserve compatibility help/error wording, while the canonical bin defaults to ilchul. The internal module rename to runctl-cli/runctl-formatters matches the stated policy of avoiding product-branded reusable implementation names.
Evidence
Verifier gate: PASS. npm ci && npm run verify exited 0.
Size gate: PASS with semantic review required. Although the PR is 288 changed lines, the effective source change is small and mostly rename/docs/test coverage; harness metrics reported 35 semantic/source lines and 201 test lines.
Revision-explanation status: not required; found.
Ilchul review harness gate: PASS, conclusion neutral, verdict COMMENT, blocking findings 0.
Remaining risks and approval rationale
Remaining risk is low and mostly operational: users with old muscle memory may still expect runctl, but the PR intentionally documents ilchul as canonical and keeps kapi as the tested compatibility alias. I found no blocking correctness, security, workflow-contract, or regression issue, and the test coverage directly exercises the renamed CLI/bin/setup paths.
Blocking issues
- none.
Warnings / risks
bin/runctl.mjs: the canonical shim still honors an existingILCHUL_RUNTIME_CLI_NAMEenvironment variable before defaulting toilchul. This is useful for aliasing/testing, but it means help/error text can be externally relabeled. I do not consider this blocking because it does not alter command behavior or execution target.
Suggestions
- Consider adding a short migration note if any users previously installed or scripted a
runctlcommand, since this PR removesrunctlfrom the packagebinsurface and makesilchulcanonical.
Looks good
package.jsoncleanly exposesilchuland preserveskapi.bin/kapi.mjscorrectly routes to the shared runtime entrypoint while forcing compatibility command naming.- README and
docs/ilchul-naming-policy.mdare aligned with the implementation. - Tests cover package-local bin usage, setup helper behavior, installed-prefix behavior, and formatter import renames.
Verification notes
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 288 changed lines required semantic review; scope is bounded and mostly tests/docs/renames.
- Revision-explanation status: not required; found.
- Ilchul harness gate: PASS — neutral/comment with 0 blocking findings.
- PR evidence includes targeted CLI/bin/check/unused/quality verification in the PR body, and the full verifier run confirms the broader suite passes.
Engine: pi
|
@kapi-agent review |
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
- Revision explanation gate failed. This PR was synchronized after a prior kapi-agent approval, but there is no current author explanation for what changed and why on the new head
7bb37fabed4865d578c463919cfaca56de93333a. Please add a review response summarizing the delta since the approved revision and why it closes/does not affect prior feedback before re-review. README.mdhas a malformed fenced code block in the Local CLI Runtime section:The closing fence is attached to the command, so the following prose can render incorrectly. Since this PR changes CLI documentation and repo instructions require README alignment with implementation, please fix the fence.npm run setup:cli ilchul --help```
Warnings / risks
- The PR summary says
kapiis kept as a compatibility alias, butpackage.jsonnow exposes onlyilchulandkapi-review, andtest/cli-bin.test.tsexplicitly asserts nokapiruntime alias. If alias removal is intentional, update the PR/body wording and revision explanation so reviewers can distinguish an intentional compatibility break from an accidental regression.
Suggestions
- Consider renaming the test title
setup helper installs kapi into an isolated prefixintest/cli-bin.test.ts; the assertion now installs/runsilchul, so the title is stale and may confuse future reviewers.
Looks good
- The runtime bin wrapper points to
src/cli/runctl-cli.tsand defaults the command name toilchul, which matches the intended public CLI naming. - Tests cover package-local execution, isolated-prefix setup, production install without dev dependencies, and help output naming.
- The implementation scope is mostly naming/bin/docs/test alignment rather than broad workflow behavior changes.
Verification notes
- Verifier gate status: PASS —
npm ci && npm run verifyexited 0. - Size gate status: PASS with semantic review required — 435 changed lines, mostly tests/docs with 74 source lines and no sensitive-path lines reported.
- Revision-explanation status: FAIL — required now and not found for the current synchronized revision.
- Ilchul review harness gate: PASS — neutral/comment, no blocking findings.
- Local inspection focused on
README.md,package.json,bin/runctl.mjs,scripts/setup-runctl-cli.mjs, andtest/cli-bin.test.ts.
Engine: pi
There was a problem hiding this comment.
kapi-agent review
Verdict: REQUEST_CHANGES
Blocking issues
-
Revision explanation gate failed. This PR was synchronized after a prior kapi-agent approval, but there is no current author explanation for what changed from the previously approved head and why. Please add a review response before requesting re-review.
-
The current revision appears to change the CLI compatibility contract without explanation. The prior approved review and current PR summary say
kapiremains a compatibility alias, but current head removesbin/kapi.mjsandpackage.jsonno longer exposes akapibin. Tests now assert there is nokapiruntime alias. If this is intentional, the PR body and revision response need to say so and explain the migration/compatibility impact. If not intentional, restore the alias. -
README.mdhas a malformed fenced block in the setup example:- `ilchul --help````
This should close the code fence on its own line. Since README alignment is required for behavior/CLI workflow changes, please fix this.
- `ilchul --help````
Warnings / risks
- The semantic scope is still bounded, but it touches public CLI behavior and docs. Removing a bin alias is user-visible and should be treated as a compatibility-breaking decision unless the linked issue explicitly approves it.
Suggestions
- Consider adding a direct test for the intended
kapibehavior either way:- if retained:
npm exec -- kapi --helpworks; - if removed: documented migration/error expectations are explicit.
- if retained:
Looks good
- The implementation consistently routes the public
ilchulbin throughbin/runctl.mjsandsrc/cli/runctl-cli.ts. - The formatter/module rename is mostly mechanical and covered by renamed tests.
npm ci && npm run verifypassed.
Verification notes
- Verification gate status: FAIL, because the revision explanation gate failed.
- Size gate status: PASS; 435 changed lines crosses the semantic review threshold, but the change is mostly tests/docs/rename work with about 74 source lines.
- Revision-explanation status: FAIL; required now, not found for current head.
- Verifier evidence:
npm ci && npm run verifyexited 0. - Ilchul review harness gate: PASS / neutral COMMENT, with 0 blocking findings.
Engine: pi
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
The original goal was to expose ilchul as the canonical public runtime CLI while keeping internal runtime implementation names product-neutral as runctl. The semantic scope is bounded to CLI bin wiring, entrypoint/formatter/setup renames, documentation/naming-policy alignment, and regression tests for the runtime command surface.
There was review/fix ping-pong after the prior approval: a later revision temporarily removed or failed to explain the kapi compatibility alias and had a malformed README fence. kapi-agent requested changes. The current revision includes an author explanation and addresses both issues.
What changed
The current revision:
- restores
bin/kapi.mjsas a thin compatibility alias; - exposes both
ilchulandkapiinpackage.json/package-lock.json; - routes both bins through
src/cli/runctl-cli.ts, withRUNCTL_COMMAND_NAMEcontrolling displayed command name; - fixes the README fenced code block;
- updates
test/cli-bin.test.tsto assert both canonicalilchulbehavior and compatibilitykapibehavior; - keeps runctl naming for internal runtime modules and formatter paths.
Why this is correct
The compatibility alias is implemented at the bin boundary without changing workflow IDs, storage roots, registry semantics, artifact contracts, or worker behavior. kapi now explicitly sets RUNCTL_COMMAND_NAME=kapi, so help/usage output remains coherent for compatibility users, while the canonical ilchul bin defaults to RUNCTL_COMMAND_NAME=ilchul.
The prior README formatting issue is fixed, and the tests now cover the user-visible contract that was previously ambiguous.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 437 changed lines triggered semantic review, but the effective source semantic scope is small and mostly rename/docs/test coverage.
- Revision explanation status: PASS — author provided the required current-head explanation.
- Ilchul review harness gate: PASS — neutral/comment, no blocking findings.
- Inspected files included
package.json,bin/kapi.mjs,bin/runctl.mjs,src/cli/runctl-cli.ts,scripts/setup-runctl-cli.mjs,test/cli-bin.test.ts, and README changes.
Remaining risks and approval rationale
Remaining risk is low: this is a public CLI surface change, but the canonical command and compatibility alias are both covered by install/help tests, and verification is green. Approval is justified because the previous blocking issues are resolved and no new correctness, safety, or workflow-contract blocker remains.
Blocking issues
- none.
Warnings / risks
README.mdemphasizesilchuland no longer explicitly showskapiusage in the Local CLI Runtime section. That is acceptable for canonical-command documentation, but if the migration window is important for users, consider adding one short compatibility sentence.
Suggestions
- Consider adding a tiny README note such as: “The same runtime subcommands are also available through
kapias a compatibility alias during migration.” This would make the public contract more discoverable without makingkapithe recommended command.
Looks good
bin/kapi.mjsis now a minimal compatibility wrapper and correctly sets the displayed command name.package.jsonexposes bothilchulandkapi.- Tests verify package-local execution, isolated-prefix install behavior, and production install behavior.
- The README fence issue from the previous review is fixed.
- Internal implementation naming stays product-neutral through
runctlpaths.
Verification notes
- Verifier gate: PASS.
- Size gate: PASS; reviewed semantically, not rejected on raw line count.
- Revision-explanation status: PASS.
- PR evidence includes
npm test,npm run check,npm run check:unused,npm run quality:budgets, and the gate-runnpm ci && npm run verify.
Engine: pi
1 participant
Summary
ilchulwhile keepingkapias the compatibility aliasrunctl/runtime-oriented namesilchul+ internalrunctlCloses #205
Verification
npm test -- test/cli-args.test.ts test/cli-bin.test.ts test/runctl-formatters.test.tsnpm run checknpm run check:unusednpm run quality:budgets