Skip to content

fix(onboard): match mux-delegated ssh forwards in untracked-forward fallback (#6099) - #6833

Merged
cv merged 2 commits into
NVIDIA:mainfrom
tedy-y:fix-6099
Jul 14, 2026
Merged

fix(onboard): match mux-delegated ssh forwards in untracked-forward fallback (#6099)#6833
cv merged 2 commits into
NVIDIA:mainfrom
tedy-y:fix-6099

Conversation

@tedy-y

@tedy-y tedy-y commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

nemoclaw onboard still deletes a healthy sandbox after "forward did not appear in list within 180000ms" in NemoClaw v0.0.81 even previous fix #6116 is merged. One reproducible case is when the host's ssh config applies ControlMaster auto to the sandbox host: the spawned ssh client delegates its -L forward to the ControlMaster mux daemon and exits, and openshell 0.0.72+ reports that exit as ssh exited before local forward listener opened — a diagnostic the untracked-forward fallback regex does not match (due to openshell's recent code change in crates/openshell-cli/src/ssh.rs#419), so the live-port probe never runs. After this change the fallback recognizes the untracked port-forward and confirms the forward via the existing live-port probe instead of rolling back.

Related Issue

Fixes #6099

Changes

  • Extend looksLikeUntrackedForward() in src/lib/onboard/forward-start.ts to also match the openshell 0.0.72 diagnostic variants (ssh exited before local forward listener opened, local forward listener was not reachable), and document the ControlMaster mux-delegation mechanism. Confirmation still requires the live-port probe, and the EADDRINUSE conflict check still runs first, so a genuinely failed ssh (no listener) keeps timing out exactly as before.
  • Add tests: regex unit coverage for the new diagnostic, an end-to-end helper test confirming ok-port-live for a mux-delegated forward with a live port, and a negative test proving the same diagnostic without a live port still times out.

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: internal forward-confirmation fallback; no user-facing docs describe the openshell diagnostic text.
  • 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: requesting maintainer sensitive-path review (onboarding forward confirmation). The change does not weaken the gate: the fallback still requires openshell's own failure notice AND a live local listener, and the port-conflict check retains precedence.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: npx vitest run --project cli src/lib/onboard/forward-start.test.ts → 27 passed (24 existing + 3 new)
  • 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)

Reproduction evidence

A/B verified on a clean macOS 26.5 / Colima (aarch64) host with nemoclaw v0.0.81 + openshell 0.0.72:

  • Control (no ControlMaster): onboard succeeds first-try; both forwards tracked in openshell forward list.
  • With Host * / ControlMaster auto + ControlPersist 600 in ~/.ssh/config: onboard fails with the exact [macOS][Onboard] nemoclaw onboard deletes the created sandbox after dashboard port-forward "did not appear in list within 180000ms" #6099 signature and deletes the sandbox. Captured mid-failure: the ControlMaster mux daemon (ssh: ~/.ssh/sockets/<user>@sandbox-22 [mux], ppid 1) holds 127.0.0.1:18789 LISTEN, the dashboard answers HTTP 200, while openshell forward list stays empty — the exact condition the extended fallback now confirms.
  • A manual foreground openshell forward start -d 127.0.0.1:18999 <sandbox> in the same state errors with ssh exited before local forward listener opened on 127.0.0.1:18999 while the mux picks up the port anyway, confirming the diagnostic text this PR adds to the regex.

A complementary root-cause option (out of scope here) is for openshell to spawn its forward ssh with -o ControlMaster=no -o ControlPath=none, which would also keep forward list tracking accurate under multiplexed ssh configs.


🤖 Generated with Claude Code

Signed-off-by: Tedy Yu tedyy@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved SSH port-forward startup detection for ControlMaster and other delegated connection scenarios.
    • Forwarding now correctly succeeds when the local listener is active, even if the forward list is temporarily empty or SSH reports a listener diagnostic.
    • Improved handling of inactive listeners so the system continues waiting and reports a timeout appropriately.

…allback (NVIDIA#6099)

Under `Host *` / `ControlMaster auto` ssh config, the ssh client spawned by
`openshell forward start --background` delegates its -L forward to the
ControlMaster mux daemon and exits. openshell 0.0.72+ reports this as
"ssh exited before local forward listener opened", which the
untracked-forward fallback regex did not match, so the live-port probe never
ran and onboard timed out and rolled back a healthy sandbox — even though the
mux daemon holds the listener and the dashboard serves HTTP 200 throughout.

Extend looksLikeUntrackedForward to also match the openshell 0.0.72
diagnostic variants. Confirmation still requires the live-port probe, and the
EADDRINUSE conflict check still runs first, so a genuinely failed ssh keeps
timing out as before.

Verified on a clean macOS 26.5 / Colima host (nemoclaw v0.0.81, openshell
0.0.72): onboard succeeds without ControlMaster, reproduces the NVIDIA#6099
rollback with ControlMaster auto, and the captured failure diagnostic matches
the added patterns.

Signed-off-by: Tedy Yu <tedyy@nvidia.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The forward-start diagnostic matcher now recognizes additional mux-delegated SSH errors. Tests cover confirming an untracked forward through a live local port and timing out when that port remains unavailable.

Changes

Forward diagnostics

Layer / File(s) Summary
Expand untracked-forward diagnostic matching
src/lib/onboard/forward-start.ts, src/lib/onboard/forward-start.test.ts
The matcher recognizes SSH/control-master listener startup failures, including the OpenSSH 0.0.72 diagnostic text.
Validate live-port confirmation outcomes
src/lib/onboard/forward-start.test.ts
Tests verify ok-port-live confirmation for a listening port and timeout behavior when the port probe remains false.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: NV QA, bug-fix, area: sandbox

Suggested reviewers: cv, laitingsheng

Sequence Diagram(s)

sequenceDiagram
  participant ForwardStart
  participant ForwardList
  participant IsPortListening
  ForwardStart->>ForwardList: Poll forward registration
  ForwardList-->>ForwardStart: Empty
  ForwardStart->>IsPortListening: Probe local port
  IsPortListening-->>ForwardStart: Live or not live
  ForwardStart-->>ForwardStart: Confirm or continue until timeout
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #6099 by treating mux-delegated untracked forwards as valid when the local port is listening, with tests covering success and timeout cases.
Out of Scope Changes check ✅ Passed The diff appears narrowly focused on forward-start detection logic and tests, with no obvious unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: matching mux-delegated SSH forwards in the untracked-forward fallback.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / medium confidence
Primary next action: No advisor follow-up required beyond maintainer review.
Findings: 0 blockers · 0 warnings · 0 optional suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (non-blocking second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron is a non-blocking second opinion. Its prose, findings, and E2E guidance do not change the primary assessment above and remain in workflow artifacts only.

E2E guidance

Advisory only: coverage and selector recommendations are non-authoritative. E2E / PR Gate independently computes and dispatches trusted jobs without consuming this output.

Recommended coverage: onboard-repair, onboard-resume, cloud-onboard
Recommended selectors: onboard-repair, onboard-resume, cloud-onboard

  • onboard-repair — Selected from the trusted checked-in E2E coverage inventory.

  • onboard-resume — Selected from the trusted checked-in E2E coverage inventory.

  • cloud-onboard — Selected from the trusted checked-in E2E coverage inventory.

  • onboard-repair — Selected as a trusted checked-in E2E job.

  • onboard-resume — Selected as a trusted checked-in E2E job.

  • cloud-onboard — Selected as a trusted checked-in E2E job.

2 optional coverage items · 2 optional selectors · 0 new-test recommendations
  • Optional coverage tunnel-lifecycle — Selected from the trusted checked-in E2E coverage inventory.
  • Optional coverage onboard-negative-paths — Selected from the trusted checked-in E2E coverage inventory.
  • Optional selector tunnel-lifecycle — Selected as a trusted checked-in E2E job.
  • Optional selector onboard-negative-paths — Selected as a trusted checked-in E2E job.

Workflow run details

This is an automated, non-authoritative review. Findings are inputs to maintainer adjudication. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision.

@tedy-y

tedy-y commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Maintainers: this PR trips the fork E2E gate because the diff touches onboarding, selecting the credential-bearing onboard-repair / onboard-resume jobs. Per the gate output, could someone approve the e2e-no-secret-exception environment on E2E / PR Gate Controller run 29310627773 (Review deployments), or use the manual workflow-dispatch resolver? The controller run has since failed closed, so it may need a re-run before the approval prompt reappears.

No fork code ran and no secret was exposed (fail-closed state); all other checks are green — cli tests, shards, typecheck, and sandbox image builds. Happy to address any review feedback on the change itself.

@tedy-y

tedy-y commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Empirical validation of this fix on the same clean macOS 26.5 / Colima reproduction host (v0.0.81 install, Host * / ControlMaster auto active, loopback inference endpoint):

  • Unpatched v0.0.81 (2026-07-13, ~/runC2-v081.log): five "Still waiting for forward" cycles → forward did not appear in list within 180000ms … ssh exited before local forward listener opened → healthy sandbox deleted, onboarding failed.
  • v0.0.81 with this PR's regex applied to dist/lib/onboard/forward-start.js (2026-07-14, ~/runD2-patched.log): zero wait cycles — the extended pattern matched openshell's diagnostic on the first poll, the live-port probe confirmed the mux-held listener, and onboarding completed (Hermes is ready, sandbox container up, registered in nemoclaw list). No rollback.

One observation for reviewers: as with the existing #6099 fallback semantics, the confirmed forward remains untracked in openshell forward list, and under ControlMaster the post-onboard dashboard health check can still report port forward not working (connection refused) — the sandbox itself is healthy. That residual symptom is the reason the PR description also mentions the complementary openshell-side option (-o ControlMaster=no -o ControlPath=none on the spawned forward ssh) as the eventual root-cause fix.

@tedy-y tedy-y self-assigned this Jul 14, 2026
@tedy-y
tedy-y requested a review from cv July 14, 2026 07:38
@cv cv added the v0.0.83 label Jul 14, 2026
@cv
cv merged commit 33d55b0 into NVIDIA:main Jul 14, 2026
67 of 70 checks passed
jyaunches added a commit that referenced this pull request Jul 14, 2026
## Summary

Add v0.0.83 release notes to `docs/about/release-notes.mdx` for pre-tag
release prep.

## Source Summary

- #6773 -> `docs/about/release-notes.mdx`: Shared inference route
changes are explicit and fail-safe; status shows recorded route, live
route, and drift.
- #6875 -> `docs/about/release-notes.mdx`: DGX Station GB300 express
setup restored; vLLM storage preflight narrowed.
- #6770 -> `docs/about/release-notes.mdx`: Risky Spark vLLM server
warning during onboarding.
- #6856 -> `docs/about/release-notes.mdx`: Re-onboard reuse preserves
tier-default brave/tavily presets.
- #6867 -> `docs/about/release-notes.mdx`: Unreachable custom endpoint
routed through transport-recovery path.
- #6860 -> `docs/about/release-notes.mdx`: Rebuild preflight uses
model-aware token field for o-series/GPT-5.
- #6845 -> `docs/about/release-notes.mdx`: Corporate CA anchored for
image build TLS.
- #6833 -> `docs/about/release-notes.mdx`: SSH ControlMaster-delegated
forwards recognized in fallback.
- #6837 -> `docs/about/release-notes.mdx`: Hermes light skin writes via
stdin on macOS.

## 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
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only release notes
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed
- [ ] Non-success, skipped, or missing CI check accepted by maintainer

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as Verified in GitHub
- [x] Normal pre-commit, commit-msg, and pre-push hooks passed
- [x] `npm run docs` passes with 0 errors

Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>

Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>
@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow bug-fix PR fixes a bug or regression labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[macOS][Onboard] nemoclaw onboard deletes the created sandbox after dashboard port-forward "did not appear in list within 180000ms"

3 participants