Add Vortex coexistence & Collections handling (Unit K)

vortex-extension/src/conflictNotifications.ts

@@ -142,8 +142,16 @@ export function isModOrDependencyInstallActive(api: types.IExtensionApi): boolea
  *  session with zero conflicts matches the "nothing changed" early-return below and
  *  never calls `dismissNotification` for an id that was never sent (harmless against
  *  the real Vortex API, which no-ops on an unknown id, but pointless noise otherwise).
- *  Exported reset hook is for test isolation only - no production caller should ever
- *  need it. */
+ *
+ *  **Exported reset hook now has a real production caller, not just test isolation**
+ *  (correcting this comment's own earlier claim otherwise): `coexistenceGuard.ts`'s
+ *  `checkCoexistenceDrift` calls this when it detects WSM's merge state changed outside
+ *  this extension (Unit K) - a stale `alreadyResolved`-based signature is no longer
+ *  trustworthy once the underlying merge state it was computed against has moved, so the
+ *  next `did-deploy` needs a fair chance to re-notify rather than matching a signature
+ *  that predates the external change. Test suites still call it for isolation between
+ *  cases too (see this file's own tests' `beforeEach`), which is the *original* reason
+ *  this was exported - just no longer the only one. */
 let lastNotifiedSignature = '';
 
 export function resetConflictNotificationState(): void {

vortex-extension/src/index.ts

@@ -1,4 +1,5 @@
 import { log, selectors, types } from 'vortex-api';
+import { refreshCoexistenceState } from './coexistenceGuard';
 import { isWsmToolAcquired, scanWsmConflicts } from './conflictScan';
 import { isModOrDependencyInstallActive, notifyConflictsIfChanged } from './conflictNotifications';
 import { isWitcher3Active, WITCHER3_GAME_ID } from './gating';
@@ -85,6 +86,18 @@ import { ensureWsmToolRegistered } from './toolAcquisition';
  * This extension must never call `context.registerGame('witcher3', ...)` - Vortex's own
  * built-in `game-witcher3` extension already owns that registration; this extension is a
  * companion to it, not a replacement.
+ *
+ * Unit K (coexistence & Collections handling) adds no new `context.register*` call - see
+ * `coexistenceGuard.ts`'s own header doc comment for the full detect/warn/reconcile
+ * design and its citations. It adds two things to what's registered above: a new
+ * `context.api.events.on('profile-did-change', ...)` listener (plain `events.on`, not
+ * `onAsync` - matches `EVENTS.md`'s own "Profile has been switched" entry, which is not
+ * marked "Async." the way `did-deploy` is), registered inside `context.once` alongside
+ * `tryRegisterWsmTool`'s own `gamemode-activated` listener; and a call to
+ * `refreshCoexistenceState` from inside `checkForConflictsAfterDeploy`, deliberately
+ * placed *above* that function's own `isModOrDependencyInstallActive` early-return - see
+ * that call site's own comment for why installing a Collection is exactly the case this
+ * ordering exists to still catch.
  */
 function main(context: types.IExtensionContext): boolean {
   // Unit J: the dependency/status dashlet. Called here, synchronously and
@@ -120,6 +133,21 @@ function main(context: types.IExtensionContext): boolean {
           error: err instanceof Error ? err.message : String(err),
         });
       });
+
+    // Unit K: the third coexistence-check trigger point (alongside profile-did-change and
+    // did-deploy, both further below) - catches drift that accumulated while a different
+    // game was active, or while Vortex itself was closed, neither of which either of the
+    // other two triggers would ever see. Fixed in code review: an earlier version of this
+    // file's own header comment (and coexistenceGuard.ts's own) documented
+    // 'gamemode-activated' as a wired trigger point without this call actually existing -
+    // a real doc/code mismatch, not just stale prose. refreshCoexistenceState never throws
+    // (its own internal try/catch) - this .catch is belt-and-suspenders only, same
+    // reasoning as the .catch immediately above.
+    refreshCoexistenceState(context.api).catch((err: unknown) => {
+      log('warn', 'witcherscriptmerger-vortex: gamemode-activated coexistence check failed', {
+        error: err instanceof Error ? err.message : String(err),
+      });
+    });
   }
 
   // onAsync's own contract (@nexusmods/vortex-api's lib/api.d.ts doc comment on
