From c4e55c00dd89bc2bb488902ebfb8263e937eb528 Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Fri, 11 Sep 2026 21:52:28 -0700 Subject: [PATCH] pr-schema-gate: an exit-0 validator that printed UNCHECKED is not a clean run A PR description was written to GitHub after the validator printed "UNCHECKED: PR body rules not checked (drafter-core not installed)" and exited 0. check_body_file returned "clean" on exit 0 without reading the output lines it had already collected. Also adds the always-loaded rule this enforces: a command can succeed and still answer from a degraded source. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DtqAxtTmdoRE7CD5BXdGbp Change-Id: I02517d067693d20cac52b5f67ddde1e7bbe6b563 --- engine/CLAUDE.core.md | 1 + engine/hooks/pr-schema-gate/detect.py | 6 ++++++ .../pr-schema-gate/tests/test_advisory.py | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/engine/CLAUDE.core.md b/engine/CLAUDE.core.md index 2e4c4253..2c2407ed 100644 --- a/engine/CLAUDE.core.md +++ b/engine/CLAUDE.core.md @@ -23,6 +23,7 @@ These override brevity. If proof makes a message longer, the message gets longer - Banned phrases about code I have not executed: "this should work", "this fixes it", "that's the bug", "now it works", "verified", "confirmed". - A repro script is proof **only** if I show it FAILING before the change and PASSING after, with both outputs pasted. A script that only passes proves nothing. - Absence of output is not proof of success. A command that printed nothing needs its exit code shown. +- A command can succeed and still answer from the wrong source. When a tool can degrade to a cache, a replica, a read-only copy, or an offline mode, exit 0 and printed data are not evidence the data is live — the degradation notice goes to stderr, not into the payload. Before reporting live state from such a tool, show the exit code and stderr of the same invocation, or name the source the answer came from. A reading also expires: after a restart, kill, or redeploy of the thing being read, every value taken before that event is stale and must be re-read before it is quoted. - If a test was skipped, timed out, or I ran a subset, say exactly which and why — never let a partial run stand in for a full one. - If the user asks "did you verify X?", answer yes or no first, then show the evidence or admit there is none. Do not re-argue the original claim. - When I catch myself about to assert something I did not observe, stop and run the check instead of writing the sentence. diff --git a/engine/hooks/pr-schema-gate/detect.py b/engine/hooks/pr-schema-gate/detect.py index 1628ee27..c9425f4e 100644 --- a/engine/hooks/pr-schema-gate/detect.py +++ b/engine/hooks/pr-schema-gate/detect.py @@ -38,6 +38,7 @@ import hashlib import json import os +import re import subprocess import sys import tempfile @@ -49,6 +50,8 @@ VALIDATOR_RELATIVE_PATH = os.path.join("scripts", "validate-pr-body.mjs") VALIDATOR_TIMEOUT_SECONDS = 3.0 VALIDATOR_OUTPUT_MAX_LINES = 20 +VACUOUS_PASS_RE = re.compile( + r"\bUNCHECKED\b|\bSKIPPED?\b|\bnot installed\b|\bno rules loaded\b", re.IGNORECASE) PENDING_TTL_SECONDS = 2 * 60 * 60 STATE_DIR_ENV = "PR_SCHEMA_GATE_STATE_DIR" @@ -300,6 +303,9 @@ def check_body_file(repo_root: str, body_path: str | None, start_dir: str) -> tu lines = [line for line in (proc.stdout + "\n" + proc.stderr).splitlines() if line.strip()] lines = lines[:VALIDATOR_OUTPUT_MAX_LINES] if proc.returncode == 0: + for line in lines: + if VACUOUS_PASS_RE.search(line): + return "unchecked", f"the validator exited 0 without checking: {line.strip()}" return "clean", "" if proc.returncode == 1: return "failed", "\n".join(lines) diff --git a/engine/hooks/pr-schema-gate/tests/test_advisory.py b/engine/hooks/pr-schema-gate/tests/test_advisory.py index 883373ee..d20eb364 100644 --- a/engine/hooks/pr-schema-gate/tests/test_advisory.py +++ b/engine/hooks/pr-schema-gate/tests/test_advisory.py @@ -31,6 +31,9 @@ "process.exit(1);\n" ) VALIDATOR_CRASHES = 'console.error("Error: Cannot find module typescript");\nprocess.exit(3);\n' +VALIDATOR_EXITS_ZERO_UNCHECKED = ( + 'console.log("UNCHECKED: PR body rules not checked (drafter-core not installed)");\n' +) VALIDATOR_HANGS = "setTimeout(() => {}, 60000);\n" @@ -165,6 +168,21 @@ def test_validator_crash_is_reported_as_unchecked_not_clean(self): self.assertIn("could not check", context) self.assertIn("exit 3", context) + def test_validator_exit_zero_with_an_unchecked_line_is_not_clean(self): + with _repo(VALIDATOR_EXITS_ZERO_UNCHECKED) as repo: + body = _body_file(repo) + code, _, context = _run(GH_PR + "edit 7 --body-file " + body, repo) + self.assertEqual(code, 0) + self.assertIn("could not check", context) + self.assertIn("exited 0 without checking", context) + + def test_validator_real_pass_stays_clean(self): + with _repo(VALIDATOR_PASSES) as repo: + body = _body_file(repo) + code, _, context = _run(GH_PR + "edit 7 --body-file " + body, repo) + self.assertEqual(code, 0) + self.assertNotIn("could not check", context) + def test_validator_timeout_is_reported_as_unchecked(self): original = detect.VALIDATOR_TIMEOUT_SECONDS detect.VALIDATOR_TIMEOUT_SECONDS = 0.5