fix(ci): stop one junk context result discarding a proven review

Tri-review of this PR found that the previous commit fixed one spurious
rejection and created another. Widening evidence candidacy from "the call
that named a changed path" to "every orchestrator context call" also
widened the *strict-parse* surface: `contextResultProvesChangedPath`
throws rather than returning false, so a single malformed payload
anywhere in the transcript now discarded a review that an earlier call
had already proven. The MCP makes that reachable without any misbehaving
model - `GITNEXUS_MCP_DEFAULT_MAX_TOKENS=12000` truncates any context
payload over ~48 KB mid-JSON and appends a marker - and it also destroyed
docs-only runs that the `no_indexable_changed_symbols` mode exempts.

Reproduced by running the workflow's own embedded script on both trees:
a proving evidence call followed by one truncated exploratory call gave
`failure_code: null` on the base and `invalid_execution_transcript` on
the head; it is `null` again here.

- payload-shape failures are caught and counted (`malformedResults`)
  instead of thrown; transcript-structural invariants (envelope, tool
  shapes, duplicate ids, empty tool_result) still fail closed
- diagnostics gained the reasons they were blind to: errored results,
  results that arrived out of order or via a sidechain, unanswered
  in-scope calls, and malformed payloads. A rejection can no longer
  print an in-scope call with every reason at zero
- a deletion-only PR no longer registers head-scoped candidates that can
  never be satisfied: an empty eligible set is out of scope, not a result
  "outside the changed paths"
- the mandatory-body prompt clause now pairs with a required `complete`
  boolean. An incomplete analysis publishes its partial body labelled
  `incomplete_analysis` instead of passing as an accepted review
- `Agent(a,b,c)` is split into six separate `Agent(x)` rules: the pinned
  base action parses allowedTools with `.flatMap((v) => v.split(","))`
  (parse-sdk-options.ts at 3553f843), which shattered the grouped rule
  into `Agent(ci-correctness-lens`, four bare names, and
  `ci-critic-lens)` before the SDK saw it. Pre-existing and unproven at
  runtime, but the split form is correct under either reading and lets
  the header's dispatch canary actually prove something

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

.github/workflows/gitnexus-review-agent.yml

@@ -1335,6 +1335,12 @@ jobs:
             Always end the run by returning that structured body field, even when a
             lane fails, a query comes back empty, or the analysis is incomplete —
             describe the gap inside the review instead of finishing without output.
+            Also return the boolean field complete: true only when you actually
+            finished the review you were asked for, and false whenever a lane
+            failed, a needed query never resolved, or you ran out of turns. A
+            false value still publishes the partial review, but labelled as
+            incomplete rather than accepted — never report true to make the run
+            look clean.
           claude_args: |
             --model claude-sonnet-5
             --add-dir "${{ runner.temp }}/gitnexus-review-pr-target"
@@ -1343,12 +1349,12 @@ jobs:
             --strict-mcp-config
             --mcp-config "${{ runner.temp }}/gitnexus-review-mcp.json"
             --tools "Read,Agent"
