refactor: consolidate confirmOrAbort across deploy, destroy, and env rm#1269
refactor: consolidate confirmOrAbort across deploy, destroy, and env rm#1269thebiglabasky merged 13 commits intomainfrom
Conversation
Extends confirmOrAbort with an optional interactiveConfirm callback for commands that need custom confirmation in interactive mode (e.g. destroy's type-the-project-name prompt). Makes dryRun optional so commands without --dry-run can omit it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces ad-hoc prompts() confirmation with confirmOrAbort so agent and CI modes get structured JSON output (exit code 2) instead of a silent prompt. No new flags added. --force/-f preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces ad-hoc text-match confirmation with confirmOrAbort. In interactive mode, the type-the-project-name safeguard is preserved via the interactiveConfirm callback. Agent/CI mode gets structured JSON output (exit code 2). No new flags added. --force/-f preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces ad-hoc prompts() confirmation with confirmOrAbort so agent and CI modes get structured JSON output (exit code 2). --preview flag is untouched (server-side dry run). --force/-f preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Restore mismatch feedback message in destroy's interactiveConfirm - Make deploy preview wording more conversational - Skip undefined/null flag values in buildConfirmCommand to avoid --config="undefined" in agent confirmCommand output Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MichaelHogers
left a comment
MichaelHogers
left a comment
There was a problem hiding this comment.
letting Claude test out the commands for a bit, might be great to get another review in from you @sorccu since these are some of the core CLI commands
| expect(api.validateAuthentication).not.toHaveBeenCalled() | ||
|
|
||
| releaseBaseInit() | ||
| await expect(initPromise).rejects.toThrow('Cannot write private member #account') |
There was a problem hiding this comment.
Seems like a pretty weird thing to test for. I assume you don't actually care about what the result of the initPromise is as long as it resolves somehow? If that's the case, I'd appreciate a comment making that clearer.
MichaelHogers
left a comment
MichaelHogers
left a comment
There was a problem hiding this comment.
no findings from my side, but would require final approval from @sorccu
| if (detectCliMode() === 'interactive') { | ||
| this.log(`\nNotice: replacing version '${version}' with latest '${packageInformation.version}'. If you wish to test with a different version, please pass the CHECKLY_CLI_VERSION environment variable.\n`) | ||
| } |
There was a problem hiding this comment.
We can get rid of this message entirely. It only causes confusion.
packages/cli/src/commands/destroy.ts
Outdated
| description: 'Destroy all project resources', | ||
| changes: [ | ||
| `PERMANENTLY delete ALL resources associated with the project "${checklyConfig.projectName}"`, | ||
| `Account: "${account.name}"`, |
There was a problem hiding this comment.
This is a bit weird. This is how it looks like:
This will:
- PERMANENTLY delete ALL resources associated with the project "temp-project-0E6CA056-1778-4577-8B8E-BA38470E689C"
- Account: "simo@checklyhq.com"
? Type the project name "temp-project-0E6CA056-1778-4577-8B8E-BA38470E689C" to confirm: ›
Why is the account its own list item? It feels very out of place. It can even feel like it's saying the account will be deleted.
For reference, when you deploy, it's on the first line:
This will:
- Deploy project "temp-project-0E6CA056-1778-4577-8B8E-BA38470E689C" to account "simo@checklyhq.com"
- Schedule checks after deploy
? Proceed? › (y/N)
|
Left a couple comments. Nothing major. Up to you whether to address them or not. Seems good to me overall. |
|
@sorccu @MichaelHogers PTAL. Re-tested with Simo's suggestions which I think makes this much nicer. I'm pretty confident we can ship this |
3 participants
Summary
Consolidates write-command confirmation around
confirmOrAbortfordeploy,destroy, andenv rm, and fixes the follow-up issues found while manually validating the branch in both interactive and agent/CI-style flows.authCommand.ts—confirmOrAbortsupports an optionalinteractiveConfirmcallback for commands needing custom confirmation in interactive mode.env rm— usesconfirmOrAbortinstead of ad-hocprompts()confirmation.destroy— usesconfirmOrAbortplusinteractiveConfirm, preserving the type-the-project-name safeguard for interactive users while returning structured confirmation JSON in agent/CI mode.deploy— usesconfirmOrAbortand now exits for confirmation before parsing, bundling, or uploading anything in agent/CI mode. This keeps confirmation output machine-readable and avoids pre-confirm side effects.baseCommand.ts/authCommand.ts— local-dev version replacement notices are suppressed outside interactive mode, andAuthCommand.init()now awaitsBaseCommand.init()so interactive notices appear in the correct order.api.spec.ts— CLI-mode detection tests now clearCODEX_*and related env vars so the suite is stable when run from Codex/Desktop environments.No breaking changes
--force/-fpreserved on all three commands2and a structuredconfirmation_requiredpayloaddeploy --previewremains a server-side dry run and does not use the confirmation gateCloses AI-163
Test plan
interactiveConfirmcallback inconfirm-or-abort.spec.tsenv rm,destroy, anddeploydeployexits for confirmation before parsing/bundling in agent modeAuthCommand.init()awaitsBaseCommand.init()npm run preparesucceedschecklyworkspace: 709 passed, 2 skippedcreate-checklyworkspace: 19 passedManual validation notes
CI=1 checkly deployemitted progress output before the confirmation JSON envelope.