Skip to content

Add post-deploy WSM conflict scanning and notification (Unit G) - #19

Merged
TheValiantOne merged 3 commits into
mainfrom
feature/vortex-conflict-scan-notifications
Aug 10, 2026
Merged

Add post-deploy WSM conflict scanning and notification (Unit G)#19
TheValiantOne merged 3 commits into
mainfrom
feature/vortex-conflict-scan-notifications

Conversation

@TheValiantOne

@TheValiantOne TheValiantOne commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Summary

Unit G of the Vortex extension plan: after a Vortex deployment finishes for Witcher 3,
scan for WSM script conflicts and show a dashboard notification when the unresolved
conflict set has changed since the last check this session. Builds on Unit E's
WsmMcpClient (PR #15) and Unit F's tool acquisition/env-var config (PR #17).

  • vortex-extension/src/conflictScan.ts (new) — scanWsmConflicts(api) spawns a
    short-lived WsmMcpClient pointed at the acquired WSM exe and Witcher 3's discovered
    game directory (via wsmEnv.ts's WSM_<KeyName> mechanism), runs scan_conflicts,
    and closes the client in a finally — matching mcpClient.ts's documented
    process-lifecycle policy exactly. isWsmToolAcquired(api) is a cheap, local
    existence check so a deploy before any WSM tool is acquired doesn't spawn a doomed
    process. Coalesces overlapping calls onto a single in-flight scan (see "Post-review
    fixes" below) and requests a tighter MCP timeout than the client's own default.
  • vortex-extension/src/conflictNotifications.ts (new) — notifyConflictsIfChanged(api, conflicts)
    takes an already-obtained scan result (deliberately, for direct unit-testability) and
    shows/updates/dismisses a dashboard notification via context.api.sendNotification.
  • vortex-extension/src/index.ts — one additive context.api.onAsync('did-deploy', ...)
    registration inside the existing context.once(...) block, gated on the deployed
    profile's
    own game (see "Post-review fixes" below for why this isn't
    isWitcher3Active(context.api), contrary to this unit's own original task
    description).

This PR went through three rounds of review: an initial implementation pass, a second
pass (prompted by a coordinator-relayed code review) that found and fixed 6 real issues,
and a third pass (the code-review skill invocation this unit's own instructions
require, whose results were delayed by an unrelated background-agent naming collision
and arrived after round 2 had already shipped) that found 7 more, 6 of which were real
and fixed — see "Post-review fixes" and "Round 3" below. All three rounds prioritized
verifying against real source over trusting a plausible-looking guess, including
catching two of my own overclaimed citations along the way.

Verification methodology — everything below is either directly run, or confirmed against real source I fetched and read (never guessed)

Per the task's own instruction to verify against real types/source rather than trust
anything secondhand (same discipline Unit E used for IRunOptions), I fetched and read
the actual Nexus-Mods/Vortex monorepo source via gh api wherever the published
@nexusmods/vortex-api npm package's lib/api.d.ts typings and docs weren't enough on
their own. Exact citations below.

did-deploy is async — onAsync, not events.on

The task's own suggested snippet used context.api.events.on('did-deploy', ...). That's
wrong. @nexusmods/vortex-api's bundled docs/EVENTS.md lists did-deploy under
"Async" events (fired via emitAndAwait), and its own README.md #### Event hooks
section gives the exact pattern:

context.api.onAsync("did-deploy", async (profileId, deployment) => { ... });

Confirmed a second, independent way: Vortex's own built-in game-witcher3 extension
registers its analogous handler the same way —
extensions/games/game-witcher3/src/index.ts:313:
context.api.onAsync("did-deploy", onDidDeploy(context.api) as any) (commit
f68defdd71c9a9a43673fe15550fdde53cd73ca2 at time of fetch). This repo's own
checkForConflictsAfterDeploy follows the same onAsync contract documented in
lib/api.d.ts: "listeners should report all errors themselves" — it never lets an
error propagate out, matching tryRegisterWsmTool's existing catch-and-log shape.

Distinct notification id, verified against the real collision risk

game-witcher3's own eventHandlers.ts (queryScriptMerge) sends its "you may need to
run the script merger" notification with id: "witcher3-merge". This unit uses
witcherscriptmerger-vortex-conflicts instead, so a user with both extensions installed
sees both rather than one silently overwriting the other's notification slot.

sendNotification/INotification shape verified directly against lib/api.d.ts

sendNotification?: (notification: INotification) => string and INotification's real
fields (id?, type: NotificationType, message, allowSuppress?, actions?) were
read directly out of the installed package's typings before writing any notification
code — not assumed from the task description's summary of scriptmerger.ts's shape.

Suppression logic — a real bug found and fixed mid-implementation

The task pointed at state.session.base.activity for the mod-install/dependency-install
gate. @nexusmods/vortex-api's published lib/api.d.ts types ISession.activity as
{[group: string]: string}. That type is wrong; confirmed against the real Vortex
reducer, not just the shipped .d.ts.

Fetched src/renderer/src/reducers/session.ts (commit
8c3793aed7063a43bf400d3d6919491b9897b2e2):

startActivity: (state, payload) => {
  const group = state.activity[payload.group] ?? new Array<string>();
  if (group.includes(payload.activityId)) return state;
  return { ...state, activity: { ...state.activity, [payload.group]: [...group, payload.activityId] } };
},
stopActivity: (state, payload) => {
  const group: string[] = state.activity[payload.group] ?? new Array<string>();
  return { ...state, activity: { ...state.activity, [payload.group]: group.filter((id) => id !== payload.activityId) } };
},

The real runtime value is a string[] per group, and stopActivity never
deletes the group key
— it leaves an empty array behind. My first implementation used
Boolean(activity.installing_dependencies), which would have seen that leftover []
(truthy in JS) as "still active" forever after the first dependency install of the
session completed, permanently suppressing every future notification. Fixed with a
shape-tolerant activityEntries() helper (conflictNotifications.ts) that handles both
the real array shape and the documented-but-stale string shape, plus a regression test
({ installing_dependencies: [] } must NOT suppress).

The two specific group/activityId checks:

  • installing_dependencies non-empty — the exact guard game-witcher3's own
    queryScriptMerge uses before its own conflict-adjacent notification:
    if ((state.session.base.activity?.installing_dependencies ?? []).length > 0) { return; } (eventHandlers.ts, same commit as above) — this real call site already
    assumes the array shape, independently confirming the reducer finding. Populated by
    startActivity("installing_dependencies", <modId>) in
    mod_management/InstallManager.ts (commit 0c087e87af09873956e6b0e48a613dab416a57d7).
  • mods group contains 'installing' — the plainer single-mod-install case,
    populated by startActivity("mods", "installing") in mod_management/InstallContext.ts
    (commit f840765cddca7277725e6e4ae04dfd2f333f155c). Deliberately not "the mods
    group is non-empty": that same key is also used for startActivity("mods", "deployment") in mod_management/index.ts (commit
    331157b72c0324eafd64dfdb0ecb197daf0a9bdb), and stopActivity("mods", "deployment")
    only fires after emitAndAwait("did-deploy", ...) resolves — after every
    did-deploy handler, including this extension's own, has run. A blanket "mods
    non-empty" check would always see this extension's own deployment window as "install
    in progress" and permanently suppress every notification. Unlike the
    installing_dependencies check, this one has no direct precedent in game-witcher3's
    own code — included because the task explicitly names "mod-install" alongside
    "dependency-install," flagged here as the less battle-tested of the two.

Post-review fixes

A coordinator-relayed code review (a separate pass, Angle D — language/framework
pitfalls) found 6 issues in the first draft of conflictScan.ts/conflictNotifications.ts.
All 6 were re-verified directly against the real code before fixing (not taken on
faith), and all 6 held up:

  1. isWsmToolAcquired swallowed every fs.access error, not just ENOENT
    unlike toolAcquisition.ts's own pathExists, which it claimed to mirror. A
    locked/permission-denied exe path (an antivirus scan, a concurrently running WSM
    process) would look identical to "nothing installed yet" and silently skip scanning
    for the rest of the session with only a misleading 'debug' log. Fixed: now
    re-throws anything that isn't ENOENT, matching pathExists in substance.

  2. lastNotifiedSignature was committed before sendNotification/
    dismissNotification actually succeeded.
    If that call threw (or the resulting
    promise-adjacent path failed for any reason), the signature was already marked
    "shown" — so the user never actually saw the notification, yet every later
    did-deploy with the identical conflict set would silently skip re-attempting it for
    the rest of the session. Fixed: the send/dismiss call is now wrapped in its own
    try/catch inside notifyConflictsIfChanged, the signature is only committed on
    success, and a failure is logged and swallowed locally (never thrown — this function
    is reachable from the onAsync handler, which must never reject). Regression-tested
    for both the send-failure and dismiss-failure paths, including a test that
    specifically distinguishes "failure correctly left retryable" from "failure silently
    treated as if it had succeeded" (my first draft of that particular test was itself
    wrong in a way that wouldn't have caught the bug — corrected before committing; see
    commit history for the honest trail).

  3. scanWsmConflicts had no in-flight coalescing, unlike toolAcquisition.ts's
    inFlightAcquisitions map for acquireWsmTool. Overlapping did-deploy events
    could spawn two concurrent WSM processes against the same mods folder and — worse —
    resolve out of order, letting a stale scan's result reach notifyConflictsIfChanged
    after a fresher one already had, showing a notification that no longer matched the
    real conflict set and recording that stale signature as "already seen." Fixed: added
    the same single-slot coalescing pattern acquireWsmTool already uses (a bare module
    slot here, not a Map, since this extension only ever scans one thing).

  4. The post-deploy scan used mcpClient.ts's general-purpose 30s-per-request
    default
    (up to ~60s worst case across the handshake + scan_conflicts call), but
    this path runs inside Vortex's own emitAndAwait('did-deploy', ...) await window —
    confirmed by reading mod_management/index.ts: stopActivity('mods', 'deployment')
    doesn't fire until every did-deploy handler resolves. A slow/hung WSM process would
    extend Vortex's own reported deployment-completion time for that long, from a hook
    the user never explicitly asked to wait on. Fixed: scanWsmConflicts now passes a
    tighter requestTimeoutMs: 15_000 specific to this call site, via
    WsmMcpClientOptions' already-public per-call override — mcpClient.ts itself is
    untouched, per this unit's own "don't touch" list.

  5. checkForConflictsAfterDeploy gated on isWitcher3Active(context.api)
    whichever game is active at the moment this async handler happens to run — rather
    than the deployed profile's own game (did-deploy's own profileId argument). Since
    this handler is one of potentially several emitAndAwait('did-deploy', ...)
    listeners, and the user can act on Vortex's UI while earlier listeners are still
    resolving, a real Witcher 3 deployment's scan could be silently skipped if the user
    switched to a different game before this handler's own turn came up — a false
    negative that misses genuine conflicts, not just an ordering nicety. Fixed: now
    resolves profileId to that specific profile's own gameId via
    selectors.profileById(state, profileId) and gates on that instead — time-invariant,
    so immune to this race regardless of when the handler actually executes.
    Deliberately deviates from this unit's own original task description ("Gate
    everything on isWitcher3Active(context.api)"); isWitcher3Active is retained
    exactly as before in tryRegisterWsmTool, where "what's active right now" genuinely
    is the correct question (it's driven by gamemode-activated, not a specific
    deployment). ANDing both checks together in checkForConflictsAfterDeploy would have
    reintroduced the exact false-negative this fix removes, so it isn't used there.
    Honesty note on a citation I initially overstated: I first described this as "the
    exact pattern game-witcher3 already uses" for its own onDidDeploy/onWillDeploy.
    That's not accurate — I verified game-witcher3's actual
    validateProfile(profileId, state) (extensions/games/game-witcher3/src/util.ts)
    directly, and it still ultimately keys off selectors.activeProfile(state).gameId
    (the same "what's active now" read this fix moves away from), just with an added
    guard that profileId matches the currently-active profile's own id. That's the
    right call for what game-witcher3 uses it for (INI/load-order bookkeeping that only
    makes sense for the actively-displayed profile) — it is not the right call for this
    extension's narrower job (telling the user about conflicts from a deployment that
    genuinely happened, which is still true information even if they've since switched
    games), so this PR deliberately drops that same-profile cross-check rather than
    copying it verbatim. Caught and corrected this overclaim before finalizing, per the
    same "verify, don't assert" standard the rest of this description holds itself to.

  6. lastNotifiedSignature initialized to undefined instead of '' (the same
    value computeConflictSignature([]) produces for "no conflicts"), so the very first
    post-deploy check of a session with zero conflicts always called
    dismissNotification for a notification id that was never sent. Harmless against the
    real Vortex API (a no-op on an unknown id) but pointless noise. Fixed: initialized to
    '' instead, so the first zero-conflict check matches the same-signature
    early-return.

