fix(cli): skip stale k3s gateway container check in Docker-driver doctor (#4502)

[yimoj]

Summary

nemoclaw doctor always reported a [fail] Gateway check in Docker-driver mode because it unconditionally inspected the legacy k3s container openshell-cluster-nemoclaw, which only exists for the Kubernetes gateway driver. This makes doctor gate the legacy inspect on the gateway driver so healthy Docker-driver installs are no longer falsely marked unhealthy.

Related Issue

Fixes #4502

Changes

• Gate the legacy openshell-cluster-<name> container inspect on the gateway driver in src/lib/actions/sandbox/doctor.ts: skip it for docker/vm drivers, run it for kubernetes, and fall back to platform detection (isLinuxDockerDriverGatewayEnabled()) for older registry entries that predate the openshellDriver field.
• Rely on the existing authoritative OpenShell status check for the Docker-driver gateway (host process or nemoclaw-openshell-gateway compat container), which already runs immediately afterward.
• De-hardcode the "not found" hint to reference the actually-inspected container name instead of the literal openshell-cluster-nemoclaw.
• Add regression tests in test/cli.test.ts: Docker-driver mode (legacy container absent but gateway healthy → no false failure, no openshell-cluster mention) and kubernetes driver (legacy container still inspected).

Type of Change

• 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)

Verification

• npx prek run --all-files passes
• npm test passes
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

Notes on verification:

• Reproduced end-to-end with this worktree's CLI (node ./bin/nemoclaw.js <sandbox> doctor --json) on a live Linux Docker-driver host: before the fix the Gateway group showed [fail] Docker container: openshell-cluster-nemoclaw not found while OpenShell status reported connected to nemoclaw; after the fix the stale check is gone and the Gateway group relies on the healthy OpenShell status check.
• npm run typecheck:cli and npx biome lint src/lib/actions/sandbox/doctor.ts pass. Targeted doctor tests pass (7/7). Remaining npm test failures on the build host are pre-existing/environmental (load-induced 5000ms CLI-spawn timeouts and a umask-0027 config-sync mode-bits assertion that fails identically on the clean base) and are unrelated to this change.

---

Signed-off-by: Yimo Jiang yimoj@nvidia.com

Summary by CodeRabbit

• Bug Fixes
  • Prevented false "legacy gateway" failures in the sandbox diagnostic tool by only checking legacy containers when the gateway driver requires it; diagnostics now show the actual container name when present.
• Tests
  • Added CLI tests to confirm correct diagnostic behavior for Docker vs Kubernetes gateway scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9841b6cb-2cdd-46f6-9a93-65ec4a98806e

📥 Commits

Reviewing files that changed from the base of the PR and between 9962844 and fddda90.

📒 Files selected for processing (1)

• test/cli.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• test/cli.test.ts

---

📝 Walkthrough

Walkthrough

Doctor now only inspects legacy openshell-cluster-* Docker containers when the sandbox driver is Kubernetes or when platform-level fallback indicates the legacy Docker gateway driver; the inspection call is guarded by a new helper and tests plus an improved error hint were added.

Changes

Legacy Gateway Container Inspection Fix

Layer / File(s)	Summary
Conditional legacy gateway inspection src/lib/actions/sandbox/doctor.ts	Adds import for isLinuxDockerDriverGatewayEnabled, introduces shouldInspectLegacyGatewayContainer(sb) helper to decide whether to probe the legacy openshell-cluster-<name> container based on sb.openshellDriver with a platform fallback, and gates the dockerInspectGateway() call in runSandboxDoctor.
Error message generalization and driver-mode tests src/lib/actions/sandbox/doctor.ts, test/cli.test.ts	Docker inspection failure hint now references the actual inspected containerName. Two Vitest cases assert that Docker-mode sandboxes skip the legacy container inspection (no "Docker container" check, no openshell-cluster strings) and Kubernetes-mode sandboxes still inspect it (check present and references openshell-cluster-nemoclaw).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• NVIDIA/NemoClaw#4608: Both PRs add driver-aware gating to avoid inspecting the legacy openshell-cluster-* k3s gateway container on docker/vm sandboxes.

