chore(ci): fail the build on a surviving merge-conflict marker

[sandeep-agami]

Why

A merge-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 in the repo currently catches one — not ruff, not gitleaks, not the test suite.

Why not pre-commit-hooks' check-merge-conflict

That hook only inspects files while a merge is in progress. A marker that is already committed — on a branch you later merge, or a resolution you fixed up in a follow-up commit — walks straight past it. This scans the tracked tree unconditionally.

The matching rule is deliberately asymmetric

One of the three markers is ambiguous:

marker	legitimate elsewhere?	rule
<<<<<<<	never	always an error
>>>>>>>	never	always an error
=======	yes — Markdown setext heading underline	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 rather than guessed at — and ordinary prose with a setext heading doesn't fail the build.

Verified both directions:

# a real conflict (the shape that motivated this — `<<<<<<<` already removed)
entry A
=======
entry A duplicate
>>>>>>> origin/some-branch

  -> ✗ docs/_probe_conflict.md:2: =======
     ✗ docs/_probe_conflict.md:4: >>>>>>> origin/some-branch     exit 1

# a legitimate setext heading
A Heading
=======

  -> ✓ no merge-conflict markers in tracked files                exit 0

What lands

• dev/check_conflict_markers.py — tracked-tree scan, skips binary suffixes, decodes defensively
• .pre-commit-config.yaml — local hook for fast feedback (bypassable, per the file's own framing)
• .github/workflows/ci.yml — its own hygiene job, so a failure reads as what it is rather than arriving under the secret-scan heading

Test plan

• python3 dev.py check green — ruff, full suite, gitleaks, vendored-lib drift all clean
• uvx pre-commit run conflict-markers --all-files → Passed
• Both YAML files parse
• Positive and negative probes shown above

[Copilot]

Pull request overview

Adds an explicit repo-wide hygiene check that fails CI (and optionally pre-commit) if merge-conflict markers survive into tracked files, using an asymmetric rule to avoid false-positives on Markdown setext headings.

Changes:

• Introduces dev/check_conflict_markers.py to scan the tracked tree for conflict markers with an “unambiguous required” rule for =======.
• Adds a local pre-commit hook (conflict-markers) for fast developer feedback.
• Adds a dedicated hygiene job to CI so failures are surfaced separately from secret scanning.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
dev/check_conflict_markers.py	New tracked-tree scanner that detects merge-conflict markers with an asymmetric matching rule.
.pre-commit-config.yaml	Adds a local hook to run the conflict-marker scan.
.github/workflows/ci.yml	Adds a dedicated CI job to run the conflict-marker scan as an unskippable gate.

Comments suppressed due to low confidence (2)

dev/check_conflict_markers.py:50

• scan() opens files without a context manager and uses errors="strict" then silently skips on UnicodeDecodeError. That can leak file descriptors in non-CPython runtimes and can also produce false negatives for text files that contain a few non-UTF-8 bytes (the conflict markers are ASCII and still searchable).

    try:
        text = open(path, encoding="utf-8", errors="strict").read()
    except (UnicodeDecodeError, OSError):
        return []  # binary or unreadable — nothing meaningful to match

dev/check_conflict_markers.py:56

• This can be simplified (and sped up slightly) by using str.startswith() with a tuple directly, instead of any(...) over the tuple.

        (n, line)
        for n, line in enumerate(lines, 1)
        if any(line.startswith(m) for m in UNAMBIGUOUS)
    ]

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sandeep-agami]

Fixed. Suffix matching is now case-insensitive (p.lower().endswith(...)), and the list grew to cover .svg-adjacent, archive, and compiled types.

The more important half of this: the suffix list was never actually the binary defence — the broad except UnicodeDecodeError was, which is why a text file with one latin-1 byte scanned clean. The authoritative test is now a NUL-byte probe over the first 8 KiB, so an unlisted binary type is handled correctly rather than silently mis-scanned, and the suffix list is a fast path only.

[sandeep-agami]

Good catch on the portability, but python would trade a Windows break for a Linux one — Debian/Ubuntu ship only python3 unless python-is-python3 is installed. (README.md:149 also states Windows is not yet validated end-to-end.)