A regression introduced while fixing #1, caught before committing: making
isWsmToolAcquired throw on non-ENOENT errors meant that call needed to move inside
checkForConflictsAfterDeploy's own try/catch rather than gating ahead of it — otherwise
a real EBUSY/EPERM would reject the onAsync('did-deploy', ...) handler's promise
straight into Vortex's own dispatch, exactly the failure mode onAsync's contract
forbids. Fixed and regression-tested (isWsmToolAcquired rejecting must still resolve
the handler, never throw).

Round 3: the delayed code-review skill findings

This unit's own instructions require invoking the code-review skill and fixing its
findings before opening the PR. That invocation happened early, but its background
result got stuck behind a same-name collision with other agents already active in this
session and only arrived after round 2 above had already shipped. It re-verified round
2's fixes directly against the committed code (confirmed all 6) and surfaced 7 more.
Each was independently re-checked against the real code rather than taken on faith; 6
were real and fixed, 1 (the integration-test build race) was already known and
disclosed in this description, so left as-is:

  1. Two doc comments (conflictScan.ts, gating.ts) still described conflict
    scanning as gated on isWitcher3Active(api)
    - stale since round 2's profileId-based
    gating fix (Split into WitcherScriptMerger.Core + host GUI/CLI/MCP project #5 above). Both corrected to describe the actual, deliberate exception.

  2. computeConflictSignature keyed only on relativePath, so a conflict whose
    contributing mod set changed between two scans (e.g. a third mod starts touching an
    already-conflicting file that was never merged) produced an identical signature to
    before, and the user was never re-notified about a real, relevant change. Fixed:
    each conflict's entry in the signature now also includes its sorted contributing mod
    names (relativePath:sortedModNames, entries newline-joined - |, :, and newline
    are all part of Windows' reserved/control-character set, so none can appear in a
    real file/directory name and collide two genuinely different conflict sets onto the
    same signature).

  3. checkForConflictsAfterDeploy spawned a full WSM process unconditionally
    whenever a tool was acquired, checking isModOrDependencyInstallActive only
    afterward, inside notifyConflictsIfChanged - which would then just discard that
    scan's result. During a dependency-install burst (e.g. installing a Collection
    triggers several deploy-per-mod cycles), each cycle would spawn and tear down a WSM
    process for a result that was thrown away. Fixed: exported
    isModOrDependencyInstallActive from conflictNotifications.ts and added a
    pre-check before ever calling scanWsmConflicts - a pure optimization, not a
    correctness fix, since the later check inside notifyConflictsIfChanged stays in
    place as defense-in-depth (activity can start during the scan itself, after a clean
    pre-check).

  4. The outer selectors.profileById(...) gate sat outside
    checkForConflictsAfterDeploy's try/catch
    - a narrower instance of the exact class
    of bug already fixed for isWsmToolAcquired in round 2 (a synchronous throw would
    reject the onAsync handler's promise, violating its documented "never reject"
    contract). Low likelihood (a plain state lookup, not a filesystem call with a real
    trigger like EBUSY), but free to close. Fixed: the try/catch now wraps the entire
    handler body, including this gate.

  5. A real TOCTOU gap between isWsmToolAcquired's existence check and
    scanWsmConflictsUncoordinated's own independent connect() call a few lines later

    • a concurrent tool re-acquisition could overwrite the exe in between. Verified real,
      but left as a documented comment, not a code change: the failure mode is already
      fully contained (WsmMcpClient.connect rejects, and the surrounding try/catch
      already logs it as a warning rather than crashing), so this is an inherent
      check-then-act gap already handled safely, not something worth adding
      synchronization machinery for.
  6. The dotnet build race between integration test files - re-confirmed real by
    the reviewer, already disclosed in this PR's own "known, pre-existing test-infra
    flake" section below (written during round 2, before this round's review even ran).
    No new action - left as documented for the same reasons already given there.

