fix(inference): pre-flight sandbox read before mutating the inference route - #7006
Conversation
… route Signed-off-by: Rui Luo <ruluo@nvidia.com>
Signed-off-by: Rui Luo <ruluo@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 (3)
📝 WalkthroughWalkthroughThe inference-set command now reads and validates the in-sandbox configuration before changing gateway or registry state. Sandbox read failures become actionable ChangesInference-set pre-flight validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant runInferenceSet
participant readInSandboxConfigOrFail
participant readSandboxConfig
participant OpenShellGateway
participant NemoClawRegistry
runInferenceSet->>readInSandboxConfigOrFail: Read sandbox configuration
readInSandboxConfigOrFail->>readSandboxConfig: Load configuration
readSandboxConfig-->>readInSandboxConfigOrFail: Configuration or SandboxConfigError
readInSandboxConfigOrFail-->>runInferenceSet: Configuration or InferenceSetError
runInferenceSet->>OpenShellGateway: Update inference route
runInferenceSet->>NemoClawRegistry: Update registry
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
cv
left a comment
There was a problem hiding this comment.
Approved on exact head 4303d48. The accepted #6997 path now reads the in-sandbox config before the first route/registry mutation, converts only SandboxConfigError while preserving diagnostics and exit code, and leaves unrelated failures untouched. Security review found no auth, credential, injection, SSRF, policy, dependency, or secret-exposure regression. Local evidence: 127 CLI inference tests + 4 integration preflight tests, CLI typecheck, and check:diff passed. Exact-diff E2E plan 016cb6b768111a8a6a2dd7bb5d775a07c63e8f3892d20b53042fb97d380add27 passed inference-routing and network-policy; advisor attempt 4 and CodeRabbit are clean.
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical dated changelog entry required before the v0.0.85 release plan can be generated. The entry summarizes the user-visible OpenShell, DGX Station, inference, MCP, onboarding, and recovery changes merged since v0.0.84 and links to their owning guides. ## Changes - Add `docs/changelog/2026-07-16.mdx` with the exact `## v0.0.85` heading, parser-safe SPDX comment, release summary, and detailed bullets. - Link every documented theme to its most specific published OpenClaw guide routes. - Reconcile the release entry with these merged source PRs: - #6726 -> `docs/changelog/2026-07-16.mdx`: Document the supported OpenShell v0.0.85 upgrade, immutable consumed artifacts, multiline exec, credential rewrite diagnostics, and child-process TLS boundary. - #6986 -> `docs/changelog/2026-07-16.mdx`: Document managed MCP behavior shared across supported agents. - #6991 and #7045 -> `docs/changelog/2026-07-16.mdx`: Document qualified DGX Station host preparation and the interactive-terminal boundary for `--station-deepseek`. - #6992, #7001, #7006, and #7044 -> `docs/changelog/2026-07-16.mdx`: Document managed-model reasoning behavior, safe inference route mutation, and verified vLLM served aliases. - #6865, #7010, and #7028 -> `docs/changelog/2026-07-16.mdx`: Document onboarding DNS recovery, explicit notice acceptance, and upgrades with user-local OpenShell. - #7005, #7021, #7029, and #7049 -> `docs/changelog/2026-07-16.mdx`: Document rebuild backup safety, no-dashboard state, managed gateway discovery, and Hermes shields topology checks. ## 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 - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the canonical heading, parser-safe SPDX comment, and detailed entry structure; the docs build validates published routes. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] 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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## 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 check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` passed 6/6. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable to this doc-only entry. - [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) — passed with 0 errors and 2 pre-existing Fern warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [x] New doc pages include SPDX header and frontmatter (new pages only) — native changelog entries use the required parser-safe MDX SPDX comment instead of frontmatter. --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for NemoClaw v0.0.85. * Documented improvements to compatibility, credential handling, setup validation, recovery workflows, endpoint configuration, gateway discovery, and runtime validation. * Added links to relevant user-guide sections. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Summary
nemoclaw inference set --no-verify --sandbox <name>against a sandbox whose container is stopped crashed with an uncaughtSandboxConfigError(raw Node stack, exit 1) and left a half-applied switch: the gateway route and registry were already mutated to the new model while the in-sandboxopenclaw.jsonstill pointed at the old one. The in-sandbox config is now read as a pre-flight gate before any mutation, so an unreadable config aborts cleanly and atomically (nothing mutated) with a clear message instead of a stack trace.Related Issue
Closes #6997
Changes
src/lib/actions/inference-set.ts: addreadInSandboxConfigOrFail(), which reads the in-sandbox config and converts aSandboxConfigError(the config could not be read or parsed — most commonly a stopped sandbox) into a cleanInferenceSetError, preserving the original diagnostic lines and exit code. It appends aStart the sandbox and retry.hint only for the stopped-sandbox case (the one that reportsIs the sandbox running?) — not for a corrupt/unparseable config, which starting the sandbox would not fix. Call it before the gateway-route mutation (captureOpenshell inference set) and the registry writes, replacing the former crash-prone read that ran after those mutations. Protected by the tests below.src/lib/actions/inference-set-degraded-state.test.ts: rewrite the read-failure case to exercise the realSandboxConfigErrorpath and assert zero mutation (route-set,updateSandbox,writeSandboxConfig,restartSandboxGatewayall uncalled) — locking the ordering so moving the read back after the mutations fails the test. The prior assertion asserted the buggy half-applied state.test/inference-set-preflight.test.ts: unit tests for the helper — readable path, stopped-sandbox conversion (error type, message, exit code), parse-failure conversion (no start hint), and pass-through of unrelated errors.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Rui Luo ruluo@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests