diff --git a/src/lib/onboard/hermes-api-port.test.ts b/src/lib/onboard/hermes-api-port.test.ts index 1a4ec55db3f..74592f060f1 100644 --- a/src/lib/onboard/hermes-api-port.test.ts +++ b/src/lib/onboard/hermes-api-port.test.ts @@ -75,16 +75,14 @@ describe("readHermesApiPort", () => { expect(readHermesApiPort({})).toBe(8642); }); - it.each([ - "8641", - "8653", - "9000", - "²", - ])("rejects %s outside the allocated Hermes API-port range", (value) => { - expect(() => readHermesApiPort({ [HERMES_API_PORT_ENV]: value })).toThrow( - /integer from 8642 through 8652/, - ); - }); + it.each(["8641", "8653", "9000", "²"])( + "rejects %s outside the allocated Hermes API-port range", + (value) => { + expect(() => readHermesApiPort({ [HERMES_API_PORT_ENV]: value })).toThrow( + /integer from 8642 through 8652/, + ); + }, + ); }); describe("findAvailableHermesApiPort", () => { @@ -157,6 +155,84 @@ describe("reserveCreateSandboxHermesApiPort", () => { expect(secondRelease).toHaveBeenCalledOnce(); }); + it("allocates past a busy default when only a route-only reservation exists (#9291)", async () => { + const release = vi.fn(async () => undefined); + const reservePort = vi + .fn() + .mockRejectedValueOnce( + Object.assign(new Error("port 8642 is already held"), { code: "EADDRINUSE" }), + ) + .mockResolvedValueOnce({ port: 8643, release }); + const env: NodeJS.ProcessEnv = {}; + + const selection = await reserveCreateSandboxHermesApiPort({ + sandboxName: "beta", + env, + getSandbox: () => ({ pendingRouteReservation: true }), + forwardListOutput: "", + isPortBoundCheck: noneBound, + registryOccupiedPorts: new Map(), + reservePort, + }); + + expect(selection.effectivePort).toBe(8643); + expect(env[HERMES_API_PORT_ENV]).toBe("8643"); + expect(reservePort.mock.calls).toEqual([[8642], [8643]]); + await selection.reservation?.release(); + expect(release).toHaveBeenCalledOnce(); + }); + + it("reports EADDRINUSE for a durable sandbox without a port instead of allocating (#9291)", async () => { + const reservePort = vi + .fn() + .mockRejectedValueOnce( + Object.assign(new Error("port 8642 is already held"), { code: "EADDRINUSE" }), + ); + const env: NodeJS.ProcessEnv = {}; + + await expect( + reserveCreateSandboxHermesApiPort({ + sandboxName: "beta", + env, + getSandbox: () => ({}), + forwardListOutput: "", + isPortBoundCheck: noneBound, + registryOccupiedPorts: new Map(), + reservePort, + }), + ).rejects.toMatchObject({ code: "EADDRINUSE" }); + + expect(reservePort.mock.calls).toEqual([[8642]]); + expect(env[HERMES_API_PORT_ENV]).toBe("8642"); + }); + + it("reports EADDRINUSE for a created sandbox that still has pendingRouteReservation (#9291)", async () => { + const reservePort = vi + .fn() + .mockRejectedValueOnce( + Object.assign(new Error("port 8642 is already held"), { code: "EADDRINUSE" }), + ); + const env: NodeJS.ProcessEnv = {}; + + await expect( + reserveCreateSandboxHermesApiPort({ + sandboxName: "beta", + env, + getSandbox: () => ({ + pendingRouteReservation: true, + createdAt: "2026-08-17T00:00:00.000Z", + }), + forwardListOutput: "", + isPortBoundCheck: noneBound, + registryOccupiedPorts: new Map(), + reservePort, + }), + ).rejects.toMatchObject({ code: "EADDRINUSE" }); + + expect(reservePort.mock.calls).toEqual([[8642]]); + expect(env[HERMES_API_PORT_ENV]).toBe("8642"); + }); + it("releases a held port when sandbox preparation fails", async () => { const release = vi.fn(async () => undefined); @@ -265,6 +341,20 @@ describe("resolveOnboardHermesApiPort", () => { expect(env[HERMES_API_PORT_ENV]).toBe("8642"); }); + it("allocates for a route-only reservation instead of pinning the default (#9291)", () => { + const env: NodeJS.ProcessEnv = {}; + const findAvailablePort = vi.fn(() => 8643); + expect( + resolveOnboardHermesApiPort("beta", { + env, + getSandbox: () => ({ pendingRouteReservation: true }), + findAvailablePort, + }), + ).toBe(8643); + expect(findAvailablePort).toHaveBeenCalledOnce(); + expect(env[HERMES_API_PORT_ENV]).toBe("8643"); + }); + it("prefers the registered port over a fresh allocation", () => { const env: NodeJS.ProcessEnv = {}; const findAvailablePort = vi.fn(() => 8644); @@ -342,9 +432,13 @@ describe("resolveVerifyAgentApiPort (#9290)", () => { it("keeps a non-Hermes agent's declared probe port", () => { expect( - resolveVerifyAgentApiPort("sb", { name: "other", healthProbe: { port: 9000 } }, { - getSandbox: () => ({ hermesApiPort: 8643 }), - }), + resolveVerifyAgentApiPort( + "sb", + { name: "other", healthProbe: { port: 9000 } }, + { + getSandbox: () => ({ hermesApiPort: 8643 }), + }, + ), ).toBe(9000); }); diff --git a/src/lib/onboard/hermes-api-port.ts b/src/lib/onboard/hermes-api-port.ts index 5359f00925f..e461eef43e5 100644 --- a/src/lib/onboard/hermes-api-port.ts +++ b/src/lib/onboard/hermes-api-port.ts @@ -23,11 +23,32 @@ import { export const HERMES_API_PORT_ENV = "NEMOCLAW_HERMES_API_PORT"; +/** Registry fields the Hermes API-port allocator reads for identity vs allocation. */ +export type HermesApiPortSandboxLookup = { + hermesApiPort?: number | null; + pendingRouteReservation?: true; + createdAt?: string; +}; + +/** + * Durable sandboxes keep a recorded (or legacy-default) API port. A route-only + * inference reservation is only a pre-create lock and must not pin the default + * port before allocation runs (#9291). + */ +function durableHermesApiPortSandbox( + registered: HermesApiPortSandboxLookup | null | undefined, +): HermesApiPortSandboxLookup | null { + if (registered == null || registry.isRouteOnlySandboxReservation(registered)) { + return null; + } + return registered; +} + export interface HermesApiPortReservationInput { agentName?: string | null; sandboxName: string; env: NodeJS.ProcessEnv; - getSandbox(name: string): { hermesApiPort?: number | null } | null | undefined; + getSandbox(name: string): HermesApiPortSandboxLookup | null | undefined; captureForwardList(): string | null; reservePort?(port: number): Promise; warn(message: string): void; @@ -181,7 +202,7 @@ function isAddressInUse(error: unknown): boolean { export async function reserveCreateSandboxHermesApiPort(options: { sandboxName: string; env?: NodeJS.ProcessEnv; - getSandbox?: (name: string) => { hermesApiPort?: number | null } | null | undefined; + getSandbox?: (name: string) => HermesApiPortSandboxLookup | null | undefined; allowRegisteredOverride?: boolean; forwardListOutput?: string | null; isPortBoundCheck?: (port: number) => boolean; @@ -191,7 +212,7 @@ export async function reserveCreateSandboxHermesApiPort(options: { }): Promise { const env = options.env ?? process.env; const getSandbox = options.getSandbox ?? registry.getSandbox; - const registered = getSandbox(options.sandboxName); + const registered = durableHermesApiPortSandbox(getSandbox(options.sandboxName)); const hasRequestedPort = Boolean(env[HERMES_API_PORT_ENV]?.trim()); const forwardListOutput = options.forwardListOutput ?? null; const forwardOwners = getOccupiedPorts(forwardListOutput); @@ -205,9 +226,11 @@ export async function reserveCreateSandboxHermesApiPort(options: { return { effectivePort, reservation: await reservePort(effectivePort) }; }; - // Explicit and already-registered ports are identity, not allocation hints. - // Preserve them and report a bind collision instead of silently changing the - // sandbox's configured endpoint. + // Explicit and durable registered ports pin the sandbox endpoint, not + // allocation hints. Preserve them and report a bind collision instead of + // silently changing the sandbox's configured endpoint. Route-only inference + // reservations are not a durable sandbox and must allocate like an + // unregistered name (#9291). if (hasRequestedPort || registered) { const effectivePort = resolveOnboardHermesApiPort(options.sandboxName, { env, @@ -335,11 +358,12 @@ export function retargetHermesApiPortInUrl(url: string, apiPort: number): string * argument through the onboarding entrypoint. The ready summary instead reads * the registry, which is equivalent because registration precedes it. * - * An existing sandbox keeps its recorded port unless the caller is the actual - * create/recreate or created-sandbox registration boundary. Other consumers - * reject a conflicting explicit value before they mutate a host forward. A - * registered sandbox without a port predates this feature and already runs on - * the default. + * An existing durable sandbox keeps its recorded port unless the caller is the + * actual create/recreate or created-sandbox registration boundary. Other + * consumers reject a conflicting explicit value before they mutate a host + * forward. A durable registered sandbox without a port predates this feature + * and already runs on the default. A route-only inference reservation is not a + * durable sandbox and must allocate like an unregistered name (#9291). * * A recreate keeps its source row, so its create and registration boundaries * may apply an explicit value. Without an explicit value, it preserves the @@ -349,7 +373,7 @@ export function resolveOnboardHermesApiPort( sandboxName: string, options: { env?: NodeJS.ProcessEnv; - getSandbox?: (name: string) => { hermesApiPort?: number | null } | null | undefined; + getSandbox?: (name: string) => HermesApiPortSandboxLookup | null | undefined; allowRegisteredOverride?: boolean; forwardListOutput?: string | null; findAvailablePort?: typeof findAvailableHermesApiPort; @@ -363,7 +387,9 @@ export function resolveOnboardHermesApiPort( env[HERMES_API_PORT_ENV] = String(port); return port; }; - const registered = (options.getSandbox ?? registry.getSandbox)(sandboxName); + const registered = durableHermesApiPortSandbox( + (options.getSandbox ?? registry.getSandbox)(sandboxName), + ); if (registered) { const registeredPort = resolveSandboxHermesApiPort(registered); if (