fix(onboard): detect unreachable Docker daemon in preflight (#2348) - #2372
Merged
Conversation
`docker info --format '{{json .}}'` exits 0 and emits a zero-value
client-side struct (ServerVersion: "", populated ServerErrors) when the
daemon is unreachable — e.g. after `colima stop`. The prior
non-empty-output check treated that as "daemon running", so preflight
passed and onboarding failed on the next step with a cryptic socket
error.
Replace the check with isDockerDaemonReachable, which parses the JSON
and uses layered signals:
- ServerErrors non-empty → unreachable (explicit negative)
- ServerVersion populated → reachable (canonical docker / podman-compat)
- version.Version populated → reachable (podman-docker alias native)
- non-JSON output → fall back to non-empty heuristic
Fixes #2348
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe PR enhances the Docker daemon reachability check by replacing a simple "non-empty output" heuristic with JSON-aware parsing that examines Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/preflight.ts (1)
206-210: Tighten the non-JSON fallback to avoid plain-text false positives.At Line 209, any non-JSON non-empty output is treated as reachable. If the output is a plain-text daemon-connect error, this can still misclassify unreachable Docker as reachable.
Suggested hardening
try { parsed = JSON.parse(text); } catch { - // Output fell outside the `--format '{{json .}}'` contract (older CLI, - // stubbed test input, etc.). The CLI at least produced something, so - // defer to the prior non-empty heuristic for backward compatibility. - return true; + // Backward-compatible fallback for non-JSON output, but avoid + // known daemon-connect error false positives. + const lowered = text.toLowerCase(); + if ( + lowered.includes("cannot connect to the docker daemon") || + lowered.includes("error during connect") + ) { + return false; + } + return true; }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e18e9543-a56f-49ce-8bab-c1dc56629ff1
📒 Files selected for processing (2)
src/lib/preflight.test.tssrc/lib/preflight.ts
Contributor
|
✨ Thanks for submitting this pull request that proposes a way to fix a bug that causes the onboard session to hang when the Docker daemon is unreachable. Related open issues: |
…ight-colima-false-positive
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Contributor
PR Review AdvisorFindings: 2 needs attention, 1 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
This is an automated advisory review. A human maintainer must make the final merge decision. |
cv
approved these changes
May 23, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Approved after maintainer salvage. Rebased/merged onto current main, moved the Docker daemon reachability fix onto the current onboard preflight module, added regression coverage for zero-value Docker JSON and plain-text daemon errors, and made the local test suite hermetic against host Git signing config and real Ollama auth-proxy spawns. Local validation: make check, npm test, npm run typecheck:cli. CI is green.
wscurran
added
area: packaging
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docker info --format '{{json .}}'exits0and emits a zero-value client-side struct ("ServerVersion":""plus a populatedServerErrorsarray) when the Docker daemon is unreachable — e.g. aftercolima stop. The prior preflight check only looked at whether stdout was non-empty, so it misread that struct as "daemon running".isDockerDaemonReachable, a layered JSON-based helper (see below).[1/8] Preflight checksnow reports Docker as unreachable with an actionable remediation (start_docker), instead of passing and then hanging at[2/8]with a crypticSocket not found: /var/run/docker.sock.Detection logic
ServerErrorsis a non-empty arrayServerVersionis a non-empty stringversion.Versionis a non-empty stringTest plan
ServerVersion: ""+ServerErrorsset) — reproduces [NemoClaw][macOS][Onboard] Preflight check [1/8] falsely reports "✓ Docker is running" when Colima daemon is stopped #2348 without a mac hostServerErrors, non-object JSON ("null"), non-JSON fallback, and podman-docker-alias native outputdockerInfoOutputto realistic JSON payloadsnpx vitest run src/lib/preflight.test.ts→ 41/41 passing locallyDOCKER_HOST=unix:///tmp/nonexistent.sock docker info --format '{{json .}}'(daemon-down → unreachable, daemon-up → reachable)Fixes #2348
Signed-off-by: Chengjie Wang chengjiew@nvidia.com
🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Bug Fixes