89 unit tests (up from 82 after round 2), 95 total with integration.

What was actually verified by running vs. by reading source

Run and green:

  • npm run typecheck && npm run build && npm run lint && npm test — 89 fast unit tests
    (src/conflictScan.test.ts, src/conflictNotifications.test.ts, updated
    src/index.test.ts), including every regression test named above (both rounds).
  • npm run test:integrationtest/conflictScan.integration.test.ts builds and spawns
    the real WitcherScriptMerger.Headless mcp server against a scratch mods folder with
    two real mods both placing conflicting.ws under content\scripts\, runs a real
    scan_conflicts, confirms it reports exactly that one genuine conflict, and feeds the
    real result into notifyConflictsIfChanged end-to-end (sends once, doesn't re-send on
    an identical second call). A second scenario confirms an empty mods folder produces no
    notification. 95 tests total across the full suite.
  • Manually traced notifyConflictsIfChanged against fabricated conflict arrays for all
    three suppression scenarios named in this unit's instructions: same-signature twice →
    no second notification; different signature → notifies; activity-in-progress → skips
    (including the real array-shape case and the stale-empty-array trap above).

Not run — confirmed only by reading real, current source (see citations above), or
genuinely unverifiable without a live Vortex install:

  • The real notification UI actually rendering, allowSuppress's in-app suppress
    behavior, and directly observing a real did-deploy fire (with a real profileId)
    from a live Vortex + Witcher 3 deployment. Needs a manual pass with real Vortex.
  • checkForConflictsAfterDeploy still spawns and awaits a WSM process inside
    emitAndAwait('did-deploy', ...)'s await window even with the tightened 15s timeout
    (fix Add Vortex extension design doc (Unit 4, design only) #4 bounds the worst case, it doesn't eliminate the underlying design tradeoff).
    game-witcher3's own onDidDeploy does async work in the same window, so this isn't
    a novel problem, but a live tester should watch for it.
  • The scan-coalescing fix (Research spike: WolvenKit as a QuickBMS/wcc_lite replacement #3) and the profileId-based gating fix (Split into WitcherScriptMerger.Core + host GUI/CLI/MCP project #5) are both unit- and
    logic-tested against fabricated/mocked scenarios, but neither has been exercised
    against a real overlapping-deployment or real-mid-deploy-game-switch scenario in a live
    Vortex — both are inherently hard to trigger reliably even with a real install.

A known, pre-existing test-infra flake — not a regression

Cold-cache npm run test:integration can fail: it runs three *.integration.test.ts
files in parallel by default, and my new file is the second one that does
dotnet build -c Debug into the exact same WitcherScriptMerger.Core/.Headless
output paths test/mcpClient.integration.test.ts already builds into — concurrent
dotnet build processes racing the same obj//bin/ output can hit file locks
(CSC : error CS2012 ... file may be locked by 'Microsoft Defender Antivirus Service')
or a NETSDK1047 restore-target mismatch from another file's concurrent restore
clobbering the shared obj/project.assets.json mid-build. Reproduced, then confirmed
non-fatal two ways: vitest run test --no-file-parallelism passes cleanly, and a
warm-cache npm run test:integration (build outputs already present, so every file's
own if (!fs.existsSync(...)) guard skips rebuilding) also passes cleanly in parallel.
This is a pre-existing property of each integration test file independently
guarding+building with no cross-file coordination, not something this unit's own code
broke — I deliberately didn't restructure the existing files' build orchestration, since
sibling units (H/I/J) may add their own integration test files with the same pattern in
parallel branches.

AI-assisted development

This PR was substantially produced by Claude Code (Sonnet 5), per this repo's
CONTRIBUTING.md disclosure requirement. All source-verification claims above were
confirmed against real, current @nexusmods/vortex-api typings and Nexus-Mods/Vortex
monorepo source fetched via gh api, not asserted from training-data memory. This PR's
history is a genuine example of the verification discipline CONTRIBUTING.md's
AI-assisted-development section asks for in practice, not just in principle, across all
three rounds: the initial implementation had a real, severe bug (the activity
array-shape issue) caught before first opening this PR; a coordinator-relayed second
review then found 6 more real issues, all independently re-verified rather than fixed on
faith (and during that pass, I caught and corrected my own overclaimed citation - fix
#5's game-witcher3 comparison - before it shipped); and a third round (this unit's own
required code-review skill invocation, whose result was delayed by an unrelated
background-agent naming collision) found 6 more genuine issues after round 2 had already
shipped, again each independently re-verified rather than accepted at face value.
Flagged explicitly because it's exactly the "looks plausible, compiles, silently wrong
(or overstated)" failure mode this repo's own AI-assisted-development guidance warns
about — the fix isn't "trust the agent less," it's "the agent re-verified its own claims
before submitting," which is what happened here, three times over.

Test plan

  • npm run typecheck
  • npm run build
  • npm run lint
  • npm test (89 unit tests)
  • npm run test:integration (95 tests total, verified both warm-cache-parallel and
    --no-file-parallelism)
  • Manual: confirm real notification rendering + allowSuppress behavior against a
    live Vortex + Witcher 3 install
  • Manual: confirm did-deploy actually fires (with a real profileId) as expected
    from a real deployment
  • Manual: exercise the profileId-based gating fix (Split into WitcherScriptMerger.Core + host GUI/CLI/MCP project #5) and the scan-coalescing fix
    (Research spike: WolvenKit as a QuickBMS/wcc_lite replacement #3) against real, hard-to-synthesize scenarios (a mid-deploy game switch;
    overlapping deployments) if practical

Chris Knight and others added 3 commits August 10, 2026 15:39
…nit G)

After a Vortex deployment finishes for Witcher 3, spawn a short-lived WsmMcpClient,
run scan_conflicts, and show a dashboard notification when the set of unresolved
conflicts has changed since the last check this session. Registered via
context.api.onAsync('did-deploy', ...), matching the real, verified event contract
(async, fired via emitAndAwait) and the built-in game-witcher3 extension's own
onDidDeploy wiring - not context.api.events.on, contrary to a plausible-looking guess.

Uses a notification id distinct from game-witcher3's own "witcher3-merge" so both
extensions can coexist without colliding on the same slot.

Skips the notification while Vortex reports mod-install or dependency-install activity
in progress, per state.session.base.activity. Found and fixed a real bug during
development: @nexusmods/vortex-api's published types describe that field as
{[group: string]: string}, but the actual Vortex reducer stores a string[] per group,
and never deletes the group key on stopActivity - it leaves an empty array behind. A
naive truthiness check would have seen that leftover [] as "still active" and
permanently suppressed every future notification after the first install ever ran.
Fixed with a shape-tolerant activityEntries() helper, regression-tested against the
empty-array case.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
…w-up)

A code-review pass surfaced 6 issues; all verified against the real code and fixed:

1. isWsmToolAcquired swallowed every fs.access error, not just ENOENT - a locked file
   (AV scan, a running WSM process) would silently look identical to "nothing
   installed". Now mirrors toolAcquisition.ts's pathExists exactly: re-throws anything
   that isn't ENOENT.

2. notifyConflictsIfChanged committed lastNotifiedSignature before the
   sendNotification/dismissNotification call actually succeeded. A failure there would
   permanently suppress the real notification for that conflict set, since the next
   check would see the same signature and skip. Now wrapped in try/catch, with the
   signature only committed on success, and the error swallowed (never thrown) per
   onAsync's contract.

3. scanWsmConflicts had no in-flight coalescing, unlike toolAcquisition.ts's
   inFlightAcquisitions. Overlapping did-deploy events could run two concurrent WSM
   processes and resolve out of order, feeding a stale result to
   notifyConflictsIfChanged after a fresher one already landed. Added the same
   single-slot coalescing pattern.

4. The post-deploy scan used mcpClient.ts's full 30s-per-request default, but this path
   runs inside Vortex's own emitAndAwait('did-deploy', ...) await window - a slow WSM
   process would extend Vortex's own reported deployment-completion time. Added a
   tighter 15s requestTimeoutMs specific to this call site (mcpClient.ts itself
   untouched - this uses its existing public per-call override).

5. checkForConflictsAfterDeploy gated on isWitcher3Active(context.api) - whichever game
   is active when the async handler happens to run - rather than the deployed
   profile's own game (did-deploy's own profileId argument). A user switching games
   between did-deploy firing and this handler's turn coming up could cause a real
   Witcher 3 deployment's scan to be silently skipped. Now resolves profileId's own
   gameId via selectors.profileById and gates on that instead - time-invariant, so
   immune to this race. (Verified against game-witcher3's own validateProfile as
   precedent for the general profileId-driven approach, but it is not a verbatim copy:
   that function still ultimately keys off the active profile, with an added
   same-profile guard - not needed for this extension's narrower job of surfacing
   conflicts from a deployment that genuinely happened.)

6. lastNotifiedSignature initialized to undefined instead of '', so the very first
   post-deploy check of a session with zero conflicts always called dismissNotification
   for an id that was never sent. Now initialized to '', matching
   computeConflictSignature([])'s own value.

Also fixes a regression introduced while addressing #1: isWsmToolAcquired's new
non-ENOENT throw must stay inside checkForConflictsAfterDeploy's try/catch, not ahead
of it, or it would reject the onAsync('did-deploy', ...) handler's promise straight
into Vortex's own dispatch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
…low-up 2)

The code-review skill invocation from earlier in this unit's work finally delivered
its results (delayed by a background-agent naming collision). It had reviewed the
already-fixed commit and confirmed those 6 fixes, but surfaced 7 further findings.
Verified each against the real code; fixed 6 of them:

- conflictScan.ts's and gating.ts's own doc comments still described conflict
  scanning as gated on isWitcher3Active(api) - stale since the profileId-based gating
  fix. Corrected both to describe the actual, deliberate exception and why.

- computeConflictSignature keyed only on relativePath, so a conflict whose
  contributing mod set changed (e.g. a third mod starts touching an already-conflicting
  file) produced the same signature as before and was silently never re-notified. Now
  includes each conflict's sorted mod names in its signature entry.

- checkForConflictsAfterDeploy spawned a full WSM process unconditionally whenever a
  tool was acquired, checking install-activity only afterward (inside
  notifyConflictsIfChanged, which would then discard the result). Exported
  isModOrDependencyInstallActive from conflictNotifications.ts and added a pre-check
  before scanning, avoiding wasted process spawns during e.g. a Collection install's
  deploy-per-mod bursts. The later check inside notifyConflictsIfChanged stays as
  defense-in-depth, since activity can start during the scan itself.

- The outer selectors.profileById(...) gate sat outside checkForConflictsAfterDeploy's
  try/catch - a narrower version of the same class of bug just fixed for
  isWsmToolAcquired. Widened the try/catch to cover the entire handler body.

Two findings were verified as real but left as documented, not code changes:
- A TOCTOU gap between isWsmToolAcquired's check and scanWsmConflicts's own connect()
  - already safely contained by the existing try/catch (a caught warning, not a
  crash), so just documented rather than adding synchronization machinery for an
  already-handled race.
- The dotnet build race between integration test files - already disclosed in this
  PR's own description as a known, pre-existing test-infra property; not restructured
  here since sibling units may add their own integration test files with the same
  pattern.

89 unit tests (up from 82), 95 total with integration.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
@TheValiantOne
TheValiantOne merged commit 839aa30 into main Aug 10, 2026
1 check passed
@TheValiantOne
TheValiantOne deleted the feature/vortex-conflict-scan-notifications branch August 10, 2026 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant