test(e2e): gate the class of defect that let a broken query pass

.squad/e2e/README.md

@@ -29,6 +29,22 @@ The E-series numbering is **chronological in the operator's head, not a coverage
 - **The status column in this file is the source of truth for what has been executed and when.** Do not let a scenario document drift from its own `Status:` header.
 - **`windows-test-baseline.md` is intentionally here** despite not being an E-scenario. It sits with the E-scenarios because operators running E-scenarios need it. Its own header calls out that it is a human-interpretation runbook, not a procedure — do not let its file location seed the "so where are E2/E3/E5+?" question.
 
+## Runbook conventions — writing a gate that cannot pass while broken
+
+E-scenario commands are executed by a human operator, not by CI, so a command that **cannot run at all** can sit in a runbook indefinitely. That is a documentation bug right up until it meets a gate whose pass condition is universally quantified — *"every `squad:{x}` on a newly created issue must be a roster Name"*. Such a condition is **vacuously true over the empty set**. A broken command that returns nothing does not merely fail to inform the verdict; it writes the *passing* answer into it.
+
+E4 Phase 3b did exactly this (#1822). Two conventions follow, and neither is optional:
+
+- **A universally-quantified gate must assert its input set is non-empty before evaluating.** Zero rows is a distinct outcome from "all rows satisfied the predicate" and must be scored separately — as `INCONCLUSIVE`, never as a verdict value. Phase 3a had this instruction and was safe; 3b did not and was not.
+- **Never let an infrastructure failure reach a `# dispositive` field.** When a command's exit code is non-zero, or its cutoff variable is unset, report `INCONCLUSIVE` and stop. Recording the "clean" value because the query came back empty inverts the meaning of the gate.
+
+Two mechanizable halves of this are enforced by `test/e2e-runbook-hygiene.test.ts`, which lints every fenced command in this folder on each build:
+
+- **`gh` is never passed `--arg`.** It is a `jq` flag; `gh` has none. `--jq` consumes it as its expression and the rest degrade into stray positionals, so `gh` exits 1 with empty stdout rather than an obvious parse error. Interpolate the value in the host shell and pass one complete `--jq` expression.
+- **Every suppressed stderr is paired with an exit-code check.** Suppression is fine for transcript readability, but unpaired it converts a hard failure into an empty result that reads as a pass.
+
+The linter reads only fenced code blocks and skips comments, so a runbook may — and should — describe these hazards in prose without tripping its own gate.
+
 ## Fixture
 
 All E-scenarios currently target `bradygaster/aspiregregator-squad-e2e`. Fixture-refresh procedure is embedded in E4's Phase 0. Do not point an E-scenario at a different fixture without first documenting why the existing fixture cannot express the scenario — a bent fixture proves less than an honest gap.

test/e2e-runbook-hygiene.test.ts

@@ -0,0 +1,197 @@
+/**
+ * E2E Runbook Hygiene
+ *
+ * E-scenario runbooks in `.squad/e2e/` ship as documented shell commands that a
+ * human operator pastes into a terminal. Nothing executes them in CI, so a
+ * command that cannot run at all can sit in a runbook indefinitely.
+ *
+ * That is only a documentation bug until it meets a gate whose pass condition is
+ * universally quantified ("every X in the result set must be Y"). Such a
+ * condition is **vacuously true over the empty set**, so a command that fails and
+ * returns nothing does not merely fail to inform the verdict — it writes the
+ * *passing* answer into it.
+ *
+ * That is exactly what happened in E4 Phase 3b (#1822): `gh` was invoked with a
+ * `--arg` flag it does not have, `--jq` swallowed `--arg` as its expression, `gh`
+ * exited 1 with empty stdout on every run, and `2>$null` hid the error. The two
+ * verdict fields marked `# dispositive` — where `true` is the FAIL evidence —
+ * were handed their `false` by an infrastructure failure.
+ *
+ * PR #1821 fixed that instance. This suite exists so the *class* cannot recur:
+ * it executes no commands and needs no fixture, network, or Actions minutes, yet
+ * it fails the build the moment a runbook regains either property that made the
+ * silent pass possible.
+ *
+ * Measured at introduction: 16 of 17 stderr-suppression sites already paired with
+ * an exit-code check, so this codifies discipline the runbooks already practice
+ * rather than imposing a new one.
+ *
+ * See `.squad/e2e/README.md` § Runbook conventions for the companion rule that
+ * cannot be mechanized: a universally-quantified gate must assert its input set
+ * is non-empty before evaluating.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { readFileSync, readdirSync } from 'node:fs';
+import { join } from 'node:path';
+
+const E2E_DIR = join(process.cwd(), '.squad', 'e2e');
+
+/** How many lines after a suppression may carry its exit-code check. */
+const EXIT_CHECK_WINDOW = 6;
+
+/**
+ * Read a text file with line endings normalized to LF.
+ *
+ * Markdown in this repo is not pinned to LF in .gitattributes, so Windows
+ * checkouts materialize CRLF. Normalize on read rather than making each regex
+ * CRLF-aware.
+ */
+function readText(filePath: string): string {
+  return readFileSync(filePath, 'utf8').replace(/\r\n/g, '\n');
+}
+
+interface CodeLine {
+  /** 1-based line number in the source markdown. */
+  line: number;
+  text: string;
+}
+
+/**
+ * Return the executable lines of every fenced code block in a markdown file.
+ *
+ * Prose is excluded because runbooks legitimately *describe* these hazards in
+ * comments — E4 documents the `--jq --arg` defect verbatim so the next reader
+ * understands why the guard exists. A linter that flagged its own postmortem
+ * would punish the documentation that prevents recurrence.
+ *
+ * Comment lines are excluded for the same reason.
+ */
+function executableLines(markdown: string): CodeLine[] {
+  const out: CodeLine[] = [];
+  let inFence = false;
+
+  markdown.split('\n').forEach((raw, idx) => {
+    if (raw.trimStart().startsWith('```')) {
+      inFence = !inFence;
+      return;
+    }
+    if (!inFence) return;
+
+    const trimmed = raw.trim();
+    if (trimmed === '' || trimmed.startsWith('#')) return;
+
+    out.push({ line: idx + 1, text: raw });
+  });
+
+  return out;
+}
+
+/**
+ * Join shell line-continuations into single logical commands.
+ *
+ * A `gh` invocation is routinely split across physical lines with a trailing
+ * PowerShell backtick or POSIX backslash. Flag misuse must be judged against the
+ * whole command, not one fragment of it. The reported line number stays that of
+ * the first physical line so the failure points at the start of the command.
+ */
+function logicalCommands(lines: CodeLine[]): CodeLine[] {
+  const out: CodeLine[] = [];
+  let pending: CodeLine | null = null;
+
+  for (const cur of lines) {
+    const continues = /[`\\]\s*$/.test(cur.text);
+    const body = cur.text.replace(/[`\\]\s*$/, ' ');
+
+    if (pending) {
+      pending = { line: pending.line, text: `${pending.text}${body}` };
+    } else {
+      pending = { line: cur.line, text: body };
+    }
+
+    if (!continues) {
+      out.push(pending);
+      pending = null;
+    }
+  }
+
+  if (pending) out.push(pending);
+  return out;
+}
+
+function runbooks(): string[] {
+  return readdirSync(E2E_DIR)
+    .filter((f) => f.endsWith('.md'))
+    .map((f) => join(E2E_DIR, f));
+}
+
+describe('E2E runbook hygiene', () => {
+  it('finds runbooks to lint', () => {
+    // Guards this suite against the failure mode it exists to prevent: if the
+    // glob silently matched nothing, every assertion below would pass over an
+    // empty set and prove nothing.
+    expect(runbooks().length).toBeGreaterThan(0);
+  });
+
+  /**
+   * `--arg` is a jq flag, not a gh flag. `gh` rejects it, and because `--jq`
+   * consumes the next token as its expression the remainder degrade into stray
+   * positionals — producing exit 1 and empty stdout rather than an obvious
+   * parse error. Interpolate the value into the jq expression in the host shell
+   * instead.
+   */
+  it('never passes --arg to gh', () => {
+    const offenders: string[] = [];
+
+    for (const file of runbooks()) {
+      for (const cmd of logicalCommands(executableLines(readText(file)))) {
+        if (/\bgh\s/.test(cmd.text) && /\s--arg\b/.test(cmd.text)) {
+          offenders.push(`${file}:${cmd.line} — ${cmd.text.trim()}`);
+        }
+      }
+    }
+
+    expect(
+      offenders,
+      `gh has no --arg flag. It exits 1 with empty stdout, which a ` +
+        `universally-quantified gate reads as a pass. Build the jq expression ` +
+        `in the shell and pass it as a single --jq argument:\n${offenders.join('\n')}`,
+    ).toEqual([]);
+  });
+
+  /**
+   * Suppressing stderr is legitimate — runbooks do it to keep transcripts
+   * readable — but only when the command's success is established some other
+   * way. Unpaired, it converts a hard failure into an empty result that reads
+   * as a pass.
+   */
+  it('pairs every stderr suppression with an exit-code check', () => {
+    const offenders: string[] = [];
+
+    for (const file of runbooks()) {
+      const lines = executableLines(readText(file));
+
+      lines.forEach((cur, idx) => {
+        if (!/2>\s*(\$null|\/dev\/null)/.test(cur.text)) return;
+
+        const window = lines
+          .slice(idx, idx + EXIT_CHECK_WINDOW)
+          .map((l) => l.text)
+          .join('\n');
+
+        const checksExit = /\$LASTEXITCODE|\$\?|\bexit\s+code\b/i.test(window);
+        if (!checksExit) {
+          offenders.push(`${file}:${cur.line} — ${cur.text.trim()}`);
+        }
+      });
+    }
+
+    expect(
+      offenders,
+      `Suppressed stderr without a nearby exit-code check. A failed command ` +
+        `then yields an empty result set, and a gate phrased "every X must be ` +
+        `Y" passes vacuously on it. Capture $LASTEXITCODE and report ` +
+        `INCONCLUSIVE — never a verdict value — when it is non-zero:\n${offenders.join('\n')}`,
+    ).toEqual([]);
+  });
+});