From b7485f880304c3412507bce4f21d8603b2b4e76a Mon Sep 17 00:00:00 2001 From: Brian Love Date: Wed, 23 Sep 2026 12:19:58 -0700 Subject: [PATCH] ci: say why a Claude review execution failed Every Claude Review run since 2026-07-08 ~06:00 UTC has errored on its first API call (is_error: true, one turn, ~250ms, $0), so no PR has been AI-reviewed since. The last real review ran at 05:34 that day; the workflow did not change in between, so the cause is outside the repo. But the verifier from #1137 prints the same line for every failure, so the log cannot tell a revoked key from an empty balance or a retired model. On an execution that reports is_error: true, print the terminal result's error text: one line, capped at 300 characters, with anything key-shaped redacted before truncating. A finished review's result is the review itself, so nothing is printed unless is_error is true; #1137's rule against printing transcript contents still holds and is still tested. This PR runs its own copy of the verifier, so its own review job will name the cause. --- scripts/verify-claude-review.mjs | 23 ++++++++++++++++ scripts/verify-claude-review.spec.mjs | 38 +++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/scripts/verify-claude-review.mjs b/scripts/verify-claude-review.mjs index 98339098b..d84cba318 100644 --- a/scripts/verify-claude-review.mjs +++ b/scripts/verify-claude-review.mjs @@ -1,6 +1,27 @@ #!/usr/bin/env node import { readFileSync } from 'node:fs'; +const MAX_REASON_CHARS = 300; + +/** + * The error Claude Code reports when an execution fails, e.g. "API Error: 400 + * ... credit balance is too low" or "Invalid API key". Without it every + * failure reads identically, and a revoked key, an empty balance and a retired + * model are indistinguishable from the job log. + * + * Only for a result that reports is_error: true. A finished review's `result` + * is the review itself, which this script must never print. The text is + * collapsed to one line, bounded, and anything key-shaped is redacted. + */ +function errorReason(result) { + if (result?.is_error !== true || typeof result.result !== 'string') return ''; + return result.result + .replace(/\s+/g, ' ') + .replace(/sk-ant-[A-Za-z0-9_-]+/g, 'sk-ant-[redacted]') + .trim() + .slice(0, MAX_REASON_CHARS); +} + // The pinned action considers subtype alone, even when is_error is true. // Inspect its documented execution_file without logging the raw transcript. try { @@ -16,6 +37,8 @@ try { console.error( 'Claude review execution failed or did not finish. No completed review is verified.' ); + const reason = errorReason(result); + if (reason) console.error(`Reason: ${reason}`); process.exitCode = 1; } else { console.log('Claude review execution completed without a reported error.'); diff --git a/scripts/verify-claude-review.spec.mjs b/scripts/verify-claude-review.spec.mjs index b7a12883c..f06df06c8 100644 --- a/scripts/verify-claude-review.spec.mjs +++ b/scripts/verify-claude-review.spec.mjs @@ -59,6 +59,44 @@ for (const [name, content] of [ }); } +test('an errored execution states its reason, so the failure is diagnosable', () => { + // Claude Code exits on an API error with is_error: true and puts the error in + // the terminal result's `result` field. Without it, every failure reads the + // same and nobody can tell a revoked key from an empty balance. + const reason = + 'API Error: 400 {"type":"error","error":{"type":"invalid_request_error","message":"Your credit balance is too low to access the Anthropic API."}}'; + const result = verify(JSON.stringify([{ ...success, is_error: true, result: reason }])); + assert.equal(result.status, 1); + assert.match(result.stderr, /Claude review execution failed/); + assert.match(result.stderr, /Reason: API Error: 400 .*credit balance is too low/); +}); + +test('the reason is one bounded line with anything key-shaped redacted', () => { + const secret = 'sk-ant-api03-SECRETSECRETSECRET_1234567890'; + const long = `Invalid API key ${secret}\n${'x'.repeat(2000)}`; + const result = verify(JSON.stringify([{ ...success, is_error: true, result: long }])); + assert.equal(result.status, 1); + const line = result.stderr.split('\n').find((l) => l.startsWith('Reason: ')); + assert.ok(line, result.stderr); + assert.ok(line.length <= 'Reason: '.length + 300, `reason is ${line.length} chars`); + assert.doesNotMatch(result.stdout + result.stderr, /SECRETSECRETSECRET/); + assert.match(line, /sk-ant-\[redacted\]/); +}); + +test('a result that is not an error never prints its text', () => { + // A finished review's `result` is the review itself: transcript-grade + // content. Only an execution that reports is_error: true gets a reason line. + for (const content of [ + JSON.stringify([{ ...success, subtype: 'error_max_turns', result: 'private-transcript-do-not-print' }]), + JSON.stringify([{ type: 'result', subtype: 'success', result: 'private-transcript-do-not-print' }]), + ]) { + const result = verify(content); + assert.equal(result.status, 1); + assert.doesNotMatch(result.stdout + result.stderr, /private-transcript-do-not-print/); + assert.doesNotMatch(result.stderr, /Reason:/); + } +}); + test('rejects a missing action output', () => { const result = spawnSync(process.execPath, [script.pathname], { encoding: 'utf8',