fix: allow yaml header edits in append-only check
This commit is contained in:
parent
e84b0757f8
commit
d6a88e483b
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
|
|
@ -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
|
||||
Loading…
Reference in New Issue