-            --allowedTools "Agent(ci-correctness-lens,ci-security-lens,ci-blast-radius-lens,ci-coverage-lens,ci-adversarial-lens,ci-critic-lens),Read(./**),Read(${{ runner.temp }}/gitnexus-review-pr-target/**),Read(${{ runner.temp }}/gitnexus-review-merge-base/**),mcp__gitnexus__list_repos,mcp__gitnexus__query,mcp__gitnexus__context,mcp__gitnexus__check,mcp__gitnexus__impact,mcp__gitnexus__explain,mcp__gitnexus__pdg_query,mcp__gitnexus__route_map,mcp__gitnexus__tool_map,mcp__gitnexus__shape_check,mcp__gitnexus__api_impact,mcp__gitnexus__trace"
+            --allowedTools "Agent(ci-correctness-lens),Agent(ci-security-lens),Agent(ci-blast-radius-lens),Agent(ci-coverage-lens),Agent(ci-adversarial-lens),Agent(ci-critic-lens),Read(./**),Read(${{ runner.temp }}/gitnexus-review-pr-target/**),Read(${{ runner.temp }}/gitnexus-review-merge-base/**),mcp__gitnexus__list_repos,mcp__gitnexus__query,mcp__gitnexus__context,mcp__gitnexus__check,mcp__gitnexus__impact,mcp__gitnexus__explain,mcp__gitnexus__pdg_query,mcp__gitnexus__route_map,mcp__gitnexus__tool_map,mcp__gitnexus__shape_check,mcp__gitnexus__api_impact,mcp__gitnexus__trace"
             --disallowedTools "Bash,Write,Edit,MultiEdit,NotebookEdit,WebFetch,WebSearch,Skill,Read(/proc/**),Read(/sys/**),Read(/dev/**),Read(${{ github.workspace }}/**),mcp__github,mcp__gitnexus__detect_changes,mcp__gitnexus__rename,mcp__gitnexus__cypher,mcp__gitnexus__group_list,mcp__gitnexus__group_sync"
             --permission-mode dontAsk
             --no-session-persistence
             --max-turns 150
-            --json-schema '{"type":"object","properties":{"body":{"type":"string","maxLength":50000}},"required":["body"],"additionalProperties":false}'
+            --json-schema '{"type":"object","properties":{"body":{"type":"string","maxLength":50000},"complete":{"type":"boolean"}},"required":["body","complete"],"additionalProperties":false}'
 
       - name: Assemble bounded review artifact
         id: artifact
@@ -1413,6 +1419,8 @@ jobs:
             index_failed: 'The review was not run because the exact-head graph index could not be built safely.',
             model_failed: 'The review agent did not produce a valid structured result.',
             invalid_model_output: 'The review agent returned an invalid structured result.',
+            incomplete_analysis:
+              'The review agent reported that it could not complete this analysis, so the partial review below is published for diagnosis rather than accepted as a review.',
             invalid_execution_transcript: 'The review execution transcript failed strict validation, so no model review was accepted.',
             missing_graph_evidence: 'The review execution did not prove a successful GitNexus context result for a symbol in an exact changed file.',
           };
@@ -1674,13 +1682,16 @@ jobs:
 
             const headRepo = path.join(process.env.GITHUB_WORKSPACE, 'pr-target');
             const baseRepo = path.join(process.env.RUNNER_TEMP, 'gitnexus-review-merge-base');
-            if (!Object.hasOwn(input, 'repo') || input.repo === headRepo) {
+            // An empty set can never be satisfied (a deletion-only PR has no
-              return changedPathManifest.headPaths;
+            // head paths), so such a call is out of scope rather than a
-            }
+            // candidate whose every result reads as "outside the changed paths".
-            if (input.repo === baseRepo) {
+            const scoped =
-              return changedPathManifest.baseEvidencePaths;
+              !Object.hasOwn(input, 'repo') || input.repo === headRepo
-            }
+                ? changedPathManifest.headPaths
-            return undefined;
+                : input.repo === baseRepo
+                  ? changedPathManifest.baseEvidencePaths
+                  : undefined;
+            return scoped && scoped.size > 0 ? scoped : undefined;
           }
 
           function validateToolResultContent(content) {
@@ -1712,6 +1723,14 @@ jobs:
             throw new Error('context tool result is not text');
           }
 
+          // Payload-shape failures are NOT transcript corruption. Every
+          // orchestrator context call is a candidate now, so an ordinary
+          // exploratory call whose result the MCP truncated at
+          // GITNEXUS_MCP_DEFAULT_MAX_TOKENS (mid-JSON, marker appended) would
+          // otherwise throw and discard a review an earlier call already
+          // proved. This throws only what the caller converts into a counted
+          // non-evidence result; structural transcript invariants still throw
+          // hard from proveGraphReview.
           function contextResultProvesChangedPath(content, eligiblePaths, rejected) {
             const text = decodeTextToolResult(content).trim();
             if (!text) throw new Error('context tool result is empty');
@@ -1784,7 +1803,11 @@ jobs:
               samples: [],
               sidechainCalls: 0,
               outOfScopeCalls: 0,
+              erroredResults: 0,
+              malformedResults: 0,
+              unusableResults: 0,
             };