Switched to language: script with a shebang instead, so pre-commit resolves the interpreter per-platform and neither bare name is hardcoded. Verified: uvx pre-commit run conflict-markers --all-files → Passed.

[sandeep-agami]

Added — tests/test_conflict_marker_gate.py, 24 cases, all green.

It covers the asymmetric rule you name, including the case that turned out to matter most: the bare separator is scoped per file, so a conflict in a.py must not indict a setext heading in b.md. Worth noting no tracked file in this repo currently contains a line of only =, so deleting that guard would have kept CI green until someone added a doc with a setext heading, at which point the gate fails the whole repo on innocent prose.

Note the bootstrap problem: a fixture with markers at column 0 would be caught by this very gate. The tests build markers at runtime ("<" * 7) and drive the script as a subprocess against a throwaway git repo, so nothing is written that the gate would later scan.

[sandeep-agami]

Review: conflict-marker gate

The design call is right, and the reasoning in the docstring is genuinely good — check-merge-conflict really does only inspect during an in-progress merge, and scanning the tracked tree unconditionally is the correct fix. The asymmetric ======= rule is sound, correctly scoped per file, and documented at the right altitude. The happy path works (0.09s over 307 files) and the gate fails closed when git is missing or the cwd isn't a repo.

But the gate has three silent-pass paths that share one root cause, and I reproduced all of them. Decisive repro — a repo whose only tracked content is three files, each holding a live <<<<<<< HEAD / ======= / >>>>>>> feature block:

Tracked files still containing LIVE conflict blocks:
  café.py
  docs/日本語/readme.md
  legacy_latin1.py

=== GATE VERDICT ===
✓ no merge-conflict markers in tracked files
exit=0

Root cause: scan() returns [] for both "I read this file and it was clean" and "I could not read this file at all." For a gate, those must not share a return value.

---

Critical

C1. git ls-files C-quotes unusual paths; the quoted string is unopenable, and the error is swallowed. dev/check_conflict_markers.py:40, compounded by :49

core.quotePath defaults to true, so any path with a non-ASCII byte, control char, " or \ comes back as a literal like "caf\303\251.py". open() raises FileNotFoundError, except OSError reports it clean. Isolated to that one config, same file:

--- ls-files (default) ---            "caf\303\251.py"
--- GATE (default) ---                ✓ no merge-conflict markers   exit=0
--- GATE, core.quotePath=false ---    ✗ café.py:2: <<<<<<< HEAD     exit=1

A non-ASCII directory hides its whole subtree (docs/日本語/readme.md above has a perfectly plain filename). Fix: git ls-files -z and split on NUL, which also disables quoting.

C2. One non-UTF-8 byte anywhere in a text file hides every marker in it. dev/check_conflict_markers.py:48-50

errors="strict" decodes all-or-nothing over the whole file, so a single stray byte discards the entire scan. legacy_latin1.py above is a .py file git itself classifies as text — full conflict block plus # author: Jos\xe9 — reported clean. Same for a UTF-16 .md (PowerShell/VS default encoding).

The markers are pure ASCII, so there is no reason to decode at all. Read bytes and match on bytes.

C3. An unreadable file is indistinguishable from a clean one. dev/check_conflict_markers.py:49-50

The comment # binary or unreadable — nothing meaningful to match states the defect as if it were the design. Also reported clean while holding conflict blocks: chmod 000 files, files tracked but absent from the worktree (sparse checkout / partial clone), and submodule gitlinks (IsADirectoryError, so a submodule's contents are never scanned). Three-state it: markers → fail, clean → pass, unreadable → fail with the path and errno.

Fixing the one root cause closes C1, C2, C3 and most of M3 below.

---

Important

I1. The hygiene job is not merge-blocking. ci.yml:52-61

The comment at :57-59 says this is "the copy that cannot be skipped with --no-verify". True of the hook, but merge-blocking is a repo setting, and right now no CI job is a required check:

• legacy branch protection on main: required_checks: ["cla-assistant"]
• rulesets "Main Protect" / "Copilot review": deletion, non_fast_forward, copilot_code_review, pull_request (0 approvals) — no required_status_checks rule at all

