Confirm before deleting a source; --yes skips for scripts - #139
Confirm before deleting a source; --yes skips for scripts#139grimicorn-agent wants to merge 6 commits into
Conversation
…e, pure validator
Independent code review trailRan the independent reviewer (Opus, fresh context, diff on stdin) across four passes; every finding was addressed. Summary per round: Round 1
Round 2
Round 3
Round 4
Skipped (intentional, with reason)
Final state: |
Resolve conflicts combining delete-confirmation feature with rotate-secret (added on main): imports, USAGE, handler map, README, and test mocks now carry both. Review-loop follow-ups: non-zero exit on failed delete, consistent empty-string-uuid picker fallthrough, clearer non-match note.
Merge + code-review trail (agent)Merged
No work discarded from either side. After resolving: lint clean, typecheck clean, 795 tests pass, build passes. Independent review loop (Opus, 3 rounds)Round 1 — fixed:
Round 2 — clean: the one finding (NO_MATCH ambiguity) was already resolved by the round-1 reword; reviewer confirmed everything else checks out. Round 3 — fixed:
Skipped (out of scope for a merge-resolution PR): style/comment-volume findings on already-reviewed feature code (inline the |
What changed
markpost sources delete <uuid>previously issued the DELETE the moment a uuid was supplied — no confirmation. Deleting a source is irreversible (it drops the ingest config and the one-time signing secret, which can never be retrieved). This adds a yes/no confirmation before the delete, matching how the rest of the CLI makes destructive actions deliberate.sources delete, defaulting to no (a bare Enter cancels), using@inquirer/prompts'confirm— mocked in tests like the existinginput/select, so the delete flow stays unit-testable.Source; the direct-uuid path looks it up (best-effort, likesources updatedoes) so a wrong-but-valid copy-pasted uuid shows its real name before it destroys anything. The lookup is cosmetic — any failure (including a timeout, whichfetchSourcesre-throws) falls back to the bare uuid rather than blocking a delete that would otherwise succeed, and a failed load is labelled distinctly from a confirmed non-match so an outage is never mis-reported as "no such source".--yesflag skips the prompt for scripts. It requires an explicit uuid (without one the picker would still open and block a script) and is rejected on non-delete subcommands, mirroring the existing--json"reject where it does nothing" guard.sources deletewithout--yesneeds a TTY on both stdin and stdout (inquirer reads stdin and renders the prompt to stdout). Without one it fails loudly pointing at--yes, instead of inquirer's stdin-EOF abort being swallowed as a Ctrl+C — which would delete nothing yet exit 0, or hang.The
--yesescape hatch decisionRather than a bare
--force, the flag is--yesand is deliberately narrow: it only skips the delete confirmation, only with an explicit uuid, and only ondelete. Scripts usemarkpost sources delete <uuid> --yes; interactive users get the prompt. This keeps the "irreversible actions are hard to trigger" property while giving automation a clean, non-hanging path.Notes
--yesusage.deletegets the TTY guard;create/updatealso prompt without a TTY, but that predates this change and is out of scope for Confirm beforesources delete <uuid>#135.--yesskips the prompt (and the label lookup),--yesrejected without a uuid / on non-delete subcommands, non-TTY stdin and redirected stdout both fail loud, Ctrl+C at the prompt is a clean exit, the label names the source (interactive + direct-uuid), a lookup miss vs a lookup failure are distinct, and a timeout doesn't abort the delete.Lint, typecheck, build, and the full test suite (722 tests) are green.
Closes #135
Follow-up suggestions
Guard create/update against non-interactive terminals—sources create(always prompts) andsources updatewith no uuid (opens a picker) hang or abort under pipes/cron the same way delete did before this PR; extend the same both-streams TTY guard (or an equivalent) to them (suggested: P3, effort: S, evidence: src/commands/sources.ts usageErrorFor / createSourceCommand / updateSourceCommand)