Skip to content

Give the unresolved-conflicts notification a "Resolve Now" button - #41

Open
TheValiantOne wants to merge 1 commit into
mainfrom
feature/vortex-remerge-prompt
Open

Give the unresolved-conflicts notification a "Resolve Now" button#41
TheValiantOne wants to merge 1 commit into
mainfrom
feature/vortex-remerge-prompt

Conversation

@TheValiantOne

Copy link
Copy Markdown
Owner

The problem

The post-deploy scan's notification shipped actions: [] and a message that named a count and nothing else.

On a real install it fired seconds before the user launched a game that wouldn't start:

12:46:27 [WARN] warning notification {"message":"WitcherScriptMerger: 5 unresolved script conflicts found"}
12:47:08 [ERRO] Failed to preview script-conflict merges (MCP timeout)

The extension had detected the problem and said so. It led nowhere — acting on it meant knowing to go find a different button, on a different page, with no indication that the consequence was a game that wouldn't launch. A warning like that reads as noise, and gets treated as noise.

The change

The notification now carries a "Resolve Now" action running the same workflow as the Mods-page toolbar action, and states the consequence:

WitcherScriptMerger: 5 unresolved script conflicts found - the game may fail to start until they are merged

The action dismisses the notification before starting the workflow, so a count that's about to be stale can't sit there through the merge and then get re-shown by the post-merge scan.

Why the resolver is injected

resolveActioncoexistenceGuardconflictNotifications. Importing resolveScriptConflicts back into conflictNotifications would close that cycle.

So it's an optional third argument, supplied by index.ts — the composition root, which already imports resolveAction. The call is wrapped in the same log-only last-resort catch registerResolveScriptConflictsAction uses around its own invocation, since resolveScriptConflicts reports its own failures to the user.

Omitting the argument keeps the old passive shape, so this is additive for any other caller.

Verification

6 new tests, 223 total, all passing. npm run typecheck and npm run lint clean.

conflictNotifications.test.ts:

  • the action's presence and title
  • invoking it runs the resolver and dismisses first — asserted by recorded call order, not just by both having been called
  • a missing dismiss callback doesn't throw
  • the message carries the consequence wording, not just the count

index.test.ts:

  • the injected callback reaches resolveScriptConflicts with the api
  • a rejected workflow doesn't escape as an unhandled rejection

One test-infrastructure note for review: index.test.ts's new './resolveAction' mock is partial (importOriginal spread, stubbing only resolveScriptConflicts). Mocking the whole module broke the existing test that asserts registerResolveScriptConflictsAction reaches context.registerAction — that one needs the real implementation.

Scope

Only the unresolved-conflicts notification. coexistenceGuard's drift notification already has a "More" action and fires during a resolveScriptConflicts run (right before the preview dialog appears), so a second button there would be redundant — left alone deliberately.

AI-assisted development

Produced by Claude Code (Opus 5). Per CONTRIBUTING.md: the log excerpt above is from a real install, and the test/typecheck/lint results are from real runs.

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FP8H6rBLCGPBFRSVsF3Kgw
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