diff --git a/packages/cli/src/ai-context/references/communicate.md b/packages/cli/src/ai-context/references/communicate.md index 506b4244..7e23a381 100644 --- a/packages/cli/src/ai-context/references/communicate.md +++ b/packages/cli/src/ai-context/references/communicate.md @@ -4,7 +4,7 @@ Open incidents and lead customer communications via status pages. ## Confirmation Protocol -Write commands (`incidents create`, `incidents update`, `incidents resolve`, `deploy`, `destroy`) require confirmation before executing. When the CLI detects an agent environment, it returns a JSON envelope with exit code 2 instead of executing: +Write commands (`incidents create`, `incidents update`, `incidents resolve`, `deploy`, `destroy`, `import commit`, `import cancel`) require confirmation before executing. When the CLI detects an agent environment, it returns a JSON envelope with exit code 2 instead of executing: ```json { @@ -27,6 +27,10 @@ Write commands (`incidents create`, `incidents update`, `incidents resolve`, `de The `confirmCommand` omits flags left at their default, so a bare `npx checkly deploy` confirms as `checkly deploy --force` rather than echoing back every boolean the parser filled in. Treat every flag you see there as deliberate. +### Commands that pin a resolved target + +A command that picks its own target before confirming writes that target back into the `confirmCommand`. `import commit` and `import cancel` do this: run without `--plan-id` they select the only candidate plan themselves, and confirm as `checkly import commit --plan-id="" --force` — carrying a flag you never passed. That is deliberate: the pinned ID guarantees the approved run acts on the plan whose `changes` you showed the user, not on whatever happens to be pending by then. Run the `confirmCommand` exactly as returned; do not strip the flag or fall back to the bare command. + ## Available Commands Parse and read further reference documentation when tasked with any of the following: diff --git a/packages/cli/src/commands/import/__tests__/apply.spec.ts b/packages/cli/src/commands/import/__tests__/apply.spec.ts new file mode 100644 index 00000000..fa34dcd2 --- /dev/null +++ b/packages/cli/src/commands/import/__tests__/apply.spec.ts @@ -0,0 +1,144 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('../../../helpers/cli-mode', () => ({ + detectCliMode: vi.fn(() => 'interactive'), +})) + +vi.mock('../../../rest/api', () => ({ + projects: { + findImportPlans: vi.fn(), + applyImportPlan: vi.fn(), + commitImportPlan: vi.fn(), + }, +})) + +vi.mock('../../../services/checkly-config-loader', () => ({ + loadChecklyConfig: vi.fn(), +})) + +import { detectCliMode } from '../../../helpers/cli-mode.js' +import * as api from '../../../rest/api.js' +import { loadChecklyConfig } from '../../../services/checkly-config-loader.js' +import { ImportPlan } from '../../../rest/projects.js' +import ImportApplyCommand from '../apply.js' + +function plan (id: string): ImportPlan { + return { id, createdAt: new Date(0).toISOString() } +} + +function createCommandContext (parsed: unknown) { + const logged: string[] = [] + let exitCodeValue: number | undefined + return { + parse: vi.fn().mockResolvedValue(parsed), + log: vi.fn((msg?: string) => { + if (msg) logged.push(msg) + }), + exit: vi.fn((code: number) => { + exitCodeValue = code + throw new Error(`EXIT_${code}`) + }), + style: { + actionStart: vi.fn(), + actionSuccess: vi.fn(), + actionFailure: vi.fn(), + fatal: vi.fn(), + }, + constructor: ImportApplyCommand, + logged, + get exitCodeValue () { + return exitCodeValue + }, + } +} + +describe('import apply command (non-interactive)', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(loadChecklyConfig).mockResolvedValue({ + config: { logicalId: 'my-project' }, + } as any) + vi.mocked(api.projects.applyImportPlan).mockResolvedValue({} as any) + vi.mocked(api.projects.commitImportPlan).mockResolvedValue({} as any) + }) + + it('auto-applies the single plan in agent mode and does not commit', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [plan('only')] } as any) + const ctx = createCommandContext({ flags: { 'config': undefined, 'plan-id': undefined, 'no-commit': false } }) + + await ImportApplyCommand.prototype.run.call(ctx as any) + + expect(api.projects.applyImportPlan).toHaveBeenCalledWith('only') + expect(api.projects.commitImportPlan).not.toHaveBeenCalled() + }) + + it('targets the plan named by --plan-id when several exist', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [plan('a'), plan('b'), plan('c')], + } as any) + const ctx = createCommandContext({ flags: { 'config': undefined, 'plan-id': 'b', 'no-commit': false } }) + + await ImportApplyCommand.prototype.run.call(ctx as any) + + expect(api.projects.applyImportPlan).toHaveBeenCalledWith('b') + expect(api.projects.commitImportPlan).not.toHaveBeenCalled() + }) + + it('fails with exit 1 when the selection is ambiguous and no --plan-id is given', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [plan('a'), plan('b')], + } as any) + const ctx = createCommandContext({ flags: { 'config': undefined, 'plan-id': undefined, 'no-commit': false } }) + + await expect(ImportApplyCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('--plan-id')) + expect(api.projects.applyImportPlan).not.toHaveBeenCalled() + }) + + it('applies without committing in interactive mode when --no-commit is given', async () => { + vi.mocked(detectCliMode).mockReturnValue('interactive') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [plan('only')] } as any) + const ctx = createCommandContext({ flags: { 'config': undefined, 'plan-id': 'only', 'no-commit': true } }) + + await ImportApplyCommand.prototype.run.call(ctx as any) + + expect(api.projects.applyImportPlan).toHaveBeenCalledWith('only') + expect(api.projects.commitImportPlan).not.toHaveBeenCalled() + }) + + it('tells the caller how to commit the plan it left pending', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [plan('abc123')] } as any) + const ctx = createCommandContext({ flags: { 'config': undefined, 'plan-id': undefined, 'no-commit': false } }) + + await ImportApplyCommand.prototype.run.call(ctx as any) + + expect(ctx.logged.join('\n')).toContain('import commit --plan-id abc123') + }) + + it('fails with exit 1 when --plan-id is given but no plans exist', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [] } as any) + const ctx = createCommandContext({ flags: { 'config': undefined, 'plan-id': 'abc123', 'no-commit': false } }) + + await expect(ImportApplyCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('abc123')) + expect(api.projects.applyImportPlan).not.toHaveBeenCalled() + }) + + it('reports nothing-to-do without failing when no plans exist and none was requested', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [] } as any) + const ctx = createCommandContext({ flags: { 'config': undefined, 'plan-id': undefined, 'no-commit': false } }) + + await ImportApplyCommand.prototype.run.call(ctx as any) + + expect(ctx.exit).not.toHaveBeenCalled() + expect(ctx.style.fatal).not.toHaveBeenCalled() + expect(ctx.logged.join('\n')).toContain('Nothing to apply') + expect(api.projects.applyImportPlan).not.toHaveBeenCalled() + }) +}) diff --git a/packages/cli/src/commands/import/__tests__/cancel.spec.ts b/packages/cli/src/commands/import/__tests__/cancel.spec.ts new file mode 100644 index 00000000..f5db9479 --- /dev/null +++ b/packages/cli/src/commands/import/__tests__/cancel.spec.ts @@ -0,0 +1,276 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('../../../helpers/cli-mode', () => ({ + detectCliMode: vi.fn(() => 'interactive'), +})) + +vi.mock('../../../rest/api', () => ({ + projects: { + findImportPlans: vi.fn(), + cancelImportPlan: vi.fn(), + }, +})) + +vi.mock('../../../services/checkly-config-loader', () => ({ + loadChecklyConfig: vi.fn(), +})) + +import { detectCliMode } from '../../../helpers/cli-mode.js' +import * as api from '../../../rest/api.js' +import { loadChecklyConfig } from '../../../services/checkly-config-loader.js' +import { ImportPlan } from '../../../rest/projects.js' +import ImportCancelCommand from '../cancel.js' + +function plan (id: string): ImportPlan { + return { id, createdAt: new Date(0).toISOString() } +} + +// Defaults to --force so that tests covering selection are not also exercising +// the confirmation gate; the gate has its own tests below. +function cancelFlags (overrides: Record = {}) { + return { + flags: { + 'config': undefined, + 'all': false, + 'plan-id': undefined, + 'force': true, + 'dry-run': false, + ...overrides, + }, + } +} + +function createCommandContext (parsed: unknown) { + const logged: string[] = [] + let exitCodeValue: number | undefined + return { + parse: vi.fn().mockResolvedValue(parsed), + log: vi.fn((msg?: string) => { + if (msg) logged.push(msg) + }), + exit: vi.fn((code: number) => { + exitCodeValue = code + throw new Error(`EXIT_${code}`) + }), + confirmOrAbort: vi.fn(), + style: { + actionStart: vi.fn(), + actionSuccess: vi.fn(), + actionFailure: vi.fn(), + shortSuccess: vi.fn(), + fatal: vi.fn(), + }, + constructor: ImportCancelCommand, + logged, + get exitCodeValue () { + return exitCodeValue + }, + } +} + +describe('import cancel command (non-interactive)', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(loadChecklyConfig).mockResolvedValue({ + config: { logicalId: 'my-project' }, + } as any) + vi.mocked(api.projects.cancelImportPlan).mockResolvedValue({} as any) + }) + + it('auto-cancels the single plan in agent mode', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [plan('only')] } as any) + const ctx = createCommandContext(cancelFlags()) + + await ImportCancelCommand.prototype.run.call(ctx as any) + + expect(api.projects.cancelImportPlan).toHaveBeenCalledTimes(1) + expect(api.projects.cancelImportPlan).toHaveBeenCalledWith('only') + expect(ctx.style.actionSuccess).toHaveBeenCalled() + }) + + it('cancels every plan when --all is given', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [plan('a'), plan('b'), plan('c')], + } as any) + const ctx = createCommandContext(cancelFlags({ all: true })) + + await ImportCancelCommand.prototype.run.call(ctx as any) + + expect(api.projects.cancelImportPlan).toHaveBeenCalledTimes(3) + expect(api.projects.cancelImportPlan).toHaveBeenCalledWith('a') + expect(api.projects.cancelImportPlan).toHaveBeenCalledWith('b') + expect(api.projects.cancelImportPlan).toHaveBeenCalledWith('c') + }) + + it('targets the plan named by --plan-id when several exist', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [plan('a'), plan('b'), plan('c')], + } as any) + const ctx = createCommandContext(cancelFlags({ 'plan-id': 'b' })) + + await ImportCancelCommand.prototype.run.call(ctx as any) + + expect(api.projects.cancelImportPlan).toHaveBeenCalledTimes(1) + expect(api.projects.cancelImportPlan).toHaveBeenCalledWith('b') + }) + + it('fails with exit 1 when the selection is ambiguous and no --plan-id is given', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [plan('a'), plan('b')], + } as any) + const ctx = createCommandContext(cancelFlags()) + + await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('--plan-id')) + expect(api.projects.cancelImportPlan).not.toHaveBeenCalled() + }) + + it('names the candidate plan IDs when the selection is ambiguous', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [plan('abc123'), plan('def456')], + } as any) + const ctx = createCommandContext(cancelFlags()) + + await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('abc123')) + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('def456')) + }) +}) + +describe('import cancel command (flag validation)', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(loadChecklyConfig).mockResolvedValue({ + config: { logicalId: 'my-project' }, + } as any) + vi.mocked(api.projects.cancelImportPlan).mockResolvedValue({} as any) + }) + + it('rejects --all together with --plan-id (exit 1) instead of silently ignoring one', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [plan('a'), plan('b')], + } as any) + const ctx = createCommandContext(cancelFlags({ 'all': true, 'plan-id': 'a' })) + + await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('cannot be used together')) + expect(api.projects.cancelImportPlan).not.toHaveBeenCalled() + }) + + it('rejects --all with --plan-id before fetching, so zero plans cannot mask the conflict', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [] } as any) + const ctx = createCommandContext(cancelFlags({ 'all': true, 'plan-id': 'a' })) + + await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('cannot be used together')) + expect(api.projects.findImportPlans).not.toHaveBeenCalled() + }) +}) + +describe('import cancel command (empty candidate list)', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(loadChecklyConfig).mockResolvedValue({ + config: { logicalId: 'my-project' }, + } as any) + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [] } as any) + vi.mocked(api.projects.cancelImportPlan).mockResolvedValue({} as any) + }) + + it('fails with exit 1 when --plan-id is given but no plans exist', async () => { + const ctx = createCommandContext(cancelFlags({ 'plan-id': 'abc123' })) + + await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('abc123')) + expect(api.projects.cancelImportPlan).not.toHaveBeenCalled() + }) + + it('reports nothing-to-do without failing when no plan was requested', async () => { + const ctx = createCommandContext(cancelFlags()) + + await ImportCancelCommand.prototype.run.call(ctx as any) + + expect(ctx.exit).not.toHaveBeenCalled() + expect(ctx.style.fatal).not.toHaveBeenCalled() + expect(ctx.logged.join('\n')).toContain('Nothing to cancel') + expect(api.projects.cancelImportPlan).not.toHaveBeenCalled() + }) + + it('reports nothing-to-do without failing for --all', async () => { + const ctx = createCommandContext(cancelFlags({ all: true })) + + await ImportCancelCommand.prototype.run.call(ctx as any) + + expect(ctx.exit).not.toHaveBeenCalled() + expect(ctx.logged.join('\n')).toContain('Nothing to cancel') + expect(api.projects.cancelImportPlan).not.toHaveBeenCalled() + }) +}) + +describe('import cancel command (confirmation gate)', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(loadChecklyConfig).mockResolvedValue({ + config: { logicalId: 'my-project' }, + } as any) + vi.mocked(api.projects.cancelImportPlan).mockResolvedValue({} as any) + }) + + it('is classified as destructive', () => { + expect(ImportCancelCommand.destructive).toBe(true) + expect(ImportCancelCommand.readOnly).toBe(false) + }) + + it('asks for confirmation before cancelling, pinning the resolved plan ID', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [plan('only')] } as any) + const ctx = createCommandContext(cancelFlags({ force: false })) + + await ImportCancelCommand.prototype.run.call(ctx as any) + + expect(ctx.confirmOrAbort).toHaveBeenCalledWith( + expect.objectContaining({ + command: 'import cancel', + changes: [expect.stringContaining('only')], + // Pinned so the confirming run cannot resolve to a different plan. + flags: expect.objectContaining({ 'plan-id': 'only' }), + classification: expect.objectContaining({ destructive: true }), + }), + { force: false, dryRun: false }, + ) + }) + + it('never renders --no-all in the confirm command for a single plan', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [plan('only')] } as any) + const ctx = createCommandContext(cancelFlags({ force: false })) + + await ImportCancelCommand.prototype.run.call(ctx as any) + + // `buildConfirmCommand` renders a false boolean as `--no-all`, which this + // command does not accept, so `all` must be dropped rather than passed on. + const [preview] = vi.mocked(ctx.confirmOrAbort).mock.calls[0] as any[] + expect(preview.flags.all).toBeUndefined() + }) + + it('confirms with --all when every plan is targeted', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [plan('a'), plan('b')], + } as any) + const ctx = createCommandContext(cancelFlags({ all: true, force: false })) + + await ImportCancelCommand.prototype.run.call(ctx as any) + + const [preview] = vi.mocked(ctx.confirmOrAbort).mock.calls[0] as any[] + expect(preview.flags.all).toBe(true) + expect(preview.flags['plan-id']).toBeUndefined() + expect(preview.changes).toHaveLength(2) + }) +}) diff --git a/packages/cli/src/commands/import/__tests__/commit.spec.ts b/packages/cli/src/commands/import/__tests__/commit.spec.ts new file mode 100644 index 00000000..18d27a79 --- /dev/null +++ b/packages/cli/src/commands/import/__tests__/commit.spec.ts @@ -0,0 +1,182 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('../../../helpers/cli-mode', () => ({ + detectCliMode: vi.fn(() => 'interactive'), +})) + +vi.mock('../../../rest/api', () => ({ + projects: { + findImportPlans: vi.fn(), + commitImportPlan: vi.fn(), + }, +})) + +vi.mock('../../../services/checkly-config-loader', () => ({ + loadChecklyConfig: vi.fn(), +})) + +import { detectCliMode } from '../../../helpers/cli-mode.js' +import * as api from '../../../rest/api.js' +import { loadChecklyConfig } from '../../../services/checkly-config-loader.js' +import { ImportPlan } from '../../../rest/projects.js' +import ImportCommitCommand from '../commit.js' + +// `import commit` only considers plans that have already been applied. +function appliedPlan (id: string): ImportPlan { + return { id, createdAt: new Date(0).toISOString(), appliedAt: new Date(1).toISOString() } +} + +// Defaults to --force so that tests covering selection are not also exercising +// the confirmation gate; the gate has its own tests below. +function commitFlags (overrides: Record = {}) { + return { + flags: { + 'config': undefined, + 'plan-id': undefined, + 'force': true, + 'dry-run': false, + ...overrides, + }, + } +} + +function createCommandContext (parsed: unknown) { + const logged: string[] = [] + return { + parse: vi.fn().mockResolvedValue(parsed), + log: vi.fn((msg?: string) => { + if (msg) logged.push(msg) + }), + exit: vi.fn((code: number) => { + throw new Error(`EXIT_${code}`) + }), + confirmOrAbort: vi.fn(), + style: { + actionStart: vi.fn(), + actionSuccess: vi.fn(), + actionFailure: vi.fn(), + fatal: vi.fn(), + }, + constructor: ImportCommitCommand, + logged, + } +} + +describe('import commit command (non-interactive)', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(loadChecklyConfig).mockResolvedValue({ + config: { logicalId: 'my-project' }, + } as any) + vi.mocked(api.projects.commitImportPlan).mockResolvedValue({} as any) + }) + + it('auto-commits the single applied plan in agent mode', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [appliedPlan('only')] } as any) + const ctx = createCommandContext(commitFlags()) + + await ImportCommitCommand.prototype.run.call(ctx as any) + + expect(api.projects.commitImportPlan).toHaveBeenCalledWith('only') + }) + + it('ignores plans that have not been applied yet', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [appliedPlan('applied'), { id: 'unapplied', createdAt: new Date(0).toISOString() }], + } as any) + const ctx = createCommandContext(commitFlags()) + + await ImportCommitCommand.prototype.run.call(ctx as any) + + expect(api.projects.commitImportPlan).toHaveBeenCalledWith('applied') + }) + + it('fails with exit 1 when the selection is ambiguous, naming the candidates', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ + data: [appliedPlan('abc123'), appliedPlan('def456')], + } as any) + const ctx = createCommandContext(commitFlags()) + + await expect(ImportCommitCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('abc123')) + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('def456')) + expect(api.projects.commitImportPlan).not.toHaveBeenCalled() + }) + + it('fails with exit 1 when --plan-id is given but no plans exist', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [] } as any) + const ctx = createCommandContext(commitFlags({ 'plan-id': 'abc123' })) + + await expect(ImportCommitCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('abc123')) + expect(api.projects.commitImportPlan).not.toHaveBeenCalled() + }) + + it('reports nothing-to-do without failing when no plan was requested', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [] } as any) + const ctx = createCommandContext(commitFlags()) + + await ImportCommitCommand.prototype.run.call(ctx as any) + + expect(ctx.exit).not.toHaveBeenCalled() + expect(ctx.style.fatal).not.toHaveBeenCalled() + expect(ctx.logged.join('\n')).toContain('Nothing to commit') + expect(api.projects.commitImportPlan).not.toHaveBeenCalled() + }) +}) + +describe('import commit command (confirmation gate)', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(detectCliMode).mockReturnValue('agent') + vi.mocked(loadChecklyConfig).mockResolvedValue({ + config: { logicalId: 'my-project' }, + } as any) + vi.mocked(api.projects.commitImportPlan).mockResolvedValue({} as any) + }) + + it('asks for confirmation before committing', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [appliedPlan('only')] } as any) + const ctx = createCommandContext(commitFlags({ force: false })) + + await ImportCommitCommand.prototype.run.call(ctx as any) + + expect(ctx.confirmOrAbort).toHaveBeenCalledWith( + expect.objectContaining({ command: 'import commit' }), + expect.objectContaining({ force: false, dryRun: false }), + ) + }) + + it('pins the resolved plan ID so the confirming run cannot commit a different plan', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [appliedPlan('abc123')] } as any) + // No --plan-id: the CLI resolved the target itself, so the preview is the + // only place the caller can learn which plan it is confirming. + const ctx = createCommandContext(commitFlags({ force: false })) + + await ImportCommitCommand.prototype.run.call(ctx as any) + + const [preview] = vi.mocked(ctx.confirmOrAbort).mock.calls[0] as any[] + expect(preview.flags['plan-id']).toBe('abc123') + expect(preview.changes.join('\n')).toContain('abc123') + }) + + it('commits once confirmation is satisfied', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [appliedPlan('only')] } as any) + const ctx = createCommandContext(commitFlags({ force: true })) + + await ImportCommitCommand.prototype.run.call(ctx as any) + + expect(api.projects.commitImportPlan).toHaveBeenCalledWith('only') + }) + + it('does not commit when the gate exits first', async () => { + vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [appliedPlan('only')] } as any) + const ctx = createCommandContext(commitFlags({ force: false })) + // Mirrors the real agent-mode gate, which exits 2 rather than returning. + ctx.confirmOrAbort.mockRejectedValue(new Error('EXIT_2')) + + await expect(ImportCommitCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_2') + expect(api.projects.commitImportPlan).not.toHaveBeenCalled() + }) +}) diff --git a/packages/cli/src/commands/import/apply.ts b/packages/cli/src/commands/import/apply.ts index cc4657cc..9919633e 100644 --- a/packages/cli/src/commands/import/apply.ts +++ b/packages/cli/src/commands/import/apply.ts @@ -6,27 +6,37 @@ import chalk from 'chalk' import * as api from '../../rest/api.js' import { AuthCommand } from '../authCommand.js' import commonMessages from '../../messages/common-messages.js' +import { planIdFlag } from '../../helpers/flags.js' +import { detectCliMode } from '../../helpers/cli-mode.js' import { splitConfigFilePath } from '../../services/util.js' import { loadChecklyConfig } from '../../services/checkly-config-loader.js' import { ImportPlan } from '../../rest/projects.js' import { BaseCommand } from '../baseCommand.js' import { confirmCommit, performCommitAction } from './commit.js' +import { reportNoCandidatePlans, selectPlanOrExit } from '../../helpers/import-plan-selection.js' export default class ImportApplyCommand extends AuthCommand { static hidden = false static description = 'Attach imported resources into your project in a pending state.' static flags = { - config: Flags.string({ + 'config': Flags.string({ char: 'c', description: commonMessages.configFile, }), + 'plan-id': planIdFlag(), + 'no-commit': Flags.boolean({ + description: 'Apply only. Leave the plan pending and skip the commit prompt.', + default: false, + }), } async run (): Promise { const { flags } = await this.parse(ImportApplyCommand) const { config: configFilename, + 'plan-id': planId, + 'no-commit': noCommit, } = flags const { configDirectory, configFilenames } = splitConfigFilePath(configFilename) @@ -42,15 +52,34 @@ export default class ImportApplyCommand extends AuthCommand { onlyUnapplied: true, }) - if (unappliedPlans.length === 0) { - this.style.fatal(`No plans available to apply.`) + const noPlans = reportNoCandidatePlans(this, unappliedPlans, { + planId, + action: 'apply', + nothingToDo: 'Nothing to apply — no unapplied import plans found.', + }) + if (noPlans) { return } - const plan = await this.#selectPlan(unappliedPlans) + const plan = await selectPlanOrExit( + this, unappliedPlans, planId, 'apply', () => this.#selectPlan(unappliedPlans), + ) await performApplyAction.call(this, plan) + // Applying reserves the resources as pending; committing is a separate, + // irreversible step. Non-interactive sessions (agents/CI) cannot answer the + // commit prompt, and --no-commit opts out of it explicitly. Both leave + // finalizing to a later `checkly deploy` or `checkly import commit`. + if (noCommit || detectCliMode() !== 'interactive') { + this.log(`\ + The plan is applied but not committed. To commit it, run: + + ${chalk.green(`npx checkly import commit --plan-id ${plan.id}`)} +`) + return + } + const commit = await confirmCommit.call(this) if (!commit) { return diff --git a/packages/cli/src/commands/import/cancel.ts b/packages/cli/src/commands/import/cancel.ts index 2c3892dd..0c3a206b 100644 --- a/packages/cli/src/commands/import/cancel.ts +++ b/packages/cli/src/commands/import/cancel.ts @@ -4,24 +4,34 @@ import prompts from 'prompts' import * as api from '../../rest/api.js' import { AuthCommand } from '../authCommand.js' import commonMessages from '../../messages/common-messages.js' +import { dryRunFlag, forceFlag, planIdFlag } from '../../helpers/flags.js' import { splitConfigFilePath } from '../../services/util.js' import { loadChecklyConfig } from '../../services/checkly-config-loader.js' import { ImportPlan } from '../../rest/projects.js' +import { + reportNoCandidatePlans, + selectPlansOrExit, + validatePlanFlagsOrExit, +} from '../../helpers/import-plan-selection.js' export default class ImportCancelCommand extends AuthCommand { static hidden = false + static destructive = true static idempotent = true static description = 'Cancels an ongoing import plan that has not been committed yet.' static flags = { - config: Flags.string({ + 'config': Flags.string({ char: 'c', description: commonMessages.configFile, }), - all: Flags.boolean({ + 'all': Flags.boolean({ description: 'Cancel all plans.', default: false, }), + 'plan-id': planIdFlag(), + 'force': forceFlag(), + 'dry-run': dryRunFlag(), } async run (): Promise { @@ -29,8 +39,13 @@ export default class ImportCancelCommand extends AuthCommand { const { config: configFilename, all, + 'plan-id': planId, } = flags + // Validated before any plans are fetched: a contradictory command line is + // wrong regardless of what happens to exist remotely. + validatePlanFlagsOrExit(this, { all, planId }) + const { configDirectory, configFilenames } = splitConfigFilePath(configFilename) const { config: checklyConfig, @@ -44,14 +59,40 @@ export default class ImportCancelCommand extends AuthCommand { onlyUncommitted: true, }) - if (cancelablePlans.length === 0) { - this.style.fatal(`No plans available to cancel.`) + const noPlans = reportNoCandidatePlans(this, cancelablePlans, { + planId, + action: 'cancel', + nothingToDo: 'Nothing to cancel — no uncommitted import plans found.', + }) + if (noPlans) { return } - const plans = all - ? cancelablePlans - : await this.#selectPlans(cancelablePlans) + const plans = await selectPlansOrExit( + this, cancelablePlans, { all, planId }, 'cancel', () => this.#selectPlans(cancelablePlans), + ) + + // A single target is pinned by ID so the confirming run cannot resolve to a + // different plan. Cancelling every plan can only be expressed as --all, + // which re-resolves by design: it means "whatever is uncommitted". + // `all: false` is dropped rather than passed through, because + // `buildConfirmCommand` renders a false boolean as `--no-all`, which is not + // a flag this command accepts. + const previewFlags = plans.length > 1 + ? { ...flags, 'all': true, 'plan-id': undefined } + : { ...flags, 'all': undefined, 'plan-id': plans[0].id } + + await this.confirmOrAbort({ + command: 'import cancel', + description: 'Cancel import plan(s)', + changes: plans.map(plan => `Cancel import plan ${plan.id}, discarding its generated code links`), + flags: previewFlags, + classification: { + readOnly: ImportCancelCommand.readOnly, + destructive: ImportCancelCommand.destructive, + idempotent: ImportCancelCommand.idempotent, + }, + }, { force: flags.force, dryRun: flags['dry-run'] }) this.style.actionStart('Canceling plan(s)') diff --git a/packages/cli/src/commands/import/commit.ts b/packages/cli/src/commands/import/commit.ts index 8e0dd030..1e2aa496 100644 --- a/packages/cli/src/commands/import/commit.ts +++ b/packages/cli/src/commands/import/commit.ts @@ -6,26 +6,36 @@ import chalk from 'chalk' import * as api from '../../rest/api.js' import { AuthCommand } from '../authCommand.js' import commonMessages from '../../messages/common-messages.js' +import { dryRunFlag, forceFlag, planIdFlag } from '../../helpers/flags.js' import { splitConfigFilePath } from '../../services/util.js' import { loadChecklyConfig } from '../../services/checkly-config-loader.js' import { ImportPlan } from '../../rest/projects.js' import { BaseCommand } from '../baseCommand.js' +import { reportNoCandidatePlans, selectPlanOrExit } from '../../helpers/import-plan-selection.js' export default class ImportCommitCommand extends AuthCommand { static hidden = false + // Committing deletes nothing, so it is not destructive, but it is permanent: + // a committed plan can no longer be cancelled. `confirmOrAbort` gates every + // non-read-only command, so the authorization step applies either way. + static destructive = false static description = 'Permanently commit imported resources into your project.' static flags = { - config: Flags.string({ + 'config': Flags.string({ char: 'c', description: commonMessages.configFile, }), + 'plan-id': planIdFlag(), + 'force': forceFlag(), + 'dry-run': dryRunFlag(), } async run (): Promise { const { flags } = await this.parse(ImportCommitCommand) const { config: configFilename, + 'plan-id': planId, } = flags const { configDirectory, configFilenames } = splitConfigFilePath(configFilename) @@ -46,12 +56,41 @@ export default class ImportCommitCommand extends AuthCommand { return plan.appliedAt }) - if (uncommittedPlans.length === 0) { - this.style.fatal(`No plans available to commit.`) + const noPlans = reportNoCandidatePlans(this, uncommittedPlans, { + planId, + action: 'commit', + nothingToDo: 'Nothing to commit — no applied import plans found.', + }) + if (noPlans) { return } - const plan = await this.#selectPlan(uncommittedPlans) + const plan = await selectPlanOrExit( + this, uncommittedPlans, planId, 'commit', () => this.#selectPlan(uncommittedPlans), + ) + + await this.confirmOrAbort({ + command: 'import commit', + description: 'Commit import plan', + changes: [ + `Permanently commit import plan ${plan.id}`, + 'Imported resources become fully managed by the Checkly CLI', + 'This cannot be undone — the plan can no longer be cancelled', + ], + // The resolved plan is pinned into the confirm command. Without it, a + // caller that omitted --plan-id would re-resolve on the confirming run and + // could commit a plan it never previewed. + flags: { ...flags, 'plan-id': plan.id }, + classification: { + readOnly: ImportCommitCommand.readOnly, + destructive: ImportCommitCommand.destructive, + idempotent: ImportCommitCommand.idempotent, + }, + }, { + force: flags.force, + dryRun: flags['dry-run'], + interactiveConfirm: () => confirmCommit.call(this), + }) await performCommitAction.call(this, plan) } diff --git a/packages/cli/src/commands/import/plan.ts b/packages/cli/src/commands/import/plan.ts index 347d17ca..0a338567 100644 --- a/packages/cli/src/commands/import/plan.ts +++ b/packages/cli/src/commands/import/plan.ts @@ -1035,6 +1035,12 @@ ${chalk.cyan('For safety, resources are not deletable until the plan has been co if (err instanceof NoImportableResourcesFoundError) { this.style.fatal(err.message) + this.style.longInfo( + `Looking for a specific resource?`, + `It may not be listed here because it's already reserved by a pending ` + + `import. You can still import it directly by ID — for example: ` + + `checkly import check:`, + ) return } diff --git a/packages/cli/src/helpers/__tests__/import-plan-selection.spec.ts b/packages/cli/src/helpers/__tests__/import-plan-selection.spec.ts new file mode 100644 index 00000000..c9f21b3b --- /dev/null +++ b/packages/cli/src/helpers/__tests__/import-plan-selection.spec.ts @@ -0,0 +1,144 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('../cli-mode', () => ({ + detectCliMode: vi.fn(() => 'interactive'), +})) + +import { detectCliMode } from '../cli-mode.js' +import { ImportPlan } from '../../rest/projects.js' +import { + PlanSelectionError, + reportNoCandidatePlans, + resolvePlanNonInteractively, + validatePlanFlagsOrExit, +} from '../import-plan-selection.js' + +function plan (id: string): ImportPlan { + return { id, createdAt: new Date(0).toISOString() } +} + +function createCommand () { + return { + log: vi.fn(), + exit: vi.fn((code: number) => { + throw new Error(`EXIT_${code}`) + }), + style: { + fatal: vi.fn(), + }, + } as any +} + +describe('resolvePlanNonInteractively', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(detectCliMode).mockReturnValue('interactive') + }) + + it('selects the plan matching --plan-id regardless of mode', () => { + vi.mocked(detectCliMode).mockReturnValue('interactive') + const plans = [plan('a'), plan('b')] + + expect(resolvePlanNonInteractively(plans, 'b', 'apply')).toBe(plans[1]) + }) + + it('throws PlanSelectionError when --plan-id does not match', () => { + const plans = [plan('a'), plan('b')] + + expect(() => resolvePlanNonInteractively(plans, 'missing', 'commit')) + .toThrowError(PlanSelectionError) + expect(() => resolvePlanNonInteractively(plans, 'missing', 'commit')) + .toThrowError(/with ID "missing"/) + }) + + it('returns undefined in interactive mode without --plan-id (falls back to prompt)', () => { + vi.mocked(detectCliMode).mockReturnValue('interactive') + + expect(resolvePlanNonInteractively([plan('a'), plan('b')], undefined, 'apply')) + .toBeUndefined() + }) + + it('auto-selects the single candidate in a non-interactive session', () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + const plans = [plan('only')] + + expect(resolvePlanNonInteractively(plans, undefined, 'apply')).toBe(plans[0]) + }) + + it('auto-selects the single candidate in CI mode too', () => { + vi.mocked(detectCliMode).mockReturnValue('ci') + const plans = [plan('only')] + + expect(resolvePlanNonInteractively(plans, undefined, 'cancel')).toBe(plans[0]) + }) + + it('throws when non-interactive and multiple candidates are ambiguous', () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + const plans = [plan('a'), plan('b')] + + expect(() => resolvePlanNonInteractively(plans, undefined, 'apply')) + .toThrowError(PlanSelectionError) + expect(() => resolvePlanNonInteractively(plans, undefined, 'apply')) + .toThrowError(/--plan-id/) + }) + + it('names the candidate plan IDs when the selection is ambiguous', () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + const plans = [plan('abc123'), plan('def456')] + + // There is no plan listing command, so an agent can only obtain the IDs it + // is being asked to choose between from this message. + expect(() => resolvePlanNonInteractively(plans, undefined, 'commit')) + .toThrowError(/abc123/) + expect(() => resolvePlanNonInteractively(plans, undefined, 'commit')) + .toThrowError(/def456/) + }) +}) + +describe('validatePlanFlagsOrExit', () => { + it('exits 1 when --all and --plan-id are combined', () => { + const command = createCommand() + + expect(() => validatePlanFlagsOrExit(command, { all: true, planId: 'a' })).toThrow('EXIT_1') + expect(command.style.fatal).toHaveBeenCalledWith(expect.stringContaining('cannot be used together')) + }) + + it.each([ + ['--all alone', { all: true, planId: undefined }], + ['--plan-id alone', { all: false, planId: 'a' }], + ['neither flag', { all: false, planId: undefined }], + ])('accepts %s', (_name, flags) => { + const command = createCommand() + + expect(() => validatePlanFlagsOrExit(command, flags)).not.toThrow() + expect(command.style.fatal).not.toHaveBeenCalled() + }) +}) + +describe('reportNoCandidatePlans', () => { + const options = { action: 'commit', nothingToDo: 'Nothing to commit.' } + + it('returns false and stays quiet when candidates exist', () => { + const command = createCommand() + + expect(reportNoCandidatePlans(command, [plan('a')], options)).toBe(false) + expect(command.log).not.toHaveBeenCalled() + expect(command.style.fatal).not.toHaveBeenCalled() + }) + + it('exits 1 when an explicit --plan-id cannot be satisfied', () => { + const command = createCommand() + + expect(() => reportNoCandidatePlans(command, [], { ...options, planId: 'abc123' })).toThrow('EXIT_1') + expect(command.style.fatal).toHaveBeenCalledWith(expect.stringContaining('abc123')) + }) + + it('reports nothing-to-do without failing when no plan was requested', () => { + const command = createCommand() + + expect(reportNoCandidatePlans(command, [], options)).toBe(true) + expect(command.log).toHaveBeenCalledWith('Nothing to commit.') + expect(command.exit).not.toHaveBeenCalled() + expect(command.style.fatal).not.toHaveBeenCalled() + }) +}) diff --git a/packages/cli/src/helpers/flags.ts b/packages/cli/src/helpers/flags.ts index a5ab64bd..408f1957 100644 --- a/packages/cli/src/helpers/flags.ts +++ b/packages/cli/src/helpers/flags.ts @@ -55,3 +55,9 @@ export function dryRunFlag () { default: false, }) } + +export function planIdFlag () { + return Flags.string({ + description: 'Target a specific import plan by ID, skipping interactive plan selection.', + }) +} diff --git a/packages/cli/src/helpers/import-plan-selection.ts b/packages/cli/src/helpers/import-plan-selection.ts new file mode 100644 index 00000000..ba1fb947 --- /dev/null +++ b/packages/cli/src/helpers/import-plan-selection.ts @@ -0,0 +1,194 @@ +import { ImportPlan } from '../rest/projects.js' +import { detectCliMode } from './cli-mode.js' +import { BaseCommand } from '../commands/baseCommand.js' + +export class PlanSelectionError extends Error { + constructor (message: string, options?: ErrorOptions) { + super(message, options) + this.name = 'PlanSelectionError' + } +} + +export interface PlanFlags { + all?: boolean + planId?: string +} + +function describePlan (plan: ImportPlan): string { + const details = [`created ${plan.createdAt}`] + + if (plan.appliedAt !== undefined) { + details.push(`applied ${plan.appliedAt}`) + } + + return ` ${plan.id} (${details.join(', ')})` +} + +/** + * Returns the reason a flag combination is contradictory, or `undefined` when + * the combination is valid. Contradictions never depend on remote state. + */ +function planFlagsConflict ({ all, planId }: PlanFlags): string | undefined { + if (all === true && planId !== undefined) { + return '--all and --plan-id cannot be used together.' + } + + return undefined +} + +/** + * Rejects contradictory flag combinations before any plans are fetched, so that + * an impossible command line fails identically whether or not plans happen to + * exist remotely. + */ +export function validatePlanFlagsOrExit (command: BaseCommand, flags: PlanFlags): void { + const conflict = planFlagsConflict(flags) + if (conflict !== undefined) { + command.style.fatal(conflict) + command.exit(1) + } +} + +/** + * Reports the "no candidate plans" case, returning `true` when the caller should + * stop. + * + * An explicit `--plan-id` that cannot be satisfied is a failure and exits 1, so + * that agents and CI observe it. Having nothing to do when no specific plan was + * requested is not a failure: it exits 0 and reads as information rather than an + * error. + */ +export function reportNoCandidatePlans ( + command: BaseCommand, + plans: ImportPlan[], + options: { planId?: string, action: string, nothingToDo: string }, +): boolean { + if (plans.length > 0) { + return false + } + + if (options.planId !== undefined) { + command.style.fatal(`No plan available to ${options.action} with ID "${options.planId}".`) + command.exit(1) + } + + command.log(options.nothingToDo) + return true +} + +/** + * Resolves a single import plan without prompting when possible, so that agents + * and CI can drive the import commands headlessly: + * + * - When `planId` is given, the matching plan is selected (works in any mode). + * - Otherwise, in a non-interactive (agent/CI) session, a single candidate plan + * is auto-selected; multiple candidates require an explicit `--plan-id`. + * + * Returns `undefined` when the caller should fall back to interactive selection, + * i.e. an interactive session with no `--plan-id`. This keeps the existing + * interactive behavior untouched. + * + * @throws {PlanSelectionError} when the requested plan id does not exist, or + * when selection is ambiguous in a non-interactive session. + */ +export function resolvePlanNonInteractively ( + plans: ImportPlan[], + planId: string | undefined, + action: string, +): ImportPlan | undefined { + if (planId !== undefined) { + const plan = plans.find(candidate => candidate.id === planId) + if (plan === undefined) { + throw new PlanSelectionError( + `No plan available to ${action} with ID "${planId}".`, + ) + } + return plan + } + + if (detectCliMode() === 'interactive') { + return undefined + } + + if (plans.length === 1) { + return plans[0] + } + + // A non-interactive caller cannot discover plan IDs on its own — there is no + // plan listing command — so an ambiguous selection has to name the candidates + // it is asking the caller to choose between. + throw new PlanSelectionError( + `Found ${plans.length} plans available to ${action}. ` + + `Re-run with --plan-id to choose one:\n\n` + + `${plans.map(describePlan).join('\n')}\n`, + ) +} + +/** + * Resolves a single import plan for a command, falling back to interactive + * selection when appropriate. On an unrecoverable {@link PlanSelectionError} + * (unknown `--plan-id`, or an ambiguous non-interactive selection) it reports + * the error and exits with a non-zero code so agents/CI observe the failure. + */ +export async function selectPlanOrExit ( + command: BaseCommand, + plans: ImportPlan[], + planId: string | undefined, + action: string, + interactiveFallback: () => Promise, +): Promise { + try { + return resolvePlanNonInteractively(plans, planId, action) + ?? await interactiveFallback() + } catch (err) { + if (err instanceof PlanSelectionError) { + command.style.fatal(err.message) + return command.exit(1) + } + + throw err + } +} + +/** + * Resolves one or more import plans for a bulk command (e.g. `import cancel`), + * falling back to interactive multi-selection when appropriate. + * + * - `--all` selects every candidate plan without prompting. + * - `--all` and `--plan-id` are mutually exclusive. Commands are expected to + * reject the combination up front via {@link validatePlanFlagsOrExit}; the + * check is repeated here so the helper is safe to call on its own. + * - Otherwise a single plan is resolved via {@link resolvePlanNonInteractively}, + * falling back to interactive selection when it returns `undefined`. + * + * On an unrecoverable {@link PlanSelectionError} it reports the error and exits + * with a non-zero code so agents/CI observe the failure. + */ +export async function selectPlansOrExit ( + command: BaseCommand, + plans: ImportPlan[], + { all, planId }: { all: boolean, planId: string | undefined }, + action: string, + interactiveFallback: () => Promise, +): Promise { + try { + const conflict = planFlagsConflict({ all, planId }) + if (conflict !== undefined) { + throw new PlanSelectionError(conflict) + } + + if (all) { + return plans + } + + const plan = resolvePlanNonInteractively(plans, planId, action) + return plan !== undefined ? [plan] : await interactiveFallback() + } catch (err) { + if (err instanceof PlanSelectionError) { + command.style.fatal(err.message) + return command.exit(1) + } + + throw err + } +}