sequenceDiagram
  participant User
  participant CLI
  participant Sandbox
  participant Openshell
  participant Docker
  participant DNSProxy
  User->>CLI: run sandbox connect
  CLI->>Sandbox: probe gateway/status
  CLI->>Openshell: sandbox exec -> GET inference.local/v1/models
  Openshell-->>CLI: probe response (200/503)
  alt probe unhealthy
    CLI->>Docker: kubectl get service kube-dns
    Docker-->>CLI: service IP / empty
    alt service empty
      CLI->>Docker: kubectl get endpoints kube-dns
      Docker-->>CLI: endpoint IP
    end
    CLI->>DNSProxy: runSetupDnsProxy -> repair
    DNSProxy-->>CLI: repaired
  end
  CLI->>User: connect result (non-fatal on repair failures)

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

PR Review Notes

Triaged this PR in a fresh worktree. Overall the DNS repair work is solid, but there are a few blockers to resolve before merge — chiefly a supersession question against your own #3223, a rebase against current main, and one custom-preset correctness bug.

🔴 Blockers (must fix before merge)

1. Merge conflicts with main. mergeStateStatus: DIRTY, branch is 4 commits behind origin/main. Rebase is needed, and in particular please verify that hasChatCompletionsToolCall / requireChatCompletionsToolCalling: true on the Ollama validation path (added by fix: complete #2731, #2697, and #2727 reliability fixes #2737) survives the rebase — the two-dot diff currently shows them being "removed" as an artifact of the stale base.

2. Supersession vs. PR fix: skip Brave policy preset for unsupported agents #3223. fix: skip Brave policy preset for unsupported agents #3223 targets the same Hermes-excludes-Brave problem with a different, more extensible, capability-based approach (webSearchSupported: boolean threaded through computeSetupPresetSuggestions / setupPoliciesWithSelection). This PR uses a hardcoded AGENT_POLICY_PRESET_EXCLUSIONS: { hermes: ["brave"] } table. Both cannot land. My recommendation: take fix: skip Brave policy preset for unsupported agents #3223's capability-based API and drop the exclusion table from this PR, so adding a new agent that doesn't support Brave is "set agentSupportsWebSearch to false" rather than "edit a CLI-side agent-name table."

3. clampPolicyPresetNames silently drops custom presets whose name collides with an exclusion. listPolicyPresetsForAgent() intentionally keeps policies.listCustomPresets(sandboxName), but clampPolicyPresetNames() filters purely by name — so a user's custom brave preset in a Hermes sandbox is interactively selectable, then wiped by resume / non-interactive paths. clampPolicyPresetNames needs to be source-aware (accept sandboxName and skip exclusion for names that are registered custom presets). CodeRabbit flagged the same issue.

🟡 Warnings (should fix)

1. src/lib/onboard.ts monolith growth: +69 net lines (10,488 → 10,557). This is over the +20-line push-back threshold we're holding refactoring PRs to. The new helpers — AGENT_POLICY_PRESET_EXCLUSIONS, isPolicyPresetExcludedForAgent, filterPolicyPresetsForAgent, resolvePolicyPresetAgentName, listPolicyPresetsForAgent, clampPolicyPresetNames — are a coherent policy-domain concept and belong in src/lib/policies.ts or a new src/lib/policies/agent-exclusions.ts with co-located tests. They're extraction-ready. Ref Architectural improvement: Extract rebuild recreate path from onboard monolith and canonicalize credential resolution #2306.

2. Exclusions can leak back in reuse / resume flows. allPresets is filtered at the top, but chosen can be re-populated from previously-applied state or resume-selected presets before the final syncPresetSelection call, so Hermes may end up re-applying brave across a resume. Either clamp at every mutation site of chosen, or clamp once at the bottom just before the sync call. (Also noted by CodeRabbit.)

3. test/onboard.test.ts:58 — isOnboardTestInternals runtime guard not updated for the new filterPolicyPresetsForAgent member. The type and destructuring were updated but the typeof value.filterPolicyPresetsForAgent === "function" check is missing from the narrowing guard. If the module ever stops exporting it, the test crashes at use-site instead of failing the guard clearly.

4. src/lib/actions/sandbox/connect.ts — new sh -c "..." shell-string probe. Not a security regression (all args are literal), but it's a new shell-string callsite for fix(security): migrate remaining shell-string callsites to argv arrays #1889. Please either factor isSandboxInferenceRouteHealthy into an argv form or leave a comment explaining why the case / output capture makes sh -c unavoidable here.

5. Scope creep. The PR title is DNS-only but the branch contains three concerns: DNS repair, Hermes/Brave preset clamping, and nemohermes uninstall routing. At minimum the uninstall commit is unrelated and would merge faster as its own PR. The DNS repair commit (9610b04a2) is the cleanest piece here and could land on its own tomorrow if separated from the policy-preset work.

6. src/commands/uninstall.ts — CLI_NAME replaced with literal "nemoclaw uninstall". If the intent is to force NemoClaw branding even under the nemohermes alias, that should be justified in a comment. As written it reads like a regression against fix: brand NemoHermes uninstall goodbye #3220's branding work.

🔵 Suggestions

1. DNS upstream discovery — consider also accepting kube-dns / coredns in namespaces other than kube-system for future flexibility. Not blocking; matches existing endpoints-fallback scope.

2. Document NEMOCLAW_DISABLE_INFERENCE_ROUTE_REPAIR. Good escape hatch; please mention it in the PR body and a docs file so operators can discover it.

3. Colocate INFERENCE_ROUTE_PROBE_TIMEOUT_MS = 10_000 with OPENSHELL_PROBE_TIMEOUT_MS in openshell/timeouts.ts. Makes future timeout tuning one-file.

✅ What's Good

• The DNS preference (kube-dns service ClusterIP → CoreDNS endpoint IP → 8.8.8.8) is a real, defensible improvement — service IPs survive pod rotation, endpoint IPs don't.
• The behavioral test in test/sandbox-connect-inference.test.ts is excellent: it stubs docker + openshell, uses a real execFileSync of the connect path, and asserts on both docker-call sequencing (new path preferred, fallback not taken) and user-visible log strings. Right level of test for this logic short of full E2E.
• ensureSandboxInferenceRoute extraction is clean — one function called at all three probe outcomes (running / recovered / failed), with consistent quiet handling. Good internal refactor even without the agent-exclusion work.
• The repair → re-probe → succeed-or-warn flow fails open with a clear warning instead of blocking connect. Correct choice for a DNS-recovery fix.

Test depth recommendation: 🔴 E2E required

The DNS repair path runs a real-world sequence — openshell sandbox exec → sh -c → curl → inference.local → DNS proxy repair → re-probe. The stubbed behavioral test proves the CLI makes the right calls in the right order, but it cannot prove the repair actually fixes the broken DNS route on a real cluster — which is the whole point of the change.

Concrete suggested E2E scenario to add before merge:

1. Create a sandbox, run nemoclaw sandbox connect once to confirm inference.local works.
2. Artificially break the DNS proxy (e.g., rewrite /etc/resolv.conf inside the sandbox, or restart DNS proxy with a bogus upstream).
3. Run nemoclaw sandbox connect again.
4. Assert stdout contains inference.local is unavailable inside '<name>'. Repairing sandbox DNS proxy... followed by inference.local route repaired., and that a subsequent openshell sandbox exec -- curl -sk https://inference.local/v1/models returns HTTP 200.
5. Negative case: set NEMOCLAW_DISABLE_INFERENCE_ROUTE_REPAIR=1 and verify repair is skipped.

Trigger via nightly-dispatch.yml with a test-filter targeting the new scenario, or run locally on sparky.

🤖 E2E Advisor Recommendation

Ran the e2e-advisor on this PR from my fork (workflow not yet available upstream). Full run: jyaunches/NemoClaw#25580284587

Recommended E2E jobs to run before merge

High-priority (required):

• sandbox-operations-e2e — exercises the new inference.local probe + DNS-proxy auto-repair on connect / --probe-only (this is the core change)
• hermes-e2e — covers the new AGENT_POLICY_PRESET_EXCLUSIONS map (hermes drops brave)
• cloud-onboard-e2e — policy preset validation + resume reapply path changed

Medium-priority:

• inference-routing-e2e — DNS proxy upstream selection + inference route swap both affect routing/credential isolation
• cloud-inference-e2e — the kube-dns service → endpoints change alters the forwarder's upstream; could silently break inference.local resolution

Suggested dispatch

Workflow: .github/workflows/nightly-e2e.yaml
jobs: sandbox-operations-e2e,inference-routing-e2e,cloud-inference-e2e,hermes-e2e,cloud-onboard-e2e

Coverage gaps flagged (worth adding)

1. sandbox-connect DNS-repair path — no existing E2E exercises it. Suggested scenario: start a sandbox, disrupt the in-sandbox DNS proxy (kill /tmp/dns-proxy.py or clobber /etc/resolv.conf), run nemoclaw <name> connect --probe-only, and assert curl https://inference.local/v1/models recovers.
2. hermes-onboard preset exclusion — AGENT_POLICY_PRESET_EXCLUSIONS has no dedicated test. A future agent added to the map or a regression removing the filter would silently re-allow disallowed presets. Suggested: non-interactive Hermes onboard with NEMOCLAW_POLICY_PRESETS=brave,pypi,npm and assert brave is dropped from the Hermes sandbox but still allowed on an openclaw onboard.

Optional (lower priority)

onboard-resume-e2e, sandbox-survival-e2e, issue-2478-crash-loop-recovery-e2e, hermes-discord-e2e, docs-validation-e2e

Generated by the E2E advisor prototype (deterministic + Pi semantic analysis). Static diff analysis only — no PR code executed.

@jyaunches I went through both of your comments and addressed them point by point.

PR Review Notes

Blockers

1. Merge conflicts with current main
   Resolved. I merged current origin/main into the PR branch in 9a3739336. The only conflict surface was the test/onboard.test.ts internals guard, and the resolved guard keeps all three relevant checks: hasChatCompletionsToolCall, hasChatCompletionsToolCallLeak, and filterSetupPolicyPresets. The PR is mergeable against current main after the merge.

2. Supersession vs. fix: skip Brave policy preset for unsupported agents #3223
   Addressed in a0426e414. I took the fix: skip Brave policy preset for unsupported agents #3223 direction and removed the hardcoded AGENT_POLICY_PRESET_EXCLUSIONS / agent-name filtering table from this PR. Brave is now filtered from setup policy presets through the existing agentSupportsWebSearch(...) capability signal, threaded as webSearchSupported into computeSetupPresetSuggestions(...) and setupPoliciesWithSelection(...). This means the decision is capability-based rather than tied to a CLI-side agent-name table.

   I also moved the setup preset support helpers into the policy domain in src/lib/policies.ts: setupPolicyPresetSupported, filterSetupPolicyPresets, listSetupPolicyPresets, and clampSetupPolicyPresetNames. That keeps the policy filtering/clamping logic out of the onboard monolith.

3. Custom preset collision in clampPolicyPresetNames
   Addressed. The clamp path is now source-aware: clampSetupPolicyPresetNames(...) receives the sandbox's custom preset names and preserves those names even when the same name would be unsupported as a built-in. I added coverage for both resume and non-interactive flows where a sandbox custom preset named brave must survive while the built-in brave preset is removed when web search is unsupported.

Warnings

1. src/lib/onboard.ts growth / policy-domain extraction
   Addressed in a0426e414. The new setup preset helpers are now in src/lib/policies.ts, and src/lib/onboard.ts now calls into the policy module instead of carrying an agent-exclusion helper cluster inline.

2. Exclusions leaking back through reuse / resume
   Addressed. Current applied presets, recorded resume presets, and explicit selected resume presets are clamped through the same support-aware helper before syncPresetSelection(...). I added a specific unsupported-only resume test so selectedPresets: ["brave"] with webSearchSupported: false removes Brave instead of falling back to tier defaults.

3. isOnboardTestInternals guard
   Addressed. The guard now checks the exported preset-filter helper (filterSetupPolicyPresets) before tests destructure/use it.

4. New sh -c inference probe
   Addressed. I left it as sh -c because the curl write-out, response body capture, and status classification need to execute inside the sandbox as one bounded probe; I added a comment explaining that and noting that sandboxName remains an argv value, so user input is not interpolated into the shell script.

5. Scope creep
   I did not split the PR at this point. The branch still contains DNS route repair, policy preset capability filtering, and nemohermes uninstall routing, but each path now has focused tests and the policy work has been aligned with fix: skip Brave policy preset for unsupported agents #3223's capability-based shape. The uninstall change is intentionally small and directly covered by test/nemohermes-alias.test.ts / test/uninstall.test.ts.

6. src/commands/uninstall.ts literal usage string
   Addressed. I added an inline comment explaining that the usage is intentionally global under the nemohermes alias because nemohermes uninstall is the package uninstaller, not a sandbox-scoped action.

Suggestions

1. Alternate CoreDNS namespaces
   Not changed in this patch. The current PR keeps the existing kube-system/kube-dns contract and only changes the preference order from endpoint IP first to service ClusterIP first with endpoint fallback.

2. Document NEMOCLAW_DISABLE_INFERENCE_ROUTE_REPAIR
   Addressed in 5c25526a and reflected in the PR body. The env var is now documented in docs/reference/commands.md and the generated user-reference skill mirror.

3. Move inference route timeout constant
   Addressed in a0426e414. OPENSHELL_INFERENCE_ROUTE_PROBE_TIMEOUT_MS now lives next to the other OpenShell timeout constants in src/lib/adapters/openshell/timeouts.ts.

E2E Advisor Recommendation

I dispatched the exact recommended targeted nightly run on this PR branch:

nightly-e2e jobs: sandbox-operations-e2e,inference-routing-e2e,cloud-inference-e2e,hermes-e2e,cloud-onboard-e2e

Run: https://github.com/NVIDIA/NemoClaw/actions/runs/25592421054

Local validation after the reviewer-response patch

• npm run build:cli
• npm run typecheck:cli
• npx vitest run test/policy-tiers-onboard.test.ts test/onboard-preset-diff.test.ts
• npx vitest run test/onboard.test.ts -t "computeSetupPresetSuggestions|agentSupportsWebSearch|configureWebSearch|prints numbered step headers"
• npx vitest run test/sandbox-connect-inference.test.ts src/lib/actions/dns/index.test.ts test/nemohermes-alias.test.ts test/uninstall.test.ts src/lib/actions/sandbox/oclif-command-adapters.test.ts src/lib/commands/simple-global-oclif-adapters.test.ts
• git diff --check

Selective E2E Results — ✅ All requested jobs passed

Run: 25592421054
Branch: fix/inference-local-dns-recovery
Requested jobs: sandbox-operations-e2e,inference-routing-e2e,cloud-inference-e2e,hermes-e2e,cloud-onboard-e2e
Summary: 5 passed, 0 failed, 0 skipped