refactor(cli): split bin.ts into per-command CommandModule files#246
Closed
refactor(cli): split bin.ts into per-command CommandModule files#246
Conversation
Contributor
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Wrong path depth for package.json version fallback
- Added a third '..' level to the resolve() call so that from dist/src/commands/context.js it correctly traverses up three directories to reach /package.json.
- ✅ Fixed: Node version check moved below all module imports
- Moved the NODE_VERSION_RANGE check and process.exit(1) to immediately after the semver/red/dotenv imports and before any clack-dependent imports like ./src/commands.
Or push these changes by commenting:
@cursor push f560ffbee3
Preview (f560ffbee3)
diff --git a/bin.ts b/bin.ts
--- a/bin.ts
+++ b/bin.ts
@@ -28,6 +28,16 @@
import { config as loadDotenv } from 'dotenv';
loadDotenv();
+// Have to run this above the other imports because they are importing clack that
+// has the problematic imports.
+const NODE_VERSION_RANGE = '>=18.17.0';
+if (!satisfies(process.version, NODE_VERSION_RANGE)) {
+ red(
+ `Amplitude wizard requires Node.js ${NODE_VERSION_RANGE}. You are using Node.js ${process.version}. Please upgrade your Node.js version.`,
+ );
+ process.exit(1);
+}
+
import yargs from 'yargs';
import { hideBin } from 'yargs/helpers';
import chalk from 'chalk';
@@ -88,17 +98,6 @@
manifestCommand,
} from './src/commands';
-const NODE_VERSION_RANGE = '>=18.17.0';
-
-// Have to run this above the other imports because they are importing clack that
-// has the problematic imports.
-if (!satisfies(process.version, NODE_VERSION_RANGE)) {
- red(
- `Amplitude wizard requires Node.js ${NODE_VERSION_RANGE}. You are using Node.js ${process.version}. Please upgrade your Node.js version.`,
- );
- process.exit(1);
-}
-
// ── Observability bootstrap ─────────────────────────────────────────
// Initialize structured logging early so all code paths can use it.
// The terminal sink routes log output through the UI singleton (getUI()),
diff --git a/src/commands/context.ts b/src/commands/context.ts
--- a/src/commands/context.ts
+++ b/src/commands/context.ts
@@ -24,7 +24,7 @@
.parse(
JSON.parse(
readFileSync(
- resolve(dirname(__filename), '..', '..', 'package.json'),
+ resolve(dirname(__filename), '..', '..', '..', 'package.json'),
'utf-8',
),
),You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit c1e0113. Configure here.
This was referenced Apr 25, 2026
Extract every yargs `.command(...)` body from the 2,768-line bin.ts into
its own `src/commands/<name>.ts` file exporting a `CommandModule`. Shared
helpers (session builder, non-interactive credential resolution, direct
signup wrapper) live in `src/commands/helpers.ts`; CLI invocation strings
and version resolution live in `src/commands/context.ts`. The barrel
`src/commands/index.ts` makes adding a new command a one-line change.
bin.ts is now 475 lines and contains only:
- inherited-env sanitization + NODE_ENV resolution + Node version gate
- chalk color resolution and dotenv loading
- observability bootstrap (logger, Sentry, analytics, update-notifier)
- global yargs options + `.command(...)` registrations
- the `.check()` for `--app-id`, `.strict()`, help/version aliases
Behaviour preserved:
- `.strict()` still rejects `--app-ids=foo` and other typos
- `--app-id=notanumber` still fails the numeric `.check()`
- `AMPLITUDE_WIZARD_DEV=1 --help` exits 0 (hidden flags revealed)
- All hidden env-only shadow flags (`dev`, `log`, `token`, `agent`,
`install-dir`, `classic`) preserved so `.strict()` accepts the
AMPLITUDE_WIZARD_* env-var passthroughs on every subcommand
- The `--auto-approve` / `--yes` / `--force` capability matrix and the
write-gate logic in the default command's agent-mode auto-detect are
moved verbatim — no semantic change
Tests: zone-resolution call-site drift guard updated to allowlist the new
files (`src/commands/default.ts`, `src/commands/helpers.ts`) that took
over the pre-OAuth RegionSelect gate + verbose-log display from bin.ts.
This actually drops the failing test count from 77 → 76 vs origin/main.
All other failing tests (TUI snapshots / KeyHintBar / HeaderBar) are
pre-existing on origin/main and unrelated to this split.
This redo replaces the previous PR #246 commit, which was 25 commits
behind main and would have required resolving an 1,866-line conflict
block. The design is unchanged from the earlier review.
kelsonpw
force-pushed
the
feat/cli-bin-commands-split
branch
from
c1e0113 to
586acb9
Compare
Collaborator
Author
|
Force-pushed a structural redo onto current main. The earlier branch (c1e0113) was 25 commits behind main, and a regular rebase produced a 1,866-line conflict block — main had grown plan/apply/verify (#269), the --auto-approve/--yes/--force capability matrix and write-gate (#254), --resume (#259), needs_input/INPUT_REQUIRED (#253), inner-agent lifecycle events (#270), and adjacent bin.ts touches. Rather than mechanical conflict-resolution, I redid the extraction fresh against origin/main, reusing the same CommandModule pattern (src/commands/.ts + helpers.ts + context.ts + barrel index.ts) the original PR established. bin.ts is now 475 lines (down from 2,768) and contains only env sanitization, NODE_ENV/Node version gates, observability bootstrap, global yargs options, and |
Two test fixes surfaced when main was merged into this PR:
1. The bin.ts split widened the import graph reachable from cli.test.ts —
at least one extracted command (or its transitive dep, e.g. the
commandments / wizard-tools chain) now pulls DEMO_MODE through the
mocked constants module. Mocking it false matches the production
default (DEMO_MODE_WIZARD env unset).
2. The "invokes runWizard in CI mode without --api-key when --install-dir
is set" test exercises the full credential-resolution path without an
--api-key shortcut. Combined with runCLI calling vi.resetModules() +
await import('../../bin.ts') — which now imports all 14
src/commands/*.ts modules transitively — it pushes past the default
5s testTimeout under parallel-load. Bumped to 15s. Other CI-mode
tests use --api-key and stay fast.
Both are scoped, per-test fixes — no vitest.config.ts change to avoid
collision with parallel work.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kelsonpw
added a commit
that referenced
this pull request
Apr 26, 2026
Replaces closed PR #246, redone off current main. Pure mechanical refactor. bin.ts has been the conflict magnet of the repo — every CLI PR edits the same 2700+ line file. Per-command files isolate ownership: a new flag, a new mcp subcommand, a new whoami enhancement — none of those should require touching the same file as everything else. Every flag, env-var passthrough, help text, exit code, strict-mode rejection, and .check() validator is byte-identical post-split. bin.ts: 2768 → 473 lines. Structure: src/commands/ context.ts CLI_INVOCATION, WIZARD_VERSION, IS_WIZARD_DEV helpers.ts buildSessionFromOptions, resolveNonInteractiveCredentials, runDirectSignupIfRequested, lazyRunWizard index.ts barrel export default.ts $0 — interactive/CI/agent/classic dispatch (849 lines) login.ts OAuth login flow logout.ts Clear stored credentials whoami.ts, feedback.ts, slack.ts, region.ts, detect.ts, status.ts plan.ts, apply.ts, verify.ts (sub-agent commands from #269) auth.ts Subcommand dispatcher: status / token mcp.ts Subcommand dispatcher: add / remove / serve manifest.ts Print agent-mode manifest Critical preserve-list (independently verified): In default.ts: - 4-way handler dispatch order: agent → ci|yes|force → classic → TUI - --auto-approve does NOT promote to agent mode - --env deprecation warning paths - Fire-and-forget fetchAmplitudeUser org/workspace/appId hydration - Crash-recovery checkpoint with three onEnterScreen save/clear hooks - Direct-signup → fall-through-to-OAuth with signupTokensObtained fallback - Pre-detection branch (detectAmplitudeInProject + setAmplitudePreDetected + waitForPreDetectedChoice + resetForAgentAfterPreDetected) - SIGINT handler installed immediately after startTUI() In apply.ts: - spawn(process.execPath, [process.argv[1] ?? '', '--agent', '--yes', ...]) - AMPLITUDE_WIZARD_PLAN_ID env passthrough - 4-way switch on resolvePlan result.kind - mode.allowWrites gate exits with WRITE_REFUSED (13) In bin.ts: - sanitizeNestedClaudeEnv() runs first - NODE_ENV path-based detection from #249 - Node >=20 version gate - All 21 flags: --auto-approve and --force from #254 preserved - .check() for --app-id numeric on root yargs - .env('AMPLITUDE_WIZARD') for env-var passthrough shadows Verification: - 20/20 --help outputs byte-identical - 4/4 strict-mode rejections byte-identical - 2/2 env-var passthroughs byte-identical - pnpm tsc --noEmit: clean - pnpm lint: clean (1 pre-existing warning, unrelated) - pnpm test: 1497 passed, 17 skipped, 0 failed - pnpm build smoke: passes src/lib/__tests__/zone-resolution.invariants.test.ts holds a hardcoded file-path allowlist for session.region reads. The two new paths (default.ts, helpers.ts) inherit the read from bin.ts. Structural accommodation; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
9 tasks
Collaborator
Author
|
closed, superseded by #296 |
kelsonpw
added a commit
that referenced
this pull request
Apr 27, 2026
Replaces closed PR #246, redone off current main. Pure mechanical refactor. bin.ts has been the conflict magnet of the repo — every CLI PR edits the same 2700+ line file. Per-command files isolate ownership: a new flag, a new mcp subcommand, a new whoami enhancement — none of those should require touching the same file as everything else. Every flag, env-var passthrough, help text, exit code, strict-mode rejection, and .check() validator is byte-identical post-split. bin.ts: 2768 → 473 lines. Structure: src/commands/ context.ts CLI_INVOCATION, WIZARD_VERSION, IS_WIZARD_DEV helpers.ts buildSessionFromOptions, resolveNonInteractiveCredentials, runDirectSignupIfRequested, lazyRunWizard index.ts barrel export default.ts $0 — interactive/CI/agent/classic dispatch (849 lines) login.ts OAuth login flow logout.ts Clear stored credentials whoami.ts, feedback.ts, slack.ts, region.ts, detect.ts, status.ts plan.ts, apply.ts, verify.ts (sub-agent commands from #269) auth.ts Subcommand dispatcher: status / token mcp.ts Subcommand dispatcher: add / remove / serve manifest.ts Print agent-mode manifest Critical preserve-list (independently verified): In default.ts: - 4-way handler dispatch order: agent → ci|yes|force → classic → TUI - --auto-approve does NOT promote to agent mode - --env deprecation warning paths - Fire-and-forget fetchAmplitudeUser org/workspace/appId hydration - Crash-recovery checkpoint with three onEnterScreen save/clear hooks - Direct-signup → fall-through-to-OAuth with signupTokensObtained fallback - Pre-detection branch (detectAmplitudeInProject + setAmplitudePreDetected + waitForPreDetectedChoice + resetForAgentAfterPreDetected) - SIGINT handler installed immediately after startTUI() In apply.ts: - spawn(process.execPath, [process.argv[1] ?? '', '--agent', '--yes', ...]) - AMPLITUDE_WIZARD_PLAN_ID env passthrough - 4-way switch on resolvePlan result.kind - mode.allowWrites gate exits with WRITE_REFUSED (13) In bin.ts: - sanitizeNestedClaudeEnv() runs first - NODE_ENV path-based detection from #249 - Node >=20 version gate - All 21 flags: --auto-approve and --force from #254 preserved - .check() for --app-id numeric on root yargs - .env('AMPLITUDE_WIZARD') for env-var passthrough shadows Verification: - 20/20 --help outputs byte-identical - 4/4 strict-mode rejections byte-identical - 2/2 env-var passthroughs byte-identical - pnpm tsc --noEmit: clean - pnpm lint: clean (1 pre-existing warning, unrelated) - pnpm test: 1497 passed, 17 skipped, 0 failed - pnpm build smoke: passes src/lib/__tests__/zone-resolution.invariants.test.ts holds a hardcoded file-path allowlist for session.region reads. The two new paths (default.ts, helpers.ts) inherit the read from bin.ts. Structural accommodation; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kelsonpw
added a commit
that referenced
this pull request
Apr 29, 2026
Replaces closed PR #246, redone off current main. Pure mechanical refactor. bin.ts has been the conflict magnet of the repo — every CLI PR edits the same 2700+ line file. Per-command files isolate ownership: a new flag, a new mcp subcommand, a new whoami enhancement — none of those should require touching the same file as everything else. Every flag, env-var passthrough, help text, exit code, strict-mode rejection, and .check() validator is byte-identical post-split. bin.ts: 2768 → 473 lines. Structure: src/commands/ context.ts CLI_INVOCATION, WIZARD_VERSION, IS_WIZARD_DEV helpers.ts buildSessionFromOptions, resolveNonInteractiveCredentials, runDirectSignupIfRequested, lazyRunWizard index.ts barrel export default.ts $0 — interactive/CI/agent/classic dispatch (849 lines) login.ts OAuth login flow logout.ts Clear stored credentials whoami.ts, feedback.ts, slack.ts, region.ts, detect.ts, status.ts plan.ts, apply.ts, verify.ts (sub-agent commands from #269) auth.ts Subcommand dispatcher: status / token mcp.ts Subcommand dispatcher: add / remove / serve manifest.ts Print agent-mode manifest Critical preserve-list (independently verified): In default.ts: - 4-way handler dispatch order: agent → ci|yes|force → classic → TUI - --auto-approve does NOT promote to agent mode - --env deprecation warning paths - Fire-and-forget fetchAmplitudeUser org/workspace/appId hydration - Crash-recovery checkpoint with three onEnterScreen save/clear hooks - Direct-signup → fall-through-to-OAuth with signupTokensObtained fallback - Pre-detection branch (detectAmplitudeInProject + setAmplitudePreDetected + waitForPreDetectedChoice + resetForAgentAfterPreDetected) - SIGINT handler installed immediately after startTUI() In apply.ts: - spawn(process.execPath, [process.argv[1] ?? '', '--agent', '--yes', ...]) - AMPLITUDE_WIZARD_PLAN_ID env passthrough - 4-way switch on resolvePlan result.kind - mode.allowWrites gate exits with WRITE_REFUSED (13) In bin.ts: - sanitizeNestedClaudeEnv() runs first - NODE_ENV path-based detection from #249 - Node >=20 version gate - All 21 flags: --auto-approve and --force from #254 preserved - .check() for --app-id numeric on root yargs - .env('AMPLITUDE_WIZARD') for env-var passthrough shadows Verification: - 20/20 --help outputs byte-identical - 4/4 strict-mode rejections byte-identical - 2/2 env-var passthroughs byte-identical - pnpm tsc --noEmit: clean - pnpm lint: clean (1 pre-existing warning, unrelated) - pnpm test: 1497 passed, 17 skipped, 0 failed - pnpm build smoke: passes src/lib/__tests__/zone-resolution.invariants.test.ts holds a hardcoded file-path allowlist for session.region reads. The two new paths (default.ts, helpers.ts) inherit the read from bin.ts. Structural accommodation; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kelsonpw
added a commit
that referenced
this pull request
Apr 29, 2026
* refactor(cli): split bin.ts into per-command CommandModule files Replaces closed PR #246, redone off current main. Pure mechanical refactor. bin.ts has been the conflict magnet of the repo — every CLI PR edits the same 2700+ line file. Per-command files isolate ownership: a new flag, a new mcp subcommand, a new whoami enhancement — none of those should require touching the same file as everything else. Every flag, env-var passthrough, help text, exit code, strict-mode rejection, and .check() validator is byte-identical post-split. bin.ts: 2768 → 473 lines. Structure: src/commands/ context.ts CLI_INVOCATION, WIZARD_VERSION, IS_WIZARD_DEV helpers.ts buildSessionFromOptions, resolveNonInteractiveCredentials, runDirectSignupIfRequested, lazyRunWizard index.ts barrel export default.ts $0 — interactive/CI/agent/classic dispatch (849 lines) login.ts OAuth login flow logout.ts Clear stored credentials whoami.ts, feedback.ts, slack.ts, region.ts, detect.ts, status.ts plan.ts, apply.ts, verify.ts (sub-agent commands from #269) auth.ts Subcommand dispatcher: status / token mcp.ts Subcommand dispatcher: add / remove / serve manifest.ts Print agent-mode manifest Critical preserve-list (independently verified): In default.ts: - 4-way handler dispatch order: agent → ci|yes|force → classic → TUI - --auto-approve does NOT promote to agent mode - --env deprecation warning paths - Fire-and-forget fetchAmplitudeUser org/workspace/appId hydration - Crash-recovery checkpoint with three onEnterScreen save/clear hooks - Direct-signup → fall-through-to-OAuth with signupTokensObtained fallback - Pre-detection branch (detectAmplitudeInProject + setAmplitudePreDetected + waitForPreDetectedChoice + resetForAgentAfterPreDetected) - SIGINT handler installed immediately after startTUI() In apply.ts: - spawn(process.execPath, [process.argv[1] ?? '', '--agent', '--yes', ...]) - AMPLITUDE_WIZARD_PLAN_ID env passthrough - 4-way switch on resolvePlan result.kind - mode.allowWrites gate exits with WRITE_REFUSED (13) In bin.ts: - sanitizeNestedClaudeEnv() runs first - NODE_ENV path-based detection from #249 - Node >=20 version gate - All 21 flags: --auto-approve and --force from #254 preserved - .check() for --app-id numeric on root yargs - .env('AMPLITUDE_WIZARD') for env-var passthrough shadows Verification: - 20/20 --help outputs byte-identical - 4/4 strict-mode rejections byte-identical - 2/2 env-var passthroughs byte-identical - pnpm tsc --noEmit: clean - pnpm lint: clean (1 pre-existing warning, unrelated) - pnpm test: 1497 passed, 17 skipped, 0 failed - pnpm build smoke: passes src/lib/__tests__/zone-resolution.invariants.test.ts holds a hardcoded file-path allowlist for session.region reads. The two new paths (default.ts, helpers.ts) inherit the read from bin.ts. Structural accommodation; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(commands): import parseEventPlanContent from event-plan-parser Address Cursor Bugbot finding: the dynamic import in `default.ts` was sourcing `parseEventPlanContent` from `agent-interface.js`, which defeats the stated purpose of keeping the Claude Agent SDK / UI singleton out of the bin.ts load path. The lightweight `event-plan-parser.js` module was extracted specifically for CLI/ops callers — use it directly here. Behavior unchanged (`agent-interface.ts` still re-exports for back-compat). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extract every yargs
.command(...)handler from the monolithic 2416-linebin.tsinto individualCommandModulefiles undersrc/commands/*.ts.bin.tsnow contains only the global yargs config, observability bootstrap, and the registration chain — going from 2416 → 428 lines.Why
bin.tshas been the conflict magnet of the repo. PR #227 alone hit two rebase conflicts in this file within 24 hours, and every CLI change (new flag, new subcommand, mode tweak) edits it. Per-command files isolate ownership so unrelated CLI work can land in parallel without cross-PR rebase pain.What's preserved
Every flag, subcommand,
--helpline, and exit code is unchanged.node ./dist/bin.js --helpproduces byte-identical output before and after for the top-level command and every subcommand. Verified:.strict()still rejects typos (--app-ids=foo→Unknown arguments: app-ids, appIds).check()still validates--app-id(--app-id=notanumber→--app-id must be a positive integer)dev,log,token,agent,install-dir,classic) remain — strict mode would reject env-var injection without themAMPLITUDE_WIZARD_DEV=1 node ./dist/bin.js --helpstill works (env passthrough)--api-key,--local-mcp,--workspace-id,--org,--env)Files
The zone-resolution call-site invariant test had
bin.tsallowlisted for the pre-OAuth RegionSelect gate read;src/commands/default.tsinherits that read and is added to the allowlist with the same justification.Test plan
pnpm tsc --noEmitcleanpnpm build+ smoke test passespnpm test— 1233 passed, no regressions vsmainpnpm lintcleandiff <baseline> <after>for--help,detect --help,auth --help,mcp --help,login --help— all byte-identicalnode dist/bin.js --app-ids=foo→ strict rejection (unchanged)node dist/bin.js --app-id=notanumber→.check()rejection (unchanged)AMPLITUDE_WIZARD_DEV=1 node dist/bin.js --help→ env passthrough worksnode dist/bin.js manifest→ JSON output unchangedsrc/__tests__/cli.test.ts(CI/TUI auth/login/logout/whoami/feedback) all passOut of scope
src/lib/,src/ui/, orsrc/frameworks/🤖 Generated with Claude Code
Note
Medium Risk
Medium risk due to a large refactor of the CLI entrypoint and command dispatch; behavior should be unchanged, but routing/flag parsing and non-interactive flows could regress if any command module wiring differs from the previous monolith.
Overview
Refactors the CLI by extracting the previously monolithic
bin.tsyargs command implementations into dedicatedsrc/commands/*.tsCommandModules, with shared logic moved intosrc/commands/helpers.tsand shared constants intosrc/commands/context.ts.bin.tsnow primarily bootstraps environment/observability, configures global yargs options, and registers the imported command modules; command behavior (agent/CI/classic/TUI dispatch, auth, planning/apply/verify/status, MCP, etc.) is moved without intended functional changes.Tests and invariants are adjusted to reflect the wider import graph (
DEMO_MODEmock added, one CI test timeout increased, and zone-resolution allowlists updated), and--app-idvalidation typing is tightened to acceptstring | number | nullinputs.Reviewed by Cursor Bugbot for commit fa36c5a. Bugbot is set up for automated code reviews on this repo. Configure here.