feat(openshell): upgrade managed runtime to v0.0.101 - #8660
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
📝 WalkthroughWalkthroughOpenShell pins, trust metadata, manifests, compatibility references, and test fixtures were updated from v0.0.99 to v0.0.101 across production workflows, installers, runtime code, documentation, and validation tests. ChangesOpenShell v0.0.101 migration
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-8660.docs.buildwithfern.com/nemoclaw |
|
Exact-head E2E is running for candidate The trusted controller validated the PR head, packaged the exact-commit CLI, and launched the matrix. One early failure is classified as a workflow/candidate version mismatch, not an upgrade regression: trusted workflow Other lanes remain in progress. The PR-triggered rootless Podman check installed the pinned v0.0.101 runtime successfully and is exercising the lifecycle now. No later candidate push has invalidated this run. |
|
E2E follow-up: the full run above was canceled after the trusted-workflow audit confirmed that its main-sourced OpenShell and MCP pins make it unusable as v0.0.101 qualification evidence. Focused replacements are running:
The repository currently has no supported pre-merge focused Hermes MCP dispatch that can use this PR's updated static workflow pins. That controller skew is recorded as an infrastructure limitation, not treated as an OpenShell upgrade blocker; the candidate's Hermes MCP regression suites passed locally and its PR-triggered Hermes managed-startup check is green. |
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
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 `@docs/security/openshell-0.0.72-compatibility-review.mdx`:
- Line 38: Align OpenShell release references across all affected sites: in
docs/security/openshell-0.0.72-compatibility-review.mdx:38, keep the historical
0.0.72 evidence separate and update current compatibility guidance to reference
the reviewed release whose title, evidence, tests, and runtime version match;
update the corresponding OpenShell references in
docs/reference/commands.mdx:4399, docs/reference/troubleshooting.mdx:349, and
docs/security/gateway-authentication-controls.mdx:56 to use the matching review
and release identity while preserving the valid published route.
In `@scripts/install-openshell.sh`:
- Around line 40-49: The stable preflight must handle an existing OpenShell
0.0.99 installation before applying the MIN_VERSION gate. Update the
version-check flow in scripts/install-openshell.sh so 0.0.99 reaches
openshell_has_required_messaging_features and pinned_sandbox_build_version()
rather than the below-minimum upgrade branch, while preserving normal
minimum-version behavior; add a regression test covering an existing 0.0.99
installation.
🪄 Autofix
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: 1be47261-eee9-479e-a98f-e5a43daa61c5
📒 Files selected for processing (74)
.github/workflows/e2e.yaml.github/workflows/podman-cpu-proof.yamlagents/hermes/Dockerfileagents/hermes/mcp-config-transaction.pyagents/hermes/runtime-config-guard.pyagents/hermes/start.shdocs/deployment/set-up-mcp-bridge.mdxdocs/manage-sandboxes/add-mcp-server.mdxdocs/manage-sandboxes/update-sandboxes.mdxdocs/reference/commands.mdxdocs/reference/configure-runtime-identity.mdxdocs/reference/troubleshooting.mdxdocs/security/best-practices.mdxdocs/security/gateway-authentication-controls.mdxdocs/security/openshell-0.0.72-compatibility-review.mdxnemoclaw-blueprint/blueprint.yamlnemoclaw/src/shared/openshell-policy-boundary.ctsscripts/brev-launchable-ci-cpu.shscripts/checks/managed-image-protected-runtime-contract.tsscripts/install-openshell.shscripts/install.shscripts/update-hermes-agent.shsrc/lib/actions/sandbox/mcp-bridge-input-validation.test.tssrc/lib/actions/sandbox/mcp-bridge-url-validation.tssrc/lib/actions/sandbox/mcp-bridge-validation.tssrc/lib/actions/sandbox/openshell-child-visible-credentials.v0.0.101.jsonsrc/lib/onboard/docker-driver-gateway-runtime.test.tssrc/lib/onboard/docker-driver-gateway-runtime.tssrc/lib/onboard/experimental/portable-demo-lifecycle.tssrc/lib/onboard/forward-start.tssrc/lib/onboard/managed-bootstrap/podman-held-workload.test.tssrc/lib/onboard/managed-bootstrap/podman-held-workload.tssrc/lib/onboard/openshell-feature-gate.test.tssrc/lib/onboard/openshell-feature-gate.tssrc/lib/onboard/openshell-install.test.tssrc/lib/onboard/openshell-install.tssrc/lib/onboard/openshell-version.tssrc/lib/onboard/runtime-provider/podman-lifecycle.tssrc/lib/policy/index.tstest/brev-launchable-ci-cpu-checksum.test.tstest/deepagents-mcp-legacy-lifecycle.test.tstest/e2e/live/mcp-bridge-sandbox.tstest/e2e/live/network-policy.test.tstest/e2e/live/openclaw-plugin-runtime-exdev.test.tstest/e2e/live/openshell-gateway-auth-source-contract-helpers.tstest/e2e/live/openshell-gateway-auth-source-contract.test.tstest/e2e/live/openshell-gateway-upgrade.test.tstest/e2e/live/podman-cpu-lifecycle-helpers.tstest/e2e/live/podman-cpu-lifecycle.test.tstest/e2e/support/mcp-bridge-sandbox.test.tstest/e2e/support/podman-cpu-proof-workflow.test.tstest/fixtures/openshell-v0.0.101test/hermes-doctor-config-hash.test.tstest/hermes-final-image-layout.test.tstest/hermes-mcp-config-transaction.test.tstest/hermes-mcp-credential-boundary-manifest.test.tstest/install-openshell-version-check.test.tstest/install-openshell-version-pin.test.tstest/installer-sandbox-build-trust.test.tstest/mcp-add-crash-consistency.test.tstest/mcp-destroy-lifecycle.test.tstest/mcp-openshell-workflow.test.tstest/mcp-policy-key-ownership.test.tstest/mcp-restart-policy-order.test.tstest/openshell-0.0.101-migration-review.test.tstest/openshell-0.0.85-migration-review.test.tstest/openshell-channel-workflow.test.tstest/openshell-e2e-qualification.test.tstest/pr-risk-plan.test.tstest/runner.test.tstest/sandbox-provisioning.test.tstest/sandbox-rlimit-hooks.test.tstest/update-hermes-agent-script.test.tstools/e2e/mcp-workflow-boundary.mts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Exact-head validation refreshed after focused follow-up commit Local follow-up evidence is green: 131/131 workflow-boundary tests, 4/4 installer-pin tests including a full Fresh runs: The prior |
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)
test/install-openshell-version-pin.test.ts (1)
339-339: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAssert the pinned tag for every downloaded component.
At Line [362], the test checks only that
v0.0.101appears somewhere indownloads. It does not reject another tag for the gateway or sandbox. In the0.0.102case, a0.0.99or0.0.100request can pass because Line [366] excludes onlyv0.0.102. The fake tar setup at Line [339] writes0.0.101for every archive, so the final replacement check cannot detect a mixed download set. Assert that every logged asset request usesv0.0.101and verify all three installed components.As per path instructions, review tests for behavioral confidence rather than implementation lock-in. Based on the PR objective, every production consumer must select the same trusted release.
Also applies to: 354-388, 397-421
🤖 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 `@test/install-openshell-version-pin.test.ts` at line 339, Strengthen the version-pin assertions in the test cases around the fake tar setup and download checks so every logged asset request uses v0.0.101, rather than merely checking that the tag appears and v0.0.102 is absent. Verify all three installed components—CLI, gateway, and sandbox—select the same trusted release, while keeping the assertions focused on observable download and installation behavior.Source: Path instructions
🤖 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 `@test/install-openshell-version-pin.test.ts`:
- Line 339: Strengthen the version-pin assertions in the test cases around the
fake tar setup and download checks so every logged asset request uses v0.0.101,
rather than merely checking that the tag appears and v0.0.102 is absent. Verify
all three installed components—CLI, gateway, and sandbox—select the same trusted
release, while keeping the assertions focused on observable download and
installation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a1d140b1-c366-4a71-85ad-0eee7f482a79
📒 Files selected for processing (10)
ci/platform-matrix.jsondocs/reference/platform-support.mdxtest/exit-code-user-error-surfaces.test.tstest/install-openshell-version-pin.test.tstest/onboard-gateway-port-conflict-fast-fail.test.tstest/rebuild-credential-preflight.test.tstest/rebuild-shields-auto-unlock.test.tstest/rebuild-stale-recovery.test.tstest/repro-2201.test.tstools/e2e/cli-artifact-workflow-boundary.mts
## Summary `nemoclaw <sandbox> agent` reported success for a turn that never happened. Both captured transports treated "the child exited 0" as the only success signal, so an exec that returned status `0` with zero bytes on stdout **and** stderr was relayed as a completed turn — no output, no warning, no non-zero exit, and no signal to the caller that the message was never delivered. This PR makes that state fail loud, and fixes two adjacent defects on the same dispatch. It also includes #8857, which rejects a second false-success state: a response whose authoritative run metadata marks the turn incomplete or abandoned. ## Related Issue Refs #8796 Deliberately `Refs`, not `Closes` — see [Scope](#scope). This PR fixes the *reporting* contract, which is NemoClaw-owned and proven. It does not establish why the dispatch was empty. ## Changes **1. Empty-dispatch guard (the reported contract violation).** A delivered OpenClaw turn cannot be byte-empty on both streams: the in-sandbox NemoClaw plugin writes its registration banner to stderr on every invocation (`nemoclaw/src/index.ts:404-419`, already documented in `docs/reference/commands.mdx`). A zero-exit, zero-byte dispatch is therefore reported as a failure with the documented recovery paths instead of a successful turn. - Requires **both** streams empty, so a quiet-but-real turn never misfires. - Excludes non-zero status, signal kills (`status === null`) and transport errors — those already report themselves. - On the JSON path it runs ahead of the stdout write, so machine-readable stdout stays byte-empty and no provenance line is appended for a turn that never ran. **2. Restore the owning-gateway `-g` pin on both transports.** #7113 established the explicit gateway argument as the per-subprocess authority precisely because the process-global active selection can be changed by another CLI at any moment — `gateway-state.ts:540` documents this ("never trust that process-global state ... The explicit gateway argument below is the per-subprocess authority"). #8191 dropped that pin when it moved the non-JSON transport off `execSandbox` onto a raw `spawnSync`; the JSON transport (#5683) predates #7113 and never had it. Restored on both. **3. Stop handing an interactive terminal to a non-interactive dispatch.** #8191 also hard-coded `stdio[0] = "inherit"`, so a live TTY was forwarded into a dispatch whose stdout and stderr are pipes. A TTY is now withheld; a genuine pipe or redirect is still forwarded, so `printf 'ping' | nemoclaw my-assistant agent --agent main` keeps working. **This is fd hygiene, not a delivery fix** — see below. **4. Reject response envelopes that mark the turn incomplete.** A turn can produce a JSON trace and exit `0` even though its authoritative metadata carries `error.kind = incomplete_turn`, `livenessState = abandoned`, or `replayInvalid = true`. The wrapper now preserves stdout byte-for-byte, reports those markers and verify-before-retry guidance on stderr, and exits `1`. It selects only the final matching OpenClaw response envelope — local `{ payloads, meta }` or gateway `{ status, result: { payloads, meta } }` — so earlier JSON progress records, tool results, and tool-call arguments cannot reclassify a completed turn. The classifier and stdio shape live in a new `passthrough-dispatch.ts`; the operator-facing failure text lives beside the existing help copy in `passthrough-help.ts`. ## Scope Symptoms (a) exit 0, (b) no output and (d) `--json` silent are fixed and NemoClaw-owned. The merged #8857 follow-up also fixes the NemoClaw-owned case where an emitted JSON response explicitly marks the turn incomplete. Symptom (c) — the message is never delivered, no session created — is **not** fixed here, and I could not identify its cause. I traced this against the exact OpenShell the reporter ran (tag `v0.0.85`) to test the leading hypothesis, that a live TTY on fd 0 combined with `--no-tty` caused the drop. **That hypothesis is disproven at the source level:** - `crates/openshell-cli/src/run.rs:2942` — a terminal fd 0 yields an *empty* stdin payload without reading; `/dev/null` yields an empty payload after a 0-byte read. Identical value, and no blocking read on a terminal. - `run.rs:2965` — `--no-tty` pins `tty=false` either way. - `run.rs:2968` — the interactive RPC is gated on `tty_override == Some(true)`, so `--no-tty` never reaches it. - `run.rs:2981-2993` — the command vector is sent unconditionally. With `--no-tty`, OpenShell builds a **byte-identical `ExecSandboxRequest`** whether fd 0 is a live terminal or `/dev/null`. So change 3 alters nothing OpenShell can observe. It is still worth doing — a documented non-interactive one-shot should not hand a terminal to a captured dispatch — but it is not why anything would start working, and I have removed the earlier claim that it was. That also disposes of the two upstream TTY/exec fixes landed after `v0.0.85` (`a2cd5f8e`, first in `v0.0.88`; `0d5e5c53`, first in `v0.0.93`): both are on the *interactive* exec path, which this dispatch never takes. **What I did find upstream** is a matching silent-success shape in the OpenShell CLI itself: ```rust // crates/openshell-cli/src/run.rs:2997 let mut exit_code = 0i32; // only overwritten by an Exit event (:3014) ... Ok(exit_code) // :3021 — stream ended, no Exit event -> 0 ``` The server already has the correct anti-default for the analogous case — `exec_loop_result` maps a missing exit status to `Status::unavailable("exec relay closed before the command reported an exit status")` (`crates/openshell-server/src/grpc/sandbox.rs:1507-1516`) — but the CLI does not apply the same rule to its own stream. If that is what the reporter hit, the root cause is upstream and this guard is the correct host-side response until it is fixed. I'm happy to file that OpenShell issue if maintainers agree with the reading. Note also that the reporter ran OpenShell `0.0.85` while current `main` ships `0.0.101` (#8660), so a re-test on current main is worth doing before assuming the delivery symptom is still live. **Residual, out of scope:** `proc.exit()` runs in the same tick as the stdout write on both transports, truncating replies past ~80 KiB when stdout is a pipe. The fix is `process.exitCode` + a normal return, which changes two `: never` signatures. Separate PR. ## Testing - `npx vitest run --project cli src/lib/openclaw/ src/lib/actions/sandbox/agent/ test/openclaw-agent-json.test.ts` — 8 files, 148 tests, all pass - `npx vitest run --project cli src/lib/actions/sandbox/` — 2685/2686 pass. The one failure, `gateway-restart-hermes-drift.test.ts:126`, reproduces on a clean tree without this diff: it shells out to host `python3` and uses `zip(strict=)`, which needs ≥3.10. - `npm run typecheck:cli`, `npm run lint`, `npm run build:cli` — clean - Repo gates: source-architecture budget, test-file-size budget, source-shape test budget, test-title style, layer import boundaries — all pass. The source-architecture budget records the expected fan-in increases for `core/shell-quote.ts` (26 to 27) and `security/redact.ts` (51 to 52); the new module avoids importing `cli/branding`, whose fan-in remains 86. New coverage: the classifier's misfire cases (stdout-only, stderr-only, non-zero, signal-killed, transport error), the stdio shape in both stdin postures, the diagnostic copy, and — on both transports — the empty-dispatch exit, the `-g` argv pin, and the withheld TTY. Completion-marker coverage includes each accepted marker, healthy and non-JSON responses, tool-result and tool-argument false positives, log-prefixed framing, and a marker-bearing JSON progress record followed by a healthy response at both classifier and transport layers. The JSON pin test asserts against the real `buildOpenshellExecArgs`, so it pins the actual argv (`["sandbox","exec","--name","alpha","-g","nemoclaw-8081"]`). Four pre-existing `passthrough-json.test.ts` cases gained injected seams: without them the new defaults would read the developer's real `~/.nemoclaw` registry and the real fd 0. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Agent dispatches that exit successfully without producing output now fail clearly with status `1`. - Added recovery guidance, including direct execution and gateway recovery commands. - Sensitive information is redacted from recovery diagnostics. - Interactive terminal input is no longer forwarded during non-interactive dispatches. - Genuine piped or redirected input continues to be preserved. - Agent commands now target the explicitly resolved owning gateway. - **Documentation** - Updated command reference documentation with silent-dispatch failure behavior, recovery guidance, gateway selection, and input-handling details. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Maintainer Readiness Evidence - Scope increase: this PR has a substantial net line increase, dominated by focused regression tests and command-reference documentation. - Security review: all nine categories pass. Recovery commands retain shell quoting, detected credential values are redacted, and a changed diagnostic command is marked non-replayable. No authorization bypass, dependency, cryptography, or configuration weakening was introduced. - Verification: 148 targeted agent/OpenClaw tests, 24 gateway-health tests, and 30 OpenClaw startup compatibility tests pass; the documentation build and full PR validation pass. The four maintainer commits include DCO sign-off and appear as Verified in GitHub. - CI classification: the earlier installer failure reproduced as passing locally and was transient. The earlier gateway-health failure reproduced locally as an upstream fixture regression and is fixed by including the shared launcher in the extracted fixture. The branch now includes current `main`. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/reference/commands.mdx` documents shell quoting, credential redaction, runnable versus non-replayable recovery guidance, and final-response-envelope selection that ignores earlier JSON progress records. The OpenClaw-only generated variant and published routes were verified. - Agent: Codex Desktop <!-- docs-review-head-sha: 625fec1 --> <!-- docs-review-agents-blob-sha: c4923a3 --> --------- Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: prekshivyas <prekshiv@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Summary
Upgrade NemoClaw's managed OpenShell runtime from v0.0.99 to the reviewed v0.0.101 release identities, including the Hermes MCP compatibility path. Preserve only the v0.0.99 sandbox fallback needed to migrate existing installations, and add focused regression coverage and user documentation for the changed behavior.
Related Issue
Closes #8606
Changes
Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/deployment/set-up-mcp-bridge.mdx,docs/manage-sandboxes/add-mcp-server.mdx,docs/manage-sandboxes/update-sandboxes.mdx,docs/reference/commands.mdx,docs/reference/configure-runtime-identity.mdx,docs/reference/platform-support.mdx,docs/reference/troubleshooting.mdx,docs/security/best-practices.mdx,docs/security/gateway-authentication-controls.mdx, anddocs/security/openshell-0.0.72-compatibility-review.mdxDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailableFS_IOC_GETFLAGSpermission failures outside the changed behavior.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run checks:repository,npm run typecheck:cli, pluginnpm run typecheck, repository project/title/source-shape checks, live installer hash verification, and changed-file Biome checks passed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Release Update
Documentation
Tests