diff --git a/.github/workflows/witness-token-gate.yml b/.github/workflows/witness-token-gate.yml new file mode 100644 index 00000000..9302ad47 --- /dev/null +++ b/.github/workflows/witness-token-gate.yml @@ -0,0 +1,33 @@ +name: Witness token gate + +# Verifies that every `# WITNESS: ::` marker declared in a +# `taosmd/` source file resolves to a real token inside the named test file: +# the test file must exist and `` must appear in it. This catches the +# regression where a source comment cites a test as the justification for a +# constant, but a revert deletes the cited witness from the test while the +# test file itself survives and the suite stays green. +# +# Only the explicit `WITNESS:` marker is an assertion; bare mentions of test +# filenames in ordinary prose are ignored, so explanatory prose about a +# removal is not flagged. +# +# See scripts/check_witness_token.py for the implementation. + +on: + pull_request: + types: [opened, synchronize, reopened] + +jobs: + witness-token-gate: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v7 + - uses: astral-sh/setup-uv@v7 + - name: Install Python + run: uv python install 3.12 + - name: Install deps + run: uv sync --python 3.12 + - name: Check that declared witness tokens resolve + run: python scripts/check_witness_token.py diff --git a/changelog.d/tsk-s2keyh-witness-gate.md b/changelog.d/tsk-s2keyh-witness-gate.md new file mode 100644 index 00000000..e7d84739 --- /dev/null +++ b/changelog.d/tsk-s2keyh-witness-gate.md @@ -0,0 +1,8 @@ +### Added +- A `witness-token-gate` CI check (`scripts/check_witness_token.py`) that + verifies any `# WITNESS: ::` marker in a `taosmd/` source file + resolves to a real `` inside the named test file. A source comment that + cites a test as the justification for a constant can no longer stay green after + the witness it points at is deleted from that test, because the gate resolves the + token, not just the path. Only the explicit `WITNESS:` marker is an assertion: a + bare test-filename mention in ordinary prose is ignored. diff --git a/scripts/check_witness_token.py b/scripts/check_witness_token.py new file mode 100644 index 00000000..b493da71 --- /dev/null +++ b/scripts/check_witness_token.py @@ -0,0 +1,177 @@ +#!/usr/bin/env python3 +"""Witness token gate. + +Verifies that every explicit ``# WITNESS: ::`` marker in a source +file resolves to something real INSIDE the named test: the referenced test file +must exist and ```` must appear in it as a case-sensitive substring (grep +semantics). + +Why a token, not a path: a source comment that cites a test as the justification +for a constant is only as good as what the test still contains. A path-only check +passes green while the cited witness tokens are deleted from the test, because the +test file itself survives. This gate cannot be fooled by that -- it resolves the +TOKEN, not the path. The gate is non-vacuous in both directions: it fails when a +declared token is removed (file stays importable), and it does NOT fire on a bare +test-filename mention in ordinary prose. + +Marker contract (the only thing that triggers the check): + + # WITNESS: tests/test_foo.py::some_grepable_token + +- The ``WITNESS:`` marker is all-caps and opt-in. A bare mention of a test + filename in prose -- whether describing a removal or stating a fact -- + declares nothing and is invisible to the gate. Only lines containing the + ``WITNESS:`` marker are parsed, so prose like "the witness was removed from + test_resume_arm_time.py" is never a declaration. +- The text after the marker is split on the FIRST ``::``: the left side is the + test file path (resolved relative to the repo root), the right side is the token + to grep for. Splitting on the first ``::`` lets a token itself contain ``::`` + (for example a pytest node id), matching grep semantics. ``::`` is never part + of a relative file path, so no escaping is required. +- The token is matched as a case-sensitive substring, the same match ``grep`` + would perform. + +Scope: ``taosmd/**/*.py``. Witnesses are declared in source, not in tests -- +scanning ``tests/`` for markers would let a test's own example markers (which +name fixture trees that do not exist in the checked-out repo) produce false +violations. The sibling normalise-handle gate uses the same ``taosmd/`` scope. + +An optional ``WITNESS_GATE_ROOT`` environment variable overrides the repo root +the gate scans (defaulting to the tree containing this script). This lets the +gate target an arbitrary checkout, a staged tree, or a fixture directory -- useful +for CI-on-staged-changes and for reproducible local demos. + +Usage: + python scripts/check_witness_token.py + WITNESS_GATE_ROOT=/tmp/fixture python scripts/check_witness_token.py + (exits 0 when every declared witness resolves, 1 otherwise) +""" +from __future__ import annotations + +import os +import re +import sys +from dataclasses import dataclass +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parent.parent +REPO_ROOT = Path(os.environ.get("WITNESS_GATE_ROOT") or _REPO_ROOT) +WITNESS_RE = re.compile(r"#\s*WITNESS:\s*(.+)$") + + +@dataclass +class WitnessClaim: + source_file: str + line_number: int + raw: str + test_file: str = "" + token: str = "" + + +@dataclass +class Violation: + claim: WitnessClaim + reason: str + + +def _relative_source(file_path: Path, repo_root: Path) -> str: + try: + return str(file_path.relative_to(repo_root)) + except ValueError: + return str(file_path) + + +def _extract_claims(file_path: Path, repo_root: Path) -> list[WitnessClaim]: + try: + source = file_path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + print( + f"witness-gate: skipping {file_path}: unreadable", + file=sys.stderr, + ) + return [] + + rel = _relative_source(file_path, repo_root) + claims: list[WitnessClaim] = [] + for lineno, line in enumerate(source.splitlines(), start=1): + match = WITNESS_RE.search(line) + if not match: + continue + raw = match.group(1).strip() + claim = WitnessClaim(source_file=rel, line_number=lineno, raw=raw) + if "::" in raw: + test_file, token = raw.split("::", 1) + claim.test_file = test_file.strip() + claim.token = token.strip() + claims.append(claim) + return claims + + +def _resolve_test_file(test_file: str, repo_root: Path) -> Path | None: + candidate = repo_root / test_file + if candidate.is_file(): + return candidate + return None + + +def _read_text(path: Path) -> str: + return path.read_text(encoding="utf-8", errors="ignore") + + +def _source_files(repo_root: Path) -> list[Path]: + return sorted(set(repo_root.glob("taosmd/**/*.py"))) + + +def _check_claim( + claim: WitnessClaim, + repo_root: Path, + contents: dict[Path, str], +) -> Violation | None: + if not claim.test_file or not claim.token: + return Violation( + claim, + "malformed WITNESS marker: expected ::", + ) + target = _resolve_test_file(claim.test_file, repo_root) + if target is None: + return Violation(claim, f"test file not found: {claim.test_file}") + if target not in contents: + try: + contents[target] = _read_text(target) + except OSError: + contents[target] = "" + if claim.token not in contents[target]: + return Violation( + claim, + f"witness token not found in {claim.test_file}", + ) + return None + + +def check_witnesses(repo_root: Path = REPO_ROOT) -> list[Violation]: + violations: list[Violation] = [] + contents: dict[Path, str] = {} + for source in _source_files(repo_root): + for claim in _extract_claims(source, repo_root): + violation = _check_claim(claim, repo_root, contents) + if violation is not None: + violations.append(violation) + return violations + + +def main(argv: list[str] | None = None) -> int: + violations = check_witnesses(REPO_ROOT) + if violations: + print("WITNESS GATE FAIL:") + for v in violations: + print( + f" {v.claim.source_file}:{v.claim.line_number}: " + f"WITNESS {v.claim.raw} -> {v.reason}" + ) + return 1 + print("witness-gate: clean") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_witness_gate.py b/tests/test_witness_gate.py new file mode 100644 index 00000000..1e7b2e62 --- /dev/null +++ b/tests/test_witness_gate.py @@ -0,0 +1,278 @@ +"""Tests for scripts/check_witness_token.py. + +Proves the witness-token gate in both directions: + +* RED, motivating: a source file declares ``# WITNESS: ::`` but the + token is absent from that test -- the gate exits non-zero and names the source + file, the test and the token. +* RED, non-vacuity: the passing case (token present) passes, then ONLY the token + is deleted from the test file (the file stays present and importable) and the + gate flips to non-zero. A path-only check could not see this. +* GREEN, true positive: a declared witness whose token IS present exits 0. +* GREEN, false positive: a source file that merely mentions a test filename in + ordinary prose, with no ``WITNESS:`` marker, is not flagged. +""" +from __future__ import annotations + +import io +import os +import subprocess +import sys +from contextlib import redirect_stdout +from pathlib import Path + +import scripts.check_witness_token as wtg +from scripts.check_witness_token import ( + _extract_claims, + _resolve_test_file, + check_witnesses, + main, +) + +REPO_ROOT = Path(__file__).resolve().parent.parent +GATE_SCRIPT = REPO_ROOT / "scripts" / "check_witness_token.py" + +TEST_SRC = "taosmd/svc.py" +TEST_TEST = "tests/test_foo.py" +TOKEN = "MAX_FIRE_TO_DELETE" + +GREEN_SRC = ( + "# Constants justified by a live witness in the suite.\n" + f"# WITNESS: {TEST_TEST}::{TOKEN}\n" + "RETRY_LEAD_SECONDS = 60\n" +) +GREEN_TEST = ( + f"{TOKEN} = 1\n" + "\n" + "def test_arm_time():\n" + f" assert {TOKEN} == 1\n" +) +RED_TEST = ( + "# The witness token was removed by a revert; the citation stayed.\n" + "\n" + "def test_arm_time():\n" + " assert 1 == 1\n" +) +PROSE_SRC = ( + "# NOTE: test_foo.py\"; that witness does not exist in that file, and the\n" + "# suite stayed green through the revert.\n" +) + + +def _repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + (repo / "taosmd").mkdir(parents=True) + (repo / "tests").mkdir(parents=True) + (repo / "taosmd" / "__init__.py").write_text("") + return repo + + +def _write(repo: Path, rel: str, text: str) -> Path: + path = repo / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + return path + + +def _green_repo(tmp_path: Path) -> Path: + repo = _repo(tmp_path) + _write(repo, TEST_SRC, GREEN_SRC) + _write(repo, TEST_TEST, GREEN_TEST) + return repo + + +def _run_main(repo: Path) -> tuple[int, str]: + original = wtg.REPO_ROOT + wtg.REPO_ROOT = repo + buf = io.StringIO() + try: + with redirect_stdout(buf): + rc = main([]) + finally: + wtg.REPO_ROOT = original + return rc, buf.getvalue() + + +def _run_main_clean(repo: Path) -> None: + rc, out = _run_main(repo) + assert rc == 0, out + + +def _run_cli(repo: Path) -> subprocess.CompletedProcess: + env = os.environ.copy() + env["WITNESS_GATE_ROOT"] = str(repo) + return subprocess.run( + [sys.executable, str(GATE_SCRIPT)], + capture_output=True, + text=True, + env=env, + ) + + +# ---------------------------------------------------------------------- +# unit tests for the parser +# ---------------------------------------------------------------------- + + +class TestExtractClaims: + def test_parses_bare_marker(self, tmp_path): + f = _write(tmp_path, TEST_SRC, f"# WITNESS: {TEST_TEST}::{TOKEN}\n") + claims = _extract_claims(f, tmp_path) + assert len(claims) == 1 + c = claims[0] + assert c.source_file == TEST_SRC + assert c.line_number == 1 + assert c.test_file == TEST_TEST + assert c.token == TOKEN + + def test_inline_comment_marker(self, tmp_path): + f = _write( + tmp_path, + TEST_SRC, + f"RETRY_LEAD_SECONDS = 60 # WITNESS: {TEST_TEST}::{TOKEN}\n", + ) + claims = _extract_claims(f, tmp_path) + assert len(claims) == 1 + assert claims[0].test_file == TEST_TEST + assert claims[0].token == TOKEN + assert claims[0].line_number == 1 + + def test_no_space_after_colon(self, tmp_path): + f = _write(tmp_path, TEST_SRC, f"# WITNESS:{TEST_TEST}::{TOKEN}\n") + claims = _extract_claims(f, tmp_path) + assert len(claims) == 1 + assert claims[0].test_file == TEST_TEST + assert claims[0].token == TOKEN + + def test_token_may_contain_double_colon(self, tmp_path): + f = _write(tmp_path, TEST_SRC, f"# WITNESS: {TEST_TEST}::node::inner\n") + claims = _extract_claims(f, tmp_path) + assert len(claims) == 1 + assert claims[0].token == "node::inner" + + def test_missing_separator_is_malformed(self, tmp_path): + f = _write(tmp_path, TEST_SRC, f"# WITNESS: {TEST_TEST}\n") + claims = _extract_claims(f, tmp_path) + assert len(claims) == 1 + assert claims[0].test_file == "" + assert claims[0].token == "" + + def test_empty_payload_ignored(self, tmp_path): + f = _write(tmp_path, TEST_SRC, "# WITNESS:\n") + assert _extract_claims(f, tmp_path) == [] + + def test_lowercase_witness_in_prose_ignored(self, tmp_path): + f = _write(tmp_path, TEST_SRC, f"# witness: {TEST_TEST}::{TOKEN}\n") + assert _extract_claims(f, tmp_path) == [] + + def test_bare_filename_in_prose_ignored(self, tmp_path): + f = _write( + tmp_path, + TEST_SRC, + "# See test_foo.py for the sample above.\n", + ) + assert _extract_claims(f, tmp_path) == [] + + def test_unreadable_file_skipped(self, tmp_path): + f = _write(tmp_path, TEST_SRC, f"# WITNESS: {TEST_TEST}::{TOKEN}\n") + f.chmod(0o000) + try: + assert _extract_claims(f, tmp_path) == [] + finally: + f.chmod(0o644) + + +class TestResolveTestFile: + def test_resolves_repo_relative(self, tmp_path): + repo = _repo(tmp_path) + _write(repo, TEST_TEST, f"{TOKEN} = 1\n") + assert _resolve_test_file(TEST_TEST, repo) == repo / TEST_TEST + + def test_returns_none_when_absent(self, tmp_path): + repo = _repo(tmp_path) + assert _resolve_test_file(TEST_TEST, repo) is None + + +# ---------------------------------------------------------------------- +# integration tests: the four required scenarios +# ---------------------------------------------------------------------- + + +class TestWitnessGateIntegration: + def test_red_motivating_token_absent(self, tmp_path): + repo = _green_repo(tmp_path) + # The witness token is absent from the test file (the revert case). + _write(repo, TEST_TEST, RED_TEST) + + violations = check_witnesses(repo) + assert len(violations) == 1 + v = violations[0] + assert v.claim.source_file == TEST_SRC + assert v.claim.test_file == TEST_TEST + assert v.claim.token == TOKEN + assert "not found" in v.reason + + rc, out = _run_main(repo) + assert rc == 1 + assert "WITNESS GATE FAIL" in out + assert TEST_SRC in out + assert TEST_TEST in out + assert TOKEN in out + + def test_red_non_vacuity_flips_when_token_removed(self, tmp_path): + repo = _green_repo(tmp_path) + + # GREEN baseline: token present -> clean. + assert check_witnesses(repo) == [] + _run_main_clean(repo) + + # Delete ONLY the token from the test file. The file stays present and + # importable (it still defines test_arm_time), so a path-only check + # could never see this -- the gate must flip to non-zero. + _write(repo, TEST_TEST, RED_TEST) + violations = check_witnesses(repo) + assert len(violations) == 1 + assert violations[0].claim.token == TOKEN + + rc, _out = _run_main(repo) + assert rc == 1 + + def test_green_true_positive(self, tmp_path): + repo = _green_repo(tmp_path) + assert check_witnesses(repo) == [] + rc, out = _run_main(repo) + assert rc == 0 + assert "witness-gate: clean" in out + + def test_green_prose_mention_ignored(self, tmp_path): + repo = _green_repo(tmp_path) + _write(repo, "taosmd/prose.py", PROSE_SRC) + # No WITNESS marker anywhere; the prose mention of the test filename + # must not be treated as a declaration. + assert check_witnesses(repo) == [] + rc, out = _run_main(repo) + assert rc == 0 + assert "witness-gate: clean" in out + + +# ---------------------------------------------------------------------- +# CLI subprocess tests: prove the real script exit codes (Layer A path) +# ---------------------------------------------------------------------- + + +class TestWitnessGateCLI: + def test_cli_red_exit_nonzero(self, tmp_path): + repo = _green_repo(tmp_path) + _write(repo, TEST_TEST, RED_TEST) + result = _run_cli(repo) + assert result.returncode == 1 + assert "WITNESS GATE FAIL" in result.stdout + assert TEST_SRC in result.stdout + assert TEST_TEST in result.stdout + assert TOKEN in result.stdout + + def test_cli_green_exit_zero(self, tmp_path): + repo = _green_repo(tmp_path) + result = _run_cli(repo) + assert result.returncode == 0 + assert "witness-gate: clean" in result.stdout