Only fail aggregate run on infrastructure error

README.md

@@ -206,7 +206,7 @@ ANTHROPIC_CUSTOM_HEADERS=X-Workspace: code-review
 
 #### Outputs
 
-`pass`, `flagged`, `neutral`, and `total-cost`. Result counts distinguish findings (`flagged`) from operational misses (`neutral`). `total-cost` is empty when any contributing cost is unknown. Reviews are posted inline as `COMMENT` reviews; resolution and deduplication judges always run when they have work. These behaviors are fixed, not configurable. There are no per-invocation cost ceilings.
+`pass`, `flagged`, `neutral`, and `total-cost`. Result counts distinguish findings (`flagged`) from non-failing outcomes (`neutral`), including unusable model output; each neutral check result includes a `cause` of `infrastructure` or `invalid_output`. The aggregate check fails only for infrastructure errors. `total-cost` is empty when any contributing cost is unknown. Reviews are posted inline as `COMMENT` reviews; resolution and deduplication judges always run when they have work. These behaviors are fixed, not configurable. There are no per-invocation cost ceilings.
 
 ### Local CLI
 
@@ -233,7 +233,7 @@ WEAVE_CHECKS_PROVIDER_ENV='ANTHROPIC_BASE_URL=https://llm-gateway.example.com' \
 npx @weave-os/checks run --provider anthropic --checks-dir .weave-checks --base origin/main --format markdown
 ```
 
-`run` exits 1 when at least one check flags findings, 2 for invalid options or setup, and 0 when there are no findings. Use `--no-fail` to report findings without a non-zero exit. Neutral operational misses do not fail the run. `--format` supports `text`, `markdown`, and `json`; `--output <file>` writes the JSON result as well. Prompts, transcripts, and per-check results are stored in a temporary directory by default; `--artifacts-dir` keeps them at the path you choose. These artifacts contain the diff and agent output, so handle them as repository data.
+`run` exits 1 when at least one check flags findings, 2 for invalid options or setup, and 0 when there are no findings. Use `--no-fail` to report findings without a non-zero exit. Neutral outcomes do not fail the run. `--format` supports `text`, `markdown`, and `json`; `--output <file>` writes the JSON result as well. Prompts, transcripts, and per-check results are stored in a temporary directory by default; `--artifacts-dir` keeps them at the path you choose. These artifacts contain the diff and agent output, so handle them as repository data.
 
 `list --format json` prints the validated matrix. `print-schema` prints the result schema; `print-schema --kind resolution` and `--kind dedup` print the judge schemas.
 

src/ci.mjs

@@ -3,7 +3,7 @@
 //
 // Each is a small function over an injected REST client so the contract the
 // action relies on -- creation before checkout, a cleanup that never
-// overwrites the worker's own close, and failure on incomplete reviews -- is
+// overwrites the worker's own close, and failure on infrastructure errors -- is
 // unit tested (ci.test.mjs).
 
 import { appendFileSync, existsSync, readFileSync } from "node:fs";
@@ -34,9 +34,10 @@ export async function createAggregate({ rest, repository, headSha, aggregateName
 // When the worker reached its own close (success or its top-level catch), it
 // wrote `completePath` and this is a no-op, so the worker's specific message
 // is never overwritten with this generic one. Otherwise -- a crash in
-// checkout, preparation, discovery, or before the worker could PATCH --
-// close it as failure so branch protection cannot accept a run that never
-// produced a review. Findings remain neutral; only incomplete reviews fail.
+// checkout, preparation, or before the worker could PATCH because of an
+// infrastructure/setup failure -- close it as failure so branch protection
+// cannot accept a run that never reached the coordinator. Findings and
+// unusable model output remain neutral when the worker finishes normally.
 export async function closeAggregate({
   rest,
   repository,

src/parse.mjs

@@ -599,6 +599,11 @@ export const OUTCOME = Object.freeze({
   FAIL: "fail",
 });
 
+export const NEUTRAL_CAUSE = Object.freeze({
+  INFRASTRUCTURE: "infrastructure",
+  INVALID_OUTPUT: "invalid_output",
+});
+
 // The outcome names published in JSON results and action outputs. Internally
 // a FAIL verdict is OUTCOME.FAIL, but it never publishes as a failed check
 // (see worker.mjs's header), so the public vocabulary says what it is: the
@@ -659,8 +664,8 @@ export const CLI_RESULT_SUBTYPE = Object.freeze({
 
 // Closed set of interpretResult() outcomes. OK means the verdict object is
 // ready to validate; RETRYABLE means the CLI succeeded but skipped
-// structured output (worth one more ask); DEFINITE is a crash or parse
-// error that would just repeat.
+// structured output (worth one more ask); DEFINITE is not worth retrying.
+// `cause` separates infrastructure failures from unusable model output.
 export const INTERPRET_OUTCOME = Object.freeze({
   OK: "ok",
   RETRYABLE: "retryable",
@@ -676,8 +681,8 @@ export const GITHUB_CONCLUSION = Object.freeze({
 // HTTP verbs the worker's GitHub REST wrapper actually sends. Kept next to
 // GITHUB_CONCLUSION so callers (worker.mjs's github()) restate nothing as
 // raw strings -- a typo "GETT" would silently 404 instead of failing loudly.
-// Operational misses and coordinator failures publish `failure`, while a
-// completed review with findings remains `neutral`.
+// Infrastructure and coordinator failures publish `failure`; findings and
+// unusable model output publish `neutral`.
 export const HTTP_METHOD = Object.freeze({
   GET: "GET",
   POST: "POST",
@@ -807,6 +812,7 @@ export function interpretResult(cli) {
   ) {
     return {
       outcome: INTERPRET_OUTCOME.DEFINITE,
+      cause: NEUTRAL_CAUSE.INFRASTRUCTURE,
       value: { error: `Claude reported ${cli?.subtype ?? cli?.api_error_status ?? "an error"}` },
     };
   }
@@ -817,6 +823,7 @@ export function interpretResult(cli) {
   } catch (error) {
     return {
       outcome: INTERPRET_OUTCOME.DEFINITE,
+      cause: NEUTRAL_CAUSE.INVALID_OUTPUT,
       value: { error: `structured_output was a string but was not valid JSON: ${error.message}` },
     };
   }
@@ -835,6 +842,7 @@ export function interpretResult(cli) {
     } catch {
       return {
         outcome: INTERPRET_OUTCOME.RETRYABLE,
+        cause: NEUTRAL_CAUSE.INVALID_OUTPUT,
         value: {
           error:
             "structured_output was missing and the prose result did not contain a verdict JSON object",
@@ -846,6 +854,7 @@ export function interpretResult(cli) {
 
   return {
     outcome: INTERPRET_OUTCOME.RETRYABLE,
+    cause: NEUTRAL_CAUSE.INVALID_OUTPUT,
     value: {
       error: "structured_output was missing and the prose result had no verdict",
       rawResult: cli.result,

src/parse.test.mjs

@@ -30,6 +30,7 @@ import {
   isCheckFile,
   parseIgnoreList,
   OUTCOME,
+  NEUTRAL_CAUSE,
   parseAddedLines,
   parseCheckFile,
   parseStructuredOutput,
@@ -667,6 +668,7 @@ describe("interpretResult", () => {
     };
     const interpreted = interpretResult(cli);
     assert.equal(interpreted.outcome, "retryable");
+    assert.equal(interpreted.cause, NEUTRAL_CAUSE.INVALID_OUTPUT);
     assert.match(interpreted.value.error, /structured_output was missing/);
     assert.equal(interpreted.value.rawResult, cli.result);
   });
@@ -677,7 +679,9 @@ describe("interpretResult", () => {
     // a schema/serialization bug, not a skipped tool call, so a retry isn't
     // expected to help.
     const cli = { subtype: "success", structured_output: "{not json" };
-    assert.equal(interpretResult(cli).outcome, "definite");
+    const interpreted = interpretResult(cli);
+    assert.equal(interpreted.outcome, "definite");
+    assert.equal(interpreted.cause, NEUTRAL_CAUSE.INVALID_OUTPUT);
   });
 
   it("recovers a verdict JSON literal embedded in prose as ok", () => {
@@ -728,17 +732,21 @@ describe("interpretResult", () => {
     const cli = { subtype: "error_max_turns" };
     const interpreted = interpretResult(cli);
     assert.equal(interpreted.outcome, "definite");
+    assert.equal(interpreted.cause, NEUTRAL_CAUSE.INFRASTRUCTURE);
     assert.match(interpreted.value.error, /error_max_turns/);
   });
 
   it("is definite when is_error is true even with subtype success", () => {
     const cli = { subtype: "success", is_error: true };
     assert.equal(interpretResult(cli).outcome, "definite");
+    assert.equal(interpretResult(cli).cause, NEUTRAL_CAUSE.INFRASTRUCTURE);
   });
 
-  it("is definite for a null or non-object cli", () => {
+  it("is definite for a null or non-object cli and marks it as infrastructure", () => {
     assert.equal(interpretResult(null).outcome, "definite");
+    assert.equal(interpretResult(null).cause, NEUTRAL_CAUSE.INFRASTRUCTURE);
     assert.equal(interpretResult(undefined).outcome, "definite");
+    assert.equal(interpretResult(undefined).cause, NEUTRAL_CAUSE.INFRASTRUCTURE);
   });
 
   it("is retryable when structured_output is an array, not an object", () => {

src/runner.mjs

@@ -30,6 +30,7 @@ import process from "node:process";
 import {
   CLI_RESULT_SUBTYPE,
   INTERPRET_OUTCOME,
+  NEUTRAL_CAUSE,
   OUTCOME,
   VERDICT,
   interpretResult,
@@ -294,6 +295,7 @@ export function normalize(invocation, addedLines) {
     // failed invocation as free.
     return {
       outcome: OUTCOME.NEUTRAL,
+      cause: NEUTRAL_CAUSE.INFRASTRUCTURE,
       error: exitError(invocation),
       cost: invocationCost(invocation),
       duration: invocationDuration(invocation),
@@ -306,6 +308,7 @@ export function normalize(invocation, addedLines) {
   if (invocation.cli === null) {
     return {
       outcome: OUTCOME.NEUTRAL,
+      cause: NEUTRAL_CAUSE.INFRASTRUCTURE,
       error: "Claude stream-json output had no terminal result event",
       cost: invocationCost(invocation),
       duration: null,
@@ -328,12 +331,19 @@ export function normalize(invocation, addedLines) {
     return interpreted.outcome === INTERPRET_OUTCOME.RETRYABLE ?
         {
           outcome: OUTCOME.NEUTRAL,
+          cause: interpreted.cause,
           ...interpreted.value,
           retryable: true,
           cost,
           duration,
         }
-      : { outcome: OUTCOME.NEUTRAL, ...interpreted.value, cost, duration };
+      : {
+          outcome: OUTCOME.NEUTRAL,
+          cause: interpreted.cause,
+          ...interpreted.value,
+          cost,
+          duration,
+        };
   }
   // The structured_output route (the common case once the model cooperates)
   // needs the full result validated against the diff. interpretResult already
@@ -361,6 +371,7 @@ export function validateAndFinalize(resultObject, invocation, cli, addedLines) {
     // neutral run from a parse-time miss doesn't look free.
     return {
       outcome: OUTCOME.NEUTRAL,
+      cause: NEUTRAL_CAUSE.INVALID_OUTPUT,
       error: `Invalid structured result: ${error.message}`,
       cost,
       duration,
@@ -370,6 +381,7 @@ export function validateAndFinalize(resultObject, invocation, cli, addedLines) {
   if (validated.verdict === VERDICT.FAIL && validated.accepted.length === 0) {
     return {
       outcome: OUTCOME.NEUTRAL,
+      cause: NEUTRAL_CAUSE.INVALID_OUTPUT,
       error: "FAIL had no valid anchored findings after diff validation.",
       proseFallbacks: validated.proseFallbacks,
       rejected: validated.rejected,

src/runner.test.mjs

@@ -5,7 +5,14 @@ import path from "node:path";
 import process from "node:process";
 import { afterEach, describe, it } from "node:test";
 
-import { CLI_RESULT_SUBTYPE, CLUSTER_LOW, MODEL_HAIKU, OUTCOME, VERDICT } from "./parse.mjs";
+import {
+  CLI_RESULT_SUBTYPE,
+  CLUSTER_LOW,
+  MODEL_HAIKU,
+  NEUTRAL_CAUSE,
+  OUTCOME,
+  VERDICT,
+} from "./parse.mjs";
 import {
   evaluateCheck,
   formatDuration,
@@ -414,6 +421,7 @@ describe("runClaude", () => {
 
     assert.equal(invocation.code, 3);
     assert.equal(result.outcome, OUTCOME.NEUTRAL);
+    assert.equal(result.cause, NEUTRAL_CAUSE.INFRASTRUCTURE);
     assert.equal(result.error, "Claude CLI exited 3");
   });
 
@@ -436,6 +444,7 @@ describe("runClaude", () => {
     const result = normalize(invocation, new Map());
 
     assert.equal(result.outcome, OUTCOME.NEUTRAL);
+    assert.equal(result.cause, NEUTRAL_CAUSE.INFRASTRUCTURE);
     assert.equal(result.error, "Claude CLI exited 1: Prompt is too long");
   });
 });
@@ -459,6 +468,14 @@ describe("normalize", () => {
     };
   }
 
+  it("keeps malformed structured output neutral and classifies it as output", () => {
+    const result = normalize(invocation(cli("{not json")), addedLines);
+
+    assert.equal(result.outcome, OUTCOME.NEUTRAL);
+    assert.equal(result.cause, NEUTRAL_CAUSE.INVALID_OUTPUT);
+    assert.match(result.error, /was not valid JSON/);
+  });
+
   it("accepts a PASS verdict", () => {
     const result = normalize(
       invocation(cli({ verdict: VERDICT.PASS, reason: "no violations" })),
@@ -533,6 +550,7 @@ describe("normalize", () => {
       addedLines,
     );
     assert.equal(result.outcome, OUTCOME.NEUTRAL);
+    assert.equal(result.cause, NEUTRAL_CAUSE.INVALID_OUTPUT);
     assert.match(result.error, /no valid anchored findings/);
     assert.equal(result.proseFallbacks.length, 0);
     assert.equal(result.rejected.length, 1);
@@ -543,6 +561,7 @@ describe("normalize", () => {
   it("treats a missing terminal result event as neutral with an unknown duration", () => {
     const result = normalize(invocation(null), addedLines);
     assert.equal(result.outcome, OUTCOME.NEUTRAL);
+    assert.equal(result.cause, NEUTRAL_CAUSE.INFRASTRUCTURE);
     assert.match(result.error, /no terminal result event/);
     assert.equal(result.duration, null);
   });
@@ -561,6 +580,7 @@ describe("normalize", () => {
       addedLines,
     );
     assert.equal(result.outcome, OUTCOME.NEUTRAL);
+    assert.equal(result.cause, NEUTRAL_CAUSE.INVALID_OUTPUT);
     assert.equal(result.retryable, true);
   });
 });
@@ -574,6 +594,7 @@ describe("validateAndFinalize", () => {
       new Map(),
     );
     assert.equal(result.outcome, OUTCOME.NEUTRAL);
+    assert.equal(result.cause, NEUTRAL_CAUSE.INVALID_OUTPUT);
     assert.match(result.error, /Invalid structured result/);
     assert.equal(result.cost, 0.25);
     assert.equal(result.duration, 400);