Suggested labels

Docker, fix, NemoClaw CLI, OpenShell, Sandbox

Suggested reviewers

• cv
• cjagwani

Poem

🐰 I nibble code where gates reside,
I peek for containers, but only when I should.
Docker naps undisturbed, Kubernetes checked with pride,
No phantom fails — the burrow's quiet and good.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing a false Gateway check failure by skipping the legacy k3s container check when using Docker driver.
Linked Issues check	✅ Passed	The PR implements the fix requested in #4502: conditional skipping of stale k3s container lookup for Docker-driver environments and proper container name handling.
Out of Scope Changes check	✅ Passed	All changes directly address the scope of #4502: doctor gateway check fix, container inspection logic, and regression test coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

test/cli.test.ts (1)

2755-2788: ⚡ Quick win

Assert that docker inspect was never invoked.

This test proves the user-visible outcome, but not the core contract from the fix. If doctor still called docker inspect and then ignored that failure, these assertions would still pass. Recording fake Docker argv here and asserting no inspect call occurs would make the regression much tighter.

Suggested test hardening

+    const dockerCalls = path.join(setup.home, "docker-calls");
     fs.writeFileSync(
       path.join(setup.localBin, "docker"),
       [
         "#!/usr/bin/env bash",
+        `printf '%s\\n' "$*" >> ${JSON.stringify(dockerCalls)}`,
         'if [ "$1" = "info" ]; then echo "24.0.0"; exit 0; fi',
         'if [ "$1" = "inspect" ]; then echo "Error: No such object: $3" >&2; exit 1; fi',
         "exit 0",
       ].join("\n"),
       { mode: 0o755 },
@@
     expect(report.checks.find((check) => check.label === "Docker container")).toBeUndefined();
+    expect(fs.readFileSync(dockerCalls, "utf8")).not.toMatch(/\binspect\b/);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/cli.test.ts` around lines 2755 - 2788, Add an assertion that the fake
docker binary was never invoked with the "inspect" subcommand by modifying the
stub created at path.join(setup.localBin, "docker") to record its invocations
(e.g., append "$@" to a temp log file) and after calling setup.runDoctor("alpha
doctor --json") assert that the log file does not contain the word "inspect";
locate the docker stub creation in the test (the fs.writeFileSync that writes
the docker script) and the test invocation using setup.runDoctor and r to add
the logging behavior and the assertion against the recorded invocations.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/cli.test.ts`:
- Around line 2755-2788: Add an assertion that the fake docker binary was never
invoked with the "inspect" subcommand by modifying the stub created at
path.join(setup.localBin, "docker") to record its invocations (e.g., append "$@"
to a temp log file) and after calling setup.runDoctor("alpha doctor --json")
assert that the log file does not contain the word "inspect"; locate the docker
stub creation in the test (the fs.writeFileSync that writes the docker script)
and the test invocation using setup.runDoctor and r to add the logging behavior
and the assertion against the recorded invocations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f13549ec-373c-43cd-b13f-7db867bb20a9

📥 Commits

Reviewing files that changed from the base of the PR and between cab8f39 and 3b49be6.

📒 Files selected for processing (2)

• src/lib/actions/sandbox/doctor.ts
• test/cli.test.ts

[prekshivyas]

APPROVE.

Correctly gates the legacy openshell-cluster-<name> k3s container inspect on the gateway driver — skip for docker/vm, keep for kubernetes, platform-fallback for pre-openshellDriver entries — matching the driver taxonomy already used in gateway-failure-classifier.ts. The Docker-driver regression test asserts docker inspect is never called (not merely that its failure is tolerated), so it fails on pre-fix code. CI green on 9962844, mergeable, zero open threads. Resolves #4502.

Non-blocking nits:

• src/lib/actions/dns/index.ts:193,275 still filter docker ps on the same stale openshell-cluster name this PR removes from doctor — likely the same latent issue in another path; worth a follow-up.
• The vm and null/platform-fallback branches of shouldInspectLegacyGatewayContainer have no test.

Signed-off-by: Prekshi Vyas prekshiv@nvidia.com