Skip to content

Commit 726986a

Browse files
rsliterprekshivyas
andauthored
fix(onboard): recreate gateway when Docker network is missing (#9671)
<!-- markdownlint-disable MD041 --> ## Summary Onboarding reused a running NemoClaw-managed OpenShell gateway even when its configured Docker network was missing, so later sandbox creation failed with a Docker 404. This change verifies the exact configured network before reuse, marks an authoritative gateway stale when that network is absent, and stops safely when inspection is inconclusive. ## Related Issue Fixes #9594 ## Changes - Inspect the configured Docker network through the existing managed gateway reuse boundary and classify exact evidence as present, absent, or inconclusive. - Mark a reused NemoClaw-managed OpenShell gateway stale only after its process identity and lifecycle authority are proven, then let the existing gateway lifecycle owner retire and restart it. - Fail before provider or sandbox mutation when network inspection is inconclusive or the gateway is not under NemoClaw lifecycle authority. The change does not create, remove, or modify a foreign Docker network. - Add regression coverage for the missing-network failure, exact-name validation, permission and malformed-output failures, foreign-gateway handling, restart ordering, and partial restart failure state. - Reproduce the escaped defect before the production fix: `npx vitest run --project cli src/lib/onboard/gateway-reuse.test.ts` failed 1 of 9 tests because the missing network was reported as healthy. The final focused run passed 47 of 47 tests. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Root-cause and sensitive-workflow review passed exact commit `6d136ebc0c8c43b7120ef213a06681a1c4d42203`. It covered command construction, exact lifecycle authority, present/absent/inconclusive result classification, foreign resource isolation, pre-mutation failure, restart ordering, and partial failure state. - [x] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: The user approved a narrow waiver for `env-var-docs`, which fails identically on clean base `01e0b92645a68d4ff772dce20ee5758def7211cc` because `NEMOCLAW_BEDROCK_RUNTIME_ADAPTER_PORT` and `NEMOCLAW_HTTPS_PIN_RUNTIME_ADAPTER_PORT` are both documented and still retained in the stale allowlist. This PR changes neither entry; every other applicable hook passed. ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: not applicable; `scripts/prepare-dgx-station-host.sh` is unchanged - Station profile/scenario: not applicable - Result: not applicable - Supporting evidence: not applicable ## Documentation Writer Review - [x] Documentation writer reviewed the completed changes - Result: `no-docs-needed` - Reviewed commit: `6d136ebc0c8c43b7120ef213a06681a1c4d42203` - AGENTS.md blob: `513518cdfca42e3a18fed71109e6d0eb60151d13` - Evidence: The change restores the documented stale-gateway recreation behavior when the configured Docker network is absent. It does not change a command, configuration, default, or schema. The existing behavior is documented in `docs/reference/commands.mdx` lines 884-890. - Agent surface: Codex Desktop <!-- docs-review-head-sha: a608468 --> <!-- docs-review-agents-blob-sha: 513518c --> ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [ ] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable — commit-message and pre-push hooks passed; all applicable pre-commit hooks passed except the narrowly waived inherited `env-var-docs` failure recorded above - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run --project cli src/lib/onboard/gateway-reuse.test.ts src/lib/onboard/machine/handlers/gateway.test.ts` passed 47 of 47 tests. The final `npm run test:changed` passed 32 growth guardrails and 47 affected tests. `npm run typecheck:cli` and `npm run checks:repository` passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not applicable to this focused onboarding lifecycle fix; the targeted tests, changed-test selection, CLI typecheck, repository checks, and normal hooks except the recorded waiver passed - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added Docker network verification before reusing gateways. - Added detection for available, missing, or unverifiable networks. - Automatically marks gateways stale and recreates them when their managed network is missing. - Added safeguards to prevent state changes when network verification fails. - **Bug Fixes** - Improved gateway lifecycle handling during network recreation and failed restarts. - Ensured gateway listener and process reuse consistently validate network availability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
1 parent 9ed1547 commit 726986a

4 files changed

Lines changed: 341 additions & 15 deletions

File tree

src/lib/onboard.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -742,8 +742,7 @@ const { getGatewayReuseSnapshot, selectNamedGatewayForReuseIfNeeded } =
742742
const { refreshDockerDriverGatewayReuseState } =
743743
gatewayReuse.createDockerDriverGatewayReuseApplication({
744744
gatewayName: () => GATEWAY_NAME,
745-
getGatewayCompatContainerName: () =>
746-
gatewayBinding.resolveGatewayCompatContainerName(GATEWAY_PORT),
745+
getGatewayCompatContainerName: () => gatewayBinding.resolveGatewayCompatContainerName(GATEWAY_PORT),
747746
isDockerDriverGatewayEnabled: isLinuxDockerDriverGatewayEnabled,
748747
resolveOpenShellGatewayBinary,
749748
getDockerDriverGatewayEnv,
@@ -756,6 +755,7 @@ const { refreshDockerDriverGatewayReuseState } =
756755
checkGatewayPortAvailable,
757756
getDockerDriverGatewayPortListenerPid,
758757
rememberDockerDriverGatewayPid,
758+
runDockerNetworkInspect: docker.dockerRun,
759759
});
760760

761761
const { getSandboxReuseState, getSandboxRecreateObservation, waitForSandboxRecreateDeleteAbsence } =

src/lib/onboard/gateway-reuse.test.ts

Lines changed: 150 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,77 @@ import { describe, expect, it, vi } from "vitest";
55

66
import { OPENSHELL_PROBE_TIMEOUT_MS } from "../adapters/openshell/timeouts";
77
import {
8+
classifyDockerDriverNetworkInspection,
89
createDockerDriverGatewayReuseApplication,
910
type DockerDriverGatewayReuseApplicationDeps,
1011
createGatewayReuseHelpers,
12+
inspectDockerDriverNetwork,
1113
} from "./gateway-reuse";
1214

15+
function dockerNetworkInspectResult(
16+
status: number | null,
17+
stdout = "",
18+
stderr = "",
19+
error?: Error,
20+
) {
21+
return { error, status, stderr, stdout };
22+
}
23+
24+
describe("Docker-driver network inspection", () => {
25+
it("inspects the exact configured Docker network with a bounded command", () => {
26+
const runDocker = vi.fn(() => dockerNetworkInspectResult(0, "openshell-docker"));
27+
28+
expect(inspectDockerDriverNetwork("openshell-docker", runDocker)).toEqual({
29+
kind: "present",
30+
});
31+
expect(runDocker).toHaveBeenCalledWith(
32+
["network", "inspect", "--format", "{{.Name}}", "openshell-docker"],
33+
{
34+
ignoreError: true,
35+
suppressOutput: true,
36+
timeout: OPENSHELL_PROBE_TIMEOUT_MS,
37+
},
38+
);
39+
});
40+
41+
it("accepts the exact configured Docker network", () => {
42+
expect(
43+
classifyDockerDriverNetworkInspection(
44+
"openshell-docker",
45+
dockerNetworkInspectResult(0, "openshell-docker\n"),
46+
),
47+
).toEqual({ kind: "present" });
48+
});
49+
50+
it.each([
51+
"Error response from daemon: No such network: openshell-docker",
52+
"network openshell-docker not found",
53+
])("classifies an exact missing configured network as absent: %s (#9594)", (stderr) => {
54+
expect(
55+
classifyDockerDriverNetworkInspection(
56+
"openshell-docker",
57+
dockerNetworkInspectResult(1, "", stderr),
58+
),
59+
).toEqual({ kind: "absent" });
60+
});
61+
62+
it.each([
63+
dockerNetworkInspectResult(1, "", "No such network: unrelated-network"),
64+
dockerNetworkInspectResult(1, "", "registry metadata not found"),
65+
dockerNetworkInspectResult(0, "another-network"),
66+
dockerNetworkInspectResult(
67+
1,
68+
"",
69+
"No such network: openshell-docker",
70+
new Error("Docker transport failed"),
71+
),
72+
])("keeps ambiguous Docker network inspection inconclusive [case %#] (#9594)", (result) => {
73+
expect(classifyDockerDriverNetworkInspection("openshell-docker", result)).toEqual({
74+
kind: "inconclusive",
75+
});
76+
});
77+
});
78+
1379
describe("gateway reuse snapshot", () => {
1480
it("bounds OpenShell gateway inspection probes (#6752)", () => {
1581
const runCaptureOpenshell = vi.fn(() => "");
@@ -96,7 +162,10 @@ function createDockerDriverReuseApplication(
96162
getGatewayCompatContainerName: () => "openshell-gateway-nemoclaw",
97163
isDockerDriverGatewayEnabled: () => true,
98164
resolveOpenShellGatewayBinary: () => "/opt/openshell-gateway",
99-
getDockerDriverGatewayEnv: () => ({ OPENSHELL_DRIVERS: "docker" }),
165+
getDockerDriverGatewayEnv: () => ({
166+
OPENSHELL_DRIVERS: "docker",
167+
OPENSHELL_DOCKER_NETWORK_NAME: "openshell-docker",
168+
}),
100169
runCaptureOpenshell: vi.fn(() => "openshell 0.0.99"),
101170
getDockerDriverGatewayStateDir: () => "/tmp/nemoclaw-gateway",
102171
resolveOpenShellSandboxBinary: () => "/opt/openshell-sandbox",
@@ -108,14 +177,19 @@ function createDockerDriverReuseApplication(
108177
rememberDockerDriverGatewayPid: vi.fn(),
109178
buildDockerDriverGatewayRuntimeIdentity: vi.fn(() => ({
110179
launch: null,
111-
desiredEnv: { OPENSHELL_DRIVERS: "docker" },
180+
desiredEnv: {
181+
OPENSHELL_DRIVERS: "docker",
182+
OPENSHELL_DOCKER_NETWORK_NAME: "openshell-docker",
183+
},
112184
driftGatewayBin: "/opt/openshell-gateway",
113185
identityGatewayBin: "/opt/openshell-gateway",
114186
})),
115187
resolveDriftGatewayBin: vi.fn((runtimeIdentity, gatewayBin) =>
116188
runtimeIdentity ? runtimeIdentity.driftGatewayBin : gatewayBin,
117189
),
118190
getTrustedActiveOpenShellGatewayUserServicePid: vi.fn(() => null),
191+
runDockerNetworkInspect: vi.fn(() => dockerNetworkInspectResult(0, "openshell-docker")),
192+
inspectDockerDriverNetwork: vi.fn(() => ({ kind: "present" as const })),
119193
log: vi.fn(),
120194
...overrides,
121195
});
@@ -158,7 +232,32 @@ describe("Docker-driver gateway reuse application", () => {
158232
);
159233
});
160234

161-
it("adopts a matching gateway port listener when the PID file is absent", async () => {
235+
it("marks a reused NemoClaw-managed gateway stale when its Docker network is absent (#9594)", async () => {
236+
const log = vi.fn();
237+
const application = createDockerDriverReuseApplication({
238+
inspectDockerDriverNetwork: vi.fn(() => ({ kind: "absent" as const })),
239+
log,
240+
});
241+
242+
await expect(application.refreshDockerDriverGatewayReuseState("healthy")).resolves.toBe(
243+
"stale",
244+
);
245+
expect(log).toHaveBeenCalledWith(
246+
' Existing NemoClaw-managed OpenShell gateway network "openshell-docker" is absent; the gateway will be recreated.',
247+
);
248+
});
249+
250+
it("stops before reuse when Docker network inspection is inconclusive (#9594)", async () => {
251+
const application = createDockerDriverReuseApplication({
252+
inspectDockerDriverNetwork: vi.fn(() => ({ kind: "inconclusive" as const })),
253+
});
254+
255+
await expect(application.refreshDockerDriverGatewayReuseState("healthy")).rejects.toThrow(
256+
'NemoClaw could not verify Docker network "openshell-docker" before reusing the NemoClaw-managed OpenShell gateway. Check Docker daemon access and the configured network, then rerun `nemoclaw onboard`.',
257+
);
258+
});
259+
260+
it("reuses the trusted gateway port listener when the PID file is absent (#9594)", async () => {
162261
const rememberDockerDriverGatewayPid = vi.fn();
163262
const getDockerDriverGatewayReuseDrift = vi.fn(() => null);
164263
const application = createDockerDriverReuseApplication({
@@ -167,7 +266,7 @@ describe("Docker-driver gateway reuse application", () => {
167266
checkGatewayPortAvailable: vi.fn(async () => ({ ok: false, pid: 731 })),
168267
getDockerDriverGatewayPortListenerPid: vi.fn(() => 731),
169268
getDockerDriverGatewayReuseDrift,
170-
getTrustedActiveOpenShellGatewayUserServicePid: vi.fn(() => 900),
269+
getTrustedActiveOpenShellGatewayUserServicePid: vi.fn(() => 731),
171270
rememberDockerDriverGatewayPid,
172271
});
173272

@@ -176,26 +275,70 @@ describe("Docker-driver gateway reuse application", () => {
176275
);
177276
expect(getDockerDriverGatewayReuseDrift).toHaveBeenCalledWith(
178277
731,
179-
{ OPENSHELL_DRIVERS: "docker" },
278+
{
279+
OPENSHELL_DRIVERS: "docker",
280+
OPENSHELL_DOCKER_NETWORK_NAME: "openshell-docker",
281+
},
180282
"/opt/openshell-gateway",
181-
900,
283+
731,
284+
);
285+
expect(rememberDockerDriverGatewayPid).not.toHaveBeenCalled();
286+
});
287+
288+
it("stops before recording a mismatched gateway port listener when the Docker network is absent (#9594)", async () => {
289+
const rememberDockerDriverGatewayPid = vi.fn();
290+
const getDockerDriverGatewayReuseDrift = vi.fn(() => null);
291+
const application = createDockerDriverReuseApplication({
292+
getDockerDriverGatewayPid: () => null,
293+
isDockerDriverGatewayProcessAlive: () => false,
294+
checkGatewayPortAvailable: vi.fn(async () => ({ ok: false, pid: 731 })),
295+
getDockerDriverGatewayPortListenerPid: vi.fn(() => 731),
296+
getDockerDriverGatewayReuseDrift,
297+
getTrustedActiveOpenShellGatewayUserServicePid: vi.fn(() => 900),
298+
inspectDockerDriverNetwork: vi.fn(() => ({ kind: "absent" as const })),
299+
rememberDockerDriverGatewayPid,
300+
});
301+
302+
await expect(application.refreshDockerDriverGatewayReuseState("healthy")).rejects.toThrow(
303+
'Docker network "openshell-docker" is absent, but NemoClaw could not verify the running gateway\'s lifecycle authority. Restart the gateway through its lifecycle authority, then rerun `nemoclaw onboard`.',
182304
);
183-
expect(rememberDockerDriverGatewayPid).toHaveBeenCalledWith(731);
305+
expect(getDockerDriverGatewayReuseDrift).not.toHaveBeenCalled();
306+
expect(rememberDockerDriverGatewayPid).not.toHaveBeenCalled();
184307
});
185308

186309
it("preserves a reachable selected gateway when the port owner is ambiguous", async () => {
187310
const rememberDockerDriverGatewayPid = vi.fn();
311+
const inspectDockerDriverNetwork = vi.fn(() => ({ kind: "present" as const }));
188312
const application = createDockerDriverReuseApplication({
189313
getDockerDriverGatewayPid: () => null,
190314
isDockerDriverGatewayProcessAlive: () => false,
191315
checkGatewayPortAvailable: vi.fn(async () => ({ ok: false, pid: null })),
192316
getDockerDriverGatewayPortListenerPid: vi.fn(() => null),
317+
inspectDockerDriverNetwork,
193318
rememberDockerDriverGatewayPid,
194319
});
195320

196321
await expect(application.refreshDockerDriverGatewayReuseState("healthy")).resolves.toBe(
197322
"healthy",
198323
);
324+
expect(inspectDockerDriverNetwork).toHaveBeenCalledWith("openshell-docker");
325+
expect(rememberDockerDriverGatewayPid).not.toHaveBeenCalled();
326+
});
327+
328+
it("does not recreate a gateway with unproven lifecycle authority when its network is absent (#9594)", async () => {
329+
const rememberDockerDriverGatewayPid = vi.fn();
330+
const application = createDockerDriverReuseApplication({
331+
getDockerDriverGatewayPid: () => null,
332+
isDockerDriverGatewayProcessAlive: () => false,
333+
checkGatewayPortAvailable: vi.fn(async () => ({ ok: false, pid: null })),
334+
getDockerDriverGatewayPortListenerPid: vi.fn(() => null),
335+
inspectDockerDriverNetwork: vi.fn(() => ({ kind: "absent" as const })),
336+
rememberDockerDriverGatewayPid,
337+
});
338+
339+
await expect(application.refreshDockerDriverGatewayReuseState("healthy")).rejects.toThrow(
340+
'Docker network "openshell-docker" is absent, but NemoClaw could not verify the running gateway\'s lifecycle authority. Restart the gateway through its lifecycle authority, then rerun `nemoclaw onboard`.',
341+
);
199342
expect(rememberDockerDriverGatewayPid).not.toHaveBeenCalled();
200343
});
201344

0 commit comments

Comments
 (0)