Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 107 additions & 13 deletions src/lib/onboard/hermes-api-port.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
});

Expand Down
52 changes: 39 additions & 13 deletions src/lib/onboard/hermes-api-port.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<DashboardPortReservation>;
warn(message: string): void;
Expand Down Expand Up @@ -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;
Expand All @@ -191,7 +212,7 @@ export async function reserveCreateSandboxHermesApiPort(options: {
}): Promise<ReservedCreateSandboxHermesApiPortResult> {
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);
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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 (
Expand Down
Loading