diff --git a/src/lib/actions/sandbox/forward-recovery.ts b/src/lib/actions/sandbox/forward-recovery.ts index 76fc83eeeb3..7115a4b79c0 100644 --- a/src/lib/actions/sandbox/forward-recovery.ts +++ b/src/lib/actions/sandbox/forward-recovery.ts @@ -41,7 +41,11 @@ import { getHermesDashboardRecoveryConfig, } from "./hermes-dashboard-recovery"; -type SandboxPortAgent = { forwardPort?: unknown; runtime?: { kind?: unknown } } | null; +type SandboxPortAgent = { + forwardPort?: unknown; + forward_ports?: unknown; + runtime?: { kind?: unknown }; +} | null; type SandboxPortDeps = { getSandbox?: typeof registry.getSandbox; @@ -416,6 +420,31 @@ export function recoverMessagingHostForward( return recovered; } +function resolveDeclaredAgentForwardPorts( + sandbox: ReturnType, + primaryPort: number, + agent: SandboxPortAgent, + hermesDashboardPort: number | null, +): number[] { + const declared = agent?.forward_ports; + if (!Array.isArray(declared)) return []; + const covered = new Set([primaryPort]); + if (isValidPort(agent?.forwardPort)) covered.add(agent.forwardPort); + if (isValidPort(hermesDashboardPort)) covered.add(hermesDashboardPort); + const ports: number[] = []; + for (const candidate of declared) { + if (typeof candidate !== "number") continue; + if (!Number.isInteger(candidate) || candidate < 1024 || candidate > 65535) continue; + if (covered.has(candidate)) continue; + const port = + candidate === HERMES_OPENAI_API_PORT ? resolveSandboxHermesApiPort(sandbox ?? {}) : candidate; + if (covered.has(port)) continue; + covered.add(port); + ports.push(port); + } + return ports; +} + /** * Re-establish every declared `forward_ports` entry on the active agent * manifest that is not already owned by another recovery helper. The @@ -437,27 +466,17 @@ export function ensureDeclaredAgentForwardPortsHealthy( ): boolean | null { const agent = agentRuntime.getSessionAgent(sandboxName); if (!agent) return null; - const declared = (agent as { forward_ports?: unknown }).forward_ports; - if (!Array.isArray(declared) || declared.length === 0) return null; const hermesDashboard = getHermesDashboardRecoveryConfig(sandboxName); const sandbox = registry.getSandbox(sandboxName); - const skipSet = new Set([primaryPort]); - // The manifest's own primary entry is the default dashboard port. This - // sandbox's dashboard forward lives at primaryPort and is recovered by - // ensureSandboxPortForward, so the default must not be probed again. - if (isValidPort(agent.forwardPort)) skipSet.add(agent.forwardPort); - if (hermesDashboard && Number.isInteger(hermesDashboard.publicPort)) { - skipSet.add(hermesDashboard.publicPort); - } - let sawCovered = false; + const ports = resolveDeclaredAgentForwardPorts( + sandbox, + primaryPort, + agent, + hermesDashboard?.publicPort ?? null, + ); + if (ports.length === 0) return null; let allHealthy = true; - for (const candidate of declared) { - if (typeof candidate !== "number") continue; - if (!Number.isInteger(candidate) || candidate < 1024 || candidate > 65535) continue; - if (skipSet.has(candidate)) continue; - const port = - candidate === HERMES_OPENAI_API_PORT ? resolveSandboxHermesApiPort(sandbox ?? {}) : candidate; - sawCovered = true; + for (const port of ports) { const health = isSandboxPortForwardHealthy(sandboxName, port); if (health === true) continue; if (health === "occupied") { @@ -468,7 +487,6 @@ export function ensureDeclaredAgentForwardPortsHealthy( allHealthy = false; } } - if (!sawCovered) return null; return allHealthy; } @@ -493,18 +511,13 @@ export function areSandboxLaunchForwardsHealthy( if (hermesDashboard) requiredPorts.add(hermesDashboard.publicPort); const messagingForward = getSandboxMessagingHostForward(sandboxName); if (messagingForward) requiredPorts.add(messagingForward.port); - const declared = (agent as { forward_ports?: unknown } | null)?.forward_ports; - if (Array.isArray(declared)) { - for (const candidate of declared) { - if ( - typeof candidate === "number" && - Number.isInteger(candidate) && - candidate >= 1024 && - candidate <= 65535 - ) { - requiredPorts.add(candidate); - } - } + for (const port of resolveDeclaredAgentForwardPorts( + sandbox, + primaryPort, + agent, + hermesDashboard?.publicPort ?? null, + )) { + requiredPorts.add(port); } const result = captureOpenshell(["forward", "list", "--gateway", owningGatewayName], { ignoreError: true, diff --git a/test/cli/connect-recovery.test.ts b/test/cli/connect-recovery.test.ts index d2d76f539fc..f7545144570 100644 --- a/test/cli/connect-recovery.test.ts +++ b/test/cli/connect-recovery.test.ts @@ -375,7 +375,7 @@ describe("CLI connect recovery process contracts", () => { }, ); - it("recovers stopped Hermes agents through privileged Docker control instead of SSH", async () => { + it("recovers a stopped Hermes Agent gateway with its assigned forwards through privileged Docker control (#9716)", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-connect-probe-agent-")); const localBin = path.join(home, "bin"); const openshellCalls = path.join(home, "openshell-calls"); @@ -383,7 +383,12 @@ describe("CLI connect recovery process contracts", () => { const sshCalls = path.join(home, "ssh-calls"); const stateFile = path.join(home, "probe-state"); fs.mkdirSync(localBin, { recursive: true }); - writeSandboxRegistry(home, { ...launchReadinessRegistryFixture(), agent: "hermes" }); + writeSandboxRegistry(home, { + ...launchReadinessRegistryFixture(), + agent: "hermes", + dashboardPort: 18790, + hermesApiPort: 8643, + }); fs.writeFileSync(stateFile, "stopped"); fs.writeFileSync( path.join(localBin, "openshell"), @@ -415,7 +420,7 @@ describe("CLI connect recovery process contracts", () => { ' echo UNEXPECTED_SSH_CONFIG >> "$calls"', " exit 1", "fi", - 'if [ "$1" = "forward" ] && [ "$2" = "list" ]; then { echo "alpha 127.0.0.1 18789 12345 running"; echo "alpha 127.0.0.1 8642 12346 running"; }; exit 0; fi', + 'if [ "$1" = "forward" ] && [ "$2" = "list" ]; then { echo "control 127.0.0.1 18789 12345 running"; echo "control 127.0.0.1 8642 12346 running"; echo "alpha 127.0.0.1 18790 12347 running"; echo "alpha 127.0.0.1 8643 12348 running"; }; exit 0; fi', 'if [ "$1" = "forward" ]; then exit 99; fi', ...launchReadinessObservationStubLines, "exit 0", @@ -424,7 +429,7 @@ describe("CLI connect recovery process contracts", () => { ); writeGatewayControlDockerStub(localBin, { callsFile: dockerCalls, stateFile }); writeRecordingCommand(localBin, "ssh", sshCalls, 98); - const stopForwardListeners = await startForwardListeners([18789, 8642]); + const stopForwardListeners = await startForwardListeners([18790, 8643]); try { const result = runWithEnv("alpha connect --probe-only", { diff --git a/test/launch-readiness-forward-observation.test.ts b/test/launch-readiness-forward-observation.test.ts index a6c6548759b..f698e98b747 100644 --- a/test/launch-readiness-forward-observation.test.ts +++ b/test/launch-readiness-forward-observation.test.ts @@ -48,6 +48,34 @@ beta 127.0.0.1 18790 12346 running`, }); }); +it("checks sandbox-owned Hermes forwards instead of manifest defaults (#9716)", () => { + mockLaunchForwardObservation({ + status: 0, + output: `SANDBOX BIND PORT PID STATUS +alpha 127.0.0.1 18789 12345 running +alpha 127.0.0.1 8642 12346 running +beta 127.0.0.1 18790 12347 running +beta 127.0.0.1 8643 12348 running`, + }); + vi.mocked(agentRuntime.getSessionAgent).mockReturnValue({ + name: "hermes", + runtime: { kind: "gateway" }, + forwardPort: 18789, + forward_ports: [18789, 8642], + } as never); + vi.mocked(registry.getSandbox).mockReturnValue({ + name: "beta", + agent: "hermes", + dashboardPort: 18790, + hermesApiPort: 8643, + gatewayName: "nemoclaw", + gatewayPort: 8080, + }); + + expect(areSandboxLaunchForwardsHealthy("beta", "nemoclaw")).toBe(true); + expect(vi.mocked(forwardHealth.isLocalForwardReachable).mock.calls).toEqual([[18790], [8643]]); +}); + it("rejects a reachable listener when the owning forward row is missing (#8942)", () => { mockLaunchForwardObservation({ status: 0,