fix(quad-cli): transport-integrity nonce + PR #16 review follow-ups

orchestration/skills/quad-cli-consensus-gate.md

@@ -209,9 +209,50 @@ The top-level report additionally carries, when relevant:
 
 - `status: "advisory-degraded" | "no-reviewers"` — present only when reviewer count is below `--min-reviewers` or zero
 - `reviewers_effective: N` — the number of CLIs that returned schema-valid output this run
-- `reviewers` — the declared and effective tool names plus structured stage/cause diagnostics for every dropped reviewer
+
+The following two fields are **only** populated when the caller passes `--artifact <path>` — unlike
+`status`/`reviewers_effective`, which are always present when relevant, these carry an additional
+per-tool CLI re-invocation cost (see below) that is not paid on a default run:
+
+- `reviewers` — the declared and effective tool names, transport-integrity nonce status per declared
+  tool (see Transport-Integrity Nonce), plus structured stage/cause diagnostics for every dropped reviewer
 - `environment` — the orchestrator commit, OS/architecture, Node version, and per-tool CLI versions
 
+> **Cost of `--artifact`:** populating `environment.cli_versions` re-invokes every declared CLI with
+> `--version` (up to four sequential, 10-second-capped `spawnSync` calls) even for tools that already
+> ran successfully in the main review. This happens **after** the gate-timeout-bounded review phase, so
+> it can add up to ~40s of wall-clock time that is **not** covered by `--gate-timeout`. Mock runs
+> (`QUAD_CLI_MOCK_DIR`) skip the real spawn and resolve instantly.
+
+## Transport-Integrity Nonce
+
+A schema-valid JSON response only proves a reviewer returned *some* well-formed report — it does not
+prove the reviewer actually received and processed the *entire* prompt this run sent it. A truncated
+transport, a stale cached response, or a model silently working from a partial prompt can all still
+produce schema-valid JSON.
+
+Each run generates a random per-run token (`transport_nonce`, 16 hex characters) and instructs every
+reviewer to echo it back verbatim as a top-level `transport_nonce` string field. The instruction and
+token are placed **after** the `<diff>...</diff>` block in the prompt, not before it — the realistic
+failure mode is tail truncation of the (usually large) diff body, not truncation of the short
+instructions header, so the token is only reachable once the reviewer has received the whole diff. Any
+truncation before that point yields `not-echoed`, never a false `confirmed`.
+
+The orchestrator compares the echoed value against the token it issued and records one of four states
+per declared tool in `reviewers.nonce_status` (only present under `--artifact`, alongside the rest of
+`reviewers`):
+
+| Status | Meaning |
+|--------|---------|
+| `confirmed` | The reviewer echoed the exact token — positive proof it received this run's full, unmodified prompt. |
+| `mismatch` | The reviewer echoed a *different* value. This is unambiguous: the transport or the reviewer altered the prompt, so the report is dropped from consensus even though its JSON was otherwise schema-valid (`stage: "response"`, `primary_cause: "transport-integrity"`). |
+| `not-echoed` | No `transport_nonce` field was present. This is **deliberately not a failure** — an older CLI or a model that never echoes unrecognized fields is indistinguishable, from the orchestrator's side, from one that silently ignored the instruction. Recorded for observability only; does not affect `reviewers_effective` or `blocking`. |
+| `unavailable` | The reviewer never produced a parseable response at all (resolve/spawn/transport failure) — there was nothing to check the nonce against. |
+
+`QUAD_CLI_TRANSPORT_NONCE` overrides the generated token for deterministic test fixtures; it is
+test-only and must never be set for a production run (a fixed token defeats the transport-integrity
+guarantee the mechanism exists to provide).
+
 Normalized matching uses repo-relative paths with forward slashes. Paths are **not blindly lowercased** by default — `rule_id` normalization **is** lowercased (via `schemas/rule-aliases.json` alias resolution), since rule identifiers are conventionally case-insensitive across tools while file paths are not.
 
 ## Exit Codes

schemas/quad-cli-merged-report.json

@@ -58,6 +58,12 @@
               "ms": { "type": "integer" }
             }
           }
+        },
+        "nonce_status": {
+          "type": "object",
+          "additionalProperties": {
+            "enum": ["confirmed", "not-echoed", "mismatch", "unavailable"]
+          }
         }
       }
     },

scripts/quad-cli-orchestrate.mjs

@@ -1,3 +1,4 @@
+import { randomBytes } from "node:crypto";
 import { existsSync, readFileSync, writeFileSync } from "node:fs";
 import { homedir } from "node:os";
 import { basename, dirname, isAbsolute, join, resolve } from "node:path";
@@ -51,6 +52,16 @@ const SEVERITY_RANK = {
   blocker: 4,
 };
 const DEFAULT_MAX_PROMPT_BYTES = 256 * 1024;
+const NONCE_STATUSES = ["confirmed", "not-echoed", "mismatch", "unavailable"];
+
+// Test-only override so fixture mock responses (static JSON files) can deterministically
+// match this run's issued nonce; never set in production use. Mirrors the existing
+// QUAD_CLI_MOCK_DIR / QUAD_CLI_TIMEOUT_MS test-only environment variable convention.
+function generateTransportNonce() {
+  const override = process.env.QUAD_CLI_TRANSPORT_NONCE;
+  if (override) return override;
+  return randomBytes(8).toString("hex");
+}
 
 export async function main() {
   try {
@@ -68,7 +79,7 @@ export async function main() {
 
     if (!diffResult.body.trim()) {
       const emptyReport = createReport([], {
-        reviewers: options.artifact ? { declared: [], effective: [], dropped: [] } : undefined,
+        reviewers: options.artifact ? { declared: [], effective: [], dropped: [], nonce_status: {} } : undefined,
         environment: options.artifact ? buildEnvironment(options.mockDir) : undefined,
       });
       if (!validateMergedReport(emptyReport).ok) {
@@ -82,7 +93,8 @@ export async function main() {
       process.exit(0);
     }
 
-    const prompt = buildPrompt(diffResult.body);
+    const transportNonce = generateTransportNonce();
+    const prompt = buildPrompt(diffResult.body, transportNonce);
     const promptBytes = Buffer.byteLength(prompt, "utf8");
 
     if (promptBytes > options.maxPromptBytes) {
@@ -159,6 +171,20 @@ export async function main() {
       result.findingsCount = parsed.value.findings.length;
       result.ignoredTopLevelKeys = validation.ignoredTopLevelKeys;
 
+      const nonceStatus = deriveNonceStatus(parsed.value.transport_nonce, transportNonce);
+      result.nonceStatus = nonceStatus;
+      if (nonceStatus === "mismatch") {
+        // A wrong (but present) echo proves this reviewer did not faithfully receive/process
+        // this run's exact prompt — do not trust its findings for consensus, even though the
+        // JSON itself is schema-valid. A missing echo ("not-echoed") stays ambiguous (older
+        // CLI, or a model that ignored the instruction) and is NOT treated as a failure.
+        result.status = "error";
+        result.class = "transport-integrity";
+        result.reason = "transport_nonce did not match this run's issued token";
+        erroredReviewers += 1;
+        continue;
+      }
+
       validReports.push({
         tool: result.tool,
         findings: parsed.value.findings.map((finding) => ({
@@ -635,7 +661,7 @@ function countPatchLines(section) {
     .length;
 }
 
-function buildPrompt(diffBody) {
+function buildPrompt(diffBody, transportNonce = null) {
   return [
     "You are one reviewer in a four-CLI consensus gate.",
     "Review the diff for correctness, security, reliability, and maintainability issues that deserve structured findings.",
@@ -645,6 +671,7 @@ function buildPrompt(diffBody) {
       {
         schema_version: "1",
         generated_by: "quad-cli-orchestrate",
+        transport_nonce: "<echo this run's transport-integrity token here, verbatim>",
         findings: [
           {
             path: "path/to/file",
@@ -667,6 +694,18 @@ function buildPrompt(diffBody) {
     "<diff>",
     diffBody,
     "</diff>",
+    // The transport-integrity instruction is deliberately placed AFTER the diff, not before
+    // it. The realistic transport failure this guards against is tail truncation of the
+    // (usually large) diff body, not truncation of the short instructions header. Putting the
+    // token/instruction here means a reviewer can only produce "confirmed" by having received
+    // this entire prompt, diff included; any truncation before this point yields "not-echoed"
+    // (never a false "confirmed").
+    ...(transportNonce
+      ? [
+          `This run's transport-integrity token is: ${transportNonce}`,
+          'Set "transport_nonce" to that exact token, unmodified, in your JSON response — this proves you received this entire prompt, including the diff above.',
+        ]
+      : []),
   ].join("\n");
 }
 
@@ -985,6 +1024,7 @@ function emitReport(report, artifactPath) {
 }
 
 function deriveFailureStage(result) {
+  if (result.class === "transport-integrity") return "response";
   if (result.class === "schema-invalid") return "schema";
   if (result.class === "invalid-response") return "response";
   if (result.launcherKind === "not-found") return "resolve";
@@ -993,6 +1033,19 @@ function deriveFailureStage(result) {
   return "spawn";
 }
 
+// Compares a reviewer's echoed transport_nonce against the token this run issued.
+// - "confirmed": the reviewer echoed back the exact token — proves it received/processed
+//   this run's full prompt, not a truncated or stale one.
+// - "mismatch": the reviewer echoed a *different* value — proves the transport or the
+//   reviewer corrupted/altered the prompt; the report is not trustworthy for consensus.
+// - "not-echoed": the field is absent (or not a string). This is deliberately NOT treated
+//   as a failure: older CLIs and models that never learned this field will always land
+//   here, and that is indistinguishable from a model that silently ignored the instruction.
+function deriveNonceStatus(echoed, expected) {
+  if (typeof echoed !== "string" || echoed.length === 0) return "not-echoed";
+  return echoed === expected ? "confirmed" : "mismatch";
+}
+
 function redactResolvedPath(value, home = homedir()) {
   if (!value) return "-";
   const rawPath = String(value);
@@ -1006,22 +1059,54 @@ function redactResolvedPath(value, home = homedir()) {
   return isAbsolute(rawPath) ? basename(rawPath) : basename(normalizedPath);
 }
 
+const TRAILING_PUNCTUATION = /[.,;:)\]]+$/;
+
+function looksLikePathToken(token) {
+  return isAbsolute(token) || /^[A-Za-z]:[\\/]/.test(token) || token.startsWith("/") || token.startsWith("\\\\");
+}
+
+// observed_failure is free-form text (a CLI's stderr/error first line) and can contain an
+// absolute path even though it already passed through sanitizeReason (which only strips
+// control characters and truncates length, not paths). Apply the same home-dir/basename
+// redaction as redactResolvedPath, but per whitespace-delimited token, so surrounding
+// diagnostic text (e.g. "Mock response not found: <path>") is preserved.
+function redactPathTokensInText(text, home = homedir()) {
+  if (!text) return text;
+  return text
+    .split(/(\s+)/)
+    .map((token) => {
+      if (!token || /^\s+$/.test(token)) return token;
+      const trailingMatch = token.match(TRAILING_PUNCTUATION);
+      const trailing = trailingMatch ? trailingMatch[0] : "";
+      const core = trailing ? token.slice(0, -trailing.length) : token;
+      if (!looksLikePathToken(core)) return token;
+      return `${redactResolvedPath(core, home)}${trailing}`;
+    })
+    .join("");
+}
+
 function buildReviewerRoster(parsedResults, validReports) {
   const effective = validReports.map(({ tool }) => tool);
   const effectiveSet = new Set(effective);
+  // "unavailable" covers every result that never reached JSON parsing (resolve/spawn/
+  // transport failures) — there was no response body to check for an echoed nonce at all.
+  const nonceStatus = Object.fromEntries(
+    parsedResults.map((result) => [result.tool, result.nonceStatus ?? "unavailable"])
+  );
   return {
     declared: parsedResults.map(({ tool }) => tool),
     effective,
     dropped: parsedResults.filter(({ tool }) => !effectiveSet.has(tool)).map((result) => ({
       tool: result.tool,
       stage: deriveFailureStage(result),
       primary_cause: result.class ?? "internal",
-      observed_failure: sanitizeReason(result.reason || result.stderr || result.error || "-") || "-",
+      observed_failure: redactPathTokensInText(sanitizeReason(result.reason || result.stderr || result.error || "-")) || "-",
       launcher_kind: result.launcherKind ?? "-",
       resolved_path: redactResolvedPath(result.resolvedPath),
       exit_code: Number.isInteger(result.exitCode) ? result.exitCode : null,
       ms: Number.isInteger(result.ms) ? result.ms : 0,
     })),
+    nonce_status: nonceStatus,
   };
 }
 
@@ -1059,10 +1144,20 @@ function buildEnvironment(mockDir) {
 
 function validateReviewerRoster(reviewers) {
   if (!reviewers || typeof reviewers !== "object" || Array.isArray(reviewers)) return false;
-  if (!Object.keys(reviewers).every((key) => ["declared", "effective", "dropped"].includes(key))) return false;
+  if (!Object.keys(reviewers).every((key) => ["declared", "effective", "dropped", "nonce_status"].includes(key)))
+    return false;
   if (!Array.isArray(reviewers.declared) || !reviewers.declared.every((tool) => typeof tool === "string")) return false;
   if (!Array.isArray(reviewers.effective) || !reviewers.effective.every((tool) => typeof tool === "string")) return false;
   if (!Array.isArray(reviewers.dropped)) return false;
+  if (
+    Object.hasOwn(reviewers, "nonce_status") &&
+    (!reviewers.nonce_status ||
+      typeof reviewers.nonce_status !== "object" ||
+      Array.isArray(reviewers.nonce_status) ||
+      !Object.values(reviewers.nonce_status).every((value) => NONCE_STATUSES.includes(value)))
+  ) {
+    return false;
+  }
   const allowedStages = ["resolve", "spawn", "transport", "response", "schema"];
   const droppedKeys = ["tool", "stage", "primary_cause", "observed_failure", "launcher_kind", "resolved_path", "exit_code", "ms"];
   return reviewers.dropped.every((entry) =>
@@ -1103,7 +1198,10 @@ export {
   normalizeRuleId,
   createReport,
   deriveFailureStage,
+  deriveNonceStatus,
+  generateTransportNonce,
   redactResolvedPath,
+  redactPathTokensInText,
   buildReviewerRoster,
 };
 

tests/fixtures/quad-cli/mock-responses-nonce/claude.json

@@ -0,0 +1,6 @@
+{
+  "schema_version": "1",
+  "generated_by": "quad-cli-orchestrate",
+  "transport_nonce": "test-fixture-nonce",
+  "findings": []
+}

tests/fixtures/quad-cli/mock-responses-nonce/codex.json

@@ -0,0 +1,5 @@
+{
+  "schema_version": "1",
+  "generated_by": "quad-cli-orchestrate",
+  "findings": []
+}

tests/fixtures/quad-cli/mock-responses-nonce/cursor-agent.json

@@ -0,0 +1,6 @@
+{
+  "schema_version": "1",
+  "generated_by": "quad-cli-orchestrate",
+  "transport_nonce": "a-different-token-entirely",
+  "findings": []
+}

tests/quad-cli-orchestrate.test.js

@@ -16,7 +16,9 @@ import {
   validateMergedReport,
   parseArgs,
   deriveFailureStage,
+  deriveNonceStatus,
   redactResolvedPath,
+  redactPathTokensInText,
 } from "../scripts/quad-cli-orchestrate.mjs";
 
 const ROOT = resolve(import.meta.dirname, "..");
@@ -405,4 +407,77 @@ describe("reviewer roster and environment artifact", () => {
       assert.equal(validateMergedReport(report).ok, true);
     });
   });
+
+  it("redacts a home-directory path embedded inside free-form observed_failure text (does not just basename resolved_path)", () => {
+    const home = homedir();
+    const leaky = `Mock response not found: ${join(home, "private", "agy.json")}`;
+    const redacted = redactPathTokensInText(leaky, home);
+    assert.match(redacted, /^Mock response not found: ~[\\/]/);
+    assert.equal(redacted.toLowerCase().includes(home.replace(/\\/g, "/").toLowerCase()), false);
+    assert.equal(redactPathTokensInText("no path here at all", home), "no path here at all");
+  });
+});
+
+describe("quad-cli transport-integrity nonce (--diff-file fixture, mock-responses-nonce)", () => {
+  const NONCE_MOCK_DIR = resolve(FIXTURES_DIR, "mock-responses-nonce");
+  const TEST_NONCE = "test-fixture-nonce";
+
+  function withNonceFixture(callback) {
+    const tempDir = mkdtempSync(join(tmpdir(), "quad-cli-nonce-"));
+    const artifactPath = join(tempDir, "report.json");
+    try {
+      const result = runScript(["--diff-file", resolve(FIXTURES_DIR, "smoke.diff"), "--artifact", artifactPath], {
+        QUAD_CLI_MOCK_DIR: NONCE_MOCK_DIR,
+        QUAD_CLI_TRANSPORT_NONCE: TEST_NONCE,
+      });
+      callback({ result, report: JSON.parse(result.stdout) });
+    } finally {
+      rmSync(tempDir, { recursive: true, force: true });
+    }
+  }
+
+  it("deriveNonceStatus: exact match is confirmed, different value is mismatch, missing/non-string is not-echoed", () => {
+    assert.equal(deriveNonceStatus("abc123", "abc123"), "confirmed");
+    assert.equal(deriveNonceStatus("wrong", "abc123"), "mismatch");
+    assert.equal(deriveNonceStatus(undefined, "abc123"), "not-echoed");
+    assert.equal(deriveNonceStatus(42, "abc123"), "not-echoed");
+    assert.equal(deriveNonceStatus("", "abc123"), "not-echoed");
+  });
+
+  it("records confirmed/not-echoed/mismatch/unavailable per declared tool in reviewers.nonce_status", () => {
+    withNonceFixture(({ report }) => {
+      assert.deepEqual(report.reviewers.nonce_status, {
+        claude: "confirmed",
+        codex: "not-echoed",
+        "cursor-agent": "mismatch",
+        agy: "unavailable",
+      });
+    });
+  });
+
+  it("drops a reviewer whose echoed nonce mismatches, even though its JSON was schema-valid", () => {
+    withNonceFixture(({ report }) => {
+      assert.deepEqual(report.reviewers.effective, ["claude", "codex"]);
+      const dropped = report.reviewers.dropped.find(({ tool }) => tool === "cursor-agent");
+      assert.ok(dropped, "cursor-agent must be recorded as dropped");
+      assert.equal(dropped.stage, "response");
+      assert.equal(dropped.primary_cause, "transport-integrity");
+    });
+  });
+
+  it("does NOT drop a reviewer that simply never echoed the field (not-echoed stays effective)", () => {
+    withNonceFixture(({ report }) => {
+      assert.ok(report.reviewers.effective.includes("codex"));
+      assert.equal(report.reviewers.dropped.some(({ tool }) => tool === "codex"), false);
+    });
+  });
+
+  it("QUAD_CLI_TRANSPORT_NONCE override is test-only and produces a deterministic, non-empty nonce", () => {
+    withNonceFixture(({ report }) => {
+      // reviewers_effective reflects the two non-dropped reviewers (claude confirmed, codex not-echoed);
+      // this indirectly proves the same override value was used to build the prompt every declared
+      // reviewer saw, since claude's canned response only matches "test-fixture-nonce" by fixture design.
+      assert.equal(report.reviewers_effective, 2);
+    });
+  });
 });