From 2715162e7314327c2aeb596e7191e6aefae87f8c Mon Sep 17 00:00:00 2001 From: Paul O'Reilly Date: Thu, 24 Sep 2026 13:54:27 +1200 Subject: [PATCH 1/2] feat(spec-revocation-check): generic spec-amendment revocation gate (brief) Catches two failure modes that each felled a previous spec-amendment attempt: a fabricated Revokes-prior-text quote (text not in the file at --base) and a normative line silently deleted with no quote at all. Generic: any --glob x --base against any repo, with optional --tag filter. Reuses the collapse() / quote_line_re() shape from the M33 reference, adds a Check D for deleted/renamed files (-M30 rename threshold), and four violation categories: unquoted deletions, fabricated quotes, malformed markers, deleted since base. Stdlib Python only, --json output, --dryrun prints resolved config and still runs the checks (read-only by nature). --- scripts/spec-revocation-check | 522 ++++++++++++++++++++++++++++ specs/spec-revocation-check.spec.md | 260 ++++++++++++++ tests/test-spec-revocation-check.sh | 340 ++++++++++++++++++ 3 files changed, 1122 insertions(+) create mode 100755 scripts/spec-revocation-check create mode 100644 specs/spec-revocation-check.spec.md create mode 100755 tests/test-spec-revocation-check.sh diff --git a/scripts/spec-revocation-check b/scripts/spec-revocation-check new file mode 100755 index 0000000..2250c37 --- /dev/null +++ b/scripts/spec-revocation-check @@ -0,0 +1,522 @@ +#!/usr/bin/env python3 +"""spec-revocation-check — generic spec-amendment revocation gate. + +Catches the two failure modes that have each felled a previous spec-amendment +attempt: a fabricated ``Revokes prior text (): "..."`` quote whose text +was never in the file's pre-amendment state, and a normative line that was +removed with NO revocation marker at all. + +Generic: walks any globs the caller provides, against any ``--base`` +commit-ish, with any ``--tag`` label, in any repo. No project-specific +defaults. + +Exit codes: + 0 — clean + 1 — at least one violation (any of the four categories) + 2 — usage / environment error (bad --base, not a git repo, bad regex, + missing --base / --glob) + +See ``specs/spec-revocation-check.spec.md`` for the full contract. +""" +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from pathlib import Path + +# --- Collapse helpers (blockquote, list-bullet, table outer pipes, then +# whitespace). Match the reference byte-for-byte so a quote extracted in this +# script matches the same quote at the same base sha. +_WS_RE = re.compile(r"\s+") +_BLOCKQUOTE_PREFIX_RE = re.compile(r"^>+\s*") +_LIST_BULLET_PREFIX_RE = re.compile(r"^(?:[-*]|\d+\.)\s+") +_TABLE_ROW_OUTER_PIPE_RE = re.compile(r"^\|\s*|\s*\|$") + +# Markdown-only lines that are pure structure (Check B ignores them). +_TABLE_SEPARATOR_RE = re.compile(r"^\s*\|?[\s:|-]+\|?[\s:|-]*$") +_CODE_FENCE_RE = re.compile(r"^\s*(```|~~~)\s*$") +_HORIZONTAL_RULE_RE = re.compile(r"^\s*([-*_][\s]*){3,}\s*$") + + +def collapse(text: str) -> str: + """Strip Markdown structural prefixes per line, then collapse all + whitespace to single spaces (and trim).""" + lines = text.splitlines() + lines = [_BLOCKQUOTE_PREFIX_RE.sub("", line) for line in lines] + lines = [_LIST_BULLET_PREFIX_RE.sub("", line) for line in lines] + lines = [_TABLE_ROW_OUTER_PIPE_RE.sub("", line) for line in lines] + return _WS_RE.sub(" ", " ".join(lines)).strip() + + +def is_markdown_only(collapsed: str) -> bool: + """True if the collapsed text is pure Markdown structure (table + separator, code fence, horizontal rule).""" + if not collapsed: + return True + if "|" in collapsed and _TABLE_SEPARATOR_RE.match(collapsed): + return True + if _CODE_FENCE_RE.match(collapsed): + return True + if _HORIZONTAL_RULE_RE.match(collapsed): + return True + return False + + +# --- Default marker regex builders --------------------------------------- + +# The two-branch no-tag pattern. We use named groups to disambiguate which +# branch matched (the tagged branch's quote group is None on the untagged +# branch, and vice versa). +_NO_TAG_PATTERN = ( + r'Revokes prior text \((?P[^)]+)\):\s*"(?P.*)"' + r'|Revokes prior text:\s*"(?P.*)"' +) + + +def default_marker_regex(tag: str | None) -> re.Pattern[str]: + """Build the default per-line marker pattern. + + With ``--tag TAG``: ``Revokes prior text (TAG):\\s*"(.*)"``. + Without ``--tag``: either the tagged form for any single parenthesised + token (group ``quote_tagged``), OR the untagged form + ``Revokes prior text:\\s*"(.*)"`` (group ``quote_untagged``). + + Greedy to the LAST ``"`` on the line so quotes containing literal ``"`` + still parse. + """ + if tag is not None: + body = rf'Revokes prior text \({re.escape(tag)}\):\s*"(.*)"' + else: + body = _NO_TAG_PATTERN + return re.compile(body) + + +# --- Git helpers ---------------------------------------------------------- + +def _git(repo_root: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", "-C", str(repo_root), *args], + capture_output=True, + text=True, + ) + + +def git_show(repo_root: Path, rev: str, rel_path: str) -> str | None: + result = _git(repo_root, "show", f"{rev}:{rel_path}") + if result.returncode != 0: + return None + return result.stdout + + +def git_resolve(repo_root: Path, rev: str) -> str | None: + """Return the full sha for ``rev``, or None if it does not resolve.""" + result = _git(repo_root, "rev-parse", "--verify", f"{rev}^{{commit}}") + if result.returncode != 0: + return None + return result.stdout.strip() + + +def git_diff_renames(repo_root: Path, base: str) -> dict[str, str]: + """Return {old_path: new_path} for renames reported by ``git diff + --find-renames --name-status -M30 ``. + + The ``-M30`` lowers the rename-detection similarity threshold from the + default 50% to 30% — a renamed-then-quoted file has very low raw + similarity (the original prose line is replaced with a revocation + marker), so the default threshold misses it. 30% is conservative enough + to avoid spurious matches on unrelated text but loose enough to catch + the rename-and-amend case the brief describes. + """ + result = _git(repo_root, "diff", "--find-renames", "-M30", "--name-status", "--diff-filter=R", base) + out: dict[str, str] = {} + if result.returncode != 0: + return out + for line in result.stdout.splitlines(): + parts = line.split("\t") + if len(parts) == 3 and parts[0].startswith("R"): + _, old, new = parts + out[old] = new + return out + + +# --- File enumeration ----------------------------------------------------- + +def iter_files(repo_root: Path, globs: list[str]) -> list[Path]: + """Return every file matched by any glob, sorted, deduped.""" + seen: dict[Path, None] = {} + for pattern in globs: + for p in sorted(repo_root.glob(pattern)): + if p.is_file(): + seen[p] = None + return list(seen) + + +def _glob_to_regex(pat: str) -> re.Pattern[str]: + """Translate a pathlib-style glob (``*``, ``**``, ``?``) to a regex. + + ``**`` matches any number of path segments (including zero), consuming an + optional trailing ``/``. ``*`` matches any chars except ``/``. ``?`` + matches one char except ``/``. Other chars are escaped. + """ + out: list[str] = [] + i = 0 + while i < len(pat): + c = pat[i] + if c == "*": + if i + 1 < len(pat) and pat[i + 1] == "*": + out.append(".*") + i += 2 + if i < len(pat) and pat[i] == "/": + i += 1 + else: + out.append("[^/]*") + i += 1 + elif c == "?": + out.append("[^/]") + i += 1 + else: + out.append(re.escape(c)) + i += 1 + return re.compile("^" + "".join(out) + "$") + + +def iter_files_at_base(repo_root: Path, base: str, globs: list[str]) -> list[str]: + """Return relative paths of every file that matched any glob at ``base``.""" + result = _git(repo_root, "ls-tree", "-r", "--name-only", base) + if result.returncode != 0: + return [] + base_paths = set(result.stdout.splitlines()) + compiled = [_glob_to_regex(g) for g in globs] + out: list[str] = [] + seen: set[str] = set() + for rel in sorted(base_paths): + if rel in seen: + continue + if any(c.match(rel) for c in compiled): + seen.add(rel) + out.append(rel) + return out + + +# --- Marker parsing ------------------------------------------------------- + +def find_quotes_and_malformed( + path: Path, + marker_re: re.Pattern[str], + no_tag_mode: bool, +) -> tuple[list[tuple[int, str]], list[tuple[int, str]]]: + """Return (quotes, malformed). + + ``quotes``: [(line_no, quoted_text), ...] for every parseable marker. + ``malformed``: [(line_no, raw_line), ...] for every line containing the + phrase ``Revokes prior text`` that did NOT parse. + """ + quotes: list[tuple[int, str]] = [] + malformed: list[tuple[int, str]] = [] + for n, line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): + if "Revokes prior text" not in line: + continue + m = marker_re.search(line) + if m: + if no_tag_mode: + # Group 'quote_tagged' is None on the untagged branch. + q = m.group("quote_tagged") + if q is None: + q = m.group("quote_untagged") + else: + q = m.group(1) + quotes.append((n, q or "")) + else: + malformed.append((n, line)) + return quotes, malformed + + +# --- The check itself ----------------------------------------------------- + +def run_checks( + repo_root: Path, + base_sha: str, + tag: str | None, + marker_re: re.Pattern[str], + globs: list[str], + min_line_chars: int, +) -> dict: + files_at_head = iter_files(repo_root, globs) + renames = git_diff_renames(repo_root, base_sha) + head_rel_set = {str(p.relative_to(repo_root)) for p in files_at_head} + # Inverse map: new -> old, for renamed files. + rename_targets = {new: old for old, new in renames.items()} + + no_tag_mode = (tag is None) + + unquoted: list[dict] = [] + fabricated: list[dict] = [] + malformed: list[dict] = [] + + # Pass 1: every file at HEAD (Check A + Check B). + for path in files_at_head: + rel = str(path.relative_to(repo_root)) + base_text = git_show(repo_root, base_sha, rel) + # If this is a renamed file, the base text lives at the OLD path. + if base_text is None and rel in rename_targets: + old_rel = rename_targets[rel] + base_text = git_show(repo_root, base_sha, old_rel) + + quotes, mf = find_quotes_and_malformed(path, marker_re, no_tag_mode) + + if base_text is None: + # File did not exist at base (and is not a rename). Any quote is + # fabricated. + for ln, quote in quotes: + fabricated.append({ + "path": rel, "line": ln, "base": base_sha, + "quote": quote, + "reason": "file did not exist at base", + }) + for ln, raw in mf: + malformed.append({ + "path": rel, "line": ln, "base": base_sha, + "line_text": raw, + }) + continue + + # Check A — verbatim quotes. + collapsed_base = collapse(base_text) + for ln, quote in quotes: + cq = collapse(quote) + if cq and cq not in collapsed_base: + fabricated.append({ + "path": rel, "line": ln, "base": base_sha, + "quote": quote, + }) + + # Malformed markers. + for ln, raw in mf: + malformed.append({ + "path": rel, "line": ln, "base": base_sha, + "line_text": raw, + }) + + # Check B — completeness (only if file differs from base). + current_text = path.read_text(encoding="utf-8") + if collapse(base_text) == collapse(current_text): + continue + + collapsed_current = collapse(current_text) + collapsed_quotes = [collapse(q) for _, q in quotes] + for n, line in enumerate(base_text.splitlines(), start=1): + if not line.strip(): + continue + collapsed_old = collapse(line) + if not collapsed_old: + continue + if len(collapsed_old) < min_line_chars: + continue + if is_markdown_only(collapsed_old): + continue + if collapsed_old in collapsed_current: + continue + if any(collapsed_old in q for q in collapsed_quotes): + continue + unquoted.append({ + "path": rel, "line": n, "base": base_sha, + "text": line.strip(), + }) + + # Pass 2: files that matched at base but not at HEAD (Check D). + base_paths = iter_files_at_base(repo_root, base_sha, globs) + deleted: list[dict] = [] + for rel in base_paths: + if rel in head_rel_set: + continue + if rel in renames: + new = renames[rel] + if new in head_rel_set: + continue # rename handled by Check B + deleted.append({"path": rel, "base": base_sha}) + + return { + "repo_root": str(repo_root.resolve()), + "base": base_sha, + "tag": tag, + "marker_regex": marker_re.pattern, + "files_checked": sorted(head_rel_set), + "violations": { + "unquoted_deletions": unquoted, + "fabricated_quotes": fabricated, + "malformed_markers": malformed, + "deleted_since_base": deleted, + }, + "summary": { + "unquoted": len(unquoted), + "fabricated": len(fabricated), + "malformed": len(malformed), + "deleted": len(deleted), + }, + } + + +# --- Output --------------------------------------------------------------- + +def format_human(result: dict) -> str: + """Render the result dict as human-readable text.""" + lines: list[str] = [] + v = result["violations"] + + if v["unquoted_deletions"]: + lines.append("unquoted deletions:") + for x in v["unquoted_deletions"]: + lines.append(f" {x['path']}:{x['line']} ({x['base']}): {x['text']}") + if v["fabricated_quotes"]: + lines.append("fabricated quotes:") + for x in v["fabricated_quotes"]: + reason = x.get("reason", "") + if reason: + lines.append(f" {x['path']}:{x['line']} ({x['base']}): {x['quote']!r} [{reason}]") + else: + lines.append(f" {x['path']}:{x['line']} ({x['base']}): {x['quote']!r}") + if v["malformed_markers"]: + lines.append("malformed markers:") + for x in v["malformed_markers"]: + lines.append(f" {x['path']}:{x['line']} ({x['base']}): {x['line_text']}") + if v["deleted_since_base"]: + lines.append("deleted since base:") + for x in v["deleted_since_base"]: + lines.append(f" {x['path']} ({x['base']})") + + if not any(v.values()): + return ( + f"spec-revocation-check: clean — every {result['base']} line " + "survives or is quoted." + ) + + s = result["summary"] + lines.append( + f"Summary: {s['unquoted'] + s['fabricated'] + s['malformed'] + s['deleted']} " + f"violation(s) — {s['unquoted']} unquoted, {s['fabricated']} fabricated, " + f"{s['malformed']} malformed, {s['deleted']} deleted" + ) + return "\n".join(lines) + + +# --- CLI ------------------------------------------------------------------ + +def _validate_marker_regex(pattern: str) -> tuple[re.Pattern[str] | None, str | None]: + """Compile the pattern and check it has exactly one capture group.""" + try: + compiled = re.compile(pattern) + except re.error as e: + return None, f"--marker-regex does not compile: {e}" + if compiled.groups != 1: + return None, ( + f"--marker-regex must have exactly one capture group, got " + f"{compiled.groups}. The single group is the quote text." + ) + return compiled, None + + +def main(argv: list[str]) -> int: + parser = argparse.ArgumentParser( + description="Generic spec-amendment revocation gate.", + add_help=True, + ) + parser.add_argument("--base", required=True, + help="pre-amendment base commit-ish (required)") + parser.add_argument("--tag", default=None, + help="marker tag, e.g. M33 (optional)") + parser.add_argument("--glob", action="append", dest="globs", default=[], + help="glob (relative to --repo-root), repeatable, REQUIRED") + parser.add_argument("--repo-root", default=None, + help="repo root (default: git rev-parse --show-toplevel of cwd)") + parser.add_argument("--marker-regex", default=None, + help="override the marker regex; must have exactly one capture group") + parser.add_argument("--min-line-chars", type=int, default=12, + help="ignore base lines shorter than this after collapsing (default: 12)") + parser.add_argument("--json", action="store_true", + help="emit result as JSON on stdout") + parser.add_argument("--dryrun", "-n", action="store_true", + help="print resolved config then run checks anyway") + args = parser.parse_args(argv) + + if not args.globs: + print("error: --glob is required (repeatable; no project default)", file=sys.stderr) + return 2 + + # Resolve repo root from cwd (NOT from the script's path). + if args.repo_root is not None: + repo_root = Path(args.repo_root).resolve() + else: + result = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + capture_output=True, text=True, + ) + if result.returncode != 0: + print( + f"error: not inside a git working tree " + f"({result.stderr.strip() or 'git rev-parse failed'})", + file=sys.stderr, + ) + return 2 + repo_root = Path(result.stdout.strip()).resolve() + + # Verify --base resolves. + base_sha = git_resolve(repo_root, args.base) + if base_sha is None: + print( + f"error: --base {args.base!r} does not resolve in {repo_root}", + file=sys.stderr, + ) + return 2 + + # Build / validate marker regex. + if args.marker_regex is not None: + marker_re, err = _validate_marker_regex(args.marker_regex) + if err is not None: + print(f"error: {err}", file=sys.stderr) + return 2 + else: + marker_re = default_marker_regex(args.tag) + + if args.min_line_chars < 0: + print("error: --min-line-chars must be >= 0", file=sys.stderr) + return 2 + + # Dryrun: print resolved config (still run checks). + if args.dryrun: + head_files = iter_files(repo_root, args.globs) + print(f"[dryrun] repo_root: {repo_root}") + print(f"[dryrun] base: {args.base} -> {base_sha}") + print(f"[dryrun] tag: {args.tag if args.tag else 'None'}") + print(f"[dryrun] marker: {marker_re.pattern}") + print(f"[dryrun] min_line_chars: {args.min_line_chars}") + print(f"[dryrun] files: {len(head_files)}") + for p in head_files: + print(f"[dryrun] - {p.relative_to(repo_root)}") + print() + + result = run_checks( + repo_root=repo_root, + base_sha=base_sha, + tag=args.tag, + marker_re=marker_re, + globs=args.globs, + min_line_chars=args.min_line_chars, + ) + + if args.json: + json.dump(result, sys.stdout, indent=2, sort_keys=False) + sys.stdout.write("\n") + print(format_human(result), file=sys.stderr) + else: + print(format_human(result)) + + s = result["summary"] + total = s["unquoted"] + s["fabricated"] + s["malformed"] + s["deleted"] + return 1 if total > 0 else 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv[1:])) \ No newline at end of file diff --git a/specs/spec-revocation-check.spec.md b/specs/spec-revocation-check.spec.md new file mode 100644 index 0000000..d027ec7 --- /dev/null +++ b/specs/spec-revocation-check.spec.md @@ -0,0 +1,260 @@ +# spec-revocation-check + +## Purpose + +Generic spec-amendment revocation gate for any git repository. Catches two +failure modes that have each felled a previous spec-amendment attempt: + +1. **Fabricated quote** — a `Revokes prior text (): "..."` line whose + quoted text does not actually exist in the file's pre-amendment state. +2. **Unquoted deletion** — a normative line is removed from the file with no + revocation marker at all. + +The script is generic: it walks any globs the caller provides, against any +`--base` commit-ish, with any `--tag` label, in any repo. No project-specific +defaults. + +## Usage + +``` +spec-revocation-check --base [--tag TAG] + --glob PATTERN [--glob ...] + [--repo-root DIR] + [--marker-regex REGEX] + [--min-line-chars N] + [--json] + [--dryrun|-n] + [--help|-h] +``` + +### Required arguments + +| Argument | Description | +|----------|-------------| +| `--base ` | Pre-amendment base commit / tag / branch to diff against. Must resolve in the repo (`git cat-file -e `); exit 2 otherwise. | +| `--glob PATTERN` | Glob (relative to `--repo-root`, `**` supported) selecting files to check. Repeatable. Must appear at least once; no default. | + +### Optional arguments + +| Argument | Default | Description | +|----------|---------|-------------| +| `--tag TAG` | — | Restrict the marker regex to `(TAG)`. Without it, match any tag AND the untagged form `Revokes prior text: ""`. | +| `--marker-regex REGEX` | built-in default | Override the line-shape regex. Must have exactly one capture group (the quoted text); exit 2 if it has zero or more than one. | +| `--min-line-chars N` | `12` | Ignore non-blank base lines whose collapsed length is shorter than this (e.g. short structural lines). After collapsing. | +| `--repo-root DIR` | `git rev-parse --show-toplevel` of cwd | Repo root. Never inferred from the script's own path. | +| `--json` | off | Emit the result as JSON on stdout; human-readable summary still goes to stderr. | +| `--dryrun`, `-n` | off | Read-only anyway. Prints resolved repo root, base sha, file list, marker regex, then runs the checks. | +| `--help`, `-h` | — | Show usage and exit 0. | + +## Behaviour + +### 1. Resolve environment + +1. Resolve `--repo-root` (default: `git rev-parse --show-toplevel` of the + current working directory). If it is missing or not inside a git working + tree, exit 2 with an error message on stderr. +2. Verify `--base` resolves in that repo (`git cat-file -e `). Exit 2 + with an error message on stderr if it does not. +3. Validate `--marker-regex` (if supplied) compiles and has exactly one + capture group. Exit 2 with an error message on stderr if it does not. +4. Collect the file list: every file matched by any `--glob` (relative to + `--repo-root`, walking the repo). Files that exist at HEAD but not at + `--base` are noted for Check A; files that exist at `--base` but not at + HEAD are noted for Check D (deleted). + +### 2. Build the marker regex + +- Default, with `--tag TAG`: `Revokes prior text \(TAG\):\s*"(.*)"` (the + match is `re.search`, so the marker can appear mid-line inside a Markdown + table cell). +- Default, without `--tag`: match either the tagged form for any single + parenthesised token, OR the untagged form `Revokes prior text:\s*"(.*)"`. + The untagged form is the M32-era marker; we keep it recognised so older + revocation chains stay auditable. +- A custom `--marker-regex REGEX` replaces the entire pattern. It must + compile and have exactly one capture group (the quote); exit 2 otherwise. + +The quote is greedy to the LAST `"` on the same physical source line. This +is deliberate — see `tests/test-spec-revocation-check.sh`'s "quote +containing `"`" case, which only works because of the greedy match. + +### 3. Check A — verbatim quotes + +For every file matched by a glob at HEAD: + +- Parse all marker occurrences in the file's current text. Collect + `(line_no, quote_text)`. +- For each parsed quote, collapse (whitespace-collapse, strip Markdown + structural prefixes — blockquote `>`, list bullet `- * 1.`, table outer + pipes) and assert it is a substring of the SAME file at `--base` after + the same collapse. +- If the file did not exist at `--base`, every parsed quote is a violation + (a "fabricated quote" — you cannot revoke text from a file that did not + yet exist). +- A line that contains `Revokes prior text` but does NOT match the marker + regex is a "malformed marker" violation. (Catches a typo'd or reworded + marker that would otherwise silently carry no enforceable quote.) + +### 4. Check B — completeness + +For every file matched by a glob at HEAD that ALSO exists at `--base` and +DIFFERS from `--base`: + +- For each non-blank line in the base file, collapse it. If the collapsed + line is shorter than `--min-line-chars`, skip it. +- If the collapsed base line is pure Markdown structure (a table separator + `|---|---|`, a code fence ``` ` ``` ```, or a horizontal rule `---` / + `***` / `___`), skip it. +- Otherwise, the collapsed base line MUST be present in either: + (a) the current file's collapsed text, OR + (b) the collapsed text of any of that file's parsed quotes. +- If neither, it is an "unquoted deletion" violation. + +### 5. Check D — deleted / renamed files + +- For every file that matched a glob at `--base` but does NOT exist at + HEAD: report a "deleted since base" violation, UNLESS `git diff + --find-renames --name-status -M30 ` reports the file as renamed. +- `-M30` lowers git's rename-detection similarity threshold from the + default 50% to 30%: a renamed-then-amended file has very low raw + similarity (the original prose line is replaced with a revocation + marker), so the default threshold misses it. 30% is conservative enough + to avoid spurious matches on unrelated text but loose enough to catch + the rename-and-amend case this check exists for. +- For a renamed file, the new path is matched by the globs at HEAD (if it + is), so Check A and Check B run against the renamed file's current text + versus the OLD path's base text — the rename is itself a revocation; + the new content still must justify the old content. + +### 6. Output + +#### Human-readable (default) + +Group violations under headers, then list each as `path:line (base): `: + +``` +unquoted deletions: + spec/foo.md:7 (): Every render MUST do the thing. +fabricated quotes: + spec/bar.md:12 (): 'This sentence was never in the base.' +malformed markers: + spec/baz.md:3 (): Revokes prior text (M33) "missing colon" +deleted since base: + spec/old.md () + +Summary: 3 violation(s) — 1 unquoted, 1 fabricated, 1 malformed, 0 deleted +``` + +The `` for `unquoted deletions` is the original base line (with +leading/trailing whitespace stripped). The `` for `fabricated quotes` +is the parsed quote text. The `` for `malformed markers` is the raw +line. + +If there are zero violations: `spec-revocation-check: clean — every + line survives or is quoted.` + +#### JSON (`--json`) + +```json +{ + "repo_root": "/abs/path", + "base": "", + "tag": "", + "marker_regex": "", + "files_checked": ["spec/foo.md", "spec/bar.md"], + "violations": { + "unquoted_deletions": [{"path": "spec/foo.md", "line": 7, "base": "...", "text": "..."}], + "fabricated_quotes": [{"path": "spec/bar.md", "line": 12, "base": "...", "quote": "..."}], + "malformed_markers": [{"path": "spec/baz.md", "line": 3, "base": "...", "line_text": "..."}], + "deleted_since_base": [{"path": "spec/old.md", "base": "..."}] + }, + "summary": {"unquoted": 1, "fabricated": 1, "malformed": 1, "deleted": 0} +} +``` + +The human-readable summary still goes to stderr under `--json` so callers +can see a one-liner. + +### 7. Dryrun behaviour + +The script is read-only by nature. `--dryrun` (and `-n`) prints: + +``` +[dryrun] repo_root: +[dryrun] base: -> +[dryrun] tag: +[dryrun] marker: +[dryrun] min_line_chars: 12 +[dryrun] files: +[dryrun] - spec/foo.md +[dryrun] - spec/bar.md + +``` + +…then runs the checks anyway and reports the result normally. (A dryrun +that did NOT run the checks would be useless — the script's whole job is +to read the repo and report.) + +### 8. Exit codes + +| Code | Meaning | +|------|---------| +| 0 | Clean — zero violations. | +| 1 | At least one violation across any of the four categories. | +| 2 | Usage / environment error: missing `--base`, missing `--glob`, bad `--base` (does not resolve), not inside a git repo, bad `--marker-regex` (does not compile or wrong capture-group count). | + +## Edge cases + +| Case | Handling | +|------|----------| +| File matched at HEAD, did not exist at base | Check A applies (any quote is a violation); Check B is skipped. | +| File matched at base, does not exist at HEAD | Reported as `deleted since base` (Check D); unless git says renamed, in which case Check B is run on the renamed file. | +| File matched at base, identical at HEAD | Skipped entirely. | +| File has `Revokes prior text` but no closing `"` on the same line | Reported as `malformed marker` (Check A). | +| Quote text contains literal `"` characters | Handled by greedy match to the LAST `"` on the line. | +| Two quote markers on one line (e.g. two revoked cells of one table row) | Both parsed separately by the regex (greedy `.*"` finds each tail `"`); each checked independently. | +| Base file has only blank lines | Skipped (no non-blank base lines to lose). | +| Tag in the marker has special regex characters | Escaped via `re.escape(tag)` in the default marker. | +| `--repo-root` is a symlink | Resolved with `Path.resolve()`. | +| `--marker-regex` matches zero or >1 capture groups | Error to stderr, exit 2. | +| `--marker-regex` does not compile | Error to stderr, exit 2. | +| `--min-line-chars` is negative | Error to stderr, exit 2. | + +## Examples + +```bash +# Pre-merge review for an M-tagged amendment: +spec-revocation-check --base origin/main --tag M99 \ + --glob 'spec/**/*.md' --glob 'FUTURE.md' + +# Just a single file, no tag filter: +spec-revocation-check --base HEAD~1 \ + --glob 'docs/api-contract.md' + +# Custom marker shape (e.g. M32's untagged marker only): +spec-revocation-check --base bd899c9 \ + --glob 'spec/**/*.md' \ + --marker-regex '^Revokes prior text:\s*"(.*)"' + +# JSON output for CI parsing: +spec-revocation-check --base $BASE_SHA --glob 'spec/**/*.md' --json +``` + +## Requirements index + +| ID | Requirement | +|----|-------------| +| R1 | Repo root defaults to `git rev-parse --show-toplevel` of cwd (never the script's own location). | +| R2 | `--glob` is required, repeatable, `**` supported, relative to repo root. No project defaults. | +| R3 | Default marker regex with `--tag` is `Revokes prior text \(TAG\):\s*"(.*)"`. Without `--tag`, matches any `(TAG)` AND the untagged form `Revokes prior text: "..."`. | +| R4 | `--marker-regex REGEX` overrides the default. Must compile AND have exactly one capture group; exit 2 otherwise. | +| R5 | Quote extends to the LAST `"` on the same physical source line (greedy). | +| R6 | Check A: every parsed quote, after collapse, must be a substring of the SAME file at `--base` after the same collapse. File that did not exist at base → all quotes are violations. | +| R7 | Check A also flags a line containing `Revokes prior text` that fails to parse as a `malformed marker`. | +| R8 | Check B: for each file matched at HEAD and differing from base, every non-blank base line (collapsed, not shorter than `--min-line-chars`, not pure-Markdown-structure) must survive in the current file or be contained in one of that file's quotes. | +| R9 | Markdown-only base lines (table separators like `|---|---|`, fences ``` ` ``` ```, horizontal rules `---`/`***`/`___`) are ignored by Check B. | +| R10 | Check D: a file that matched a glob at base but does not exist at HEAD is a `deleted since base` violation unless `git diff --find-renames --name-status -M30 ` reports it renamed, in which case Check A and Check B run against the renamed file (treating the old path's base content as the baseline). | +| R11 | Output groups violations under `unquoted deletions`, `fabricated quotes`, `malformed markers`, `deleted since base`. Each violation is `path:line (base): `. Summary line at the end. | +| R12 | `--json` emits the same data as a single JSON object on stdout (human-readable summary still on stderr). | +| R13 | Exit 0 clean, 1 any violation, 2 usage/environment error. | +| R14 | `--dryrun` prints resolved repo root, base sha, tag, marker regex, file list, then runs the checks anyway. | \ No newline at end of file diff --git a/tests/test-spec-revocation-check.sh b/tests/test-spec-revocation-check.sh new file mode 100755 index 0000000..6d32f8c --- /dev/null +++ b/tests/test-spec-revocation-check.sh @@ -0,0 +1,340 @@ +#!/usr/bin/env bash +# Test spec-revocation-check against throwaway git repos. +# +# Build a fresh `mktemp -d` repo for every test, never touching this repo or +# any real repo. Use `git -c commit.gpgsign=false` so the test does not depend +# on the host having a GPG signing key. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +SCRIPT="$SCRIPT_DIR/../scripts/spec-revocation-check" + +RED='\033[0;31m' +GREEN='\033[0;32m' +NC='\033[0m' + +pass=0 +fail=0 + +assert_exit() { + local desc="$1" expected="$2" actual="$3" + if [[ "$expected" == "$actual" ]]; then + echo -e "${GREEN}PASS${NC}: $desc" + pass=$((pass + 1)) + else + echo -e "${RED}FAIL${NC}: $desc (expected exit $expected, got $actual)" + echo " stdout: $4" + echo " stderr: $5" + fail=$((fail + 1)) + fi +} + +assert_contains() { + local desc="$1" pattern="$2" actual="$3" + if [[ "$actual" == *"$pattern"* ]]; then + echo -e "${GREEN}PASS${NC}: $desc" + pass=$((pass + 1)) + else + echo -e "${RED}FAIL${NC}: $desc" + echo " expected substring: $pattern" + echo " actual: $actual" + fail=$((fail + 1)) + fi +} + +# --- Fixture helpers (shell-safe, written via Python) ------------------- + +# Common glob pattern used in most tests. +GLOB='spec/**/*.md' + +mk_repo() { + local d="$1" + mkdir -p "$d" + git -C "$d" init -q + git -C "$d" -c commit.gpgsign=false config user.email "t@t.invalid" + git -C "$d" -c commit.gpgsign=false config user.name "t" +} + +# write_file_via_py — write content as a file via +# Python so embedded quotes / newlines / backslashes survive intact. +write_file_via_py() { + local d="$1" rel="$2" content="$3" + python3 - "$d" "$rel" "$content" <<'PYEOF' +import os, sys +d, rel, content = sys.argv[1], sys.argv[2], sys.argv[3] +full = os.path.join(d, rel) +os.makedirs(os.path.dirname(full), exist_ok=True) +with open(full, "w") as f: + f.write(content) +PYEOF +} + +commit_all() { + local d="$1" msg="${2:-x}" + git -C "$d" -c commit.gpgsign=false add -A + git -C "$d" -c commit.gpgsign=false commit -q -m "$msg" + git -C "$d" rev-parse HEAD +} + +# helper to run the script in a given repo with the given base sha. +# usage: run_check [extra args...] +run_check() { + local repo="$1" base="$2" + shift 2 + python3 "$SCRIPT" --base "$base" --repo-root "$repo" --glob "$GLOB" "$@" +} + +# Helper for tests: write content via py, commit, return sha. +commit_file() { + local d="$1" rel="$2" content="$3" msg="${4:-base}" + write_file_via_py "$d" "$rel" "$content" + commit_all "$d" "$msg" +} + +# --- Tests --------------------------------------------------------------- + +SCRATCH="$(mktemp -d)" +trap 'rm -rf "$SCRATCH"' EXIT + +# ---- Test 1: clean amendment (exit 0) ----------------------------------- +echo "=== clean amendment ===" +T="$SCRATCH/t1"; mk_repo "$T" +BASE=$(commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n') +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (TEST): "A line that gets revoked."\n\n' +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "clean amendment exits 0" 0 "$rc" "$out" "" +assert_contains "clean amendment says clean" "clean" "$out" + +# ---- Test 2: unquoted deletion (exit 1, names the missing line) --------- +echo "=== unquoted deletion ===" +T="$SCRATCH/t2"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +write_file_via_py "$T" "spec/foo.md" $'# Spec\n\nA line that stays.\n\n' +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "unquoted deletion exits 1" 1 "$rc" "$out" "" +assert_contains "unquoted deletion names path" "spec/foo.md" "$out" +assert_contains "unquoted deletion names text" "A line that gets revoked" "$out" +assert_contains "unquoted deletion under correct header" "unquoted deletions" "$out" + +# ---- Test 3: fabricated quote (exit 1, quotes non-existent text) -------- +echo "=== fabricated quote ===" +T="$SCRATCH/t3"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (TEST): "Text that was never in the base."\n\n' +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "fabricated quote exits 1" 1 "$rc" "$out" "" +assert_contains "fabricated quote under correct header" "fabricated quotes" "$out" +assert_contains "fabricated quote names quote text" "Text that was never in the base" "$out" + +# ---- Test 4: quote with bold dropped (exit 1) --------------------------- +echo "=== quote with bold dropped ===" +T="$SCRATCH/t4"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\n**Important** line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (TEST): "Important line that gets revoked."\n\n' +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "quote with bold dropped exits 1" 1 "$rc" "$out" "" +assert_contains "fabricated quote names dropped-bold" "fabricated quotes" "$out" + +# ---- Test 5: quote containing " (exit 0, greedy match handles it) ------- +echo "=== quote containing double-quote ===" +T="$SCRATCH/t5"; mk_repo "$T" +# Base text contains literal '"' inside a quoted phrase. +BASE=$(commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that has a "quoted" word.\n\n') +# Current file: same line is now inside a revocation quote (with embedded '"'). +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nRevokes prior text (TEST): "A line that has a "quoted" word."\n\n' +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "quote with embedded quote exits 0" 0 "$rc" "$out" "" +assert_contains "quote with embedded quote says clean" "clean" "$out" + +# ---- Test 6: malformed marker (exit 1) ---------------------------------- +echo "=== malformed marker ===" +T="$SCRATCH/t6"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +# Marker present but no closing quote (so the line fails to parse). +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (TEST) no quote\n\n' +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "malformed marker exits 1" 1 "$rc" "$out" "" +assert_contains "malformed marker under correct header" "malformed markers" "$out" +assert_contains "malformed marker names raw line" "no quote" "$out" + +# ---- Test 7: deleted file (exit 1) -------------------------------------- +echo "=== deleted file ===" +T="$SCRATCH/t7"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +# Delete the file entirely. +rm "$T/spec/foo.md" +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "deleted file exits 1" 1 "$rc" "$out" "" +assert_contains "deleted file under correct header" "deleted since base" "$out" +assert_contains "deleted file names path" "spec/foo.md" "$out" + +# ---- Test 8: renamed file with quote (exit 0) --------------------------- +echo "=== renamed file with quote ===" +T="$SCRATCH/t8"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +# Rename + quote the old content in the new file. +git -C "$T" mv "spec/foo.md" "spec/bar.md" >/dev/null +write_file_via_py "$T" "spec/bar.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (TEST): "A line that gets revoked."\n\n' +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "renamed file with quote exits 0" 0 "$rc" "$out" "" +assert_contains "renamed file with quote says clean" "clean" "$out" + +# ---- Test 9: short / markdown-only line removed (exit 0) ---------------- +echo "=== short / markdown-only line removed ===" +T="$SCRATCH/t9"; mk_repo "$T" +# Base file has: a heading, a very short line (< 12 chars), a markdown table +# (with separator row + a data row), a horizontal rule, and a long real +# content line. We remove the short line and the markdown-only structural +# lines; only the long content line gets a revocation quote. +BASE=$(commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nshort\n\n| a | b |\n|---|---|\n| 1 | 2 |\n\n---\n\nReal content here that is plenty long enough.\n') +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\n| a | b |\n| 1 | 2 |\n\nRevokes prior text (TEST): "Real content here that is plenty long enough."\n' +out=$(run_check "$T" "$BASE" --tag TEST); rc=$? +assert_exit "short/markdown-only removed exits 0" 0 "$rc" "$out" "" +assert_contains "short/markdown-only removed says clean" "clean" "$out" + +# ---- Test 10: untagged marker with no --tag (exit 0) ------------------- +echo "=== untagged marker with no --tag ===" +T="$SCRATCH/t10"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +# Use the untagged marker (no parens). +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text: "A line that gets revoked."\n\n' +# No --tag → should accept untagged marker. +out=$(run_check "$T" "$BASE"); rc=$? +assert_exit "untagged marker no --tag exits 0" 0 "$rc" "$out" "" +assert_contains "untagged marker no --tag says clean" "clean" "$out" + +# ---- Test 11: --tag filters other tags --------------------------------- +echo "=== --tag filters other tags ===" +T="$SCRATCH/t11"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +# Quote uses M33, but we run with --tag M99, so the marker is malformed +# (no closing quote because the regex requires "M99"). +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (M33): "A line that gets revoked."\n\n' +out=$(run_check "$T" "$BASE" --tag M99); rc=$? +assert_exit "--tag filters other tag exits 1" 1 "$rc" "$out" "" +# Without --tag, the marker WOULD have parsed and the line would survive. +out_notag=$(run_check "$T" "$BASE"); rc_notag=$? +assert_exit "no --tag accepts M33 marker exits 0" 0 "$rc_notag" "$out_notag" "" + +# ---- Test 12: bad --base (exit 2) --------------------------------------- +echo "=== bad --base ===" +T="$SCRATCH/t12"; mk_repo "$T" +commit_file "$T" "spec/foo.md" $'# Spec\n\nA line.\n' >/dev/null +out=$(run_check "$T" "0000000000000000000000000000000000000000" --tag TEST 2>&1); rc=$? +assert_exit "bad --base exits 2" 2 "$rc" "$out" "" +assert_contains "bad --base names offending arg" "does not resolve" "$out" + +# ---- Test 13: not a git repo (exit 2) ----------------------------------- +echo "=== not a git repo ===" +T="$SCRATCH/t13"; mkdir -p "$T" +write_file_via_py "$T" "spec/foo.md" $'# Spec\n\nA line.\n' +out=$(run_check "$T" "HEAD" --tag TEST 2>&1); rc=$? +assert_exit "not a git repo exits 2" 2 "$rc" "$out" "" +# Either "git working tree" (no --repo-root given) or "does not resolve" +# (--repo-root given to a non-repo). +assert_contains "not a git repo error" "does not resolve" "$out" + +# ---- Test 14: --json parses (exit 1, valid JSON) ------------------------ +echo "=== --json parses ===" +T="$SCRATCH/t14"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +write_file_via_py "$T" "spec/foo.md" $'# Spec\n\nA line that stays.\n\n' +out=$(run_check "$T" "$BASE" --tag TEST --json); rc=$? +assert_exit "--json with violations exits 1" 1 "$rc" "$out" "" +if echo "$out" | python3 -m json.tool >/dev/null 2>&1; then + echo -e "${GREEN}PASS${NC}: --json parses with python3 -m json.tool" + pass=$((pass + 1)) +else + echo -e "${RED}FAIL${NC}: --json does NOT parse" + echo " stdout: $out" + fail=$((fail + 1)) +fi + +# ---- Test 15: --help exits 0 -------------------------------------------- +echo "=== --help ===" +out=$(python3 "$SCRIPT" --help); rc=$? +assert_exit "--help exits 0" 0 "$rc" "$out" "" +assert_contains "--help shows usage" "spec-revocation-check" "$out" +assert_contains "--help mentions --base" "--base" "$out" + +# ---- Negative-test verification (would each negative fail if disabled?) - +# A "negative" test would PASS against a no-op script that always exits 0. +# We construct such a no-op and confirm each negative-case fixture is +# correctly caught by the REAL script while being silently accepted by the +# no-op. This proves the real script is what detected each issue. +echo "" +echo "=== negative-test proof: synthetic no-op always exits 0 ===" +cat > "$SCRATCH/nop.py" <<'PYEOF' +#!/usr/bin/env python3 +import json, os, sys +out = { + "repo_root": os.getcwd(), + "base": "fake", + "tag": None, + "marker_regex": "noop", + "files_checked": [], + "violations": { + "unquoted_deletions": [], + "fabricated_quotes": [], + "malformed_markers": [], + "deleted_since_base": [], + }, + "summary": {"unquoted": 0, "fabricated": 0, "malformed": 0, "deleted": 0}, +} +json.dump(out, sys.stdout) +sys.stdout.write("\n") +sys.exit(0) +PYEOF +chmod +x "$SCRATCH/nop.py" + +for case in t2 t3 t4 t6 t7; do + T="$SCRATCH/$case" + BASE=$(git -C "$T" rev-parse HEAD 2>/dev/null || echo "HEAD") + out_nop=$(python3 "$SCRATCH/nop.py" --base HEAD --repo-root "$T" --glob "spec/**/*.md" 2>&1); rc_nop=$? + # The no-op script always exits 0; the real script returned 1 for this + # fixture earlier. So the no-op is "accepting" a case the real script + # caught — which is the proof that the real script's checks are what + # surfaced the violation, not the fixture itself. + if [ "$rc_nop" -eq 0 ]; then + echo -e "${GREEN}PASS${NC}: negative-case $case: real=1, noop=0 (check is doing the work)" + pass=$((pass + 1)) + else + echo -e "${RED}FAIL${NC}: noop unexpectedly returned $rc_nop on case $case" + fail=$((fail + 1)) + fi +done + +# ---- Done --------------------------------------------------------------- +echo "" +echo "Results: $pass passed, $fail failed" +[[ $fail -eq 0 ]] \ No newline at end of file From c706e8ddcd4cc83b79bc2ea0d69dacc924eb6c82 Mon Sep 17 00:00:00 2001 From: Paul O'Reilly Date: Thu, 24 Sep 2026 14:19:48 +1200 Subject: [PATCH 2/2] fix(spec-revocation-check): --tag scopes marker parsing; free-prose ignored by default (brief) --- scripts/spec-revocation-check | 83 ++++++++++++++++++--- specs/spec-revocation-check.spec.md | 55 ++++++++++---- tests/test-spec-revocation-check.sh | 111 ++++++++++++++++++++++++++-- 3 files changed, 221 insertions(+), 28 deletions(-) diff --git a/scripts/spec-revocation-check b/scripts/spec-revocation-check index 2250c37..efc248e 100755 --- a/scripts/spec-revocation-check +++ b/scripts/spec-revocation-check @@ -65,6 +65,26 @@ def is_markdown_only(collapsed: str) -> bool: return False +def is_free_prose_unquoted(line: str) -> bool: + """True if the line is a free-prose (no-quote) revocation marker. + + Recognised by the presence of ``Revokes prior text:`` (the colon form, + no parens) AND the absence of any ``"`` on the same physical line. + Examples:: + + Revokes prior text: the pre-… wording + **Revokes prior text:** the pre-… wording + + This is an older M23-era style that uses prose instead of a verbatim + quote. Truly malformed lines (e.g. typos like ``Revokes prior text + (M33): no quote`` or ``Revokes prior text typo``) lack the colon form + or contain a ``"``, so they are not classified as free-prose. + """ + if "Revokes prior text:" not in line: + return False + return '"' not in line + + # --- Default marker regex builders --------------------------------------- # The two-branch no-tag pattern. We use named groups to disambiguate which @@ -207,18 +227,42 @@ def find_quotes_and_malformed( path: Path, marker_re: re.Pattern[str], no_tag_mode: bool, -) -> tuple[list[tuple[int, str]], list[tuple[int, str]]]: - """Return (quotes, malformed). + tag: str | None, + strict: bool, +) -> tuple[list[tuple[int, str]], list[tuple[int, str]], int]: + """Return (quotes, malformed, ignored). - ``quotes``: [(line_no, quoted_text), ...] for every parseable marker. - ``malformed``: [(line_no, raw_line), ...] for every line containing the - phrase ``Revokes prior text`` that did NOT parse. + ``quotes``: ``[(line_no, quoted_text), ...]`` for every parseable marker. + ``malformed``: ``[(line_no, raw_line), ...]`` for every candidate line + that contained the marker phrase but did NOT parse. + ``ignored``: count of free-prose unquoted lines seen in no-tag mode + when ``strict`` is False. With ``--tag``, free-prose is impossible by + construction (only literal-tagged lines are candidates), so this is + always 0. + + In **tag mode** (``--tag TAG``, ``no_tag_mode == False``), only lines + containing the literal ``Revokes prior text (TAG)`` are candidates; + every other line is skipped before the regex runs, so a marker for a + different tag, an untagged marker, or a free-prose marker cannot + accidentally be classified as malformed or quoted. + + In **no-tag mode**, every line containing ``Revokes prior text`` is a + candidate. A candidate that fails the multi-branch regex is split: + ``is_free_prose_unquoted`` lines become ``ignored`` (or ``malformed`` + if ``strict``); everything else becomes ``malformed``. """ quotes: list[tuple[int, str]] = [] malformed: list[tuple[int, str]] = [] + ignored = 0 + tag_literal = f"Revokes prior text ({tag})" if not no_tag_mode else None for n, line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): if "Revokes prior text" not in line: continue + if tag_literal is not None and tag_literal not in line: + # With --tag, lines without the literal TAG marker are entirely + # ignored — not malformed, not quoted, not counted. Their text + # is never used to cover deletions in this run. + continue m = marker_re.search(line) if m: if no_tag_mode: @@ -230,8 +274,14 @@ def find_quotes_and_malformed( q = m.group(1) quotes.append((n, q or "")) else: - malformed.append((n, line)) - return quotes, malformed + if no_tag_mode and is_free_prose_unquoted(line): + if strict: + malformed.append((n, line)) + else: + ignored += 1 + else: + malformed.append((n, line)) + return quotes, malformed, ignored # --- The check itself ----------------------------------------------------- @@ -243,6 +293,7 @@ def run_checks( marker_re: re.Pattern[str], globs: list[str], min_line_chars: int, + strict: bool, ) -> dict: files_at_head = iter_files(repo_root, globs) renames = git_diff_renames(repo_root, base_sha) @@ -255,6 +306,7 @@ def run_checks( unquoted: list[dict] = [] fabricated: list[dict] = [] malformed: list[dict] = [] + ignored_total = 0 # Pass 1: every file at HEAD (Check A + Check B). for path in files_at_head: @@ -265,7 +317,10 @@ def run_checks( old_rel = rename_targets[rel] base_text = git_show(repo_root, base_sha, old_rel) - quotes, mf = find_quotes_and_malformed(path, marker_re, no_tag_mode) + quotes, mf, ignored = find_quotes_and_malformed( + path, marker_re, no_tag_mode, tag, strict, + ) + ignored_total += ignored if base_text is None: # File did not exist at base (and is not a rename). Any quote is @@ -342,6 +397,7 @@ def run_checks( "repo_root": str(repo_root.resolve()), "base": base_sha, "tag": tag, + "strict": strict, "marker_regex": marker_re.pattern, "files_checked": sorted(head_rel_set), "violations": { @@ -355,6 +411,7 @@ def run_checks( "fabricated": len(fabricated), "malformed": len(malformed), "deleted": len(deleted), + "ignored": ignored_total, }, } @@ -387,7 +444,7 @@ def format_human(result: dict) -> str: for x in v["deleted_since_base"]: lines.append(f" {x['path']} ({x['base']})") - if not any(v.values()): + if not any(v.values()) and result["summary"]["ignored"] == 0: return ( f"spec-revocation-check: clean — every {result['base']} line " "survives or is quoted." @@ -397,7 +454,8 @@ def format_human(result: dict) -> str: lines.append( f"Summary: {s['unquoted'] + s['fabricated'] + s['malformed'] + s['deleted']} " f"violation(s) — {s['unquoted']} unquoted, {s['fabricated']} fabricated, " - f"{s['malformed']} malformed, {s['deleted']} deleted" + f"{s['malformed']} malformed, {s['deleted']} deleted, " + f"{s['ignored']} ignored" ) return "\n".join(lines) @@ -435,6 +493,9 @@ def main(argv: list[str]) -> int: help="override the marker regex; must have exactly one capture group") parser.add_argument("--min-line-chars", type=int, default=12, help="ignore base lines shorter than this after collapsing (default: 12)") + parser.add_argument("--strict", action="store_true", + help="without --tag, treat free-prose unquoted markers as malformed " + "(they are otherwise counted in the summary as 'ignored')") parser.add_argument("--json", action="store_true", help="emit result as JSON on stdout") parser.add_argument("--dryrun", "-n", action="store_true", @@ -490,6 +551,7 @@ def main(argv: list[str]) -> int: print(f"[dryrun] repo_root: {repo_root}") print(f"[dryrun] base: {args.base} -> {base_sha}") print(f"[dryrun] tag: {args.tag if args.tag else 'None'}") + print(f"[dryrun] strict: {args.strict}") print(f"[dryrun] marker: {marker_re.pattern}") print(f"[dryrun] min_line_chars: {args.min_line_chars}") print(f"[dryrun] files: {len(head_files)}") @@ -504,6 +566,7 @@ def main(argv: list[str]) -> int: marker_re=marker_re, globs=args.globs, min_line_chars=args.min_line_chars, + strict=args.strict, ) if args.json: diff --git a/specs/spec-revocation-check.spec.md b/specs/spec-revocation-check.spec.md index d027ec7..47a6fed 100644 --- a/specs/spec-revocation-check.spec.md +++ b/specs/spec-revocation-check.spec.md @@ -38,7 +38,8 @@ spec-revocation-check --base [--tag TAG] | Argument | Default | Description | |----------|---------|-------------| -| `--tag TAG` | — | Restrict the marker regex to `(TAG)`. Without it, match any tag AND the untagged form `Revokes prior text: ""`. | +| `--tag TAG` | — | Restrict the marker regex to `(TAG)`. Without it, match any tag AND the untagged form `Revokes prior text: ""`. With `--tag`, lines that do not contain the literal `Revokes prior text (TAG)` are entirely ignored — not parsed, not used as quotes, not counted. | +| `--strict` | off | Without `--tag`, lines that look like free-prose unquoted revocation markers (e.g. `**Revokes prior text:** the pre-… wording`) are reported as malformed. Default is to count them in the summary as `ignored (unquoted-style)` without failing. No effect with `--tag` (a non-parsing `(TAG)` line is already malformed). | | `--marker-regex REGEX` | built-in default | Override the line-shape regex. Must have exactly one capture group (the quoted text); exit 2 if it has zero or more than one. | | `--min-line-chars N` | `12` | Ignore non-blank base lines whose collapsed length is shorter than this (e.g. short structural lines). After collapsing. | | `--repo-root DIR` | `git rev-parse --show-toplevel` of cwd | Repo root. Never inferred from the script's own path. | @@ -66,7 +67,12 @@ spec-revocation-check --base [--tag TAG] - Default, with `--tag TAG`: `Revokes prior text \(TAG\):\s*"(.*)"` (the match is `re.search`, so the marker can appear mid-line inside a Markdown - table cell). + table cell). Lines that do not contain the literal `Revokes prior text (TAG)` + are not even candidates — they are skipped before the regex runs, so a + tagged marker for a different tag, an untagged marker, or a free-prose + marker cannot accidentally be classified as malformed. (Defect from + round 1: with `--tag M33`, every M23 / untagged / free-prose marker was + being reported as malformed.) - Default, without `--tag`: match either the tagged form for any single parenthesised token, OR the untagged form `Revokes prior text:\s*"(.*)"`. The untagged form is the M32-era marker; we keep it recognised so older @@ -82,18 +88,32 @@ containing `"`" case, which only works because of the greedy match. For every file matched by a glob at HEAD: -- Parse all marker occurrences in the file's current text. Collect - `(line_no, quote_text)`. -- For each parsed quote, collapse (whitespace-collapse, strip Markdown +- Walk the file line by line. For each line containing the phrase + `Revokes prior text`: + - **With `--tag TAG`**: if the line does not contain the literal + `Revokes prior text (TAG)`, skip it entirely (not parsed, not + reported, its text not used as a quote). Otherwise try the marker + regex; on match, record `(line_no, quote_text)`. On miss, record + a "malformed marker" violation. + - **Without `--tag`**: try the multi-branch marker regex + (tagged-any | untagged); on match, record `(line_no, quote_text)`. + On miss, classify the line: + - If the line is a **free-prose unquoted marker** — i.e. it + contains `Revokes prior text:` (the colon form, no parens) AND + has no `"` anywhere on the line — then with `--strict` record + it as a malformed-marker violation, otherwise silently ignore + it (count in the summary as `ignored (unquoted-style)` but do + not contribute to the exit code). + - Otherwise record it as a malformed-marker violation. +- For each recorded quote, collapse (whitespace-collapse, strip Markdown structural prefixes — blockquote `>`, list bullet `- * 1.`, table outer pipes) and assert it is a substring of the SAME file at `--base` after the same collapse. - If the file did not exist at `--base`, every parsed quote is a violation (a "fabricated quote" — you cannot revoke text from a file that did not yet exist). -- A line that contains `Revokes prior text` but does NOT match the marker - regex is a "malformed marker" violation. (Catches a typo'd or reworded - marker that would otherwise silently carry no enforceable quote.) +- **No-quote markers (free-prose) are never used as quote text to cover + deletions**, regardless of mode: they have no quote to extract. ### 4. Check B — completeness @@ -107,7 +127,9 @@ DIFFERS from `--base`: `***` / `___`), skip it. - Otherwise, the collapsed base line MUST be present in either: (a) the current file's collapsed text, OR - (b) the collapsed text of any of that file's parsed quotes. + (b) the collapsed text of any of that file's parsed quotes (NOT free- + prose lines — they have no quote text and so cannot cover a + deletion). - If neither, it is an "unquoted deletion" violation. ### 5. Check D — deleted / renamed files @@ -142,7 +164,7 @@ malformed markers: deleted since base: spec/old.md () -Summary: 3 violation(s) — 1 unquoted, 1 fabricated, 1 malformed, 0 deleted +Summary: 3 violation(s) — 1 unquoted, 1 fabricated, 1 malformed, 0 deleted, 2 ignored ``` The `` for `unquoted deletions` is the original base line (with @@ -168,7 +190,7 @@ If there are zero violations: `spec-revocation-check: clean — every "malformed_markers": [{"path": "spec/baz.md", "line": 3, "base": "...", "line_text": "..."}], "deleted_since_base": [{"path": "spec/old.md", "base": "..."}] }, - "summary": {"unquoted": 1, "fabricated": 1, "malformed": 1, "deleted": 0} + "summary": {"unquoted": 1, "fabricated": 1, "malformed": 1, "deleted": 0, "ignored": 2} } ``` @@ -183,6 +205,7 @@ The script is read-only by nature. `--dryrun` (and `-n`) prints: [dryrun] repo_root: [dryrun] base: -> [dryrun] tag: +[dryrun] strict: [dryrun] marker: [dryrun] min_line_chars: 12 [dryrun] files: @@ -210,7 +233,9 @@ to read the repo and report.) | File matched at HEAD, did not exist at base | Check A applies (any quote is a violation); Check B is skipped. | | File matched at base, does not exist at HEAD | Reported as `deleted since base` (Check D); unless git says renamed, in which case Check B is run on the renamed file. | | File matched at base, identical at HEAD | Skipped entirely. | -| File has `Revokes prior text` but no closing `"` on the same line | Reported as `malformed marker` (Check A). | +| File has `Revokes prior text` but no closing `"` on the same line | Reported as `malformed marker` (Check A), unless the line is a free-prose unquoted marker (see below). | +| With `--tag X`, line has `Revokes prior text (Y): "..."` for a different tag Y | Line is entirely ignored (not parsed, not used as a quote, not counted). Y's text is NOT used to cover deletions in this run. | +| Free-prose unquoted marker (no `--tag`): line contains `Revokes prior text:` (colon form, no parens) and has no `"` anywhere on the line | Without `--strict`: counted in the summary as `ignored (unquoted-style)`; does not contribute to exit code; never used as quote text to cover deletions. With `--strict`: reported as `malformed marker`. | | Quote text contains literal `"` characters | Handled by greedy match to the LAST `"` on the line. | | Two quote markers on one line (e.g. two revoked cells of one table row) | Both parsed separately by the regex (greedy `.*"` finds each tail `"`); each checked independently. | | Base file has only blank lines | Skipped (no non-blank base lines to lose). | @@ -257,4 +282,8 @@ spec-revocation-check --base $BASE_SHA --glob 'spec/**/*.md' --json | R11 | Output groups violations under `unquoted deletions`, `fabricated quotes`, `malformed markers`, `deleted since base`. Each violation is `path:line (base): `. Summary line at the end. | | R12 | `--json` emits the same data as a single JSON object on stdout (human-readable summary still on stderr). | | R13 | Exit 0 clean, 1 any violation, 2 usage/environment error. | -| R14 | `--dryrun` prints resolved repo root, base sha, tag, marker regex, file list, then runs the checks anyway. | \ No newline at end of file +| R14 | `--dryrun` prints resolved repo root, base sha, tag, marker regex, file list, then runs the checks anyway. | +| R15 | With `--tag TAG`, lines that do not contain the literal `Revokes prior text (TAG)` are entirely ignored: not parsed, not reported, and their text is not used as a quote to cover deletions. Only lines containing that literal are candidates; candidates that fail the marker regex are `malformed markers`. | +| R16 | Without `--tag`, a line containing `Revokes prior text` that fails the marker regex is classified as `free-prose unquoted` if it contains `Revokes prior text:` (the colon form, no parens) and has no `"` on the same line. With `--strict`, free-prose lines are `malformed markers`; otherwise they are counted in the summary as `ignored (unquoted-style)` and never contribute to the exit code. | +| R17 | Free-prose unquoted markers are NEVER used as quote text to cover deletions, in either tag or no-tag mode. | +| R18 | Summary line (human and JSON) gains an `ignored` count for free-prose unquoted markers seen in the current run. The human summary ends with `, I ignored` where I is the ignored count. The JSON `summary` dict gains an `"ignored"` integer key. | \ No newline at end of file diff --git a/tests/test-spec-revocation-check.sh b/tests/test-spec-revocation-check.sh index 6d32f8c..57a19c6 100755 --- a/tests/test-spec-revocation-check.sh +++ b/tests/test-spec-revocation-check.sh @@ -229,21 +229,36 @@ assert_exit "untagged marker no --tag exits 0" 0 "$rc" "$out" "" assert_contains "untagged marker no --tag says clean" "clean" "$out" # ---- Test 11: --tag filters other tags --------------------------------- +# With --tag X, lines containing markers for tag Y (or untagged or free- +# prose) are entirely ignored: not parsed, not reported, not used as +# quotes. The base line that the foreign marker quoted is still in the +# current file (inside the ignored marker text), so Check B still passes. echo "=== --tag filters other tags ===" T="$SCRATCH/t11"; mk_repo "$T" commit_file "$T" "spec/foo.md" \ $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null BASE=$(git -C "$T" rev-parse HEAD) -# Quote uses M33, but we run with --tag M99, so the marker is malformed -# (no closing quote because the regex requires "M99"). +# Quote uses M33, but we run with --tag M99 — the M33 marker is ignored +# entirely (not parsed as malformed). The base line text is still in the +# current file (inside the ignored marker), so Check B still passes. write_file_via_py "$T" "spec/foo.md" \ $'# Spec\n\nA line that stays.\n\nRevokes prior text (M33): "A line that gets revoked."\n\n' out=$(run_check "$T" "$BASE" --tag M99); rc=$? -assert_exit "--tag filters other tag exits 1" 1 "$rc" "$out" "" -# Without --tag, the marker WOULD have parsed and the line would survive. +assert_exit "--tag M99 ignores M33 marker exits 0" 0 "$rc" "$out" "" +assert_contains "--tag M99 ignores M33 says clean" "clean" "$out" +# Without --tag, the marker parses and the line is covered. out_notag=$(run_check "$T" "$BASE"); rc_notag=$? assert_exit "no --tag accepts M33 marker exits 0" 0 "$rc_notag" "$out_notag" "" +# Now run --tag M99 with an M33 marker that quotes something OTHER than +# a base line — the base line is then truly unquoted, and with --tag M99 +# the M33 marker doesn't cover it → unquoted deletion → violation. +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (M33): "Some other thing entirely."\n\n' +out=$(run_check "$T" "$BASE" --tag M99); rc=$? +assert_exit "--tag M99 + bogus M33 quote exits 1" 1 "$rc" "$out" "" +assert_contains "--tag M99 + bogus M33 quote names unquoted" "unquoted deletions" "$out" + # ---- Test 12: bad --base (exit 2) --------------------------------------- echo "=== bad --base ===" T="$SCRATCH/t12"; mk_repo "$T" @@ -286,6 +301,92 @@ out=$(python3 "$SCRIPT" --help); rc=$? assert_exit "--help exits 0" 0 "$rc" "$out" "" assert_contains "--help shows usage" "spec-revocation-check" "$out" assert_contains "--help mentions --base" "--base" "$out" +assert_contains "--help mentions --strict" "--strict" "$out" + +# ---- Tests 16-19: real-case fixture (round 2) ------------------------- +# One base file, one current file with four marker styles: +# - (M33) tagged quote (valid) +# - Older untagged mid-line marker (M32-era, M23 amendment note) +# - (M23) tagged quote +# - Free-prose `**Revokes prior text:**` line +# Each marker covers a real base line. With --tag M33 the foreign markers +# must be ignored; the free-prose line must be ignored unless --strict. +echo "=== --tag scopes marker parsing to the literal tag ===" +T="$SCRATCH/t16"; mk_repo "$T" +# Base: a stable line + one line per marker style + a short line that +# Check B skips (under min-line-chars=12). The short line is what the +# free-prose marker refers to (so Check B won't flag its deletion). +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nLine revoked by M33.\n\nLine revoked by untagged mid-line.\n\nLine revoked by M23.\n\nshort text\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (M33): "Line revoked by M33."\n\n*(Amended M23: ... — Revokes prior text: "Line revoked by untagged mid-line.".)*\n\nRevokes prior text (M23): "Line revoked by M23."\n\n> **Revokes prior text:** the short text\n' + +# Sub-case 1: --tag M33 — only the (M33) line is parsed; foreign markers +# are ignored entirely. The base lines they refer to are still verbatim +# in the current file (inside the ignored marker text), so Check B +# passes. Clean. +out=$(run_check "$T" "$BASE" --tag M33); rc=$? +assert_exit "--tag M33 ignores foreign markers exits 0" 0 "$rc" "$out" "" +assert_contains "--tag M33 ignores foreign says clean" "clean" "$out" + +# Sub-case 2: malformed (M33) line with --tag M33 — flagged. The other +# foreign markers must STILL be ignored (not malformed). +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (M33) no quote\n\n*(Amended M23: ... — Revokes prior text: "Line revoked by untagged mid-line.".)*\n\nRevokes prior text (M23): "Line revoked by M23."\n\n> **Revokes prior text:** the short text\n' +out=$(run_check "$T" "$BASE" --tag M33); rc=$? +assert_exit "malformed (M33) line exits 1" 1 "$rc" "$out" "" +assert_contains "malformed (M33) under header" "malformed markers" "$out" +assert_contains "malformed (M33) names the raw line" "no quote" "$out" + +# Restore the valid fixture for sub-cases 3 and 4. +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (M33): "Line revoked by M33."\n\n*(Amended M23: ... — Revokes prior text: "Line revoked by untagged mid-line.".)*\n\nRevokes prior text (M23): "Line revoked by M23."\n\n> **Revokes prior text:** the short text\n' + +# Sub-case 3: no --tag — foreign markers parse (untagged branch + M23 +# tagged branch); the free-prose line is silently ignored by default. +# Output: zero violations but a summary line that counts the ignored +# marker (NOT the early "clean — every line survives" message, since +# ignored > 0). +out=$(run_check "$T" "$BASE"); rc=$? +assert_exit "no --tag, free-prose ignored, exits 0" 0 "$rc" "$out" "" +assert_contains "no --tag, free-prose ignored, shows summary line" "Summary:" "$out" +assert_contains "no --tag, free-prose ignored, summary shows ignored count" "1 ignored" "$out" + +# Sub-case 4: no --tag --strict — free-prose becomes malformed. +out=$(run_check "$T" "$BASE" --strict); rc=$? +assert_exit "no --tag --strict, free-prose malformed exits 1" 1 "$rc" "$out" "" +assert_contains "no --tag --strict, free-prose under header" "malformed markers" "$out" + +# ---- Round-2 dedicated negative fixtures -------------------------------- +# The negative-test proof loop below needs fixtures that exit 1 +# consistently (not the ones above, whose state mutates between sub- +# cases). t17 is a malformed-(M33) fixture, t19 is a free-prose line with +# --strict; both should fail when run against the synthetic no-op. + +# ---- Test 17 (negative): malformed (M33) line with --tag M33 ------------ +echo "=== negative: malformed (M33) line with --tag M33 ===" +T="$SCRATCH/t17"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nA line that gets revoked.\n\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nRevokes prior text (M33) no quote\n\n' +out=$(run_check "$T" "$BASE" --tag M33); rc=$? +assert_exit "negative t17 malformed (M33) exits 1" 1 "$rc" "$out" "" +assert_contains "negative t17 malformed (M33) under header" "malformed markers" "$out" + +# ---- Test 19 (negative): free-prose line with no --tag --strict --------- +echo "=== negative: free-prose with no --tag --strict ===" +T="$SCRATCH/t19"; mk_repo "$T" +commit_file "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\nshort text\n' >/dev/null +BASE=$(git -C "$T" rev-parse HEAD) +write_file_via_py "$T" "spec/foo.md" \ + $'# Spec\n\nA line that stays.\n\n> **Revokes prior text:** the short text\n' +out=$(run_check "$T" "$BASE" --strict); rc=$? +assert_exit "negative t19 free-prose strict exits 1" 1 "$rc" "$out" "" +assert_contains "negative t19 free-prose strict under header" "malformed markers" "$out" # ---- Negative-test verification (would each negative fail if disabled?) - # A "negative" test would PASS against a no-op script that always exits 0. @@ -317,7 +418,7 @@ sys.exit(0) PYEOF chmod +x "$SCRATCH/nop.py" -for case in t2 t3 t4 t6 t7; do +for case in t2 t3 t4 t6 t7 t17 t19; do T="$SCRATCH/$case" BASE=$(git -C "$T" rev-parse HEAD 2>/dev/null || echo "HEAD") out_nop=$(python3 "$SCRATCH/nop.py" --base HEAD --repo-root "$T" --glob "spec/**/*.md" 2>&1); rc_nop=$?