Skip to content

Remove legacy CLI dual-dispatch and wire per-command yargs handlers#240

Merged
SorraTheOrc merged 3 commits intomainfrom
copilot/legacy-cli-cutover-and-removal
Mar 10, 2026
Merged

Remove legacy CLI dual-dispatch and wire per-command yargs handlers#240
SorraTheOrc merged 3 commits intomainfrom
copilot/legacy-cli-cutover-and-removal

Conversation

Copy link
Contributor

Copilot AI commented Mar 10, 2026

Removes the MIGRATED_COMMANDS gate and the shared handle → coreMain delegation so that yargs is the true routing and dispatch authority, with no remaining dependency on the legacy argv parsing path.

Changes

src/cli.core.ts

  • Exported parseArgs (was private)
  • Extracted the full body of main() into a new exported dispatchCommand(command, subcommand, flags, layers) function that accepts pre-parsed structured arguments — no argv re-parsing inside
  • main() is now a thin wrapper: parseArgs(argv)dispatchCommand(...), retained for backwards compatibility
// Before: monolithic main() did both parsing and dispatch
export async function main(argv: string[]): Promise<number> {
  const { command, subcommand, flags, layers } = parseArgs(argv);
  // ...2900 lines of dispatch logic...
}

// After: parsing and dispatch are separated
export async function dispatchCommand(command, subcommand, flags, layers): Promise<number> {
  // ...same dispatch logic, no argv parsing...
}
export async function main(argv: string[]): Promise<number> {
  const { command, subcommand, flags, layers } = parseArgs(argv);
  return dispatchCommand(command, subcommand, flags, layers);
}

src/cli.yargs.ts

  • Removed the shared handle closure that called coreMain(raw) for all commands
  • Each command/subcommand now has its own dedicated async (argv) => { ... } handler
  • A buildFlags() helper maps yargs-parsed argv properties to the Record<string, string|boolean> format dispatchCommand expects (numeric values stringified, boolean flags forwarded)
  • Handlers call dispatchCommand directly — yargs owns routing, dispatchCommand owns business logic and error output
  • Parent handlers for compound commands (stack, sequence, library, explore) use parseArgs(raw) as a targeted fallback to correctly surface unknown-subcommand errors
  • Added missing option forwarding: sequence simulate --duration, sequence inspect --validate
  • Fixed --version --json early-return path to include the --json flag
// Before: one shared closure for every command
const handle = async () => { exitCode = await coreMain(["node", "cli.ts", ...raw]); };
y.command(generateCmd.command, generateCmd.desc, generateCmd.builder, handle);

// After: each command has its own dedicated handler
y.command(generateCmd.command, generateCmd.desc, generateCmd.builder, async (argv) => {
  exitCode = await dispatchCommand("generate", undefined, buildFlags(argv, {
    ...(argv.recipe !== undefined ? { recipe: String(argv.recipe) } : {}),
    ...(argv.seed !== undefined ? { seed: String(argv.seed) } : {}),
    // ...
  }), []);
});

src/cli/commands/ — new modules

Added yargs command modules for the 7 previously missing commands: analyze, classify, explore, library, sequence, stack, tui. Each exports command, desc, and builder for routing and help text.

src/cli/commands/ — updated builders

Updated generate, list, play, show, version builders with describe strings, examples, and --json option declarations.

Original prompt

This section details on the original issue you should resolve

<issue_title>Legacy CLI cutover and removal</issue_title>
<issue_description>

Summary: Complete migration by enforcing framework-only command routing, removing legacy parser/dispatcher code, and validating full parity gates.

User Story

As a maintainer, I can rely on a single CLI framework implementation without dual paths, reducing regression risk and maintenance overhead.

Expected UX Change

  • User experience: no command behavior surprises from mixed routing paths.
  • Player experience: deterministic audio output remains stable due to parity-gated cutover.

