fix(inference): accept ip neighbor output that omits the filtered dev by Dongni-Yang · Pull Request #8527 · NVIDIA/NemoClaw

Dongni-Yang · 2026-08-07T04:46:25Z

Summary

The DGX Spark managed-cluster connectivity probe runs ip -j neigh show to <peer> dev <netdev>. ip applies dev as a server-side filter and then omits the dev key from the JSON it prints, so comparing the parsed dev against the requested netdev compared an empty string against the netdev and rejected every rail. A healthy two-rail direct ConnectX-7 fabric could never qualify, and the managed vLLM onboard aborted at [3/8] Configuring inference provider on every dual DGX Spark. The probe now treats an absent dev as belonging to the requested netdev, and the failure message names the rail and the probe that rejected the fabric instead of naming all three probes at once.

Related Issue

Closes #8519

Changes

connectivityCheck accepts a neighbor entry whose dev key is absent, because ip already filtered on dev. This matches the assumption the sibling probe in src/lib/inference/vllm-station-cluster.ts has always made.
probeConnectivity returns ManagedClusterConnectivityFailure | null instead of boolean, so the caller can report which rail and which probe failed. null means every rail passed every probe.
probeManagedClusterManagedServingCapability renders that failure. Before: Direct route, neighbor, or jumbo connectivity failed on {hostname}. After: The neighbor check failed on {hostname} rail {netdev}., or Managed cluster connectivity needs exactly two direct ConnectX-7 rails on {hostname}. when the candidate rail set is unusable and no per-rail probe ran.
The message uses the check names the documentation already uses — route, neighbor, and jumbo-frame.
The unit fixture emitted a dev key that real ip never returns for this query, which is why unit tests passed while every real fabric was rejected. The fixture now mirrors the real output, and that change alone turns the existing test red on main.

