From 053a3088b2b77e33aa49b10eebc5c37c6a83ec55 Mon Sep 17 00:00:00 2001 From: jwfing Date: Thu, 7 May 2026 15:23:45 -0700 Subject: [PATCH] fix(branch): -y flag was shadowed by same-named global option (+ bump 0.1.69) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commander v13 assigns the value to the parent program's option when both parent and subcommand declare the same flag, leaving the subcommand's opts.yes undefined. -y, --yes is already global on the root program, so remove the redundant local declarations on branch merge/reset/delete and read yes from getRootOpts(cmd) which walks to the root. Adds a regression test that nests under a `branch` parent (matching real CLI wiring) and asserts clack.confirm is not called when -y is passed without --json. The prior --yes test paired with --json, where --json alone bypassed the prompt — so it never actually exercised -y. Co-Authored-By: Claude Opus 4.7 (1M context) --- package.json | 2 +- src/commands/branch/delete.ts | 7 +++---- src/commands/branch/merge.test.ts | 29 +++++++++++++++++++++++++++++ src/commands/branch/merge.ts | 6 ++---- src/commands/branch/reset.ts | 7 +++---- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index c8b8dfa..030e1f7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@insforge/cli", - "version": "0.1.68", + "version": "0.1.69", "description": "InsForge CLI - Command line tool for InsForge platform", "type": "module", "bin": { diff --git a/src/commands/branch/delete.ts b/src/commands/branch/delete.ts index 4322268..ab80e34 100644 --- a/src/commands/branch/delete.ts +++ b/src/commands/branch/delete.ts @@ -12,9 +12,8 @@ export function registerBranchDeleteCommand(branch: Command): void { branch .command('delete ') .description('Delete a branch') - .option('-y, --yes', 'Skip confirmation') - .action(async (name: string, opts: { yes?: boolean }, cmd) => { - const { json, apiUrl } = getRootOpts(cmd); + .action(async (name: string, _opts: Record, cmd) => { + const { json, apiUrl, yes } = getRootOpts(cmd); try { await requireAuth(apiUrl); const project = getProjectConfig(); @@ -25,7 +24,7 @@ export function registerBranchDeleteCommand(branch: Command): void { const target = branches.find(b => b.name === name); if (!target) throw new CLIError(`Branch '${name}' not found.`); - if (!opts.yes && !json) { + if (!yes && !json) { const confirmed = await clack.confirm({ message: `Delete branch '${name}'? This terminates its EC2 instance.`, }); diff --git a/src/commands/branch/merge.test.ts b/src/commands/branch/merge.test.ts index 9d20033..63794b4 100644 --- a/src/commands/branch/merge.test.ts +++ b/src/commands/branch/merge.test.ts @@ -53,10 +53,18 @@ vi.mock('../../lib/analytics.js', () => ({ shutdownAnalytics: vi.fn(async () => {}), })); +const clackConfirmMock = vi.hoisted(() => vi.fn(async () => true)); +vi.mock('@clack/prompts', () => ({ + confirm: clackConfirmMock, + isCancel: () => false, +})); + describe('branch merge', () => { beforeEach(() => { vi.clearAllMocks(); fsMock.writeFileSync.mockReset(); + clackConfirmMock.mockClear(); + clackConfirmMock.mockResolvedValue(true); }); it('--dry-run prints rendered_sql + summary, does not call execute', async () => { @@ -112,6 +120,27 @@ describe('branch merge', () => { expect(mergeBranchExecuteApi).toHaveBeenCalledWith('b1', undefined); }); + // Regression: -y on the merge subcommand previously got shadowed by the + // same-named global option, so the prompt fired even when the user passed -y. + // Test nests under a `branch` parent to mirror the real CLI wiring, since + // the shadowing only manifested through that registration path. + it('-y after positional skips confirmation prompt (no --json)', async () => { + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + const branch = program.command('branch'); + registerBranchMergeCommand(branch); + const origLog = console.log; + console.log = () => {}; + try { + await program.parseAsync(['branch', 'merge', 'feat-x', '-y'], { from: 'user' }); + } finally { + console.log = origLog; + } + expect(clackConfirmMock).not.toHaveBeenCalled(); + const { mergeBranchExecuteApi } = await import('../../lib/api/platform.js'); + expect(mergeBranchExecuteApi).toHaveBeenCalledWith('b1', undefined); + }); + it('conflict path exits with code 2 and prints per-conflict summary', async () => { const { mergeBranchDryRunApi } = await import('../../lib/api/platform.js'); (mergeBranchDryRunApi as any).mockResolvedValueOnce({ diff --git a/src/commands/branch/merge.ts b/src/commands/branch/merge.ts index d270690..6f36e25 100644 --- a/src/commands/branch/merge.ts +++ b/src/commands/branch/merge.ts @@ -14,7 +14,6 @@ import { captureEvent, shutdownAnalytics } from '../../lib/analytics.js'; interface MergeOptions { dryRun?: boolean; - yes?: boolean; saveSql?: string; } @@ -23,10 +22,9 @@ export function registerBranchMergeCommand(branch: Command): void { .command('merge ') .description('Merge a branch back to its parent project') .option('--dry-run', 'Compute the diff and print rendered SQL; do not apply') - .option('-y, --yes', 'Skip confirmation prompt') .option('--save-sql ', 'Write rendered SQL preview to a file') .action(async (name: string, opts: MergeOptions, cmd) => { - const { json, apiUrl } = getRootOpts(cmd); + const { json, apiUrl, yes } = getRootOpts(cmd); try { await requireAuth(apiUrl); const project = getProjectConfig(); @@ -89,7 +87,7 @@ export function registerBranchMergeCommand(branch: Command): void { } // Confirm before executing (unless --yes or --json). - if (!opts.yes && !json) { + if (!yes && !json) { const parentLabel = project.branched_from?.project_name ?? project.project_name; const confirmed = await clack.confirm({ message: `Apply this merge to parent project '${parentLabel}'?`, diff --git a/src/commands/branch/reset.ts b/src/commands/branch/reset.ts index b14537a..12718d7 100644 --- a/src/commands/branch/reset.ts +++ b/src/commands/branch/reset.ts @@ -18,9 +18,8 @@ export function registerBranchResetCommand(branch: Command): void { branch .command('reset ') .description("Reset a branch's database back to T0 (the parent snapshot at branch creation)") - .option('-y, --yes', 'Skip confirmation') - .action(async (name: string, opts: { yes?: boolean }, cmd) => { - const { json, apiUrl } = getRootOpts(cmd); + .action(async (name: string, _opts: Record, cmd) => { + const { json, apiUrl, yes } = getRootOpts(cmd); try { await requireAuth(apiUrl); const project = getProjectConfig(); @@ -40,7 +39,7 @@ export function registerBranchResetCommand(branch: Command): void { } const entryState = target.branch_state; - if (!opts.yes && !json) { + if (!yes && !json) { const confirmed = await clack.confirm({ message: `Reset branch '${name}' back to T0? This wipes all schema/data/policy/function/migration changes made on the branch since creation.` +