--resolve must not report a merge it could not check

[rbuergi]

Treats 13 Copilot review findings left unanswered on five satellite PRs merged on 2026-09-18 (Reinsurance#223/#225, Crm#130, Manufacturing#103, SocialMedia#208). Every one of the 13 now has a reply on its thread; this PR carries the half that belongs in core's canonical.

Why here. The findings on Reinsurance#225 and SocialMedia#208 are against scripts/gen-manifests.py, which is a vendored copy of this repo's .github/scripts/gen-manifests.py. Measured md5 of origin/main: core 6cc07aa32f23d5770b24aeaf967e183b, and SocialMedia, Manufacturing, Reinsurance and Crm all identical. So each real finding is one defect replicated across five repositories, not a defect in one PR. Core lands first; the four re-copies follow.

Three ways --resolve reported a merge it had not checked

1. A verification step incapable of failing. The closing "are any paths still unmerged?" read (git(...) or "").strip(). git() returns None when it cannot run, so a git that could not answer became an empty result and the run printed ✓ … the merge can be committed. _unmerged_paths now returns list[str] | None and both ends of the function read it. The identical swallow was two lines away in _loose_worktree_paths (the safety refusal itself) and in _dirty_locks (the #1023 staging fix); both now propagate None and are refused by the caller, because replacing one swallow while leaving its neighbours is failing the review twice.

2. A file with the right name is not a file we generate. Locks were classified by suffix (p.endswith("/manifest.lock")). generate() writes exactly <package>/manifest.lock for the packages the config declares, so scripts/manifest.lock and a nested Mod/Nested/manifest.lock were classified as generated — regeneration left them untouched (neither is a package), git add staged their conflict markers, and the closing check then found nothing unmerged and reported success. Both the refusal and _dirty_locks now classify against plugin_dirs(root).

3. The refusal and the hashing enumerated different file sets. _loose_worktree_paths asked git with --exclude-standard, which deliberately hides ignored untracked files. hash_files() asks nobody — it walks the filesystem, and only manifest.lock/.DS_Store are exempt. So an ignored Module/output.trx was hashed into the lock --resolve staged and was invisible to the check meant to prevent exactly that; CI then reds on a lock describing a file that is in no commit. Rather than restate the hashing rules a second time (which is how they drifted apart), the enumeration is split out of the hashing — _package_paths, _project_paths, _module_source_dirs, hashed_paths — and the check reads the same enumerators. Ignored files outside that set (bin/, obj/, node_modules/ — the great majority) still do not block, or the refusal would make --resolve unusable.

The split moves no version

hash_files was refactored, so the load-bearing claim is that it still hashes the same bytes. Old canonical and new, loaded side by side against the four satellites' real main trees:

MeshWeaver.Reinsurance   17 package(s), enumeration identical=True, moduleVersion mismatches=0
MeshWeaver.Crm            1 package(s), enumeration identical=True, moduleVersion mismatches=0
MeshWeaver.SocialMedia    5 package(s), enumeration identical=True, moduleVersion mismatches=0
MeshWeaver.Manufacturing  1 package(s), enumeration identical=True, moduleVersion mismatches=0
PARITY OK

Each fix proved able to fail

Three new self-test cases (2c, 2d, 2e), and each was re-run with that fix and only that fix deliberately reverted in a scratch copy:

revert	self-test	the assertion that caught it
suffix classifier restored	exit 1	an unmerged scripts/manifest.lock is NOT a generated lock … must be refused with exit 1, got 0 · scripts/manifest.lock stopped being unmerged — --resolve staged a conflict the generator does not own, markers and all (and the same pair for Mod/Nested/manifest.lock)
ignored files dropped from the loose set	exit 1	an IGNORED untracked file that hash_files() hashes must be refused, got 0
closing verification swallows None	exit 1	--resolve reported the merge committable while git could not say whether any path was still unmerged

Unreverted: exit 0. Each fixture also asserts it reproduces its own hazard before asserting the fix (the non-package lock really carries <<<<<<<; Mod/output.trx really is invisible to git status and really is in hashed_paths), so none of the three can pass vacuously. Case 2d additionally carries a negative control — an ignored scripts/scratch.trx, which generate() never hashes, must NOT block — so the fix cannot degenerate into "refuse every ignored file". node-repo-validate.yml runs --self-test for every satellite, so a regression reds in the repo where it happens.

Two findings are declined, with the measurement

Answered on their threads, not silently:

• Pre-staged edits are invisible to the check (4050708444). Measured 2026-09-19: git merge refuses to start against a dirty index at all — both a staged edit and a staged new file give "Your local changes to the following files would be overwritten by merge", exit 2, no merge. So a pre-staged edit cannot be present when --resolve runs; the remaining route is a deliberate git add of something unrelated during the conflict, which is the documented design (self-test case 2b asserts a staged file must not block) and not implementable as proposed — during a merge every merged path is staged, so "verify staged paths came from the merge" has nothing to compare against. The measurement is now recorded on case 2b.
• spec_from_file_location may return None (4050590430, Manufacturing#103). Measured: for a path ending .py it never returns None — not for a missing file, not for an unreadable one (both return a ModuleSpec carrying a SourceFileLoader); it returns None only for an unrecognised suffix, and this path is always project-closure.py. The existing path.is_file() guard already runs first. A broken or unreadable file raises from exec_module with a traceback naming the file and line, which is more informative than the SystemExit the finding asks for.

Filed rather than folded in

#4774 — the dot-directory enumerator divergence (4050708495 / 4050491291). Real: plugin_dirs skips .-directories, validate-repos.py does not, and check-skip-sets.py compares only the two declared skip SETS, so the divergence is invisible to the guard that exists to catch exactly this. Not fixable here: validate-repos.py and check-skip-sets.py are satellite-local with no canonical, and their enumerators differ per repo (Crm and Plugins carry four root.iterdir() sites; Education enumerates through is_course_dir).

Verification

gen-manifests.py --self-test                                      EXIT=0
dotnet build -c Release -warnaserror (Documentation.Test, one .csproj)
                                                  0 Warning(s)  0 Error(s)
dotnet test --filter WhatsNew|DocumentationLinkIntegrity      2/2 passed

The What's New entry's first draft failed WhatsNewEntryIntegrityTest on a : inside a plain YAML scalar — recorded because it is the guard demonstrating it can fail on this diff, not merely that it passes.

🤖 Generated with Claude Code

---

Also in this PR: what a core-only review gate costs the fleet

Doc/Architecture/ReviewFindingsAnswered said nothing about the fact that this check exists here and nowhere else. Its "What it does not see" list now names the other six repositories, with the measurement (2026-09-19):

• no satellite — and not MeshWeaver.Plugins either, against the usual assumption — requires Automatic review answered or carries review-answered.yml. All of them carry a Copilot review for default branch ruleset, so the review is requested everywhere and required to be answered only here;
• over the last 20 merged PRs of each of six repos: 120 merged, 72 carried findings, 55 merged with at least one unanswered — and in all 55 the ratio is N of N, never a partial. That is about twice the rate this repository measured before its own gate landed (32 of 60);
• the rollout order is recorded because it is load-bearing: five of the six use classic protection, where an absent required context blocks every PR in the repo forever (Plugins#1453), so the lane lands observe-only first and the context is added only after it is seen publishing.

Proposal in #4776.

[Copilot]

Copilot review overview

🟢 Approval recommended

The reviewed changes address the reported merge-resolution hazards with regression coverage.

Review effort: Lite
Findings: None

What changed in this PR

Hardens gen-manifests.py --resolve to fail safely when merge state cannot be verified.

Changes:

• Aligns conflict checks with generated and hashed paths.
• Propagates Git verification failures.
• Adds regression self-tests and documentation.

File	Summary
src/​MeshWeaver.Documentation/​Data/​WhatsNew/​2026-09-19-a-merge-helper-that-reported-a-merge-nobody-had-checked.md	Documents the user-visible fix.
.github/​scripts/​gen-manifests.py	Strengthens conflict resolution and adds regression coverage.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Test Results (shard 0)

265 tests   265 ✅  2m 45s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 15d5c38.

♻️ This comment has been updated with latest results.

[rbuergi]

Two further findings from treating this set, both filed rather than folded in:

• Every repo but core merges past unanswered review findings — 55 of 120 sampled merges, and Plugins is not covered either #4776 — every repository but core merges past unanswered review findings. Measured over the last 20 merged PRs of six repos: 55 of 120 merges carried at least one unanswered finding, and in all 55 the ratio is N of N — not one finding answered on any of them. MeshWeaver.Plugins is in the exposed set too; it carries neither the workflow nor the required context. This set of 13 was the normal case, not an incident.
• Education and Plugins still carry all three --resolve defects, and no guard compares a vendored gen-manifests.py to the canonical #4777 — MeshWeaver.Education (1c35825e…, 963 lines) and MeshWeaver.Plugins (a0b4fa03…, 1215 lines) carry all three of these defects at different line numbers, and neither can be re-copied: Education has no gen-manifests.config.json (the canonical refuses rather than guessing, correctly), and Plugins loads project-closure.py eagerly at module scope. A canonical adoption in Plugins would be version-neutral — measured, 0 of 61 moduleVersions move — so it is a code reconciliation, not a release. Nothing compares a vendored gen-manifests.py to the canonical; check-resolver-copy.py guards only resolve-platform.py, and its own header cites gen-manifests.py as the precedent for why that guard exists.

[github-actions]

Test Results (shard 3)

431 tests   431 ✅  55s ⏱️
  3 suites    0 💤
  3 files      0 ❌

Results for commit 15d5c38.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 1)

1 551 tests   1 551 ✅  3m 9s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit 15d5c38.

[github-actions]

Test Results (shard 2)

710 tests   518 ✅  5m 35s ⏱️
  3 suites  192 💤
  3 files      0 ❌

Results for commit 15d5c38.

[github-actions]

Test Results (shard 4)

    3 files      3 suites   5m 35s ⏱️
1 857 tests 1 857 ✅ 0 💤 0 ❌
1 858 runs  1 858 ✅ 0 💤 0 ❌

Results for commit 15d5c38.

[github-actions]

Test Results (shard 5)

    5 files      5 suites   11m 47s ⏱️
3 661 tests 3 659 ✅ 2 💤 0 ❌
3 665 runs  3 663 ✅ 2 💤 0 ❌

Results for commit 15d5c38.

[github-actions]

Test Results

   17 files     17 suites   29m 48s ⏱️
8 475 tests 8 281 ✅ 194 💤 0 ❌
8 480 runs  8 286 ✅ 194 💤 0 ❌

Results for commit 15d5c38.