diff --git a/.github/workflows/build-desktop-dev.yml b/.github/workflows/build-desktop-dev.yml index 00986976..b6548c71 100644 --- a/.github/workflows/build-desktop-dev.yml +++ b/.github/workflows/build-desktop-dev.yml @@ -89,7 +89,8 @@ jobs: - name: Install desktop dependencies working-directory: desktop - run: bun install + # The x64 Windows runner also packages ARM64; retain both native addons. + run: bun install --cpu="*" - name: Install adapter dependencies working-directory: adapters diff --git a/.github/workflows/release-desktop.yml b/.github/workflows/release-desktop.yml index ec3fe81d..677b2fd8 100644 --- a/.github/workflows/release-desktop.yml +++ b/.github/workflows/release-desktop.yml @@ -200,7 +200,8 @@ jobs: - name: Install desktop dependencies working-directory: desktop - run: bun install + # The x64 Windows runner also packages ARM64; retain both native addons. + run: bun install --cpu="*" - name: Install adapter dependencies working-directory: adapters diff --git a/desktop/electron/services/publicAccess.test.ts b/desktop/electron/services/publicAccess.test.ts index 4d582324..37ded359 100644 --- a/desktop/electron/services/publicAccess.test.ts +++ b/desktop/electron/services/publicAccess.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import path from 'node:path' -import { PublicAccessManager, classifyPublicAccessError, migratePublicAccessSettings } from './publicAccess' +import { PublicAccessManager, PUBLIC_ACCESS_CONSENT_VERSION, classifyPublicAccessError, migratePublicAccessSettings, forwardPublicAccess } from './publicAccess' const directories: string[] = [] const managers: PublicAccessManager[] = [] @@ -50,6 +50,22 @@ describe('ngrok host management', () => { await expect(manager.start(0)).rejects.toThrow('consent') expect(forward).not.toHaveBeenCalled() }) + it('does not restore v1 consent after remote settings capabilities expand', async () => { + const { directory } = fixture() + const file = path.join(directory, 'public-access-private.json') + writeFileSync(file, JSON.stringify({ version: 1, authtoken: 'fake-token', autoStart: true, consentVersion: 1, future: 'preserved' })) + const request = vi.fn(async (route: string) => route === '/enable' ? { port: 32123 } : {}) + const forward = vi.fn(async () => ({ url: () => 'https://fixture.ngrok-free.app', close: vi.fn(async () => {}) })) + const manager = new PublicAccessManager({ directory, backend: { request: request as never }, forward }) + managers.push(manager) + await manager.restore() + await expect(manager.start(1)).rejects.toThrow('consent') + expect(forward).not.toHaveBeenCalled() + expect(manager.getStatus().consentVersion).toBe(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) + expect(forward).toHaveBeenCalledTimes(1) + expect(JSON.parse(readFileSync(file, 'utf8'))).toMatchObject({ consentVersion: 2, future: 'preserved' }) + }) it('migrates persisted settings with unknown fields and consent', async () => { const { directory } = fixture() writeFileSync(path.join(directory, 'public-access-private.json'), JSON.stringify({ authtoken: 'fake-token', extra: true })) @@ -71,17 +87,17 @@ describe('ngrok host management', () => { it('deduplicates starts and binds the public origin only after tunnel creation', async () => { const { manager, request, forward } = fixture() await manager.saveCredential('fake-token') - await Promise.all([manager.start(1), manager.start(1)]) + await Promise.all([manager.start(PUBLIC_ACCESS_CONSENT_VERSION), manager.start(PUBLIC_ACCESS_CONSENT_VERSION)]) expect(forward).toHaveBeenCalledTimes(1) expect(request).toHaveBeenLastCalledWith('/origin', 'PUT', { publicUrl: 'https://fixture.ngrok-free.app' }) - expect(manager.getStatus()).toMatchObject({ state: 'online', consentVersion: 1 }) + expect(manager.getStatus()).toMatchObject({ state: 'online', consentVersion: PUBLIC_ACCESS_CONSENT_VERSION }) }) it('closes late SDK results after stop without publishing their URL', async () => { const { manager, forward, listener, request } = fixture() await manager.saveCredential('fake-token') let finish!: (value: typeof listener) => void forward.mockImplementation(() => new Promise(resolve => { finish = resolve })) - const start = manager.start(1) + const start = manager.start(PUBLIC_ACCESS_CONSENT_VERSION) await Promise.resolve() await manager.stop() finish(listener) @@ -95,19 +111,19 @@ describe('ngrok host management', () => { await manager.saveCredential('fake-token') let finish!: (value: typeof listener) => void forward.mockImplementation(() => new Promise(resolve => { finish = resolve })) - const start = manager.start(1) + const start = manager.start(PUBLIC_ACCESS_CONSENT_VERSION) await Promise.resolve() await manager.dispose() finish(listener) await start - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) expect(forward).toHaveBeenCalledTimes(1) expect(listener.close).toHaveBeenCalled() }) it('rebinds on sidecar restart and drops the old tunnel', async () => { const { manager, forward, listener } = fixture() await manager.saveCredential('fake-token') - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) await manager.serverUnavailable() expect(manager.getStatus().state).toBe('reconnecting') await manager.serverChanged() @@ -118,7 +134,7 @@ describe('ngrok host management', () => { it('reflects SDK reconnect notifications and ignores callbacks after disable', async () => { const { manager, forward } = fixture() await manager.saveCredential('fake-token') - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) const config = forward.mock.calls[0]![0] as { onStatusChange: (state: string) => void } config.onStatusChange('closed') expect(manager.getStatus().state).toBe('reconnecting') @@ -133,13 +149,13 @@ describe('ngrok host management', () => { const { manager, forward } = fixture() await manager.saveCredential('fake-token') forward.mockRejectedValueOnce(new Error('network unavailable fake-token')) - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) expect(manager.getStatus()).toMatchObject({ state: 'reconnecting', error: 'network' }) await advance(1000) expect(manager.getStatus().state).toBe('online') await manager.stop() forward.mockRejectedValueOnce(new Error('authtoken invalid fake-token')) - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) expect(manager.getStatus()).toMatchObject({ state: 'failed', error: 'auth' }) await advance(60_000) expect(forward).toHaveBeenCalledTimes(3) @@ -149,7 +165,7 @@ describe('ngrok host management', () => { vi.useFakeTimers() const { manager, forward, listener } = fixture() await manager.saveCredential('fake-token') - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) const config = forward.mock.calls[0]![0] as { onStatusChange: (state: string) => void } forward.mockRejectedValueOnce(new Error('ERR_NGROK_105 authentication denied')) config.onStatusChange('closed') @@ -168,14 +184,14 @@ describe('ngrok host management', () => { (value as { onStatusChange: (state: string) => void }).onStatusChange('closed') return listener }) - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) expect(manager.getStatus().state).toBe('reconnecting') }) it('disables the local entry immediately and bounds a hanging SDK close', async () => { vi.useFakeTimers() const { manager, listener, request } = fixture() await manager.saveCredential('fake-token') - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) listener.close.mockImplementation(() => new Promise(() => {})) const stopped = manager.stop() expect(request).toHaveBeenLastCalledWith('/disable', 'POST') @@ -184,11 +200,30 @@ describe('ngrok host management', () => { await stopped expect(manager.getStatus().state).toBe('disabled') }) + it('does not publish an old connection error after stop during backend cleanup', async () => { + const { manager, forward, request } = fixture() + await manager.saveCredential('fake-token') + forward.mockRejectedValueOnce(new Error('network unavailable')) + let finishDisable!: () => void + let disableCount = 0 + request.mockImplementation(async route => { + if (route === '/enable') return { port: 32123 } + if (route === '/disable' && ++disableCount === 1) await new Promise(resolve => { finishDisable = resolve }) + return {} + }) + const connecting = manager.start(PUBLIC_ACCESS_CONSENT_VERSION) + for (let i = 0; i < 20; i++) await Promise.resolve() + expect(finishDisable).toBeDefined() + await manager.stop() + finishDisable() + await connecting + expect(manager.getStatus()).toMatchObject({ state: 'disabled', error: null }) + }) it('keeps auto-start opt-in across normal stop', async () => { const { manager, directory, request, forward } = fixture() await manager.saveCredential('fake-token') manager.setAutoStart(true) - await manager.start(1) + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) await manager.stop() const restored = new PublicAccessManager({ directory, backend: { request: request as never }, forward }) managers.push(restored) @@ -196,3 +231,127 @@ describe('ngrok host management', () => { expect(restored.getStatus().state).toBe('online') }) }) + +// Models v1.7.0: forward() retains one session and its original credentials and +// callbacks after listener.close(). Explicit builders own independent sessions. +function sdkFixture() { + type Session = { token: string, disconnected: () => boolean, heartbeat: (latency: number | null) => void, close: ReturnType } + const sessions: Session[] = [] + const tokens: string[] = [] + const listenerCloses: ReturnType[] = [] + const listen = vi.fn(async (session: Session) => { + tokens.push(session.token) + const close = vi.fn(async () => {}) + listenerCloses.push(close) + return { url: (): string => 'https://fixture.ngrok-free.app', close } + }) + class SessionBuilder { + token = '' + disconnected = () => true + heartbeat = (_latency: number | null) => {} + authtoken(token: string) { + this.token = token + return this + } + handleDisconnection(handler: () => boolean) { + this.disconnected = handler + return this + } + handleHeartbeat(handler: (latency: number | null) => void) { + this.heartbeat = handler + return this + } + async connect() { + const session = { token: this.token, disconnected: this.disconnected, heartbeat: this.heartbeat, close: vi.fn(async () => {}) } + sessions.push(session) + return { ...session, httpEndpoint: () => ({ listenAndForward: () => listen(session) }) } + } + } + let singleton: Session | undefined + const forward = vi.fn(async (config: { authtoken: string, onStatusChange: (status: string) => void }) => { + if (!singleton) { + singleton = { + token: config.authtoken, + disconnected: () => { + config.onStatusChange('closed') + return true + }, + heartbeat: () => config.onStatusChange('connected'), + close: vi.fn(async () => {}), + } + sessions.push(singleton) + } + return listen(singleton) + }) + const sdk = { SessionBuilder, forward } as never + return { sdk, sessions, tokens, listenerCloses, listen } +} + +describe('ngrok SDK session ownership', () => { + it('changes credentials and reconnect callbacks after reopening and closes owned sessions', async () => { + const sdk = sdkFixture() + const { directory, request } = fixture() + const manager = new PublicAccessManager({ directory, backend: { request: request as never }, forward: config => forwardPublicAccess(config, sdk.sdk) }) + managers.push(manager) + await manager.saveCredential('first-account') + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) + await manager.saveCredential('second-account') + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) + expect(sdk.tokens).toEqual(['first-account', 'second-account']) + expect(sdk.sessions[0]!.close).toHaveBeenCalledTimes(1) + sdk.sessions[0]!.disconnected() + expect(manager.getStatus().state).toBe('online') + sdk.sessions[1]!.disconnected() + expect(manager.getStatus().state).toBe('reconnecting') + sdk.sessions[1]!.heartbeat(null) + expect(manager.getStatus().state).toBe('reconnecting') + sdk.sessions[1]!.heartbeat(3) + expect(manager.getStatus().state).toBe('online') + await manager.deleteCredential() + expect(sdk.sessions[1]!.close).toHaveBeenCalledTimes(1) + expect(sdk.listenerCloses.every(close => close.mock.calls.length === 1)).toBe(true) + }) + it('replaces a stalled session and retains callbacks on the replacement', async () => { + vi.useFakeTimers() + const sdk = sdkFixture() + const { directory, request } = fixture() + const manager = new PublicAccessManager({ directory, backend: { request: request as never }, forward: config => forwardPublicAccess(config, sdk.sdk) }) + managers.push(manager) + await manager.saveCredential('fake-token') + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) + sdk.sessions[0]!.disconnected() + await advance(30_000) + for (let i = 0; i < 30; i++) await Promise.resolve() + expect(sdk.sessions).toHaveLength(2) + expect(sdk.sessions[0]!.close).toHaveBeenCalledTimes(1) + expect(manager.getStatus().state).toBe('online') + sdk.sessions[1]!.disconnected() + expect(manager.getStatus().state).toBe('reconnecting') + sdk.sessions[1]!.heartbeat(4) + expect(manager.getStatus().state).toBe('online') + }) + it('closes only the old owned session when its listener arrives after a new start', async () => { + const sdk = sdkFixture() + const { directory, request } = fixture() + const manager = new PublicAccessManager({ directory, backend: { request: request as never }, forward: config => forwardPublicAccess(config, sdk.sdk) }) + managers.push(manager) + await manager.saveCredential('fake-token') + let finish!: (listener: Awaited>) => void + sdk.listen.mockImplementationOnce(() => new Promise(resolve => { finish = resolve })) + const first = manager.start(PUBLIC_ACCESS_CONSENT_VERSION) + for (let i = 0; i < 20; i++) await Promise.resolve() + await manager.stop() + await manager.start(PUBLIC_ACCESS_CONSENT_VERSION) + finish({ url: () => 'https://old.ngrok-free.app', close: vi.fn(async () => {}) }) + await first + expect(sdk.sessions[0]!.close).toHaveBeenCalledTimes(1) + expect(sdk.sessions[1]!.close).not.toHaveBeenCalled() + expect(manager.getStatus()).toMatchObject({ state: 'online', publicUrl: 'https://fixture.ngrok-free.app' }) + }) + it('closes the session even when endpoint creation fails', async () => { + const sdk = sdkFixture() + sdk.listen.mockRejectedValueOnce(new Error('network unavailable')) + await expect(forwardPublicAccess({ addr: '127.0.0.1:1', authtoken: 'fake', onStatusChange: () => {} }, sdk.sdk)).rejects.toThrow('network unavailable') + expect(sdk.sessions[0]!.close).toHaveBeenCalledTimes(1) + }) +}) diff --git a/desktop/electron/services/publicAccess.ts b/desktop/electron/services/publicAccess.ts index cd03bcd0..f6d2a5bb 100644 --- a/desktop/electron/services/publicAccess.ts +++ b/desktop/electron/services/publicAccess.ts @@ -1,5 +1,7 @@ import { chmodSync, existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' import path from 'node:path' +import { PUBLIC_ACCESS_CONSENT_VERSION } from '../../src/lib/desktopHost/types' +export { PUBLIC_ACCESS_CONSENT_VERSION } from '../../src/lib/desktopHost/types' export type PublicAccessStatus = { state: 'unconfigured' | 'disabled' | 'connecting' | 'online' | 'reconnecting' | 'failed' @@ -11,13 +13,56 @@ export type PublicAccessStatus = { } type Settings = Record & { version: number, authtoken: string, autoStart: boolean, consentVersion: number } type Listener = { url(): string | null, close(): Promise } +type ForwardConfig = { addr: string, authtoken: string, onStatusChange: (status: string) => void } +type NgrokSdk = Pick +export async function forwardPublicAccess(config: ForwardConfig, sdk: NgrokSdk): Promise { + // forward() retains a process-global session, including its original token + // and callbacks. Own a session per attempt so stop and credential rotation + // also end authentication, without closing another generation's session. + let closed = false + const session = await new sdk.SessionBuilder() + .authtoken(config.authtoken) + .handleDisconnection(() => { + if (!closed) config.onStatusChange('closed') + return true + }) + .handleHeartbeat(latency => { + // The native SDK can pass null when a heartbeat has no response. + if (!closed && typeof latency === 'number') config.onStatusChange('connected') + }) + .connect() + try { + const listener = await session.httpEndpoint().listenAndForward(`http://${config.addr}`) + return { + url: () => listener.url(), + async close() { + closed = true + await Promise.all([closePublicAccessResource(listener), closePublicAccessResource(session)]) + }, + } + } catch (error) { + closed = true + await closePublicAccessResource(session) + throw error + } +} + +async function closePublicAccessResource(resource: { close(): Promise }) { + let timeout: ReturnType | undefined + try { + await Promise.race([ + resource.close(), + new Promise(resolve => { timeout = setTimeout(resolve, 3_000) }), + ]) + } catch { /* SDK errors can contain credentials; do not log them. */ } + finally { if (timeout) clearTimeout(timeout) } +} type Backend = { request(route: string, method: string, body?: unknown): Promise } type Options = { directory: string backend: Backend forward?: (config: { addr: string, authtoken: string, onStatusChange: (status: string) => void }) => Promise } -export const PUBLIC_ACCESS_CONSENT_VERSION = 1 /** Forward migration is additive and keeps unrecognized fields in this private file. */ export function migratePublicAccessSettings(raw: unknown): Settings { @@ -140,7 +185,7 @@ export class PublicAccessManager { const { port } = await this.options.backend.request<{ port: number }>('/enable', 'POST') if (generation !== this.generation) return this.getStatus() if (!Number.isInteger(port) || port < 1 || port > 65535) throw new Error('Invalid listener port') - const forward = this.options.forward ?? (async config => (await import('@ngrok/ngrok')).forward(config)) + const forward = this.options.forward ?? (async config => forwardPublicAccess(config, await import('@ngrok/ngrok'))) let disconnected = false const onStatusChange = (state: string) => { if (generation !== this.generation) return @@ -173,6 +218,7 @@ export class PublicAccessManager { await this.closeListener(listener) if (generation !== this.generation) return this.getStatus() await this.options.backend.request('/disable', 'POST').catch(() => {}) + if (generation !== this.generation) return this.getStatus() this.status.error = classifyPublicAccessError(error) this.status.state = 'failed' if (this.status.error === 'network' && this.wanted) { @@ -186,14 +232,7 @@ export class PublicAccessManager { private async closeListener(listener: Listener | null) { if (!listener) return - let timeout: ReturnType | undefined - try { - await Promise.race([ - listener.close(), - new Promise(resolve => { timeout = setTimeout(resolve, 3_000) }), - ]) - } catch { /* SDK errors can contain credentials; do not log them. */ } - finally { if (timeout) clearTimeout(timeout) } + await closePublicAccessResource(listener) } private async reconnectDisconnected(generation: number) { diff --git a/desktop/electron/services/publicAccessPackaging.test.ts b/desktop/electron/services/publicAccessPackaging.test.ts index 501c7d6e..eecb3529 100644 --- a/desktop/electron/services/publicAccessPackaging.test.ts +++ b/desktop/electron/services/publicAccessPackaging.test.ts @@ -1,8 +1,12 @@ +import { readFileSync } from 'node:fs' +import { parse } from 'yaml' import { createRequire } from 'node:module' +import path from 'node:path' import { describe, expect, it } from 'vitest' const require = createRequire(import.meta.url) const manifest = require('../../package.json') +const repositoryRoot = path.resolve(path.dirname(require.resolve('../../package.json')), '..') describe('ngrok native packaging', () => { it('ships and unpacks SDK native dependencies instead of bundling the loader', () => { expect(manifest.dependencies['@ngrok/ngrok']).toBeTruthy() @@ -10,13 +14,30 @@ describe('ngrok native packaging', () => { expect(manifest.build.files).toContain('node_modules/@ngrok/**') expect(manifest.scripts['build:electron']).toContain('--external @ngrok/ngrok') const sdkManifest = require('@ngrok/ngrok/package.json') - for (const target of ['darwin-arm64', 'darwin-x64', 'win32-x64-msvc']) { + for (const target of ['darwin-arm64', 'darwin-x64', 'win32-x64-msvc', 'win32-arm64-msvc']) { expect(sdkManifest.optionalDependencies[`@ngrok/ngrok-${target}`]).toBeTruthy() } }) + it.each(['release-desktop.yml', 'build-desktop-dev.yml'])('%s installs native addons for cross-architecture packages', workflow => { + const source = parse(readFileSync(path.join(repositoryRoot, '.github', 'workflows', workflow), 'utf8')) + const jobs = Object.values(source.jobs) as { steps?: { name?: string, run?: string, 'working-directory'?: string }[] }[] + const desktopInstalls = jobs.flatMap(job => job.steps ?? []).filter(step => step.name === 'Install desktop dependencies') + expect(desktopInstalls.length).toBeGreaterThan(0) + for (const step of desktopInstalls) { + expect(step['working-directory']).toBe('desktop') + // Windows ARM64 is built on the x64 runner. Host-only installation omits + // ngrok-win32-arm64-msvc, and npmRebuild:false cannot fetch it at packaging. + expect(step.run).toContain('--cpu="*"') + } + }) it('loads the current host native addon without opening an ngrok session', () => { const sdk = require('@ngrok/ngrok') expect(typeof sdk.forward).toBe('function') expect(typeof sdk.disconnect).toBe('function') + const builder = new sdk.SessionBuilder() + expect(typeof builder.authtoken).toBe('function') + expect(typeof builder.handleDisconnection).toBe('function') + expect(typeof builder.handleHeartbeat).toBe('function') + expect(typeof builder.connect).toBe('function') }) }) diff --git a/desktop/src/__tests__/agentsSettings.test.tsx b/desktop/src/__tests__/agentsSettings.test.tsx index 04bf4489..0f652db3 100644 --- a/desktop/src/__tests__/agentsSettings.test.tsx +++ b/desktop/src/__tests__/agentsSettings.test.tsx @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { act, fireEvent, render, screen } from '@testing-library/react' import '@testing-library/jest-dom' -import { Settings } from '../pages/Settings' +import { DesktopSettings as Settings } from '../pages/Settings' import { useAgentStore } from '../stores/agentStore' import { useSkillStore } from '../stores/skillStore' import { useSettingsStore } from '../stores/settingsStore' diff --git a/desktop/src/__tests__/diagnosticsSettings.test.tsx b/desktop/src/__tests__/diagnosticsSettings.test.tsx index b08fe2f5..c336a71d 100644 --- a/desktop/src/__tests__/diagnosticsSettings.test.tsx +++ b/desktop/src/__tests__/diagnosticsSettings.test.tsx @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import '@testing-library/jest-dom' -import { Settings } from '../pages/Settings' +import { DesktopSettings as Settings } from '../pages/Settings' import { SAFE_DOCTOR_STORAGE_KEYS } from '../lib/doctorRepair' import { useSessionStore } from '../stores/sessionStore' import { useSettingsStore } from '../stores/settingsStore' diff --git a/desktop/src/__tests__/generalSettings.test.tsx b/desktop/src/__tests__/generalSettings.test.tsx index b2b48f4e..fe1a6c84 100644 --- a/desktop/src/__tests__/generalSettings.test.tsx +++ b/desktop/src/__tests__/generalSettings.test.tsx @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import '@testing-library/jest-dom' -import { Settings } from '../pages/Settings' +import { DesktopSettings as Settings } from '../pages/Settings' import { useSettingsStore } from '../stores/settingsStore' import { useUIStore } from '../stores/uiStore' import { useUpdateStore } from '../stores/updateStore' diff --git a/desktop/src/__tests__/petSettingsNavigation.test.tsx b/desktop/src/__tests__/petSettingsNavigation.test.tsx index 9f0d84d7..1b82f03c 100644 --- a/desktop/src/__tests__/petSettingsNavigation.test.tsx +++ b/desktop/src/__tests__/petSettingsNavigation.test.tsx @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { fireEvent, render, screen } from '@testing-library/react' import '@testing-library/jest-dom' -import { Settings } from '../pages/Settings' +import { DesktopSettings as Settings } from '../pages/Settings' import { useSettingsStore } from '../stores/settingsStore' import { useUIStore } from '../stores/uiStore' diff --git a/desktop/src/__tests__/pluginsSettings.test.tsx b/desktop/src/__tests__/pluginsSettings.test.tsx index 64d52c43..8c2cd8e3 100644 --- a/desktop/src/__tests__/pluginsSettings.test.tsx +++ b/desktop/src/__tests__/pluginsSettings.test.tsx @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import '@testing-library/jest-dom' -import { Settings } from '../pages/Settings' +import { DesktopSettings as Settings } from '../pages/Settings' import { usePluginStore } from '../stores/pluginStore' import { useSettingsStore } from '../stores/settingsStore' import { useSessionStore } from '../stores/sessionStore' diff --git a/desktop/src/__tests__/skillsSettings.test.tsx b/desktop/src/__tests__/skillsSettings.test.tsx index c73bd9c9..7af302e7 100644 --- a/desktop/src/__tests__/skillsSettings.test.tsx +++ b/desktop/src/__tests__/skillsSettings.test.tsx @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { act, fireEvent, render, screen, within } from '@testing-library/react' import '@testing-library/jest-dom' -import { Settings } from '../pages/Settings' +import { DesktopSettings as Settings } from '../pages/Settings' import { useSkillStore } from '../stores/skillStore' import { useSettingsStore } from '../stores/settingsStore' import { useSessionStore } from '../stores/sessionStore' diff --git a/desktop/src/api/publicAccess.test.ts b/desktop/src/api/publicAccess.test.ts index ba153456..5bb69260 100644 --- a/desktop/src/api/publicAccess.test.ts +++ b/desktop/src/api/publicAccess.test.ts @@ -11,8 +11,9 @@ it('pairs at the current origin with cookies and no bearer or URL secret', async })) }) it('does not expose an upstream response body in errors', async () => { - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, text: async () => 'sensitive upstream body' })) + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 401, text: async () => 'sensitive upstream body' })) await expect(remoteAccessApi.session()).rejects.toThrow('Remote access request failed') + await expect(remoteAccessApi.claim('phone', 'expired')).rejects.toMatchObject({ status: 401 }) }) it('routes local device approval through authenticated local API', async () => { const post = vi.spyOn(api, 'post').mockResolvedValue({}) diff --git a/desktop/src/api/publicAccess.ts b/desktop/src/api/publicAccess.ts index 8dd06376..8d451d34 100644 --- a/desktop/src/api/publicAccess.ts +++ b/desktop/src/api/publicAccess.ts @@ -1,4 +1,4 @@ -import { api } from './client' +import { api, ApiError } from './client' export type PublicAccessDevice = { id: string, name: string, createdAt: number, expiresAt: number } export type PublicAccessServerStatus = { @@ -30,7 +30,7 @@ async function remoteRequest(path: string, body?: unknown): Promise { headers: body === undefined ? undefined : { 'Content-Type': 'application/json' }, body: body === undefined ? undefined : JSON.stringify(body), }) - if (!response.ok) throw new Error('Remote access request failed') + if (!response.ok) throw new ApiError(response.status, 'Remote access request failed') return await response.json() as T } finally { clearTimeout(timeout) diff --git a/desktop/src/components/layout/AppShell.test.tsx b/desktop/src/components/layout/AppShell.test.tsx index d64df0ce..a4699c9e 100644 --- a/desktop/src/components/layout/AppShell.test.tsx +++ b/desktop/src/components/layout/AppShell.test.tsx @@ -668,7 +668,7 @@ describe('AppShell boot flow', () => { expect(screen.getByTestId('mobile-sidebar-toggle')).toHaveClass('h-11', 'w-11') }) - it('keeps browser H5 mobile on chat tabs when settings was restored as active', async () => { + it('keeps browser H5 settings active alongside existing chat tabs', async () => { mocks.isMobile = true mocks.tabState.activeTabId = '__settings__' mocks.tabState.tabs = [ @@ -680,8 +680,18 @@ describe('AppShell boot flow', () => { await screen.findByText('content loaded') expect(screen.queryByText('tabs loaded')).not.toBeInTheDocument() - await waitFor(() => { - expect(mocks.setActiveTab).toHaveBeenCalledWith('session-1') - }) + expect(mocks.setActiveTab).not.toHaveBeenCalled() + expect(mocks.tabState.activeTabId).toBe('__settings__') + expect(screen.getByTestId('mobile-session-header')).toHaveTextContent('sidebar.settings') + }) + + it('keeps mobile settings active when no chat session exists', async () => { + mocks.isMobile = true + mocks.tabState.activeTabId = '__settings__' + mocks.tabState.tabs = [{ sessionId: '__settings__', title: 'Settings', type: 'settings', status: 'idle' }] + render() + await screen.findByText('content loaded') + expect(mocks.tabState.activeTabId).toBe('__settings__') + expect(mocks.setActiveTab).not.toHaveBeenCalled() }) }) diff --git a/desktop/src/components/layout/AppShell.tsx b/desktop/src/components/layout/AppShell.tsx index e6a1f70b..9237ae62 100644 --- a/desktop/src/components/layout/AppShell.tsx +++ b/desktop/src/components/layout/AppShell.tsx @@ -250,7 +250,7 @@ export function AppShell() { useEffect(() => { if (!ready || !isMobileShell) return - if (isChatTab(activeTab) || (!activeTab && !activeTabId)) return + if (isChatTab(activeTab) || activeTab?.type === 'settings' || (!activeTab && !activeTabId)) return const nextChatTab = tabs.find(isChatTab) if (nextChatTab) { setActiveTab(nextChatTab.sessionId) @@ -371,7 +371,9 @@ export function AppShell() { aria-controls="sidebar-shell" aria-expanded={effectiveSidebarOpen} /> - {isActiveChatTab ? ( + {activeTab?.type === 'settings' ? ( +

