Skip to content

Nothing detects the merge-conflict markers dotnet format writes into multi-targeted sources #347

Description

@phmatray

Problem / motivation

dotnet format in apply mode can write a literal merge-conflict block into a .cs file:

<<<<<<< TODO: Unmerged change from project 'FormCraft(net10.0)', Before:
            return false;
=======
        {
            return false;
>>>>>>> After

The result does not compile — CS8300: Merge conflict marker encountered, followed by a cascade of
CS1519/CS1026/CS8124 as the parser unwinds.

This is measured, not hypothetical. It happened twice while landing #307: first on
FormCraft/Forms/Rendering/FieldRendererBase.cs, then on two FormCraft.ForFluentUI files in a later
pass. Both times it was caught only because someone happened to run a grep.

Cause. All three shipping projects are <TargetFrameworks>net8.0;net10.0</TargetFrameworks>, so
dotnet format computes each fix once per target framework. When the two results differ it emits the
collision as a conflict block rather than failing. There is no --framework option to avoid it;
scoping the run with --include <file> did avoid it in practice.

Why this is newly urgent. #301/#307 added the Format CI gate and told contributors, in
CLAUDE.md, to run dotnet format FormCraft.sln to fix a failure. The repo now actively directs
people to run the command that can corrupt their tree. Verify mode never writes, so CI itself is
safe
— the exposure is entirely on the apply path, which is to say on the contributor.

Today the only thing between that and a committed, non-compiling file is a human remembering
grep -rl '<<<<<<< TODO' --include='*.cs' .. That instruction is now written into CLAUDE.md and
.claude/skills/repo-profile.md, which is worth something — but documentation is the weakest possible
guard against something a tool does silently, and #307 is the proof: the instruction existed by the
second occurrence and the corruption still landed in the working tree.

Proposed solution

A test in FormCraft.UnitTests/Ci/ that fails when any tracked .cs file contains a conflict marker
at the start of a line. That directory already exists for exactly this class of repo-level invariant —
GitignoreTests, TestReportingTests, WorkflowSourceTests, VersionTagRuleTests — and
WorkflowSource.RepoRoot already provides the repo-root walk, so this needs no new infrastructure.

It fails in dotnet test, which is where a contributor already looks and which both ci.yml and
continuous.yml run.

Alternatives considered

  • A check inside the Nuke Format target. Closest to the point of damage. Rejected as the primary:
    Format is a verify target that runs dotnet format --verify-no-changes, and giving it a second,
    unrelated job muddies what a Format failure means. It also only protects people who run that
    target, whereas the corruption is committed by people running the apply command.
  • A git pre-commit hook. The only option that blocks the commit itself. Rejected: hooks are not
    installed by a clone, so it protects only contributors who opted in — and the ones who need it are
    the ones who did not.
  • Nothing; keep the documented grep. Rejected on the evidence above — the documentation existed
    and the corruption still happened.

Area

Build, CI and repo hygiene — FormCraft.UnitTests/Ci/, with WorkflowSource.RepoRoot reused for the
repo-root walk.

Related: #301, #276


🧠 Brainstorm

Problem & context

#301 made .editorconfig enforceable and #307 landed it, together with a documented instruction to run
dotnet format FormCraft.sln when the gate fails. The formatter turns out to be unsafe on this
repository's multi-targeted projects, in a way that produces a file that does not compile and that no
automated check notices.

The failure has the shape this repo keeps filing issues about — #276 ("the .html test report is
promised but unenforced"), and #301 itself ("the .editorconfig severities are read by nothing"). A
rule exists, it is written down, and nothing executes it.

Approaches

A. A Ci/ unit test scanning tracked sources. Fits the established pattern, needs no new
infrastructure, and fails where contributors already look. Costs one more repo-wide file walk in the
test suite (milliseconds). Recommended.

B. A guard inside the Nuke Format target. Closest to the damage and trivially cheap, but
conflates "the tree is unformatted" with "the tree is corrupt", and only fires for people who run the
verify target — not the apply command that causes it.

C. A pre-commit hook. The only thing that can actually block the commit, and the only thing that
requires opt-in installation. Worth revisiting if the repo ever adopts a hook manager; not worth
introducing one for this.

Recommendation

Approach A. The goal is that a corrupted file cannot reach dev unnoticed, and a test in the suite
CI already runs achieves that without redefining what the Format target means.

Three details that decide whether the test works at all, all of which will bite on the first attempt:

  • The guard must not find itself. A test searching for <<<<<<< contains <<<<<<<. Build the
    needle at runtime (new string('<', 7)) rather than excluding the file — an exclusion would create
    exactly one .cs file in the repo where a real marker is invisible.
  • It must not descend into .claude/worktrees/. That directory holds other agents' full checkouts,
    which can legitimately be mid-merge with real conflict markers in them. A naive walk from the repo
    root would fail this test for reasons entirely outside the repository. bin/ and obj/ are excluded
    for the ordinary reason (generated sources, e.g. the Razor source generator's output).
  • Match <<<<<<< and >>>>>>>, not =======. A line of exactly seven = is a plausible comment
    divider; the angle-bracket markers are not plausible as anything else. Anchor at line start.

Scoping to *.cs also sidesteps the prose problem: README.md, CLAUDE.md and
.claude/skills/repo-profile.md all now discuss these markers deliberately (that is how this hazard
is documented), and none of them is a .cs file.

📋 Spec

Goal

A tracked .cs file containing a merge-conflict marker fails dotnet test, and therefore fails CI,
rather than reaching review as a file that does not compile.

Scope

In: one test class in FormCraft.UnitTests/Ci/, reusing WorkflowSource.RepoRoot.

Non-goals: changing the Format target or ci.yml; changing .editorconfig; installing a git
hook; detecting the corruption before it is written (only dotnet format itself could do that);
scanning non-.cs files.

Where the gap is

flowchart LR
    A["contributor runs<br/>dotnet format (apply)"] --> B{"multi-TFM<br/>collision?"}
    B -->|no| C["clean fix"]
    B -->|yes| D["conflict block<br/>written into .cs"]
    D --> E["does not compile"]
    E -.->|"caught only by a<br/>remembered grep ❌"| F["commit"]
    D --> G["NEW: Ci guard test"]
    G -->|fails dotnet test| H["caught in CI ✅"]

    style D fill:#ffd7d7,stroke:#b02b2b
    style G fill:#cfe6ff,stroke:#2b6cb0
Loading

Behaviour rules

  1. Scan tracked .cs sources under the repo root, skipping any directory segment named bin,
    obj, or .claude. The .claude exclusion is load-bearing, not tidiness: .claude/worktrees/
    holds other agents' checkouts which may legitimately be mid-merge.
  2. Detect <<<<<<< and >>>>>>> at the start of a line. Not ======= — seven = is a plausible
    comment divider. Both markers are checked so a half-resolved file is caught from either end.
  3. The needle is constructed at runtime so this file does not match itself, and so no file needs to
    be excluded from the scan.
  4. The failure message names every offending file and line, and says what to do: re-run the
    formatter scoped with --include <file>, then hand-resolve. A contributor hitting this has just had
    a tool corrupt their code and does not yet know that is what happened.
  5. A negative control is required. A guard that scanned nothing would pass forever; the detection
    path is exercised against synthetic content, the same way CollectionItemShapeGuardTests proves its
    own guard can still fail.

Assumptions

  • The repo-root walk in WorkflowSource.RepoRoot is the right locator; it is what every other Ci/
    test uses.
  • Enumerating the filesystem is preferred over shelling out to git ls-files. It matches the pure-IO
    style of the existing Ci/ tests and avoids a process dependency; the three excluded directory names
    cover what git ls-files would have excluded here.

Related decision — explicitly NOT in scope

Revisit EnforceCodeStyleInBuild=true once the Format gate has held for a few releases. #301
rejected it as the entry point: with TreatWarningsAsErrors=true it is a hard build break until the
very last violation is fixed, and it adds an analyzer pass to every incremental build. Now that the
tree is clean the first objection is much weaker, and adopting it would cost one property and no diff.

This is recorded here because this is the issue someone reading about format enforcement will find. It
is a separate decision with a time-based trigger and deliberately has no task in the plan below —
do not bundle it in.

🛠️ Implementation plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: a tracked .cs file carrying a merge-conflict marker fails dotnet test.

Architecture: one repo-invariant test in FormCraft.UnitTests/Ci/, alongside the existing
GitignoreTests / TestReportingTests / WorkflowSourceTests, reusing WorkflowSource.RepoRoot.

Tech Stack: .NET 8 / .NET 10, xUnit v3 + Shouldly, Microsoft.Testing.Platform, NUKE.

Global Constraints

  • TreatWarningsAsErrors=true — do not relax it.
  • Do not exclude the guard's own file from the scan. Build the marker string at runtime instead;
    an exclusion creates one .cs file where a real marker is invisible.
  • Do not walk into .claude/ — it holds other agents' worktrees, which may be mid-merge. Also
    skip bin/obj (generated sources).
  • dotnet format --verify-no-changes must stay clean — the new file has to satisfy the The .editorconfig's style rules are enforced nowhere, so 574 violations have accumulated #301 gate.
  • Never hand-edit CHANGELOG.md.
  • Commit identity: git -c user.email=phmatray@gmail.com -c user.name="Philippe Matray".
  • Conventional Commits; scope ci or test.

Task 1: The guard, red against synthetic content first

Files: create FormCraft.UnitTests/Ci/ConflictMarkerTests.cs.

Interfaces: an internal static FindMarkers(IEnumerable<string> files) (or equivalent) returning
(file, line, text) offences, so the detection path is testable without writing a corrupt file to disk.

  • Step 1: Write the failing test for the detection path: feed synthetic lines containing a runtime-built <<<<<<< and >>>>>>> and assert both are reported with their line numbers; assert a line of seven = and a line mentioning the marker mid-sentence are not reported.
  • Step 2: Run dotnet test FormCraft.UnitTests/FormCraft.UnitTests.csproj -c Release -- --filter-class FormCraft.UnitTests.Ci.ConflictMarkerTests → FAIL (type does not exist).
  • Step 3: Implement the detector: match <<<<<<< / >>>>>>> at line start, with the needle built at runtime (new string('<', 7)) so this file never matches itself.
  • Step 4: Re-run the filter → PASS.
  • Step 5: Commit: test(ci): detect merge-conflict markers in C# sources.

Task 2: Point it at the repository

Files: modify FormCraft.UnitTests/Ci/ConflictMarkerTests.cs.

Interfaces: a [Fact] scanning every .cs under WorkflowSource.RepoRoot, skipping directory
segments named bin, obj, .claude.

  • Step 1: Add the repo-wide [Fact], with a failure message naming each offending file:line and telling the reader to re-run dotnet format scoped with --include <file> and hand-resolve.
  • Step 2: Add a test asserting the enumeration excludes bin, obj and .claude — the exclusions are load-bearing (.claude/worktrees/ holds other checkouts that may be mid-merge), so a regression there would make this test fail for reasons outside the repo.
  • Step 3: Add a test asserting the enumeration actually finds something (a non-zero file count), so a broken walk cannot pass vacuously — the failure mode CollectionItemShapeGuardTests guards against for its own detector.
  • Step 4: Run the filter → PASS on the current clean tree.
  • Step 5: Verify the guard can still fail end-to-end: temporarily write a marker into a scratch .cs file under a scanned directory, confirm the repo-wide test fails and names it, then delete it. Record the observed message in the commit body.
  • Step 6: Run dotnet test -c Release → PASS (full suite) and ./build.sh Format → PASS.
  • Step 7: Commit: test(ci): fail the build when a tracked C# file carries a conflict marker.

Task 3: Point the documentation at the guard

Files: modify CLAUDE.md, .claude/skills/repo-profile.md, README.md.

Interfaces: none.

  • Step 1: Update CLAUDE.md's code-style section: the documented grep -rl '<<<<<<< TODO' is now a diagnostic aid, not the guard — dotnet test is the guard. Keep the --include <file> workaround, which is still the fix.
  • Step 2: Update .claude/skills/repo-profile.md's Format/lint entry the same way, so the profile the lifecycle skills read is not the stale copy.
  • Step 3: Add a ## 🎉 Unreleased bullet to README.md (union hot-spot — keep it additive).
  • Step 4: Run dotnet test -c Release → PASS — FormCraft.UnitTests/Ci/ClaudeMdTestCommandsTests asserts on CLAUDE.md's contents, so a doc edit can genuinely fail here.
  • Step 5: Commit: docs: point the conflict-marker guidance at the test that enforces it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:mediumFix when possibletype:choreMaintenance, dependencies, tooling

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions