Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions vortex-extension/src/conflictNotifications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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();

Expand Down
34 changes: 31 additions & 3 deletions vortex-extension/src/conflictNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
69 changes: 67 additions & 2 deletions vortex-extension/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const {
notifyConflictsIfChangedMock,
isModOrDependencyInstallActiveMock,
refreshCoexistenceStateMock,
resolveScriptConflictsMock,
} = vi.hoisted(() => ({
ensureWsmToolRegisteredMock: vi.fn(),
registerWsmStatusDashletMock: vi.fn(),
Expand All @@ -21,6 +22,7 @@ const {
notifyConflictsIfChangedMock: vi.fn(),
isModOrDependencyInstallActiveMock: vi.fn(),
refreshCoexistenceStateMock: vi.fn(),
resolveScriptConflictsMock: vi.fn(),
}));

vi.mock('./toolAcquisition', () => ({
Expand All @@ -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<typeof import('./resolveAction')>()),
resolveScriptConflicts: resolveScriptConflictsMock,
}));

vi.mock('./conflictScan', () => ({
isWsmToolAcquired: isWsmToolAcquiredMock,
scanWsmConflicts: scanWsmConflictsMock,
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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
Expand Down
18 changes: 16 additions & 2 deletions vortex-extension/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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),
Expand Down
Loading