diff --git a/apps/desktop/electron/cloud-session-recovery.test.ts b/apps/desktop/electron/cloud-session-recovery.test.ts new file mode 100644 index 0000000000..72d676f3c7 --- /dev/null +++ b/apps/desktop/electron/cloud-session-recovery.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it, vi } from 'vitest' + +import { createCloudSessionRecovery } from './cloud-session-recovery' + +const cloud = 'https://test-agent.agents.nousresearch.com' +const rejection = (statusCode = 401) => Object.assign(new Error('rejected'), { statusCode }) + +describe('Cloud cookie session recovery', () => { + it('recovers an expired cookie session before minting a new ticket', async () => { + const restore = vi.fn(async () => true) + const run = createCloudSessionRecovery({ hasNativeSession: () => false, restoreCookieSession: restore }) + const mint = vi.fn().mockRejectedValueOnce(rejection()).mockResolvedValueOnce('fresh-ticket') + expect(await run(cloud, mint)).toBe('fresh-ticket') + expect(restore).toHaveBeenCalledExactlyOnceWith(cloud) + expect(mint).toHaveBeenCalledTimes(2) + }) + + it('does not recover non-Cloud, non-401 or native-session failures', async () => { + const restore = vi.fn(async () => true) + + const run = createCloudSessionRecovery({ + hasNativeSession: url => url.endsWith('native.agents.nousresearch.com'), + restoreCookieSession: restore + }) + + for (const [url, status] of [ + ['https://example.com', 401], + [cloud, 403], + [cloud, 500], + ['https://native.agents.nousresearch.com', 401] + ] as const) { + const error = rejection(status) + await expect( + run(url, async () => { + throw error + }) + ).rejects.toBe(error) + } + + expect(restore).not.toHaveBeenCalled() + }) + + it('coalesces recovery without reusing single-use tickets', async () => { + let release!: (restored: boolean) => void + + const restore = vi.fn( + () => + new Promise(resolve => { + release = resolve + }) + ) + + const run = createCloudSessionRecovery({ hasNativeSession: () => false, restoreCookieSession: restore }) + const firstMint = vi.fn().mockRejectedValueOnce(rejection()).mockResolvedValueOnce('ticket-1') + const secondMint = vi.fn().mockRejectedValueOnce(rejection()).mockResolvedValueOnce('ticket-2') + const first = run(cloud, firstMint) + const second = run(cloud, secondMint) + await vi.waitFor(() => expect(restore).toHaveBeenCalledTimes(1)) + release(true) + expect(await Promise.all([first, second])).toEqual(['ticket-1', 'ticket-2']) + }) + + it('does not retry under a native identity selected during background recovery', async () => { + let native = false + + const restore = vi.fn(async () => { + native = true + + return true + }) + + const run = createCloudSessionRecovery({ hasNativeSession: () => native, restoreCookieSession: restore }) + const error = rejection() + + const mint = vi.fn(async () => { + throw error + }) + + await expect(run(cloud, mint)).rejects.toBe(error) + expect(mint).toHaveBeenCalledTimes(1) + }) + + it('backs off failed recovery and bounds a successful recovery to one retry', async () => { + let clock = 0 + const restore = vi.fn().mockResolvedValueOnce(false).mockResolvedValue(true) + + const run = createCloudSessionRecovery({ + hasNativeSession: () => false, + restoreCookieSession: restore, + now: () => clock + }) + + const error = rejection() + + const mint = vi.fn(async () => { + throw error + }) + + await expect(run(cloud, mint)).rejects.toBe(error) + await expect(run(cloud, mint)).rejects.toBe(error) + expect(restore).toHaveBeenCalledTimes(1) + clock = 60_001 + await expect(run(cloud, mint)).rejects.toBe(error) + expect(restore).toHaveBeenCalledTimes(2) + expect(mint).toHaveBeenCalledTimes(4) + }) +}) diff --git a/apps/desktop/electron/cloud-session-recovery.ts b/apps/desktop/electron/cloud-session-recovery.ts new file mode 100644 index 0000000000..afcd887578 --- /dev/null +++ b/apps/desktop/electron/cloud-session-recovery.ts @@ -0,0 +1,76 @@ +import { isNousCloudAgentUrl } from './backend-health' + +interface CloudRecoveryDeps { + hasNativeSession: (baseUrl: string) => boolean + restoreCookieSession: (baseUrl: string) => Promise + onRecovered?: (baseUrl: string) => void + now?: () => number +} + +/** Coalesce the cookie refresh, never the single-use ticket minted afterward. */ +export function createCloudSessionRecovery(deps: CloudRecoveryDeps) { + const inflight = new Map>() + const failedAt = new Map() + const now = deps.now ?? Date.now + + return async (baseUrl: string, mint: () => Promise): Promise => { + const hadNativeSession = deps.hasNativeSession(baseUrl) + + try { + return await mint() + } catch (error) { + if ( + !isNousCloudAgentUrl(baseUrl) || + hadNativeSession || + deps.hasNativeSession(baseUrl) || + (error as { statusCode?: number })?.statusCode !== 401 + ) { + throw error + } + + let recovery = inflight.get(baseUrl) + + if (!recovery) { + const failed = failedAt.get(baseUrl) + + if (failed !== undefined && now() - failed < 60_000) { + throw error + } + + recovery = Promise.resolve() + .then(() => deps.restoreCookieSession(baseUrl)) + .catch(() => false) + .then(restored => { + if (!restored) { + failedAt.set(baseUrl, now()) + } + + return restored + }) + .finally(() => inflight.delete(baseUrl)) + inflight.set(baseUrl, recovery) + } + + if (!(await recovery)) { + throw error + } + + // Native login may have changed while the background cookie flow ran. + // Never turn that transition into a cross-identity fallback. + if (deps.hasNativeSession(baseUrl)) { + throw error + } + + try { + const ticket = await mint() + failedAt.delete(baseUrl) + deps.onRecovered?.(baseUrl) + + return ticket + } catch (retryError) { + failedAt.set(baseUrl, now()) + throw retryError + } + } + } +} diff --git a/apps/desktop/electron/connection-caches.test.ts b/apps/desktop/electron/connection-caches.test.ts index 4c665463f7..5c1e7cca40 100644 --- a/apps/desktop/electron/connection-caches.test.ts +++ b/apps/desktop/electron/connection-caches.test.ts @@ -5,6 +5,7 @@ import { beforeEach, test } from 'vitest' import { connectionInstallIds, evictConnectionCaches, + rosterSourceErrors, sshInventoryAttemptedAt, sshRosterCache } from './connection-caches' @@ -14,12 +15,14 @@ beforeEach(() => { sshRosterCache.clear() sshInventoryAttemptedAt.clear() connectionInstallIds.clear() + rosterSourceErrors.clear() }) function seed(id: string) { sshRosterCache.set(id, ['default', 'dixie']) sshInventoryAttemptedAt.set(id, Date.now()) connectionInstallIds.set(id, { id: 'aaa', ts: Date.now() }) + rosterSourceErrors.set(id, 'previous failure') } test('evicting a connection id forgets every cache keyed by it', () => { @@ -33,10 +36,12 @@ test('evicting a connection id forgets every cache keyed by it', () => { assert.equal(sshRosterCache.has('mac-mini'), false) assert.equal(sshInventoryAttemptedAt.has('mac-mini'), false) assert.equal(connectionInstallIds.has('mac-mini'), false) + assert.equal(rosterSourceErrors.has('mac-mini'), false) // Its neighbours are untouched. assert.deepEqual(sshRosterCache.get('spark'), ['default', 'dixie']) assert.equal(connectionInstallIds.get('spark')?.id, 'aaa') + assert.equal(rosterSourceErrors.get('spark'), 'previous failure') }) test('an evicted id enumerates from the live target again instead of serving the old one', () => { @@ -66,4 +71,5 @@ test('evicting an unknown or empty id is a no-op', () => { assert.equal(sshRosterCache.size, 1) assert.equal(sshInventoryAttemptedAt.size, 1) assert.equal(connectionInstallIds.size, 1) + assert.equal(rosterSourceErrors.size, 1) }) diff --git a/apps/desktop/electron/connection-caches.ts b/apps/desktop/electron/connection-caches.ts index ac02b2aa49..fc15660fe2 100644 --- a/apps/desktop/electron/connection-caches.ts +++ b/apps/desktop/electron/connection-caches.ts @@ -24,7 +24,15 @@ export const sshInventoryAttemptedAt = new Map() */ export const connectionInstallIds = new Map() -const CONNECTION_SCOPED_CACHES: Map[] = [sshRosterCache, sshInventoryAttemptedAt, connectionInstallIds] +/** Last logged roster failure per connection, so repeat polls do not spam the log. */ +export const rosterSourceErrors = new Map() + +const CONNECTION_SCOPED_CACHES: Map[] = [ + sshRosterCache, + sshInventoryAttemptedAt, + connectionInstallIds, + rosterSourceErrors +] /** * Forget everything cached about a connection id. Call whenever that id stops naming the machine diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 1d59212503..0f5f9d277d 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -114,12 +114,14 @@ import { provisionCliLinks } from './cli-provision' import { closeStopFailureMessage, finishWindowsCloseStop, type RuntimeLock } from './close-stop-kill' import { shouldAttemptCloudBootCascade } from './cloud-boot-cascade' import { discoverWithTeamFallback } from './cloud-discovery' +import { createCloudSessionRecovery } from './cloud-session-recovery' import { installCommandScreenshot } from './command-screenshot' import { writeComposerPaste } from './composer-paste' import { applyConnectionChange, teardownSshState } from './connection-apply' import { connectionInstallIds, evictConnectionCaches, + rosterSourceErrors, sshInventoryAttemptedAt, sshRosterCache } from './connection-caches' @@ -453,6 +455,7 @@ import { planLaunchSwitches, readDesktopLaunchConfig } from './renderer-heap-fla import { loadRendererLoadErrorPage } from './renderer-load-error-page' import { attachRendererConsoleCapture, formatRendererBoundaryReport } from './renderer-log' import { fetchRosterSourceData } from './roster-source-fetch' +import { rosterSourceStatus } from './roster-source-status' import { classifyStoredSecret, readSecretStoragePolicy, @@ -7441,7 +7444,7 @@ async function clearOauthSession(baseUrl) { // ``/auth/login`` → portal ``/oauth/authorize`` (auto-approves org members) // → ``/auth/callback``, which sets the gateway cookie with NO interactive // prompt. This is the per-agent cloud cascade (decisions.md Q5). -function openOauthLoginWindow(baseUrl, { silent = false } = {}) { +function openOauthLoginWindow(baseUrl, { silent = false, background = false } = {}) { return new Promise((resolve, reject) => { if (!app.isReady()) { reject(new Error('Desktop is not ready to start an OAuth login.')) @@ -7461,6 +7464,7 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) { let win = null let pollTimer = null let revealTimer = null + let deadlineTimer = null const finish = err => { if (settled) { @@ -7477,6 +7481,10 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) { clearTimeout(revealTimer) } + if (deadlineTimer) { + clearTimeout(deadlineTimer) + } + try { if (win && !win.isDestroyed()) { win.destroy() @@ -7513,7 +7521,7 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) { // only reveal it as a fallback if the cascade DOESN'T complete quickly // (e.g. the portal session lapsed and the gate fell through to the // interactive chooser) — see the reveal timer below. - show: !silent, + show: !silent && !background, webPreferences: { contextIsolation: true, nodeIntegration: false, @@ -7545,7 +7553,7 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) { // loop-guard tripped, etc.) and the window is now showing an interactive // page. Reveal it so the user can complete sign-in manually rather than // staring at nothing. Cleared on finish(). - if (silent && win) { + if (silent && win && !background) { revealTimer = setTimeout(() => { try { if (!settled && win && !win.isDestroyed() && !win.isVisible()) { @@ -7557,6 +7565,10 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) { }, 2500) } + if (background) { + deadlineTimer = setTimeout(() => finish(new Error('Cloud session recovery requires sign-in.')), 12_000) + } + win.on('closed', () => { if (!settled) { finish(new Error('Login window closed before authentication completed.')) @@ -7576,6 +7588,14 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) { `OAuth login: attaching ${Object.keys(loginHeaders).length} extra gateway header(s) to ${new URL(normalizedBase).host}` ) win.loadURL(loginUrl, oauthLoginLoadUrlOptions(loginHeaders)).catch(error => { + // Callback navigation can abort the original load after setting cookies. + // Keep the bounded hidden recovery alive long enough to observe them. + if (background && (Number(error?.code) === -3 || /\bERR_ABORTED\b/.test(String(error?.message)))) { + void checkCookie() + + return + } + finish(error instanceof Error ? error : new Error(String(error))) }) }) @@ -7876,20 +7896,40 @@ async function readGatewayFileDataUrl(connection: GatewayFileConnection, request return dataUrl } -// Mint a single-use WS ticket for a gated gateway. Native bearer first (one -// forced rotation on a confirmed 401, #95701), OAuth cookie partition second. -// Transient transport blips (brief host unreachable, 5xx, timeouts) are retried -// a few times before failing — those 1-3s flaps were promoting into the -// full-screen "couldn't start" lockout on reconnect. Ticket POSTs are -// replay-safe; arbitrary REST mutations never use this retry loop. -async function mintGatewayWsTicket(baseUrl: string, headers: Record = {}): Promise { - return withTransientRetries( - (): Promise => - mintOauthGatewayWsTicket(baseUrl, { ensureNativeAccessToken, fetchJson, fetchJsonViaOauthSession }, headers), - { - isRetryable: (error: Error): boolean => - !(error instanceof NativeAuthChangedError) && !isGatewayAuthRejection(error) +// Recover a Cloud cookie only after a confirmed cookie-auth ticket 401. +// Each caller still mints its own single-use ticket after the shared recovery. +const recoverCloudCookieSession = createCloudSessionRecovery({ + hasNativeSession, + onRecovered: baseUrl => rememberLog(`[cloud] saved gateway session recovered for ${hostLabelFromBaseUrl(baseUrl)}`), + restoreCookieSession: async baseUrl => { + // The saved portal identity is the authority for cookie agent sessions. + // Roster polling must never reveal an interactive sign-in window. + if (!(await hasLivePortalSession())) { + return false } + + if (!(await hasPortalAccessToken()) && !(await renewPortalAccessSilently())) { + return false + } + + await openOauthLoginWindow(baseUrl, { silent: true, background: true }) + + return true + } +}) + +// Native bearer first (including one forced 401 rotation), OAuth cookie second. +// Transient ticket POST failures keep their bounded retry before Cloud recovery. +async function mintGatewayWsTicket(baseUrl: string, headers: Record = {}): Promise { + return recoverCloudCookieSession(baseUrl, () => + withTransientRetries( + (): Promise => + mintOauthGatewayWsTicket(baseUrl, { ensureNativeAccessToken, fetchJson, fetchJsonViaOauthSession }, headers), + { + isRetryable: (error: Error): boolean => + !(error instanceof NativeAuthChangedError) && !isGatewayAuthRejection(error) + } + ) ) } @@ -15706,7 +15746,7 @@ ipcMain.handle('hermes:connections:test', async (_event, id) => { // would spawn tunnels the user never asked for); once dialed, their pooled // descriptor serves the enumeration like any remote. Last-known SSH profile // lists are reused so switching the window back to local does not empty Bot Mode. -// These three live in ./connection-caches, which states (and tests) the invariant they share: +// Connection caches live in ./connection-caches, which states (and tests) the invariant they share: // each is keyed by connection id and is only valid while that id names the same machine, so // removing a connection or re-pointing it must evict them (`evictConnectionCaches`). const SSH_INVENTORY_RETRY_MS = 60_000 @@ -15837,9 +15877,12 @@ async function enumerateRegistryAgentSources(registry = readDesktopConnectionsRe return Promise.all( registry.connections.map(async connection => { + let sourceFailureDetail = '' + let raw: { connection: typeof connection error?: string + needsSignIn?: boolean installId?: string profiles: null | string[] profileMetadata?: Record @@ -15953,7 +15996,26 @@ async function enumerateRegistryAgentSources(registry = readDesktopConnectionsRe } } } catch (error: any) { - raw = { connection, profiles: null, error: String(error?.message || error) } + sourceFailureDetail = [error?.statusCode, error?.cause?.message].filter(Boolean).join(' | ') + raw = { + connection, + profiles: null, + error: redactSecrets(String(error?.message || error)), + needsSignIn: isReauthRequiredError(error) + } + } + + if (raw.error && raw.error !== 'connect-on-demand') { + const diagnostic = redactSecrets([raw.error, sourceFailureDetail].filter(Boolean).join(' | ')) + .replace(/[\r\n]+/g, ' ') + .slice(0, 800) + + if (rosterSourceErrors.get(connection.id) !== diagnostic) { + rememberLog(`[fleet-roster] ${connection.id}: ${diagnostic}`) + rosterSourceErrors.set(connection.id, diagnostic) + } + } else if (raw.profiles && rosterSourceErrors.delete(connection.id)) { + rememberLog(`[fleet-roster] ${connection.id}: connection recovered`) } if (raw.profiles && raw.profiles.length > 0) { @@ -15965,6 +16027,7 @@ async function enumerateRegistryAgentSources(registry = readDesktopConnectionsRe return { connection, ...remembered, + ...(raw.needsSignIn ? { needsSignIn: true } : {}), ...(raw.installId ? { installId: raw.installId } : {}), ...(raw.profileMetadata ? { profileMetadata: raw.profileMetadata } : {}) } @@ -15984,13 +16047,12 @@ ipcMain.handle('hermes:agents:roster', async () => { // instead of appending duplicates (remote-only desktops doubled every // bot otherwise; see #88344). primaryConnectionId: registry.primary, - sources: enumerations.map(({ connection, error, installId, profiles }) => ({ + sources: enumerations.map(({ connection, error, installId, profiles, needsSignIn }) => ({ connectionId: connection.id, label: connection.label, kind: connection.kind, - reachable: profiles !== null, - ...(installId ? { installId } : {}), - ...(error ? { error } : {}) + ...rosterSourceStatus({ profiles, error, needsSignIn }), + ...(installId ? { installId } : {}) })) } }) diff --git a/apps/desktop/electron/roster-source-status.test.ts b/apps/desktop/electron/roster-source-status.test.ts new file mode 100644 index 0000000000..5255f8b3df --- /dev/null +++ b/apps/desktop/electron/roster-source-status.test.ts @@ -0,0 +1,13 @@ +import { expect, it } from 'vitest' + +import { rosterSourceStatus } from './roster-source-status' + +it('does not report stale cached profiles as a reachable or signed-in gateway', () => { + expect(rosterSourceStatus({ profiles: ['default'], error: 'OAuth expired', needsSignIn: true })).toEqual({ + reachable: false, + error: 'OAuth expired', + needsSignIn: true + }) + expect(rosterSourceStatus({ profiles: ['default'], error: 'timed out' }).reachable).toBe(false) + expect(rosterSourceStatus({ profiles: ['default'] }).reachable).toBe(true) +}) diff --git a/apps/desktop/electron/roster-source-status.ts b/apps/desktop/electron/roster-source-status.ts new file mode 100644 index 0000000000..4cf433c181 --- /dev/null +++ b/apps/desktop/electron/roster-source-status.ts @@ -0,0 +1,7 @@ +export function rosterSourceStatus(source: { profiles: string[] | null; error?: string; needsSignIn?: boolean }) { + return { + reachable: source.profiles !== null && (!source.error || source.error === 'connect-on-demand'), + ...(source.error ? { error: source.error } : {}), + ...(source.needsSignIn ? { needsSignIn: true } : {}) + } +} diff --git a/apps/desktop/src/app/chat/sidebar/fleet-rail.test.ts b/apps/desktop/src/app/chat/sidebar/fleet-rail.test.ts index f6b9a11ea9..db226e67fd 100644 --- a/apps/desktop/src/app/chat/sidebar/fleet-rail.test.ts +++ b/apps/desktop/src/app/chat/sidebar/fleet-rail.test.ts @@ -95,6 +95,22 @@ describe('buildRestGroups', () => { expect(vps?.named).toEqual([]) }) + it('keeps an expired Cloud source visible but not reachable even with cached profiles', () => { + const expired: DesktopAgentRoster = { + ...roster, + sources: roster.sources.map(source => + source.connectionId === 'pandora' + ? { ...source, reachable: false, error: 'OAuth expired', needsSignIn: true } + : source + ) + } + + const groups = buildRestGroups({ activeConnectionId: 'local', connections, roster: expired }) + const cloud = groups.find(group => group.connectionId === 'pandora') + expect(cloud).toMatchObject({ reachable: false, error: 'OAuth expired', needsSignIn: true }) + expect(cloud?.named.map(agent => agent.profile)).toEqual(['omer', 'scout']) + }) + it('shows every gateway with just its default before the roster has loaded', () => { const groups = buildRestGroups({ activeConnectionId: 'pandora', connections, roster: null }) diff --git a/apps/desktop/src/app/chat/sidebar/fleet-rail.ts b/apps/desktop/src/app/chat/sidebar/fleet-rail.ts index ac4b80d043..10de23cfa6 100644 --- a/apps/desktop/src/app/chat/sidebar/fleet-rail.ts +++ b/apps/desktop/src/app/chat/sidebar/fleet-rail.ts @@ -22,6 +22,8 @@ export interface FleetGroup { kind: DesktopConnectionKind label: string reachable: boolean + error?: string + needsSignIn?: boolean /** The gateway's default profile — every Hermes home has one, so a group * always carries it even before the roster has been enumerated. */ defaultAgent: FleetAgent @@ -94,6 +96,8 @@ export function buildRestGroups({ kind: connection.kind, label: connection.label, reachable: source?.reachable ?? true, + ...(source?.error && source.error !== 'connect-on-demand' ? { error: source.error } : {}), + ...(source?.needsSignIn ? { needsSignIn: true } : {}), defaultAgent: toAgent(DEFAULT_PROFILE, defaultRow?.handle), named }) diff --git a/apps/desktop/src/app/chat/sidebar/profile-switcher.tsx b/apps/desktop/src/app/chat/sidebar/profile-switcher.tsx index f6d512afc7..20e16d086e 100644 --- a/apps/desktop/src/app/chat/sidebar/profile-switcher.tsx +++ b/apps/desktop/src/app/chat/sidebar/profile-switcher.tsx @@ -1180,7 +1180,11 @@ function FleetRestGroup({ }) { const { t } = useI18n() const p = t.profiles - const dividerLabel = group.reachable ? p.fleet.gateway(group.label) : p.fleet.gatewayUnreachable(group.label) + + const dividerLabel = group.reachable + ? p.fleet.gateway(group.label) + : `${group.needsSignIn ? `${p.fleet.gateway(group.label)} · ${t.settings.toolsets.needsSignIn}` : p.fleet.gatewayUnreachable(group.label)}${group.error ? `\n${group.error}` : ''}` + const defaultKey = fleetRouteKey(group.connectionId, group.defaultAgent.profile) // At rest, This device is a backend switch, not Home. The house glyph stays // on the active gateway's default profile. diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index b42db8b2f7..91a292e026 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -1155,6 +1155,7 @@ export interface DesktopAgentRoster { kind: DesktopConnectionKind reachable: boolean error?: string + needsSignIn?: boolean // Stable backend identity (/api/status install_id) when known. installId?: string }[]