fix(inference): pre-flight sandbox read before mutating the inference route - #7004
fix(inference): pre-flight sandbox read before mutating the inference route#7004rluo8 wants to merge 3 commits into
Conversation
… route 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesInference set preflight
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Sequence Diagram(s)sequenceDiagram
participant CLI
participant runInferenceSetWithoutHostLock
participant readSandboxConfig
participant Gateway
participant updateSandbox
CLI->>runInferenceSetWithoutHostLock: set inference configuration
runInferenceSetWithoutHostLock->>readSandboxConfig: read sandbox config
readSandboxConfig-->>runInferenceSetWithoutHostLock: config or translated error
runInferenceSetWithoutHostLock->>Gateway: mutate route after successful preflight
runInferenceSetWithoutHostLock->>updateSandbox: update sandbox state
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/inference-set-preflight.test.ts`:
- Line 16: Update the describe block title for readInSandboxConfigOrFail so the
issue reference appears as the required final “(`#6997`)” suffix, after the test
description.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e991ba3e-ab43-45a4-86bc-85b094c96997
📒 Files selected for processing (3)
src/lib/actions/inference-set-degraded-state.test.tssrc/lib/actions/inference-set.tstest/inference-set-preflight.test.ts
Signed-off-by: Rui Luo <ruluo@nvidia.com>
2 participants
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