feat(quad-cli): record declared vs effective reviewers and add a live smoke preflight

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

@@ -153,13 +153,21 @@ If any cap is exceeded, the gate is **skipped with a warning on stderr** and exi
 | `--max-prompt-bytes <n>` | Skip gate above this prompt byte size; default `262144` |
 | `--advisory-only` | Never fail the gate outright for below-minimum reviewers; still exits `3` on total (zero-reviewer) outage unless `--allow-zero-reviewers` is also set |
 | `--min-reviewers <n>` | Minimum valid reviewers required to trust consensus; default `2`. Below this, the report is marked `status: "advisory-degraded"` |
+| `--artifact <path>` | Write the final validated report object to the given JSON file as well as stdout; omitted by default |
 | `--allow-zero-reviewers` | Allow a run with **zero** valid reviewers to exit `0` (marked `status: "no-reviewers"`) instead of exit `3`. Without this flag, zero reviewers always exits `3`, even under `--advisory-only` |
 | `--timeout <ms>` | Override every per-runner timeout. Defaults are Claude `120000`, Codex `240000`, Cursor Agent `240000`, and Antigravity `360000`; `QUAD_CLI_TIMEOUT_<TOOL>_MS` or `QUAD_CLI_TIMEOUT_MS` can override them without code changes |
 | `--gate-timeout <ms>` | Whole parallel gate deadline including one transient retry; default `725000`, also configurable with `QUAD_CLI_GATE_TIMEOUT_MS` |
 | `--diff-file <path>` | Test-only convenience flag to read a unified diff from a file instead of git |
 
 Mock/test execution is controlled solely by the `QUAD_CLI_MOCK_DIR` environment variable (see Usage Examples) — there is no separate test-mode flag to keep in sync with it.
 
+> `--min-reviewers` defaults to `2` and stays there. Four reviewers running successfully does not
+> raise the bar, because reviewer *availability* fails for reasons unrelated to review quality —
+> expired OAuth, transient network, a launcher that is not on this machine. Measurement supports
+> rejecting `4` as a gate threshold; it does not establish that any specific higher number is better.
+> What the contract requires instead is that a run records **which reviewers were declared, which
+> were effective, and why the rest dropped out**, so a degraded run is never mistaken for a healthy one.
+
 Timeout cleanup targets the runner's process tree (`taskkill /T /F` on Windows and a detached process group on POSIX). This is best-effort: PID reuse and process-snapshot races mean the orchestrator records what it attempted but does not claim that every descendant was certainly terminated.
 Antigravity's internal `--print-timeout` stays shorter than its outer timeout (five minutes under the default six-minute outer timeout), so diagnostics can distinguish which layer stopped the run.
 
@@ -201,6 +209,8 @@ 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
+- `environment` — the orchestrator commit, OS/architecture, Node version, and per-tool CLI versions
 
 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.
 
@@ -215,6 +225,17 @@ Normalized matching uses repo-relative paths with forward slashes. Paths are **n
 
 With `--advisory-only`, a nonzero-but-below-minimum reviewer count exits `0` instead of `3` (with `status: "advisory-degraded"` in the report); a **zero**-reviewer outage still exits `3` unless `--allow-zero-reviewers` is also set.
 
+## Live Smoke Preflight
+
+The smoke scripts run a fixed small diff through the orchestrator. They are local preflight commands and are intentionally not part of `npm test` or CI.
+
+| Command | Code | Meaning |
+|---------|------|---------|
+| `npm run smoke:quad` | `0` | All four declared reviewers returned schema-valid output |
+| `npm run smoke:quad` | `1` | At least one declared reviewer was unavailable, unauthenticated, or non-compliant |
+| `npm run smoke:quad:available` | `0` | All four declared reviewers were available and returned schema-valid output |
+| `npm run smoke:quad:available` | `4` | At least one declared reviewer was missing or invalid; the command prints `status="degraded"` and names each dropped tool |
+
 ## Usage Examples
 
 ### Base/Head Range

package.json