{t('sidebar.settings')}

+ ) : isActiveChatTab ? (

{mobileSessionTitle} diff --git a/desktop/src/components/layout/H5ConnectionView.test.tsx b/desktop/src/components/layout/H5ConnectionView.test.tsx new file mode 100644 index 00000000..dc29361f --- /dev/null +++ b/desktop/src/components/layout/H5ConnectionView.test.tsx @@ -0,0 +1,28 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import '@testing-library/jest-dom' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { H5ConnectionView } from './H5ConnectionView' +import { readStoredH5Connection, saveAndVerifyH5Connection } from '../../lib/desktopRuntime' +import { useSettingsStore } from '../../stores/settingsStore' + +vi.mock('../../lib/desktopRuntime', () => ({ readStoredH5Connection: vi.fn(), saveAndVerifyH5Connection: vi.fn() })) +beforeEach(() => { + useSettingsStore.setState({ locale: 'en' }) + vi.mocked(readStoredH5Connection).mockReturnValue({ serverUrl: 'https://paired.example', token: 'saved-token' }) + vi.mocked(saveAndVerifyH5Connection).mockResolvedValue('https://paired.example') +}) +afterEach(() => { cleanup(); vi.resetAllMocks() }) + +it('retries a remembered pairing after a network failure without asking for its token', async () => { + const connected = vi.fn() + render() + expect(screen.getByLabelText(/H5 Token/)).toHaveValue('') + fireEvent.click(screen.getByRole('button', { name: 'Retry' })) + await vi.waitFor(() => expect(connected).toHaveBeenCalledOnce()) + expect(saveAndVerifyH5Connection).toHaveBeenCalledWith('https://paired.example', 'saved-token') +}) + +it('does not offer the saved credential to another entered server', () => { + render() + expect(screen.queryByRole('button', { name: 'Retry' })).not.toBeInTheDocument() +}) diff --git a/desktop/src/components/layout/H5ConnectionView.tsx b/desktop/src/components/layout/H5ConnectionView.tsx index 5a0b479e..de5bc24e 100644 --- a/desktop/src/components/layout/H5ConnectionView.tsx +++ b/desktop/src/components/layout/H5ConnectionView.tsx @@ -1,6 +1,6 @@ import type { FormEvent } from 'react' import { useState } from 'react' -import { saveAndVerifyH5Connection } from '../../lib/desktopRuntime' +import { readStoredH5Connection, saveAndVerifyH5Connection } from '../../lib/desktopRuntime' import { Button } from '@/components/ui/Button' import { Card } from '@/components/ui/Card' import { Input } from '@/components/ui/Input' @@ -23,6 +23,8 @@ export function H5ConnectionView({ const [token, setToken] = useState('') const [error, setError] = useState(initialError ?? '') const [submitting, setSubmitting] = useState(false) + const remembered = readStoredH5Connection() + const canRetry = remembered.token && remembered.serverUrl === serverUrl.trim().replace(/\/$/, '') const handleSubmit = async (event: FormEvent) => { event.preventDefault() @@ -95,6 +97,16 @@ export function H5ConnectionView({ + {canRetry && }

diff --git a/desktop/src/components/layout/Sidebar.test.tsx b/desktop/src/components/layout/Sidebar.test.tsx index d41a63a4..1b63f1fc 100644 --- a/desktop/src/components/layout/Sidebar.test.tsx +++ b/desktop/src/components/layout/Sidebar.test.tsx @@ -2074,7 +2074,11 @@ describe('Sidebar', () => { expect(screen.queryByRole('button', { name: 'Scheduled' })).not.toBeInTheDocument() expect(screen.queryByRole('button', { name: 'Skills Market' })).not.toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Settings' })).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Settings' })).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: 'Settings' })) + expect(useTabStore.getState().activeTabId).toBe('__settings__') + expect(onRequestClose).toHaveBeenCalledTimes(1) + onRequestClose.mockClear() fireEvent.click(screen.getByRole('button', { name: /Open Session/ })) expect(onRequestClose).toHaveBeenCalledTimes(1) diff --git a/desktop/src/components/layout/Sidebar.tsx b/desktop/src/components/layout/Sidebar.tsx index ae5419b2..b99db230 100644 --- a/desktop/src/components/layout/Sidebar.tsx +++ b/desktop/src/components/layout/Sidebar.tsx @@ -1472,7 +1472,7 @@ export function Sidebar({