diff --git a/docs/output-contract.md b/docs/output-contract.md index f74b1eeb..76d93946 100644 --- a/docs/output-contract.md +++ b/docs/output-contract.md @@ -99,11 +99,14 @@ change in the PR. | Code | Meaning | |---|---| | 0 | Success. Also a declined confirmation, `--dry-run` previews, and runs whose checks passed or only degraded (a degraded check does not fail the run). A run stopped with Ctrl-C has no code of its own: it exits 1 if a result that arrived failed, and 0 otherwise. | -| 1 | Failure: a failed check or run error, no matching checks under `--fail-on-no-matching` (the default), API errors, invalid configuration. | -| 2 | Usage error (unknown flags/args, oclif parse errors), or an error a command reports through oclif's `this.error`. The message is on stderr and stdout is empty. | +| 1 | Failure: a failed check or run error, no matching checks under `--fail-on-no-matching` (the default), API errors, invalid configuration, or a failure reported only on stderr. | +| 2 | Usage error: unknown flags or arguments, oclif parse errors, or invalid flag or argument values a command rejects with `this.error`. The message is on stderr and stdout is empty. | | 3 | `confirmation_required` (agent and CI mode): a write command stopped for approval and printed the envelope on stdout. | | 127 | Unknown command. | +oclif's `this.error` exits 2 by default, so commands report any other failure +through it with `{ exit: 1 }`; a bare `this.error` is for invalid input. + ## Notes - Colours follow stdout: chalk decides from stdout whether to colour, for diff --git a/packages/cli/e2e/__tests__/account-plan.spec.ts b/packages/cli/e2e/__tests__/account-plan.spec.ts index 8f41d0d0..dd99abcd 100644 --- a/packages/cli/e2e/__tests__/account-plan.spec.ts +++ b/packages/cli/e2e/__tests__/account-plan.spec.ts @@ -79,7 +79,7 @@ describe('checkly account plan', () => { expect.unreachable('Expected command to fail') } catch (err) { if (err instanceof ExecaError) { - expect(err.exitCode).toBe(2) + expect(err.exitCode).toBe(1) expect(err.stderr).toContain('not found') } else { throw err diff --git a/packages/cli/src/ai-context/skill.md b/packages/cli/src/ai-context/skill.md index e89b233c..3efa4c93 100644 --- a/packages/cli/src/ai-context/skill.md +++ b/packages/cli/src/ai-context/skill.md @@ -87,7 +87,7 @@ Run `npx checkly skills communicate` for the full protocol details, or `npx chec ## Output Streams and Exit Codes -stdout carries only the command's result: results, `--output json` documents (most failures print a JSON error document too), API pass-through bodies, and the confirmation/dry-run envelopes above. Progress, warnings, hints, prompts, and human-readable errors go to stderr, so `npx checkly --output json | jq` sees nothing but the JSON. Check the exit code first, since some failures leave stdout empty: 0 success (degraded checks do not fail a run; read the results to spot them), 1 failure, 2 a usage error (or another error reported only on stderr), 3 `confirmation_required`, 127 an unknown command. +stdout carries only the command's result: results, `--output json` documents (most failures print a JSON error document too), API pass-through bodies, and the confirmation/dry-run envelopes above. Progress, warnings, hints, prompts, and human-readable errors go to stderr, so `npx checkly --output json | jq` sees nothing but the JSON. Check the exit code first, since some failures leave stdout empty: 0 success (degraded checks do not fail a run; read the results to spot them), 1 failure, 2 a usage error, 3 `confirmation_required`, 127 an unknown command. ## API Pass-Through (fallback for any endpoint) diff --git a/packages/cli/src/commands/__tests__/login.spec.ts b/packages/cli/src/commands/__tests__/login.spec.ts index c02bd665..39bee624 100644 --- a/packages/cli/src/commands/__tests__/login.spec.ts +++ b/packages/cli/src/commands/__tests__/login.spec.ts @@ -1185,6 +1185,21 @@ describe('checkly login', () => { expect(error.oclif).toMatchObject({ exit: 1 }) }) + it('fails with exit 1 when the account prompt is cancelled', async () => { + vi.mocked(api.accounts.getAll).mockResolvedValue({ + data: [{ id: 'acc-1', name: 'Acme' }, { id: 'acc-2', name: 'Globex' }], + } as any) + vi.mocked(prompts).mockImplementationOnce((_question, options) => { + options!.onCancel!({} as any, {}) + return Promise.resolve({}) + }) + const cmd = createCommand() + const error = await cmd.run().catch(error => error) + + expect(error.message).toBe('Command cancelled.\n') + expect(error.oclif).toMatchObject({ exit: 1 }) + }) + it.each([ ['an expired code', () => deviceFlow.pollForTokens.mockRejectedValueOnce( new DeviceFlowError('expired_token', 'The login code expired.')), 'The login code expired.'], diff --git a/packages/cli/src/commands/account/plan.ts b/packages/cli/src/commands/account/plan.ts index c577822f..ddca3217 100644 --- a/packages/cli/src/commands/account/plan.ts +++ b/packages/cli/src/commands/account/plan.ts @@ -75,7 +75,10 @@ export default class AccountPlan extends AuthCommand { if (args.key) { const entitlement = plan.entitlements.find(e => e.key === args.key) if (!entitlement) { - this.error(`Entitlement "${args.key}" not found. Use "checkly account plan" to see available keys.`) + this.error( + `Entitlement "${args.key}" not found. Use "checkly account plan" to see available keys.`, + { exit: 1 }, + ) } if (flags.output === 'json') { diff --git a/packages/cli/src/commands/authCommand.ts b/packages/cli/src/commands/authCommand.ts index ae3fc9de..123c7fa0 100644 --- a/packages/cli/src/commands/authCommand.ts +++ b/packages/cli/src/commands/authCommand.ts @@ -59,8 +59,8 @@ async function loginInlineIfNeeded (command: BaseCommand): Promise { /** * Exit code of a write command that stopped for confirmation in agent or CI * mode, after printing the `confirmation_required` envelope. It differs from - * oclif's 2 (usage errors and `this.error()`), so the exit code alone says - * whether the command needs approval. + * oclif's 2 for usage errors, so the exit code alone says whether the command + * needs approval. */ export const CONFIRMATION_REQUIRED_EXIT_CODE = 3 diff --git a/packages/cli/src/commands/login.ts b/packages/cli/src/commands/login.ts index b4d99c78..f47d97b3 100644 --- a/packages/cli/src/commands/login.ts +++ b/packages/cli/src/commands/login.ts @@ -995,7 +995,7 @@ export default class Login extends BaseCommand { this.#atBlankLine = false const selected = await selectAccount(accounts, { - onCancel: () => this.error('Command cancelled.\n'), + onCancel: () => this.error('Command cancelled.\n', { exit: 1 }), }) if (!selected) { throw new Error('You must select a valid Checkly account name.') diff --git a/packages/cli/src/commands/skills/__tests__/index.spec.ts b/packages/cli/src/commands/skills/__tests__/index.spec.ts new file mode 100644 index 00000000..cf7a427b --- /dev/null +++ b/packages/cli/src/commands/skills/__tests__/index.spec.ts @@ -0,0 +1,46 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('fs/promises', () => ({ + readFile: vi.fn(), + readdir: vi.fn(), +})) + +import { readFile } from 'fs/promises' +import { ACTIONS } from '../../../ai-context/context.js' +import Skills from '../index.js' + +const mockConfig = { + runHook: vi.fn().mockResolvedValue({ successes: [], failures: [] }), +} as any + +function createCommand (...args: string[]) { + const cmd = new Skills(args, mockConfig) + cmd.log = vi.fn() as any + cmd.logToStderr = vi.fn() as any + return cmd +} + +describe('checkly skills', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('fails with exit 1 when the action file cannot be read', async () => { + vi.mocked(readFile).mockRejectedValue(new Error('ENOENT')) + const cmd = createCommand(ACTIONS[0].id) + + await expect(cmd.run()).rejects.toMatchObject({ + message: `Could not read action file for "${ACTIONS[0].id}".`, + oclif: { exit: 1 }, + }) + }) + + it('rejects an unknown action as a usage error', async () => { + const cmd = createCommand('no-such-action') + + await expect(cmd.run()).rejects.toMatchObject({ + message: expect.stringContaining('Action "no-such-action" not found.'), + oclif: { exit: 2 }, + }) + }) +}) diff --git a/packages/cli/src/commands/skills/__tests__/install.spec.ts b/packages/cli/src/commands/skills/__tests__/install.spec.ts index 9f261085..b216144d 100644 --- a/packages/cli/src/commands/skills/__tests__/install.spec.ts +++ b/packages/cli/src/commands/skills/__tests__/install.spec.ts @@ -167,7 +167,10 @@ describe('skills install', () => { vi.mocked(readFile).mockRejectedValue(new Error('ENOENT')) const cmd = createCommand('--target', 'claude', '--force') - await expect(cmd.run()).rejects.toThrow('Failed to read skill file') + await expect(cmd.run()).rejects.toMatchObject({ + message: expect.stringContaining('Failed to read skill file'), + oclif: { exit: 1 }, + }) }) it('errors when target directory cannot be created', async () => { @@ -181,7 +184,10 @@ describe('skills install', () => { vi.mocked(writeFile).mockRejectedValue(new Error('EACCES')) const cmd = createCommand('--target', 'claude', '--force') - await expect(cmd.run()).rejects.toThrow('Failed to write skill file') + await expect(cmd.run()).rejects.toMatchObject({ + message: expect.stringContaining('Failed to write skill file'), + oclif: { exit: 1 }, + }) }) }) diff --git a/packages/cli/src/commands/skills/index.ts b/packages/cli/src/commands/skills/index.ts index a4fd117e..53302cbf 100644 --- a/packages/cli/src/commands/skills/index.ts +++ b/packages/cli/src/commands/skills/index.ts @@ -101,7 +101,7 @@ export default class Skills extends BaseCommand { const content = await readFile(actionPath, 'utf8') this.log(content) } catch { - this.error(`Could not read action file for "${args.action}".`) + this.error(`Could not read action file for "${args.action}".`, { exit: 1 }) } } } diff --git a/packages/cli/src/commands/skills/install.ts b/packages/cli/src/commands/skills/install.ts index b4f06749..94549547 100644 --- a/packages/cli/src/commands/skills/install.ts +++ b/packages/cli/src/commands/skills/install.ts @@ -82,7 +82,7 @@ export default class SkillsInstall extends BaseCommand { try { return await readSkillFile() } catch (err: any) { - this.error(err.message) + this.error(err.message, { exit: 1 }) } } @@ -125,7 +125,7 @@ export default class SkillsInstall extends BaseCommand { const writtenPath = await writeSkillToTarget(targetDir, content) this.style.shortSuccess(`Installed Checkly agent skill to: ${writtenPath}`) } catch (err: any) { - this.error(err.message) + this.error(err.message, { exit: 1 }) } } diff --git a/packages/cli/src/commands/switch.ts b/packages/cli/src/commands/switch.ts index 575a632d..c7dcc3d0 100644 --- a/packages/cli/src/commands/switch.ts +++ b/packages/cli/src/commands/switch.ts @@ -26,7 +26,7 @@ export default class Switch extends AuthCommand { const { 'account-id': accountId } = flags const onCancel = (): void => { - this.error('Command cancelled.\n') + this.error('Command cancelled.\n', { exit: 1 }) } if (accountId) { diff --git a/skills/checkly/SKILL.md b/skills/checkly/SKILL.md index db7703ad..93dde83d 100644 --- a/skills/checkly/SKILL.md +++ b/skills/checkly/SKILL.md @@ -87,7 +87,7 @@ Run `npx checkly skills communicate` for the full protocol details, or `npx chec ## Output Streams and Exit Codes -stdout carries only the command's result: results, `--output json` documents (most failures print a JSON error document too), API pass-through bodies, and the confirmation/dry-run envelopes above. Progress, warnings, hints, prompts, and human-readable errors go to stderr, so `npx checkly --output json | jq` sees nothing but the JSON. Check the exit code first, since some failures leave stdout empty: 0 success (degraded checks do not fail a run; read the results to spot them), 1 failure, 2 a usage error (or another error reported only on stderr), 3 `confirmation_required`, 127 an unknown command. +stdout carries only the command's result: results, `--output json` documents (most failures print a JSON error document too), API pass-through bodies, and the confirmation/dry-run envelopes above. Progress, warnings, hints, prompts, and human-readable errors go to stderr, so `npx checkly --output json | jq` sees nothing but the JSON. Check the exit code first, since some failures leave stdout empty: 0 success (degraded checks do not fail a run; read the results to spot them), 1 failure, 2 a usage error, 3 `confirmation_required`, 127 an unknown command. ## API Pass-Through (fallback for any endpoint)