@@ -30,7 +30,9 @@
     "build:plugins": "node scripts/build-plugin-bundles.mjs",
     "check:plugins": "node scripts/build-plugin-bundles.mjs --check",
     "lint:skills": "node scripts/vally-gate.mjs",
-    "lint:plugins": "node scripts/vally-gate.mjs --root plugins/copilot-native-workflows/skills --strict-all"
+    "lint:plugins": "node scripts/vally-gate.mjs --root plugins/copilot-native-workflows/skills --strict-all",
+    "smoke:quad": "node scripts/quad-cli-smoke.mjs",
+    "smoke:quad:available": "node scripts/quad-cli-smoke.mjs --available"
   },
   "devDependencies": {
     "@microsoft/vally": "0.13.0",

schemas/quad-cli-merged-report.json

@@ -1,7 +1,7 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
   "title": "Quad CLI Merged Consensus Report",
-  "description": "Shape of the FINAL orchestrator output (after merging raw per-CLI reports). This is intentionally a separate schema from quad-cli-report.json, which models a single raw reviewer's input and forbids the extra fields (blocking/contributors/families/effective_votes/status/reviewers_effective) that the merge step adds. Validating orchestrator OUTPUT against the raw input schema will always fail once findings.length > 0 because of quad-cli-report.json's additionalProperties:false.",
+  "description": "Shape of the FINAL orchestrator output (after merging raw per-CLI reports). This is intentionally a separate schema from quad-cli-report.json, which models a single raw reviewer's input and forbids the extra fields (blocking/contributors/families/effective_votes/status/reviewers_effective/reviewers/environment) that the merge step adds. Validating orchestrator OUTPUT against the raw input schema will always fail once findings.length > 0 because of quad-cli-report.json's additionalProperties:false.",
   "type": "object",
   "additionalProperties": false,
   "required": ["schema_version", "generated_by", "findings"],
@@ -19,6 +19,64 @@
       "type": "integer",
       "minimum": 0
     },
