Skip to content

Commit bbad512

Browse files
committed
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:<port> 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.
1 parent fa82753 commit bbad512

5 files changed

Lines changed: 168 additions & 26 deletions

File tree

apps/desktop/src/host-supervisor.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,26 @@ const DEFAULT_SHUTDOWN_TIMEOUT_MS = 5_000
99
const TASKKILL_TIMEOUT_MS = 5_000
1010
const MAX_STARTUP_OUTPUT_CHARS = 32_768
1111

12+
export const DESKTOP_PACKAGED_PORT = 24_827
13+
export const DESKTOP_DEV_PORT = 24_828
14+
15+
/** Resolve the fixed Host port, with an optional validated environment override. */
16+
export function resolveDesktopPort(env: NodeJS.ProcessEnv, isPackaged: boolean): number {
17+
const value = env['PYTHINKER_DESKTOP_PORT']
18+
if (value === undefined) return isPackaged ? DESKTOP_PACKAGED_PORT : DESKTOP_DEV_PORT
19+
20+
const port = Number(value)
21+
if (!/^\d+$/u.test(value) || !Number.isInteger(port) || port < 1 || port > 65_535) {
22+
throw new Error(`PYTHINKER_DESKTOP_PORT must be an integer from 1 to 65535; received ${JSON.stringify(value)}`)
23+
}
24+
return port
25+
}
26+
27+
/** Return whether Host output reports that its fixed port is already occupied. */
28+
export function isPortInUseError(message: string): boolean {
29+
return /EADDRINUSE|address already in use/iu.test(message)
30+
}
31+
1232
/** Incremental parser for the Web Host's canonical readiness line. */
1333
export interface ReadinessParser {
1434
/**
@@ -257,6 +277,8 @@ export interface SpawnPythinkerServerOptions {
257277
readonly cwd: string
258278
/** Frozen environment for the Host process. */
259279
readonly env: NodeJS.ProcessEnv
280+
/** Fixed loopback port for the Host server. */
281+
readonly port: number
260282
/** Run the Electron executable as its bundled Node runtime. */
261283
readonly electronRunAsNode?: boolean
262284
}
@@ -272,7 +294,7 @@ function streamAdapter(stream: NodeJS.ReadableStream): HostChild['stdout'] {
272294
}
273295

