fix(connect): recover when active gateway has no spec for the sandbox#5382
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
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)
📝 WalkthroughWalkthroughClassify gRPC responses containing "sandbox has no spec" as a missing-ownership sandbox state (both query paths) and add tests validating detection and owning-gateway recovery; export and inject a ChangesSandbox Gateway State Drift Recovery
Dashboard Onboarding Registry Dependency Injection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorFailed: Could not parse JSON from advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-advisor-raw-output.txt |
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: None Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorFailed: Could not parse JSON from advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-scenario-advisor-raw-output.txt |
PR Review AdvisorFindings: 0 needs attention, 1 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
This is an automated advisory review. A human maintainer must make the final merge decision. |
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Summary
When a sandbox is onboarded against one NemoClaw gateway and the user later switches the active OpenShell gateway to a sibling instance,
nemoclaw <sandbox> connectcallsopenshell sandbox get <sandbox>against the wrong gateway and receives the gRPC replysandbox has no spec. The lookup currently falls through to the catch-allunknown_errorbucket, sogetReconciledSandboxGatewayStatenever attempts recovery and the user is told the sandbox could not be verified. Treat that reply as a missing-on-this-gateway signal, route it through the existing named-gateway reconciler, and let itopenshell gateway selectthe sandbox's owning gateway and retry — so the connect proceeds without manual intervention.Changes
src/lib/actions/sandbox/gateway-state.ts— extend themissingpattern ingetSandboxGatewayStateandgetSandboxGatewayStateForStatusto coversandbox has no spec. The reconciler already self-heals amissinglookup against the sandbox's recorded gateway, so this single classification change wires the existing recovery path to the multi-instance case without any new branch logic.src/lib/onboard/dashboard.ts— accept an optionallistSandboxesdep onOnboardDashboardDepsand thread it through togetRegistryOccupiedDashboardPortsinensureDashboardForward. Production callers keep the live-registry default; tests can inject a stub so the dashboard helpers do not read whatever sandboxes happen to live in the test runner's~/.nemoclaw/sandboxes.json.src/lib/onboard/dashboard-port.ts— re-exportListSandboxesFnsodashboard.ts(and any other consumer of the dep seam) can name the type without duplicating the shape.test/onboard-dashboard.test.ts— passlistSandboxes: () => ({ sandboxes: [] })into everycreateOnboardDashboardHelperscall so the existing port-allocation expectations stay independent of the runner's real registry.src/lib/actions/sandbox/gateway-state-drift.test.ts— two regression tests: (1)getSandboxGatewayStateclassifies thesandbox has no specgRPC reply asmissing; (2)reconcileMissingAgainstNamedGatewayselects the sandbox's owning gateway and retries when the active gateway is a sibling that does not know about the sandbox.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
Bug Fixes
New Features
Tests