feat(cli): complete the gunshi migration — install/ci ports and legacy parser removal - #454
Conversation
|
Warning Review limit reached
Next review available in: 18 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe CLI migrates install, CI, docs, and explain commands to Gunshi. It replaces the shared parser with ChangesGunshi CLI migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as Top-level CLI
participant Gunshi as Gunshi command handler
participant Workflow as Install or CI workflow
participant IO as CLI I/O
CLI->>Gunshi: dispatch selected subcommand
Gunshi->>Workflow: validate arguments and execute command
Workflow->>IO: write output or workflow files
IO-->>Gunshi: return diagnostics and status
Gunshi-->>CLI: return exit code
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/cli/test/gunshi-ci.test.ts (1)
37-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName each matrix test by its asserted behavior.
Line 60 uses titles such as
"install"and"--help". These titles identify argv input but not the expected result. Rename each cell to state the observable behavior, such as"install creates the workflow"or"--help prints help and exits 0".As per coding guidelines, name tests after the behavior they verify, not the reasoning behind the implementation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/test/gunshi-ci.test.ts` around lines 37 - 61, Rename every title in the `cells` matrix to describe the observable behavior asserted by its snapshot, including success, help, error, and dispatch cases, rather than merely echoing the argv. Preserve each `args` value and ensure names state outcomes such as workflow creation, help with exit 0, or literal-token dispatch behavior.Source: Coding guidelines
packages/cli/test/gunshi-install.test.ts (1)
60-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName each generated test by its expected behavior.
The current names mostly identify argv shapes. Rename the
cells[].namevalues to state the observable result, such as help output, a validation error, or ignored post-terminator input.As per coding guidelines, tests must be named after the behavior they verify.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/test/gunshi-install.test.ts` around lines 60 - 63, Rename the name values in the cells test cases to describe the observable behavior each case verifies, such as displaying help, returning a validation error, or ignoring input after the terminator. Keep the generated test loop and snapshot assertions unchanged.Source: Coding guidelines
packages/cli/src/resolve-args.ts (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the behavior-restating comment.
The comment duplicates the
toListimplementation. Remove it, or replace it with a constraint that the code cannot express.As per coding guidelines, comments must explain constraints, rejected alternatives, or non-local dependencies, and must not merely restate code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/resolve-args.ts` at line 15, Remove the behavior-restating comment immediately above toList; leave the implementation unchanged unless replacing it with a necessary non-local constraint or design rationale that cannot be expressed in code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/cli/src/resolve-args.ts`:
- Line 15: Remove the behavior-restating comment immediately above toList; leave
the implementation unchanged unless replacing it with a necessary non-local
constraint or design rationale that cannot be expressed in code.
In `@packages/cli/test/gunshi-ci.test.ts`:
- Around line 37-61: Rename every title in the `cells` matrix to describe the
observable behavior asserted by its snapshot, including success, help, error,
and dispatch cases, rather than merely echoing the argv. Preserve each `args`
value and ensure names state outcomes such as workflow creation, help with exit
0, or literal-token dispatch behavior.
In `@packages/cli/test/gunshi-install.test.ts`:
- Around line 60-63: Rename the name values in the cells test cases to describe
the observable behavior each case verifies, such as displaying help, returning a
validation error, or ignoring input after the terminator. Keep the generated
test loop and snapshot assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 01b085db-3e50-4d53-a8a0-329ac588f5c7
⛔ Files ignored due to path filters (5)
packages/cli/test/__snapshots__/gunshi-ci.test.ts.snapis excluded by!**/*.snappackages/cli/test/__snapshots__/gunshi-docs-parity.test.ts.snapis excluded by!**/*.snappackages/cli/test/__snapshots__/gunshi-explain-parity.test.ts.snapis excluded by!**/*.snappackages/cli/test/__snapshots__/gunshi-install.test.ts.snapis excluded by!**/*.snappackages/cli/test/__snapshots__/help-golden.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (26)
.changeset/gunshi-migration-complete.mddocs/superpowers/specs/2026-08-10-gunshi-cli-migration-design.mdpackages/cli/src/ci/cli.tspackages/cli/src/ci/workflow.tspackages/cli/src/cli-args.tspackages/cli/src/cli.tspackages/cli/src/docs/cli.tspackages/cli/src/explain.tspackages/cli/src/gunshi/analyze.tspackages/cli/src/gunshi/ci.tspackages/cli/src/gunshi/docs.tspackages/cli/src/gunshi/explain.tspackages/cli/src/gunshi/guard.tspackages/cli/src/gunshi/install.tspackages/cli/src/install/args.tspackages/cli/src/install/cli.tspackages/cli/src/resolve-args.tspackages/cli/test/ci/cli.test.tspackages/cli/test/cli-contract.test.tspackages/cli/test/docs-cli.test.tspackages/cli/test/explain.test.tspackages/cli/test/gunshi-ci.test.tspackages/cli/test/gunshi-docs-parity.test.tspackages/cli/test/gunshi-explain-parity.test.tspackages/cli/test/gunshi-install.test.tspackages/cli/test/install/cli.test.ts
💤 Files with no reviewable changes (3)
- packages/cli/src/ci/cli.ts
- packages/cli/src/explain.ts
- packages/cli/src/cli-args.ts
…y parser removal (#448) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
819497b to
d855a8c
Compare
|
Independent review rounds (fresh-context reviewer, dual oracles: current main + the pre-gunshi baseline):
🤖 Generated with Claude Code |
1 participant
Phase 3, the final phase of the gunshi migration (#448 plan):
installandcijoin the analyzer/docs/explainongunshi/bone, and the legacy parsing layer is deleted. The coordinator byte-compared main's dist against this branch across 15 surfaces: every functional cell identical; the only diffs are the two declared movements.Declared movements (changeset:
svelte-vitalsminor)docs/explain/install/ci--helpadopt the root's hybrid format — generated OPTIONS from thedefine()declarations, curated prose preserved. Flag descriptions now live in one place across the entire CLI; the help-drift class is dead on every surface. (Root--helpand--versionare byte-identical to main.)ci <unknown-subcommand>prints its guidance to stderr instead of stdout before exiting 2 — resolving the Phase-0-discovered exception as fix-not-accept: stdout is now empty on every exit-2 path, no asterisks. A same-class grep found no other instance.The deletion pass
ci/cli.tsandcli-args.tsare gone;docs/cli.ts/explain.tsshrink to data modules (help prose + pure renderers); the diff/baseline shadow parse absorbed its value-shape logic per the design-doc obligation. Two survivors, recorded as a deliberate interpretation in the Phase 3 addendum:parseRunArgs/parseInstallArgsremain as self-contained error-path helpers — a guard hit needs the legacy-shaped re-parse to reproduce exact error wording, and byte-parity dominates a literal reading of the deletion list.Coverage through the transition
The 47 docs/explain parity cells (whose legacy oracle was being deleted) were converted to snapshot pins before deletion; 28 new pins cover the install/ci argv matrices; three discriminator cells pin why
ci's outer dispatch stays a literal token compare (promotion/stripping there would dispatch shapes legacy never did — writing files where legacy errored). One more undocumented gunshi behavior found and neutralized:generate()force-appends a-v, --versionrow regardless of declarations (stripAutoVersionLine).Verification
Full gates green: 2,670 tests (cli 1,044), e2e 7/7, smoke 8/8, check:publish 0. Contract suite: one cell deliberately updated (the ci stderr movement); help goldens: exactly the four sub-command keys.
An independent fresh-context review of the completed migration follows as a comment before merge.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes