fix(cli): route sessions delete to the native Hermes command - #7682
Conversation
sessions delete always refused on a Hermes sandbox: deleteSandboxSession validated an OpenClaw canonical session key and called the OpenClaw gateway admin RPC, which #7588 refuses for non-OpenClaw agents. Its sibling verbs already dispatch by agent kind — sessions list through the passthrough and sessions export through its own Hermes branch (#5526) — leaving delete as the one actionable verb with no route, even though Hermes ships a native `hermes sessions delete <id> --yes` over its own session store. Route Hermes delete to that native command, mirroring sessions export. A Hermes branch at the top of deleteSandboxSession, before OpenClaw key validation, runs the native delete in-sandbox and takes a native Hermes session id as-is. The OpenClaw-only --agent (other than the hermes no-op alias), --keep-transcript, --json, and --verbose flags are refused rather than silently ignored. The native id is refused when it could be parsed as a flag. reset stays refused: Hermes has no native reset, and the refusal hint now also lists the available delete. Closes #7642 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
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:
📝 WalkthroughWalkthroughHermes sandbox session deletion now routes to ChangesHermes session deletion
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CLI
participant deleteSandboxSession
participant HermesSandbox
CLI->>deleteSandboxSession: sessions delete <id>
deleteSandboxSession->>HermesSandbox: hermes sessions delete <id> --yes
HermesSandbox-->>deleteSandboxSession: native command completes
deleteSandboxSession-->>CLI: exit with command status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-7682.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 138a5f6 in the TypeScript / code-coverage/cliThe overall coverage in commit 138a5f6 in the Show a code coverage summary of the most impacted files.
Updated |
![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 `@src/lib/actions/sandbox/sessions/delete.ts`:
- Around line 154-165: The deleteHermesSession branch must reflect that
execSandbox terminates the process and never returns in production. Change
deleteHermesSession to use a terminal Promise<never> contract, or replace
execSandbox with a non-exiting helper that produces a real deletion result;
update the corresponding test in src/lib/actions/sandbox/sessions/delete.test.ts
at lines 188-204 to match the chosen behavior.
🪄 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: 5698eb9a-e8cb-4aa3-b583-05d4ecbfcef0
📒 Files selected for processing (6)
docs/reference/commands.mdxsrc/commands/sandbox/sessions/delete.tssrc/lib/actions/sandbox/sessions/delete.test.tssrc/lib/actions/sandbox/sessions/delete.tssrc/lib/actions/sandbox/sessions/gateway-rpc.tstest/sandbox-sessions-admin-agent-cli.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
1 warning · 0 suggestionsWarningsWarnings do not block.
|
execSandbox ends by calling process.exit with the native command's exit
code, so deleteHermesSession never returned in production; the previous
`return { key, removedTranscript: false }` was reachable only under the
test's resolving execSandbox mock. Type the branch as Promise<never> and
end it after execSandbox, and model execSandbox's process-exit in the
tests so the routing assertions no longer depend on an unreachable
return value.
Refs #7642
Signed-off-by: Dongni Yang <dongniy@nvidia.com>
validateHermesSessionId rejects empty, leading-dash, and whitespace-containing ids, but only the leading-dash branch was exercised. Parameterize the invalid-id test over a leading dash, an empty string, whitespace-only, and embedded whitespace so a future change cannot weaken the empty or whitespace branch without a regression failure. Refs #7642 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
Addressed advisor warning PRA-1 in 17fe863: the invalid-id test is now parameterized over a leading dash, an empty string, whitespace-only, and embedded whitespace — each asserts a non-zero exit and no Signed-off-by: Dongni Yang dongniy@nvidia.com |
rejectOpenClawOnlyDeleteOptions refuses --json or --verbose on a Hermes sandbox, but only the --json case was exercised. Parameterize the rejection test over both flags so a regression cannot let --verbose reach the native command. Refs #7642 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
Addressed advisor warning PRA-1 (--verbose coverage) in 7909d25: the OpenClaw-only output-flag rejection test is now parameterized over both Signed-off-by: Dongni Yang dongniy@nvidia.com |
The Hermes delete integration test covered only a successful native delete. Add a stub-OpenShell case where `hermes sessions delete` exits nonzero and assert the public command returns that exit code and makes no gateway RPC, pinning execSandbox's exit-code propagation as the terminal contract for the Hermes branch. Refs #7642 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
Addressed advisor warning PRA-1 (native delete failure path) in 2873383: the stub OpenShell now takes a native-delete exit code, and a new integration test drives Signed-off-by: Dongni Yang dongniy@nvidia.com |
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/sandbox-sessions-admin-agent-cli.test.ts`:
- Around line 106-108: Update the native-failure test assertions around
result.code, result.out, and gatewayRpcCalls(openshellLog) to also verify that
result.out does not contain “OPENCLAW_GATEWAY_TOKEN”. Keep the assertion at the
public CLI output boundary and preserve the existing exit-code, refusal-text,
and RPC-log checks.
🪄 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: 7a01d15e-774c-44ea-808d-f6ee84079d62
📒 Files selected for processing (1)
test/sandbox-sessions-admin-agent-cli.test.ts
The native-failure test checked the exit code, refusal text, and RPC log but not the public output, so a regression that printed OPENCLAW_GATEWAY_TOKEN on the Hermes failure path could pass unnoticed. Assert its absence, matching the success test. Refs #7642 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Exact-head security review
Reviewed the complete six-file diff and the native Hermes execution boundary:
Exact-head evidence: No human, hardware, or external sensitive-path review is claimed by this comment. |
Automated-review dispositionExact pair: head
All review threads are resolved. CodeRabbit's automatic review is currently paused after the branch's commit sequence; no reviewer-request write was made. Repository-owned exact-head automated review and CI results will be monitored before merge. No human approval is claimed by this disposition. |
Secondary advisor warning dispositionExact pair: head The repository-published primary assessment is
The second opinion's generic holistic-security warning is satisfied by the exact-head nine-category security review at #7682 (comment). Published advisor result: #7682 (comment). Workflow artifacts: https://github.com/NVIDIA/NemoClaw/actions/runs/30379337682. No human approval, product-scope approval, or hardware review is claimed by this disposition. |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Exact-head security review
Reviewed the complete six-file diff and the native Hermes execution boundary:
Exact-head evidence: No human, hardware, or external sensitive-path review is claimed by this comment. |
Automated-review dispositionExact pair: head
All review threads are resolved. CodeRabbit's automatic review is currently paused after the branch's commit sequence; no reviewer-request write was made. Repository-owned exact-head automated review and CI results will be monitored before merge. No human approval is claimed by this disposition. |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Exact-head security review
Reviewed the complete seven-file diff, including the test-only workflow subprocess timeout adjustments, and the native Hermes execution boundary:
Exact-head evidence: No human, hardware, or external sensitive-path review is claimed by this comment. |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Exact-head security review
Reviewed the complete seven-file diff, including the test-only workflow subprocess timeout adjustments, and the native Hermes execution boundary:
Exact-head evidence: No human, hardware, or external sensitive-path review is claimed by this comment. |
Exact-head automated-review disposition
Exact-head validation passed: CLI session tests 154/154, public integration tests 5/5, affected E2E-support tests 33/33, build, both typechecks, repository checks, docs, and all diff-aware hook lanes. No other actionable automated-review finding is currently known. Fresh advisors for this head remain required before merge. |
wscurran
added
area: cli
|
Maintainer product-scope gate: the PR itself records that approval is pending, and I found no later comment or linked decision accepting this maintained Hermes |
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Reviewed exact head 3042261. Agent routing occurs before OpenClaw key canonicalization, Hermes-only flags are rejected instead of ignored, native session ids are passed as discrete argv with leading-dash and whitespace validation, and the native exit status is propagated without touching OpenClaw credentials or RPCs. Focused unit and CLI tests cover success, rejection, and failure paths; current CI is green. I found no blocking correctness, security, compatibility, or regression issue.
|
Separate exact-head CI note for |
<!-- markdownlint-disable MD041 --> ## Summary Prepares the canonical v0.0.102 release documentation from the current release-labeled scope. The change adds a dated changelog for all 38 user-facing shipping PRs and corrects the OpenClaw agent command reference for the behavior delivered by #8191. ## Changes - Add `docs/changelog/2026-08-04.mdx` with the v0.0.102 release summary, detailed behavior changes, support boundaries, security evidence links, and links to durable documentation. - Update `docs/reference/commands.mdx` to describe non-JSON OpenClaw output capture, its combined limit, marker handling, stream suppression, recovery guidance, and exit behavior. - [#8167](#8167) -> `docs/changelog/2026-08-04.mdx`: Records authenticated attachment of operator-managed llama.cpp servers. - [#8129](#8129) -> `docs/changelog/2026-08-04.mdx`: Records the Experimental managed vLLM profile for two DGX Spark systems. - [#7983](#7983) -> `docs/changelog/2026-08-04.mdx`: Records qualification of the May 2026 GB300WS factory image. - [#8207](#8207) -> `docs/changelog/2026-08-04.mdx`: Records the qualified DGX Station driver transaction. - [#8208](#8208) -> `docs/changelog/2026-08-04.mdx`: Records mode-bound Express resume state. - [#8158](#8158) -> `docs/changelog/2026-08-04.mdx`: Records recovery of host-global dual-Station runtime ownership. - [#8145](#8145) -> `docs/changelog/2026-08-04.mdx`: Records Windows-host Ollama validation from Docker Desktop's network context. - [#8190](#8190) -> `docs/changelog/2026-08-04.mdx`: Records HTTP model pulls when WSL has no local Ollama executable. - [#8195](#8195) -> `docs/changelog/2026-08-04.mdx`: Records reuse of a healthy installer-managed CLI. - [#8053](#8053) -> `docs/changelog/2026-08-04.mdx`: Records early rejection of incompatible OpenShell gateway versions. - [#8098](#8098) -> `docs/changelog/2026-08-04.mdx`: Records the bounded package-service-to-standalone gateway recovery transition. - [#8216](#8216) -> `docs/changelog/2026-08-04.mdx`: Records the final dashboard port selected during multi-sandbox onboarding. - [#8146](#8146) -> `docs/changelog/2026-08-04.mdx`: Records managed startup-state restoration for stopped sandboxes. - [#8092](#8092) -> `docs/changelog/2026-08-04.mdx`: Records gateway watchdog recovery for classified not-serving states. - [#8182](#8182) -> `docs/changelog/2026-08-04.mdx`: Records consistent managed-recovery wait configuration. - [#8040](#8040) -> `docs/changelog/2026-08-04.mdx`: Records Docker sandbox rollback authority through late validation. - [#8130](#8130) -> `docs/changelog/2026-08-04.mdx`: Records bounded Shields deadline recovery and durable containment. - [#8086](#8086) -> `docs/changelog/2026-08-04.mdx`: Records repair of narrowly validated permission-only configuration drift. - [#8122](#8122) -> `docs/changelog/2026-08-04.mdx`: Records prompt failure and guidance for corrupt transition locks. - [#8124](#8124) -> `docs/changelog/2026-08-04.mdx`: Records policy restoration flags, previews, and target revalidation. - [#7886](#7886) -> `docs/changelog/2026-08-04.mdx`: Records explicit destruction after pre-delete Shields hardening failures while preserving recovery authority. - [#7901](#7901) -> `docs/changelog/2026-08-04.mdx`: Records multi-port uninstall behavior and shared-resource preservation. - [#7984](#7984) -> `docs/changelog/2026-08-04.mdx`: Records one classified transient remote MCP startup retry. - [#7954](#7954) -> `docs/changelog/2026-08-04.mdx`: Records bounded hosted-inference probe replies. - [#7574](#7574) -> `docs/changelog/2026-08-04.mdx`: Records preservation of validated reasoning capabilities through onboarding. - [#8089](#8089) -> `docs/changelog/2026-08-04.mdx`: Records proxy routing for Hermes WhatsApp pairing and media traffic. - [#7682](#7682) -> `docs/changelog/2026-08-04.mdx`: Records native Hermes session deletion and identifier validation. - [#8150](#8150) -> `docs/changelog/2026-08-04.mdx`: Records corporate CA trust for LangChain Deep Agents Code image builds. - [#8156](#8156) -> `docs/changelog/2026-08-04.mdx`: Records reviewed managed runtime dependency remediation. - [#8180](#8180) -> `docs/changelog/2026-08-04.mdx`: Records reviewed MCP discovery runtime dependency updates. - [#8196](#8196) -> `docs/changelog/2026-08-04.mdx`: Records private npm dependency remediation across managed images. - [#8203](#8203) -> `docs/changelog/2026-08-04.mdx`: Records reviewed Hermes and LangChain Deep Agents Code Python dependency updates. - [#8125](#8125) -> `docs/changelog/2026-08-04.mdx`: Records bounded diagnostics for invalid enumerated CLI values. - [#8193](#8193) -> `docs/changelog/2026-08-04.mdx`: Records bounded diagnostics for unresolved sandbox base images. - [#8118](#8118) -> `docs/changelog/2026-08-04.mdx`: Records bounded diagnostics for changed gateway authority. - [#8191](#8191) -> `docs/changelog/2026-08-04.mdx`, `docs/reference/commands.mdx`: Records output capture, marker handling, recovery guidance, and exit behavior for non-JSON OpenClaw agent commands. - [#8187](#8187) -> `docs/changelog/2026-08-04.mdx`: Records the aligned interactive-installation start across supported agents. - [#8153](#8153) -> `docs/changelog/2026-08-04.mdx`: Records current product capabilities and support boundaries. ## 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 - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This documentation-only release preparation does not change executable behavior. Existing changelog and published-route tests pass. - [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: Independently reviewed `docs/changelog/2026-08-04.mdx` and `docs/reference/commands.mdx` at commit `b89913780`. All 38 user-facing v0.0.102 PRs are represented, #8191 behavior matches the implementation, and the writing rules, documentation style, controlled terminology, route structure, and skip policy pass review. Targeted tests pass 36/36 and the documentation build completes with 0 errors. - Agent: Codex Desktop independent documentation writer <!-- docs-review-head-sha: b899137 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - 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 - [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 - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project integration test/changelog-docs.test.ts test/check-docs-published-routes.test.ts` passed 36/36. - [x] Applicable broad gate passed — not applicable to documentation-only changes; `npm run docs` completed successfully with 0 errors. - [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) — completed with 0 errors and 2 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) — the native dated changelog uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added release notes for v0.0.102, covering authentication, hardware setup, WSL, installer recovery, sandbox resilience, policy management, inference reliability, CLI improvements, and unified quickstarts. - Updated command documentation to explain how non-JSON agent output is collected, replayed, and reported. - **Bug Fixes** - Improved command-output recovery guidance when output exceeds limits or contains unsupported fallback markers. - Preserved accurate command exit-status reporting after output processing. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
7 participants
Summary
Route
sessions deleteon a registered Hermes sandbox to the nativehermes sessions delete <id> --yescommand. OpenClaw sandboxes keep the existing gateway RPC behavior, while Hermes receives native-ID validation, explicit flag refusals, focused tests, and matching command documentation.Exact review pair: head
117044bbfc249064f5a77b64509aa80b03d7f1ad, base7f4b490824bb9b8c95405ee97ce913cb333b0e7b.Related Issue
Closes #7642.
Product-scope approval is pending explicit maintainer confirmation independent of GitHub
mergeStateStatus; #7642 is still markedneeds: triage. Do not merge until ownership, lifecycle, compatibility, security, and validation expectations for this maintained Hermes behavior are accepted and linked.Changes
--keep-transcript,--json,--verbose, and non-Hermes--agentvalues.src/lib/state/registry.tsstays within its source-architecture fan-in budget. The sessions action tests and public CLI integration test protect this current consumer.upstream/mainwith signed, non-force merge commits whenever main advances; the reviewed behavior diff remains unchanged.Type of Change
Quality Gates
The exact-head fix and evidence for the prior valid advisor blocker are recorded at #7682 (comment). Fresh exact-pair automated review is pending; any new finding must be resolved or explicitly dispositioned before merge.
Documentation Writer Review
docs-updateddocs/reference/commands.mdxaccurately documents Hermes routing, native IDs,--yes, supported alias behavior, rejected flags, and a copyable example. The writer also reviewed the CLI help, action comments, and test titles across all seven changed files.npm run docspassed with 0 errors and 2 repository-baseline warnings; build, focused tests, integration tests, affected E2E-support tests, type-check, repository checks, and all diff-aware hook lanes passed for the exact head./root/pr7682_docs_writerDGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.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 unavailablenpx --no-install vitest run --project cli src/lib/actions/sandbox/sessions/passed 154/154;npx --no-install vitest run --project integration test/sandbox-sessions-admin-agent-cli.test.tspassed 5/5; the complete affected E2E-support file passed 33/33;npm run typecheck:cliandnpm --prefix nemoclaw run typecheckpassed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable to this focused sessions-routing change;npm run checkspassed at 1,394 files and 4,091 edges, and the targeted behavior and exact diff-aware hook lanes cover the affected surfaces.npm run docsbuilds without warnings (doc changes only)npm run build:clipassed.npm run checkspassed at 1,394 files, 4,091 edges, and 1,898 Vitest candidates.npm run docspassed with 0 errors and 2 repository-baseline warnings. Diff-aware pre-commit, commit-message, and pre-push lanes passed; commitlint reported 0 errors and one non-blocking footer-spacing warning on the contributor's existing first commit. All 16 displayed PR commits are GitHub Verified.Signed-off-by: Dongni Yang dongniy@nvidia.com