+    "reviewers": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": ["declared", "effective", "dropped"],
+      "properties": {
+        "declared": {
+          "type": "array",
+          "items": { "type": "string" }
+        },
+        "effective": {
+          "type": "array",
+          "items": { "type": "string" }
+        },
+        "dropped": {
+          "type": "array",
+          "items": {
+            "type": "object",
+            "additionalProperties": false,
+            "required": [
+              "tool",
+              "stage",
+              "primary_cause",
+              "observed_failure",
+              "launcher_kind",
+              "resolved_path",
+              "exit_code",
+              "ms"
+            ],
+            "properties": {
+              "tool": { "type": "string" },
+              "stage": { "enum": ["resolve", "spawn", "transport", "response", "schema"] },
+              "primary_cause": { "type": "string" },
+              "observed_failure": { "type": "string" },
+              "launcher_kind": { "type": "string" },
+              "resolved_path": { "type": "string" },
+              "exit_code": { "type": ["integer", "null"] },
+              "ms": { "type": "integer" }
+            }
+          }
+        }
+      }
+    },
+    "environment": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": ["orchestrator_commit", "os", "node", "cli_versions"],
+      "properties": {
+        "orchestrator_commit": { "type": "string" },
+        "os": { "type": "string" },
+        "node": { "type": "string" },
+        "cli_versions": {
+          "type": "object",
+          "additionalProperties": {
+            "type": ["string", "null"]
+          }
+        }
+      }
+    },
     "findings": {
       "type": "array",
       "items": {

scripts/quad-cli-orchestrate.mjs

@@ -1,13 +1,18 @@
-import { existsSync, readFileSync } from "node:fs";
-import { dirname, join, resolve } from "node:path";
+import { existsSync, readFileSync, writeFileSync } from "node:fs";
+import { homedir } from "node:os";
+import { basename, dirname, isAbsolute, join, resolve } from "node:path";
 import { fileURLToPath } from "node:url";
+import { spawnSync } from "node:child_process";
 import {
+  buildSpawnSpec,
   buildInvocation,
   formatRunnerDiagnostic,
   parseJsonReport,
+  resolveLauncher,
   resolveGateTimeout,
   resolveToolTimeout,
   runRunnerWithRetry,
+  sanitizeReason,
 } from "./quad-cli-transport.mjs";
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
@@ -23,6 +28,8 @@ const ALLOWED_MERGED_TOP_LEVEL_KEYS = new Set([
   "findings",
   "status",
   "reviewers_effective",
+  "reviewers",
+  "environment",
 ]);
 const ALLOWED_MERGED_FINDING_KEYS = new Set([
   "path",
@@ -60,8 +67,17 @@ export async function main() {
     }
 
     if (!diffResult.body.trim()) {
-      const emptyReport = createReport([]);
-      process.stdout.write(`${JSON.stringify(emptyReport, null, 2)}\n`);
+      const emptyReport = createReport([], {
+        reviewers: options.artifact ? { declared: [], effective: [], dropped: [] } : undefined,
+        environment: options.artifact ? buildEnvironment(options.mockDir) : undefined,
+      });
+      if (!validateMergedReport(emptyReport).ok) {
+        console.error(
+          "quad-cli-consensus-gate: internal error — merged report failed schemas/quad-cli-merged-report.json validation."
+        );
+        process.exit(2);
+      }
+      emitReport(emptyReport, options.artifact);
       console.error("quad-cli-consensus-gate: no eligible textual diff content to review.");
       process.exit(0);
     }
@@ -178,6 +194,8 @@ export async function main() {
     const finalReport = createReport(mergedFindings, {
       status: status !== "ok" ? status : undefined,
       reviewersEffective: validReviewerCount,
+      reviewers: options.artifact ? buildReviewerRoster(parsedResults, validReports) : undefined,
+      environment: options.artifact ? buildEnvironment(options.mockDir) : undefined,
     });
 
     // Defensive self-check: the merged/final report has a distinct shape from the raw
@@ -194,7 +212,7 @@ export async function main() {
       process.exit(2);
     }
 
-    process.stdout.write(`${JSON.stringify(finalReport, null, 2)}\n`);
+    emitReport(finalReport, options.artifact);
     console.error(
       `quad-cli-consensus-gate: ${blockingCount} blocking, ${advisoryCount} advisory, ${validReviewerCount} valid reviewers, ${erroredReviewers} errored reviewers.`
     );
@@ -239,6 +257,7 @@ function parseArgs(argv) {
     diffFile: null,
     minReviewers: 2,
     allowZeroReviewers: false,
+    artifact: null,
     mockDir: process.env.QUAD_CLI_MOCK_DIR ? resolve(process.env.QUAD_CLI_MOCK_DIR) : null,
   };
 
@@ -290,6 +309,9 @@ function parseArgs(argv) {
       case "--allow-zero-reviewers":
         options.allowZeroReviewers = true;
         break;
+      case "--artifact":
+        options.artifact = resolve(requireValue(argv, ++index, "--artifact"));
+        break;
       default:
         throw new Error(`Unknown argument: ${arg}`);
     }
@@ -727,6 +749,14 @@ function validateMergedReport(report) {
     return { ok: false };
   }
 
+  if (Object.hasOwn(report, "reviewers") && !validateReviewerRoster(report.reviewers)) {
+    return { ok: false };
+  }
+
+  if (Object.hasOwn(report, "environment") && !validateEnvironment(report.environment)) {
+    return { ok: false };
+  }
+
   if (!Array.isArray(report.findings)) {
     return { ok: false };
   }
@@ -937,9 +967,123 @@ function createReport(findings, meta = {}) {
     report.reviewers_effective = meta.reviewersEffective;
   }
 
+  if (meta.reviewers) {
+    report.reviewers = meta.reviewers;
+  }
+
+  if (meta.environment) {
+    report.environment = meta.environment;
+  }
+
   return report;
 }
 
+function emitReport(report, artifactPath) {
+  const serializedReport = `${JSON.stringify(report, null, 2)}\n`;
+  if (artifactPath) writeFileSync(artifactPath, serializedReport, "utf8");
+  process.stdout.write(serializedReport);
+}
+
+function deriveFailureStage(result) {
+  if (result.class === "schema-invalid") return "schema";
+  if (result.class === "invalid-response") return "response";
+  if (result.launcherKind === "not-found") return "resolve";
+  if (result.class === "timeout" || result.timedOut) return "transport";
+  if (Number.isInteger(result.exitCode)) return "response";
+  return "spawn";
+}
+
+function redactResolvedPath(value, home = homedir()) {
+  if (!value) return "-";
+  const rawPath = String(value);
+  const normalizedPath = rawPath.replace(/\\/g, "/");
+  const normalizedHome = String(home).replace(/\\/g, "/").replace(/\/$/, "");
+  const foldedPath = normalizedPath.toLocaleLowerCase("en-US");
+  const foldedHome = normalizedHome.toLocaleLowerCase("en-US");
+  if (foldedPath === foldedHome || foldedPath.startsWith(`${foldedHome}/`)) {
+    return `~${normalizedPath.slice(normalizedHome.length)}`;
+  }
+  return isAbsolute(rawPath) ? basename(rawPath) : basename(normalizedPath);
+}
+
+function buildReviewerRoster(parsedResults, validReports) {
+  const effective = validReports.map(({ tool }) => tool);
+  const effectiveSet = new Set(effective);
+  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 || "-") || "-",
+      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,
+    })),
+  };
+}
+
+function readOrchestratorCommit() {
+  const result = spawnSync("git", ["rev-parse", "HEAD"], { cwd: ROOT, encoding: "utf8", windowsHide: true });
+  return result.status === 0 && result.stdout?.trim() ? result.stdout.trim() : "unknown";
+}
+
+function probeCliVersion(tool) {
+  const launcher = resolveLauncher(tool);
+  if (launcher.kind === "not-found") return null;
+  const spec = buildSpawnSpec(launcher, ["--version"]);
+  const result = spawnSync(spec.command, spec.args, {
+    cwd: ROOT,
+    encoding: "utf8",
+    timeout: 10_000,
+    windowsHide: true,
+    windowsVerbatimArguments: spec.windowsVerbatimArguments,
+  });
+  if (result.status !== 0) return null;
+  return sanitizeReason(result.stdout || result.stderr) || null;
+}
+
+function buildEnvironment(mockDir) {
+  const cliVersions = Object.fromEntries(
+    TOOL_ORDER.map((tool) => [tool, mockDir ? (existsSync(join(mockDir, `${tool}.json`)) ? "mock" : null) : probeCliVersion(tool)])
+  );
+  return {
+    orchestrator_commit: readOrchestratorCommit(),
+    os: `${process.platform}/${process.arch}`,
+    node: process.version,
+    cli_versions: cliVersions,
+  };
+}
+
+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 (!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;
+  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) =>
+    entry && typeof entry === "object" && !Array.isArray(entry) &&
+    Object.keys(entry).length === droppedKeys.length && Object.keys(entry).every((key) => droppedKeys.includes(key)) &&
+    typeof entry.tool === "string" && allowedStages.includes(entry.stage) &&
+    typeof entry.primary_cause === "string" && typeof entry.observed_failure === "string" &&
+    typeof entry.launcher_kind === "string" && typeof entry.resolved_path === "string" &&
+    (entry.exit_code === null || Number.isInteger(entry.exit_code)) && Number.isInteger(entry.ms)
+  );
+}
+
+function validateEnvironment(environment) {
+  if (!environment || typeof environment !== "object" || Array.isArray(environment)) return false;
+  if (!Object.keys(environment).every((key) => ["orchestrator_commit", "os", "node", "cli_versions"].includes(key))) return false;
+  return typeof environment.orchestrator_commit === "string" && typeof environment.os === "string" &&
+    typeof environment.node === "string" && environment.cli_versions && typeof environment.cli_versions === "object" &&
+    !Array.isArray(environment.cli_versions) &&
+    Object.values(environment.cli_versions).every((version) => version === null || typeof version === "string");
+}
+
 export {
   parseArgs,
   loadRuleAliases,
@@ -958,6 +1102,9 @@ export {
   normalizePath,
   normalizeRuleId,
   createReport,
+  deriveFailureStage,
+  redactResolvedPath,
+  buildReviewerRoster,
 };
 
 const isDirectRun = process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url);