@@ -172,6 +200,18 @@ function main(context: types.IExtensionContext): boolean {
         return;
       }
 
+      // Unit K (coexistence & Collections handling): deliberately called *before* the
+      // isModOrDependencyInstallActive gate just below, not after it. That gate exists to
+      // skip *conflict scanning* during a mod/dependency-install burst (see its own doc
+      // comment) - but installing a Vortex Collection that bundles script merges is
+      // exactly the window in which game-witcher3's own importScriptMerges() can silently
+      // overwrite this extension's merge output (coexistenceGuard.ts's own header comment,
+      // hazard 1). A coexistence check gated behind the same early-return would never see
+      // the one did-deploy where that overwrite actually happened. refreshCoexistenceState
+      // never throws (own try/catch) and is a purely best-effort secondary signal, so it
+      // can't affect what follows either way.
+      await refreshCoexistenceState(context.api);
+
       if (isModOrDependencyInstallActive(context.api)) {
         // Purely an optimization, not a correctness requirement - notifyConflictsIfChanged
         // (conflictNotifications.ts) checks this same condition again on whatever result
@@ -195,6 +235,42 @@ function main(context: types.IExtensionContext): boolean {
     }
   }
 
+  // Unit K (coexistence & Collections handling): the `profile-did-change` trigger point -
+  // see coexistenceGuard.ts's own header comment for why `profile-will-change` itself is
+  // deliberately not used instead (a plain, synchronous events.on emit whose listeners
+  // Vortex never awaits, so there's no reliable way to bracket the built-in extension's
+  // own file moves around it). Gated like checkForConflictsAfterDeploy gates did-deploy:
+  // resolves the *switched-to* profile's own gameId via selectors.profileById rather than
+  // isWitcher3Active(context.api) - by the time profile-did-change fires the active game
+  // already is the new profile's game, so the two would normally agree, but resolving it
+  // from the event's own profileId keeps this consistent with that existing reasoning
+  // rather than assuming it.
+  function checkCoexistenceOnProfileChange(profileId: string): void {
+    let gameId: string | undefined;
+    try {
+      gameId = selectors.profileById(context.api.getState(), profileId)?.gameId;
+    } catch (err) {
+      log('warn', 'witcherscriptmerger-vortex: profile-did-change coexistence check failed to read state', {
+        error: err instanceof Error ? err.message : String(err),
+      });
+      return;
+    }
+
+    if (gameId !== WITCHER3_GAME_ID) {
+      return;
+    }
+
+    // refreshCoexistenceState never throws (its own internal try/catch) - this .catch is
+    // belt-and-suspenders only, matching tryRegisterWsmTool's own reasoning just above:
+    // a plain events.on listener that returns a rejected promise becomes an unhandled
+    // rejection in Vortex's own process rather than a contained extension failure.
+    refreshCoexistenceState(context.api).catch((err: unknown) => {
+      log('warn', 'witcherscriptmerger-vortex: profile-did-change coexistence check failed', {
+        error: err instanceof Error ? err.message : String(err),
+      });
+    });
+  }
+
   // Registered synchronously here, not inside context.once below - see
   // mergeHistoryDashlet.ts's own registerMergeHistoryDashlet doc comment for why a
   // register call specifically must not be deferred into once, unlike
