From eaceb626a801e0bd51b4839c3200f2cf5dd41e24 Mon Sep 17 00:00:00 2001 From: Sandeep Date: Mon, 27 Jul 2026 15:48:12 -0700 Subject: [PATCH 1/2] chore(ci): fail the build on a surviving merge-conflict marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A conflict marker that outlives its resolution is invisible to review, renders broken on GitHub, and can leak an internal branch name into a release artifact. Nothing currently catches one. Deliberately not pre-commit-hooks' `check-merge-conflict`: that hook only inspects files while a merge is in progress, so a marker already committed on a branch you later merge walks straight past it. This scans the tracked tree unconditionally, and CI re-runs it as the copy that cannot be skipped with `--no-verify`. The matching rule is asymmetric on purpose, because one marker is ambiguous: `<<<<<<< ` / `>>>>>>> ` never legitimate -> always an error `=======` a valid Markdown setext heading underline, so it is reported ONLY when the same file also carries an unambiguous marker A real conflict always leaves at least one unambiguous marker, so the ambiguous one becomes attributable instead of guessed at — and ordinary prose with a setext heading does not fail the build. Both behaviours are covered by the probes in the PR description. Runs as its own CI job rather than a step on `gitleaks`, so a failure reads as what it is instead of arriving under a secret-scan heading. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 11 +++++ .pre-commit-config.yaml | 14 +++++++ dev/check_conflict_markers.py | 79 +++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+) create mode 100644 dev/check_conflict_markers.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16c8fe68..be443612 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,6 +49,17 @@ jobs: --with-editable "packages/agami-core[model,server]" pytest tests/ -q --cov=plugins --cov=packages/agami-core/src --cov-report=term-missing + hygiene: + name: hygiene (conflict markers) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + # Its own job rather than a step on `gitleaks`, so a failure here reads as what it + # is instead of arriving under a secret-scan heading. Pre-commit runs the same + # script locally; this is the copy that cannot be skipped with `--no-verify`. + - name: no merge-conflict markers + run: python3 dev/check_conflict_markers.py + gitleaks: name: gitleaks (secret scan) runs-on: ubuntu-latest diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 01ad1548..112fcf65 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -27,6 +27,20 @@ repos: hooks: - id: gitleaks + # Merge-conflict markers — a marker that survives a resolution is invisible to review + # and renders broken in a public repo. Deliberately NOT pre-commit-hooks' + # `check-merge-conflict`: that one only inspects files while a merge is in progress, so a + # marker already committed on a branch you later merge walks straight past it. This scans + # the tracked tree unconditionally. CI re-runs it as the unbypassable gate. + - repo: local + hooks: + - id: conflict-markers + name: no merge-conflict markers + entry: python3 dev/check_conflict_markers.py + language: system + always_run: true + pass_filenames: false + # Test suite — runs at PRE-PUSH (not on every commit) so commits stay fast but nothing # red leaves your machine. Same deps + command CI uses. - repo: local diff --git a/dev/check_conflict_markers.py b/dev/check_conflict_markers.py new file mode 100644 index 00000000..3df6dcbb --- /dev/null +++ b/dev/check_conflict_markers.py @@ -0,0 +1,79 @@ +#!/usr/bin/env python3 +"""Fail if a merge-conflict marker survived into a tracked file. + +Why this exists as its own check rather than pre-commit's `check-merge-conflict`: +that hook only inspects files **while a merge is in progress**, so a marker that is +committed and only noticed later — or one that arrives on a branch you merged — walks +straight past it. This runs over the tracked tree unconditionally, in CI, where it +cannot be skipped with `--no-verify`. + +The matching rule is deliberately asymmetric, because one of the three markers is +ambiguous: + + `<<<<<<< ` and `>>>>>>> ` never occur in legitimate content -> always an error. + `=======` is a valid Markdown setext heading underline + (`Title` on one line, `=======` under it), so flagging it + unconditionally would false-positive on ordinary prose. + It is only reported when the same file also carries one of + the unambiguous markers. + +That asymmetry is the point: a real conflict always leaves at least one unambiguous +marker, and the ambiguous one is then attributable rather than guessed at. +""" + +from __future__ import annotations + +import subprocess +import sys + +# The two markers that are never legitimate content, and the ambiguous third. +UNAMBIGUOUS = ("<<<<<<< ", ">>>>>>> ") +AMBIGUOUS = "=======" + +# Binary-ish suffixes we should not try to decode. +SKIP_SUFFIXES = (".png", ".jpg", ".jpeg", ".gif", ".ico", ".pdf", ".woff", ".woff2", ".zip", ".gz") + + +def tracked_files() -> list[str]: + """Every file git tracks, so an untracked scratch file cannot fail the build.""" + out = subprocess.run( + ["git", "ls-files"], capture_output=True, text=True, check=True + ).stdout + return [p for p in out.splitlines() if p and not p.endswith(SKIP_SUFFIXES)] + + +def scan(path: str) -> list[tuple[int, str]]: + """Return (line_number, line) for each conflict marker in one file.""" + try: + text = open(path, encoding="utf-8", errors="strict").read() + except (UnicodeDecodeError, OSError): + return [] # binary or unreadable — nothing meaningful to match + lines = text.splitlines() + hard = [ + (n, line) + for n, line in enumerate(lines, 1) + if any(line.startswith(m) for m in UNAMBIGUOUS) + ] + if not hard: + return [] # no unambiguous marker -> a bare `=======` is a setext heading, not a conflict + soft = [(n, line) for n, line in enumerate(lines, 1) if line.rstrip() == AMBIGUOUS] + return sorted(hard + soft) + + +def main() -> int: + findings = [(p, n, line) for p in tracked_files() for n, line in scan(p)] + if not findings: + print("✓ no merge-conflict markers in tracked files") + return 0 + print("✗ merge-conflict markers found in tracked files:\n", file=sys.stderr) + for path, n, line in findings: + print(f" {path}:{n}: {line[:80]}", file=sys.stderr) + print( + "\nResolve the conflict and remove every marker before committing.", + file=sys.stderr, + ) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) From b1a85baab05a85e36a84ee923d5edcf1c0e931e5 Mon Sep 17 00:00:00 2001 From: Sandeep Date: Mon, 27 Jul 2026 16:14:49 -0700 Subject: [PATCH 2/2] fix(ci): close the conflict-marker gate's silent-pass paths, and test it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the gate reporting a pass over files it never read. Three tracked files holding live conflict blocks — café.py, docs/日本語/readme.md, and a .py with one latin-1 byte — scanned clean and exited 0. Root cause: scan() returned the same value for "read it, it was clean" and "couldn't read it at all". - git ls-files -z: core.quotePath C-quotes any non-ASCII/control/quote path into a string that isn't openable, so the file was skipped silently and a non-ASCII directory hid its whole subtree. - Match on bytes, not a strict UTF-8 decode: one latin-1 byte anywhere discarded the entire file's scan. Verified git does text-merge such files and does write markers into them. - Unreadable is now its own failing state (chmod 000, sparse checkouts, submodule gitlinks), not silence. - Detect diff3/zdiff3's fourth marker. The message said "remove every marker" while printing only three of four; deleting exactly what was printed left a residue that re-scanned clean. - Match runs of 7+ so .gitattributes conflict-marker-size is covered, and make the label optional. - Report the scanned-file count and refuse to pass on an empty tree; chdir to the repo root so a subdirectory run can't pass having scanned nothing. - Surface git's own stderr; dedupe an unmerged index; case-insensitive suffix skip, with a NUL-byte probe as the authoritative binary test (Copilot). - Context manager on the read, startswith-tuple removed in favour of regexes (Copilot). Binary/UTF-16 files stay skipped on purpose: the NUL probe is git's own rule, and a real merge of two UTF-16 files leaves our side in place with no markers. pre-commit hook moves to `language: script` — neither `python3` (absent on Windows) nor `python` (absent on Debian/Ubuntu) is portable. Wire-up gaps the gate had fallen into: dev/ was linted by neither CI nor dev.py (TARGETS had `dev.py` the file, not `dev` the directory), and `dev.py check` didn't run the gate although CLAUDE.md and CONTRIBUTING.md both promise it is "the same checks CI gates on". tests/test_conflict_marker_gate.py locks all of it, including the asymmetric rule's per-file scoping — no tracked file in the repo has a setext underline, so deleting that guard would otherwise stay green until a doc tripped it. --- .github/workflows/ci.yml | 2 +- .pre-commit-config.yaml | 8 +- CLAUDE.md | 2 +- CONTRIBUTING.md | 2 +- dev.py | 14 +- dev/check_conflict_markers.py | 173 +++++++++++++++------ tests/test_conflict_marker_gate.py | 238 +++++++++++++++++++++++++++++ 7 files changed, 384 insertions(+), 55 deletions(-) mode change 100644 => 100755 dev/check_conflict_markers.py create mode 100644 tests/test_conflict_marker_gate.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be443612..5133ae04 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ jobs: # Lint + import sorting — blocking. Rules come from pyproject.toml. # packages/ holds the agami-core library. - name: ruff check - run: uvx ruff@0.15.19 check plugins packages tests + run: uvx ruff@0.15.19 check plugins packages tests dev.py dev # Format check is informational for now: the tree has a large unformatted # backlog (~74 files). Flip `continue-on-error` off after a dedicated diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 112fcf65..1decf589 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -36,8 +36,12 @@ repos: hooks: - id: conflict-markers name: no merge-conflict markers - entry: python3 dev/check_conflict_markers.py - language: system + entry: dev/check_conflict_markers.py + # `language: script` runs the file via its shebang and lets pre-commit resolve the + # interpreter per-platform. Neither bare name is portable: `python3` is absent on + # Windows (the official installer ships `python.exe` + the `py` launcher), and + # `python` is absent on Debian/Ubuntu without `python-is-python3`. + language: script always_run: true pass_filenames: false diff --git a/CLAUDE.md b/CLAUDE.md index f72b4546..22ec5dae 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ demand by `uvx`, the same on macOS / Linux / Windows. From the repo root: ```bash uv run dev.py setup # once: wire the local pre-commit hooks -uv run dev.py check # ruff + tests + gitleaks — the same checks CI gates on +uv run dev.py check # ruff + tests + gitleaks + conflict markers — the same checks CI gates on uv run dev.py cover # did the lines I changed get tested? (patch coverage) ``` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c239ecc4..87426565 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,7 +40,7 @@ tiny cross-platform task runner (`dev.py`) wraps it all: ```bash uv run dev.py setup # once: wire the pre-commit hooks (ruff + gitleaks on commit, tests on push) -uv run dev.py check # the whole gate locally — ruff + tests + gitleaks (same as CI) +uv run dev.py check # the whole gate locally — ruff + tests + gitleaks + conflict markers (same as CI) uv run dev.py cover # did the lines I changed get tested? (patch coverage) ``` diff --git a/dev.py b/dev.py index ada5e1b2..114c27c3 100644 --- a/dev.py +++ b/dev.py @@ -10,7 +10,7 @@ Tasks: setup wire the local pre-commit hooks (ruff + gitleaks on commit; tests on push) - check the full local gate: ruff lint + tests + gitleaks (what CI runs) + check the full local gate: ruff lint + tests + gitleaks + conflict markers (what CI runs) test just the test suite lint just ruff (lint + format check) fmt apply ruff's auto-formatter to the tree @@ -30,7 +30,9 @@ # The suite imports the agami-core library, so install it editable with the [model] # extra (pydantic/pyyaml/sqlglot). DB drivers are omitted on purpose — those tests skip without a DB. TEST_DEPS = ["--with", "pytest-cov", "--with-editable", "packages/agami-core[model,server]"] -TARGETS = ["plugins", "packages", "tests", "dev.py"] +# `dev` (the directory) as well as `dev.py` (the file) — the helper scripts under dev/ were +# unlinted until the conflict-marker gate landed there. +TARGETS = ["plugins", "packages", "tests", "dev.py", "dev"] _ROOT = Path(__file__).resolve().parent # The plugin's runtime scripts import a small stdlib-only slice of the agami-core library. The @@ -73,6 +75,11 @@ def secrets() -> int: return run(["uvx", "pre-commit", "run", "gitleaks", "--all-files"]) +def conflicts() -> int: + """Fail on a merge-conflict marker that survived into a tracked file (CI's `hygiene` job).""" + return run([sys.executable, str(_ROOT / "dev" / "check_conflict_markers.py")]) + + def sync_lib() -> int: """Regenerate plugins/agami/lib/ from packages/agami-core/src (the vendored closure the scripts import).""" for rel in _VENDORED: @@ -98,10 +105,11 @@ def _lib_drift() -> int: def check() -> int: - """ruff lint + tests + gitleaks + the vendored-lib drift check — the same checks CI gates on.""" + """ruff + tests + gitleaks + conflict markers + vendored-lib drift — the same checks CI gates on.""" rc = lint() rc |= test() rc |= secrets() + rc |= conflicts() rc |= _lib_drift() print("\n✓ all checks passed" if rc == 0 else "\n✗ some checks failed") return rc diff --git a/dev/check_conflict_markers.py b/dev/check_conflict_markers.py old mode 100644 new mode 100755 index 3df6dcbb..485e47ce --- a/dev/check_conflict_markers.py +++ b/dev/check_conflict_markers.py @@ -7,71 +7,150 @@ straight past it. This runs over the tracked tree unconditionally, in CI, where it cannot be skipped with `--no-verify`. -The matching rule is deliberately asymmetric, because one of the three markers is -ambiguous: - - `<<<<<<< ` and `>>>>>>> ` never occur in legitimate content -> always an error. - `=======` is a valid Markdown setext heading underline - (`Title` on one line, `=======` under it), so flagging it - unconditionally would false-positive on ordinary prose. - It is only reported when the same file also carries one of - the unambiguous markers. - -That asymmetry is the point: a real conflict always leaves at least one unambiguous -marker, and the ambiguous one is then attributable rather than guessed at. +The matching rule is deliberately asymmetric, because two of the four markers a +conflict can leave behind are ambiguous: + + open / close (a `<` or `>` run) never occur in legitimate content -> always an error. + base-with-label (`|` run + text) likewise -> always an error. + a bare `=` run is also a Markdown setext heading underline (`Title` + on one line, the rule under it), so flagging it + unconditionally would false-positive on prose. + a bare `|` run is also a degenerate Markdown table row of empty cells. + +The two bare forms are reported only when the same file also carries an unambiguous +marker. That asymmetry is the point: a real conflict always leaves at least one +unambiguous marker, so the ambiguous ones become *attributable* rather than guessed at. + +Three states, never two: markers -> fail; clean -> pass; unreadable -> fail loudly. +A file we could not read is *unknown*, not clean, and a gate must never collapse those +two into the same answer. """ from __future__ import annotations +import os +import re import subprocess import sys -# The two markers that are never legitimate content, and the ambiguous third. -UNAMBIGUOUS = ("<<<<<<< ", ">>>>>>> ") -AMBIGUOUS = "=======" - -# Binary-ish suffixes we should not try to decode. -SKIP_SUFFIXES = (".png", ".jpg", ".jpeg", ".gif", ".ico", ".pdf", ".woff", ".woff2", ".zip", ".gz") +# Marker runs are 7 characters by default, but `.gitattributes` can widen them via +# `conflict-marker-size`, so match a run of 7-or-more. The trailing label ("HEAD", +# "origin/main") is optional: git always writes one, hand-resolved conflicts often don't. +UNAMBIGUOUS = ( + re.compile(rb"^<{7,}(?: .*)?$"), # ours + re.compile(rb"^>{7,}(?: .*)?$"), # theirs + re.compile(rb"^\|{7,} .+$"), # diff3/zdiff3 common ancestor, WITH a label +) +AMBIGUOUS = ( + re.compile(rb"^={7,}$"), # also a Markdown setext heading underline + re.compile(rb"^\|{7,}$"), # also a Markdown table row of empty cells +) + +# Suffixes we skip outright. This is a fast path, NOT the binary defence — the +# authoritative test is the NUL-byte probe in `scan()`, so an unlisted binary type is +# still handled correctly instead of being silently mis-scanned. +SKIP_SUFFIXES = ( + ".png", ".jpg", ".jpeg", ".gif", ".ico", ".bmp", ".webp", ".pdf", + ".woff", ".woff2", ".ttf", ".otf", ".eot", ".zip", ".gz", ".bz2", ".xz", + ".7z", ".jar", ".class", ".so", ".dylib", ".dll", ".exe", ".pyc", ".whl", + ".parquet", ".xlsx", ".docx", ".pptx", ".db", ".sqlite", ".mp4", ".mov", +) + +BINARY_SNIFF_BYTES = 8192 + + +def _git_bytes(*args: str) -> bytes: + """Run a git command, surfacing git's own message rather than a bare exit code.""" + proc = subprocess.run(["git", *args], capture_output=True) + if proc.returncode != 0: + detail = proc.stderr.decode("utf-8", "replace").strip() or f"exit {proc.returncode}" + sys.exit(f"✗ git {' '.join(args)} failed: {detail}") + return proc.stdout + + +def repo_root() -> str: + """The repo's top level, so a run from a subdirectory still scans the whole tree. + + `git ls-files` is cwd-relative: invoked from a clean subdirectory it lists only that + subtree, and the gate would print a reassuring pass having scanned almost nothing. + """ + return _git_bytes("rev-parse", "--show-toplevel").decode("utf-8", "replace").strip() def tracked_files() -> list[str]: - """Every file git tracks, so an untracked scratch file cannot fail the build.""" - out = subprocess.run( - ["git", "ls-files"], capture_output=True, text=True, check=True - ).stdout - return [p for p in out.splitlines() if p and not p.endswith(SKIP_SUFFIXES)] - - -def scan(path: str) -> list[tuple[int, str]]: - """Return (line_number, line) for each conflict marker in one file.""" + """Every file git tracks, so an untracked scratch file cannot fail the build. + + `-z` is load-bearing: without it `core.quotePath` (on by default) C-quotes any path + holding a non-ASCII byte, a control character, a quote or a backslash — turning + `café.py` into the literal `"caf\\303\\251.py"`, which is not an openable path. The + file would then be skipped silently, and a non-ASCII *directory* would hide its + entire subtree. + """ + raw = _git_bytes("ls-files", "-z") + paths = [os.fsdecode(p) for p in raw.split(b"\0") if p] + # An unmerged index lists a path once per stage; collapse while preserving order. + unique = list(dict.fromkeys(paths)) + return [p for p in unique if not p.lower().endswith(SKIP_SUFFIXES)] + + +def scan(path: str) -> tuple[list[tuple[int, str]], str | None]: + """Return (findings, error) for one file — exactly one of the two is meaningful. + + Matching runs on BYTES: the markers are pure ASCII, so decoding buys nothing and + costs correctness. A single latin-1 or UTF-16 byte anywhere in an otherwise ordinary + text file makes a whole-file strict decode raise, which would discard the scan. + """ try: - text = open(path, encoding="utf-8", errors="strict").read() - except (UnicodeDecodeError, OSError): - return [] # binary or unreadable — nothing meaningful to match - lines = text.splitlines() + with open(path, "rb") as fh: + data = fh.read() + except OSError as exc: + return [], f"{type(exc).__name__}: {exc.strerror or exc}" + if b"\0" in data[:BINARY_SNIFF_BYTES]: + return [], None # genuinely binary — no text to match + lines = [line.rstrip(b"\r") for line in data.split(b"\n")] hard = [ - (n, line) - for n, line in enumerate(lines, 1) - if any(line.startswith(m) for m in UNAMBIGUOUS) + (n, line) for n, line in enumerate(lines, 1) if any(p.match(line) for p in UNAMBIGUOUS) ] if not hard: - return [] # no unambiguous marker -> a bare `=======` is a setext heading, not a conflict - soft = [(n, line) for n, line in enumerate(lines, 1) if line.rstrip() == AMBIGUOUS] - return sorted(hard + soft) + # No unambiguous marker, so a bare `=` run here is a setext heading, not a conflict. + return [], None + soft = [(n, line) for n, line in enumerate(lines, 1) if any(p.match(line) for p in AMBIGUOUS)] + return [(n, line.decode("utf-8", "replace")) for n, line in sorted(hard + soft)], None def main() -> int: - findings = [(p, n, line) for p in tracked_files() for n, line in scan(p)] - if not findings: - print("✓ no merge-conflict markers in tracked files") + os.chdir(repo_root()) + paths = tracked_files() + if not paths: + print("✗ no tracked files to scan — refusing to report a pass", file=sys.stderr) + return 1 + + findings: list[tuple[str, int, str]] = [] + unreadable: list[tuple[str, str]] = [] + for path in paths: + hits, error = scan(path) + if error: + unreadable.append((path, error)) + findings.extend((path, n, line) for n, line in hits) + + if not findings and not unreadable: + print(f"✓ no merge-conflict markers ({len(paths)} tracked files scanned)") return 0 - print("✗ merge-conflict markers found in tracked files:\n", file=sys.stderr) - for path, n, line in findings: - print(f" {path}:{n}: {line[:80]}", file=sys.stderr) - print( - "\nResolve the conflict and remove every marker before committing.", - file=sys.stderr, - ) + + if findings: + print("✗ merge-conflict markers found in tracked files:\n", file=sys.stderr) + for path, n, line in findings: + print(f" {path}:{n}: {line[:80]}", file=sys.stderr) + print( + "\nResolve the conflict and remove every marker before committing" + " — a diff3/zdiff3 conflict leaves four, not three.", + file=sys.stderr, + ) + if unreadable: + # Unknown, not clean. Fail rather than certify a file we never actually read. + print("\n✗ tracked files could not be read, so they were not checked:\n", file=sys.stderr) + for path, error in unreadable: + print(f" {path}: {error}", file=sys.stderr) return 1 diff --git a/tests/test_conflict_marker_gate.py b/tests/test_conflict_marker_gate.py new file mode 100644 index 00000000..202c4c75 --- /dev/null +++ b/tests/test_conflict_marker_gate.py @@ -0,0 +1,238 @@ +"""Guard: the conflict-marker gate actually gates. + +`dev/check_conflict_markers.py` is a CI-blocking check whose only failure mode that +matters is the quiet one — reporting a pass over files it never read. Every case below +was a real silent bypass at some point in review, so each is a regression lock rather +than a hypothetical. + +Two conventions here are deliberate: + +* **Markers are built at runtime** (`"<" * 7`), never written as literals. A literal at + column 0 in this file would be found by the gate scanning its own tree; indentation + inside a triple-quoted string protects it only until someone reflows the file. +* **The script is driven as a subprocess with `cwd=` a throwaway git repo.** That scopes + `git ls-files` to the fixture and keeps the real tree out of it. `git add -A` alone is + enough — `git ls-files` reads the index, and committing would need a git identity that + CI runners don't have. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +REPO = Path(__file__).resolve().parent.parent +SCRIPT = REPO / "dev" / "check_conflict_markers.py" + +# Built at runtime so this file never contains a marker at column 0. `git merge` writes a +# label after the run; a hand-resolved conflict often doesn't, so both forms are tested. +OPEN = "<" * 7 +BASE = "|" * 7 +SEP = "=" * 7 +CLOSE = ">" * 7 + +CONFLICT = f"keep\n{OPEN} HEAD\nmine\n{SEP}\ntheirs\n{CLOSE} feature\n" + + +def run_gate(repo: Path) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(SCRIPT)], cwd=repo, capture_output=True, text=True + ) + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True) + return tmp_path + + +def track(repo: Path, name: str, content: str | bytes = CONFLICT) -> Path: + path = repo / name + path.parent.mkdir(parents=True, exist_ok=True) + if isinstance(content, bytes): + path.write_bytes(content) + else: + path.write_text(content, encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=repo, check=True) + return path + + +# --- the core asymmetric rule ------------------------------------------------------- + + +def test_clean_tree_passes(repo): + track(repo, "a.py", "print('hello')\n") + result = run_gate(repo) + assert result.returncode == 0, result.stderr + assert "1 tracked files scanned" in result.stdout # the denominator, so "scanned nothing" can't read as a pass + + +def test_full_conflict_block_fails_and_names_every_line(repo): + track(repo, "a.py") + result = run_gate(repo) + assert result.returncode == 1 + for lineno in (2, 4, 6): # open, separator, close + assert f"a.py:{lineno}" in result.stderr, result.stderr + + +@pytest.mark.parametrize("marker", [OPEN, CLOSE, f"{BASE} ancestor"]) +def test_each_unambiguous_marker_alone_fails(repo, marker): + """A partial resolution leaves one marker behind; each must fail on its own.""" + track(repo, "a.md", f"text\n{marker}\nmore\n") + assert run_gate(repo).returncode == 1 + + +def test_bare_separator_alone_is_a_setext_heading_not_a_conflict(repo): + """The false-positive this gate's asymmetry exists to avoid.""" + track(repo, "doc.md", f"A Heading\n{SEP}\n\nprose\n") + result = run_gate(repo) + assert result.returncode == 0, result.stderr + + +def test_bare_separator_is_scoped_per_file_not_per_tree(repo): + """Load-bearing: a conflict in one file must not indict a setext heading in another.""" + track(repo, "conflicted.py", CONFLICT) + track(repo, "innocent.md", f"A Heading\n{SEP}\n\nprose\n") + result = run_gate(repo) + assert result.returncode == 1 + assert "conflicted.py" in result.stderr + assert "innocent.md" not in result.stderr, result.stderr + + +def test_indented_marker_does_not_fire(repo): + """Markers anchor at column 0 — this is what lets docs discuss them when indented.""" + track(repo, "a.md", f"text\n {OPEN} HEAD\n {CLOSE} other\n") + assert run_gate(repo).returncode == 0 + + +# --- the silent-bypass regressions -------------------------------------------------- + + +@pytest.mark.parametrize( + "name", + [ + "café.py", # non-ASCII filename: core.quotePath C-quotes it out of existence + "docs/日本語/readme.md", # non-ASCII *directory* hid its whole subtree + "back\\slash.py", + "q'uote.py", + ], +) +def test_unusual_paths_are_still_scanned(repo, name): + """Regression: `git ls-files` without `-z` returns an unopenable C-quoted string.""" + if name != os.fsdecode(os.fsencode(name)): + pytest.skip("filesystem cannot represent this name") + track(repo, name) + result = run_gate(repo) + assert result.returncode == 1, f"{name} was silently skipped:\n{result.stdout}" + + +def test_non_utf8_text_file_is_still_scanned(repo): + """Regression: one latin-1 byte used to discard the whole file's scan.""" + track(repo, "legacy.py", CONFLICT.encode() + b"# author: Jos\xe9\n") + result = run_gate(repo) + assert result.returncode == 1, result.stdout + assert "legacy.py" in result.stderr + + +def test_utf16_file_is_treated_as_binary_like_git_does(repo): + """UTF-16 is skipped, and that is correct rather than a gap. + + The NUL-byte probe in `scan()` is deliberately the same rule git itself uses. Verified + against a real `git merge` of two UTF-16 files: git declines the text merge and leaves + our side in place, writing NO markers at all. So a UTF-16 file cannot carry a + git-authored conflict, and skipping it costs nothing. Contrast `legacy.py` above — + latin-1 has no NUL bytes, so git *does* text-merge it and *does* write markers. + """ + track(repo, "win.md", CONFLICT.encode("utf-16")) + assert run_gate(repo).returncode == 0 + + +def test_unreadable_file_fails_rather_than_reporting_clean(repo): + """A file we could not read is unknown, not clean — the two must not share an answer.""" + path = track(repo, "locked.py") + path.chmod(0o000) + try: + result = run_gate(repo) + assert result.returncode == 1 + assert "could not be read" in result.stderr, result.stderr + assert "locked.py" in result.stderr + finally: + path.chmod(0o644) # else tmp_path cleanup fails + + +def test_widened_conflict_marker_size_is_matched(repo): + """`.gitattributes` can set conflict-marker-size; git then writes longer runs.""" + wide = f"keep\n{'<' * 20} HEAD\nmine\n{'=' * 20}\ntheirs\n{'>' * 20} other\n" + track(repo, "a.txt", wide) + assert run_gate(repo).returncode == 1 + + +def test_binary_file_is_skipped_not_crashed(repo): + track(repo, "blob.bin", b"\x00\x01\x02" + CONFLICT.encode()) + assert run_gate(repo).returncode == 0 + + +def test_untracked_file_cannot_fail_the_build(repo): + """Scratch files must not break the hook, or developers start using --no-verify.""" + track(repo, "tracked.py", "ok\n") + (repo / "scratch.py").write_text(CONFLICT) # deliberately not git-added + assert run_gate(repo).returncode == 0 + + +def test_run_from_subdirectory_still_scans_whole_tree(repo): + """`git ls-files` is cwd-relative; a subdir run used to pass having scanned nothing.""" + track(repo, "top.py", CONFLICT) + sub = repo / "sub" + sub.mkdir(exist_ok=True) + track(repo, "sub/clean.py", "ok\n") + result = run_gate(sub) + assert result.returncode == 1, result.stdout + assert "top.py" in result.stderr + + +def test_empty_tree_refuses_to_report_a_pass(repo): + result = run_gate(repo) + assert result.returncode == 1 + assert "no tracked files" in result.stderr + + +def test_diff3_residue_after_removing_only_what_was_reported(repo): + """The message says remove *every* marker; the base marker must be one of them. + + Reported lines used to omit `|||||||`, so a developer who deleted exactly what was + printed shipped the base version's content under a gate that then went green. + """ + track(repo, "a.md", f"keep\n{BASE} ancestor\nBASE CONTENT\n{SEP}\ntheirs\n") + result = run_gate(repo) + assert result.returncode == 1 + assert "a.md:2" in result.stderr, result.stderr + + +# --- the gate is actually wired up -------------------------------------------------- + + +def test_ci_job_invokes_the_script(): + wf = yaml.safe_load((REPO / ".github" / "workflows" / "ci.yml").read_text()) + steps = wf["jobs"]["hygiene"]["steps"] + assert any("dev/check_conflict_markers.py" in s.get("run", "") for s in steps), steps + + +def test_precommit_hook_invokes_the_script(): + cfg = yaml.safe_load((REPO / ".pre-commit-config.yaml").read_text()) + hooks = [h for r in cfg["repos"] for h in r["hooks"] if h["id"] == "conflict-markers"] + assert hooks, "the conflict-markers hook is gone" + assert "dev/check_conflict_markers.py" in hooks[0]["entry"] + + +def test_gate_passes_on_this_repo(): + """Self-scan. Also catches the tempting `startswith` -> `in` 'fix', which would make + the script fail on its own docstring — a failure you cannot resolve by fixing code.""" + result = subprocess.run( + [sys.executable, str(SCRIPT)], cwd=REPO, capture_output=True, text=True + ) + assert result.returncode == 0, result.stdout + result.stderr