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),