@@ -204,6 +280,7 @@ function main(context: types.IExtensionContext): boolean {
   context.once(() => {
     tryRegisterWsmTool();
     context.api.events.on('gamemode-activated', tryRegisterWsmTool);
+    context.api.events.on('profile-did-change', checkCoexistenceOnProfileChange);
     context.api.onAsync('did-deploy', checkForConflictsAfterDeploy);
   });
 

vortex-extension/src/resolveAction.test.ts

@@ -1,9 +1,29 @@
-import { describe, expect, it, vi } from 'vitest';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { WITCHER3_GAME_ID } from './gating';
 import { WSM_TOOL_ID } from './discoveredTool';
-import { MergeConflictsResult, WsmMcpClientOptions } from './mcpClient';
+import { GetStatusResult, MergeConflictsResult, WsmMcpClientOptions } from './mcpClient';
 import { resolveScriptConflicts, WsmMergeClient } from './resolveAction';
 
+// Unit K: isolates resolveAction.ts's own logic from coexistenceGuard.ts's real
+// behavior (its own snapshot/compare logic is covered directly by
+// coexistenceGuard.test.ts instead) - same rationale as every other sibling-module mock
+// in this codebase (e.g. index.test.ts's own mocks). Without this, the real
+// computeMergeStateSnapshot would call the fake client's own getStatus/listMerges below
+// and attempt a real recursive fs walk against whatever `status.modsDirectory` those
+// stubs return - harmless in practice (an ENOENT-tolerant walk against a nonsense path)
+// but untested and not this file's concern.
+const { computeMergeStateSnapshotMock, recordOwnMergeStateSnapshotMock, checkCoexistenceDriftMock } = vi.hoisted(() => ({
+  computeMergeStateSnapshotMock: vi.fn(),
+  recordOwnMergeStateSnapshotMock: vi.fn(),
+  checkCoexistenceDriftMock: vi.fn(),
+}));
+
+vi.mock('./coexistenceGuard', () => ({
+  computeMergeStateSnapshot: computeMergeStateSnapshotMock,
+  recordOwnMergeStateSnapshot: recordOwnMergeStateSnapshotMock,
+  checkCoexistenceDrift: checkCoexistenceDriftMock,
+}));
+
 function mergeResult(overrides: Partial<MergeConflictsResult> = {}): MergeConflictsResult {
   return {
     merged: [],
@@ -73,10 +93,27 @@ function fakeApi(options: {
   };
 }
 
+function fakeStatus(overrides: Partial<GetStatusResult> = {}): GetStatusResult {
+  return {
+    gameDirectory: 'C:\\Games\\Witcher3',
+    modsDirectory: 'C:\\Games\\Witcher3\\Mods',
+    dependenciesValid: true,
+    textMergeDependenciesValid: true,
+    bundleDependenciesValid: true,
+    modsDirectoryExists: true,
+    mergedModName: 'mod0000_MergedFiles',
+    conflictCount: 0,
+    ...overrides,
+  };
+}
+
 /** A fake `connect` - returns queued results/errors in call order and records every
  *  `exePath`/`env` it was called with, plus how many of the clients it produced were
  *  closed - proves `resolveScriptConflicts` closes every client it opens, even on the
- *  error path (via `finally`). */
+ *  error path (via `finally`). `getStatus`/`listMerges` are trivial stubs, never
+ *  meaningfully exercised here since `./coexistenceGuard` (the only thing that would
+ *  call them, via `computeMergeStateSnapshot`) is mocked above - they exist only to
+ *  satisfy `WsmMergeClient`'s type. */
 function fakeConnect(outcomes: Array<{ result?: MergeConflictsResult; error?: Error }>) {
   const calls: WsmMcpClientOptions[] = [];
   let closedCount = 0;
@@ -92,6 +129,8 @@ function fakeConnect(outcomes: Array<{ result?: MergeConflictsResult; error?: Er
         }
         return outcome.result!;
       },
+      getStatus: async () => fakeStatus(),
+      listMerges: async () => [],
       close: async () => {
         closedCount += 1;
       },
@@ -102,6 +141,16 @@ function fakeConnect(outcomes: Array<{ result?: MergeConflictsResult; error?: Er
 }
 
 describe('resolveScriptConflicts', () => {
+  beforeEach(() => {
+    computeMergeStateSnapshotMock.mockReset().mockResolvedValue({
+      folderListingSignature: '',
+      mergeHistorySignature: '',
+      mergedModName: 'mod0000_MergedFiles',
+    });
+    recordOwnMergeStateSnapshotMock.mockReset();
+    checkCoexistenceDriftMock.mockReset();
+  });
+
   it('shows an error notification and never connects when no WSM tool has been registered', async () => {
     const { api, notifications, showDialogCalls } = fakeApi({});
     const { connect } = fakeConnect([]);
@@ -171,6 +220,65 @@ describe('resolveScriptConflicts', () => {
     expect(showDialogCalls[1].type).toBe('success');
   });
 
+  // Unit K: reconciles coexistenceGuard.ts's own "last known merge state" against this
+  // extension's own just-completed workflow, using the same still-open client each time -
+  // see resolveAction.ts's own comment in runMergeConflictsWorkflow for why the preview
+  // and the real merge deliberately go through *different* coexistenceGuard.ts functions
+  // (checkCoexistenceDrift for the no-write preview vs. the silent
+  // recordOwnMergeStateSnapshot for the real, writing merge), not the same one for both.
+  it('checks for coexistence drift (does not silently re-baseline) after the dry-run preview, and silently re-baselines only after the real merge', async () => {
+    const { api } = fakeApi({
+      toolPath: 'C:\\wsm\\WitcherScriptMerger.Headless.exe',
+      dialogResponses: [{ action: 'Merge Now' }],
+    });
+    const preview = mergeResult({ merged: ['a.ws'], skipped: ['b.xml'] });
+    const final = mergeResult({ merged: ['a.ws'], skipped: [], dryRun: false });
+    const { connect } = fakeConnect([{ result: preview }, { result: final }]);
+    const previewSnapshot = { folderListingSignature: 'sig1', mergeHistorySignature: 'sig2', mergedModName: 'mod0000_MergedFiles' };
+    const finalSnapshot = { folderListingSignature: 'sig3', mergeHistorySignature: 'sig4', mergedModName: 'mod0000_MergedFiles' };
+    computeMergeStateSnapshotMock.mockReset().mockResolvedValueOnce(previewSnapshot).mockResolvedValueOnce(finalSnapshot);
+
+    await resolveScriptConflicts(api, { connect });
+
+    // Once per connected client (preview, then the real merge) - each call receives
+    // whichever client instance was open at that point, per computeMergeStateSnapshot's
+    // own "already-open client" contract.
+    expect(computeMergeStateSnapshotMock).toHaveBeenCalledTimes(2);
+
+    // Preview (dryRun: true, no write performed) - compared against the existing
+    // baseline, never silently adopted as the new one. A regression here (an earlier
+    // version of this code unconditionally called recordOwnMergeStateSnapshot for both
+    // calls) would let a preview-then-cancel workflow silently erase evidence of real,
+    // undetected drift with no notification ever shown - caught in code review.
+    expect(checkCoexistenceDriftMock).toHaveBeenCalledTimes(1);
+    expect(checkCoexistenceDriftMock).toHaveBeenCalledWith(api, previewSnapshot);
+    expect(recordOwnMergeStateSnapshotMock).not.toHaveBeenCalledWith(previewSnapshot);
+
+    // Real merge (dryRun: false) - this extension's own write, silently adopted as the
+    // new known-good baseline, no comparison/notification.
+    expect(recordOwnMergeStateSnapshotMock).toHaveBeenCalledTimes(1);
+    expect(recordOwnMergeStateSnapshotMock).toHaveBeenCalledWith(finalSnapshot);
+    expect(checkCoexistenceDriftMock).not.toHaveBeenCalledWith(api, finalSnapshot);
+  });
+
+  it('does not let a coexistence-guard snapshot failure affect the merge result the user sees', async () => {
+    const { api, showDialogCalls } = fakeApi({
+      toolPath: 'C:\\wsm\\WitcherScriptMerger.Headless.exe',
+      dialogResponses: [{ action: 'Merge Now' }],
+    });
+    const preview = mergeResult({ merged: ['a.ws'], skipped: [] });
+    const final = mergeResult({ merged: ['a.ws'], skipped: [], dryRun: false });
+    const { connect } = fakeConnect([{ result: preview }, { result: final }]);
+    computeMergeStateSnapshotMock.mockReset().mockRejectedValue(new Error('snapshot failed'));
+
+    await resolveScriptConflicts(api, { connect });
+
+    expect(showDialogCalls).toHaveLength(2);
+    expect(showDialogCalls[1].type).toBe('success');
+    expect(recordOwnMergeStateSnapshotMock).not.toHaveBeenCalled();
+    expect(checkCoexistenceDriftMock).not.toHaveBeenCalled();
+  });
+
   it('shows an "info" (not "success") result dialog when the real merge still leaves skipped files', async () => {
     const { api, showDialogCalls } = fakeApi({
       toolPath: 'C:\\wsm\\WitcherScriptMerger.Headless.exe',