Merge pull request #67 from off-grid-ai/fix/port-fallback-review-followups

fix(ports): CodeRabbit follow-ups on the merged port-fallback work

Author: alichherawalla

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

@@ -57,13 +57,21 @@ describe('model-server.ts — the gateway itself falls back off a held port', ()
   it('scans for a free gateway port with pickFreePort before listening', () => {
     expect(src).toMatch(/boundGatewayPort = \(await pickFreePort\(port\)\)/)
     // It binds the LIVE chosen port, not the fixed GATEWAY_PORT constant.
-    expect(src).toMatch(/server\.listen\(boundGatewayPort/)
+    expect(src).toMatch(/\.listen\(boundGatewayPort/)
   })
 
   it('exposes the live gateway port via getGatewayPort()', () => {
     expect(src).toMatch(/getGatewayPort\(\): number\s*{\s*return boundGatewayPort/)
   })
 
+  it('startup resolves only after bind and rejects (clearing state) on a bind error', () => {
+    // A bind failure must reject and drop the dead server, not resolve a false success that leaves
+    // `server` set forever. Guard the await-the-listen shape: reject on error, null the server.
+    expect(src).toMatch(/await new Promise<void>\(\(resolve, reject\)/)
+    expect(src).toMatch(/once\('listening'/)
+    expect(src).toMatch(/catch[\s\S]*?server = null[\s\S]*?throw e/)
+  })
+
   it('self-report routes advertise the LIVE bound port, never the preferred param', () => {
     // After a fallback, /, /health, /openapi.json, /docs, /v1 must point clients at the port the
     // gateway actually bound (boundGatewayPort), not the stale preferred `port` it may have moved off.
@@ -85,6 +93,8 @@ describe('setup.ts — health pings read the LIVE ports, never fixed constants',
 
   it('pings the live gateway port via getGatewayPort(), not a GATEWAY_PORT constant', () => {
     expect(src).toMatch(/pingJson\(getGatewayPort\(\)\)/)
-    expect(src).not.toMatch(/\bLLAMA_PORT\b/)
+    // Guard the actual regression this describes: setup must not reach for the fixed gateway-port
+    // constant again (it reads the live getGatewayPort()); LLAMA_PORT was never the concern here.
+    expect(src).not.toMatch(/\bGATEWAY_PORT\b/)
   })
 })

src/main/free-port.ts

@@ -1,6 +1,6 @@
 // Gradual-increment free-port fallback for the loopback services (llama-server, gateway, media).
 // The ports in shared/ports.ts are PREFERRED, not mandatory: if another app owns one (LM Studio on
-// :8439, a second Off Grid instance, anything else), we must not fight over it or dead-end — we scan
+// :8439, a second Off Grid AI Desktop instance, anything else), we must not fight over it or dead-end — we scan
 // upward (+1, +2, …) for the first free port and bind there. The candidate math is pure (unit-
 // tested); the socket probe is injected so selection is testable without real sockets.
 

src/main/media-server.ts

@@ -68,7 +68,7 @@ export class LoopbackMediaServer {
   }
 
   private async listenOnFreePort(): Promise<void> {
-    // The preferred media port (MEDIA_PORT) may be taken by another Off Grid instance; scan upward
+    // The preferred media port (MEDIA_PORT) may be taken by another Off Grid AI Desktop instance; scan upward
     // for a free one. requestedPort 0 = let the OS assign (tests) — inherently free. urlFor() serves
     // the LIVE boundPort, so downstream links follow wherever it bound.
     const target =

src/main/model-server.ts

@@ -930,7 +930,7 @@ async function handleImageEdit(
 }
 
 // ─── Server ──────────────────────────────────────────────────────────────────
-/** The port the gateway actually bound. Falls back off GATEWAY_PORT when it's taken (a 2nd Off Grid
+/** The port the gateway actually bound. Falls back off GATEWAY_PORT when it's taken (a 2nd Off Grid AI Desktop
  *  instance); consumers (setup health ping, the Gateway UI) must read this LIVE value. */
 let boundGatewayPort = GATEWAY_PORT
 export function getGatewayPort(): number {
@@ -1220,15 +1220,39 @@ export async function startModelServer(port = GATEWAY_PORT): Promise<void> {
   server.headersTimeout = 0 // no cap on time-to-headers
   server.keepAliveTimeout = 60_000
 
-  server.on('error', (e) => console.error('[model-server]', e))
   // The gateway has no authentication. Bind the socket itself to loopback so no
   // route can become LAN-accessible through a missing per-handler authorization check.
-  server.listen(boundGatewayPort, GATEWAY_HOST, () => {
-    console.log(
-      `[model-server] multimodal gateway at http://${GATEWAY_HOST}:${boundGatewayPort}/v1`
-    )
-  })
-  startingGateway = false
+  // Resolve only AFTER the socket actually binds; a bind failure must reject and clear state so
+  // callers (which .catch it) can surface the failure and a later restart can retry — rather than a
+  // resolved promise leaving a dead `server` set forever and IPC reporting a false success.
+  const listening = server
+  try {
+    await new Promise<void>((resolve, reject) => {
+      const onError = (e: Error): void => {
+        listening.removeListener('listening', onListening)
+        reject(e)
+      }
+      const onListening = (): void => {
+        listening.removeListener('error', onError)
+        // Hand ongoing runtime errors to the logger now that startup succeeded.
+        listening.on('error', (e) => console.error('[model-server]', e))
+        console.log(
+          `[model-server] multimodal gateway at http://${GATEWAY_HOST}:${boundGatewayPort}/v1`
+        )
+        resolve()
+      }
+      listening.once('error', onError)
+      listening.once('listening', onListening)
+      listening.listen(boundGatewayPort, GATEWAY_HOST)
+    })
+  } catch (e) {
+    // Bind failed — drop the dead server and reset so a retry starts clean.
+    server = null
+    boundGatewayPort = GATEWAY_PORT
+    throw e
+  } finally {
+    startingGateway = false
+  }
 }
 
 export function stopModelServer(): void {