From 406edd45a93a82cee97ccd07f6399b0e4f64eb3d Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Fri, 21 Aug 2026 23:07:36 -0400 Subject: [PATCH] Remove the merge_conflicts deadline entirely rather than raising it The previous commit gave merge_conflicts a ten-minute deadline instead of mcpClient.ts's 30s default. That was the wrong fix: a wall-clock limit is the wrong instrument for this call at any value. A merge's runtime is a function of the user's load order, with no bound anyone can pick in advance, so a deadline can only ever fire on a merge that is running normally and simply needs longer - and firing means abandoning it mid-flight, which is how the merged mod ended up empty and the game unable to start in the first place. Both call sites (dry-run preview and real merge) now pass NO_REQUEST_TIMEOUT. Waiting indefinitely is safe because liveness, not the clock, is what detects a dead server: WsmMcpClient's constructor already wires failAllPending to the child's exit, error and stdin-error events, so a WSM process that crashes, is killed, or closes its pipes rejects every in-flight request immediately with a WsmMcpProcessError. A deadline adds nothing to that except the ability to give up on a process that is alive and still working. Still applied per call, not as connect()'s client-wide requestTimeoutMs: that also bounds the initialize handshake, where a few seconds of silence means WSM failed to start and should keep failing fast. request() now takes number | null | undefined - undefined means "client default", null means "no deadline" - and PendingRequest.timer widens to allow undefined (clearTimeout(undefined) is a no-op, so settle paths are unchanged). A timeout can therefore no longer originate from the merge itself, so reportFailure's timeout wording now points at WSM failing to start rather than being slow. Tests updated (219 total): the merge call never settles after an hour of fake time; it still rejects promptly when the child exits or its stdin errors (the liveness net that makes this safe); a call with no override still uses the client default; an unbounded call does not stop later calls on the same client timing out; and the handshake keeps the short default. typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FP8H6rBLCGPBFRSVsF3Kgw --- vortex-extension/README.md | 16 +++-- vortex-extension/src/mcpClient.test.ts | 66 +++++++++++++----- vortex-extension/src/mcpClient.ts | 54 ++++++++++----- vortex-extension/src/resolveAction.test.ts | 34 ++++----- vortex-extension/src/resolveAction.ts | 80 ++++++++++++---------- 5 files changed, 155 insertions(+), 95 deletions(-) diff --git a/vortex-extension/README.md b/vortex-extension/README.md index 197d02f..00e0a58 100644 --- a/vortex-extension/README.md +++ b/vortex-extension/README.md @@ -53,13 +53,15 @@ does anything for any other game. whole-file merge failed but merging function-by-function succeeded), and - only on confirmation - spawns a second process to run the real merge and shows its result. v1 scope, deliberately: merges every detected conflict in one pass; there's no per-file - selection or custom merge-order override yet. Both `merge_conflicts` calls get their own - ten-minute deadline (`MERGE_CALL_TIMEOUT_MS`) rather than `mcpClient.ts`'s - general-purpose 30s `DEFAULT_REQUEST_TIMEOUT_MS` — a merge's runtime scales with the - load order, and the dry-run preview costs the same as the merge it previews (it does the - full three-way merge and only skips the writes). The deadline is passed per call, so the - `initialize` handshake keeps the short default and a WSM process that fails to start - still fails fast. + selection or custom merge-order override yet. Both `merge_conflicts` calls run with **no + deadline** (`MERGE_CALL_TIMEOUT` = `NO_REQUEST_TIMEOUT`) rather than `mcpClient.ts`'s + general-purpose 30s `DEFAULT_REQUEST_TIMEOUT_MS` — a merge's runtime is the user's load + order, so any wall-clock limit can only fire on a merge that is working normally, and the + dry-run preview costs the same as the merge it previews (it does the full three-way merge + and only skips the writes). That's safe because the client fails every in-flight request + the moment the WSM process exits, errors, or closes its pipes, so liveness — not the + clock — detects a dead server. Applied per call, so the `initialize` handshake keeps the + short default and a WSM process that fails to start still fails fast. - **A merge-history dashboard tile** (`src/mergeHistoryDashlet.ts`): lists every merge WSM has already recorded (via its MCP `list_merges` tool) - relative path, which merged mod folder holds the result, and each source mod's recorded hash - with a manual diff --git a/vortex-extension/src/mcpClient.test.ts b/vortex-extension/src/mcpClient.test.ts index cea65de..4defe98 100644 --- a/vortex-extension/src/mcpClient.test.ts +++ b/vortex-extension/src/mcpClient.test.ts @@ -8,7 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() })); vi.mock('child_process', () => ({ spawn: spawnMock })); -const { WsmMcpClient } = await import('./mcpClient'); +const { WsmMcpClient, NO_REQUEST_TIMEOUT } = await import('./mcpClient'); /** A newline-delimited-JSON stand-in for a spawned WSM `mcp` process. Answers * `initialize` (so connect() resolves) and, by default, nothing else - leaving a @@ -52,6 +52,16 @@ function fakeChild() { return { child, written }; } +/** Tracks settlement without letting an expected rejection escape as unhandled. */ +function track(p: Promise) { + const state = { settled: false, rejected: false, error: undefined as unknown }; + p.then( + () => { state.settled = true; }, + (err) => { state.settled = true; state.rejected = true; state.error = err; }, + ); + return state; +} + describe('WsmMcpClient request deadlines', () => { beforeEach(() => { spawnMock.mockReset(); @@ -65,22 +75,44 @@ describe('WsmMcpClient request deadlines', () => { async function connectFake(requestTimeoutMs?: number) { const { child, written } = fakeChild(); spawnMock.mockReturnValue(child); - return { client: await WsmMcpClient.connect({ exePath: 'C:\\wsm\\fake.exe', requestTimeoutMs }), written }; + const client = await WsmMcpClient.connect({ exePath: 'C:\\wsm\\fake.exe', requestTimeoutMs }); + return { client, child, written }; } // The bug this whole change exists for: merge_conflicts used to inherit the 30s // general-purpose default, and a big load order's merge (or its equally expensive - // dry-run preview) blew straight through it. - it('honors a per-call timeout override instead of the client default', async () => { + // dry-run preview) blew straight through it. A merge now gets no deadline at all. + it('never times out a call made with NO_REQUEST_TIMEOUT, however long it runs', async () => { const { client } = await connectFake(30_000); - const pending = client.mergeConflicts({ dryRun: true }, 600_000); - const assertion = expect(pending).rejects.toThrow(/timed out after 600000ms/); + const state = track(client.mergeConflicts({ dryRun: true }, NO_REQUEST_TIMEOUT)); + + // an hour of wall clock - far past the 30s client default + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + + expect(state.settled).toBe(false); + }); + + // Liveness, not the clock, is what makes an unbounded wait safe: if WSM dies the + // request must still reject promptly rather than hang forever. + it('still rejects an unbounded call when the WSM process exits', async () => { + const { client, child } = await connectFake(30_000); + + const pending = client.mergeConflicts({ dryRun: true }, NO_REQUEST_TIMEOUT); + const assertion = expect(pending).rejects.toThrow(/exited unexpectedly \(code 1\)/); + + child.emit('exit', 1); + + await assertion; + }); + + it('still rejects an unbounded call when the WSM process fails at the pipe level', async () => { + const { client, child } = await connectFake(30_000); + + const pending = client.mergeConflicts({ dryRun: true }, NO_REQUEST_TIMEOUT); + const assertion = expect(pending).rejects.toThrow(/stdin error: broken pipe/); - // Well past the 30s client default - must NOT have rejected yet. - await vi.advanceTimersByTimeAsync(120_000); - // ...and now past the override. - await vi.advanceTimersByTimeAsync(600_000); + child.stdin.emit('error', new Error('broken pipe')); await assertion; }); @@ -96,14 +128,13 @@ describe('WsmMcpClient request deadlines', () => { await assertion; }); - // The override must stay scoped to the one call. If it leaked onto the client it would - // also bound the initialize handshake, and a WSM process that fails to start would hang - // for the full merge-sized deadline instead of failing fast. - it('does not let a per-call override leak into later calls on the same client', async () => { + // The unbounded wait must stay scoped to the one call. If it leaked onto the client it + // would also cover the initialize handshake, and a WSM process that fails to start would + // hang forever instead of failing fast. + it('does not let an unbounded call stop later calls on the same client from timing out', async () => { const { client } = await connectFake(30_000); - const long = client.mergeConflicts({ dryRun: true }, 600_000); - const longAssertion = expect(long).rejects.toThrow(/timed out after 600000ms/); + const unbounded = track(client.mergeConflicts({ dryRun: true }, NO_REQUEST_TIMEOUT)); const short = client.getStatus(); const shortAssertion = expect(short).rejects.toThrow(/timed out after 30000ms/); @@ -111,8 +142,7 @@ describe('WsmMcpClient request deadlines', () => { await vi.advanceTimersByTimeAsync(30_000); await shortAssertion; - await vi.advanceTimersByTimeAsync(600_000); - await longAssertion; + expect(unbounded.settled).toBe(false); }); it('uses the client default for the initialize handshake', async () => { diff --git a/vortex-extension/src/mcpClient.ts b/vortex-extension/src/mcpClient.ts index 151c937..9da7b45 100644 --- a/vortex-extension/src/mcpClient.ts +++ b/vortex-extension/src/mcpClient.ts @@ -34,6 +34,18 @@ const MCP_PROTOCOL_VERSION = '2025-06-18'; const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; +/** + * Pass as a call's `timeoutMs` to wait indefinitely instead of applying a deadline. + * + * Safe because this client does not rely on a wall-clock deadline to notice a dead + * server: the constructor wires `failAllPending` to the child's `exit`, `error` and + * stdin-`error` events, so a WSM process that crashes, is killed, or closes its pipes + * rejects every in-flight request immediately with a `WsmMcpProcessError`. What a + * deadline adds on top of that is only the ability to give up on a process that is alive + * and still working - which, for a merge, is precisely the case you must not abandon. + */ +export const NO_REQUEST_TIMEOUT = null; + /** How much of the child process's stderr to retain for diagnostics on failure. */ const STDERR_TAIL_LIMIT = 4000; @@ -217,7 +229,10 @@ interface McpToolCallResult { interface PendingRequest { resolve: (value: unknown) => void; reject: (reason: unknown) => void; - timer: ReturnType; + /** `undefined` for a request made with `NO_REQUEST_TIMEOUT` - there is no timer to + * clear when it settles. `clearTimeout(undefined)` is a no-op, so every call site + * stays unchanged. */ + timer: ReturnType | undefined; } export class WsmMcpClient { @@ -323,16 +338,17 @@ export class WsmMcpClient { } /** - * `timeoutMs` overrides this client's `requestTimeoutMs` for this one call. Use it for a - * tool whose runtime scales with the user's data rather than being bounded by round-trip - * latency - `merge_conflicts` above all (see `resolveAction.ts`'s - * `MERGE_CALL_TIMEOUT_MS`). Overriding per-call, rather than raising the whole client's - * `requestTimeoutMs` at `connect` time, is deliberate: that value also bounds the - * `initialize` handshake, and a handshake that hasn't answered in a few seconds means a - * WSM process that failed to start, not one that's working hard - it should fail fast - * instead of inheriting a merge-sized deadline. + * `timeoutMs` overrides this client's `requestTimeoutMs` for this one call; pass + * `NO_REQUEST_TIMEOUT` (null) to wait indefinitely. Use the latter for a tool whose + * runtime is the user's data rather than a round trip - `merge_conflicts` above all (see + * `resolveAction.ts`'s `MERGE_CALL_TIMEOUT`). + * + * Overriding per-call, rather than changing the whole client's `requestTimeoutMs` at + * `connect` time, is deliberate: that value also bounds the `initialize` handshake, and a + * handshake that hasn't answered in a few seconds means a WSM process that failed to + * start, not one that's working hard - it should keep failing fast. */ - async callTool(name: string, args?: Record, timeoutMs?: number): Promise { + async callTool(name: string, args?: Record, timeoutMs?: number | null): Promise { const result = (await this.request('tools/call', { name, arguments: args ?? {}, @@ -369,7 +385,7 @@ export class WsmMcpClient { return this.callTool('scan_conflicts'); } - mergeConflicts(args?: MergeConflictsArgs, timeoutMs?: number): Promise { + mergeConflicts(args?: MergeConflictsArgs, timeoutMs?: number | null): Promise { return this.callTool('merge_conflicts', args as Record | undefined, timeoutMs); } @@ -422,16 +438,20 @@ export class WsmMcpClient { } } - private request(method: string, params?: unknown, timeoutMs?: number): Promise { + private request(method: string, params?: unknown, timeoutMs?: number | null): Promise { const id = this.nextId++; const message: JsonRpcRequest = { jsonrpc: '2.0', id, method, params }; - const deadlineMs = timeoutMs ?? this.requestTimeoutMs; + // `undefined` means "use the client default"; `NO_REQUEST_TIMEOUT` (null) means + // "no deadline at all" - see its own comment for why that's safe here. + const deadlineMs = timeoutMs === undefined ? this.requestTimeoutMs : timeoutMs; return new Promise((resolve, reject) => { - const timer = setTimeout(() => { - this.pending.delete(id); - reject(new Error(`WSM MCP request '${method}' timed out after ${deadlineMs}ms`)); - }, deadlineMs); + const timer = deadlineMs === NO_REQUEST_TIMEOUT + ? undefined + : setTimeout(() => { + this.pending.delete(id); + reject(new Error(`WSM MCP request '${method}' timed out after ${deadlineMs}ms`)); + }, deadlineMs); this.pending.set(id, { resolve, reject, timer }); this.writeMessage(message); diff --git a/vortex-extension/src/resolveAction.test.ts b/vortex-extension/src/resolveAction.test.ts index c754c01..1ee6d22 100644 --- a/vortex-extension/src/resolveAction.test.ts +++ b/vortex-extension/src/resolveAction.test.ts @@ -118,7 +118,7 @@ function fakeConnect(outcomes: Array<{ result?: MergeConflictsResult; error?: Er const calls: WsmMcpClientOptions[] = []; // Every mergeConflicts invocation, so a test can assert what deadline the call was // given - see the MERGE_CALL_TIMEOUT_MS tests below. - const mergeCalls: Array<{ args: unknown; timeoutMs: number | undefined }> = []; + const mergeCalls: Array<{ args: unknown; timeoutMs: number | null | undefined }> = []; let closedCount = 0; let callIndex = 0; @@ -126,7 +126,7 @@ function fakeConnect(outcomes: Array<{ result?: MergeConflictsResult; error?: Er calls.push(options); const outcome = outcomes[callIndex++]; return { - mergeConflicts: async (args?: unknown, timeoutMs?: number) => { + mergeConflicts: async (args?: unknown, timeoutMs?: number | null) => { mergeCalls.push({ args, timeoutMs }); if (outcome.error) { throw outcome.error; @@ -357,23 +357,23 @@ describe('resolveScriptConflicts', () => { // mcpClient.ts's general-purpose 30s DEFAULT_REQUEST_TIMEOUT_MS, the dry-run preview // exceeded it, and resolveScriptConflicts bailed at the preview stage - the real merge // never ran, leaving an unmerged mod0000_MergedFiles and a game that wouldn't start. - // Both calls must carry the long, merge-sized deadline: a preview does the same - // scan-and-merge computation as the real merge and only skips the writes, so sizing it - // as if it were cheap is precisely what broke. - const TEN_MINUTES_MS = 10 * 60 * 1000; - - it('gives the dry-run preview a merge-sized timeout, not the general-purpose request default', async () => { + // + // Both calls must now carry NO deadline (null): a merge's runtime is the user's load + // order, so any wall-clock limit can only ever fire on a merge that is working normally. + // A preview is not the cheap one either - it does the same scan-and-merge computation and + // only skips the writes. + it('gives the dry-run preview no deadline, not the general-purpose request default', async () => { const { api } = fakeApi({ toolPath: 'C:\\wsm\\WitcherScriptMerger.Headless.exe' }); const { connect, mergeCalls } = fakeConnect([{ result: mergeResult() }]); await resolveScriptConflicts(api, { connect }); expect(mergeCalls).toHaveLength(1); - expect(mergeCalls[0].timeoutMs).toBe(TEN_MINUTES_MS); + expect(mergeCalls[0].timeoutMs).toBeNull(); expect((mergeCalls[0].args as { dryRun?: boolean }).dryRun).toBe(true); }); - it('gives the real merge the same merge-sized timeout as the preview', async () => { + it('gives the real merge no deadline either', async () => { const { api } = fakeApi({ toolPath: 'C:\\wsm\\WitcherScriptMerger.Headless.exe', dialogResponses: [{ action: 'Merge Now' }], @@ -385,14 +385,14 @@ describe('resolveScriptConflicts', () => { await resolveScriptConflicts(api, { connect }); expect(mergeCalls).toHaveLength(2); - expect(mergeCalls[0].timeoutMs).toBe(TEN_MINUTES_MS); - expect(mergeCalls[1].timeoutMs).toBe(TEN_MINUTES_MS); + expect(mergeCalls[0].timeoutMs).toBeNull(); + expect(mergeCalls[1].timeoutMs).toBeNull(); expect((mergeCalls[1].args as { dryRun?: boolean }).dryRun).toBe(false); }); - // The long deadline belongs to the merge call alone. connect() must NOT be handed it as + // The unbounded wait belongs to the merge call alone. connect() must NOT be handed it as // the client-wide requestTimeoutMs, which also bounds the initialize handshake - a WSM - // process that fails to start should still fail fast rather than hang for ten minutes. + // process that fails to start must still fail fast rather than hang forever. it('does not widen the client-wide request timeout (the initialize handshake must still fail fast)', async () => { const { api } = fakeApi({ toolPath: 'C:\\wsm\\WitcherScriptMerger.Headless.exe' }); const { connect, calls } = fakeConnect([{ result: mergeResult() }]); @@ -402,11 +402,11 @@ describe('resolveScriptConflicts', () => { expect(calls[0].requestTimeoutMs).toBeUndefined(); }); - it('explains that nothing was merged when the merge call times out, rather than surfacing a bare transport error', async () => { + it('explains that nothing was merged when a call times out, rather than surfacing a bare transport error', async () => { const { api, errorNotifications } = fakeApi({ toolPath: 'C:\\wsm\\WitcherScriptMerger.Headless.exe', }); - const timeout = new Error("WSM MCP request 'tools/call' timed out after 600000ms"); + const timeout = new Error("WSM MCP request 'initialize' timed out after 30000ms"); const { connect } = fakeConnect([{ error: timeout }]); await resolveScriptConflicts(api, { connect }); @@ -414,7 +414,7 @@ describe('resolveScriptConflicts', () => { expect(errorNotifications).toHaveLength(1); const shown = errorNotifications[0].message; expect(shown).toContain('nothing was merged'); - expect(shown).toContain('10 minutes'); + expect(shown).toContain('as long as it needs'); // the original error is still handed over as the detail, not swallowed expect(errorNotifications[0].detail).toBe(timeout); }); diff --git a/vortex-extension/src/resolveAction.ts b/vortex-extension/src/resolveAction.ts index 391e8d5..e14bc2c 100644 --- a/vortex-extension/src/resolveAction.ts +++ b/vortex-extension/src/resolveAction.ts @@ -2,7 +2,7 @@ import { log, selectors, types } from 'vortex-api'; import { checkCoexistenceDrift, computeMergeStateSnapshot, recordOwnMergeStateSnapshot } from './coexistenceGuard'; import { WSM_TOOL_ID } from './discoveredTool'; import { isWitcher3Active, WITCHER3_GAME_ID } from './gating'; -import { GetStatusResult, ListMergesResult, MergeConflictsArgs, MergeConflictsResult, WsmMcpClient, WsmMcpClientOptions } from './mcpClient'; +import { GetStatusResult, ListMergesResult, MergeConflictsArgs, MergeConflictsResult, NO_REQUEST_TIMEOUT, WsmMcpClient, WsmMcpClientOptions } from './mcpClient'; import { buildMergeSummaryDialogContent } from './mergePanel'; import { mergeWithProcessEnv } from './wsmEnv'; @@ -43,6 +43,39 @@ import { mergeWithProcessEnv } from './wsmEnv'; const ACTIVITY_NOTIFICATION_ID = 'witcherscriptmerger-vortex-resolve-conflicts-activity'; +/** + * A `merge_conflicts` call - preview or real merge - gets **no deadline at all**. + * + * `mcpClient.ts`'s general-purpose `DEFAULT_REQUEST_TIMEOUT_MS` (30s) used to apply here, + * and it was not merely too short - a wall-clock limit is the wrong instrument for this + * call. A merge's runtime is a function of the user's load order, with no bound anyone can + * pick in advance, so any deadline fires *only* on a merge that is running normally and + * simply needs longer. The failure it caused was severe and silent: on a real 274-mod load + * order with 44 conflicting script files the dry-run preview alone blew past 30s, so + * `resolveScriptConflicts` reported "Failed to preview script-conflict merges" and + * returned at the preview stage - the real merge below it never ran. The user was left + * with an unmerged (and, because a prior deploy had already cleared it, empty) + * `mod0000_MergedFiles` and a game that would not start, with ~200 script compile errors + * naming members the merge is supposed to add to vanilla scripts. + * + * Note this applies to the dry-run preview just as much as the real merge: a preview does + * the entire scan-and-three-way-merge computation and only skips the writes, so it costs + * essentially the same time as the merge it previews. Treating it as the cheap one is what + * produced the failure above. + * + * Waiting indefinitely is safe because liveness, not the clock, is what detects a dead + * server: `WsmMcpClient` fails every in-flight request the moment the child process exits, + * errors, or closes its pipes (see `NO_REQUEST_TIMEOUT`). The only case a deadline would + * still catch is a WSM process that is alive but wedged - and for that, an + * unbounded wait plus a visible activity notification is better than silently abandoning a + * merge mid-flight, which is exactly how the merged mod ended up empty. + * + * Deliberately applied per call, NOT as `connect`'s client-wide `requestTimeoutMs`: that + * also bounds the `initialize` handshake, where a few seconds of silence means WSM failed + * to start and should still fail fast. + */ +const MERGE_CALL_TIMEOUT = NO_REQUEST_TIMEOUT; + /** The subset of `WsmMcpClient` this file actually needs - lets unit tests inject a * fake without spawning a real WSM process (mirrors `toolAcquisition.ts`'s own * `client`/`extractor` test seams). @@ -54,33 +87,9 @@ const ACTIVITY_NOTIFICATION_ID = 'witcherscriptmerger-vortex-resolve-conflicts-a * needs both. Reusing the same already-connected client (rather than opening a third * one just for this) is the entire point - see that function's own doc comment on why * it takes a client instead of connecting its own. */ -/** - * How long to allow a single `merge_conflicts` MCP call - preview or real merge - before - * giving up. Ten minutes, matching `nexusDownloader.ts`'s `DEFAULT_DOWNLOAD_TIMEOUT_MS` - * precedent for "an operation whose runtime is the user's data, not a round trip". - * - * `mcpClient.ts`'s general-purpose `DEFAULT_REQUEST_TIMEOUT_MS` (30s) is far too short - * here, and this is not theoretical: on a real 274-mod load order with 44 conflicting - * script files, the *preview* alone blew past 30s, so `resolveScriptConflicts` reported - * "Failed to preview script-conflict merges" and returned at the preview stage - the real - * merge below it never ran at all. The user was left with an unmerged (and, because a - * prior deploy had already cleared it, empty) `mod0000_MergedFiles` and a game that - * refused to start, with ~200 script compile errors naming members the merge is supposed - * to add to vanilla scripts. - * - * Note this applies to the dry-run preview just as much as the real merge: a preview does - * the entire scan-and-three-way-merge computation and only skips the writes, so it costs - * essentially the same time as the merge it is previewing. Sizing this off "it's only a - * preview" would reintroduce the same bug. - * - * Deliberately NOT passed to `connect` as the client-wide `requestTimeoutMs`: that also - * bounds the `initialize` handshake, which should still fail fast when WSM can't start - - * see `WsmMcpClient.callTool`'s own comment on the per-call override. - */ -const MERGE_CALL_TIMEOUT_MS = 10 * 60 * 1000; export interface WsmMergeClient { - mergeConflicts(args?: MergeConflictsArgs, timeoutMs?: number): Promise; + mergeConflicts(args?: MergeConflictsArgs, timeoutMs?: number | null): Promise; getStatus(): Promise; listMerges(): Promise; close(): Promise; @@ -240,7 +249,7 @@ async function runMergeConflictsWorkflow( try { const client = await connect({ exePath, env }); try { - const result = await client.mergeConflicts(args, MERGE_CALL_TIMEOUT_MS); + const result = await client.mergeConflicts(args, MERGE_CALL_TIMEOUT); // 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 @@ -299,13 +308,12 @@ async function runMergeConflictsWorkflow( } /** - * A timed-out `merge_conflicts` call reads, on its own, as a bare transport error - * ("request 'tools/call' timed out after ...ms") that says nothing about what it means for - * the user's install. It means specifically that *no merge was written* - and if a deploy - * had already cleared the merged mod, that leaves the game unable to start, which is a far - * worse outcome than the wording suggests. `isTimeout` spots that case so `reportFailure` - * can say so, and point at the one thing that actually helps (a huge load order simply - * needing longer than `MERGE_CALL_TIMEOUT_MS`). + * With `MERGE_CALL_TIMEOUT` unbounded, a timeout can no longer come from the merge itself + * - only from the short-deadline calls around it, above all the `initialize` handshake, + * which means the WSM process never came up. On its own that surfaces as a bare transport + * error ("request 'initialize' timed out after 30000ms") that says nothing about what it + * means for the install: no merge ran, and if a deploy had already cleared the merged mod, + * the game may not start. `isTimeout` spots that case so `reportFailure` can say so. */ function isTimeout(err: unknown): boolean { return err instanceof Error && /timed out after \d+ms/.test(err.message); @@ -319,9 +327,9 @@ function reportFailure(api: types.IExtensionApi, message: string, err: unknown): timedOut: isTimeout(err), }); const shown = isTimeout(err) - ? `${message}: Script Merger did not finish within ${Math.round(MERGE_CALL_TIMEOUT_MS / 60000)} minutes, so nothing was merged. ` + ? `${message}: Script Merger did not respond, so nothing was merged. ` + 'Your merged mod has been left untouched - if it is empty or out of date, the game may fail to start until a merge completes. ' - + 'Very large load orders can need longer; running Script Merger directly will also do the merge.' + + 'The merge itself is allowed to run for as long as it needs, so this points at Script Merger failing to start rather than being slow.' : message; api.showErrorNotification?.(shown, detail); }