+            const answeredCalls = new Set();
             const candidateCalls = new Map();
             const successfulResults = new Map();
             const seenToolCalls = new Set();
@@ -1886,14 +1909,25 @@ jobs:
                   }
                   seenToolResults.add(block.tool_use_id);
                   const candidate = candidateCalls.get(block.tool_use_id);
-                  if (
+                  if (candidate && (sidechain || messageIndex <= candidate.messageIndex)) {
-                    !sidechain &&
+                    rejected.unusableResults += 1;
-                    block.is_error !== true &&
+                  } else if (candidate && block.is_error === true) {
-                    candidate &&
+                    rejected.erroredResults += 1;
-                    messageIndex > candidate.messageIndex &&
+                  } else if (candidate) {
-                    contextResultProvesChangedPath(block.content, candidate.eligiblePaths, rejected)
+                    answeredCalls.add(block.tool_use_id);
-                  ) {
+                    let proved = false;
-                    successfulResults.set(block.tool_use_id, messageIndex);
+                    try {
+                      proved = contextResultProvesChangedPath(
+                        block.content,
+                        candidate.eligiblePaths,
+                        rejected,
+                      );
+                    } catch {
+                      // A malformed or truncated payload means this call is not
+                      // the evidence call — never that the transcript is corrupt.
+                      rejected.malformedResults += 1;
+                    }
+                    if (proved) successfulResults.set(block.tool_use_id, messageIndex);
                   }
                 }
               }
@@ -1911,7 +1945,12 @@ jobs:
                 `orchestrator context calls out of scope (no selector or unknown repo): ` +
                 `${rejected.outOfScopeCalls}; ` +
                 `sidechain context calls ignored: ${rejected.sidechainCalls}; ` +
+                `in-scope calls with no usable result: ` +
+                `${candidateCalls.size - answeredCalls.size}` +
+                ` (errored ${rejected.erroredResults}, out of order or sidechained ` +
+                `${rejected.unusableResults}); ` +
                 `results that resolved nothing: ${rejected.unresolved}; ` +
+                `results too malformed or truncated to parse: ${rejected.malformedResults}; ` +
                 `results outside the changed paths: ${rejected.offPath}` +
                 (rejected.samples.length > 0 ? ` (${rejected.samples.join(', ')})` : ''),
               headHasIndexableSymbol:
@@ -1987,22 +2026,32 @@ jobs:
                     if (
                       !parsed ||
                       Array.isArray(parsed) ||
-                      Object.keys(parsed).length !== 1 ||
+                      Object.keys(parsed).length !== 2 ||
                       typeof parsed.body !== 'string' ||
-                      parsed.body.trim().length === 0
+                      parsed.body.trim().length === 0 ||
+                      typeof parsed.complete !== 'boolean'
                     ) {
                       throw new Error('structured output shape mismatch');
                     }
-                    status = 'success';
+                    // The prompt asks for a body even when the analysis could
-                    failureCode = 'none';
+                    // not finish, so completeness must be reported separately —
-                    graphEvidenceMode = {
+                    // otherwise a degraded run publishes as an accepted review.
-                      mode: graphEvidence.hasContextEvidence
+                    if (parsed.complete) {
-                        ? 'context'
+                      status = 'success';
-                        : 'no_indexable_changed_symbols',
+                      failureCode = 'none';
-                      head_has_indexable_symbol: graphEvidence.headHasIndexableSymbol,
+                      graphEvidenceMode = {
-                      base_has_indexable_symbol: graphEvidence.baseHasIndexableSymbol,
+                        mode: graphEvidence.hasContextEvidence
-                    };
+                          ? 'context'
-                    body = parsed.body;
+                          : 'no_indexable_changed_symbols',
+                        head_has_indexable_symbol: graphEvidence.headHasIndexableSymbol,
+                        base_has_indexable_symbol: graphEvidence.baseHasIndexableSymbol,
+                      };
+                      body = parsed.body;
+                    } else {
+                      failureCode = 'incomplete_analysis';
+                      body = `${failureMessages.incomplete_analysis}\n\n${parsed.body}`;
+                      console.error('Review rejected: the model reported an incomplete analysis.');
+                    }
                   } catch {
                     failureCode = 'invalid_model_output';
                     body = failureMessages[failureCode];

gitnexus/test/unit/review-agent-workflow.test.ts

@@ -326,6 +326,68 @@ function reviewTranscript({
   ];
 }
 
+// Two orchestrator context calls in one turn: the first proves the evidence,
+// the second is an ordinary exploratory call whose result may be junk.
+function twoCallTranscript({
+  firstResult,
+  secondResult,
+}: {
+  firstResult: string;
+  secondResult: string;
+}): Array<Record<string, unknown>> {
+  return [
+    {
+      type: 'system',
+      subtype: 'init',
+      session_id: 'session-1',
+      uuid: '11111111-1111-4111-8111-111111111111',
+    },
+    {
+      type: 'assistant',
+      parent_tool_use_id: null,
+      session_id: 'session-1',
+      uuid: '22222222-2222-4222-8222-222222222222',
+      message: {
+        role: 'assistant',
+        content: [
+          {
+            type: 'tool_use',
+            id: 'tool-1',
+            name: 'mcp__gitnexus__context',
+            input: { name: 'statusCommand' },
+          },
+          {
+            type: 'tool_use',
+            id: 'tool-2',
+            name: 'mcp__gitnexus__context',
+            input: { name: 'bigHotSymbol' },
+          },
+        ],
+      },
+    },
+    {
+      type: 'user',
+      parent_tool_use_id: null,
+      session_id: 'session-1',
+      uuid: '33333333-3333-4333-8333-333333333333',
+      message: {
+        role: 'user',
+        content: [
+          { type: 'tool_result', tool_use_id: 'tool-1', is_error: false, content: firstResult },
+          { type: 'tool_result', tool_use_id: 'tool-2', is_error: false, content: secondResult },
+        ],
+      },
+    },
+    {
+      type: 'result',
+      subtype: 'success',
+      is_error: false,
+      session_id: 'session-1',
+      uuid: '44444444-4444-4444-8444-444444444444',
+    },
+  ];
+}
+
 function reviewTranscriptWithoutTools(): Array<Record<string, unknown>> {
   return [
     {
@@ -362,7 +424,7 @@ function runArtifactScenario({
   executionFileOutput,
   noIndexableChangedSymbols = false,
   rawTranscript = JSON.stringify(reviewTranscript()),
-  structuredOutput = JSON.stringify({ body: 'Accepted graph-backed review' }),
+  structuredOutput = JSON.stringify({ body: 'Accepted graph-backed review', complete: true }),
 }: ArtifactScenario = {}) {
   const script = embeddedNodeScript('analyze', 'Assemble bounded review artifact');
   const runnerTemp = mkdtempSync(path.join(tmpdir(), 'gitnexus-review-artifact-'));
@@ -1175,9 +1237,13 @@ describe('gitnexus review-agent workflow security contract', () => {
     // control-SHA personas; Agent is not bare-denied (deny beats allow), and
     // lane calls cannot satisfy the evidence gate.
     expect(analyze).toContain('--tools "Read,Agent"');
-    expect(allowedTools).toContain(
+    // One rule per persona, never a grouped Agent(a,b,c): the pinned base
-      'Agent(ci-correctness-lens,ci-security-lens,ci-blast-radius-lens,ci-coverage-lens,ci-adversarial-lens,ci-critic-lens)',
+    // action parses allowedTools with `.flatMap((v) => v.split(","))`, which
-    );
+    // would shatter a grouped rule into `Agent(ci-correctness-lens`, bare
+    // names, and `ci-critic-lens)` before the SDK ever sees it.
+    expect(allowedToolRules).toContain('Agent(ci-correctness-lens)');
+    expect(allowedToolRules).toContain('Agent(ci-critic-lens)');
+    expect(allowedTools).not.toMatch(/Agent\([^)]*,/);
     expect(allowedTools).not.toContain('Task');
     const disallowedTools = analyze.match(/--disallowedTools "([^"]+)"/)?.[1] ?? '';
     const disallowedToolRules = disallowedTools.split(',');
@@ -1218,12 +1284,12 @@ describe('gitnexus review-agent workflow security contract', () => {
     // parse time), so the canary is the acceptance gate for that. What a unit
     // test CAN pin is that the scoped allowlist, the persona filenames, and each
     // persona's frontmatter name are the same set — catching a rename or typo in
-    // any of the three without auth.
+    // any of the three without auth. Names are read one-per-rule because the
+    // pinned action splits allowedTools on commas.
     const analyze = jobBlock('analyze');
     const allowed = analyze.match(/--allowedTools "([^"]+)"/)?.[1] ?? '';
-    const allowlistNames = (allowed.match(/Agent\(([^)]+)\)/)?.[1] ?? '')
+    const allowlistNames = [...allowed.matchAll(/Agent\(([^),]+)\)/g)]
-      .split(',')
+      .map((match) => match[1].trim())
-      .map((name) => name.trim())
       .sort();
 
     const personasDir = path.resolve(
@@ -1618,7 +1684,11 @@ describe('gitnexus review-agent workflow security contract', () => {
         }),
       ),
     });
-    expect(uidOnly.artifact.failure_code).toBeNull();
+    expect(uidOnly.artifact).toMatchObject({
+      status: 'success',
+      failure_code: null,
+      graph_evidence: { mode: 'context' },
+    });
 
     const noSelector = runArtifactScenario({
       rawTranscript: JSON.stringify(reviewTranscript({ toolInput: { kind: 'Function' } })),
@@ -1659,6 +1729,92 @@ describe('gitnexus review-agent workflow security contract', () => {
     expect(noCalls.stderr).toContain(
       'orchestrator context calls out of scope (no selector or unknown repo): 0',
     );
+
+    // An in-scope call whose result errored must not read as "no calls made".
+    const erroredResult = runArtifactScenario({
+      rawTranscript: JSON.stringify(reviewTranscript({ resultIsError: true })),
+    });
+    expect(erroredResult.stderr).toContain('in-scope calls with no usable result: 1 (errored 1');
+
+    const unresolved = runArtifactScenario({
+      rawTranscript: JSON.stringify(
+        reviewTranscript({
+          toolResultContent: `${JSON.stringify({ error: "Symbol 'x' not found" })}\n\n---\n**Next:** retry.`,
+        }),
+      ),
+    });
+    expect(unresolved.stderr).toContain('results that resolved nothing: 1');
+  });
+
+  it('treats a malformed context payload as non-evidence, not as a corrupt transcript', () => {
+    // The MCP truncates any context payload over GITNEXUS_MCP_DEFAULT_MAX_TOKENS
+    // mid-JSON. Every orchestrator context call is a candidate, so throwing on a
+    // payload-shape failure would let one truncated exploratory call discard a
+    // review that an earlier call already proved.
+    const proved = contextResultContent();
+    const truncated = `${JSON.stringify({ status: 'found', symbol: { uid: 'u' } }).slice(0, 30)}\n…`;
+    const provedThenTruncated = runArtifactScenario({
+      rawTranscript: JSON.stringify(
+        twoCallTranscript({ firstResult: proved, secondResult: truncated }),
+      ),
+    });
+    expect(provedThenTruncated.artifact).toMatchObject({
+      status: 'success',
+      failure_code: null,
+      graph_evidence: { mode: 'context' },
+    });
+
+    // With no proving call, the same truncated payload is counted, not thrown.
+    const truncatedOnly = runArtifactScenario({
+      rawTranscript: JSON.stringify(reviewTranscript({ toolResultContent: truncated })),
+    });
+    expect(truncatedOnly.artifact.failure_code).toBe('missing_graph_evidence');
+    expect(truncatedOnly.stderr).toContain('results too malformed or truncated to parse: 1');
+
+    // Structural transcript invariants must still fail closed.
+    const structural = runArtifactScenario({ rawTranscript: '{not-json' });
+    expect(structural.artifact.failure_code).toBe('invalid_execution_transcript');
+  });
+
+  it('scopes a deletion-only PR to the merge-base set instead of an empty head set', () => {
+    const deletedPath = 'gitnexus/src/cli/deleted-command.ts';
+    const headScopedCall = runArtifactScenario({
+      basePaths: [deletedPath],
+      changedPaths: [],
+      rawTranscript: JSON.stringify(
+        reviewTranscript({
+          toolInput: { name: 'deletedCommand' },
+          toolResultContent: contextResultContent(deletedPath),
+        }),
+      ),
+    });
+    // headPaths is empty, so the call can never be satisfied: report it as out
+    // of scope rather than as a result "outside the changed paths".
+    expect(headScopedCall.artifact.failure_code).toBe('missing_graph_evidence');
+    expect(headScopedCall.stderr).toContain('orchestrator context calls in scope: 0');
+    expect(headScopedCall.stderr).toContain(
+      'orchestrator context calls out of scope (no selector or unknown repo): 1',
+    );
+    expect(headScopedCall.stderr).toContain('results outside the changed paths: 0');
+  });
+
+  it('publishes an incomplete analysis as a labelled failure, never as an accepted review', () => {
+    const incomplete = runArtifactScenario({
+      structuredOutput: JSON.stringify({ body: 'Partial review, two lanes died', complete: false }),
+    });
+    expect(incomplete.artifact).toMatchObject({
+      status: 'failure',
+      failure_code: 'incomplete_analysis',
+      graph_evidence: null,
+    });
+    expect(incomplete.artifact.body).toContain('could not complete this analysis');
+    expect(incomplete.artifact.body).toContain('Partial review, two lanes died');
+    expect(incomplete.stderr).toContain('the model reported an incomplete analysis');
+
+    const missingField = runArtifactScenario({
+      structuredOutput: JSON.stringify({ body: 'No completeness signal' }),
+    });
+    expect(missingField.artifact.failure_code).toBe('invalid_model_output');
   });
 
   it('accepts SDK text-block results with omitted is_error', () => {
@@ -1697,13 +1853,17 @@ describe('gitnexus review-agent workflow security contract', () => {
     expect(wrongPath.artifact.failure_code).toBe('missing_graph_evidence');
   });
 
-  it('fails closed on malformed or empty context result content', () => {
+  it('separates malformed context payloads from structurally invalid tool results', () => {
+    // Payload shape is the MCP's business and can fail for benign reasons
+    // (truncation at the output budget), so it demotes one call to non-evidence.
     const malformed = runArtifactScenario({
       rawTranscript: JSON.stringify(reviewTranscript({ toolResultContent: '{not-json' })),
     });
-    expect(malformed.artifact.failure_code).toBe('invalid_execution_transcript');
+    expect(malformed.artifact.failure_code).toBe('missing_graph_evidence');
-    expect(malformed.stderr).toContain('context tool result is not strict JSON');
+    expect(malformed.stderr).toContain('results too malformed or truncated to parse: 1');
 
+    // An empty tool_result is a transcript-structural violation, not a payload
+    // shape, and still fails the whole run closed.
     const empty = runArtifactScenario({
       rawTranscript: JSON.stringify(reviewTranscript({ toolResultContent: '   ' })),
     });