From d6a88e483bd4a88933250b867ea1a267ff913562 Mon Sep 17 00:00:00 2001 From: rob Date: Sat, 1 Nov 2025 23:05:27 -0300 Subject: [PATCH] fix: allow yaml header edits in append-only check --- assets/hooks/pre-commit | 26 +------- automation/append_only.py | 137 ++++++++++++++++++++++++++++++++++++++ tests/test_append_only.py | 82 +++++++++++++++++++++++ 3 files changed, 220 insertions(+), 25 deletions(-) create mode 100644 automation/append_only.py create mode 100644 tests/test_append_only.py diff --git a/assets/hooks/pre-commit b/assets/hooks/pre-commit index 09d21eb..127a381 100755 --- a/assets/hooks/pre-commit +++ b/assets/hooks/pre-commit @@ -38,33 +38,9 @@ cd "$ROOT" # ============================================================================ check_append_only_discussion() { local disc_file="$1" - local diff_output - - # Get the staged diff for this file - diff_output=$(git diff --cached "$disc_file") - - # Look for deletion lines (lines starting with "-" but not "---" file markers) - # If we find any, the user may have deleted existing content - if echo "$diff_output" | grep -E "^-[^-]" | grep -Ev "^--- a/" | grep -Ev "^\+\+\+ b/"; then - echo >&2 "[pre-commit] Error: Deletions or modifications detected in existing lines of $disc_file." - echo >&2 " Discussion files must be append-only, except for allowed header fields." + if ! python3 -m automation.append_only "$disc_file"; then return 1 fi - - # Check for modifications to YAML header fields - # These are allowed (e.g., updating status: OPEN → READY_FOR_IMPLEMENTATION) - # A more robust check would parse YAML front matter instead of using regex - local header_modified=0 - if echo "$diff_output" | grep -E "^[-+]" | grep -E "^(status|created|updated|feature_id|stage_id):" > /dev/null; then - header_modified=1 - fi - - # If there are additions and no body deletions, we're good - # This assumes additions are at the end (append-only) - if echo "$diff_output" | grep -E "^\+[^+]" | grep -Ev "^\+\+\+ b/" > /dev/null && [ "$header_modified" -eq 0 ]; then - : # Valid: additions only (append-only pattern) - fi - return 0 } diff --git a/automation/append_only.py b/automation/append_only.py new file mode 100644 index 0000000..5ab1c4a --- /dev/null +++ b/automation/append_only.py @@ -0,0 +1,137 @@ +""" +Append-only discussion checker. + +Validates that staged changes to discussion Markdown files only modify YAML +front matter; the body must remain append-only (no deletions or edits). +""" +from __future__ import annotations + +import argparse +import re +import subprocess +import sys +from pathlib import Path +from typing import Tuple + +DIFF_HUNK_RE = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") + + +def _run_git_show(ref: str, path: Path) -> str | None: + """Return file contents for ref:path, or None if unavailable.""" + result = subprocess.run( + ["git", "show", f"{ref}:{path.as_posix()}"], + capture_output=True, + text=True, + ) + if result.returncode != 0: + return None + return result.stdout + + +def _find_front_matter_end(text: str | None) -> int: + """Return 1-based line number of closing --- in YAML front matter.""" + if not text: + return 0 + lines = text.splitlines() + if not lines or lines[0].strip() != "---": + return 0 + for idx, line in enumerate(lines[1:], start=2): + if line.strip() == "---": + return idx + return len(lines) + + +def _get_staged_diff(path: Path) -> str: + result = subprocess.run( + ["git", "diff", "--cached", "--unified=0", "--", path.as_posix()], + capture_output=True, + text=True, + check=False, + ) + # git diff exits 0 regardless of diff presence + return result.stdout + + +def _is_allowed(diff_text: str, header_end: int) -> Tuple[bool, str]: + """ + Return (allowed, offending_line) for staged diff. + + header_end is the line number of the closing front matter delimiter in + the previous version (0 if no front matter detected). + """ + current_old_line = None + for line in diff_text.splitlines(): + if line.startswith("diff --git") or line.startswith("index "): + continue + if line.startswith("--- ") or line.startswith("+++ "): + continue + if line.startswith("\\"): + continue + if line.startswith("@@"): + match = DIFF_HUNK_RE.match(line) + if match: + current_old_line = int(match.group(1)) + else: + current_old_line = None + continue + if current_old_line is None: + continue + + prefix = line[:1] + if prefix == "-": + # header_end == 0 means no header detected => forbid deletions + if header_end == 0 or current_old_line > header_end: + return False, line + current_old_line += 1 + elif prefix == " ": + current_old_line += 1 + elif prefix == "+": + # additions do not advance old line counter + pass + + return True, "" + + +def check_append_only(path: Path) -> Tuple[bool, str]: + diff_text = _get_staged_diff(path) + if not diff_text.strip(): + return True, "" + + old_content = _run_git_show("HEAD", path) + header_end = _find_front_matter_end(old_content) + allowed, offending = _is_allowed(diff_text, header_end) + return allowed, offending + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Ensure discussion files are append-only outside YAML header." + ) + parser.add_argument("path", help="Discussion file path (relative or absolute)") + args = parser.parse_args(argv) + + raw_path = Path(args.path) + path = raw_path + cwd = Path.cwd().resolve() + if raw_path.is_absolute(): + try: + path = raw_path.resolve().relative_to(cwd) + except ValueError: + sys.stderr.write( + f"[pre-commit] Error: {raw_path} is outside the repository root\n" + ) + return 1 + + allowed, offending_line = check_append_only(path) + if allowed: + return 0 + + sys.stderr.write( + "[pre-commit] Error: Discussion files must be append-only; " + f"modification detected outside YAML header:\n {offending_line}\n" + ) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_append_only.py b/tests/test_append_only.py new file mode 100644 index 0000000..da3150f --- /dev/null +++ b/tests/test_append_only.py @@ -0,0 +1,82 @@ +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] +PY_ENV = os.environ.copy() +PY_ENV["PYTHONPATH"] = ( + f"{REPO_ROOT}:{PY_ENV['PYTHONPATH']}" + if PY_ENV.get("PYTHONPATH") + else str(REPO_ROOT) +) + + +def run(cmd: list[str], cwd: Path) -> subprocess.CompletedProcess[str]: + return subprocess.run(cmd, cwd=cwd, check=True, text=True, capture_output=True) + + +@pytest.fixture() +def git_repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + run(["git", "init"], cwd=repo) + run(["git", "config", "user.email", "dev@example.com"], cwd=repo) + run(["git", "config", "user.name", "Dev"], cwd=repo) + + content = """--- +type: feature-discussion +status: OPEN +feature_id: FR_1 +--- + +## Summary +Initial summary + +## Participation +- Maintainer: Kickoff. VOTE: READY +""" + (repo / "Docs/features/FR_1/discussions").mkdir(parents=True, exist_ok=True) + discussion = repo / "Docs/features/FR_1/discussions/example.discussion.md" + discussion.write_text(content, encoding="utf-8") + run(["git", "add", discussion.as_posix()], cwd=repo) + run(["git", "commit", "-m", "init"], cwd=repo) + + return repo + + +def test_header_change_allowed(git_repo: Path) -> None: + discussion = git_repo / "Docs/features/FR_1/discussions/example.discussion.md" + lines = discussion.read_text(encoding="utf-8").splitlines() + lines[2] = "status: READY_FOR_DESIGN" + discussion.write_text("\n".join(lines) + "\n", encoding="utf-8") + run(["git", "add", discussion.as_posix()], cwd=git_repo) + + proc = subprocess.run( + [sys.executable, "-m", "automation.append_only", discussion.as_posix()], + cwd=git_repo, + text=True, + env=PY_ENV, + ) + assert proc.returncode == 0 + + +def test_body_edit_blocked(git_repo: Path) -> None: + discussion = git_repo / "Docs/features/FR_1/discussions/example.discussion.md" + text = discussion.read_text(encoding="utf-8") + discussion.write_text( + text.replace("Initial summary", "Updated summary"), encoding="utf-8" + ) + run(["git", "add", discussion.as_posix()], cwd=git_repo) + + proc = subprocess.run( + [sys.executable, "-m", "automation.append_only", discussion.as_posix()], + cwd=git_repo, + capture_output=True, + text=True, + env=PY_ENV, + ) + assert proc.returncode == 1 + assert "append-only" in proc.stderr