From 4a99489e9aacb8bcfae7ccded52aefb933f1b207 Mon Sep 17 00:00:00 2001 From: Brian Love Date: Wed, 23 Sep 2026 07:48:58 -0700 Subject: [PATCH 1/2] fix(ci): surface failed Claude review executions --- .github/workflows/claude-review.yml | 13 +++- scripts/ci-workflow.spec.mjs | 1 + scripts/verify-claude-review.mjs | 28 ++++++++ scripts/verify-claude-review.spec.mjs | 98 +++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 scripts/verify-claude-review.mjs create mode 100644 scripts/verify-claude-review.spec.mjs diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 0c35c758c..85d1eb2ca 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -1,7 +1,7 @@ name: Claude Review -# Genuine, advisory AI code review on every PR. Posts findings as PR -# comments. NOT a required status check — it never blocks a merge. +# Advisory AI code review. Posts findings as PR comments. Execution failures +# fail this job; it remains outside the required CI status check. # Uses `pull_request` (not pull_request_target) so ANTHROPIC_API_KEY is # never exposed to fork PRs. @@ -14,6 +14,7 @@ permissions: jobs: review: + if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest timeout-minutes: 15 permissions: @@ -26,8 +27,8 @@ jobs: fetch-depth: 1 - name: Claude review + id: claude-review uses: anthropics/claude-code-action@806af32823ef69c8ef357086c573a902af641307 # v1 - continue-on-error: true with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} # Post as the workflow's own GITHUB_TOKEN. Without this the action @@ -66,3 +67,9 @@ jobs: --model claude-sonnet-4-6 --max-turns 30 --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)" + + - name: Verify review execution + if: ${{ !cancelled() }} + env: + CLAUDE_EXECUTION_FILE: ${{ steps['claude-review'].outputs.execution_file }} + run: node scripts/verify-claude-review.mjs diff --git a/scripts/ci-workflow.spec.mjs b/scripts/ci-workflow.spec.mjs index 3b7754b56..4dd97661a 100644 --- a/scripts/ci-workflow.spec.mjs +++ b/scripts/ci-workflow.spec.mjs @@ -3,6 +3,7 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; import { privateScaffoldProjects } from './react-parity/package-policy.mjs'; import { spawnSync } from 'node:child_process'; +import './verify-claude-review.spec.mjs'; function escapeRegExp(value) { return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); diff --git a/scripts/verify-claude-review.mjs b/scripts/verify-claude-review.mjs new file mode 100644 index 000000000..98339098b --- /dev/null +++ b/scripts/verify-claude-review.mjs @@ -0,0 +1,28 @@ +#!/usr/bin/env node +import { readFileSync } from 'node:fs'; + +// The pinned action considers subtype alone, even when is_error is true. +// Inspect its documented execution_file without logging the raw transcript. +try { + const messages = JSON.parse( + readFileSync(process.env.CLAUDE_EXECUTION_FILE, 'utf8') + ); + const result = Array.isArray(messages) ? messages.at(-1) : undefined; + if ( + result?.type !== 'result' || + result.subtype !== 'success' || + result.is_error !== false + ) { + console.error( + 'Claude review execution failed or did not finish. No completed review is verified.' + ); + process.exitCode = 1; + } else { + console.log('Claude review execution completed without a reported error.'); + } +} catch { + console.error( + 'Claude review execution could not be verified: execution output is missing or unreadable.' + ); + process.exitCode = 1; +} diff --git a/scripts/verify-claude-review.spec.mjs b/scripts/verify-claude-review.spec.mjs new file mode 100644 index 000000000..b7a12883c --- /dev/null +++ b/scripts/verify-claude-review.spec.mjs @@ -0,0 +1,98 @@ +import assert from 'node:assert/strict'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { test } from 'node:test'; + +const script = new URL('./verify-claude-review.mjs', import.meta.url); +const success = { type: 'result', subtype: 'success', is_error: false }; + +function verify(content) { + const directory = mkdtempSync(join(tmpdir(), 'claude-review-test-')); + try { + const executionFile = join(directory, 'execution.json'); + if (content !== undefined) writeFileSync(executionFile, content); + return spawnSync(process.execPath, [script.pathname], { + encoding: 'utf8', + env: { ...process.env, CLAUDE_EXECUTION_FILE: executionFile }, + }); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +} + +test('accepts a successful terminal execution result', () => { + const result = verify(JSON.stringify([{ type: 'system' }, success])); + assert.equal(result.status, 0, result.stderr); +}); + +for (const [name, content] of [ + [ + 'observed false success', + JSON.stringify([{ ...success, is_error: true, num_turns: 1 }]), + ], + ['turn limit', JSON.stringify([{ ...success, subtype: 'error_max_turns' }])], + [ + 'missing error flag', + JSON.stringify([{ type: 'result', subtype: 'success' }]), + ], + ['no result', JSON.stringify([{ type: 'system' }])], + ['nonterminal result', JSON.stringify([success, { type: 'assistant' }])], + ['empty transcript', '[]'], + ['wrong shape', JSON.stringify(success)], + ['null result', '[null]'], + ['malformed JSON', '{private-transcript-do-not-print'], + ['missing file', undefined], +]) { + test(`rejects ${name} without exposing transcript contents`, () => { + const result = verify(content); + assert.equal(result.status, 1); + assert.match( + result.stderr, + /Claude review execution could not be verified|Claude review execution failed/ + ); + assert.doesNotMatch( + result.stdout + result.stderr, + /private-transcript-do-not-print/ + ); + }); +} + +test('rejects a missing action output', () => { + const result = spawnSync(process.execPath, [script.pathname], { + encoding: 'utf8', + env: { ...process.env, CLAUDE_EXECUTION_FILE: '' }, + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /Claude review execution could not be verified/); +}); + +test('the advisory workflow checks execution and does not swallow failures', () => { + const workflow = readFileSync( + new URL('../.github/workflows/claude-review.yml', import.meta.url), + 'utf8' + ); + assert.doesNotMatch(workflow, /continue-on-error:\s*true/); + assert.match(workflow, /id: claude-review/); + assert.match( + workflow, + /CLAUDE_EXECUTION_FILE:.*steps\['claude-review'\]\.outputs\.execution_file/ + ); + assert.match(workflow, /run: node scripts\/verify-claude-review\.mjs/); + assert.match(workflow, /if:.*!cancelled\(\)/); + assert.match( + workflow, + /if: github\.event\.pull_request\.head\.repo\.full_name == github\.repository/ + ); + const ci = readFileSync( + new URL('../.github/workflows/ci.yml', import.meta.url), + 'utf8' + ); + assert.match(ci, /run: node --test scripts\/ci-workflow\.spec\.mjs/); + const suite = readFileSync( + new URL('./ci-workflow.spec.mjs', import.meta.url), + 'utf8' + ); + assert.match(suite, /import '\.\/verify-claude-review\.spec\.mjs'/); +}); From 9d051ed686d3c4793a8b201417b9eaff0417fbd9 Mon Sep 17 00:00:00 2001 From: Brian Love Date: Wed, 23 Sep 2026 07:54:49 -0700 Subject: [PATCH 2/2] fix(ci): keep review regressions in the Node test runner --- scripts/vite.config.mts | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/vite.config.mts b/scripts/vite.config.mts index e56eb6cee..dd400d374 100644 --- a/scripts/vite.config.mts +++ b/scripts/vite.config.mts @@ -20,6 +20,7 @@ export default defineConfig({ // node:test suites — run by ci.yml directly, not by vitest. 'ci-scope.spec.mjs', 'ci-workflow.spec.mjs', + 'verify-claude-review.spec.mjs', 'cockpit-matrix.spec.mjs', 'cockpit-ports.spec.mjs', 'cockpit-runtime-bridge-coverage.spec.mjs',