fix(onboard): short-circuit gateway start when Docker daemon is unreachable (#2347)

[chengjiew]

Summary

When Colima/Docker is stopped on macOS, nemoclaw onboard failed at [2/8] Starting OpenShell gateway with Socket not found: /var/run/docker.sock and then kept retrying the health poll for up to ~15 minutes (3 attempts × ~300s ARM64 health-wait) before surfacing a generic openshell doctor logs troubleshooting message. The issue author reported this as an indefinite hang (against v0.0.23, which predated the existing NEMOCLAW_GATEWAY_START_TIMEOUT bound added in #1830).

This PR makes that failure mode fast-fail with an actionable error instead of burning ~15 minutes:

• Adds a pure classifyGatewayStartFailure(output) helper in src/lib/validation.ts that recognizes the Socket not found: /var/run/docker.sock (macOS Colima stopped) and Cannot connect to the Docker daemon (Linux dockerd stopped) signatures emitted by openshell gateway start.
• In startGatewayWithOptions, when openshell gateway start exits non-zero AND the output classifies as docker_unreachable, aborts the retry loop via pRetry.AbortError — no further health polls, no further retries.
• In the onboard failure branch, prints a short platform-specific remediation (colima start on macOS, sudo systemctl start docker on Linux) in place of the openshell troubleshooting dump.

Picks option (b) from the issue — does not auto-invoke colima start or systemctl start docker.

Relationship to #2348 / PR #2372

PR #2372 (for sibling issue #2348) adds the preflight detection so onboard fast-fails at step [1/8] when Docker is already stopped before onboard runs. That PR is the primary defense; this PR is the defense-in-depth for the case where the daemon dies mid-onboard or preflight is bypassed. Together they cover both halves of what the #2347 issue asks for.

The helper name (classifyGatewayStartFailure) parallels the existing classifySandboxCreateFailure and classifyValidationFailure conventions in src/lib/validation.ts.

Behavior unchanged on the success path

The classifier is only consulted when startResult.status !== 0. When openshell gateway start succeeds, or when it exits non-zero for reasons other than the Docker socket (e.g. slow k3s bootstrap, image pull), the retry + health-poll behavior is identical to before.

Testing

• Added 6 unit tests for classifyGatewayStartFailure:
  • macOS Colima-stopped signature (Socket not found: /var/run/docker.sock)
  • Linux dockerd-stopped signature (Cannot connect to the Docker daemon at unix:///var/run/docker.sock)
  • Bare Failed to create Docker client marker
  • Free-form docker daemon is not running wording
  • Slow-bootstrap output (Helm chart apply, pod startup duration) returns unknown so the retry loop stays engaged — regression guard
  • Empty / missing output → unknown
• npx vitest run src/lib/validation.test.ts → 48/48 passing
• npx vitest run test/gateway-start-wait.test.ts test/onboard.test.ts test/onboard-readiness.test.ts → 176/176 passing
• npx tsc -p tsconfig.src.json / npx tsc -p tsconfig.cli.json clean
• Live macOS repro (colima stop → nemoclaw onboard): not run. The change is exercised by the unit tests on the pure classifier + is a surgical edit to a well-tested code path.

Diff size

3 files changed, 119 insertions(+), 0 deletions(-).

Fixes #2347

Signed-off-by: Chengjie Wang chengjiew@nvidia.com

Summary by CodeRabbit

• New Features

  • Enhanced gateway startup error detection to identify Docker daemon connectivity issues and provide targeted OS-specific instructions for resolution, streamlining troubleshooting.

• Tests

  • Added test coverage for Docker daemon failure classification across multiple error scenarios.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

Implementation of Docker daemon detection during gateway startup failures. The code classifies gateway startup errors as "docker_unreachable" or "unknown", aborts retries when Docker is unreachable, and displays platform-specific Docker restart instructions upon final failure.

Changes

Cohort / File(s)	Summary
Validation Framework src/lib/validation.ts	New GatewayStartFailure interface and classifyGatewayStartFailure() function that detects Docker daemon reachability issues from gateway output, recognizing macOS socket errors, Linux daemon connection failures, and Docker client creation errors.
Validation Tests src/lib/validation.test.ts	Test suite verifying classifier correctly identifies Docker-unreachable error signatures (missing sockets, daemon connection failures, client creation errors) and returns unknown for ambiguous or healthy-looking startup logs.
Onboard Integration src/lib/onboard.ts	Imports classifier function, integrates into retry loop to detect unrecoverable Docker failures, aborts retries immediately when Docker is unreachable, and prints OS-specific Docker startup commands on final failure instead of generic logs.

Sequence Diagram

sequenceDiagram
    actor User
    participant Onboard
    participant Classifier
    participant Gateway
    participant Docker
    
    User->>Onboard: Start onboard
    Onboard->>Gateway: Attempt gateway start
    Gateway->>Docker: Connect to daemon
    Docker-->>Gateway: Connection failed (daemon unreachable)
    Gateway-->>Onboard: Return error output
    Onboard->>Classifier: Classify failure type
    Classifier-->>Onboard: { kind: "docker_unreachable" }
    Onboard->>Onboard: Mark as unrecoverable
    Onboard->>Onboard: Abort p-retry loop
    Onboard->>User: Print OS-specific Docker restart command
    Onboard->>Onboard: Exit process

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A daemon that slumbers, a socket that hides—
The gateway would fail and the onboard gets stuck!
But now we detect when the Docker won't start,
We classify failures with precision and heart,
And tell the kind user exactly what's needed—
Colima or systemctl, their plea shall be heeded! 🚀

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title directly addresses the main change: short-circuiting gateway start when Docker daemon is unreachable, which is the primary objective of the PR.
Linked Issues check	✅ Passed	The code changes fully implement the issue requirements: detects Docker unreachability via classifyGatewayStartFailure, aborts retries with AbortError, and prints OS-specific remediation messages (option b from issue #2347).
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #2347: validation.ts adds the classifier, onboard.ts integrates Docker detection logic, and validation.test.ts provides comprehensive test coverage.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/2347-onboard-docker-hang

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/onboard.ts (1)

6848-6848: Optional: keep onboard exports narrowly scoped.

If no external consumer needs this symbol from onboard, consider not re-exporting classifyGatewayStartFailure and keeping it sourced from validation only.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/onboard.ts` at line 6848, The export list in onboard.ts re-exports
classifyGatewayStartFailure but it appears unused externally; remove
classifyGatewayStartFailure from onboard's public exports and leave its
import/definition in validation so external callers import it from validation
instead; locate the export array or export block in onboard.ts (where
classifyGatewayStartFailure currently appears) and delete that symbol, then run
a project-wide search for classifyGatewayStartFailure to ensure all internal
uses still import it from validation and update any import sites if necessary.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/onboard.ts`:
- Line 6848: The export list in onboard.ts re-exports
classifyGatewayStartFailure but it appears unused externally; remove
classifyGatewayStartFailure from onboard's public exports and leave its
import/definition in validation so external callers import it from validation
instead; locate the export array or export block in onboard.ts (where
classifyGatewayStartFailure currently appears) and delete that symbol, then run
a project-wide search for classifyGatewayStartFailure to ensure all internal
uses still import it from validation and update any import sites if necessary.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bd4f6ec1-f5c1-4629-be48-1082ea7e1a9c

📥 Commits

Reviewing files that changed from the base of the PR and between 5b1082b and 16d7d02.

📒 Files selected for processing (3)

• src/lib/onboard.ts
• src/lib/validation.test.ts
• src/lib/validation.ts

[ericksoa]

Clean, well-scoped fix. Turns a 15-minute bounded hang into an immediate actionable error.

Looks good

• Pure classifier in validation.ts — follows the existing classifySandboxCreateFailure / classifyValidationFailure pattern. Four regex patterns cover the known Docker-down signatures across macOS (Colima) and Linux (dockerd).
• Short-circuit via pRetry.AbortError — correct mechanism to bail out of the retry loop without further health polls.
• Platform-specific remediation — colima start on darwin, systemctl start docker on linux, generic fallback otherwise.
• No impact on success path — classifier only consulted when startResult.status !== 0.
• Regression guard — the "slow bootstrap" test case ensures normal k3s startup output (HelmChart, pod startup duration) is classified as unknown, keeping the retry loop engaged for legitimate slow starts.
• Good relationship with #2372 — this is defense-in-depth for daemon death mid-onboard; #2372 covers preflight. Together they close both halves of #2347.

Same note as #2356: the commit is authored by Test User <test@example.com> — git config needs fixing for future PRs. Not blocking since squash-merge uses the PR author identity.

LGTM.

[ericksoa]

Revising my earlier approval based on deeper analysis of the classifier.

Blocking: classifyGatewayStartFailure over-matches on "Failed to create Docker client"

The regex /Failed to create Docker client/i matches anywhere in the output, including historical or explanatory text. For example, output like:

client created successfully; previous Failed to create Docker client issue fixed

...would trigger docker_unreachable and abort retries with misleading Docker remediation, even though the daemon is fine. This converts a previously retryable gateway-start failure into an immediate hard failure — contradicting the PR's claim that non-Docker failure behavior is unchanged.

Recommended fix

Tighten the matcher to require stronger surrounding error context. For example, anchor the "Failed to create Docker client" pattern to line-start or require it to co-occur with a socket/daemon keyword:

/^\s*(?:Error:\s*)?Failed to create Docker client/im

Or require the "Socket not found" / "Cannot connect" patterns as the primary matchers and drop the broad "Failed to create Docker client" standalone match entirely — the first two patterns already cover the known macOS and Linux signatures.

Recommended test coverage

Add a regression test for classifyGatewayStartFailure with explanatory/history text containing "Failed to create Docker client" in a non-error context, asserting it returns unknown (not docker_unreachable).

Also add

An integration/regression test around startGatewayWithOptions that stubs gateway-start output to docker-unreachable text and asserts zero health polls plus the Docker-specific stderr output.

What still looks good

• The architecture (pure classifier + pRetry.AbortError short-circuit) is correct.
• The platform-specific remediation UX is good.
• The slow-bootstrap regression guard test is the right idea — just needs a false-positive case too.
• The relationship with #2372 (preflight) is well-scoped.

[wscurran]

Thanks for this. Since #2006 has merged in the same gateway-start path, could you please rebase this PR on current main and confirm the Docker-unreachable fast-fail behavior is still needed for #2347? Also please address the existing review feedback so we can reassess this as a targeted follow-up rather than treating it as superseded.

[wscurran]

Closing due to inactivity. This PR needs to be rebased against current main and has unresolved review feedback; review has been blocked for 7+ days without an update.

The linked issue #2347 remains open. Feel free to reopen this PR once it is rebased and the Docker-unreachable fast-fail behavior is confirmed still needed, or open a fresh PR with the targeted follow-up. Thanks for contributing!