From d593ef382cbb23f8cee2f256a565a29b463575f8 Mon Sep 17 00:00:00 2001 From: Akkari Date: Thu, 12 Mar 2026 19:57:12 -0700 Subject: [PATCH] enhance: PS-007 repo-wide link resolution + staleness prevention - Add _find_repo_root() to both prescreen implementations (ref-impl + Copilot CLI port) - Fix traversal guard: resolve relative links against repo root (.git boundary), not file parent dir. Allows ../../target.md within repo while blocking escape. - Add --scan-links [DIR] batch mode to Copilot CLI prescreen (walks all .md files) - Add check_framework_version_strings() to validate-docs.py (flags stale version headers in framework docs vs critic-status.yaml) - 9 new tests (4 ref-impl traversal + 5 batch scan) Self-test: 0 broken links across 62 .md files. Full suite: 883 passed, 6 skipped. Origin: PS-007 task (2026-03-12), PR #16 retro (broken links from file moves). --- ports/copilot-cli/quorum-prescreen.py | 97 ++++++++++++++++++- ports/copilot-cli/test_quorum_prescreen.py | 61 ++++++++++++ reference-implementation/quorum/prescreen.py | 27 +++++- .../tests/test_prescreen.py | 57 +++++++++++ tools/validate-docs.py | 55 +++++++++++ 5 files changed, 293 insertions(+), 4 deletions(-) create mode 100644 ports/copilot-cli/test_quorum_prescreen.py diff --git a/ports/copilot-cli/quorum-prescreen.py b/ports/copilot-cli/quorum-prescreen.py index 1e5b3f7..2d3713f 100755 --- a/ports/copilot-cli/quorum-prescreen.py +++ b/ports/copilot-cli/quorum-prescreen.py @@ -13,6 +13,7 @@ Usage: python3 quorum-prescreen.py + python3 quorum-prescreen.py --scan-links [DIR] """ from __future__ import annotations @@ -164,6 +165,23 @@ def _mask(m: re.Match) -> str: return redacted[:max_len] +# ── Repo-root discovery ────────────────────────────────────────────────────── + +def _find_repo_root(start: Path) -> Path: + """Walk up from *start* to find the nearest directory containing .git. + + Returns *start* (resolved) if no .git is found. + """ + current = start.resolve() + while True: + if (current / ".git").exists(): + return current + parent = current.parent + if parent == current: + return start.resolve() + current = parent + + # ── Check dict factory helpers ──────────────────────────────────────────────── def _make_pass( @@ -412,6 +430,7 @@ def ps006_python_syntax(artifact_path: Path, artifact_text: str) -> dict: def ps007_broken_md_links(artifact_path: Path, artifact_text: str) -> dict: """PS-007: Detect broken relative links in Markdown files.""" base_dir = artifact_path.parent + repo_root = _find_repo_root(base_dir) broken: list[tuple[int, str, str]] = [] # (line_no, text, target) for i, line in enumerate(artifact_text.splitlines(), start=1): @@ -425,6 +444,11 @@ def ps007_broken_md_links(artifact_path: Path, artifact_text: str) -> dict: if not path_part: continue # anchor-only link resolved = (base_dir / path_part).resolve() + # Skip links that escape the repo root + try: + resolved.relative_to(repo_root) + except ValueError: + continue # path traversal — escapes repo boundary if not resolved.exists(): broken.append((i, match.group(1), target)) @@ -448,6 +472,61 @@ def ps007_broken_md_links(artifact_path: Path, artifact_text: str) -> dict: ) +# ── Batch link scanning ────────────────────────────────────────────────────── + +_SKIP_DIRS = { + ".git", "node_modules", "venv", "__pycache__", "dist", ".hypothesis", + "quorum-runs", # historical run output — links are point-in-time snapshots + "golden-test-set", # test artifacts with intentionally broken links + "external-reviews", "reviews", # external review snapshots +} + + +def scan_all_links(repo_root: Path | None = None) -> dict: + """Walk all .md files under *repo_root* and run PS-007 on each. + + Returns a consolidated JSON-serialisable dict with per-file results + and a summary. If *repo_root* is None, auto-detect from cwd. + """ + if repo_root is None: + repo_root = _find_repo_root(Path.cwd()) + + if not repo_root.is_dir(): + return {"error": f"Not a directory: {repo_root}", "files": [], "total_broken": 0} + + md_files: list[Path] = [] + for md_file in sorted(repo_root.rglob("*.md")): + parts = md_file.relative_to(repo_root).parts + if any(p.startswith(".") or p in _SKIP_DIRS for p in parts): + continue + md_files.append(md_file) + + results: list[dict] = [] + total_broken = 0 + + for md_file in md_files: + try: + text = md_file.read_text(encoding="utf-8", errors="replace") + except OSError: + continue + check = ps007_broken_md_links(md_file, text) + if check["status"] == "FAIL": + rel = str(md_file.relative_to(repo_root)) + # Extract broken count from description + count_match = re.match(r"(\d+) broken", check.get("details", "")) + file_broken = int(count_match.group(1)) if count_match else 0 + total_broken += file_broken + results.append({"file": rel, **check}) + + return { + "repo_root": str(repo_root), + "files_scanned": len(md_files), + "files_with_broken_links": len(results), + "total_broken": total_broken, + "results": results, + } + + # ── PS-008: TODO markers ───────────────────────────────────────────────────── def ps008_todo_markers(artifact_path: Path, artifact_text: str) -> dict: @@ -686,8 +765,24 @@ def run_prescreen(target_path: Path) -> dict: def main() -> None: + # ── --scan-links mode ──────────────────────────────────────────────── + if len(sys.argv) >= 2 and sys.argv[1] == "--scan-links": + scan_dir = Path(sys.argv[2]).resolve() if len(sys.argv) >= 3 else None + result = scan_all_links(scan_dir) + if "error" in result: + print(f"Error: {result['error']}", file=sys.stderr) + sys.exit(1) + json.dump(result, sys.stdout, indent=2) + print() + sys.exit(0 if result["total_broken"] == 0 else 1) + + # ── Single-file mode (existing behaviour) ──────────────────────────── if len(sys.argv) != 2: - print("Usage: quorum-prescreen.py ", file=sys.stderr) + print( + "Usage: quorum-prescreen.py \n" + " quorum-prescreen.py --scan-links [DIR]", + file=sys.stderr, + ) sys.exit(2) target = Path(sys.argv[1]).resolve() diff --git a/ports/copilot-cli/test_quorum_prescreen.py b/ports/copilot-cli/test_quorum_prescreen.py new file mode 100644 index 0000000..8974b45 --- /dev/null +++ b/ports/copilot-cli/test_quorum_prescreen.py @@ -0,0 +1,61 @@ +"""Tests for the Copilot CLI quorum-prescreen.py — batch link scanning.""" + +from __future__ import annotations + +import importlib +import sys +from pathlib import Path + +import pytest + +# Add the copilot-cli directory to sys.path so we can import the module +_CLI_DIR = str(Path(__file__).resolve().parent) +if _CLI_DIR not in sys.path: + sys.path.insert(0, _CLI_DIR) + +# Import from the hyphenated filename +quorum_prescreen = importlib.import_module("quorum-prescreen") + + +class TestScanAllLinks: + """Tests for the scan_all_links batch scanning function.""" + + def test_clean_repo_returns_zero_broken(self, tmp_path): + (tmp_path / ".git").mkdir() + (tmp_path / "README.md").write_text("# Hello\n[other](other.md)\n") + (tmp_path / "other.md").write_text("# Other\n") + result = quorum_prescreen.scan_all_links(tmp_path) + assert result["total_broken"] == 0 + assert result["files_with_broken_links"] == 0 + + def test_broken_link_detected_across_files(self, tmp_path): + (tmp_path / ".git").mkdir() + (tmp_path / "README.md").write_text("[missing](NONEXISTENT.md)\n") + (tmp_path / "docs").mkdir() + (tmp_path / "docs" / "guide.md").write_text("[also missing](nope.md)\n") + result = quorum_prescreen.scan_all_links(tmp_path) + assert result["files_with_broken_links"] == 2 + assert result["total_broken"] >= 2 + + def test_skips_hidden_dirs(self, tmp_path): + (tmp_path / ".git").mkdir() + git_md = tmp_path / ".git" / "description.md" + git_md.write_text("[broken](nope.md)\n") + (tmp_path / "README.md").write_text("# Clean\n") + result = quorum_prescreen.scan_all_links(tmp_path) + assert result["files_with_broken_links"] == 0 + + def test_upward_links_within_repo_not_false_positive(self, tmp_path): + (tmp_path / ".git").mkdir() + (tmp_path / "SPEC.md").write_text("# Spec\n") + sub = tmp_path / "docs" / "guides" + sub.mkdir(parents=True) + (sub / "tutorial.md").write_text("[spec](../../SPEC.md)\n") + result = quorum_prescreen.scan_all_links(tmp_path) + assert result["total_broken"] == 0 + + def test_non_directory_returns_error(self, tmp_path): + f = tmp_path / "file.txt" + f.write_text("hi") + result = quorum_prescreen.scan_all_links(f) + assert "error" in result diff --git a/reference-implementation/quorum/prescreen.py b/reference-implementation/quorum/prescreen.py index 06fc5a8..dc7a699 100644 --- a/reference-implementation/quorum/prescreen.py +++ b/reference-implementation/quorum/prescreen.py @@ -98,6 +98,26 @@ _RE_TRAILING_SPACE = re.compile(r"[ \t]+$", re.MULTILINE) +# ── Repo-root discovery ─────────────────────────────────────────────────────── + +def _find_repo_root(start: Path) -> Path: + """Walk up from *start* to find the nearest directory containing .git. + + Returns *start* (resolved) if no .git is found — this preserves the + old behaviour where files outside a repo are bounded to their own + directory. + """ + current = start.resolve() + while True: + if (current / ".git").exists(): + return current + parent = current.parent + if parent == current: + # Reached filesystem root without finding .git + return start.resolve() + current = parent + + # ── Helper ───────────────────────────────────────────────────────────────────── def _scan_lines( @@ -442,6 +462,7 @@ def _ps007_broken_md_links( ) -> PreScreenCheck: """PS-007: Detect broken relative links in Markdown files.""" base_dir = artifact_path.parent + repo_root = _find_repo_root(base_dir) broken: list[tuple[int, str, str]] = [] # (line_no, text, target) for i, line in enumerate(artifact_text.splitlines(), start=1): @@ -458,11 +479,11 @@ def _ps007_broken_md_links( if not path_part: continue # anchor-only link resolved = (base_dir / path_part).resolve() - # V002 fix: skip links that escape artifact directory + # V002 fix: skip links that escape the repo root try: - resolved.relative_to(base_dir.resolve()) + resolved.relative_to(repo_root) except ValueError: - continue # path traversal — don't follow + continue # path traversal — escapes repo boundary if not resolved.exists(): broken.append((i, match.group(1), target)) diff --git a/reference-implementation/tests/test_prescreen.py b/reference-implementation/tests/test_prescreen.py index 073d60d..16b9258 100644 --- a/reference-implementation/tests/test_prescreen.py +++ b/reference-implementation/tests/test_prescreen.py @@ -406,6 +406,63 @@ def test_ignores_mailto_links(self, ps, tmp_path): assert check.result == "PASS" +class TestPrescreenPS007RepoRootTraversal: + """Tests for the V002 fix: repo-root-bounded traversal guard.""" + + def test_upward_traversal_within_repo_allowed(self, ps, tmp_path): + """A link like ../../target.md that resolves inside the repo root should PASS.""" + (tmp_path / ".git").mkdir() + (tmp_path / "target.md").write_text("# Target\n") + subdir = tmp_path / "docs" / "guides" + subdir.mkdir(parents=True) + f = subdir / "tutorial.md" + f.write_text("[spec](../../target.md)\n") + result = ps.run(f, f.read_text()) + check = next(c for c in result.checks if c.id == "PS-007") + assert check.result == "PASS", ( + "Up-traversal to repo root should not be flagged as broken" + ) + + def test_upward_traversal_outside_repo_blocked(self, ps, tmp_path): + """A link that escapes past the .git boundary should be silently skipped.""" + repo = tmp_path / "myrepo" + repo.mkdir() + (repo / ".git").mkdir() + (tmp_path / "outside.md").write_text("# Outside\n") + f = repo / "README.md" + f.write_text("[escape](../outside.md)\n") + result = ps.run(f, f.read_text()) + check = next(c for c in result.checks if c.id == "PS-007") + assert check.result == "PASS", ( + "Links escaping repo root should be silently skipped, not flagged broken" + ) + + def test_no_git_dir_falls_back_to_parent(self, ps, tmp_path): + """Without .git, falls back to artifact parent dir (old behaviour).""" + subdir = tmp_path / "sub" + subdir.mkdir() + f = subdir / "doc.md" + f.write_text("[up](../nonexistent.md)\n") + result = ps.run(f, f.read_text()) + check = next(c for c in result.checks if c.id == "PS-007") + # Without .git, _find_repo_root returns the start dir (artifact parent). + # ../nonexistent.md resolves outside that boundary, so it's silently skipped. + assert check.result == "PASS" + + def test_broken_link_within_repo_still_detected(self, ps, tmp_path): + """A broken link that stays within repo boundaries is still caught.""" + (tmp_path / ".git").mkdir() + subdir = tmp_path / "docs" + subdir.mkdir() + f = subdir / "guide.md" + f.write_text("[missing](../NONEXISTENT.md)\n") + result = ps.run(f, f.read_text()) + check = next(c for c in result.checks if c.id == "PS-007") + assert check.result == "FAIL", ( + "Broken links within repo should still be detected" + ) + + # ── PS-008: TODO Markers ───────────────────────────────────────────────────── diff --git a/tools/validate-docs.py b/tools/validate-docs.py index fae0799..0ab3766 100755 --- a/tools/validate-docs.py +++ b/tools/validate-docs.py @@ -338,6 +338,59 @@ def check_depth_config_claims( return findings +# ── Framework version header staleness ──────────────────────────────────────── + +# File basenames where version headers are expected and should be current +_FRAMEWORK_DOC_SUFFIXES = ("_FRAMEWORK.md", "IMPLEMENTATION.md", "TESTER_CRITIC_BRIEF.md") + +_RE_VERSION_HEADER = re.compile( + r""" + (?: + \*{2}v(\d+\.\d+\.\d+)\s+(?:State|Status) # **v0.5.1 State + | \#{1,4}\s+v(\d+\.\d+\.\d+)\s+(?:State|Status) # ## v0.5.1 Status + | Status\s*\(v(\d+\.\d+\.\d+)\) # Status (v0.5.1) + ) + """, + re.VERBOSE | re.IGNORECASE, +) + + +def check_framework_version_strings( + lines: list[str], manifest_version: str, file_path: Path +) -> list[str]: + """Flag version strings in framework docs that don't match the manifest. + + Only applies to files whose name ends with a known framework suffix. + Skips version strings inside fenced code blocks. + """ + name = str(file_path) + if not any(name.endswith(s) for s in _FRAMEWORK_DOC_SUFFIXES): + return [] + + findings: list[str] = [] + in_code_block = False + + for i, line in enumerate(lines, 1): + stripped = line.strip() + if stripped.startswith("```"): + in_code_block = not in_code_block + continue + if in_code_block: + continue + + m = _RE_VERSION_HEADER.search(line) + if m: + found_version = next(g for g in m.groups() if g is not None) + if found_version != manifest_version: + findings.append( + f" {file_path}:{i}: Stale version header " + f"'v{found_version}' (manifest={manifest_version}): " + f"{line.strip()[:MAX_LINE_DISPLAY_CHARS]}" + ) + + return findings + + def validate_docs(repo_root: Path) -> list[str]: """Run all validation checks and return list of findings.""" manifest = load_manifest(repo_root) @@ -365,6 +418,7 @@ def validate_docs(repo_root: Path) -> list[str]: all_findings.extend(check_stale_status_markers(lines, shipped, rel_path)) all_findings.extend(check_roadmap_shipped(lines, shipped, rel_path)) all_findings.extend(check_depth_config_claims(lines, manifest, rel_path)) + all_findings.extend(check_framework_version_strings(lines, manifest_version, rel_path)) return all_findings @@ -417,6 +471,7 @@ def main() -> int: all_findings.extend(check_stale_status_markers(lines, shipped, rel_path)) all_findings.extend(check_roadmap_shipped(lines, shipped, rel_path)) all_findings.extend(check_depth_config_claims(lines, manifest, rel_path)) + all_findings.extend(check_framework_version_strings(lines, manifest_version, rel_path)) if all_findings: print(f"FINDINGS ({len(all_findings)}):\n")