Conversation
nhopeatall
approved these changes
Mar 16, 2026
Collaborator
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Clean, well-scoped implementation that replaces the hard-error confirmation pattern with an interactive prompt across all 4 targeted delete commands. The confirm() utility is well-designed, tests are thorough, and CI is green. LGTM.
Notes
- The
confirm()utility correctly handles all three cases:--yesflag bypass, non-TTY auto-accept, and interactive TTY prompt. - Using
process.exit(1)directly (rather than oclif'sthis.error()) is the right call here sinceconfirm()is a standalone utility without access to theCommandinstance. - The non-TTY auto-accept behavior is the correct design for CI/scripting compatibility and matches the acceptance criteria.
- Test coverage is solid — 11 tests covering all branches including edge cases (empty input, case insensitivity, readline cleanup).
- The existing
integration-credentials.test.tsupdate correctly reflects the new behavior in non-TTY test environments.
Minor observation (not blocking): Two other destructive commands (definitions delete and definitions reset) still use the old this.error('Pass --yes to confirm...') pattern. These are explicitly out of scope per the card, but could be migrated in a follow-up for consistency.
🕵️ claude-code · claude-opus-4-6 · run details
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
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
src/cli/dashboard/_shared/confirm.tswith a reusableconfirm(message, skipFlag)utility that reads y/n from stdinthis.error('Pass --yes to confirm deletion.')) with the interactive prompt in all 4 destructive commands:projects delete,users delete,agents delete,projects credentials-deleteWhat was implemented
confirm()utility: Uses Node's built-inreadlinemodule — no new dependencies--yesflag is passedprocess.stdin.isTTY === undefinedorfalse) for CI/script compatibility<message> [y/N]:prompt and reads user input; exits with code 1 on anything other thany/Yawait confirm('<what will be deleted>?', flags.yes)instead of erroringintegration-credentials.test.ts"rejects without --yes flag" updated to reflect new non-TTY auto-accept behaviortests/unit/cli/dashboard/confirm.test.tscovering: yes-flag bypass, non-TTY auto-accept (undefined and false), TTY prompts (y/Y acceptance, n/empty/non-y rejection, prompt format, readline cleanup)Test plan
Card
https://trello.com/c/0m5h19yI/442-as-a-user-i-want-interactive-confirmations-for-destructive-commands-so-that-i-dont-accidentally-delete-resources
🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details