274296
/**
275-
* Spawn the production Pythinker server on an OS-assigned loopback port.
297+
* Spawn the production Pythinker server on a fixed loopback port.
276298
* @param options - Node runtime, built CLI and process environment.
277299
* @returns The child handle consumed by {@link createHostSupervisor}.
278300
*/
@@ -286,7 +308,7 @@ export function spawnPythinkerServer(options: SpawnPythinkerServerOptions): Host
286308
'run',
287309
'--foreground',
288310
'--port',
289-
'0',
311+
String(options.port),
290312
'--log-level',
291313
'error',
292314
], {

apps/desktop/src/main.ts

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@ import {
2323
setCrashPhase,
2424
track,
2525
} from '@pymodel/pythinker-telemetry'
26-
import { createHostSupervisor, spawnPythinkerServer, type HostSupervisor } from './host-supervisor'
26+
import {
27+
createHostSupervisor,
28+
isPortInUseError,
29+
resolveDesktopPort,
30+
spawnPythinkerServer,
31+
type HostSupervisor,
32+
} from './host-supervisor'
2733
import { createSplashWindow } from './splash'
2834
import {
2935
checkForUpdatesNow,
@@ -303,6 +309,7 @@ async function boot(): Promise<void> {
303309
if (bootQuitPromise !== undefined) return
304310
initializeDesktopTelemetry()
305311
const paths = hostPaths()
312+
const port = resolveDesktopPort(process.env, app.isPackaged)
306313
assertHostArtifacts(paths)
307314
const splash = createSplashWindow(desktopResources('splash'))
308315
const destroySplash = (): void => {
@@ -313,26 +320,43 @@ async function boot(): Promise<void> {
313320
const trayFrames = loadTrayImages()
314321
createTray(trayFrames)
315322
stopTrayAnimation = startTrayAnimation(trayFrames)
316-
host = createHostSupervisor({
317-
spawnHost: () => spawnPythinkerServer({
318-
...paths,
319-
env: {
320-
...process.env,
321-
PYTHINKER_DESKTOP: '1',
323+
for (;;) {
324+
host = createHostSupervisor({
325+
spawnHost: () => spawnPythinkerServer({
326+
...paths,
327+
env: {
328+
...process.env,
329+
PYTHINKER_DESKTOP: '1',
330+
},
331+
port,
332+
}),
333+
log: chunk => process.stderr.write(chunk),
334+
onUnexpectedExit: ({ code, signal }) => {
335+
console.error(`desktop Host exited unexpectedly (code ${String(code)}, signal ${String(signal)})`)
336+
void requestAppQuit()
322337
},
323-
}),
324-
log: chunk => process.stderr.write(chunk),
325-
onUnexpectedExit: ({ code, signal }) => {
326-
console.error(`desktop Host exited unexpectedly (code ${String(code)}, signal ${String(signal)})`)
327-
void requestAppQuit()
328-
},
329-
})
330-
try {
331-
hostOrigin = await host.start()
332-
track('desktop_server_ready')
333-
} catch (error) {
334-
track('desktop_server_failed')
335-
throw error
338+
})
339+
try {
340+
hostOrigin = await host.start()
341+
track('desktop_server_ready')
342+
break
343+
} catch (error) {
344+
track('desktop_server_failed')
345+
const message = error instanceof Error ? error.message : String(error)
346+
if (!isPortInUseError(message)) throw error
347+
348+
const result = await dialog.showMessageBox({
349+
type: 'error',
350+
buttons: ['Retry', 'Quit'],
351+
defaultId: 0,
352+
cancelId: 1,
353+
title: `${APP_NAME} needs its fixed port`,
354+
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.`,
355+
})
356+
if (result.response === 0) continue
357+
await requestAppQuit()
358+
return
359+
}
336360
}
337361
stopTrayAnimation?.()
338362
stopTrayAnimation = undefined

apps/desktop/src/updater.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { readFileSync, writeFileSync } from 'node:fs'
1+
import { existsSync, readFileSync, writeFileSync } from 'node:fs'
22
import { join } from 'node:path'
33
import { app, type BrowserWindow } from 'electron'
44
import electronUpdater from 'electron-updater'
55

66
const { autoUpdater } = electronUpdater
77
const UPDATE_SETTINGS_FILE = 'update-settings.json'
8+
const UPDATES_UNAVAILABLE_MESSAGE = 'Updates are not available for this build'
89
const INITIAL_CHECK_DELAY_MS = 10_000
910
const CHECK_INTERVAL_MS = 4 * 60 * 60 * 1_000
1011

@@ -117,6 +118,20 @@ function clearTimers(): void {
117118
checkInterval = undefined
118119
}
119120

121+
function hasUpdateConfig(): boolean {
122+
return existsSync(join(process.resourcesPath, 'app-update.yml'))
123+
}
124+
125+
function disableUpdates(): UpdateState {
126+
updateState({
127+
status: 'disabled',
128+
message: UPDATES_UNAVAILABLE_MESSAGE,
129+
version: undefined,
130+
percent: undefined,
131+
})
132+
return state
133+
}
134+
120135
function scheduleChecks(): void {
121136
if (checkInterval !== undefined) return
122137
checkInterval = setInterval(() => {
@@ -181,6 +196,10 @@ export function initUpdater(
181196
clearTimers()
182197

183198
if (!app.isPackaged) return
199+
if (!hasUpdateConfig()) {
200+
disableUpdates()
201+
return
202+
}
184203

185204
try {
186205
autoUpdater.autoDownload = settings.autoUpdate
@@ -232,6 +251,7 @@ export async function checkForUpdatesNow(): Promise<UpdateState> {
232251
updateState({ status: 'disabled' })
233252
return state
234253
}
254+
if (!hasUpdateConfig()) return disableUpdates()
235255

236256
try {
237257
autoUpdater.autoDownload = true
@@ -251,6 +271,7 @@ export function quitAndInstallNow(): UpdateState {
251271
updateState({ status: 'disabled' })
252272
return state
253273
}
274+
if (!hasUpdateConfig()) return disableUpdates()
254275

255276
try {
256277
updateTelemetryTrack('desktop_update_install')

apps/desktop/tests/host-supervisor.spec.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
createReadinessParser,
66
type HostChild,
77
} from '../src/host-supervisor'
8+
import * as hostSupervisor from '../src/host-supervisor'
89

910
vi.mock('node:child_process', { spy: true })
1011

@@ -112,6 +113,34 @@ describe('desktop Host readiness', () => {
112113
})
113114
})
114115

116+
describe('desktop Host port', () => {
117+
it('uses fixed ports for packaged and development builds without an override', () => {
118+
expect(hostSupervisor.DESKTOP_PACKAGED_PORT).toBe(24_827)
119+
expect(hostSupervisor.DESKTOP_DEV_PORT).toBe(24_828)
120+
expect(hostSupervisor.resolveDesktopPort({}, true)).toBe(24_827)
121+
expect(hostSupervisor.resolveDesktopPort({}, false)).toBe(24_828)
122+
})
123+
124+
it('uses a valid port override for packaged and development builds', () => {
125+
const env = { PYTHINKER_DESKTOP_PORT: '45231' }
126+
127+
expect(hostSupervisor.resolveDesktopPort(env, true)).toBe(45_231)
128+
expect(hostSupervisor.resolveDesktopPort(env, false)).toBe(45_231)
129+
})
130+
131+
it.each(['not-a-port', '70000'])('rejects an invalid port override: %s', (value) => {
132+
expect(() => hostSupervisor.resolveDesktopPort({ PYTHINKER_DESKTOP_PORT: value }, true))
133+
.toThrow(new RegExp(`PYTHINKER_DESKTOP_PORT.*${value}`, 'u'))
134+
})
135+
136+
it('detects output that reports a port collision', () => {
137+
expect(hostSupervisor.isPortInUseError(
138+
'listen EADDRINUSE: address already in use 127.0.0.1:24827',
139+
)).toBe(true)
140+
expect(hostSupervisor.isPortInUseError('desktop Host exited before readiness (code 1, signal null)')).toBe(false)
141+
})
142+
})
143+
115144
describe('desktop Host supervisor', () => {
116145
it('starts one child for concurrent callers and returns its stdout readiness URL', async () => {
117146
const child = new FakeHostChild()
@@ -296,6 +325,7 @@ describe('desktop Host process', () => {
296325
cliEntry: '/Applications/Pythinker.app/Contents/Resources/host/node_modules/@pymodel/pythinker-code/dist/launcher.mjs',
297326
cwd: '/Users/tester',
298327
env: { PYTHINKER_DESKTOP: '1' },
328+
port: 24_827,
299329
electronRunAsNode: true,
300330
})
301331

@@ -307,7 +337,7 @@ describe('desktop Host process', () => {
307337
'run',
308338
'--foreground',
309339
'--port',
310-
'0',
340+
'24827',
311341
'--log-level',
312342
'error',
313343
],
@@ -341,6 +371,7 @@ describe('desktop Host process', () => {
341371
cliEntry: '/tmp/launcher.mjs',
342372
cwd: '/tmp',
343373
env: {},
374+
port: 24_827,
344375
})
345376
host.kill('SIGTERM')
346377

@@ -379,6 +410,7 @@ describe('desktop Host process', () => {
379410
cliEntry: '/tmp/launcher.mjs',
380411
cwd: '/tmp',
381412
env: {},
413+
port: 24_827,
382414
})
383415
host.kill('SIGTERM')
384416

@@ -404,6 +436,7 @@ describe('desktop Host process', () => {
404436
cliEntry: '/tmp/launcher.mjs',
405437
cwd: '/tmp',
406438
env: {},
439+
port: 24_827,
407440
})
408441
host.kill('SIGTERM')
409442

apps/desktop/tests/updater.spec.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,49 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
66
vi.mock('electron', () => ({
77
app: {
88
isPackaged: false,
9-
getPath: () => '',
9+
getPath: vi.fn(() => ''),
10+
once: vi.fn(),
1011
},
1112
}))
1213

1314
vi.mock('electron-updater', () => ({
14-
default: { autoUpdater: {} },
15+
default: {
16+
autoUpdater: {
17+
on: vi.fn(),
18+
checkForUpdates: vi.fn(),
19+
quitAndInstall: vi.fn(),
20+
},
21+
},
1522
}))
1623

24+
import { app } from 'electron'
25+
import electronUpdater from 'electron-updater'
1726
import {
27+
getUpdateState,
28+
initUpdater,
1829
readUpdateSettings,
1930
trackUpdateTransition,
2031
writeUpdateSettings,
2132
type UpdateState,
2233
} from '../src/updater'
2334

2435
const temporaryDirectories: string[] = []
36+
const resourcesPathDescriptor = Object.getOwnPropertyDescriptor(process, 'resourcesPath')
37+
const { autoUpdater } = electronUpdater
2538

2639
afterEach(() => {
2740
for (const directory of temporaryDirectories.splice(0)) {
2841
rmSync(directory, { recursive: true, force: true })
2942
}
43+
Object.defineProperty(app, 'isPackaged', { configurable: true, value: false })
44+
vi.mocked(app.getPath).mockReset()
45+
vi.mocked(app.getPath).mockReturnValue('')
46+
if (resourcesPathDescriptor === undefined) {
47+
Reflect.deleteProperty(process, 'resourcesPath')
48+
} else {
49+
Object.defineProperty(process, 'resourcesPath', resourcesPathDescriptor)
50+
}
51+
vi.clearAllMocks()
3052
})
3153

3254
function temporaryDirectory(): string {
@@ -76,3 +98,23 @@ describe('update telemetry transitions', () => {
7698
])
7799
})
78100
})
101+
102+
describe('packaged builds without update metadata', () => {
103+
it('disables updates without wiring updater events', () => {
104+
const directory = temporaryDirectory()
105+
writeUpdateSettings(directory, { autoUpdate: false })
106+
vi.mocked(app.getPath).mockReturnValue(directory)
107+
Object.defineProperty(app, 'isPackaged', { configurable: true, value: true })
108+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
109+
110+
initUpdater(() => undefined)
111+
112+
expect(getUpdateState()).toMatchObject({
113+
status: 'disabled',
114+
message: 'Updates are not available for this build',
115+
autoUpdate: false,
116+
})
117+
expect(autoUpdater.on).not.toHaveBeenCalled()
118+
expect(app.once).not.toHaveBeenCalled()
119+
})
120+
})

0 commit comments

Comments
 (0)