fix(cli): probe live sandbox agent versions#4550
Conversation
Signed-off-by: Revant Patel <revant.h.patel@gmail.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)
📝 WalkthroughWalkthroughAdds a VersionCheckOptions contract, treats empty SSH probe output as failure, conditionally forces live SSH probing for running sandboxes in status and upgrade flows, preserves cached registry agentVersion during metadata updates, broadens readiness parsing, and adds tests and CLI cases validating probe-first behavior. ChangesLive sandbox version probing and drift detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
Signed-off-by: Revant Patel <revant.h.patel@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/status.ts (1)
314-342:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFix
statusagent version probing so it reports live versions when cachedagentVersionis missing.In
src/lib/actions/sandbox/status.ts,shouldProbeSandboxRuntimeVersion(...)returnslookup.state === "present" && Boolean(sandbox.agentVersion), so whensandbox.agentVersionisnullstatus setsskipProbe: trueandcheckAgentVersionreturnssandboxVersion: null. The logging only prints anAgent:line whenversionCheck.sandboxVersionexists, so running sandboxes with missing cached metadata can produce no agent version output.This is inconsistent with
src/lib/actions/upgrade-sandboxes.ts, which forces{ forceProbe: true }for all live sandboxes (liveNames.has(sandboxName)), independent of cachedagentVersion, so rebuild/upgrade can determine the actual live agent version.Align status with upgrade by probing for running sandboxes even when
sandbox.agentVersionisnull(e.g., remove theBoolean(sandbox.agentVersion)gating fromshouldProbeSandboxRuntimeVersion).🤖 Prompt for 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. In `@src/lib/actions/sandbox/status.ts` around lines 314 - 342, The status code currently sets shouldProbeRuntimeVersion = lookup.state === "present" && Boolean(sb.agentVersion), which prevents probing live sandboxes when cached sb.agentVersion is null; change the logic so shouldProbeRuntimeVersion is true for any live sandbox (i.e., remove the Boolean(sb.agentVersion) gating), and pass that into sandboxVersion.checkAgentVersion (forceProbe: shouldProbeRuntimeVersion, skipProbe: !shouldProbeRuntimeVersion) so live sandboxes are probed for their actual runtime agent version even when cached metadata is missing; update references to shouldProbeRuntimeVersion, lookup.state, sb.agentVersion, and the sandboxVersion.checkAgentVersion call accordingly.
🤖 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.
Outside diff comments:
In `@src/lib/actions/sandbox/status.ts`:
- Around line 314-342: The status code currently sets shouldProbeRuntimeVersion
= lookup.state === "present" && Boolean(sb.agentVersion), which prevents probing
live sandboxes when cached sb.agentVersion is null; change the logic so
shouldProbeRuntimeVersion is true for any live sandbox (i.e., remove the
Boolean(sb.agentVersion) gating), and pass that into
sandboxVersion.checkAgentVersion (forceProbe: shouldProbeRuntimeVersion,
skipProbe: !shouldProbeRuntimeVersion) so live sandboxes are probed for their
actual runtime agent version even when cached metadata is missing; update
references to shouldProbeRuntimeVersion, lookup.state, sb.agentVersion, and the
sandboxVersion.checkAgentVersion call accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 05f54ea1-b125-4b1c-90d7-06bdf5cdfa82
📒 Files selected for processing (2)
src/lib/actions/sandbox/status.tstest/cli.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/cli.test.ts
Signed-off-by: Test User <test@example.com>
|
@cv I pushed a fix for the failing |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/lib/runtime-recovery.test.ts`:
- Around line 62-77: The test for parseReadySandboxNames needs to assert that
sandboxes with PHASE "NotReady" are excluded: update the test case inside the it
block that calls parseReadySandboxNames to include an extra line with a sandbox
whose PHASE is "NotReady" (e.g., "zeta ... NotReady") and ensure the expected
result array passed to toEqual does not contain that name; reference
parseReadySandboxNames and the current test that checks for
["alpha","epsilon","delta"] when adding the NotReady row and asserting it is
absent.
In `@src/lib/runtime-recovery.ts`:
- Around line 54-55: The current check uses cols.includes(...) which can match
tokens outside the PHASE column; instead extract the single PHASE token from
cols (e.g., let phase = cols[PHASE_INDEX] or parse the token known to represent
phase) and then evaluate it exactly: const isReadyOrRunning = phase === "Ready"
|| phase === "Running"; if (!isReadyOrRunning || phase === "NotReady") continue;
Update the logic that computes isReadyOrRunning and the subsequent check to use
the single phase variable (phase) rather than cols.includes.
🪄 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: 05d1011a-4a2a-476e-8e38-8f0ec1aac6d9
📒 Files selected for processing (3)
src/lib/runtime-recovery.test.tssrc/lib/runtime-recovery.tstest/cli.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/cli.test.ts
Signed-off-by: Test User <test@example.com>
Signed-off-by: Test User <test@example.com>
2 participants
Summary
NemoClaw now verifies the OpenClaw version actually running inside live sandboxes before reporting status or deciding whether an upgrade is needed. This prevents reused sandboxes from being marked up to date only because cached host metadata matches the current expected version.
Related Issue
Fixes #4429
Changes
statusandupgrade-sandboxes.sshwith an empty config.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Additional checks run:
npm run build:clinpm run typecheck:clinpx vitest run src/lib/onboard/sandbox-registry-metadata.test.ts src/lib/sandbox/version.test.ts src/lib/domain/maintenance/upgrade.test.ts src/lib/actions/gateway-drift-preflight.test.ts src/lib/actions/sandbox/status.test.tsgit diff --check upstream/main...HEADNote: local full hook execution was not completed because the repo's CLI coverage hook recursively invoked itself through a temporary git commit in a test fixture. The branch was pushed with local hooks skipped after the focused checks above passed.
Signed-off-by: Revant Patel revant.h.patel@gmail.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests