Skip to content

fix(onboard): mirror Docker host aliases in bridge probe - #3457

Closed
ericksoa wants to merge 1 commit into
mainfrom
fix/sandbox-bridge-probe-host-alias
Closed

ericksoa wants to merge 1 commit into
mainfrom
fix/sandbox-bridge-probe-host-alias

Conversation

@ericksoa

@ericksoa ericksoa commented May 13, 2026 •

Copy link
Copy Markdown
Member

Summary

  • keep the sandbox bridge reachability probe, but make its helper container match OpenShell Docker sandboxes by adding host.openshell.internal and host.docker.internal host-gateway aliases
  • treat host alias/name-resolution failures as an inconclusive probe instead of blocking onboarding with a false firewall remediation
  • add regression coverage for the host alias args and the DNS-failure classification

Validation

  • npm run build:cli
  • npm run typecheck:cli
  • npx vitest run src/lib/onboard/gateway-sandbox-reachability.test.ts test/gateway-liveness-probe.test.ts
  • npx @biomejs/biome lint src/lib/onboard/gateway-sandbox-reachability.ts src/lib/onboard/gateway-sandbox-reachability.test.ts
  • git diff --check
  • live Docker sanity check: helper container without aliases fails with nc: bad address host.openshell.internal; with the OpenShell aliases, host.openshell.internal resolves and the probe command succeeds

Context

This is the surgical follow-up to #3441. The new probe used the same Docker network as real sandboxes but did not include the host aliases that OpenShell adds to sandbox containers, so hostname-resolution failures could be misreported as a host firewall block.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced sandbox probe error handling to detect and properly report DNS/name-resolution failures within containers, distinguishing them from network connectivity issues with improved status messages.
  • Tests

    • Added comprehensive test cases for gateway sandbox reachability probes, covering DNS failure scenarios and verifying host-gateway mapping configurations.

Review Change Stack

@ericksoa ericksoa self-assigned this May 13, 2026
@coderabbitai

coderabbitai Bot commented May 13, 2026 •

Copy link
Copy Markdown
Contributor

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: eee8018f-40b3-4d64-8218-784b05e39b11

📥 Commits

Reviewing files that changed from the base of the PR and between c517d62 and c279a28.

📒 Files selected for processing (2)
  • src/lib/onboard/gateway-sandbox-reachability.test.ts
  • src/lib/onboard/gateway-sandbox-reachability.ts

📝 Walkthrough

Walkthrough

The PR enhances the sandbox bridge reachability probe to detect DNS/name-resolution failures within the container, map host.docker.internal as a host-gateway alias, and return probe_unavailable when resolution fails rather than a TCP failure code.

Changes

Docker internal host resolution

Layer / File(s) Summary
Name-resolution failure detection and container host mapping
src/lib/onboard/gateway-sandbox-reachability.ts
Introduces DOCKER_HOST_INTERNAL_NAME constant, adds isNameResolutionFailure() helper to detect DNS errors from probe output, extends Docker probe args with --add-host mapping for host.docker.internal:host-gateway, and conditionally returns probe_unavailable when probe status 1 matches a name-resolution failure pattern.
Test coverage for resolution handling and host mappings
src/lib/onboard/gateway-sandbox-reachability.test.ts
Adds test case verifying that name-resolution failures return probe_unavailable with diagnostic detail, and updates argv assertions to confirm both host.openshell.internal:host-gateway and host.docker.internal:host-gateway mappings are passed to the probe container.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3441: Main PR's updates to gateway-sandbox-reachability.ts (adding host.docker.internal mapping via --add-host and returning probe_unavailable on name-resolution failures) directly refine the same sandbox-bridge reachability probe logic introduced/validated by PR #3441.

Suggested labels

fix, Docker, OpenShell, bug

Suggested reviewers

  • cv

Poem

🐰 A rabbit hops through Docker's gate,
Where host.docker.internal waits,
Now names resolve without a fright,
The probe lights up: success is bright! 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 'fix(onboard): mirror Docker host aliases in bridge probe' directly and specifically describes the main change: adding Docker host aliases to the bridge probe helper container.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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/sandbox-bridge-probe-host-alias

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

@github-actions

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: double-onboard-e2e, onboard-repair-e2e, onboard-resume-e2e
Optional E2E: gateway-health-honest-e2e, sandbox-survival-e2e, network-policy-e2e

Dispatch hint: double-onboard-e2e,onboard-repair-e2e,onboard-resume-e2e

Workflow run

Full advisor summary

Pi Semantic E2E Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required E2E

  • double-onboard-e2e: Runs the full Docker-driver onboarding flow twice on a real Ubuntu runner, which is exactly the path that invokes verifySandboxBridgeGatewayReachableOrExit. Any regression in the new --add-host probe argv or probe_unavailable classification would surface as an onboard error or a missing/extra warning here.
  • onboard-repair-e2e: Exercises onboard repair on the Docker-driver path; the sandbox-bridge probe runs as part of that flow and any false-positive tcp_failed (the failure mode this PR fixes by adding --add-host) would block repair onboarding.
  • onboard-resume-e2e: Exercises onboard resume on the Docker-driver path. The probe is reached on a healthy gateway resume, so this validates that the new probe argv (extra --add-host flags) does not regress the happy-path reachability classification.

Optional E2E

  • gateway-health-honest-e2e: Adjacent coverage guard for Docker-driver gateway readiness/honesty checks that run alongside the sandbox-bridge probe during onboard. Useful confidence that the reachability probe and HTTP/TCP readiness probes still compose correctly.
  • sandbox-survival-e2e: Validates that sandboxes attached to openshell-docker can actually reach the gateway via host.openshell.internal post-onboard. Indirectly confirms the same name-resolution path the probe now exercises.
  • network-policy-e2e: Exercises sandbox→host networking under the Docker-driver bridge; gives additional signal that --add-host changes don't perturb sandbox egress behavior.

New E2E recommendations

  • onboarding/probe-resilience (low): There is no E2E that fault-injects a Docker environment where host.openshell.internal does not resolve inside the probe container (older Docker without host-gateway support, custom CNI, or seccomp restrictions). The new probe_unavailable branch is unit-tested but not exercised end-to-end, and its job is precisely to keep onboard from hard-failing in those environments.
    • Suggested test: Add an onboard E2E variant that runs onboarding with a stubbed/blocked --add-host (e.g., DOCKER_HOST pointing at a daemon shim that ignores host-gateway) and asserts onboard prints the 'Could not run the sandbox bridge reachability probe ... continuing' warning and still completes successfully.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: double-onboard-e2e,onboard-repair-e2e,onboard-resume-e2e

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 25793861473
Branch: fix/sandbox-bridge-probe-host-alias
Requested jobs: all (no filter)
Summary: 5 passed, 34 failed, 2 skipped

Job Result
brave-search-e2e ✅ success
cloud-e2e ❌ failure
cloud-inference-e2e ❌ failure
cloud-onboard-e2e ❌ failure
credential-migration-e2e ❌ failure
credential-sanitization-e2e ❌ failure
deployment-services-e2e ❌ failure
device-auth-health-e2e ❌ failure
diagnostics-e2e ❌ failure
docs-validation-e2e ❌ failure
double-onboard-e2e ❌ failure
gpu-double-onboard-e2e ⏭️ skipped
gpu-e2e ⏭️ skipped
hermes-discord-e2e ❌ failure
hermes-e2e ❌ failure
hermes-inference-switch-e2e ❌ failure
hermes-slack-e2e ❌ failure
inference-routing-e2e ✅ success
issue-2478-crash-loop-recovery-e2e ❌ failure
kimi-inference-compat-e2e ✅ success
launchable-smoke-e2e ❌ failure
messaging-compatible-endpoint-e2e ❌ failure
messaging-providers-e2e ❌ failure
network-policy-e2e ❌ failure
onboard-repair-e2e ❌ failure
onboard-resume-e2e ❌ failure
openclaw-inference-switch-e2e ❌ failure
openshell-gateway-upgrade-e2e ❌ failure
overlayfs-autofix-e2e ✅ success
rebuild-hermes-e2e ❌ failure
rebuild-hermes-stale-base-e2e ❌ failure
rebuild-openclaw-e2e ✅ success
runtime-overrides-e2e ❌ failure
sandbox-operations-e2e ❌ failure
sandbox-survival-e2e ❌ failure
shields-config-e2e ❌ failure
skill-agent-e2e ❌ failure
snapshot-commands-e2e ❌ failure
telegram-injection-e2e ❌ failure
token-rotation-e2e ❌ failure
upgrade-stale-sandbox-e2e ❌ failure

Failed jobs: cloud-e2e, cloud-inference-e2e, cloud-onboard-e2e, credential-migration-e2e, credential-sanitization-e2e, deployment-services-e2e, device-auth-health-e2e, diagnostics-e2e, docs-validation-e2e, double-onboard-e2e, hermes-discord-e2e, hermes-e2e, hermes-inference-switch-e2e, hermes-slack-e2e, issue-2478-crash-loop-recovery-e2e, launchable-smoke-e2e, messaging-compatible-endpoint-e2e, messaging-providers-e2e, network-policy-e2e, onboard-repair-e2e, onboard-resume-e2e, openclaw-inference-switch-e2e, openshell-gateway-upgrade-e2e, rebuild-hermes-e2e, rebuild-hermes-stale-base-e2e, runtime-overrides-e2e, sandbox-operations-e2e, sandbox-survival-e2e, shields-config-e2e, skill-agent-e2e, snapshot-commands-e2e, telegram-injection-e2e, token-rotation-e2e, upgrade-stale-sandbox-e2e. Check run artifacts for logs.

@wscurran

Copy link
Copy Markdown
Contributor

@ericksoa ericksoa closed this Jun 2, 2026
@wscurran wscurran added area: packaging Packages, images, registries, installers, or distribution area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression platform: container Affects Docker, containerd, Podman, or images and removed area: packaging Packages, images, registries, installers, or distribution Docker labels Jun 3, 2026
@cv
cv deleted the fix/sandbox-bridge-probe-host-alias branch June 28, 2026 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression platform: container Affects Docker, containerd, Podman, or images

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants