From 730ae93e3b88f72d46314140b1cb0b720ac35331 Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Fri, 21 Aug 2026 23:23:01 -0400 Subject: [PATCH] Give the unresolved-conflicts notification a "Resolve Now" button The post-deploy scan's notification shipped `actions: []` and a message that named a count and nothing else. On a real install it fired ("WitcherScriptMerger: 5 unresolved script conflicts found") seconds before the user launched a game that would not start, with an empty mod0000_MergedFiles - the extension had detected the problem, said so, and led nowhere, because acting on it meant knowing to go find a different button on a different page. The notification now carries a "Resolve Now" action running the same workflow as the Mods-page toolbar action, and says the game may fail to start until the conflicts are merged. The action dismisses the notification before starting, so a count that is about to be stale can't sit there through the merge and then be re-shown by the post-merge scan. The resolver is injected as an optional third argument rather than imported: resolveAction imports coexistenceGuard, which imports conflictNotifications, so importing it back would close a cycle. index.ts - the composition root, which already imports resolveAction - supplies it, wrapping the call in the same log-only last-resort catch registerResolveScriptConflictsAction uses, since resolveScriptConflicts reports its own failures to the user. Omitting the argument keeps the old passive shape, so nothing else that calls this function changes. 6 new tests (223 total): the action's presence and title, that invoking it runs the resolver and dismisses first (asserted by call order), that a missing dismiss callback doesn't throw, the consequence wording, and - in index.test.ts - that the injected callback reaches resolveScriptConflicts with the api and that a rejected workflow doesn't escape. index.test.ts's './resolveAction' mock is partial (importOriginal spread) so registerResolveScriptConflictsAction stays real for the existing registration test. typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FP8H6rBLCGPBFRSVsF3Kgw --- vortex-extension/README.md | 6 ++ .../src/conflictNotifications.test.ts | 54 +++++++++++++++ vortex-extension/src/conflictNotifications.ts | 34 ++++++++- vortex-extension/src/index.test.ts | 69 ++++++++++++++++++- vortex-extension/src/index.ts | 18 ++++- 5 files changed, 174 insertions(+), 7 deletions(-) diff --git a/vortex-extension/README.md b/vortex-extension/README.md index 197d02f..e34750e 100644 --- a/vortex-extension/README.md +++ b/vortex-extension/README.md @@ -60,6 +60,12 @@ does anything for any other game. full three-way merge and only skips the writes). The deadline is passed per call, so the `initialize` handshake keeps the short default and a WSM process that fails to start still fails fast. +- **An actionable unresolved-conflicts notification** (`src/conflictNotifications.ts`): + the post-deploy scan's warning carries a **Resolve Now** button that runs the same + workflow as the toolbar action, and says that the game may fail to start until the + conflicts are merged. The callback is injected from `index.ts` rather than imported + directly, because `resolveAction` → `coexistenceGuard` → `conflictNotifications` would + otherwise close an import cycle. - **A merge-history dashboard tile** (`src/mergeHistoryDashlet.ts`): lists every merge WSM has already recorded (via its MCP `list_merges` tool) - relative path, which merged mod folder holds the result, and each source mod's recorded hash - with a manual diff --git a/vortex-extension/src/conflictNotifications.test.ts b/vortex-extension/src/conflictNotifications.test.ts index 685b299..15f7e2f 100644 --- a/vortex-extension/src/conflictNotifications.test.ts +++ b/vortex-extension/src/conflictNotifications.test.ts @@ -78,6 +78,7 @@ describe('notifyConflictsIfChanged', () => { const notification = api.sendNotification.mock.calls[0][0]; expect(notification.id).toBe(WSM_CONFLICTS_NOTIFICATION_ID); expect(notification.allowSuppress).toBe(true); + // no onResolveNow supplied here, so no action button - see the "Resolve Now" tests below expect(notification.actions).toEqual([]); expect(typeof notification.type).toBe('string'); expect(typeof notification.message).toBe('string'); @@ -87,6 +88,59 @@ describe('notifyConflictsIfChanged', () => { expect(WSM_CONFLICTS_NOTIFICATION_ID).not.toBe('witcher3-merge'); }); + // Regression coverage for the passive version of this notification: it shipped + // `actions: []` and a message that said nothing about consequence. On a real install it + // fired ("5 unresolved script conflicts found") seconds before the user launched a game + // that would not start with an empty merged mod - the information was present and led + // nowhere. See notifyConflictsIfChanged's own doc comment. + it('adds a "Resolve Now" action when a resolver is supplied', () => { + const api = fakeApi(); + + notifyConflictsIfChanged(api as never, [conflict('a.ws')], () => undefined); + + const notification = api.sendNotification.mock.calls[0][0]; + expect(notification.actions).toHaveLength(1); + expect(notification.actions[0].title).toBe('Resolve Now'); + }); + + it('runs the resolver, and dismisses the notification first, when "Resolve Now" is invoked', () => { + const api = fakeApi(); + const order: string[] = []; + const dismiss = vi.fn(() => { order.push('dismiss'); }); + const onResolveNow = vi.fn(() => { order.push('resolve'); }); + + notifyConflictsIfChanged(api as never, [conflict('a.ws')], onResolveNow); + + const notification = api.sendNotification.mock.calls[0][0]; + notification.actions[0].action(dismiss); + + expect(onResolveNow).toHaveBeenCalledTimes(1); + // dismissed before the workflow starts, so a now-stale count can't sit there while the + // merge runs and then get re-shown by the post-merge scan + expect(order).toEqual(['dismiss', 'resolve']); + }); + + it('does not require a dismiss callback to be passed to the action', () => { + const api = fakeApi(); + const onResolveNow = vi.fn(); + + notifyConflictsIfChanged(api as never, [conflict('a.ws')], onResolveNow); + + const notification = api.sendNotification.mock.calls[0][0]; + expect(() => notification.actions[0].action()).not.toThrow(); + expect(onResolveNow).toHaveBeenCalledTimes(1); + }); + + it('warns that the game may fail to start, not just that conflicts exist', () => { + const api = fakeApi(); + + notifyConflictsIfChanged(api as never, [conflict('a.ws')], () => undefined); + + const notification = api.sendNotification.mock.calls[0][0]; + expect(notification.message).toContain('unresolved script conflict'); + expect(notification.message).toContain('may fail to start'); + }); + it('does not re-notify on a second call with the identical conflict set (same signature)', () => { const api = fakeApi(); diff --git a/vortex-extension/src/conflictNotifications.ts b/vortex-extension/src/conflictNotifications.ts index 5ca38ef..64b74bd 100644 --- a/vortex-extension/src/conflictNotifications.ts +++ b/vortex-extension/src/conflictNotifications.ts @@ -194,7 +194,26 @@ export function resetConflictNotificationState(): void { * with that same conflict set would silently skip re-attempting it for the rest of the * session (the same-signature early-return above would treat it as "already shown"). */ -export function notifyConflictsIfChanged(api: types.IExtensionApi, conflicts: ScanConflictsResult): void { +/** + * `onResolveNow`, when supplied, turns this from a passive warning into an actionable one: + * the notification gains a "Resolve Now" button running the same workflow as the Mods-page + * toolbar action. Injected rather than imported so this module stays free of a cycle - + * `resolveAction` imports `coexistenceGuard`, which imports this file - and so the callback + * is trivially stubbable in tests. `index.ts` (the composition root, which already imports + * `resolveAction`) is what supplies it. + * + * This exists because the passive version demonstrably did not work. On a real install this + * notification fired ("5 unresolved script conflicts found") seconds before the user + * launched into a game that would not start, with an empty merged mod - the information was + * right there and led nowhere, because `actions` was empty and the message said nothing + * about the consequence. A warning whose only remedy is "go find a different button" is one + * the user reads as noise. + */ +export function notifyConflictsIfChanged( + api: types.IExtensionApi, + conflicts: ScanConflictsResult, + onResolveNow?: () => void, +): void { if (isModOrDependencyInstallActive(api)) { log('debug', 'witcherscriptmerger-vortex: mod/dependency install activity in progress - skipping conflict notification check'); return; @@ -214,9 +233,18 @@ export function notifyConflictsIfChanged(api: types.IExtensionApi, conflicts: Sc api.sendNotification?.({ id: WSM_CONFLICTS_NOTIFICATION_ID, type: 'warning', - message: `WitcherScriptMerger: ${unresolved.length} unresolved script conflict${unresolved.length === 1 ? '' : 's'} found`, + message: `WitcherScriptMerger: ${unresolved.length} unresolved script conflict${unresolved.length === 1 ? '' : 's'} found` + + ' - the game may fail to start until they are merged', allowSuppress: true, - actions: [], + actions: onResolveNow === undefined ? [] : [ + { + title: 'Resolve Now', + action: (dismiss?: () => void) => { + dismiss?.(); + onResolveNow(); + }, + }, + ], }); } } catch (err) { diff --git a/vortex-extension/src/index.test.ts b/vortex-extension/src/index.test.ts index e5e3548..7d0de7b 100644 --- a/vortex-extension/src/index.test.ts +++ b/vortex-extension/src/index.test.ts @@ -13,6 +13,7 @@ const { notifyConflictsIfChangedMock, isModOrDependencyInstallActiveMock, refreshCoexistenceStateMock, + resolveScriptConflictsMock, } = vi.hoisted(() => ({ ensureWsmToolRegisteredMock: vi.fn(), registerWsmStatusDashletMock: vi.fn(), @@ -21,6 +22,7 @@ const { notifyConflictsIfChangedMock: vi.fn(), isModOrDependencyInstallActiveMock: vi.fn(), refreshCoexistenceStateMock: vi.fn(), + resolveScriptConflictsMock: vi.fn(), })); vi.mock('./toolAcquisition', () => ({ @@ -34,6 +36,15 @@ vi.mock('./statusTile', () => ({ registerWsmStatusDashlet: registerWsmStatusDashletMock, })); +// Partial mock, deliberately: index.ts now also imports resolveScriptConflicts, to hand +// notifyConflictsIfChanged the callback behind the notification's "Resolve Now" button. +// Only that one export is stubbed - registerResolveScriptConflictsAction stays real, +// because a test below asserts it reaches context.registerAction with the right shape. +vi.mock('./resolveAction', async (importOriginal) => ({ + ...(await importOriginal()), + resolveScriptConflicts: resolveScriptConflictsMock, +})); + vi.mock('./conflictScan', () => ({ isWsmToolAcquired: isWsmToolAcquiredMock, scanWsmConflicts: scanWsmConflictsMock, @@ -357,7 +368,7 @@ describe('main (index.ts)', () => { await fireAsyncEvent('did-deploy', 'profile1', undefined); expect(scanWsmConflictsMock).toHaveBeenCalledTimes(1); - expect(notifyConflictsIfChangedMock).toHaveBeenCalledWith(context.api, conflicts); + expect(notifyConflictsIfChangedMock).toHaveBeenCalledWith(context.api, conflicts, expect.any(Function)); }); it('skips scanning (without throwing) when no WSM tool has been acquired yet', async () => { @@ -396,7 +407,61 @@ describe('main (index.ts)', () => { await fireAsyncEvent('did-deploy', 'profile1', undefined); expect(scanWsmConflictsMock).toHaveBeenCalledTimes(1); - expect(notifyConflictsIfChangedMock).toHaveBeenCalledWith(context.api, conflicts); + expect(notifyConflictsIfChangedMock).toHaveBeenCalledWith(context.api, conflicts, expect.any(Function)); + }); + + // The whole point of the third argument: the notification's "Resolve Now" button has + // to actually run the merge workflow, not just exist. Regression coverage for the + // passive version, which shipped `actions: []` and left the user to go find a + // different button - on a real install it fired seconds before a game that wouldn't + // start, and led nowhere. + it('hands notifyConflictsIfChanged a callback that runs the resolve-conflicts workflow', async () => { + ensureWsmToolRegisteredMock.mockClear().mockResolvedValue(false); + isWsmToolAcquiredMock.mockClear().mockResolvedValue(true); + isModOrDependencyInstallActiveMock.mockClear().mockReturnValue(false); + refreshCoexistenceStateMock.mockClear().mockResolvedValue(undefined); + scanWsmConflictsMock.mockClear().mockResolvedValue([{ relativePath: 'a.ws' }]); + notifyConflictsIfChangedMock.mockClear(); + resolveScriptConflictsMock.mockReset().mockResolvedValue(undefined); + const { context, fireOnce, fireAsyncEvent } = fakeContext(WITCHER3_GAME_ID, { + profile1: { gameId: WITCHER3_GAME_ID }, + }); + + main(context); + fireOnce(); + await fireAsyncEvent('did-deploy', 'profile1', undefined); + + const onResolveNow = notifyConflictsIfChangedMock.mock.calls[0][2] as () => void; + expect(resolveScriptConflictsMock).not.toHaveBeenCalled(); + + onResolveNow(); + + expect(resolveScriptConflictsMock).toHaveBeenCalledWith(context.api); + }); + + // The callback must not let a rejected workflow escape as an unhandled rejection - + // resolveScriptConflicts reports its own failures to the user, so this is a log-only + // last-resort net, matching registerResolveScriptConflictsAction's own. + it('swallows a rejection from the Resolve Now callback rather than letting it escape', async () => { + ensureWsmToolRegisteredMock.mockClear().mockResolvedValue(false); + isWsmToolAcquiredMock.mockClear().mockResolvedValue(true); + isModOrDependencyInstallActiveMock.mockClear().mockReturnValue(false); + refreshCoexistenceStateMock.mockClear().mockResolvedValue(undefined); + scanWsmConflictsMock.mockClear().mockResolvedValue([{ relativePath: 'a.ws' }]); + notifyConflictsIfChangedMock.mockClear(); + resolveScriptConflictsMock.mockReset().mockRejectedValue(new Error('boom')); + const { context, fireOnce, fireAsyncEvent } = fakeContext(WITCHER3_GAME_ID, { + profile1: { gameId: WITCHER3_GAME_ID }, + }); + + main(context); + fireOnce(); + await fireAsyncEvent('did-deploy', 'profile1', undefined); + + const onResolveNow = notifyConflictsIfChangedMock.mock.calls[0][2] as () => void; + + expect(() => onResolveNow()).not.toThrow(); + await Promise.resolve(); }); // Fix for a real wasted-work case: notifyConflictsIfChanged would discard this diff --git a/vortex-extension/src/index.ts b/vortex-extension/src/index.ts index 9f728a4..9829a3a 100644 --- a/vortex-extension/src/index.ts +++ b/vortex-extension/src/index.ts @@ -4,7 +4,7 @@ import { isWsmToolAcquired, scanWsmConflicts } from './conflictScan'; import { isModOrDependencyInstallActive, notifyConflictsIfChanged } from './conflictNotifications'; import { isWitcher3Active, WITCHER3_GAME_ID } from './gating'; import { registerMergeHistoryDashlet } from './mergeHistoryDashlet'; -import { registerResolveScriptConflictsAction } from './resolveAction'; +import { registerResolveScriptConflictsAction, resolveScriptConflicts } from './resolveAction'; import { registerWsmStatusDashlet } from './statusTile'; import { ensureWsmToolRegistered } from './toolAcquisition'; @@ -226,7 +226,21 @@ function main(context: types.IExtensionContext): boolean { } const conflicts = await scanWsmConflicts(context.api); - notifyConflictsIfChanged(context.api, conflicts); + // The third argument is what makes the notification actionable - see + // notifyConflictsIfChanged's own doc comment. Supplied here, at the composition + // root, because this module already imports resolveAction; conflictNotifications + // importing it directly would close a cycle (resolveAction -> coexistenceGuard -> + // conflictNotifications). + notifyConflictsIfChanged(context.api, conflicts, () => { + resolveScriptConflicts(context.api).catch((err: unknown) => { + // resolveScriptConflicts reports its own failures to the user; this is the + // same last-resort, log-only net registerResolveScriptConflictsAction uses + // around its own invocation. + log('warn', 'witcherscriptmerger-vortex: Resolve Now action failed unexpectedly', { + error: err instanceof Error ? err.message : String(err), + }); + }); + }); } catch (err) { log('warn', 'witcherscriptmerger-vortex: post-deploy conflict scan failed', { error: err instanceof Error ? err.message : String(err),