fix(reviewer): a parse failure is an infra retry, not a quality reject → false STUCK (INT-2521) (#267)

When the reviewer RAN but adapter.parseReviewerOutput threw on malformed output, the
catch returned decision:'reject' — which discards the worker's work AND counts toward
the rejection→STUCK limit, turning a reviewer-side parse bug into a FALSE STUCK. (An
earlier 'revise' attempt was rejected: the CLI review reads a non-reject decision as
exit-0 success, and it spends the worker's revision budget on a reviewer-side problem.)

Now it throws a 'reviewer-stage:'-marked error (original preserved as cause): the
pipeline classifies it infra_error (backoff retry, no STUCK) and the CLI review exits
non-zero via its catch. Rate-limit / infra errors still propagate first, unchanged.

Found by the INT-2521 harness audit (finding P6). Tests: runReviewer rejects with the
reviewer-stage error when parseReviewerOutput throws (real call path); isInfraError
matches 'reviewer-stage:' but not prose. tsc clean + full vitest 1718. openswarm
review: APPROVE.

Author: unohee

src/adapters/errorClassification.test.ts

@@ -56,6 +56,12 @@ describe('isInfraError (INT-2010)', () => {
     expect(isInfraError(new Error('git-tracker: diff since snapshot failed: fatal: bad object'))).toBe(true);
   });
 
+  it('recognises a reviewer-stage parse failure as infra, not a quality reject (INT-2521)', () => {
+    expect(isInfraError(new Error('reviewer-stage: produced no parseable verdict: TypeError x'))).toBe(true);
+    // …but a task/review that merely discusses the reviewer stage is NOT infra:
+    expect(isInfraError(new Error('the reviewer stage should reject empty diffs'))).toBe(false);
+  });
+
   it('recognises local server capacity failures (5xx / loading / overloaded) (INT-2520)', () => {
     expect(isInfraError(new Error('Local API error (503): model is loading'))).toBe(true);
     expect(isInfraError(new Error('Local API error (502): bad gateway'))).toBe(true);

src/adapters/errorClassification.ts

@@ -34,6 +34,7 @@ const INFRA_ERROR_PATTERNS = [
   'socket hang up',
   'network error',
   'git-tracker:', // git snapshot/diff failed mid-run — infra, not a task verdict (colon-anchored to avoid prose) (INT-2521)
+  'reviewer-stage:', // reviewer ran but its output couldn't be parsed into a verdict — infra, not a quality reject (INT-2521)
   'fetch failed', // undici: the real code hides in error.cause.code (checked below)
   'terminated', // undici mid-stream socket drop
   'unauthorized',

src/agents/reviewer.test.ts

@@ -2,9 +2,31 @@
 // Purpose: Unit tests for reviewer module
 // Test Status: Complete
 
-import { describe, it, expect } from 'vitest';
-import { formatReviewFeedback, buildRevisionPrompt, type ReviewerOptions } from './reviewer.js';
+import { describe, it, expect, vi, afterEach, beforeAll } from 'vitest';
+import { formatReviewFeedback, buildRevisionPrompt, runReviewer, type ReviewerOptions } from './reviewer.js';
 import type { WorkerResult, ReviewResult } from './agentPair.js';
+import * as adapters from '../adapters/index.js';
+import { initLocale } from '../locale/index.js';
+
+describe('runReviewer parse failure (INT-2521)', () => {
+  beforeAll(() => { initLocale('en'); });
+  afterEach(() => { vi.restoreAllMocks(); });
+
+  it('rejects with a reviewer-stage infra error (not a quality reject) when parse throws', async () => {
+    // The reviewer RAN but its output couldn't be parsed → NOT a 'reject' (false STUCK)
+    // and NOT a 'revise' (CLI exit-0 success). It must throw the infra-marked error.
+    vi.spyOn(adapters, 'spawnCli').mockResolvedValue({ exitCode: 0, stdout: 'garbage', stderr: '', durationMs: 1 } as never);
+    vi.spyOn(adapters, 'getAdapter').mockReturnValue({
+      name: 'mock',
+      getDefaultModel: async () => 'm',
+      parseReviewerOutput: () => { throw new TypeError('cannot read property decision of undefined'); },
+    } as never);
+    const wr: WorkerResult = { success: true, summary: 's', filesChanged: ['a.ts'], commands: [], output: '' };
+    await expect(
+      runReviewer({ taskTitle: 't', taskDescription: 'd', workerResult: wr, projectPath: '/tmp/x' }),
+    ).rejects.toThrow(/reviewer-stage: produced no parseable verdict/);
+  });
+});
 
 describe('reviewer', () => {
   describe('formatReviewFeedback', () => {

src/agents/reviewer.ts

@@ -262,12 +262,16 @@ export async function runReviewer(options: ReviewerOptions): Promise<ReviewResul
     // as 'infra_error' instead of letting it masquerade as a 'reject' that
     // increments the rejection-limit STUCK counter. (INT-2010)
     if (isInfraError(error)) throw error;
-    return {
-      decision: 'reject',
-      feedback: `Reviewer execution failed: ${error instanceof Error ? error.message : String(error)}`,
-      issues: ['Error occurred during reviewer agent execution'],
-      suggestions: ['Manual review required'],
-    };
+    // Whatever is left ran the reviewer but produced NO usable verdict — most often
+    // adapter.parseReviewerOutput throwing on malformed output. This is NOT a quality
+    // 'reject' (a 'reject' discards the worker's work AND counts toward the
+    // rejection→STUCK limit, turning a reviewer-side parse bug into a false STUCK),
+    // and NOT a 'revise' (which the CLI reads as an exit-0 success and which spends
+    // the worker's revision budget on a reviewer-side problem). Throw an infra-marked
+    // error → the pipeline classifies infra_error (backoff retry, no STUCK) and the
+    // CLI exits non-zero. (INT-2521)
+    const msg = error instanceof Error ? error.message : String(error);
+    throw new Error(`reviewer-stage: produced no parseable verdict: ${msg}`, { cause: error });
   }
 }