So hygiene will go red and the merge button stays green. Pre-existing (lint-and-test and gitleaks are in the same boat) and not caused by this PR, but it nullifies this PR's central claim. ci.yml:5 already asserts "Branch protection is what makes these checks required to merge" — worth making true in the same change.

I2. diff3/zdiff3 leaves a 4th marker, and the error message walks the developer into shipping it. dev/check_conflict_markers.py:30

merge.conflictStyle=zdiff3 is widely recommended since git 2.35. A real git merge under it emits:

<<<<<<< HEAD
MINE
||||||| 563e31c      <-- never matched, never reported
BASE
=======
OTHER
>>>>>>> other

The gate reports three lines and says "remove every marker". Remove exactly what it printed and you are left with ||||||| + ======= + the base content — which re-scans clean, because no unambiguous marker survives so the ======= is read as a setext heading. Broken content ships, certified by the gate that just pointed at it. This is the same partial-resolution shape the PR was written to catch. Add "||||||| " to UNAMBIGUOUS and treat a bare ||||||| like =======.

I3. No tests, and the script lands in a double blind spot. CLAUDE.md:56 — "New behavior comes with tests."

• Not linted: dev.py:33 is TARGETS = ["plugins", "packages", "tests", "dev.py"] — that's dev.py the file, not dev/ the directory. Same for ci.yml:31.
• Not coverable: --cov=plugins --cov=packages/agami-core/src, so dev.py cover has nothing to measure and passes silently.

There is direct precedent for exactly this artifact class: tests/test_release_pypi_workflow.py exists because the workflow "isn't ruff/pytest-covered". Note also that no tracked file in the repo currently contains a line of only = — so if the if not hard guard at :57 were deleted, CI would stay green and the regression would surface later as a repo-wide failure on innocent prose.

The bootstrap problem is real (a fixture with markers at column 0 fails this gate) but avoidable with no script change: git init a tmp_path, git add -A (no commit needed — CI runners have no git identity), then run the script as a subprocess with cwd=tmp_path. Build markers at runtime ("<" * 7 + " ") so a future ruff format can't shift a literal to column 0 and detonate CI.

I4. dev.py check no longer matches CI, and two docs now overstate it. dev.py:100-107

check() runs lint + test + secrets + _lib_drift, not the new gate. Its docstring says "the same checks CI gates on"; CLAUDE.md:21 and CONTRIBUTING.md:43 make the same promise. Adding a self-scan test (I3) would fix this for free.

---

Suggestions

• S1. No escape hatch. A CONTRIBUTING.md teaching contributors how to resolve a conflict hard-fails the gate (verified). For a public repo that doc is likely to get written. There is no allowlist, path skip, or pragma.
• S2. Print the denominator. :66 — an empty repo, or running from a subdirectory (git ls-files is cwd-relative), both print ✓ having scanned nothing. Since C1–C3 all manifest as "fewer files scanned", the missing count is what makes them invisible in a CI log. cd $(git rev-parse --show-toplevel) would close the subdirectory case.
• S3. conflict-marker-size. .gitattributes can widen the markers; verified that a real git merge with conflict-marker-size=20 scans clean. ^<{7,} / ^={7,}$ / ^>{7,} regexes would cover it.
• S4. SKIP_SUFFIXES is decorative. :34 misses .svg, .parquet, .xlsx, .so, .jar, .ttf, and is case-sensitive (.PNG falls through). Those are handled by the C2 swallow instead, which is why you can't tighten the exception before making this list authoritative. Detect binary by content (NUL byte in the first 8 KiB) and delete the swallow.
• S5. Surface git's own error. :40 — capture_output=True means a failure shows returned non-zero exit status 128 and hides fatal: not a git repository.
• S6. Duplicate findings mid-merge. With an unmerged index git ls-files prints a path once per stage, so markers report 3×. --deduplicate fixes it.
• S7. open(...).read() at :48 without a context manager. Moot if you switch to bytes.

---

Checked and fine

• Per-file attribution of the ambiguous marker works correctly: a conflict in a.py plus a bare ======= in b.md reports only a.py.
• Fails closed on missing git and on non-repo cwd.
• CRLF / lone-CR handled by splitlines() + rstrip().
• A marker with no trailing space cannot come from git — even git merge-file -L '' -L '' -L '' emits the trailing space. The requirement at :30 is correct and deliberate; worth a comment so nobody "fixes" it.
• actions/checkout SHA-pinned, consistent with the rest of the workflow.
• Making hygiene its own job rather than a step on gitleaks is the right diagnosability call.

---

Recommendation: hold for C1–C3 (one root-cause fix: bytes not decode, -z not quoted paths, explicit unreadable state that fails) and I2. I1 is a one-line repo setting without which none of this gates anything.

Findings verified by probe against throwaway git repos; nothing in the repo or its worktrees was modified.

[sandeep-agami]

Addressed in b1a85ba

All Copilot comments (inline + both suppressed low-confidence ones) and the critical findings above. 1581 passed, 1 skipped; ruff clean; pre-commit hook passes.

The headline fix. The gate was reporting a pass over files it never read. Before, with three tracked files each holding a live conflict block:

café.py
docs/日本語/readme.md
legacy_latin1.py

✓ no merge-conflict markers in tracked files
exit=0

After, same tree (plus a chmod 000 file and a diff3 residue, and a legitimate setext heading that must stay silent):

✗ merge-conflict markers found in tracked files:
  café.py:2: <<<<<<< HEAD
  ...
  diff3.md:2: ||||||| base
  docs/日本語/r.md:2: <<<<<<< HEAD
  legacy_latin1.py:2: <<<<<<< HEAD

✗ tracked files could not be read, so they were not checked:
  noperm.py: PermissionError: Permission denied
exit=1

Root cause was single: 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-quoted any non-ASCII/control/quote path into a string that isn't openable, so it was skipped silently, and a non-ASCII directory hid its whole subtree.
• Match on bytes rather than a strict UTF-8 decode. One latin-1 byte anywhere discarded the whole file's scan. Verified against a real git merge that 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.
• diff3/zdiff3's fourth marker. The message said "remove every marker" while printing only three of four; deleting exactly what was printed left ||||||| + ======= + the base content, which re-scanned clean. Same partial-resolution shape the PR exists to catch.
• Runs of 7+, so .gitattributes conflict-marker-size is covered; label now optional.
• Report the denominator (✓ no merge-conflict markers (307 tracked files scanned)), refuse to pass an empty tree, and chdir to the repo root so a subdirectory run can't pass having scanned nothing.

One correction to my earlier review: I listed UTF-16 as a bypass. It isn't. The NUL-byte probe is git's own binary rule, and a real merge of two UTF-16 files leaves our side in place with no markers written at all — so that state isn't reachable. Test documents it as intended behaviour rather than a gap.

Wire-up gaps the gate had itself fallen into:

• dev/ was linted by neither CI nor dev.py — TARGETS had dev.py the file, not dev the directory. Added to both; ruff was already clean there.
• dev.py check didn't run the gate, though CLAUDE.md and CONTRIBUTING.md both promise it is "the same checks CI gates on". Added conflicts() and corrected both docs.

tests/test_conflict_marker_gate.py — 24 cases. Every silent bypass above is a regression lock, plus the asymmetric rule's per-file scoping, the workflow/hook wiring, and a self-scan. Markers are built at runtime ("<" * 7) and the script is driven as a subprocess against a throwaway git repo, which sidesteps the bootstrap problem where a fixture would be caught by the gate itself.

---

Two things deliberately not done

1. The hygiene job still isn't merge-blocking. main has no required_status_checks at all — legacy protection requires only cla-assistant, and the rulesets cover deletion / non-fast-forward / Copilot review / PR-required. So this job goes red and the merge button stays green, as do lint-and-test and gitleaks. Pre-existing and not caused by this PR, but until it's set, ci.yml:5's "Branch protection is what makes these checks required to merge" isn't true. That's a repo setting and an admin call, so I left it to you.

2. No escape hatch. A CONTRIBUTING.md that teaches contributors how to resolve a conflict fails this gate (verified). Indented markers don't fire, so a 4-space indented code block is the workaround, but a fenced block at column 0 will trip it. Adding an allowlist weakens the gate, so that's a design call rather than a defect — flagging it now because it's the kind of doc a public repo tends to grow.