Add "Resolve Script Conflicts" action + merge panel to the Vortex extension - #21
Merged
Merged
Conversation
…xtension.
Fix the stale MergeConflictsResult interface in mcpClient.ts (missing
functionLevelDecisions, the function-level merge fallback's audit trail) and add
a new registerAction entry that previews a merge via mergeConflicts({dryRun:
true}), shows merged/skipped counts and the function-level decisions prominently
in a Markdown dialog, and runs the real merge on confirmation via a second,
freshly-spawned WsmMcpClient.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
Both units add their own registration to index.ts's main(): Unit G's did-deploy conflict-scan handler (registered inside context.once, an event listener) and Unit H's "Resolve Script Conflicts" action (registered directly in main(), per Vortex's own documented contract that register calls must not be deferred through context.once). Combined both, updating the shared header comment to describe all three registrations accurately. index.test.ts needed the union of both branches' test additions: Unit G's "did-deploy conflict scanning" describe block plus Unit H's action- registration test, and fakeContext's combined signature (registerAction mock, profileId-keyed profiles, onAsync support). Verified after resolution: typecheck, build, lint, 110 unit tests, and 118 total tests including integration all pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
Unit I's merge-history dashlet merged to main since this branch's last resolution. Both it and Unit H's "Resolve Script Conflicts" action register directly in main() (per the same register-outside-context.once contract both units independently discovered) - combined the imports, header comment (now describing dashlet as the third registration and the resolve action as the fourth), and both context.register* calls. index.test.ts's fakeContext needed both registerAction and registerDashlet mock support together. Verified after resolution: typecheck, build, lint, 120 unit tests, and 129 total tests including integration all pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is Unit H of the Vortex-extension work: the user-facing "Resolve Script Conflicts" action and its preview/confirm panel, driving WSM's MCP server via
WsmMcpClient(Unit E) and the tool acquired by Unit F.src/mcpClient.ts:MergeConflictsResultwas stale versus the realmerge_conflictsMCP tool (WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs), which now also returnsfunctionLevelDecisions(verified directly against that file andFileMerger.HeadlessMergeSummary.FunctionLevelDecisions, aList<string>). AddedfunctionLevelDecisions: string[]to the interface — no other parsing changes needed, sincecallToolalready does a generic JSON parse into the typed interface.src/resolveAction.tsandsrc/mergePanel.ts, wired intosrc/index.tswith one additiveregisterResolveScriptConflictsAction(context)call.context.registerAction('mod-icons', ...)— the Mods page toolbar group, confirmed against real, current Vortex source (gh search code 'registerAction("mod-icons"' --repo Nexus-Mods/Vortexturns up exactly this group for global toolbar buttons like Deploy/Purge andopen-directory's "Open Mod Staging Folder") rather than guessed.WsmMcpClient, previews viamergeConflicts({dryRun: true}), and shows a Markdown dialog (built bymergePanel.ts) with merged/skipped counts and — surfaced prominently, right after the counts, not buried — thefunctionLevelDecisionsaudit trail when non-empty.mergeConflicts({dryRun: false})) and shows the result. Two separate client instances, not one held open across the user's read-the-dialog wait — matchesmcpClient.ts's own "spawn per user-initiated workflow, tear down when done" lifecycle policy.DiffPlexMergeEngine.MergeHeadless: a real (non-dry-run)merge_conflictscall already writes a git/diff3-style conflict-marker sidecar underDiffPlexConflicts/and opens it in the OS's default editor as a side effect. The panel's messaging for skipped files just says so, rather than building a redundant launch mechanism (which would also need its own binary-acquisition step, since Unit F only downloads the GUI-less Headless build).Deliberate v1 scope choices (noted per the task description)
mergeConflictsis called with norelativePaths/orderOverrides, i.e. no per-file selection/reordering UI. Explicitly optional for v1.MergePanel.tsx). This project's TypeScript toolchain has no JSX support wired up (tsconfig.jsonsets nojsxoption, no@types/reactdependency,reactitself present only indirectly). Standing up a full React/JSX toolchain was bigger, separate scope from this unit's actual job. Instead,mergePanel.tsbuilds plainvortex-apiIDialogContent(api.showDialog), whosemdfield renders Markdown natively — sufficient for a headed, bulleted audit trail. If a later unit needs real interactive controls (e.g. per-file checkboxes), that's the point to revisit this.Code review findings fixed
An internal multi-angle code review caught three real issues in
mergePanel.ts, all fixed:functionLevelDecisionswas read unguarded; an older, already-acquired WSM binary predating this field would throw aTypeErrorescapingresolveScriptConflicts's try/catch entirely, making the action silently do nothing. Fixed with a defensive?? []default, plus an outer safety-net try/catch inresolveAction.tsfor any other unexpected failure in dialog-building/showing.mod0000\_MergedFiles\a\_b.ws) instead of the clean path. Fixed by not escaping backtick-wrapped paths (code spans already suppress Markdown interpretation); the test that had baked in the wrong expected output was corrected.pathListSectionhelper with an optional intro-text parameter.Test plan
npm run typecheck— cleannpm run build— cleannpm run lint— cleannpm test(fast unit tests) — 66 passed, including newsrc/mergePanel.test.ts(12 tests, dialog-content building) andsrc/resolveAction.test.ts(8 tests, full preview/confirm/error orchestration against fake clients)npm run test:integration— 72 passed, including two new real, no-mock tests intest/mcpClient.integration.test.tsthat build/spawn the realWitcherScriptMerger.Headless.exeagainst a scratch mods folder with genuine conflicting.ws/.xmlcontent: one pair (.ws, disjoint-line edits) that cleanly auto-solves, one pair (.xml, same-line edit, deliberately not.wsso the function-level fallback can never rescue it) that stays genuinely skipped. Confirms real dry-run/real-runmerged/skipped/functionLevelDecisionsdata is sensible, that a dry run writes nothing, and that a real run writes the merged file and aDiffPlexConflictssidecar.FileOpener.Open) — a window may briefly appear during this test. This is genuine, intentional WSM behavior being exercised end-to-end, not a test bug; theafterAllcleanup is best-effort (matches the codebase's own convention for a lock the test doesn't control) in case that program is still holding the file.FileOpener.Opencall, just not inside Vortex itself).AI-assisted development
This PR was substantially produced by Claude Code (Claude Sonnet 5), per this repo's
CONTRIBUTING.mddisclosure requirement. I reviewed the diff, ran an internal code-review pass that caught and fixed three real bugs (see above), and verified the test plan above directly.