Skip to content

Stop re-reporting the same conflicts as needing manual resolution - #47

Merged
TheValiantOne merged 2 commits into
mainfrom
feature/stale-build-preflight-and-inventory-hygiene
Aug 26, 2026
Merged

Stop re-reporting the same conflicts as needing manual resolution#47
TheValiantOne merged 2 commits into
mainfrom
feature/stale-build-preflight-and-inventory-hygiene

Conversation

@TheValiantOne

Copy link
Copy Markdown
Owner

What prompted this

A live 350-mod, game-build-4.04 install reported three conflicts as "needs manual resolution" on every run, with no way to make it stop:

File Blocking mod Enabled? Merged output
game\vehicles\horse\states\exploration.ws modFearlessRoach no (Enabled=0) none at all
game\gui\menus\mapMenu.ws modFastTravelFromAnywhere yes 4 days stale
game\r4Game.ws modAlwaysFullExp yes 4 days stale

The engine was right to decline all three — each has a source mod shipping a whole-file copy of a vanilla script taken from an older game build, so merging that side would delete vanilla code the game and the other contributing mod still call. That's the ValidateWholeFileMergeOutput invariant from #42 doing its job. The problem was that nothing said which mod to fix, and two separate gaps guaranteed the same files came back next run.

Changes

Tools/StaleBuildDetector.cs — runs the same vanilla-declaration comparison as a pre-flight, straight off the files with no merge involved, so the report names the stale mod and the remedy instead of pointing at a 2500-line file to hand-merge. Uses ScriptUnitExtractor's own ScopedName, so a pre-flight warning and a post-hoc violation name the same thing.

It is a diagnostic, never a gate — nothing here changes what does or doesn't merge, and the message deliberately stops short of predicting failure, because the check runs before any merge and can't know: on the live install one mod is missing 1 of 224 declarations and its conflict auto-solves every time, while another missing 13 reliably trips the invariant.

Inventory/MergeInventoryHygiene.cs + HasResolvedConflict — a record whose merged output file is gone was self-certifying: every hash it verifies belongs to a source mod, all present and unchanged when it's the output that was deleted. So it answered alreadyResolved: true forever, nothing ever re-merged it, and the game silently loaded exactly one of two conflicting mods. The GUI already refused to trust such a record (MainForm.RefreshMergeTreeConfirmPruneMissingMergeFile, plus two siblings for a missing/disabled source mod) — all three rules were GUI-only. They now live in Core so the CLI, MCP tools and Vortex extension see them too.

They report rather than act, deliberately: the GUI's three prompts all pass no defaultResult, so HeadlessMergeNotifier would answer its generic YesNo → No to every one and prune nothing, silently — the same defect shape as the ConfirmOutputOverwrite finding already filed in docs/bugs/function-level-merge-gap-handling.md.

ModFileIndex disabled-mod skip — a scan is a filesystem glob, so a deployed-but-disabled mod counted as a full conflict participant and could keep a conflict permanently unmergeable. Only an explicit Enabled=0 excludes a mod; one absent from mods.settings counts as enabled (the game appends unknown folders on next launch), and a missing/unreadable mods.settings disables nobody — so this can never make a scan miss conflicts on a fresh install or a Linux host. MergeDisabledMods opts out, named for the opt-out so its absence from an older App.config yields the new behavior rather than silently keeping the old one. Skipped folders are always reported, never silent.

HeadlessMergeNotifier.RouteAllOutputToStandardError — found while testing the above. Only Error/Warning/Exclamation routed to stderr, so any default-icon message a scan emits went to stdout, which in mcp mode carries the JSON-RPC frame stream. Caught by a real scan_conflicts round-trip returning a payload the client couldn't parse, with the new disabled-mod notice spliced between two protocol frames. Pre-existing, not introduced here — BuildAsync's "Can't find any mods in the Mods directory." has the identical shape — so it's fixed at the root, set by both hosts' mcp verb. CLI mode is unaffected.

Verification

Run against the real install (dry-run/MCP only; the deployed tool and live inventory were never touched — a scratch copy was used):

  • Before: 44 conflicts, 3 unmergeable, 1 inventory record with no output file.
  • After: 43 conflicts (phantom gone), 43/43 resolved, 0 skipped.
  • With WSM_MergeDisabledMods=true the phantom returns and the stale-build warning fires, naming modFearlessRoach and 13 of the 224 declarations it lacks — both features confirmed on one run.
  • A static sweep over all 44 conflicts (136 mod-copy comparisons) found the stale-build signature in exactly the 3 the engine went on to decline.
  • Separately, the two enabled stale mods' intended changes were ported onto current vanilla on that install (2 lines and 15 lines respectively — the rest of each mod file was just an out-of-date vanilla copy). Both then merged with zero warnings and zero function-level decisions, with no vanilla declaration lost and brace balance identical to vanilla. That's install remediation, not part of this PR.

dotnet build clean, 239 tests pass, dotnet format whitespace --verify-no-changes clean.

Test coverage

23 new tests. Both new test files stay inside the AppState.Settings-safety constraint in WitcherScriptMerger.Tests/CLAUDE.md — the pure two-string core of the detector is covered fully; the hygiene rules are covered only on the branches that return before resolving a real path (bundle exemption, null/empty guards, message formatting), since the flat-file ones reach Paths.ModsDirectoryAppState.SettingsEnvironment.Exit(1) in a test host. ExcludeDisabledModPaths was split out as a pure function for the same reason, matching the existing BuildIgnoredModNames precedent, and is covered fully — including that a null predicate or null path list excludes nothing, so an unreadable mods.settings can't make a scan silently report zero conflicts.

AI-assisted development

This PR was produced with Claude Code (Opus 5), per CONTRIBUTING.md. The diagnosis, the four fixes, the tests and the docs were all agent-written; every claim above was verified by actually running it against the real install and the real test suite, not inferred.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N6y39x9F3SKcS4hhpw85Yi

Chris Knight and others added 2 commits August 25, 2026 23:09
A live 350-mod run reported three conflicts as "needs manual resolution" on
every single pass, with no way to make it stop. The merge engine was right to
decline all three - each has a source mod shipping a whole-file copy of a
vanilla script taken from an older game build, so merging that side would
delete vanilla code the game and the other mod still call. But nothing said
which mod to fix, and two separate gaps guaranteed the same files came back.

Four changes, all found together:

- Tools/StaleBuildDetector.cs runs the vanilla-declaration comparison as a
  pre-flight, off the files, before any merge, so a warning names the stale mod
  and the remedy (update or disable it) instead of pointing at a 2500-line file
  to hand-merge. It's a diagnostic, never a gate - its message reports drift and
  its usual consequence rather than predicting this conflict will fail, because
  it can't know that: a real install had one mod missing 1 of 224 declarations
  whose conflict auto-solved every time, and another missing 13 that didn't.

- MergeInventory.HasResolvedConflict now requires the merged output to exist.
  Every hash it checked belonged to a source mod - all present and unchanged
  when it's the output that was deleted - so a record whose merged file was gone
  answered "resolved" forever and nothing re-merged it, leaving the game to load
  exactly one of two conflicting mods with nothing saying so. The GUI already
  refused to trust such a record; Inventory/MergeInventoryHygiene.cs lifts that
  rule and its two siblings out so the CLI, MCP and Vortex extension see them
  too. They report rather than act: the GUI's prompts pass no defaultResult, so
  HeadlessMergeNotifier would answer its generic YesNo => No and prune nothing.

- ModFileIndex now skips mod folders mods.settings marks Enabled=0. A scan is a
  filesystem glob, so a deployed-but-disabled mod counted as a full conflict
  participant and could keep a conflict permanently unmergeable - which is
  exactly what one of the three was. Only an explicit Enabled=0 excludes a mod;
  an absent or unreadable mods.settings disables nobody. MergeDisabledMods opts
  out, named for the opt-out so an older App.config yields the new behavior.

- HeadlessMergeNotifier.RouteAllOutputToStandardError, set by both hosts' mcp
  verb. Only Error/Warning/Exclamation went to stderr, so any default-icon
  message a scan emits landed on stdout - mid-JSON-RPC-stream in MCP mode.
  Caught by a real scan_conflicts round-trip coming back unparseable, with the
  new disabled-mod notice spliced between two protocol frames. Pre-existing:
  BuildAsync's "Can't find any mods" had the same shape.

Verified against the live install: 44 conflicts with 3 unmergeable and one
inventory record with no output file, before; 43 conflicts and 43/43 resolved
with zero skips, after. Re-running with WSM_MergeDisabledMods=true brings the
phantom back and prints the stale-build warning naming the mod and 13 of the
224 declarations it lacks. 239 tests pass; format check clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6y39x9F3SKcS4hhpw85Yi
…od skip

Core's CLAUDE.md gains three sections ("Disabled mods are excluded from the
conflict scan", "Stale-build pre-flight", "Inventory hygiene"), the folder map
and MCP tool return shapes are updated, and the IMergeNotifier section now
covers why MCP mode has to force notifier output to stderr.

docs/bugs/stale-mod-builds-and-phantom-merge-records.md records the episode in
the same shape as function-level-merge-gap-handling.md: what recurred, the three
real files and their blocking mods, why each gap kept it recurring, and the
before/after numbers from the live install.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6y39x9F3SKcS4hhpw85Yi
@TheValiantOne
TheValiantOne merged commit d0d3b8e into main Aug 26, 2026
2 checks passed
@TheValiantOne
TheValiantOne deleted the feature/stale-build-preflight-and-inventory-hygiene branch August 26, 2026 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant