fix(sandbox): scope the agent version probe to the sandbox's gateway - #7498
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe OpenShell adapter now scopes sandbox SSH lookups to a gateway. Agent version checks resolve persisted gateway bindings, pass valid names to probes, and return an unavailable result without probing when bindings are invalid. ChangesGateway-scoped sandbox probing
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant checkAgentVersion
participant Registry
participant captureSandboxSshConfigCommand
participant OpenShellCLI
checkAgentVersion->>Registry: Resolve persisted sandbox gateway
Registry-->>checkAgentVersion: Return gateway name or lookup failure
checkAgentVersion->>captureSandboxSshConfigCommand: Start scoped SSH probe
captureSandboxSshConfigCommand->>OpenShellCLI: Run sandbox get and sandbox ssh-config with gateway
OpenShellCLI-->>captureSandboxSshConfigCommand: Return SSH configuration
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — InformationalAdvisor assessment: Informational / medium 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: None 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
…bient gateway A rejected persisted binding fell back to an unscoped probe, which is the exact case resolveSandboxGatewayName fails closed on: OpenShell would resolve the sandbox name against its mutable current selection, so a same-named sandbox on another gateway could be probed and its version cached onto the corrupted row. An entry with no binding fields resolves to the canonical default gateway without throwing, so the rejected case is always genuine corruption. Skip the probe entirely there and report the version as unverifiable. Addresses PR review advisor finding PRA-1 on NVIDIA#7498. Signed-off-by: harjoth <harjoth.khara@gmail.com>
`openshell sandbox get` and `sandbox ssh-config` fall back to OpenShell's mutable current selection when no gateway is given. probeAgentVersion issued both unscoped, so a sandbox bound to a sibling gateway was reported missing and its version rendered as `v?` in `upgrade-sandboxes --check` — even though the gateway-scoped sandbox listing had just observed it as live. Resolve the sandbox's recorded gateway with resolveSandboxGatewayName and pass it to both hops, using the same `-g` placement as gatewayScopedArgs in actions/sandbox/gateway-state.ts. A rejected binding skips the probe entirely and reports the version as unverifiable: falling back to the ambient gateway is the case that resolver fails closed on, because a same-named sandbox on another gateway would be probed and its version cached onto the corrupted row. This fixes only the version-resolution half of NVIDIA#7429. The policy-remove idempotency failure and the orphan-detection warning are separate code paths and remain open, so this does not close the issue. Refs: NVIDIA#7429 Signed-off-by: harjoth <harjoth.khara@gmail.com>
3bf9ce7 to
74e4e81
Compare
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 `@src/lib/sandbox/version.ts`:
- Around line 101-123: Update probeAgentVersion and the public checkAgentVersion
flow so a null result from resolveProbeGatewayName is represented as a dedicated
invalid-binding outcome rather than detectionMethod "unknown" with
unavailableReason "probe-failed". Preserve "unknown"/"probe-failed" only when an
actual probe is attempted and fails, and cover the distinct invalid-binding
result through checkAgentVersion.
🪄 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: d0ef6b8d-fc6f-4793-8f9f-933367c7edac
📒 Files selected for processing (4)
src/lib/adapters/openshell/client.test.tssrc/lib/adapters/openshell/client.tssrc/lib/sandbox/version.test.tssrc/lib/sandbox/version.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/adapters/openshell/client.test.ts
- src/lib/adapters/openshell/client.ts
- src/lib/sandbox/version.test.ts
A corrupted binding skips the probe entirely, but the result still reported detectionMethod "unknown" / "probe-failed", which the VersionCheckResult contract reserves for a probe that was attempted and could not inspect the runtime. Report "unavailable" with a new "invalid-gateway-binding" reason so an operator can tell a corrupted registry row from an unreachable sandbox. checkAgentVersion resolves the binding once and passes it to probeAgentVersion instead of both re-reading the registry. Both states already bucket into the "Unknown version" report, so the operator-facing output is unchanged. Refs: NVIDIA#7429 Signed-off-by: harjoth <harjoth.khara@gmail.com>
apurvvkumaria
left a comment
There was a problem hiding this comment.
Security review: PASS on exact head d8f0aac.
Reviewed all four changed files across the repository's nine security categories. Gateway names are passed as structured argv, persisted bindings are validated by the existing resolver, invalid bindings fail closed without an ambient-gateway probe or cache write, and the diff adds no credential, policy, authorization, dependency, or cryptographic surface.
Verification: targeted CLI tests 50/50 passed; CLI and plugin builds passed; CLI typecheck passed; both commits are DCO-signed and GitHub Verified; the exact head composes cleanly with current main; documentation writer result is no-docs-needed. No blocking findings. Required CI remains mandatory and is not waived.
|
/ok to test d8f0aac |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Fix-forward pushed in signed, GitHub-verified commit Local exact-head evidence:
All fork workflows awaiting approval for this exact head have been approved and are restarting. No release tag change. |
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `docs/changelog/2026-07-25.mdx` release entry with the exact `## v0.0.96` heading. The entry reconciles all 90 first-parent commits since v0.0.95 with all 92 merged PRs in the live `v0.0.96` label ledger and groups the user-visible changes by operator journey. ## Changes - Add the parser-safe dated MDX changelog entry for v0.0.96 with root-absolute links to the focused user guides. - Source summary: - [#7194](#7194) -> `docs/changelog/2026-07-25.mdx`: Document persistent baseline network policy exclusions and their inspection, rebuild, and snapshot behavior. - [#7188](#7188), [#7427](#7427), and [#7546](#7546) -> `docs/changelog/2026-07-25.mdx`: Document DNS-backed HTTPS inference routing, keyless loopback endpoints, and provider-marker isolation. - [#7238](#7238) -> `docs/changelog/2026-07-25.mdx`: Document blueprint sandbox and provider identifier validation before state writes or OpenShell calls, with bounded terminal-safe rejection previews. - [#7319](#7319), [#7274](#7274), [#7528](#7528), [#7353](#7353), and [#7560](#7560) -> `docs/changelog/2026-07-25.mdx`: Document the managed default gateway service, onboarding readiness, and container-runtime identity safeguards. - [#7349](#7349), [#7498](#7498), [#7406](#7406), [#7196](#7196), [#7559](#7559), [#7421](#7421), [#7510](#7510), [#7295](#7295), and [#7565](#7565) -> `docs/changelog/2026-07-25.mdx`: Document gateway-scoped status, lifecycle diagnostics, managed MCP recovery, delete-edge safeguards, and fail-closed CLI prompt and command output. - [#7591](#7591) -> `docs/changelog/2026-07-25.mdx`: Document opt-in authenticated MCP tool-name discovery, its bounded and names-only contract, probe interaction, and rebuild requirement. - [#7305](#7305), [#7480](#7480), [#7471](#7471), [#7365](#7365), and [#7541](#7541) -> `docs/changelog/2026-07-25.mdx`: Document installer version checks, version-tag reporting, license guidance, WSL Ollama selection, and DGX Station vLLM detection. - [#7482](#7482), [#7466](#7466), [#7208](#7208), [#7434](#7434), and [#7586](#7586) -> `docs/changelog/2026-07-25.mdx`: Document Ollama resource details, reasoning precedence, Hermes onboarding behavior, and preserved managed Hermes BuildKit failures. - [#6830](#6830), [#7492](#7492), [#7563](#7563), and [#7582](#7582) -> `docs/changelog/2026-07-25.mdx`: Document the authoritative OpenClaw production lock, fixed managed-image dependencies, immutable Hermes base adoption, and Hermes image-size reduction. - [#7505](#7505), [#7530](#7530), [#7547](#7547), [#7508](#7508), [#7548](#7548), [#7549](#7549), [#7537](#7537), [#7534](#7534), [#7515](#7515), [#7511](#7511), [#7551](#7551), [#7562](#7562), [#7575](#7575), [#7496](#7496), [#7594](#7594), [#7595](#7595), and [#7599](#7599) -> `docs/changelog/2026-07-25.mdx`: Summarize release validation, transient and bounded dispatch reconciliation, exact pre-tag qualification, identity revalidation, npm-audit retry, sharding, image reuse, timeout, telemetry, and workflow-hardening changes. - Reconciled without separate changelog prose: - [#7539](#7539), [#7526](#7526), [#7507](#7507), [#7506](#7506), [#7519](#7519), [#7516](#7516), [#7396](#7396), [#7254](#7254), [#7583](#7583), [#7596](#7596), and [#7598](#7598): Test-harness or fixture-only changes. - [#7403](#7403), [#7161](#7161), [#6877](#6877), [#7531](#7531), [#7525](#7525), [#7522](#7522), [#7536](#7536), [#7552](#7552), [#7566](#7566), [#7553](#7553), [#7561](#7561), [#7577](#7577), [#7569](#7569), [#7585](#7585), [#7584](#7584), [#7592](#7592), [#7580](#7580), [#7571](#7571), [#7517](#7517), [#7589](#7589), [#7402](#7402), [#7558](#7558), [#7544](#7544), and [#7601](#7601): Dependency, internal recovery, validation, contributor-workflow, E2E optimization, telemetry, or CI trust changes with no separate user-facing release claim. - [#7556](#7556), [#7573](#7573), [#7576](#7576), and [#7578](#7578): Experimental repository-maintainer conflict automation with no canonical user documentation surface. ## 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 dated changelog structure, version headings, and published links. - [ ] 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: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Reviewed `docs/changelog/2026-07-25.mdx` at exact head `0f5dedb47` against 90 first-parent release commits and 92 merged PRs labeled `v0.0.96`. Verified parser-safe MDX SPDX, the exact version heading, literal CLI names, writing style, skip terms, all 20 root-absolute published links, and the accepted #7591 opt-in authenticated discovery bounds. #7544, #7599, and #7601 remain internal or CI-only release-ledger entries. Changelog tests passed 6/6, the docs build passed with 0 errors and two pre-existing Fern warnings, and `npm run check:diff` plus the final diff check passed. - Agent: Codex Desktop documentation-writer subagent <!-- docs-review-head-sha: 0f5dedb --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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`: 6/6 passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this prose-only changelog 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) — the build passed with 0 errors and 2 existing Fern warnings; the published-route check passed. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — native changelog files use the required parser-safe MDX SPDX comment and no frontmatter. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Persistent network policy exclusions with consistent restore/exclusion reporting across rebuilds/snapshots. * Opt-in MCP tool discovery via `mcp status --tools` with bounded, redacted authenticated traffic. * Improved HTTPS inference switching for custom endpoints and refreshed onboarding/model menu details. * Refined OpenShell gateway defaults for port `8080`, including more reliable readiness checks. * **Bug Fixes** * Prevent incorrect provider/model restoration after compatible-provider update failures. * Preserve managed MCP state after exec loss and tighten gateway/doctor status scoping. * **Tests** * Stronger, fail-closed release validation with hardened evidence/artifact handoff and bounded timeouts/retries. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary For `recover` and `gateway restart`, the managed gateway controller now acquires the expected-exit lock before it inspects the supervisor or gateway. Lock acquisition, gateway termination, and replacement health share one recovery deadline. If lock acquisition reaches that deadline, the controller returns `SUPERVISOR_BUSY` without publishing an expected-exit marker. This serializes consecutive Hermes `policy-remove` calls that reach the controller while gateway recovery is active. ## Related Issue Refs #7429 PR #7498 addressed the separate Hermes version probe for `upgrade-sandboxes --check`. This PR addresses the remaining consecutive `policy-remove` behavior and does not close the issue. ## Changes - Acquire the expected-exit lock before supervisor and gateway inspection, then inspect the gateway again after contention ends. - Use one recovery deadline for lock acquisition, preflight, marker publication, gateway termination, and replacement health. - Remove the marker when publication reaches the deadline. - Re-prove gateway identity before pidfd signaling, and send no signal after the deadline. - Apply the remaining deadline to loopback HTTP connection, request, status, header, and body reads. - Preserve root-owned lock checks, process identity checks, inode-safe cleanup, and marker authorization. - Add regression coverage for contention, publication cleanup, identity recapture, signal deadlines, and slow HTTP responses. - Update the OpenClaw restart-respawn harness for the shared recovery deadline. - Document the serialized gateway lifecycle and deadline behavior. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Current-revision security revalidation passed with no findings: #8262 (comment) - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: No waiver. Required GitHub Actions checks must pass. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Independent documentation review on the current revision verified the shared recovery deadline, lock/termination/replacement semantics, `SUPERVISOR_BUSY` behavior, marker cleanup, and existing retry guidance with no findings. The follow-up commit changes only test-harness isolation and clock wiring. The docs build passed with 0 errors and 2 existing warnings. - Agent: Codex Desktop <!-- docs-review-head-sha: d957bcb --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; `scripts/prepare-dgx-station-host.sh` is unchanged. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub — all 32 commits appear as `Verified`. - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable — normal commit and push hooks passed for the current follow-up. - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 4 focused files and 10 tests pass. The CLI build and type check, Python compilation, documentation build, commit hooks, and push hooks also pass. - [ ] Applicable broad gate passed — fresh required GitHub Actions checks are running for the current branch revision. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed — the independent security review passed all nine categories. - [ ] `npm run docs` builds without warnings (doc changes only) — no local docs build ran; GitHub Actions is authoritative. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) — the independent documentation writer review passed. - [ ] New doc pages include SPDX header and frontmatter (new pages only) — not applicable; no new page. ## Sensitive-Path Security Review Independent review found no security finding for PR commit `8b52f32ac37b4dc3e2832d8948c206ce9d0db1c1` against base SHA `db31c286129e878c3356eed49f76ab259561e47e`. 1. Secrets and credentials: PASS. Deadline propagation does not widen credential access, and Hermes validation remains fail closed. 2. Input validation and data sanitization: PASS. Request actions and nonces remain validated, and loopback health probes use bounded parsing. 3. Authentication and authorization: PASS. Root-owned lock validation precedes process inspection, and marker publication remains bound to gateway and controller identities. 4. Dependencies and third-party libraries: PASS. No dependency or artifact changes. 5. Error handling and logging: PASS. Failed marker publication removes only the matching inode, and lifecycle cleanup runs from the outer `finally` block. 6. Cryptography and data protection: PASS. No cryptographic mechanism or protected-data flow changes. 7. Configuration and security headers: PASS. Runtime files retain owner, type, link-count, and mode checks. 8. Security testing: PASS. Regression sources cover contention, deadline expiry, identity refresh, inode-safe cleanup, signal admission, and slow HTTP response phases. 9. System security: PASS. Recovery holds the lock from discovery through cleanup, re-proves the pidfd target before signaling, and applies one deadline to all recovery phases. The documentation records the existing shared-UID provenance and mutable-configuration TOCTOU limitations. Required GitHub Actions checks remain separate merge requirements. ## GitHub Actions GitHub Actions is authoritative for PR commit `8b52f32ac37b4dc3e2832d8948c206ce9d0db1c1` against base SHA `db31c286129e878c3356eed49f76ab259561e47e`. Required checks are running. No check has a maintainer waiver. --- Signed-off-by: harjoth <harjoth.khara@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved managed gateway lifecycle handling during concurrent expected-exit requests. * Added consistent recovery time limits across expected-exit locking, gateway termination, and replacement health checks. * Prevented stale or conflicting exit markers when the recovery deadline expires. * Improved gateway identity refresh and safe restart behavior. * Requests now correctly return “supervisor busy” when recovery cannot complete in time. * **Documentation** * Clarified expected lifecycle, locking, and timeout behavior during recovery. * **Tests** * Added integration coverage for lifecycle locking, deadline enforcement, and marker safety. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: harjoth <harjoth.khara@gmail.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com>
Summary
upgrade-sandboxes --checkreported a live Hermes sandbox asUnknown version: <name> v? → v0.18.0. The sandbox listing is scoped to the sandbox's recorded gateway, but the version probe was not:openshell sandbox getandsandbox ssh-configfall back to OpenShell's mutable current selection when no gateway is given, so a sandbox bound to a sibling gateway was looked up on the wrong one, came back "not found", and its version could not be resolved. The probe now targets the sandbox's own recorded gateway, so the check reports the real version.Related Issue
Refs: #7429
Deliberately not
Closes:. This fixes only the version-resolution half of the issue. Thepolicy-removeidempotency failure (issue 1) and the orphan-detection warning are separate code paths and remain open, so auto-closing #7429 on merge would hide unfixed work.Changes
src/lib/adapters/openshell/client.ts—captureSandboxSshConfigCommandaccepts an optionalgatewayNameand scopes both thesandbox getandsandbox ssh-configinvocations to it. The-gflag is placed after the subcommand pair, matching the existinggatewayScopedArgshelper inactions/sandbox/gateway-state.ts(duplicated rather than imported — an adapter must not depend on the actions layer). OmittinggatewayNamepreserves today's ambient-selection behavior, so the two existing callers are unchanged.src/lib/sandbox/version.ts—probeAgentVersionresolves the sandbox's recorded gateway with the existingresolveSandboxGatewayNameand passes it. When that resolver rejects a persisted binding, the probe is skipped entirely and the version is reported unverifiable — it does not fall back to an unscoped query. An entry with no binding fields resolves to the canonical default gateway without throwing, so the rejected case is always genuine corruption, and probing it unscoped would let a same-named sandbox on another gateway be probed and its version cached onto the corrupted row.No new abstraction, configuration, fallback, or compatibility path.
gatewayNameis an optional argument on one existing function, consumed by one caller, covered by the tests below.Behavior note for review: a registry row that pre-dates the per-port migration resolves to the canonical
nemoclawand is now pinned explicitly instead of inheriting OpenShell's current selection. That is the intended correction —resolveSandboxGatewayNameis already the designated authority for sandbox-scoped commands — but it does mean such a row now probesnemoclawrather than whatever gateway happened to be selected.Type of Change
Quality Gates
docs/manage-sandboxes/update-sandboxes.mdxand the changelog describe thatupgrade-sandboxesprobes the live agent version, which is still accurate. No page documents which gateway the probe targets.a826664db8c202fad81d215d1702a0188247e00b; the change scopes two read-only OpenShell lookups and fails closed on invalid bindings without granting new capability.Documentation Writer Review
no-docs-neededVerification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passednpx vitest run --project cli src/lib/adapters/openshell/client.test.ts src/lib/sandbox/version.test.ts→ 50 passed; fix-forward affected integration tests → 18 passed individually; gateway-recovery regression file → 14 passedTests
Four tests, each red against
origin/mainsources and green here (verified by restoring both source files toorigin/mainand rerunning the new tests: 5 failed → 50 passed):client.test.ts— "scopes both sandbox lookups to the requested gateway"-g <name>reaches the actual argv of both hopsversion.test.ts— "probes the sandbox's own recorded gateway…"nemoclaw-18080is probed against that gatewayversion.test.ts— "scopes a Hermes sandbox on a non-default gateway…"nemoclaw-18080resolves instead ofv?version.test.ts— "does not probe at all when the persisted gateway binding is corrupted"unavailable/invalid-gateway-bindingReview follow-ups applied
PRA-1— the first revision fell back to an unscoped probe when the binding was rejected, reintroducing the same defect for corrupted rows. Now the probe is skipped outright. Advisor isinfo_only/ 0 findings on the current head.unknown/probe-failed, which theVersionCheckResultcontract reserves for a probe that ran and failed. Now reportsunavailable/invalid-gateway-binding. Both states already bucket into the "Unknown version" report, so operator-facing output is unchanged.-gplacement with the existinggatewayScopedArgsconvention, added the Hermes + non-default-gateway test, and changedCloses:toRefs:since only half the issue is fixed.Real-behavior proof
Harness drives the real path —
checkAgentVersion→probeAgentVersion→captureSandboxSshConfigCommand→ realspawnSync— with only the two external binaries stubbed at the process boundary (openshellviaNEMOCLAW_OPENSHELL_BIN,sshviaPATH). The stub models the reported topology: sandboxhermes-sbexists only onnemoclaw-18080, while ambient selection is the defaultnemoclaw.BEFORE (
origin/main):AFTER (this branch):
The BEFORE output reproduces the reported symptom verbatim, including
v? → v0.18.0.Blast radius
npx vitest run --project cli src/lib/adapters src/lib/sandbox src/lib/state→ 936 passed, 16 failed. All 16 failures are insrc/lib/state/state-file-key-merge-behavior.test.tsandstate-file-key-merge-file-safety.test.ts, and reproduce identically on pristineorigin/mainwith this change removed — pre-existing, unrelated to this diff.npm run typecheck:cli→ 0 errors afternpm run build:cli.Honest limitations
--checktarget the recorded gateway — landed in v0.0.94, and the v0.0.90…v0.0.91 range contains no change to this path. WithLogs: Not capturedI could not bisect it. This is submitted as a defect present on currentmain, not as a bisected regression.policy-removeidempotency failure needs a live Hermes sandbox to reproduce, which I could not do.1 recorded sandbox(es) were not found on their recorded gateway: hermesrenders the sandbox name list, not a gateway name, and [NemoClaw][onboard][All Platforms] nemoclaw uninstall + reinstall flow leaves an orphaned sandbox that the reinstalled CLI can't recover or clean up #6520 already excludes orphans from the unknown-version bucket — so that line concerns a different sandbox (the clone) and is not addressed here. A maintainer should not read this diff as closing that symptom.getSshConfig(src/lib/state/sandbox.ts, feeding share-command, plugin restore, backup/restore, and the user-managed-files probe) andcaptureSandboxSshConfig(src/lib/adapters/openshell/runtime.ts, feeding share, skill install/remove, and process recovery). They have the same latent gateway-scoping gap, but I have no repro or proof for those paths and did not want to widen the blast radius. ThegatewayNameoption added here is what they would use. Happy to fold them in, or file a tracking issue, if you'd prefer.resolveProbeGatewayNamecatches broadly. A genuine registry I/O or permission error is currently reported asinvalid-gateway-binding, same as a corrupted binding. Separating those needs a typed error from the registry layer; called out rather than expanded into this diff.openshell/sshbinaries, not live DGX Spark or a real Hermes sandbox. The production code path and argv construction are real; the two external binaries are not.This PR was assisted by AI but human reviewed along the way.
Signed-off-by: harjoth harjoth.khara@gmail.com
Summary by CodeRabbit
sandbox getandsandbox ssh-configlookups to the sandbox’s configured gateway.invalid-gateway-binding) instead of attempting an SSH probe.