diff --git a/README.md b/README.md index c14dc90..8aec2fc 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ server-side records. | `markpost sync [--dry-run]` | Fetch all pending records, write each to a markdown file, and (when `autoDelete` is enabled) delete the written records from the server. `--dry-run` reports the exact write/delete plan without writing or mutating anything | | `markpost push ` | Create records from one or more markdown files, directories, or glob patterns | | `markpost get [--json]` | Fetch and display a single record; pass `--json` for machine-readable output | -| `markpost sources [uuid]` | Manage sources; `sources list --json` prints machine-readable output. `rotate-secret [uuid]` mints/replaces the signing secret of a provider source (github/zapier/shortcuts reveal a fresh secret once; stripe prompts for the new value) | +| `markpost sources [uuid] [--yes]` | Manage sources; `sources list --json` prints machine-readable output. `sources delete` asks to confirm first (deleting a source is irreversible — it drops the ingest config and one-time signing secret) and needs an interactive terminal; in scripts pass a uuid with `--yes` (`sources delete --yes`) to skip the prompt. `rotate-secret [uuid]` mints/replaces the signing secret of a provider source (github/zapier/shortcuts reveal a fresh secret once; stripe prompts for the new value) | | `markpost records list [--source ] [--status ] [--search ] [--json]` | List records without deleting them, optionally filtered by source, status, or search text; pass `--json` for machine-readable output | | `markpost config [key] [value]` | View or change the stored API token and output directory | | `markpost settings [key=value ...]` | View or change server-side sync settings (`autoSync`, `autoDelete`, `frontmatter`, `conflictStrategy`) | diff --git a/src/commands/sources.ts b/src/commands/sources.ts index 72fcb90..80922fd 100644 --- a/src/commands/sources.ts +++ b/src/commands/sources.ts @@ -1,6 +1,6 @@ import { parseArgs } from 'node:util'; import chalk from 'chalk'; -import { input, password, select } from '@inquirer/prompts'; +import { confirm, input, password, select } from '@inquirer/prompts'; import { createSource, deleteSource, @@ -34,7 +34,7 @@ export const USAGE = `Usage: markpost sources Promise + ( + uuid: string | undefined, + json: boolean, + skipConfirm: boolean, + ) => Promise >([ [LIST_SUBCOMMAND, (_uuid, json) => listSources(json)], ['create', () => createSourceCommand()], ['update', (uuid) => updateSourceCommand(uuid)], - ['delete', (uuid) => deleteSourceCommand(uuid)], + [ + DELETE_SUBCOMMAND, + (uuid, _json, skipConfirm) => deleteSourceCommand(uuid, skipConfirm), + ], ['rotate-secret', (uuid) => rotateSecretCommand(uuid)], ]); +// The invocation-level usage checks that all fail the same way (one usage +// message, non-zero exit). Returns the message to show, or null when the +// invocation is valid. Kept in one place so their ordering is a single unit +// rather than four near-identical guard blocks in the runner. +const usageErrorFor = ( + subcommand: string, + uuid: string | undefined, + json: boolean, + skipConfirm: boolean, + isInteractive: boolean, +): string | null => { + // Reject --json where it does nothing rather than silently ignoring it: + // `sources create --json | jq` would otherwise "succeed" with human text on + // stdout, losing the one-time signing secret it was trying to capture. + if (json && subcommand !== LIST_SUBCOMMAND) { + return `--json is only supported by \`sources ${LIST_SUBCOMMAND}\`.`; + } + + // --yes only skips the delete confirmation; reject it elsewhere so a + // misplaced flag fails loudly instead of appearing to take effect. + if (skipConfirm && subcommand !== DELETE_SUBCOMMAND) { + return `--yes is only supported by \`sources ${DELETE_SUBCOMMAND}\`.`; + } + + // --yes promises a non-interactive delete, so it needs an explicit uuid — + // without one the picker still opens and a script blocks on it forever. + if (skipConfirm && !uuid) { + return `--yes requires a uuid: \`markpost sources ${DELETE_SUBCOMMAND} --yes\`.`; + } + + // The confirmation prompt can't be answered without an interactive terminal: + // inquirer renders to stdout and reads stdin, and its EOF abort is swallowed + // as a Ctrl+C below — so a redirected/non-interactive `sources delete` would + // hang or delete nothing yet still exit 0. Fail loud and point scripts at + // --yes. Only delete is guarded here because it's the irreversible one; + // `create`/`update` also prompt, but that predates this change and their + // non-TTY behavior is out of scope for the delete-confirmation work. + if (subcommand === DELETE_SUBCOMMAND && !skipConfirm && !isInteractive) { + return `\`sources delete\` needs an interactive terminal to confirm; pass a uuid with --yes (\`markpost sources ${DELETE_SUBCOMMAND} --yes\`) to delete without a prompt.`; + } + + return null; +}; + export const runSourcesCommand = async (args: string[]): Promise => { // Read `--json` straight from argv so every failure below is rendered in // whichever contract the caller asked for, even one thrown before parsing. @@ -76,33 +130,41 @@ export const runSourcesCommand = async (args: string[]): Promise => { // `parseArgs` keeps --json out of the uuid slot (so `sources delete --json` // still prompts rather than trying to delete a source named "--json") and // rejects an unknown/mistyped flag. Only `list` reads json. - const { positionals } = parseArgs({ + const { positionals, values } = parseArgs({ args, allowPositionals: true, options: { json: { type: 'boolean' }, + yes: { type: 'boolean' }, }, }); const [subcommand, uuid] = positionals; + const skipConfirm = Boolean(values.yes); const handler = SOURCES_HANDLERS.get(subcommand); // Validate before the config check so a bad subcommand fails on usage - // alone, without needing a configured account. + // alone, without needing a configured account. The bad-subcommand case + // fails differently (it prints the subcommand), so it stays here; the rest + // share one usage-error shape and live in `usageErrorFor`. if (!handler) { failWithSubcommandUsage(subcommand, USAGE, json); return; } - // Reject --json where it does nothing rather than silently ignoring it: - // `sources create --json | jq` would otherwise "succeed" with human text on - // stdout, and the one-time signing secret it was trying to capture would be - // lost (see createSourceCommand's unrecoverable-secret warning). - if (json && subcommand !== LIST_SUBCOMMAND) { - failWithUsage( - `--json is only supported by \`sources ${LIST_SUBCOMMAND}\`.`, - USAGE, - json, - ); + // A prompt needs both streams to be a terminal: inquirer reads stdin and + // renders to stdout, so a redirect on either makes the confirmation + // unanswerable. + const isInteractive = Boolean(process.stdin.isTTY && process.stdout.isTTY); + const usageError = usageErrorFor( + subcommand, + uuid, + json, + skipConfirm, + isInteractive, + ); + + if (usageError) { + failWithUsage(usageError, USAGE, json); return; } @@ -110,7 +172,7 @@ export const runSourcesCommand = async (args: string[]): Promise => { return; } - await handler(uuid, json); + await handler(uuid, json, skipConfirm); } catch (error) { // A deliberate Ctrl+C at a prompt throws @inquirer's `ExitPromptError`; // that's a user abort, not a command failure, so don't flag it non-zero. @@ -300,17 +362,23 @@ const promptForSource = async ( return sources.find((source) => source.uuid === selectedUuid) ?? null; }; -const findSourceByUuid = async (uuid: string): Promise => { +// Fetch the source list and pick one out by uuid, or null if none matches. +// fetchSources() swallows transport errors (except a timeout, which +// propagates) into [], so a missing uuid is indistinguishable here from a +// failed load. Shared by the reporting `findSourceByUuid` and the best-effort +// delete label so the fetch+find isn't written three ways. +const lookupSourceByUuid = async (uuid: string): Promise => { const sources = await fetchSources(); - const source = sources.find((candidate) => candidate.uuid === uuid); + return sources.find((candidate) => candidate.uuid === uuid) ?? null; +}; + +const findSourceByUuid = async (uuid: string): Promise => { + const source = await lookupSourceByUuid(uuid); if (source) { return source; } - // fetchSources() swallows transport errors (except a timeout, which - // propagates) and returns [], so a uuid that doesn't match is - // indistinguishable here from a failed lookup. console.error( chalk.redBright( 'Source not found, or the source list could not be loaded.', @@ -363,17 +431,97 @@ const updateSourceCommand = async (uuid?: string): Promise => { await promptAndApplyRouteFolder(target); }; -const deleteSourceCommand = async (uuid?: string): Promise => { - const targetUuid = uuid ?? (await promptForSource('delete'))?.uuid; +// Deleting a source is irreversible: it drops the ingest config and the +// one-time signing secret, which can never be retrieved again. The label is +// sanitized because it may have come from an untrusted API response via the +// interactive picker. Defaults to "no" so a bare Enter cancels rather than +// deletes. Isolated here so the delete flow stays unit-testable by mocking the +// prompt. +const confirmDeletion = async (label: string): Promise => + confirm({ + message: `Delete source ${sanitizeForTerminal( + label, + )}? This drops its ingest config and one-time signing secret and cannot be undone.`, + default: false, + }); + +// An empty list from lookupSourceByUuid can't tell a genuine non-match from a +// swallowed load failure — fetchSources() folds transport errors (all but a +// timeout) into []. So this can't claim the source is absent; it mirrors +// findSourceByUuid's wording and leaves both possibilities open. +const NO_MATCH_NOTE = + 'no matching source found, or the list could not be loaded'; +// The lookup itself failed (e.g. a timeout, which fetchSources re-throws), so +// the name is simply unknown — distinct from a confirmed non-match, and never +// claiming the source doesn't exist. +const LOOKUP_FAILED_NOTE = 'source name unavailable — could not load the list'; + +// Build the confirmation label. The interactive pick already carries the +// Source; a bare-uuid delete looks the source up so the prompt names it — +// surfacing a wrong-but-valid (or non-existent) copy-pasted uuid before it +// destroys anything, rather than echoing back the exact string the user typed. +// The lookup is purely cosmetic, so it's best-effort: any failure falls back to +// the bare uuid rather than blocking a delete that would otherwise succeed. The +// three outcomes stay distinct in the label so a failed load is never +// mis-reported as a confirmed non-match. `undefined` marks a thrown lookup, +// `null` a loaded-but-missing one. +const deleteConfirmationLabel = async ( + picked: Source | null | undefined, + targetUuid: string, +): Promise => { + if (picked) { + return `${picked.name} (${targetUuid})`; + } + + const source = await lookupSourceByUuid(targetUuid).catch(() => undefined); + + if (source === undefined) { + return `${targetUuid} (${LOOKUP_FAILED_NOTE})`; + } + + return source + ? `${source.name} (${targetUuid})` + : `${targetUuid} (${NO_MATCH_NOTE})`; +}; + +// Compose label-building with the prompt into one named step so the call site +// reads as a sentence; the `||` at the call site is what short-circuits this +// away (label lookup included) under `--yes`. +const confirmSourceDeletion = async ( + picked: Source | null | undefined, + targetUuid: string, +): Promise => + confirmDeletion(await deleteConfirmationLabel(picked, targetUuid)); + +const deleteSourceCommand = async ( + uuid: string | undefined, + skipConfirm: boolean, +): Promise => { + const picked = uuid ? undefined : await promptForSource('delete'); + // `||` (not `??`) so an empty-string uuid falls through to the picked source, + // matching the truthiness branch above — otherwise `delete ""` would open the + // picker, take a selection, then silently discard it on the `!targetUuid` guard. + const targetUuid = uuid || picked?.uuid; if (!targetUuid) { return; } + const confirmed = + skipConfirm || (await confirmSourceDeletion(picked, targetUuid)); + + if (!confirmed) { + console.log('Deletion cancelled.'); + return; + } + const meta = await deleteSource(targetUuid); if (!meta) { - console.error(chalk.redBright('Failed to delete source.')); + // Exit non-zero (not a bare console.error) so a scripted `delete + // --yes || notify` catches a failed delete instead of reading it as done — + // delete now carries a documented --yes contract, like rotate-secret below. + failWithMessage('Failed to delete source.'); return; } diff --git a/tests/commands/sources.test.ts b/tests/commands/sources.test.ts index 2fe80f5..addea93 100644 --- a/tests/commands/sources.test.ts +++ b/tests/commands/sources.test.ts @@ -16,6 +16,7 @@ vi.mock('@inquirer/prompts', () => ({ input: vi.fn(), password: vi.fn(), select: vi.fn(), + confirm: vi.fn(), })); vi.mock('chalk', () => ({ default: { @@ -106,16 +107,26 @@ describe('buildEndpointUrl', () => { }); describe('runSourcesCommand', () => { + // `sources delete` now refuses to prompt without an interactive terminal + // (both stdin and stdout must be TTYs), so simulate one by default; the + // non-TTY guards have their own cases below. + const originalStdinIsTTY = process.stdin.isTTY; + const originalStdoutIsTTY = process.stdout.isTTY; + beforeEach(() => { vi.resetModules(); vi.clearAllMocks(); process.exitCode = undefined; + process.stdin.isTTY = true; + process.stdout.isTTY = true; vi.spyOn(console, 'log').mockImplementation(() => {}); vi.spyOn(console, 'error').mockImplementation(() => {}); }); afterEach(() => { process.exitCode = undefined; + process.stdin.isTTY = originalStdinIsTTY; + process.stdout.isTTY = originalStdoutIsTTY; }); it('always checks config before dispatching', async () => { @@ -710,50 +721,375 @@ describe('runSourcesCommand', () => { }); describe('delete', () => { - it('deletes directly by uuid when one is provided', async () => { - const { deleteSource } = await import('@/libs/sources.js'); + it('deletes directly by uuid when one is provided and confirmed', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([webhookSource]); vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); - const { select } = await import('@inquirer/prompts'); + const { select, confirm } = await import('@inquirer/prompts'); + vi.mocked(confirm).mockResolvedValue(true); const { runSourcesCommand } = await import('@/commands/sources.js'); await runSourcesCommand(['delete', 'abc-123']); + expect(confirm).toHaveBeenCalledTimes(1); expect(deleteSource).toHaveBeenCalledWith('abc-123'); expect(select).not.toHaveBeenCalled(); }); - it('prompts to pick a source when no uuid is given', async () => { + it('prompts to pick a source when no uuid is given, then confirms', async () => { const { fetchSources, deleteSource } = await import('@/libs/sources.js'); vi.mocked(fetchSources).mockResolvedValue([webhookSource]); vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); - const { select } = await import('@inquirer/prompts'); + const { select, confirm } = await import('@inquirer/prompts'); + vi.mocked(select).mockResolvedValue('abc-123'); + vi.mocked(confirm).mockResolvedValue(true); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete']); + + expect(deleteSource).toHaveBeenCalledWith('abc-123'); + }); + + // An empty-string uuid is falsy like an absent one, so it must open the + // picker and honor the selection — not open the picker and then silently + // discard the pick on the `!targetUuid` guard (the `??`-vs-`||` trap). + it('falls through to the picker for an empty-string uuid and deletes the pick', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([webhookSource]); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { select, confirm } = await import('@inquirer/prompts'); + vi.mocked(select).mockResolvedValue('abc-123'); + vi.mocked(confirm).mockResolvedValue(true); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', '']); + + expect(select).toHaveBeenCalledTimes(1); + expect(deleteSource).toHaveBeenCalledWith('abc-123'); + }); + + // The confirmation is the whole point of the feature: a "no" answer must + // abort before any DELETE reaches the server. + it('aborts without deleting when the confirmation is declined', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([webhookSource]); + const { confirm } = await import('@inquirer/prompts'); + vi.mocked(confirm).mockResolvedValue(false); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123']); + + expect(deleteSource).not.toHaveBeenCalled(); + expect(console.log).toHaveBeenCalledWith('Deletion cancelled.'); + // A user "no" is a clean abort, not a failure — must not exit non-zero. + expect(process.exitCode).toBeUndefined(); + }); + + // The confirm message must name the source being deleted so the user knows + // what they're destroying, and warn that it's irreversible. + it('shows the uuid and an irreversible-warning in the confirmation prompt', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([webhookSource]); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { confirm } = await import('@inquirer/prompts'); + vi.mocked(confirm).mockResolvedValue(true); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123']); + + expect(confirm).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining('abc-123'), + default: false, + }), + ); + expect(vi.mocked(confirm).mock.calls[0][0].message).toContain( + 'cannot be undone', + ); + }); + + // The direct-uuid path looks the source up (like update does) so a valid + // but wrong copy-pasted uuid shows its real name for the user to catch, + // rather than echoing back the string they typed. + it('names the looked-up source on the direct-uuid path', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([webhookSource]); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { confirm } = await import('@inquirer/prompts'); + vi.mocked(confirm).mockResolvedValue(true); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123']); + + expect(vi.mocked(confirm).mock.calls[0][0].message).toContain( + 'Webhook Source', + ); + }); + + // A lookup that can't resolve the uuid (e.g. fetchSources swallowed a + // transport error into []) must still confirm on the bare uuid — and flag + // the miss so it isn't mistaken for a matched source — never block delete. + it('flags the miss in the label when the source lookup finds nothing', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([]); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { confirm } = await import('@inquirer/prompts'); + vi.mocked(confirm).mockResolvedValue(true); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123']); + + const message = vi.mocked(confirm).mock.calls[0][0].message; + expect(message).toContain('abc-123'); + expect(message).toContain('no matching source found'); + expect(deleteSource).toHaveBeenCalledWith('abc-123'); + }); + + // The label lookup is cosmetic: a timeout (which fetchSources re-throws, not + // swallows) must not abort a delete that issued no such read before this + // change. It falls back to the bare uuid, says the name is unavailable (not + // that the source is missing), and still confirms + deletes. + it('still confirms and deletes when the label lookup times out', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + const timeoutError = Object.assign(new Error('timed out'), { + name: 'ApiTimeoutError', + }); + vi.mocked(fetchSources).mockRejectedValue(timeoutError); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { confirm } = await import('@inquirer/prompts'); + vi.mocked(confirm).mockResolvedValue(true); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123']); + + expect(confirm).toHaveBeenCalledTimes(1); + const message = vi.mocked(confirm).mock.calls[0][0].message; + expect(message).toContain('abc-123'); + // A failed load must not be reported as a confirmed non-match. + expect(message).not.toContain('no matching source found'); + expect(message).toContain('could not load the list'); + expect(deleteSource).toHaveBeenCalledWith('abc-123'); + expect(process.exitCode).toBeUndefined(); + }); + + // A hostile source name/uuid surfaced through the picker must be stripped + // before it reaches the confirm prompt, same as every other printed field. + it('sanitizes the uuid in the confirmation prompt', async () => { + const control = String.fromCharCode(0x1b); + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([ + { ...webhookSource, uuid: `abc${control}123` }, + ]); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { select, confirm } = await import('@inquirer/prompts'); + vi.mocked(select).mockResolvedValue(`abc${control}123`); + vi.mocked(confirm).mockResolvedValue(true); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete']); + + expect(vi.mocked(confirm).mock.calls[0][0].message).not.toContain( + control, + ); + expect(vi.mocked(confirm).mock.calls[0][0].message).toContain('abc 123'); + }); + + // The scripting escape hatch: --yes deletes straight away with no prompt + // and no label lookup (the short-circuit must skip fetchSources entirely). + it('skips the confirmation and the label lookup when --yes is passed', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { confirm } = await import('@inquirer/prompts'); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123', '--yes']); + + expect(confirm).not.toHaveBeenCalled(); + expect(fetchSources).not.toHaveBeenCalled(); + expect(deleteSource).toHaveBeenCalledWith('abc-123'); + }); + + // A Ctrl+C at the confirmation is a deliberate abort: it must delete + // nothing, exit 0, and stay quiet — never fall through to the DELETE. + it('aborts cleanly without deleting when the confirmation is Ctrl+C-ed', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([webhookSource]); + const { confirm } = await import('@inquirer/prompts'); + const exitPromptError = Object.assign(new Error('User force closed'), { + name: 'ExitPromptError', + }); + vi.mocked(confirm).mockRejectedValue(exitPromptError); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123']); + + expect(deleteSource).not.toHaveBeenCalled(); + expect(process.exitCode).toBeUndefined(); + expect(console.error).not.toHaveBeenCalled(); + }); + + // The interactive confirmation names the picked source so the user can + // recognize it, not just the opaque uuid they never typed. + it('names the picked source in the confirmation prompt', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([webhookSource]); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { select, confirm } = await import('@inquirer/prompts'); vi.mocked(select).mockResolvedValue('abc-123'); + vi.mocked(confirm).mockResolvedValue(true); const { runSourcesCommand } = await import('@/commands/sources.js'); await runSourcesCommand(['delete']); + const message = vi.mocked(confirm).mock.calls[0][0].message; + expect(message).toContain('Webhook Source'); + expect(message).toContain('abc-123'); + }); + + // --yes is meaningless outside delete; it must fail loudly like a misplaced + // --json rather than appearing to take effect. + it('rejects --yes on a non-delete subcommand', async () => { + const { checkConfig } = await import('@/libs/config.js'); + const { fetchSources } = await import('@/libs/sources.js'); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['list', '--yes']); + + expect(checkConfig).not.toHaveBeenCalled(); + expect(fetchSources).not.toHaveBeenCalled(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('--yes is only supported by `sources delete`.'), + ); + expect(process.exitCode).toBe(1); + }); + + // The guard is shared, but assert create and update hit it *before* + // dispatch (no prompts, no API calls) so a future reorder that moves it + // below dispatch can't slip past on the exit code alone. + it('rejects --yes on create before dispatching', async () => { + const { checkConfig } = await import('@/libs/config.js'); + const { createSource } = await import('@/libs/sources.js'); + const { select } = await import('@inquirer/prompts'); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['create', '--yes']); + + expect(checkConfig).not.toHaveBeenCalled(); + expect(select).not.toHaveBeenCalled(); + expect(createSource).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + }); + + it('rejects --yes on update before dispatching', async () => { + const { checkConfig } = await import('@/libs/config.js'); + const { fetchSources, updateSource } = await import('@/libs/sources.js'); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['update', '--yes']); + + expect(checkConfig).not.toHaveBeenCalled(); + expect(fetchSources).not.toHaveBeenCalled(); + expect(updateSource).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + }); + + // --yes promises a non-interactive delete, so with no uuid it must fail + // rather than open the picker a script can't answer. + it('rejects --yes without a uuid instead of opening the picker', async () => { + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + const { select } = await import('@inquirer/prompts'); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', '--yes']); + + expect(select).not.toHaveBeenCalled(); + expect(fetchSources).not.toHaveBeenCalled(); + expect(deleteSource).not.toHaveBeenCalled(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('--yes requires a uuid'), + ); + expect(process.exitCode).toBe(1); + }); + + // Without a TTY the prompt can't be answered and inquirer's abort is + // swallowed as Ctrl+C, so a bare non-interactive delete must fail loud + // rather than silently delete nothing and exit 0. + it('fails loudly on a non-TTY stdin delete when --yes is absent', async () => { + process.stdin.isTTY = false; + const { deleteSource } = await import('@/libs/sources.js'); + const { confirm } = await import('@inquirer/prompts'); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123']); + + expect(confirm).not.toHaveBeenCalled(); + expect(deleteSource).not.toHaveBeenCalled(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('needs an interactive terminal'), + ); + expect(process.exitCode).toBe(1); + }); + + // Redirected stdout (`sources delete > log`) leaves stdin a TTY but + // hides the prompt inquirer renders to stdout — same hang, so it must fail + // the same way. + it('fails loudly on a redirected-stdout delete when --yes is absent', async () => { + process.stdout.isTTY = false; + const { deleteSource } = await import('@/libs/sources.js'); + const { confirm } = await import('@inquirer/prompts'); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123']); + + expect(confirm).not.toHaveBeenCalled(); + expect(deleteSource).not.toHaveBeenCalled(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('needs an interactive terminal'), + ); + expect(process.exitCode).toBe(1); + }); + + // The scripting path: --yes with a uuid deletes on a non-TTY, no prompt. + it('deletes on a non-TTY when a uuid and --yes are given', async () => { + process.stdin.isTTY = false; + const { deleteSource } = await import('@/libs/sources.js'); + vi.mocked(deleteSource).mockResolvedValue({ deleted: 1 }); + const { confirm } = await import('@inquirer/prompts'); + const { runSourcesCommand } = await import('@/commands/sources.js'); + + await runSourcesCommand(['delete', 'abc-123', '--yes']); + + expect(confirm).not.toHaveBeenCalled(); expect(deleteSource).toHaveBeenCalledWith('abc-123'); }); it('does nothing when there are no sources to pick from', async () => { const { fetchSources, deleteSource } = await import('@/libs/sources.js'); vi.mocked(fetchSources).mockResolvedValue([]); + const { confirm } = await import('@inquirer/prompts'); const { runSourcesCommand } = await import('@/commands/sources.js'); await runSourcesCommand(['delete']); expect(deleteSource).not.toHaveBeenCalled(); + expect(confirm).not.toHaveBeenCalled(); expect(console.log).toHaveBeenCalledWith('No sources to delete.'); }); it('reports an error when deletion fails', async () => { - const { deleteSource } = await import('@/libs/sources.js'); + const { fetchSources, deleteSource } = await import('@/libs/sources.js'); + vi.mocked(fetchSources).mockResolvedValue([webhookSource]); vi.mocked(deleteSource).mockResolvedValue(null); + const { confirm } = await import('@inquirer/prompts'); + vi.mocked(confirm).mockResolvedValue(true); const { runSourcesCommand } = await import('@/commands/sources.js'); await runSourcesCommand(['delete', 'abc-123']); expect(console.error).toHaveBeenCalledWith('Failed to delete source.'); + // A failed delete must exit non-zero so a scripted `delete --yes || …` + // catches it instead of reading the non-delete as success. + expect(process.exitCode).toBe(1); }); });