diff --git a/src/commands/commit/commit.test.ts b/src/commands/commit/commit.test.ts index aa664898..1a5ec278 100644 --- a/src/commands/commit/commit.test.ts +++ b/src/commands/commit/commit.test.ts @@ -496,7 +496,7 @@ describe('commit command', () => { expect(mockHandleResult).not.toHaveBeenCalled() }) - it('exits non-zero without printing anything when there are no staged changes', async () => { + it('exits non-zero without printing anything to stdout when there are no staged changes', async () => { mockGetChanges.mockResolvedValue({ staged: [], unstaged: [], untracked: [] }) const writes: string[] = [] @@ -514,6 +514,43 @@ describe('commit command', () => { expect(writes).toHaveLength(0) }) + + // CMD-15: this path used to report failures via logger.verbose, which + // no-ops unless --verbose is set — the exact non-interactive command + // the installed prepare-commit-msg hook runs would fail with zero + // output anywhere, stdout or stderr. + it('surfaces a failure reason via logger.error (not logger.verbose) even without --verbose', async () => { + mockGetChanges.mockResolvedValue({ staged: [], unstaged: [], untracked: [] }) + + await expect(handler(argv, logger)).rejects.toMatchObject({ code: 1 }) + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('No staged changes detected'), + expect.anything() + ) + expect(logger.verbose).not.toHaveBeenCalledWith( + expect.stringContaining('No staged changes detected'), + expect.anything() + ) + }) + + it('surfaces the curated missing-API-key hint instead of a bare validation error', async () => { + mockGetApiKeyForModel.mockReturnValue('') + + await expect(handler(argv, logger)).rejects.toMatchObject({ code: 1 }) + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('Missing API key'), + expect.anything() + ) + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('OPENAI_API_KEY'), + expect.anything() + ) + // The rich hint pre-empts generateCommitDraft entirely — no reason + // to run diff summarization just to fail on the same missing key. + expect(mockGetChanges).not.toHaveBeenCalled() + }) }) describe('--json', () => { @@ -582,5 +619,20 @@ describe('commit command', () => { expect(typeof parsed.error).toBe('string') expect(parsed.error.length).toBeGreaterThan(0) }) + + // CMD-15: --json keeps its own contract — a missing key must still + // reach the caller as a parseable stdout payload, not the human- + // formatted handleMissingApiKey lines (which --print-message gets). + it('still emits a parseable { error } payload for a missing API key, not the human-formatted hint', async () => { + mockGetApiKeyForModel.mockReturnValue('') + + const output = await captureStdout(async () => { + await expect(handler(argv, logger)).rejects.toMatchObject({ code: 1 }) + }) + + const parsed = JSON.parse(output) + expect(typeof parsed.error).toBe('string') + expect(parsed.error.length).toBeGreaterThan(0) + }) }) }) diff --git a/src/commands/commit/handler.ts b/src/commands/commit/handler.ts index 3c441114..4e3e7dbd 100644 --- a/src/commands/commit/handler.ts +++ b/src/commands/commit/handler.ts @@ -63,13 +63,29 @@ export const handler: CommandHandler = async (argv, logger) => { } if (argv.printMessage || argv.json) { + // Plain --print-message has no JSON contract to preserve, so give it + // the same curated "set OPENAI_API_KEY / run coco init" recovery copy + // every other command gets, instead of letting a missing key surface + // only as a generic validationError further down (CMD-15). --json + // skips this: it needs a parseable stdout payload on failure, which + // the validationErrors branch below already provides via emitJson. + if (!argv.json) { + const draftConfig = loadConfig(argv) + if (draftConfig.service.authentication.type !== 'None' && !getApiKeyForModel(draftConfig)) { + handleMissingApiKey(logger, draftConfig, { command: 'commit' }) + } + } + const result = await generateCommitDraft({ git, argv, logger }) if (!result.ok || !result.draft) { + // logger.verbose no-ops unless --verbose is set, which made this + // path exit 1 with zero output for the exact non-interactive + // command the installed prepare-commit-msg hook runs (CMD-15). for (const warning of result.warnings) { - logger.verbose(warning, { color: 'yellow' }) + logger.error(warning, { color: 'yellow' }) } for (const validationError of result.validationErrors) { - logger.verbose(validationError, { color: 'red' }) + logger.error(validationError, { color: 'red' }) } if (argv.json) { // Machine consumers get a parseable error payload on stdout