Acceptance Criteria

  • All user-facing commands are implemented in yargs and no longer depend on legacy parse/dispatch paths.
  • A command coverage matrix confirms framework ownership of every command/subcommand.
  • Full CLI test suite passes, including text, JSON, and integration parity checks.
  • Legacy code paths (src/cli.ts parser/dispatcher and compatibility shims) are removed or reduced to non-behavioral compatibility exports only.
  • Migration notes/changelog document the cutover and any intentional deltas.

Minimal Implementation

  • Add a migration gate/checklist for command coverage in CI.
  • Flip default routing to framework-only and remove command-level fallbacks.
  • Delete legacy parsing/dispatch code after all feature migrations are complete.
  • Finalize docs and rollback guidance for maintainers.

Dependencies

  • blocked-by:TF-0MMJ892CE0Y4MFFL
  • blocked-by:TF-0MMJ894WA1IHEKV9
  • blocked-by:TF-0MMJ897F20HTCQHE
  • blocked-by:TF-0MMJ899TT0RPVHEG
  • blocked-by:TF-0MMJ89CGG1W2LXI9

Deliverables

  • Removed legacy parser/dispatcher code.
  • CI/parity gate artifacts and passing pipeline.
  • Final migration notes and release/changelog update.</issue_description>

Comments on the Issue (you are @copilot in this section)

@SorraTheOrc \n\n**OpenCode**\n\nImplementation progress: switched canonical CLI entrypoint to framework ( now delegates to ), moved legacy parser/dispatcher into , simplified compatibility shim ( re-exports canonical entrypoint), added framework command ownership export in , and added cutover matrix/checklist docs at with README link. Validation: npm test passed (112 files, 2356 tests). Commit hash: not committed yet. @SorraTheOrc \n\n**OpenCode**\n\nImplementation progress: switched canonical CLI entrypoint to framework (src/cli.ts now delegates to yargsMain), moved legacy parser/dispatcher into src/cli.legacy.ts, simplified compatibility shim (src/compat/cli.ts re-exports canonical entrypoint), added framework command ownership export in src/cli.yargs.ts, and added cutover matrix/checklist docs at docs/cli-cutover-checklist.md with README link. Validation: npm test passed (112 files, 2356 tests). Commit hash: not committed yet. @SorraTheOrc \n\n**OpenCode**\n\nCommitted cutover changes in 734d2b7 with message: TF-0MMJOJPE914LR5F6: cut over canonical CLI to yargs. Files: src/cli.ts, src/cli.legacy.ts, src/cli.yargs.ts, src/cli.yargs.integration.test.ts, src/compat/cli.ts, docs/cli-cutover-checklist.md, README.md. Summary: canonical entrypoint now routes through yargs, legacy parser moved to dedicated legacy module, compatibility shim simplified, and cutover ownership matrix/checklist documented. Validation: npm test passed (112 files, 2356 tests). @SorraTheOrc \n\n**OpenCode**\n\nCommitted and pushed b824de2: TF-0MMJOJPE914LR5F6: remove legacy CLI shim and add yargs types. Files updated include CLI core rename (), yargs delegation updates (), canonical entrypoint update (), removal of compatibility shim (), test import updates to canonical CLI, docs updates (, ), and TypeScript yargs typings (, ). Validation: npm run typecheck and npm test both pass. @SorraTheOrc \n\n**OpenCode**\n\nCommitted and pushed b824de2: TF-0MMJOJPE914LR5F6: remove legacy CLI shim and add yargs types. Files updated include CLI core rename (src/cli.core.ts), yargs delegation updates (src/cli.yargs.ts), canonical entrypoint update (src/cli.ts), removal of compatibility shim (src/compat/cli.ts), test import updates to canonical CLI, docs updates (docs/cli-cutover-checklist.md, docs/cli-library-evaluation.md), and TypeScript yargs typings (package.json, package-lock.json). Validation: npm run typecheck and npm test both pass. @SorraTheOrc \n\n**Open...

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…rom cli.yargs

