fix(inference): report a probe image-pull failure as its own diagnostic - #9311
Conversation
When the container reachability probe image was absent and Docker could not pull it, every probe run produced empty output and the failure was reported as a provider networking problem: "the Docker container reachability check failed" plus the lumped "image pull error or runtime failure" diagnostic. On WSL2 over a remote OpenSSH session, a Docker Desktop credential helper without a logon session blocks the pull of the public probe image and hits exactly this path. Classify the failure with data the stdout-only capture seam already provides: when `docker version` answers but `docker image inspect` finds no local copy of the probe image, the check never ran, so networking was never tested. Report it as a Docker image-pull failure, name the credential-helper cause, and print the reporter-verified recovery: pre-pull with an isolated Docker config, then resume onboarding. Unclassified failures keep the existing messages, and a present image still reports the runtime-failure diagnostic, so genuine networking negatives are unchanged. Closes #9308 Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedAn error occurred during the review process. Please try again later. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 12 reviews per rolling hour; 10 remain after this review. 📝 WalkthroughWalkthroughLocal provider validation now distinguishes unavailable Docker probe images from container runtime failures. Docker diagnostics return structured results, and regression tests cover Ollama, vLLM, and locally available probe images. ChangesLocal provider diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change provides a more accurate Docker image-pull diagnostic while preserving existing behavior for other failures; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant validateLocalProvider
participant collectContainerDiagnostics
participant Docker
validateLocalProvider->>collectContainerDiagnostics: request probe diagnostics
collectContainerDiagnostics->>Docker: check daemon and probe image
Docker-->>collectContainerDiagnostics: image availability or runtime result
collectContainerDiagnostics-->>validateLocalProvider: return structured diagnostic
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 9cbe46e in the TypeScript / code-coverage/cliThe overall coverage in commit 9cbe46e in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
1 additional E2E selection from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 4 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
The PR review advisor warned that the classifier could blame a pull for a run failure it never observed. It cannot: `docker run` pulls an absent image before it creates the container, and --add-host, policy, and seccomp failures all happen after that pull. Five runs precede the inspect check, so a pullable image would already be in the cache, and a post-pull failure keeps the generic runtime diagnostic. Record that invariant at the classifier so the next reader does not re-derive it. Refs #9308 Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Advisor warning PRA-1 — rebuttal, with the invariant now recorded in code (
|
… case The context-pinning case asserts every docker diagnostic command carries the persisted-context prefix and pins the command count. The image-pull classifier adds one context-pinned `docker version` probe on the all-empty failure path, so the count is now six. The pinning contract itself is unchanged and the new probe satisfies it. Refs #9308 Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
prekshivyas
left a comment
There was a problem hiding this comment.
The diagnostic cleanly distinguishes a missing probe image from provider-network failure without parsing unstable Docker stderr. It preserves generic runtime failure behavior when the daemon is unavailable or the image exists, keeps the selected Docker context on every probe, and provides a targeted recovery. Tests cover Ollama, vLLM, the present-image fallback, and context pinning.
Cross-issue sweep: no additional candidate issues found.
Security review: secrets/credentials — PASS; input validation/sanitization — PASS; authentication/authorization — PASS; dependencies — PASS; error handling/logging — PASS; cryptography/data protection — PASS; configuration/security headers — PASS; security testing — PASS; system security — PASS.
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical dated changelog entry required before planning the v0.0.110 release. The entry summarizes user-facing changes merged since v0.0.109 and links each change to its published documentation route and source PR. ## Changes - Add `docs/changelog/2026-08-17.mdx` with the exact `## v0.0.110` release heading. - Cover managed local inference, endpoint validation, onboarding and recovery, explicit experimental Portable OpenClaw, messaging and policy cleanup, backup and security hardening, and release qualification. - Preserve the documentation skip list and the current supported-agent matrix; test-only refactors, dormant activation work, and Pi-only changes are intentionally excluded. ### Source-to-doc mapping - #8711 -> `docs/changelog/2026-08-17.mdx`: Add the Muse Glimmer llama.cpp profile. - #9099 -> `docs/changelog/2026-08-17.mdx`: Update the Muse Glimmer vLLM runtime. - #9319 -> `docs/changelog/2026-08-17.mdx`: Select the provider required by an explicit serving profile. - #9311 -> `docs/changelog/2026-08-17.mdx`: Report probe-image pull failures separately. - #9345 -> `docs/changelog/2026-08-17.mdx`: Reuse mirrored Windows Ollama. - #9284 -> `docs/changelog/2026-08-17.mdx`: Complete the required Ollama upgrade. - #9320 -> `docs/changelog/2026-08-17.mdx`: Reject unsafe custom endpoint URLs before mutation. - #9119 -> `docs/changelog/2026-08-17.mdx`: Reject unsupported custom endpoint URL components. - #9236 -> `docs/changelog/2026-08-17.mdx`: Require native Anthropic tool-use evidence. - #9347 -> `docs/changelog/2026-08-17.mdx`: Distinguish Gemini runtime 404 diagnostics. - #9307 -> `docs/changelog/2026-08-17.mdx`: Preserve the recorded API family when only the model drifts. - #9233 -> `docs/changelog/2026-08-17.mdx`: Fail incomplete Hermes route synchronization. - #9185 -> `docs/changelog/2026-08-17.mdx`: Serialize Model Router lifecycle work across gateways. - #9112 -> `docs/changelog/2026-08-17.mdx`: Stop Model Router after the last routed sandbox is destroyed. - #9229 -> `docs/changelog/2026-08-17.mdx`: Verify fresh sandbox execution readiness. - #9299 -> `docs/changelog/2026-08-17.mdx`: Verify a separate agent API host forward before reporting ready. - #9318 -> `docs/changelog/2026-08-17.mdx`: Honor explicit sandbox recreation. - #9325 -> `docs/changelog/2026-08-17.mdx`: Measure readiness reuse windows from collection completion. - #9352 -> `docs/changelog/2026-08-17.mdx`: Guide users away from the deprecated global start command. - #9370 -> `docs/changelog/2026-08-17.mdx`: Persist managed OpenClaw agent identity. - #9366 -> `docs/changelog/2026-08-17.mdx`: Pass messaging dependencies during reused onboarding. - #9321 -> `docs/changelog/2026-08-17.mdx`: Detect proxied connect sessions. - #9285 -> `docs/changelog/2026-08-17.mdx`: Run probe-only recovery when absent authority cannot be created. - #9282 -> `docs/changelog/2026-08-17.mdx`: Complete probe-only recovery without platform evidence. - #8920 -> `docs/changelog/2026-08-17.mdx`: Preserve legacy gateway identity. - #9198 -> `docs/changelog/2026-08-17.mdx`: Report sandbox config-read failures. - #9201 -> `docs/changelog/2026-08-17.mdx`: Remove only the exact Docker orphan on destroy. - #9176 -> `docs/changelog/2026-08-17.mdx`: Use rootless Podman for Portable lifecycle operations. - #9197 -> `docs/changelog/2026-08-17.mdx`: Preflight Portable CPU delegation. - #9289 -> `docs/changelog/2026-08-17.mdx`: Narrow Portable policy defaults. - #9270 -> `docs/changelog/2026-08-17.mdx`: Preserve Portable model intent. - #9339 -> `docs/changelog/2026-08-17.mdx`: Reconcile timed-out Portable stop state. - #9209 -> `docs/changelog/2026-08-17.mdx`: Clean receipt-owned Portable Podman resources. - #9186 -> `docs/changelog/2026-08-17.mdx`: Separate Podman activation readiness. - #9376 -> `docs/changelog/2026-08-17.mdx`: Settle Portable OpenClaw pairing before readiness. - #9296 -> `docs/changelog/2026-08-17.mdx`: Retire messaging channel presets the host no longer configures. - #9327 -> `docs/changelog/2026-08-17.mdx`: Drop retired channels from reused messaging selections. - #9306 -> `docs/changelog/2026-08-17.mdx`: Remove gateway-enforced presets without a local record. - #9248 -> `docs/changelog/2026-08-17.mdx`: Activate Google Chat pairing approval. - #9374 -> `docs/changelog/2026-08-17.mdx`: Accept schema-owned messaging plan fields. - #9317 -> `docs/changelog/2026-08-17.mdx`: Accept safe hard-linked package files during backup. - #9288 -> `docs/changelog/2026-08-17.mdx`: Remove managed CLI shims with destroyed user data. - #9239 -> `docs/changelog/2026-08-17.mdx`: Read voice credentials from fixed descriptors. - #9269 -> `docs/changelog/2026-08-17.mdx`: Accept bounded native OpenClaw device modes. - #9371 -> `docs/changelog/2026-08-17.mdx`: Isolate OpenClaw startup-guard output. - #9351 -> `docs/changelog/2026-08-17.mdx`: Restore staging Launchable validation. - #9350 -> `docs/changelog/2026-08-17.mdx`: Retry transient collaborator-permission reads. - #9353 -> `docs/changelog/2026-08-17.mdx`: Retry transient exact-artifact downloads. - #9226 -> `docs/changelog/2026-08-17.mdx`: Add bounded Brev readiness diagnostics. - #9237 -> `docs/changelog/2026-08-17.mdx`: Report same-commit E2E reliability. - #9232 -> `docs/changelog/2026-08-17.mdx`: Execute native-runtime qualification. - #9275 -> `docs/changelog/2026-08-17.mdx`: Define E2E selection and retry guidance. - #9234 -> `docs/changelog/2026-08-17.mdx`: Move documentation review after merge. - #9365 -> `docs/changelog/2026-08-17.mdx`: Mount documentation reviewer inputs before startup. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated release-entry contract. - [ ] Tests not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; documentation-only change. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] 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 - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` (7 passed) - [x] 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 one prose-only changelog page; `npm run docs` passed the repository's strict documentation gate. - [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) — passed with 0 errors and the 2 existing Fern warnings. - [x] 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) — the SPDX header is present; dated changelog pages intentionally do not use frontmatter. --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.110. * Documented experimental managed llama.cpp and Portable OpenClaw profiles. * Covered inference validation, onboarding and recovery improvements, rootless lifecycle handling, messaging and policy updates, backups, credential handling, filesystem protections, and release qualification updates. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
When the container reachability probe image was absent from the local Docker cache and Docker could not pull it, onboarding reported the failure as a provider networking problem: "the Docker container reachability check failed for http://host.openshell.internal:11434" with the lumped diagnostic "Docker command failed (image pull error or runtime failure)". On WSL2 reached over a remote Windows OpenSSH session, Docker Desktop's
desktop.execredential helper has no logon session, blocks the pull of the public probe image, and hits exactly this path. After this change that case is reported as a Docker image-pull failure with the reporter-verified recovery, and networking is not blamed for a check that never ran.Related Issue
Closes #9308
Changes
src/lib/inference/local.ts: when all reachability retries and both diagnostic re-probes produce empty output, classify before reporting.classifyContainerRunFailureuses data the existing stdout-only capture seam already provides:docker versionanswers (daemon is up) whiledocker image inspectfinds no local copy ofcurlimages/curl:8.10.1(the probe image was never available). In that case:vllm-local.)credsStorein~/.docker/config.jsonfailing in a remote login session) and prints the recovery the reporter verified:DOCKER_CONFIG=$(mktemp -d) docker pull curlimages/curl:8.10.1followed bynemoclaw onboard --resume.src/lib/inference/local.test.ts: three cases — image-pull classification forollama-localandvllm-local(message, remediation, and absence of the networking misreport), and a present-image case pinning the unchanged runtime-failure report.Design notes for reviewers
RunCaptureFnseam as every other probe in this file. No new dependency-injection parameter, no stderr capture, andsrc/lib/inference/local.tsgains no new imports (its fan-out is pinned at exactly 21 inci/source-architecture-budget.json).dockerinvocations run only on the already-failed path, after three failed probe attempts and two empty diagnostic re-probes. The success path is untouched.vllm-local.tsand the auth-proxy path already use for host-responding cases, that is a small follow-up.Type of Change
Quality Gates
Validation on
7d4f96ae63:npx vitest run src/lib/inference/local.test.ts— 86 passed (3 new).npx vitest run src/lib/actions/inference-set.test.ts src/lib/onboard/inference-providers— 46 passed (consumers ofvalidateLocalProvider).npm run typecheck:cli— clean.npx oxlinton both changed files — clean.npm run test-size:check— passed (local.test.tsis 1429 lines against the 1500 budget).npm run source-shape:check—source_shape_cases=0.ifcount unchanged (21 at head, 21 at base) — new mocks dispatch on command content with ternaries.ollama-local)DOCKER_CONFIGpre-pull +onboard --resumeremediationvllm-local)Documentation Writer Review
no-docs-neededdocs/inference/set-up-ollama.mdxand the WSL pages describe onboarding behavior at a level this change does not alter.Signed-off-by: Dongni Yang dongniy@nvidia.com
Summary by CodeRabbit