From bbad5122a88a4b9eb39de43fc4ad917c2719cc3d Mon Sep 17 00:00:00 2001 From: elkaix Date: Sun, 16 Aug 2026 01:38:57 -0400 Subject: [PATCH] fix(desktop): pin the Host port and stop reporting non-updatable builds as errors Two defects reported from the packaged app. The Host was started with --port 0, so the window loaded a different http://127.0.0.1: origin on every launch. localStorage is keyed by origin, so the web UI's persisted state was discarded each time: the first-run onboarding dialog reappeared, and theme, colour scheme, UI font size, permission mode, thinking level, plan mode, starred models, unread state and the active workspace all reset. Packaged builds now use port 24827 and development uses 24828, with a validated PYTHINKER_DESKTOP_PORT override. There is deliberately no fallback port. A fallback would reintroduce the same silent data loss on exactly the machines most likely to hit a collision, so a bind failure now shows a dialog naming the port and the override, offering Retry or Quit. A locally packed build (electron-builder --dir) sets isPackaged but ships no app-update.yml, so the first update check threw ENOENT and Settings showed a red error. The updater now checks for that file first and reports the calm 'disabled' state instead. --- apps/desktop/src/host-supervisor.ts | 26 ++++++++- apps/desktop/src/main.ts | 64 +++++++++++++++------- apps/desktop/src/updater.ts | 23 +++++++- apps/desktop/tests/host-supervisor.spec.ts | 35 +++++++++++- apps/desktop/tests/updater.spec.ts | 46 +++++++++++++++- 5 files changed, 168 insertions(+), 26 deletions(-) diff --git a/apps/desktop/src/host-supervisor.ts b/apps/desktop/src/host-supervisor.ts index 89f525c8..2d40ee7c 100644 --- a/apps/desktop/src/host-supervisor.ts +++ b/apps/desktop/src/host-supervisor.ts @@ -9,6 +9,26 @@ const DEFAULT_SHUTDOWN_TIMEOUT_MS = 5_000 const TASKKILL_TIMEOUT_MS = 5_000 const MAX_STARTUP_OUTPUT_CHARS = 32_768 +export const DESKTOP_PACKAGED_PORT = 24_827 +export const DESKTOP_DEV_PORT = 24_828 + +/** Resolve the fixed Host port, with an optional validated environment override. */ +export function resolveDesktopPort(env: NodeJS.ProcessEnv, isPackaged: boolean): number { + const value = env['PYTHINKER_DESKTOP_PORT'] + if (value === undefined) return isPackaged ? DESKTOP_PACKAGED_PORT : DESKTOP_DEV_PORT + + const port = Number(value) + if (!/^\d+$/u.test(value) || !Number.isInteger(port) || port < 1 || port > 65_535) { + throw new Error(`PYTHINKER_DESKTOP_PORT must be an integer from 1 to 65535; received ${JSON.stringify(value)}`) + } + return port +} + +/** Return whether Host output reports that its fixed port is already occupied. */ +export function isPortInUseError(message: string): boolean { + return /EADDRINUSE|address already in use/iu.test(message) +} + /** Incremental parser for the Web Host's canonical readiness line. */ export interface ReadinessParser { /** @@ -257,6 +277,8 @@ export interface SpawnPythinkerServerOptions { readonly cwd: string /** Frozen environment for the Host process. */ readonly env: NodeJS.ProcessEnv + /** Fixed loopback port for the Host server. */ + readonly port: number /** Run the Electron executable as its bundled Node runtime. */ readonly electronRunAsNode?: boolean } @@ -272,7 +294,7 @@ function streamAdapter(stream: NodeJS.ReadableStream): HostChild['stdout'] { } /** - * Spawn the production Pythinker server on an OS-assigned loopback port. + * Spawn the production Pythinker server on a fixed loopback port. * @param options - Node runtime, built CLI and process environment. * @returns The child handle consumed by {@link createHostSupervisor}. */ @@ -286,7 +308,7 @@ export function spawnPythinkerServer(options: SpawnPythinkerServerOptions): Host 'run', '--foreground', '--port', - '0', + String(options.port), '--log-level', 'error', ], { diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index e954392b..adea503d 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -23,7 +23,13 @@ import { setCrashPhase, track, } from '@pymodel/pythinker-telemetry' -import { createHostSupervisor, spawnPythinkerServer, type HostSupervisor } from './host-supervisor' +import { + createHostSupervisor, + isPortInUseError, + resolveDesktopPort, + spawnPythinkerServer, + type HostSupervisor, +} from './host-supervisor' import { createSplashWindow } from './splash' import { checkForUpdatesNow, @@ -303,6 +309,7 @@ async function boot(): Promise { if (bootQuitPromise !== undefined) return initializeDesktopTelemetry() const paths = hostPaths() + const port = resolveDesktopPort(process.env, app.isPackaged) assertHostArtifacts(paths) const splash = createSplashWindow(desktopResources('splash')) const destroySplash = (): void => { @@ -313,26 +320,43 @@ async function boot(): Promise { const trayFrames = loadTrayImages() createTray(trayFrames) stopTrayAnimation = startTrayAnimation(trayFrames) - host = createHostSupervisor({ - spawnHost: () => spawnPythinkerServer({ - ...paths, - env: { - ...process.env, - PYTHINKER_DESKTOP: '1', + for (;;) { + host = createHostSupervisor({ + spawnHost: () => spawnPythinkerServer({ + ...paths, + env: { + ...process.env, + PYTHINKER_DESKTOP: '1', + }, + port, + }), + log: chunk => process.stderr.write(chunk), + onUnexpectedExit: ({ code, signal }) => { + console.error(`desktop Host exited unexpectedly (code ${String(code)}, signal ${String(signal)})`) + void requestAppQuit() }, - }), - log: chunk => process.stderr.write(chunk), - onUnexpectedExit: ({ code, signal }) => { - console.error(`desktop Host exited unexpectedly (code ${String(code)}, signal ${String(signal)})`) - void requestAppQuit() - }, - }) - try { - hostOrigin = await host.start() - track('desktop_server_ready') - } catch (error) { - track('desktop_server_failed') - throw error + }) + try { + hostOrigin = await host.start() + track('desktop_server_ready') + break + } catch (error) { + track('desktop_server_failed') + const message = error instanceof Error ? error.message : String(error) + if (!isPortInUseError(message)) throw error + + const result = await dialog.showMessageBox({ + type: 'error', + buttons: ['Retry', 'Quit'], + defaultId: 0, + cancelId: 1, + title: `${APP_NAME} needs its fixed port`, + message: `${APP_NAME} cannot use port ${String(port)}. It needs this fixed port so settings persist between launches. Free the port, or set PYTHINKER_DESKTOP_PORT to an unused port.`, + }) + if (result.response === 0) continue + await requestAppQuit() + return + } } stopTrayAnimation?.() stopTrayAnimation = undefined diff --git a/apps/desktop/src/updater.ts b/apps/desktop/src/updater.ts index cd6ebd4a..803f625e 100644 --- a/apps/desktop/src/updater.ts +++ b/apps/desktop/src/updater.ts @@ -1,10 +1,11 @@ -import { readFileSync, writeFileSync } from 'node:fs' +import { existsSync, readFileSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { app, type BrowserWindow } from 'electron' import electronUpdater from 'electron-updater' const { autoUpdater } = electronUpdater const UPDATE_SETTINGS_FILE = 'update-settings.json' +const UPDATES_UNAVAILABLE_MESSAGE = 'Updates are not available for this build' const INITIAL_CHECK_DELAY_MS = 10_000 const CHECK_INTERVAL_MS = 4 * 60 * 60 * 1_000 @@ -117,6 +118,20 @@ function clearTimers(): void { checkInterval = undefined } +function hasUpdateConfig(): boolean { + return existsSync(join(process.resourcesPath, 'app-update.yml')) +} + +function disableUpdates(): UpdateState { + updateState({ + status: 'disabled', + message: UPDATES_UNAVAILABLE_MESSAGE, + version: undefined, + percent: undefined, + }) + return state +} + function scheduleChecks(): void { if (checkInterval !== undefined) return checkInterval = setInterval(() => { @@ -181,6 +196,10 @@ export function initUpdater( clearTimers() if (!app.isPackaged) return + if (!hasUpdateConfig()) { + disableUpdates() + return + } try { autoUpdater.autoDownload = settings.autoUpdate @@ -232,6 +251,7 @@ export async function checkForUpdatesNow(): Promise { updateState({ status: 'disabled' }) return state } + if (!hasUpdateConfig()) return disableUpdates() try { autoUpdater.autoDownload = true @@ -251,6 +271,7 @@ export function quitAndInstallNow(): UpdateState { updateState({ status: 'disabled' }) return state } + if (!hasUpdateConfig()) return disableUpdates() try { updateTelemetryTrack('desktop_update_install') diff --git a/apps/desktop/tests/host-supervisor.spec.ts b/apps/desktop/tests/host-supervisor.spec.ts index c347f3dd..ff08b9c0 100644 --- a/apps/desktop/tests/host-supervisor.spec.ts +++ b/apps/desktop/tests/host-supervisor.spec.ts @@ -5,6 +5,7 @@ import { createReadinessParser, type HostChild, } from '../src/host-supervisor' +import * as hostSupervisor from '../src/host-supervisor' vi.mock('node:child_process', { spy: true }) @@ -112,6 +113,34 @@ describe('desktop Host readiness', () => { }) }) +describe('desktop Host port', () => { + it('uses fixed ports for packaged and development builds without an override', () => { + expect(hostSupervisor.DESKTOP_PACKAGED_PORT).toBe(24_827) + expect(hostSupervisor.DESKTOP_DEV_PORT).toBe(24_828) + expect(hostSupervisor.resolveDesktopPort({}, true)).toBe(24_827) + expect(hostSupervisor.resolveDesktopPort({}, false)).toBe(24_828) + }) + + it('uses a valid port override for packaged and development builds', () => { + const env = { PYTHINKER_DESKTOP_PORT: '45231' } + + expect(hostSupervisor.resolveDesktopPort(env, true)).toBe(45_231) + expect(hostSupervisor.resolveDesktopPort(env, false)).toBe(45_231) + }) + + it.each(['not-a-port', '70000'])('rejects an invalid port override: %s', (value) => { + expect(() => hostSupervisor.resolveDesktopPort({ PYTHINKER_DESKTOP_PORT: value }, true)) + .toThrow(new RegExp(`PYTHINKER_DESKTOP_PORT.*${value}`, 'u')) + }) + + it('detects output that reports a port collision', () => { + expect(hostSupervisor.isPortInUseError( + 'listen EADDRINUSE: address already in use 127.0.0.1:24827', + )).toBe(true) + expect(hostSupervisor.isPortInUseError('desktop Host exited before readiness (code 1, signal null)')).toBe(false) + }) +}) + describe('desktop Host supervisor', () => { it('starts one child for concurrent callers and returns its stdout readiness URL', async () => { const child = new FakeHostChild() @@ -296,6 +325,7 @@ describe('desktop Host process', () => { cliEntry: '/Applications/Pythinker.app/Contents/Resources/host/node_modules/@pymodel/pythinker-code/dist/launcher.mjs', cwd: '/Users/tester', env: { PYTHINKER_DESKTOP: '1' }, + port: 24_827, electronRunAsNode: true, }) @@ -307,7 +337,7 @@ describe('desktop Host process', () => { 'run', '--foreground', '--port', - '0', + '24827', '--log-level', 'error', ], @@ -341,6 +371,7 @@ describe('desktop Host process', () => { cliEntry: '/tmp/launcher.mjs', cwd: '/tmp', env: {}, + port: 24_827, }) host.kill('SIGTERM') @@ -379,6 +410,7 @@ describe('desktop Host process', () => { cliEntry: '/tmp/launcher.mjs', cwd: '/tmp', env: {}, + port: 24_827, }) host.kill('SIGTERM') @@ -404,6 +436,7 @@ describe('desktop Host process', () => { cliEntry: '/tmp/launcher.mjs', cwd: '/tmp', env: {}, + port: 24_827, }) host.kill('SIGTERM') diff --git a/apps/desktop/tests/updater.spec.ts b/apps/desktop/tests/updater.spec.ts index 232c50ae..c26899d9 100644 --- a/apps/desktop/tests/updater.spec.ts +++ b/apps/desktop/tests/updater.spec.ts @@ -6,15 +6,26 @@ import { afterEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => ({ app: { isPackaged: false, - getPath: () => '', + getPath: vi.fn(() => ''), + once: vi.fn(), }, })) vi.mock('electron-updater', () => ({ - default: { autoUpdater: {} }, + default: { + autoUpdater: { + on: vi.fn(), + checkForUpdates: vi.fn(), + quitAndInstall: vi.fn(), + }, + }, })) +import { app } from 'electron' +import electronUpdater from 'electron-updater' import { + getUpdateState, + initUpdater, readUpdateSettings, trackUpdateTransition, writeUpdateSettings, @@ -22,11 +33,22 @@ import { } from '../src/updater' const temporaryDirectories: string[] = [] +const resourcesPathDescriptor = Object.getOwnPropertyDescriptor(process, 'resourcesPath') +const { autoUpdater } = electronUpdater afterEach(() => { for (const directory of temporaryDirectories.splice(0)) { rmSync(directory, { recursive: true, force: true }) } + Object.defineProperty(app, 'isPackaged', { configurable: true, value: false }) + vi.mocked(app.getPath).mockReset() + vi.mocked(app.getPath).mockReturnValue('') + if (resourcesPathDescriptor === undefined) { + Reflect.deleteProperty(process, 'resourcesPath') + } else { + Object.defineProperty(process, 'resourcesPath', resourcesPathDescriptor) + } + vi.clearAllMocks() }) function temporaryDirectory(): string { @@ -76,3 +98,23 @@ describe('update telemetry transitions', () => { ]) }) }) + +describe('packaged builds without update metadata', () => { + it('disables updates without wiring updater events', () => { + const directory = temporaryDirectory() + writeUpdateSettings(directory, { autoUpdate: false }) + vi.mocked(app.getPath).mockReturnValue(directory) + Object.defineProperty(app, 'isPackaged', { configurable: true, value: true }) + Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory }) + + initUpdater(() => undefined) + + expect(getUpdateState()).toMatchObject({ + status: 'disabled', + message: 'Updates are not available for this build', + autoUpdate: false, + }) + expect(autoUpdater.on).not.toHaveBeenCalled() + expect(app.once).not.toHaveBeenCalled() + }) +})