Skip to content

Commit c3d8f62

Browse files
fix(ports): auto-fallback for the gateway (7878) and media (7879) ports
Increment 3+4 of the free-port work. The gateway and loopback media server now scan upward from their preferred port when a second Off Grid instance already holds it, matching the llama engine (8439) fallback. - model-server: startModelServer picks a free gateway port via pickFreePort; getGatewayPort() exposes the live bound port; async now, so fire-and-forget callers (ipc, index) use void. - setup: health pings read the LIVE ports (llm.getPort() / getGatewayPort()), never the fixed constants. - media-server: listenOnFreePort scans from MEDIA_PORT; urlFor already serves the live boundPort. - wiring guards extended for gateway + setup live-port reads; media fallback integration test.
1 parent 77de3e8 commit c3d8f62

8 files changed

Lines changed: 119 additions & 32 deletions

src/main/__tests__/media-server.integration.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,24 @@ describe('loopback media server integration', () => {
9090
await expect(server.urlFor(path.join(profile, 'captures', 'missing.png'))).resolves.toBeNull()
9191
})
9292
})
93+
94+
describe('LoopbackMediaServer — free-port fallback', () => {
95+
it('binds a DIFFERENT free port when the preferred one is already taken', async () => {
96+
const net = await import('node:net')
97+
// Occupy a port, then ask the media server to use THAT preferred port.
98+
const blocker = net.createServer()
99+
await new Promise<void>((r) => blocker.listen(0, '127.0.0.1', r))
100+
const taken = (blocker.address() as import('node:net').AddressInfo).port
101+
const media = new LoopbackMediaServer({ roots: localMediaRoots(profile), port: taken, token: 't' })
102+
try {
103+
await media.start()
104+
// Fell back: bound a real, DIFFERENT port (not the occupied one) — and urlFor serves it.
105+
const url = await media.urlFor(fixturePath('image'))
106+
expect(url).toContain('http://127.0.0.1:')
107+
expect(url).not.toContain(`:${taken}/`)
108+
} finally {
109+
await media.close()
110+
await new Promise<void>((r) => blocker.close(() => r()))
111+
}
112+
})
113+
})