ManagedClusterConnectivityFailure is not a compatibility or extension layer. Its current consumer is the failure message in probeManagedClusterManagedServingCapability, which cannot name the failing rail or probe while the probe returns a bare boolean. names the rail and the sub-check that rejected the fabric (#8519) protects that contract.

Type of Change

Code change (feature, bug fix, or refactor)
Code change with doc updates
Doc only (prose changes, no code sample modifications)
Doc only (includes code sample changes)

Quality Gates

Tests added or updated for changed behavior
Existing tests cover changed behavior — justification:
Tests not applicable — justification:
Docs updated for user-facing behavior changes
Docs not applicable — justification: No page quotes the previous message. docs/inference/set-up-vllm-on-two-dgx-sparks.mdx documents the healthy topology as a prerequisite, which is the topology this defect rejected, so the prose was already correct.
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: Requested. This changes inference host qualification and a contributor cannot self-approve it.
Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

Documentation writer subagent reviewed the completed changes
Result: no-docs-needed
Evidence: No documentation path changed. Reviewed docs/inference/set-up-vllm-on-two-dgx-sparks.mdx, docs/inference/set-up-vllm-on-two-dgx-stations.mdx, and docs/get-started/dgx-station-preparation.mdx. No page quotes the previous message, and the generic "route, neighbor, and jumbo-frame checks" phrasing stays accurate. The review changed the message to use those documented check names.
Agent: Claude Code

Verification

PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
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
Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project cli src/lib/inference/serving/managed-cluster-discovery.test.ts — 30 passed. Both new tests fail on the unfixed source (expected false to be null, expected false to deeply equal { check: 'jumbo' }) and pass after the fix. npm run typecheck:cli clean.
Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
Quality Gates section completed with required justifications or waivers
No secrets, API keys, or credentials committed
npm run docs builds without warnings (doc changes only)
Doc pages follow the style guide (doc changes only)
New doc pages include SPDX header and frontmatter (new pages only)

Notes for reviewers

I do not have two DGX Spark systems, so this is verified at the unit layer rather than on the wire. The ip behavior itself is reproducible on any Linux host:

$ ip -j neigh show to 10.172.52.2 dev enp3s0
[{"dst":"10.172.52.2","lladdr":"2c:5e:ab:5b:e4:68","state":["STALE"]}]

$ ip -j neigh show to 10.172.52.2
[{"dst":"10.172.52.2","dev":"enp3s0","lladdr":"2c:5e:ab:5b:e4:68","state":["STALE"]}]

That is iproute2 5.5, and the report is iproute2 6.1, so the defect is not specific to one iproute2 release.

src/lib/inference/serving/managed-cluster-discovery.ts has a pre-existing assist/source/organizeImports finding at its import block that also reports on unmodified main. I left it alone rather than widen this diff.

npx vitest run --project cli src/lib/inference/ reports 3 failures in src/lib/inference/vllm-station-model-staging.test.ts. Those reproduce identically with this change stashed, so they are pre-existing and unrelated.

Reporter #8519 also filed #8520 against the same install path. This change does not address that one.

Signed-off-by: Dongni Yang dongniy@nvidia.com

Summary by CodeRabbit

Bug Fixes
- Improved managed-cluster connectivity diagnostics by identifying whether failures occur during rail validation, routing, jumbo-frame, or neighbor checks.
- Network device details are now included when relevant, making connectivity issues easier to troubleshoot.
- Corrected neighbor validation when device information is omitted from command output.
- Discovery now reports the first detected connectivity failure while preserving clear success handling.

The DGX Spark managed-cluster connectivity probe runs `ip -j neigh show to <peer> dev <netdev>`. iproute2 applies `dev` as a server-side filter and then omits the `dev` key from the JSON it prints, so comparing the parsed `dev` against the requested netdev compared an empty string against the netdev and rejected every rail. A healthy two-rail direct CX-7 fabric could never qualify, and the managed vLLM onboard aborted at provider configuration on every dual DGX Spark. An absent `dev` already means the entry belongs to the requested netdev, which is what the sibling probe in vllm-station-cluster.ts assumes. The unit fixture hid the defect by emitting a `dev` key that real `ip` never returns for this query, so the fixture now mirrors the real output. Report which rail and which probe rejected the fabric instead of naming all three probes at once. A single line that fires when route, jumbo, and neighbor all pass independently leaves no way to act on the failure. Closes #8519 Signed-off-by: Dongni Yang <dongniy@nvidia.com>

The failure message called the probes "direct route" and "jumbo frame". The documentation and the DGX Station preparation guide call the same three probes route, neighbor, and jumbo-frame checks, so use those names in the message. Refs #8519 Signed-off-by: Dongni Yang <dongniy@nvidia.com>

coderabbitai · 2026-08-07T04:46:45Z

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: c318ebe7-ef16-42a9-b5b6-74775dbdb021

📥 Commits

Reviewing files that changed from the base of the PR and between eb1d2f5 and 028bb62.

📒 Files selected for processing (3)

src/lib/inference/serving/managed-cluster-discovery-production.ts
src/lib/inference/serving/managed-cluster-discovery.test.ts
src/lib/inference/serving/managed-cluster-discovery.ts

📝 Walkthrough

Walkthrough

Managed-cluster connectivity probing now returns structured failure details. Route, jumbo, neighbor, and rail-validation failures identify the failed check, with network devices included where applicable. Neighbor validation accepts JSON without a dev field, and discovery reports specific connectivity failure reasons.

Changes

Connectivity diagnostics

Layer / File(s)	Summary
Failure contract and discovery reporting `src/lib/inference/serving/managed-cluster-discovery.ts`	Adds the `ManagedClusterConnectivityFailure` union, updates the probe dependency contract, and reports specific connectivity failure reasons.
Structured connectivity probing `src/lib/inference/serving/managed-cluster-discovery-production.ts`	Returns structured route, jumbo, neighbor, and rail failures. Neighbor checks accept omitted `dev` fields. Successful probes return `null`.
Connectivity fixture and validation coverage `src/lib/inference/serving/managed-cluster-discovery.test.ts`	Adds reusable transport fixtures and assertions for successful checks, rail-specific failures, omitted neighbor devices, jumbo failures, missing neighbors, and incomplete rail sets.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ManagedClusterDiscovery
  participant probeConnectivity
  participant connectivityCheck
  ManagedClusterDiscovery->>probeConnectivity: connectivity requests
  probeConnectivity->>connectivityCheck: validate each rail
  connectivityCheck-->>probeConnectivity: failure details or null
  probeConnectivity-->>ManagedClusterDiscovery: first failure or null

Suggested reviewers: ericksoa

🚥 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 title clearly identifies the main fix for neighbor output that omits the filtered network device.
Linked Issues check	✅ Passed	The changes address omitted neighbor devices, validate connectivity checks, and report the affected rail and failed check for issue [`#8519`].
Out of Scope Changes check	✅ Passed	The implementation and test updates directly support the connectivity detection fix and failure reporting requirements in issue [`#8519`].
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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/8519-spark-neigh-dev-filter

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

github-code-quality · 2026-08-07T04:47:22Z

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 028bb62 in the fix/8519-spark-neigh... branch remains at 96%, unchanged from commit 22fd900 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 028bb62 in the fix/8519-spark-neigh... branch remains at 81%, unchanged from commit e3a824d in the main branch.

Show a code coverage summary of the most impacted files.

File	`main` e3a824d	`fix/8519-spark-neigh...` 028bb62	+/-
`src/lib/core/pr...mpt-activity.ts`	92%	67%	-25%
`src/lib/inferen...probe-models.ts`	93%	78%	-15%
`src/lib/inferen...tion-session.ts`	91%	85%	-6%
`src/lib/inferen...board-probes.ts`	95%	91%	-4%
`src/lib/inferen...y-production.ts`	47%	48%	+1%
`src/lib/credentials/store.ts`	55%	56%	+1%
`src/lib/trace.ts`	90%	94%	+4%
`src/lib/messagi...onfig-parser.ts`	93%	97%	+4%
`src/lib/messagi...onfig-parser.ts`	93%	100%	+7%
`src/lib/messagi...ink-base-url.ts`	90%	100%	+10%

_{Updated August 07, 2026 05:43 UTC}

github-actions · 2026-08-07T04:56:32Z

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
Nemotron 3 Ultra (second opinion): Failed after a partial review · low confidence · 0 blockers · 0 warnings · 0 suggestions

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

2 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

established — jumbo-frame at src/lib/inference/serving/managed-cluster-discovery.ts:942: Keep `jumbo-frame`; it matches established repository terminology.
established — rail at src/lib/inference/serving/managed-cluster-discovery.ts:157: Keep `rail`; its physical-path meaning is established and distinguishes the required paths.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: inference-routing, network-policy

1 optional E2E recommendation

spark-install

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

apurvvkumaria · 2026-08-07T05:32:44Z

CI failure classification: confirmed transient runner-pressure timeout; failed jobs are being rerun.

The failed shard timed out at src/lib/inference/serving/host-local-vllm-selection.test.ts:61 after 5 seconds. That synchronous test and file are outside this PR's diff and have not changed since #8249. The PR's managed-cluster tests were not the failing case.

Local validation on this branch completed the timed-out test together with the changed managed-cluster suite: 33/33 tests passed in 1.88 seconds after the normal CLI build. This supports an infrastructure/test-scheduling classification rather than a code defect in this PR.

I am rerunning only the failed CI jobs. The separate Nemotron advisor service failure is optional and does not replace the successful primary review advisor.

apurvvkumaria

Approved after an independent maintainer review of revision 028bb6250.

Security verdict: PASS. No blocking findings were identified.

Category	Result	Evidence
Secrets and credentials	PASS	The three-file diff adds no credential material or secret handling.
Input validation	PASS	The filtered neighbor result can omit `dev`; an explicit different device, destination, MAC address, state, malformed JSON, or missing row still fails. Route and rail inputs retain their existing validation.
Authentication and authorization	PASS	The change does not alter SSH trust, identity checks, authorization, or credential custody.
Dependencies	PASS	The change adds no dependency or package update.
Error handling and logging	PASS	Structured failures expose only the check, host, and network device required for diagnosis. They do not expose credentials or internal paths.
Cryptography and data protection	PASS	Not applicable. The change adds no cryptographic operation or data-storage path.
Configuration and secure defaults	PASS	The two-rail requirement, direct-route requirement, jumbo-frame probe, expected peer MAC check, and accepted neighbor-state allowlist remain fail-closed.
Security testing	PASS	The regression test covers real `ip` JSON without `dev`; targeted tests also cover route, jumbo-frame, neighbor, and rail-cardinality failures.
Holistic posture	PASS	The branch accepts only the field omission caused by the command's own device filter and does not weaken host, rail, SSH, route, MAC, or neighbor-state qualification.

Correctness review also confirmed deterministic first-failure reporting in the existing node and rail order, two distinct validated rail devices, and documentation-aligned route, neighbor, and jumbo-frame messages.

Documentation writer review: PASS, no documentation change needed. The two-DGX Spark page already documents the healthy topology and the route, neighbor, and jumbo-frame requirements; no page quotes the replaced aggregate error. The receipt matches this revision and the current AGENTS.md content.

Verification: the focused managed-cluster suite passed 30/30 tests, CLI type-checking passed, CodeRabbit reported no actionable comments, the primary PR Review Advisor reported no findings, all review threads are resolved, both commits are GitHub-verified and carry DCO trailers, and the DCO check passes. The focused CI rerun for the unrelated runner-pressure timeout remains subject to the normal required checks; this approval does not waive any check.

…e check (#8692)  ## Summary The dual DGX Spark managed-vLLM onboard aborted at "[3/8] Configuring inference provider" (`The route check failed …`, exit 1) on verifiably healthy clusters. The connectivity route check ran `ip -j route get <peer> from <src> oif <dev>` and matched the source against `route.prefsrc ?? route.src`; on iproute2 6.1.0 (DGX OS 7.5.0) the `from <src>` argument makes iproute2 echo the source back as the JSON `from` field instead of `prefsrc`/`src`, so the source was `undefined` and every healthy rail failed the check. This reads the source from `from` as well, matching the sibling Python probe and the neighbor-check field-shape fix in #8519/#8527. ## Related Issue Closes #8684 ## Changes - `src/lib/inference/serving/managed-cluster-discovery-production.ts`: in `connectivityCheck`, read the route source as `route.prefsrc ?? route.src ?? route.from` so the check accepts the source when iproute2 6.1.0 echoes it as `from`. One-field widening; no behavior change for outputs that carry `prefsrc`/`src`. - `src/lib/inference/serving/managed-cluster-discovery.test.ts`: the two connectivity route mocks emitted an unrealistic `{ prefsrc, scope: "link" }` shape (with a `from` invocation) that hid the defect; update them to the real iproute2 6.1.0 shape (`{ dst, from, dev, flags, uid, cache }` — source as `from`, no `prefsrc`/`scope`), and add a `#8684` regression asserting a healthy dual-Spark cluster passes. With the mocks corrected, the existing healthy-path tests fail without the source fix. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: internal connectivity-probe parsing fix; it restores the already-documented dual DGX Spark managed-vLLM onboard to work as documented, with no new command, flag, option, or documented contract. - [x] 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: `inference`/onboarding connectivity path. The change only widens the accepted iproute2 source field (`?? route.from`) to fix a false-negative route check; it does not weaken any check (dev/gateway/scope/source-match all still enforced) or touch credentials/policy. Same accepted field-shape class as #8519/#8527. Deferred to normal CODEOWNERS review. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: No documentation paths changed. The fix is an internal iproute2-output parsing correction in the managed-cluster connectivity probe; it makes the documented dual DGX Spark managed-vLLM onboard succeed on healthy clusters rather than changing any documented behavior, command, flag, or output. - Agent: Claude Code   ## 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 — command/result or justification: `npx vitest run --project cli src/lib/inference/serving/managed-cluster-discovery.test.ts` on a fresh clone on an Ubuntu host (Node v22.23.1, `npm ci` + plugin build) → 31 passed. Two-way check: reverting only `managed-cluster-discovery-production.ts` to `origin/main` (keeping the corrected tests) fails 4 healthy-cluster tests incl. the new #8684 regression (route source `undefined` = bug reproduced with the real iproute2 6.1.0 output shape the reporter captured). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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) - [ ] 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) --- Signed-off-by: Jason Ma <jama@nvidia.com>  ## Summary by CodeRabbit * **Bug Fixes** * Improved route connectivity validation for environments using newer iproute2 output. * Connectivity checks now correctly recognize source addresses reported through the `from` field. * Prevented healthy dual-rail connections from being incorrectly flagged as unavailable. * **Tests** * Added coverage for connectivity responses matching iproute2 6.1.0 output.  Signed-off-by: Jason Ma <jama@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Dongni-Yang added 2 commits August 7, 2026 12:30

Dongni-Yang added the v0.0.105 label Aug 7, 2026

apurvvkumaria added the release-target label Aug 7, 2026

apurvvkumaria self-assigned this Aug 7, 2026

apurvvkumaria approved these changes Aug 7, 2026

View reviewed changes

apurvvkumaria merged commit e99f42c into main Aug 7, 2026
82 of 86 checks passed

apurvvkumaria deleted the fix/8519-spark-neigh-dev-filter branch August 7, 2026 05:45

hulynn mentioned this pull request Aug 10, 2026

[DGX Spark][Inference] dual-Spark managed-vLLM onboard fails at the route check on a healthy cluster #8684

Closed

jason-ma-nv mentioned this pull request Aug 10, 2026

fix(inference): accept iproute2 'from' source in managed-cluster route check #8692

Merged

22 tasks

Navigation Menu

Uh oh!

fix(inference): accept ip neighbor output that omits the filtered dev - #8527

fix(inference): accept ip neighbor output that omits the filtered dev#8527
apurvvkumaria merged 2 commits into
mainfrom
fix/8519-spark-neigh-dev-filter

Dongni-Yang commented Aug 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-code-quality Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

apurvvkumaria commented Aug 7, 2026

Uh oh!

apurvvkumaria left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Dongni-Yang commented Aug 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Type of Change

Quality Gates

Documentation Writer Review

Verification

Notes for reviewers

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-code-quality Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Overview

TypeScript / code-coverage/plugin

TypeScript / code-coverage/cli

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Advisor — No blocking findings reported

Model lanes

E2E guidance

Uh oh!

apurvvkumaria commented Aug 7, 2026

Uh oh!

apurvvkumaria left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Dongni-Yang commented Aug 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading

github-code-quality Bot commented Aug 7, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading