diff --git a/apps/desktop/electron/connection-config.cjs b/apps/desktop/electron/connection-config.cjs index 0704cd6a2..c10a657d1 100644 --- a/apps/desktop/electron/connection-config.cjs +++ b/apps/desktop/electron/connection-config.cjs @@ -65,6 +65,59 @@ function buildGatewayWsUrlWithTicket(baseUrl, ticket) { return `${wsScheme}://${parsed.host}${prefix}/api/ws?ticket=${encodeURIComponent(ticket)}` } +/** + * Build the WS URL the renderer would connect with, so the connection test can + * exercise the same transport the app actually uses. + * + * The OAuth ticket-minter is injected (`mintTicket(baseUrl) -> Promise`) + * so this stays electron-free and unit-testable; main.cjs passes the real + * `mintGatewayWsTicket`. + * + * Return semantics: + * - token mode + token → ws(s)://…/api/ws?token=… + * - token mode, no token → null (genuine skip; nothing to authenticate with) + * - oauth, mint ok → ws(s)://…/api/ws?ticket=… + * - oauth, mint fails → THROWS (NOT a skip) + * + * The oauth-mint-failure throw is the important case: the real boot path + * (resolveRemoteBackend in main.cjs) treats a mint failure as a hard + * "session expired" auth error and refuses to connect. Swallowing it here + * would re-introduce the exact false-positive this test exists to catch — + * HTTP /api/status passes, the test reports "reachable", then the renderer + * can't authenticate /api/ws and boot dies with "Could not connect". + * + * @param {string} baseUrl + * @param {'token'|'oauth'} authMode + * @param {string|null} token + * @param {{ mintTicket: (baseUrl: string) => Promise }} deps + * @returns {Promise} + */ +async function resolveTestWsUrl(baseUrl, authMode, token, deps = {}) { + if (authMode === 'oauth') { + const mintTicket = deps.mintTicket + if (typeof mintTicket !== 'function') { + throw new Error('resolveTestWsUrl: a mintTicket function is required in OAuth mode.') + } + let ticket + try { + ticket = await mintTicket(baseUrl) + } catch (error) { + const err = new Error( + 'Reached the gateway over HTTP, but could not mint a WebSocket ticket for the OAuth session ' + + '(it may have expired). Open Settings → Gateway and sign in again.' + ) + err.needsOauthLogin = true + err.cause = error + throw err + } + return buildGatewayWsUrlWithTicket(baseUrl, ticket) + } + if (!token) { + return null + } + return buildGatewayWsUrl(baseUrl, token) +} + function tokenPreview(value) { const raw = String(value || '') @@ -114,5 +167,6 @@ module.exports = { cookiesHaveSession, normalizeRemoteBaseUrl, resolveAuthMode, + resolveTestWsUrl, tokenPreview } diff --git a/apps/desktop/electron/connection-config.test.cjs b/apps/desktop/electron/connection-config.test.cjs index 3182bd2ff..33f81f3b6 100644 --- a/apps/desktop/electron/connection-config.test.cjs +++ b/apps/desktop/electron/connection-config.test.cjs @@ -21,6 +21,7 @@ const { cookiesHaveSession, normalizeRemoteBaseUrl, resolveAuthMode, + resolveTestWsUrl, tokenPreview } = require('./connection-config.cjs') @@ -159,3 +160,52 @@ test('tokenPreview returns set for short tokens', () => { test('tokenPreview returns a masked suffix for long tokens', () => { assert.equal(tokenPreview('abcdefghijklmnop'), '...klmnop') }) + +// --- resolveTestWsUrl --- +// +// The "Test remote" button must exercise the same WS transport the app uses, +// and must FAIL (not skip) when an OAuth session can't mint a ws-ticket — that +// is the exact false-positive PR #39098 set out to eliminate. + +test('resolveTestWsUrl (token mode) builds a ?token= URL the WS probe can use', async () => { + const url = await resolveTestWsUrl('https://gw.example.com', 'token', 'tok123') + assert.equal(url, 'wss://gw.example.com/api/ws?token=tok123') +}) + +test('resolveTestWsUrl (token mode, no token) returns null — genuine skip', async () => { + assert.equal(await resolveTestWsUrl('https://gw.example.com', 'token', null), null) +}) + +test('resolveTestWsUrl (oauth, mint ok) builds a ?ticket= URL', async () => { + const url = await resolveTestWsUrl('https://gw.example.com', 'oauth', null, { + mintTicket: async () => 'tkt-9' + }) + assert.equal(url, 'wss://gw.example.com/api/ws?ticket=tkt-9') +}) + +test('resolveTestWsUrl (oauth, mint FAILS) throws — must NOT skip WS validation', async () => { + await assert.rejects( + () => + resolveTestWsUrl('https://gw.example.com', 'oauth', null, { + mintTicket: async () => { + throw new Error('401 ticket mint failed') + } + }), + err => { + // Actionable, points the user at re-auth, and preserves the cause + flag + // the boot overlay uses to offer a sign-in prompt. + assert.match(err.message, /WebSocket ticket/i) + assert.match(err.message, /sign in again/i) + assert.equal(err.needsOauthLogin, true) + assert.ok(err.cause instanceof Error) + return true + } + ) +}) + +test('resolveTestWsUrl (oauth) requires a mintTicket function', async () => { + await assert.rejects( + () => resolveTestWsUrl('https://gw.example.com', 'oauth', null), + /mintTicket function is required/ + ) +}) diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index 5792b1dc2..0a877719e 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -35,6 +35,7 @@ const { cookiesHaveSession, normalizeRemoteBaseUrl, resolveAuthMode, + resolveTestWsUrl, tokenPreview } = require('./connection-config.cjs') const { @@ -3770,7 +3771,7 @@ async function testDesktopConnectionConfig(input = {}) { // false-positive "reachable" while the real boot still failed with "Could not // connect to Hermes gateway". Mirror the renderer's connect here so the test // reflects the full path the app actually uses. - const wsUrl = await resolveTestWsUrl(baseUrl, authMode, token) + const wsUrl = await resolveTestWsUrl(baseUrl, authMode, token, { mintTicket: mintGatewayWsTicket }) // Skip the WS leg only when the runtime genuinely lacks a WebSocket (so an // older Electron/Node never fails the test spuriously); Electron's main // process ships a global WebSocket on every supported version. @@ -3791,26 +3792,6 @@ async function testDesktopConnectionConfig(input = {}) { } } -// Build the WS URL the renderer would connect with, so the connection test can -// exercise the same transport. OAuth gateways need a freshly minted single-use -// ticket; token/local gateways carry a long-lived token in the query string. A -// null return means we can't form a credentialed URL (e.g. OAuth without a live -// session) and the WS leg of the test is skipped rather than failing spuriously. -async function resolveTestWsUrl(baseUrl, authMode, token) { - if (authMode === 'oauth') { - try { - const ticket = await mintGatewayWsTicket(baseUrl) - return buildGatewayWsUrlWithTicket(baseUrl, ticket) - } catch { - return null - } - } - if (!token) { - return null - } - return buildGatewayWsUrl(baseUrl, token) -} - function resetBootProgressForReconnect() { updateBootProgress( {