diff --git a/conformance/lib/status.py b/conformance/lib/status.py index a91f748..d3e7ffa 100755 --- a/conformance/lib/status.py +++ b/conformance/lib/status.py @@ -297,6 +297,109 @@ _EXECUTABLE_RE = re.compile( _BLOCKED_RE = re.compile( r"^\s*" + re.escape(LABEL_PREFIX) + r"/blocked-by:\s*(.+)$", re.M) +# ---------------------------------------------------------- scaffold-marker detection +# WHY THIS IS NOT A WHOLE-FILE `"TODO" in text` +# --------------------------------------------------- +# It used to be, and that mis-reported a SAFETY-CRITICAL requirement. A test whose SUBJECT +# is placeholder detection has to name the idioms it hunts for: +# `l0-static/placeholder-guard-coupling/check.sh` carries +# `SEED = ["TBD", "TODO", "FIXME", "PLACEHOLDER", ...]` — its detection vocabulary, as data. +# A whole-file substring test read that as the test declaring itself unfinished, classified +# a 32-assertion executable check as a scaffold, raised a permanent `--strict` annotation +# conflict, and (because the same file is tagged R-STATE-03) reported R-STATE-03 as +# `exec=1 scaffold=1`. A previous engineer worked around it by rewording prose. +# +# The rule below is derived from the corpus (43 tests), not invented. Every genuine +# scaffold in this tree — `l2-behavioral/onboarding-completeness`, `l3-fault-injection/ +# alerting`, `l3-fault-injection/gitops-drift`, and no others — declares itself in BOTH of +# two structural positions, and no executable test does either: +# +# 1. a `# STATUS: ... scaffold` line in the file's documentation header. This is the +# repo's DEDICATED declaration slot (tests/README.md): 3 of 43 tests say +# `STATUS: scaffold` and they are exactly the three scaffolds; 19 others declare +# `STATUS: EXECUTABLE` / `IMPLEMENTED` / `RUNNABLE`; the rest declare nothing. +# 2. a TODO/FIXME marker in the BODY that CARRIES A MESSAGE — `echo "TODO: assert PVC +# bound ..."`, `TODO(T-4.1): hermetic SMTP capture sink`. A marker that carries a +# message is a note about work that is missing. A marker that is the ENTIRE content of +# a quoted literal (`"TODO",` in a vocabulary list) carries no message: it is the +# marker as a *datum*, which is what a placeholder DETECTOR necessarily contains. +# +# Both are still evaluated over the body of `chainsaw-test.yaml` AND of any +# `render-check.sh` / `check.sh`, and either one still OUTRANKS an +# `executable: "true"` annotation (see classify_test) — the invariant that an annotation +# must never outrank the body is unchanged. Ambiguity still only ever DOWNGRADES: an +# unrecognised marker shape, a marker in body code outside a quoted literal, or a marker +# carrying any trailing text all classify as scaffold. +# +# KNOWN, ACCEPTED IMPRECISION (stated rather than hidden): a test that embeds a marker with +# text attached but as data — e.g. `re.compile(r"TODO|FIXME")`, or a single string holding +# several idioms — is still called a scaffold. That is the conservative direction, and the +# author's remedy is the same structural discipline the corpus already uses: keep each +# idiom a standalone literal in a list. + +# Vocabulary: only the idioms that DECLARE unfinished work. Deliberately excludes `TBD`, +# `PLACEHOLDER`, `XXX`, `REPLACE-ME` — tests in this tree legitimately carry those in +# executable code as data (`l0-static/egress-paths-selected-by-rules/check.sh` compares +# authored values against `"TBD"`; `l0-static/no-plaintext-secrets/check.sh` matches +# `placeholder.*`), and none of them is used here to mark a placeholder body. +# Case-insensitive: no English word contains either token, and the corpus carries no +# lower-case occurrence, so folding case only ever ADDS detection (`echo "todo: ..."`). +_MARKER_RE = re.compile(r"\b(?:TODO|FIXME)\b", re.I) + +# The dedicated declaration slot, on a comment line, case-insensitive. The declared state +# must be the FIRST token after `STATUS:` — a `STATUS:` line declares exactly one state, +# and all three real scaffolds read `# STATUS: scaffold — `. Anchoring it +# there is what keeps `# STATUS: EXECUTABLE ... this is not a scaffold` from matching; a +# loose `.*scaffold` would turn any prose that merely uses the word into a declaration. +_STATUS_SCAFFOLD_RE = re.compile( + r"^[ \t]*#[ \t]*STATUS:[ \t]*(?:scaffold|placeholder)\b", re.M | re.I) + +# A quoted literal whose entire content is one marker word: the marker as data. +_MARKER_AS_DATUM_RE = re.compile(r"(['\"])[ \t]*(?:TODO|FIXME)[ \t]*\1", re.I) + + +def _split_doc_header(text: str) -> tuple[str, str]: + """Split a test file into (documentation header, body). + + Both file kinds in this tree open with a banner comment block that is PROSE about the + test: the Given/When/Then, the incident being closed, the mandatory "what this does NOT + prove" list (tests/README.md, L0 authoring rule 1). Prose necessarily discusses + placeholder idioms when the test hunts them, so an incidental mention there is + documentation, not a placeholder body. + + The body starts at the first line that is neither blank nor a `#` comment: + *.sh -> `set -euo pipefail` (the shebang is a comment) + *.yaml -> `apiVersion: chainsaw.kyverno.io/v1alpha1` + Comments BELOW that point are body, deliberately: a scaffold marks a missing fixture + with an inline comment (`l3-fault-injection/alerting`: `file: fixtures/smtp-sink.yaml + # TODO(T-4.1): hermetic SMTP capture sink`), and that must keep counting. + """ + lines = text.splitlines(keepends=True) + for i, ln in enumerate(lines): + s = ln.strip() + if s and not s.startswith("#"): + return "".join(lines[:i]), "".join(lines[i:]) + return text, "" # no executable line at all: it is all header, body is empty + + +def scaffold_marker(text: str) -> str: + """Why `text` declares itself a placeholder body, or "" if it does not. + + Returns a short human-readable reason (used verbatim in the hygiene report) so the + defect is actionable rather than a bare path. + """ + head, body = _split_doc_header(text) + m = _STATUS_SCAFFOLD_RE.search(head) or _STATUS_SCAFFOLD_RE.search(body) + if m: + return m.group(0).strip().lstrip("#").strip()[:80] + # Blank out markers held as pure data before looking for a marker that has a message. + m2 = _MARKER_RE.search(_MARKER_AS_DATUM_RE.sub('""', body)) + if m2: + line = body[:m2.start()].count("\n") + snippet = body.splitlines()[line].strip() + return f"placeholder marker in body: {snippet[:80]}" + return "" + def classify_test(t: dict) -> dict: """Decide EXECUTABLE vs SCAFFOLD for one discovered test, conservatively. @@ -304,14 +407,18 @@ def classify_test(t: dict) -> dict: A tagged test is only worth counting if it can actually assert something. Rules, in order, chosen so ambiguity NEVER upgrades a test: - scaffold the body contains TODO markers (a placeholder), *even if* it - is annotated executable — an annotation must never outrank the - body. That mismatch is also reported as a hygiene defect. - executable annotated `conformance.home.k3s/executable: "true"` and no TODOs. - executable-untagged no TODOs and a real runnable body, but the annotation is - missing (true of the L1 render tests, which execute a - render-check.sh + golden diff, of the L0 static checks, and - of the META self-check). + scaffold the test DECLARES itself a placeholder — a `# STATUS: scaffold` + header line, or a TODO/FIXME marker carrying a message in the + body (see `scaffold_marker` for the rule and the corpus it was + derived from) — *even if* it is annotated executable. An + annotation must never outrank the body. That mismatch is also + reported as a hygiene defect. + executable annotated `conformance.home.k3s/executable: "true"` and no + scaffold declaration. + executable-untagged no scaffold declaration and a real runnable body, but the + annotation is missing (true of the L1 render tests, which + execute a render-check.sh + golden diff, of the L0 static + checks, and of the META self-check). Counted as executable, and flagged as an annotation gap. scaffold anything else. """ @@ -322,16 +429,26 @@ def classify_test(t: dict) -> dict: # downgrades the test to a scaffold — an annotation must never outrank the body, # whichever file the body lives in. extra = "" + scripts: list[tuple[str, str]] = [] for script in ("render-check.sh", "check.sh"): sp = p.parent / script if sp.is_file(): - extra += sp.read_text() + body = sp.read_text() + extra += body + scripts.append((script, body)) declared = bool(_EXECUTABLE_RE.search(text)) - has_todo = "TODO" in text or "TODO" in extra + # Each file is classified on ITS OWN structure — the yaml and the script have different + # header blocks, so they must not be concatenated before the header/body split. + reason = scaffold_marker(text) + for script, body in scripts: + if reason: + break + r = scaffold_marker(body) + reason = f"{script}: {r}" if r else "" bm = _BLOCKED_RE.search(text) has_body = bool(extra) or bool(re.search(r"^\s+- (script|assert|apply):", text, re.M)) - if has_todo: + if reason: state = "scaffold" elif declared: state = "executable" @@ -348,7 +465,8 @@ def classify_test(t: dict) -> dict: "name": t["name"], "path": rel, "layer": t["layer"], "state": state, "declared_executable": declared, - "annotation_conflict": bool(declared and has_todo), + "annotation_conflict": bool(declared and reason), + "scaffold_reason": reason, "blocked_by": bm.group(1).strip().strip('"') if bm else "", } @@ -640,6 +758,10 @@ def build(repo: Path, registry: list[dict], tests: list[dict], impl: dict[str, l # --- hygiene defects this tool owns ------------------------------------------- hygiene = { "annotation_conflicts": [t["path"] for t in classified if t["annotation_conflict"]], + # Same set, with WHY each one was called a placeholder body, so the fix is obvious + # from the report instead of requiring the reader to re-derive the rule. + "annotation_conflict_reasons": {t["path"]: t["scaffold_reason"] + for t in classified if t["annotation_conflict"]}, "executable_untagged": [t["path"] for t in classified if t["state"] == "executable-untagged"], "registry_not_in_spec": [], @@ -910,8 +1032,10 @@ def render_md(rep: dict, nodes: list[dict], advisory: dict | None) -> str: else: o.append("- registry ↔ spec parity: **NOT CHECKED** (spec tree not available; " "this is a skip, not a pass)") - o.append(f"- tests annotated executable but containing TODO bodies: " - f"{h['annotation_conflicts'] or 'none'}") + o.append("- tests annotated executable but declaring a placeholder body: " + + (", ".join(f"`{k}` ({v})" + for k, v in sorted(h["annotation_conflict_reasons"].items())) + if h["annotation_conflicts"] else "none")) o.append(f"- executable tests missing the `{LABEL_PREFIX}/executable` annotation: " f"{h['executable_untagged'] or 'none'}") @@ -930,9 +1054,12 @@ def render_md(rep: dict, nodes: list[dict], advisory: dict | None) -> str: "reconciled |") o.append("| verifying test | `conformance.home.k3s/R-*` label on a Chainsaw test " "(the authoritative tag, same source as `coverage.md`) | exact |") - o.append("| executable vs scaffold | TODO markers in the test body outrank the " - "`executable` annotation; a real body without the annotation still counts " - "and is flagged | conservative — ambiguity never upgrades |") + o.append("| executable vs scaffold | a `# STATUS: scaffold` header line, or a " + "TODO/FIXME marker CARRYING A MESSAGE in the body (a marker held as a bare " + "quoted datum — a placeholder detector's own vocabulary — is not one); " + "either outranks the `executable` annotation, and a real body without the " + "annotation still counts and is flagged | conservative — ambiguity never " + "upgrades |") o.append("| **test actually passed** | the committed `runs/ledger.jsonl` row for " "every verifying test: JUnit result **plus** that test's own pass sentinel " "token grepped from the run log | exact where a run was recorded, refused " @@ -1127,6 +1254,70 @@ def _selftest() -> int: classify_test({"name": "s", "path": str(f), "layer": "L2"})["state"] == "scaffold") + # === THE BUG THIS DETECTOR WAS REWRITTEN FOR ============================== + # A whole-file `"TODO" in text` mis-classified `l0-static/placeholder-guard- + # coupling` — a 32-assertion executable check whose SUBJECT is placeholder + # detection, so it must name the idioms it hunts — as a scaffold, and through it + # reported the SAFETY-CRITICAL R-STATE-03 as `exec=1 scaffold=1`. These + # assertions pin BOTH directions: a marker held as DATA must not downgrade, and a + # real placeholder body must still be caught. Both halves are load-bearing; the + # second is what stops this fix from degrading into "always executable". + yml = ("metadata:\n annotations:\n" + " conformance.home.k3s/executable: \"true\"\n" + "spec:\n steps:\n - script:\n" + " content: bash ./check.sh\n") + f.write_text(yml) + detector = td2 / "check.sh" + # Shaped exactly like the real check: a prose header that discusses the idioms, + # then executable code holding them as a vocabulary list. + detector.write_text( + "#!/usr/bin/env bash\n" + "# L0 static check — placeholder vocabulary\n" + "# * SEED — loud-placeholder idioms (TBD, TODO, FIXME, PLACEHOLDER)\n" + "# STATUS: EXECUTABLE. A pure offline assertion script.\n" + "set -euo pipefail\n" + "python3 - <<'EOF'\n" + "SEED = [\"TBD\", \"TODO\", \"FIXME\", \"PLACEHOLDER\"]\n" + "assert any(w in open('/etc/hostname').read().upper() for w in SEED) is False\n" + "EOF\n") + c = classify_test({"name": "s", "path": str(f), "layer": "L0"}) + check("a marker in HEADER PROSE does not downgrade an executable check", + c["state"] == "executable" and not c["annotation_conflict"]) + check("a marker held as a bare quoted DATUM does not downgrade (the R-STATE-03 bug)", + c["scaffold_reason"] == "") + + # ... and the protection is intact: a real placeholder body is still caught, in + # each of the three shapes the corpus actually uses. + detector.write_text("#!/usr/bin/env bash\nset -euo pipefail\n" + "echo \"TODO: assert the PVC is bound\"\n") + c = classify_test({"name": "s", "path": str(f), "layer": "L0"}) + check("a TODO carrying a message in check.sh IS still a scaffold", + c["state"] == "scaffold" and c["annotation_conflict"]) + check("the scaffold reason names the file and the offending line", + c["scaffold_reason"].startswith("check.sh:") + and "TODO: assert the PVC is bound" in c["scaffold_reason"]) + detector.write_text("#!/usr/bin/env bash\nset -euo pipefail\n" + "# TODO(T-4.1) write the hermetic sink fixture\ntrue\n") + check("a TODO(T-x.y) roadmap marker in an inline body comment IS still a scaffold", + classify_test({"name": "s", "path": str(f), "layer": "L0"})["state"] + == "scaffold") + detector.write_text("#!/usr/bin/env bash\nset -euo pipefail\n" + "echo \"FIXME the extraction before trusting this\"\n") + check("an unpunctuated FIXME carrying trailing prose IS still a scaffold", + classify_test({"name": "s", "path": str(f), "layer": "L0"})["state"] + == "scaffold") + # The dedicated declaration slot outranks a body that would otherwise look real — + # this is how all three genuine scaffolds in the tree declare themselves. + detector.unlink() + f.write_text("# L3 FAULT-INJECTION SCAFFOLD — x\n" + "# STATUS: scaffold — needs T-1.4 (Flux bootstrapped).\n" + yml) + c = classify_test({"name": "s", "path": str(f), "layer": "L3"}) + check("a `# STATUS: scaffold` header declaration IS a scaffold, body notwithstanding", + c["state"] == "scaffold" and c["annotation_conflict"]) + check("`# STATUS: EXECUTABLE` is not read as a scaffold declaration", + scaffold_marker("# STATUS: EXECUTABLE. `check.sh` is a pure offline " + "assertion script; covers no scaffold.\nset -e\n") == "") + # ---------------------------------------------------------------- PROOF / ledger # These are the assertions that stand between this report and a fake green. check("empty ledger parses to no rows (pre-ledger behaviour is preserved)", @@ -1341,8 +1532,11 @@ def main() -> int: f"{h['registry_not_in_spec']} {h['spec_not_in_registry']}\n") rc = 1 if h["annotation_conflicts"]: - sys.stderr.write(f"[status] STRICT: tests annotated executable with TODO " - f"bodies: {h['annotation_conflicts']}\n") + sys.stderr.write( + "[status] STRICT: tests annotated executable that declare a placeholder " + "body: " + "; ".join(f"{k} ({v})" for k, v in + sorted(h["annotation_conflict_reasons"].items())) + + "\n") rc = 1 if rep["safety_critical_without_executable_l3"]: sys.stderr.write(f"[status] STRICT: safety-critical requirement(s) whose L3 " diff --git a/conformance/tests/README.md b/conformance/tests/README.md index 71850b7..dc10f4b 100644 --- a/conformance/tests/README.md +++ b/conformance/tests/README.md @@ -115,8 +115,30 @@ and coverage extraction exact and robust. ends up silently asserting the other cluster's expectations. L1 goldens are the one exception: they are rendering fixtures and carry the literal `${BASE_DOMAIN}` token. +### How a scaffold declares itself (and how `status.py` tells one from a verifier) + Scaffolds in this tree carry `TODO(T-x.y)` markers where a fixture/assertion lands as a -later roadmap phase delivers the capability under test. `lib/status.py` treats such a -marker — in the `chainsaw-test.yaml`, in `render-check.sh`, or in `check.sh` — as -outranking any `conformance.home.k3s/executable: "true"` annotation, so a placeholder body -can never be reported as an executable test. +later roadmap phase delivers the capability under test. `lib/status.py` classifies each +test EXECUTABLE or SCAFFOLD from the test's own files — `chainsaw-test.yaml`, +`render-check.sh`, `check.sh` — and either signal below **outranks** +`conformance.home.k3s/executable: "true"`, so a placeholder body can never be reported as +an executable test. Declare a scaffold in **both** positions: + +1. **`# STATUS: scaffold — `** in the file's header comment block. The + declared state must be the first token after `STATUS:` (executable tests say + `# STATUS: EXECUTABLE` / `IMPLEMENTED` / `RUNNABLE`). +2. **A `TODO`/`FIXME` marker carrying a message, in the body** — the file from its first + non-comment line onward. `echo "TODO(T-4.2): inject a fault to fire a rule"`, + or an inline comment on a missing fixture: + `file: fixtures/smtp-sink.yaml # TODO(T-4.1): hermetic SMTP capture sink`. + +What is **not** a scaffold marker, and why the distinction is load-bearing: a marker +mentioned in the **header prose** (the Given/When/Then and the mandatory "what this does +NOT prove" list), and a marker that is the **entire content of a quoted literal** — a +placeholder *detector*'s own vocabulary, e.g. `SEED = ["TBD", "TODO", "FIXME", ...]` in +`l0-static/placeholder-guard-coupling/check.sh`. A whole-file `"TODO" in text` test used to +classify that 32-assertion check as a scaffold and, through it, report the safety-critical +R-STATE-03 as `exec=1 scaffold=1`. If your check must hold a marker as data, keep each +idiom a **standalone quoted literal in a list**; a marker with text attached (a regex +alternation, several idioms in one string) still classifies as a scaffold, because +ambiguity here only ever downgrades.