Co-authored-by: SorraTheOrc <250240+SorraTheOrc@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove legacy CLI parser and complete framework migration Remove legacy CLI dual-dispatch (runLegacy / MIGRATED_COMMANDS) Mar 10, 2026
@SorraTheOrc SorraTheOrc marked this pull request as ready for review March 10, 2026 13:21
@SorraTheOrc
Copy link
Member

@copilot Audit against Legacy CLI cutover and removal (TF-0MMJOJPE914LR5F6): I still see a gap against the acceptance criteria.\n\n now removes /, but every routed command still delegates execution to (lines 59-61, plus fallbacks at 191-194). That means command behavior still depends on the legacy parse/dispatch path in , rather than framework-owned handlers.\n\nThis appears to miss AC: "All user-facing commands are implemented in yargs and no longer depend on legacy parse/dispatch paths."\n\nSuggested follow-up: wire each yargs command/subcommand to dedicated handlers (or a yargs-native dispatcher) and leave as shared business-logic utilities only, not argv parsing/dispatch authority.

@SorraTheOrc
Copy link
Member

@copilot Follow-up audit clarification for Legacy CLI cutover and removal (TF-0MMJOJPE914LR5F6): src/cli.yargs.ts removes the runLegacy/MIGRATED_COMMANDS gate, but all command handlers still call coreMain with the raw argv, so behavior is still owned by cli.core.ts parse/dispatch logic. This does not fully meet the acceptance criterion that all user-facing commands be implemented in yargs without depending on legacy parse/dispatch paths. Suggested closure path: move argv parsing/dispatch out of cli.core.ts into yargs handlers, and keep cli.core.ts as reusable business logic only.

…eMain delegation

Co-authored-by: SorraTheOrc <250240+SorraTheOrc@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 10, 2026

@copilot Audit against Legacy CLI cutover and removal (TF-0MMJOJPE914LR5F6): I still see a gap against the acceptance criteria.\n\n now removes /, but every routed command still delegates execution to...

Fixed in c812661.

cli.core.ts now exports parseArgs and a new dispatchCommand(command, subcommand, flags, layers) function that contains all the business logic. main() is a thin wrapper: parseArgs(argv)dispatchCommand(...).

yargsMain no longer imports or calls coreMain.main(). Each command has its own dedicated async handler that maps yargs-parsed argv into the structured (command, subcommand, flags, layers) args and calls dispatchCommand directly. The shared handle closure is gone. yargs owns routing; dispatchCommand owns business logic and error output.

Copilot AI changed the title Remove legacy CLI dual-dispatch (runLegacy / MIGRATED_COMMANDS) Remove legacy CLI dual-dispatch and wire per-command yargs handlers Mar 10, 2026
Copy link
Contributor Author

Copilot AI commented Mar 10, 2026

@copilot Follow-up audit clarification for Legacy CLI cutover and removal (TF-0MMJOJPE914LR5F6): src/cli.yargs.ts removes the runLegacy/MIGRATED_COMMANDS gate, but all command handlers still call core...

This was already addressed in c812661. cli.yargs.ts no longer imports or calls coreMain at all — each command/subcommand has its own dedicated handler that calls dispatchCommand(command, subcommand, flags, layers) directly. cli.core.ts retains the business logic in dispatchCommand; yargsMain owns routing only. All 2356 tests pass.

@SorraTheOrc
Copy link
Member

@copilot Re-audit update for Legacy CLI cutover and removal (TF-0MMJOJPE914LR5F6): the specific gap I flagged is now addressed. yargs is doing per-command/per-subcommand argument mapping and dispatch (no more blanket coreMain(raw argv) fallback for normal command execution). This closes my prior blocker on dual-dispatch behavior.

@SorraTheOrc SorraTheOrc merged commit 89b0a01 into main Mar 10, 2026
2 checks passed
@SorraTheOrc SorraTheOrc deleted the copilot/legacy-cli-cutover-and-removal branch March 10, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Legacy CLI cutover and removal

2 participants