src/main/__tests__/model-port-ownership.integration.test.ts

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
/**
2-
* RELEASE_TEST_CHECKLIST #146 - fixed model ports are single-owner.
2+
* RELEASE_TEST_CHECKLIST #146 - a held model port never dead-ends the app.
33
*
44
* The first owner is either the already-running healthy production llama-server or a separate
55
* process launching the only fake: a behaviour-faithful native llama-server boundary on the real
6-
* production port. The contender is the production LLMService. Real lsof/ps parent ownership,
7-
* loopback HTTP, GGUF validation, model resolution, startup refusal, error classification, and
8-
* chat-health presentation remain real. Cleanup only owns processes this test spawned.
6+
* production port. The contender is the production LLMService, which - rather than refusing when
7+
* the preferred port is taken - scans upward for a free port and starts its own engine there, so
8+
* the app works even when something else holds :8439. Real lsof/ps parent ownership, loopback
9+
* HTTP, GGUF validation, model resolution, free-port fallback, and chat-health presentation
10+
* remain real. Cleanup only owns processes this test spawned.
911
*/
1012
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
1113
import { execSync, spawn, type ChildProcess } from 'node:child_process'
@@ -73,10 +75,14 @@ const server = http.createServer((req, res) => {
7375
server.listen(port, '127.0.0.1', () => {
7476
const address = server.address()
7577
const actualPort = typeof address === 'object' && address ? address.port : port
76-
fs.appendFileSync(
77-
process.env.OFFGRID_TEST_ENGINE_LOG,
78-
String(process.pid) + ':' + String(actualPort) + '\\n'
79-
)
78+
// Only the test-spawned FIRST owner sets this log; the production LLMService spawning the
79+
// same binary on its fallback port does NOT, so it must not crash on a missing log path.
80+
if (process.env.OFFGRID_TEST_ENGINE_LOG) {
81+
fs.appendFileSync(
82+
process.env.OFFGRID_TEST_ENGINE_LOG,
83+
String(process.pid) + ':' + String(actualPort) + '\\n'
84+
)
85+
}
8086
})
8187
process.on('SIGTERM', () => server.close(() => process.exit(0)))
8288
`
@@ -233,19 +239,24 @@ afterAll(async () => {
233239
})
234240

235241
describe('model port ownership', () => {
236-
it('preserves the first live engine and explains the second-instance conflict (#146)', async () => {
242+
it('preserves the first live engine and falls back to a free port for the second (#146)', async () => {
237243
const [{ llm }, { getSystemHealth }, { modelPortConflictReason }] = await Promise.all([
238244
import('../llm'),
239245
import('../setup'),
240246
import('../llama-error')
241247
])
242248
const conflict = modelPortConflictReason(LLAMA_SERVER_PORT)
243249

244-
await expect(llm.init()).rejects.toThrow(conflict)
245-
expect(llm.isReady()).toBe(false)
246-
expect(llm.isStarting()).toBe(false)
247-
expect(llm.lastError()).toBe(conflict)
250+
// The preferred port is held by the first live engine. Rather than dead-ending on a
251+
// single-owner conflict, the second instance scans upward and starts its own engine on a
252+
// free port — the app just works even when something else holds :8439.
253+
await llm.init()
254+
expect(llm.isReady()).toBe(true)
255+
expect(llm.getPort()).not.toBe(LLAMA_SERVER_PORT)
256+
// The conflict reason is NOT surfaced — we moved instead of refusing.
257+
expect(llm.lastError()).not.toBe(conflict)
248258

259+
// The FIRST engine is untouched: still alive, still the sole owner of the preferred port.
249260
expect(processIsAlive(enginePid)).toBe(true)
250261
expect(await engineIsReady()).toBe(true)
251262
if (liveOwner) {
@@ -254,9 +265,14 @@ describe('model port ownership', () => {
254265
])
255266
}
256267

268+
// Chat health reports UP, on the fallback port — not down with a port-conflict detail.
257269
const chatHealth = (await getSystemHealth()).components.find(
258270
(component) => component.id === 'chat'
259271
)
260-
expect(chatHealth).toMatchObject({ status: 'down', detail: conflict, port: LLAMA_SERVER_PORT })
272+
expect(chatHealth).toMatchObject({ status: 'ready', port: llm.getPort() })
273+
expect(chatHealth?.detail).not.toBe(conflict)
274+
275+
// Tear down the second engine this test started (the first owner is cleaned up in afterAll).
276+
await llm.unload()
261277
})
262278
})

src/main/__tests__/port-fallback-wiring.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,30 @@ describe('model-server.ts — gateway proxies to the LIVE engine port', () => {
4242
expect(src).toMatch(/port: upstreamPort\(\)/)
4343
})
4444
})
45+
46+
describe('model-server.ts — the gateway itself falls back off a held port', () => {
47+
const src = read('model-server.ts')
48+
49+
it('scans for a free gateway port with pickFreePort before listening', () => {
50+
expect(src).toMatch(/boundGatewayPort = \(await pickFreePort\(port\)\)/)
51+
// It binds the LIVE chosen port, not the fixed GATEWAY_PORT constant.
52+
expect(src).toMatch(/server\.listen\(boundGatewayPort/)
53+
})
54+
55+
it('exposes the live gateway port via getGatewayPort()', () => {
56+
expect(src).toMatch(/getGatewayPort\(\): number\s*{\s*return boundGatewayPort/)
57+
})
58+
})
59+
60+
describe('setup.ts — health pings read the LIVE ports, never fixed constants', () => {
61+
const src = read('setup.ts')
62+
63+
it('pings the live llama engine port via llm.getPort()', () => {
64+
expect(src).toMatch(/pingJson\(llm\.getPort\(\)\)/)
65+
})
66+
67+
it('pings the live gateway port via getGatewayPort(), not a GATEWAY_PORT constant', () => {
68+
expect(src).toMatch(/pingJson\(getGatewayPort\(\)\)/)
69+
expect(src).not.toMatch(/\bLLAMA_PORT\b/)
70+
})
71+
})

src/main/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ app.whenReady().then(() => {
199199
}
200200
}
201201
try {
202-
startModelServer()
202+
void startModelServer()
203203
} catch (e) {
204204
console.error('[gateway] start failed', e)
205205
}
@@ -329,7 +329,7 @@ app.whenReady().then(() => {
329329
setupIPC()
330330
setupRagIPC()
331331
setupMcpIpc() // basic MCP connectors (management + chat tool extension)
332-
startModelServer() // one OpenAI-compatible local gateway on :7878 (LLM + STT)
332+
void startModelServer() // one OpenAI-compatible local gateway (LLM + STT); auto-picks a free port
333333
startMediaServer() // loopback HTTP for seekable local media (meeting videos)
334334
// Heal a stale active-model.json whose model gained a vision projector after it was
335335
// activated (e.g. Gemma 4 E2B) — turns vision on at launch if the projector is now

src/main/ipc.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1652,7 +1652,7 @@ export function setupIPC() {
16521652
} catch {
16531653
/* not running */
16541654
}
1655-
startModelServer() // re-listens; if the port is held by a non-Off-Grid process it logs and no-ops
1655+
void startModelServer() // re-listens; falls back to a free port if the preferred one is held
16561656
return { success: true }
16571657
}
16581658
return { success: false, error: `cannot restart "${id}"` }

src/main/media-server.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { randomUUID } from 'crypto'
1818
import { app } from 'electron'
1919
import { parseRange, isPathAllowed } from './media-range'
2020
import { MEDIA_PORT } from '../shared/ports'
21+
import { pickFreePort } from './free-port'
2122
import { mimeForExt } from './mime'
2223
import { localMediaRoots } from './media-roots'
2324

@@ -62,10 +63,21 @@ export class LoopbackMediaServer {
6263
start(): Promise<void> {
6364
if (this.boundPort > 0) return Promise.resolve()
6465
if (this.startPromise) return this.startPromise
66+
this.startPromise = this.listenOnFreePort()
67+
return this.startPromise
68+
}
6569

70+
private async listenOnFreePort(): Promise<void> {
71+
// The preferred media port (MEDIA_PORT) may be taken by another Off Grid instance; scan upward
72+
// for a free one. requestedPort 0 = let the OS assign (tests) — inherently free. urlFor() serves
73+
// the LIVE boundPort, so downstream links follow wherever it bound.
74+
const target =
75+
this.requestedPort > 0
76+
? ((await pickFreePort(this.requestedPort)) ?? this.requestedPort)
77+
: 0
6678
const candidate = http.createServer((req, res) => this.handle(req, res))
6779
this.server = candidate
68-
this.startPromise = new Promise<void>((resolve, reject) => {
80+
await new Promise<void>((resolve, reject) => {
6981
const fail = (error: Error): void => {
7082
if (this.server === candidate) {
7183
this.server = null
@@ -75,7 +87,7 @@ export class LoopbackMediaServer {
7587
reject(error)
7688
}
7789
candidate.once('error', fail)
78-
candidate.listen(this.requestedPort, '127.0.0.1', () => {
90+
candidate.listen(target, '127.0.0.1', () => {
7991
candidate.off('error', fail)
8092
candidate.on('error', (error) => console.error('[media-server]', error))
8193
const address = candidate.address()
@@ -89,7 +101,6 @@ export class LoopbackMediaServer {
89101
resolve()
90102
})
91103
})
92-
return this.startPromise
93104
}
94105

95106
async urlFor(absPath: string): Promise<string | null> {

src/main/model-server.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { docsText, docsHtml, openApiSpec } from './api-docs'
3535
import { handleMcpRequest } from './mcp-server'
3636
import { llm, type LlmSettings } from './llm'
3737
import { GATEWAY_HOST, GATEWAY_PORT } from '../shared/ports'
38+
import { pickFreePort } from './free-port'
3839
import { retryWithDeadline } from './lib/retry'
3940
import { resolveDims } from './model-server/dimensions'
4041
import { guardProxyStreams } from './stream-guards'
@@ -929,9 +930,20 @@ async function handleImageEdit(
929930
}
930931

931932
// ─── Server ──────────────────────────────────────────────────────────────────
932-
/** Start the unified local model gateway. Bound to loopback (local-only). */
933-
export function startModelServer(port = GATEWAY_PORT): void {
934-
if (server) return
933+
/** The port the gateway actually bound. Falls back off GATEWAY_PORT when it's taken (a 2nd Off Grid
934+
* instance); consumers (setup health ping, the Gateway UI) must read this LIVE value. */
935+
let boundGatewayPort = GATEWAY_PORT
936+
export function getGatewayPort(): number {
937+
return boundGatewayPort
938+
}
939+
let startingGateway = false
940+
941+
/** Start the unified local model gateway. Bound to loopback (local-only). Async because it scans
942+
* for a free port when the preferred one is taken. */
943+
export async function startModelServer(port = GATEWAY_PORT): Promise<void> {
944+
if (server || startingGateway) return
945+
startingGateway = true
946+
boundGatewayPort = (await pickFreePort(port)) ?? port
935947

936948
server = http.createServer(async (req, res) => {
937949
res.setHeader('Access-Control-Allow-Origin', '*')
@@ -1211,9 +1223,10 @@ export function startModelServer(port = GATEWAY_PORT): void {
12111223
server.on('error', (e) => console.error('[model-server]', e))
12121224
// The gateway has no authentication. Bind the socket itself to loopback so no
12131225
// route can become LAN-accessible through a missing per-handler authorization check.
1214-
server.listen(port, GATEWAY_HOST, () => {
1215-
console.log(`[model-server] multimodal gateway at http://${GATEWAY_HOST}:${port}/v1`)
1226+
server.listen(boundGatewayPort, GATEWAY_HOST, () => {
1227+
console.log(`[model-server] multimodal gateway at http://${GATEWAY_HOST}:${boundGatewayPort}/v1`)
12161228
})
1229+
startingGateway = false
12171230
}
12181231

12191232
export function stopModelServer(): void {

src/main/setup.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
setActiveModel,
1818
setActiveModalChoice
1919
} from './models-manager'
20-
import { LLAMA_SERVER_PORT, GATEWAY_PORT } from '../shared/ports'
20+
import { getGatewayPort } from './model-server'
2121
import { deviceNoun } from '../shared/device'
2222
import type {
2323
SystemHealthComponentContract,
@@ -50,7 +50,6 @@ export interface SetupProgress {
5050
}
5151
export type SetupProgressCb = (p: SetupProgress) => void
5252

53-
const LLAMA_PORT = LLAMA_SERVER_PORT
5453

5554
/** GET a localhost endpoint, parse JSON, with a short timeout. null on any failure. */
5655
function pingJson(port: number, path = '/health', timeoutMs = 1500): Promise<unknown | null> {
@@ -92,8 +91,8 @@ export async function getSystemHealth(): Promise<SystemHealth> {
9291

9392
// Live probes (run in parallel): the chat server and the gateway.
9493
const [llamaHealth, gatewayHealth] = await Promise.all([
95-
pingJson(LLAMA_PORT),
96-
pingJson(GATEWAY_PORT)
94+
pingJson(llm.getPort()),
95+
pingJson(getGatewayPort())
9796
])
9897

9998
// Image generation is checked in-process (no HTTP) so it works even if the
@@ -134,15 +133,15 @@ export async function getSystemHealth(): Promise<SystemHealth> {
134133
label: 'Chat model (llama-server)',
135134
status: chat,
136135
detail: chatDetail,
137-
port: LLAMA_PORT,
136+
port: llm.getPort(),
138137
canRestart: modelsExist
139138
},
140139
{
141140
id: 'gateway',
142141
label: 'Local gateway',
143142
status: gatewayHealth ? 'ready' : 'down',
144143
detail: gatewayHealth ? 'OpenAI-compatible API' : 'Not responding',
145-
port: GATEWAY_PORT,
144+
port: getGatewayPort(),
146145
canRestart: true
147146
},
148147
{
@@ -403,7 +402,7 @@ export async function autoConfigure(
403402
}
404403

405404
emit({ phase: 'verify', message: 'Verifying…', modelId: model.id, modelName: model.name })
406-
const ok = !!(await pingJson(LLAMA_PORT, '/health', 3000))
405+
const ok = !!(await pingJson(llm.getPort(), '/health', 3000))
407406

408407
// Chat is live — now set up the rest of the baseline (speech-to-text, text-to-
409408
// speech, and image outside Conservative). These are best-effort: a failure here

0 commit comments

Comments
 (0)