Report failed Claude review executions honestly

.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

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, '\\$&');

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;
+}

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'/);
+});

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',