Skip to content

feat(cli): add sandbox sessions subcommand group#4570

Closed
laitingsheng wants to merge 8 commits into
mainfrom
feat-sessions-cli-group
Closed

feat(cli): add sandbox sessions subcommand group#4570
laitingsheng wants to merge 8 commits into
mainfrom
feat-sessions-cli-group

Conversation

@laitingsheng
Copy link
Copy Markdown
Contributor

@laitingsheng laitingsheng commented May 30, 2026

Summary

Adds a nemoclaw <name> sessions {list,cleanup,rm,download} host-side subcommand group that wraps OpenClaw's in-sandbox sessions list/sessions cleanup and adds NemoClaw-side rm (per-key or whole-agent wipe) and download (per-key or whole-agent host copy). All in-sandbox work goes through openshell sandbox exec. Documents the on-disk OpenClaw session storage layout in a new Session Storage Layout reference page so audit/replay tooling no longer has to reverse-engineer the paths.

Related Issue

Resolves #834.
Fixes #3978.
Resolves #3979.

Changes

  • src/lib/actions/sandbox/sessions/: new action module split into paths.ts (path resolution + agent-id, session-key, session-id validators), store.ts (sessions.json parse + key→sessionId resolver), passthrough.ts (forwards to openclaw sessions [verb] [args...] via openshell sandbox exec), rm.ts (whole-agent and per-key wipe; resets/edits sessions.json in place), download.ts (whole-agent or per-key host copy via openshell sandbox download).
  • src/commands/sandbox/sessions.ts + src/commands/sandbox/sessions/{list,cleanup,rm,download}.ts: five new oclif command adapters. Pass-through verbs skip this.parse() so arbitrary OpenClaw flags forward verbatim; rm and download use strict args with <agent> required and [<session>] optional plus --out <dir> on download.
  • src/lib/actions/sandbox/sessions/{paths,store}.test.ts: 15 unit tests covering path resolution, agent-id / session-key / session-id validation rejection of shell metacharacters, and sessions.json parse + key resolution edge cases.
  • test/cli.test.ts: four CLI integration tests covering the pass-through wiring (sessions list --json reaches openshell sandbox exec --name <name> -- openclaw sessions list --json), the whole-agent wipe, the per-key wipe (with sessions.json read + edit), and the unknown-key error message.
  • test/e2e/test-sessions-cli.sh + .github/workflows/nightly-e2e.yaml sessions-cli-e2e job: end-to-end coverage that onboards a sandbox, seeds a session, exercises sessions list, sessions cleanup --dry-run, sessions download, sessions rm, then re-asserts the list is empty.
  • docs/reference/session-storage.mdx + docs/index.yml: new reference page documenting the per-agent sessions/ layout (sessions.json, <sessionId>.jsonl, .trajectory.jsonl, .jsonl.lock, -topic-, .reset.<ts>.), what each file is for, and which commands consume them. Closes the documentation gap in [docs] Agent session JSONL / trajectory paths are stable but undocumented #3978.
  • docs/reference/commands.mdx: new ### nemoclaw <name> sessions section with the five subcommand grammar, example invocations, and the Gateway-idleness caveat for rm.
  • src/lib/cli/public-display-defaults.ts + src/lib/cli/command-registry.test.ts: register display metadata for the five new sandbox commands and update the hardcoded command-count assertions (63→68, 35→40, 51→56, 23→24 action tokens).

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • New Features

    • New session management CLI: list, cleanup, reset, export-trajectory, plus a sandbox download command and host-download support.
    • Session validators and trajectory export/reset actions with clear success/error reporting.
  • Documentation

    • Added "Session Storage Layout" reference and expanded session-related command docs with examples and workflows.
  • Tests

    • Added unit/CLI tests and a new E2E script; nightly CI job added to run sessions CLI E2E.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 30, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a sessions CLI surface and actions: validators, passthrough list/cleanup, gateway-backed reset, trajectory export (with optional host download), sandbox download, CLI display wiring, docs, unit tests, an E2E script, and a nightly CI job.

Changes

Sessions CLI feature and testing

Layer / File(s) Summary
Session path validation and tests
src/lib/actions/sandbox/sessions/paths.ts, src/lib/actions/sandbox/sessions/paths.test.ts
Adds validateAgentId, validateSessionKey, parseAgentIdFromSessionKey and Vitest tests validating accepted/rejected formats.
Passthrough helper for sandbox sessions list/cleanup
src/lib/actions/sandbox/sessions/passthrough.ts
Adds SessionsPassthroughVerb/options, hasSessionsPassthroughHelpToken, printSessionsPassthroughHelp, and runSessionsPassthrough to forward openclaw sessions commands into a live sandbox.
Gateway-based session reset action
src/lib/actions/sandbox/sessions/reset.ts
Implements resetSandboxSession with input validation, gateway sessions.reset invocation, envelope parsing, and typed result shape.
Session trajectory export action
src/lib/actions/sandbox/sessions/export-trajectory.ts
Adds exportSandboxSessionTrajectory, summary parsing helpers (parseTrajectoryExportSummary, tryParseTrajectoryExportSummary, ensureTrailingSlash), JSON/human output modes, and optional host download.
Sandbox download action and command
src/lib/actions/sandbox/download.ts, src/commands/sandbox/download.ts
Adds downloadFromSandbox action and sandbox:download CLI command to copy sandbox paths to host destinations.
CLI command definitions for sessions group
src/commands/sandbox/sessions.ts, src/commands/sandbox/sessions/list.ts, src/commands/sandbox/sessions/cleanup.ts, src/commands/sandbox/sessions/reset.ts, src/commands/sandbox/sessions/export-trajectory.ts
Adds sandbox:sessions parent and subcommands for list, cleanup, reset, and export-trajectory that parse args/flags and delegate to the corresponding actions.
CLI display metadata and registry updates
src/lib/cli/public-display-layout.ts, src/lib/cli/public-display-sessions.ts, src/lib/cli/public-display-defaults.ts, src/lib/cli/command-registry.test.ts
Introduces PublicDisplayLayout type, session display layout entries, composes them into public defaults, and updates command-registry test expectations for new commands.
User documentation and navigation
docs/reference/session-storage.mdx, docs/reference/commands.mdx, docs/index.yml
Adds session-storage reference page, documents nemoclaw <name> download and sessions command subcommands, and updates the docs index navigation.
CLI dispatch unit tests
test/cli.test.ts
Extends CLI tests to cover sessions list, export-trajectory (with/without --save-host/--json), reset (success/error/agent mismatch), and sandbox download behavior.
End-to-end test script
test/e2e/test-sessions-cli.sh
Adds a bash E2E script that onboards a sandbox, seeds a session, and runs TC-SESS-01..05 to validate list, cleanup, export-trajectory, reset, and post-reset listing with summary reporting and preflight gating.
Nightly E2E workflow job and review guidance
.github/workflows/nightly-e2e.yaml, .coderabbit.yaml
Adds sessions-cli-e2e job to nightly E2E workflow, configures timeouts/secrets/artifact upload, wires the job into notify/report/scorecard needs, and adds coderabbit review guidance anchor.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4151: Introduces the reusable .github/workflows/e2e-script.yaml workflow runner used by the new sessions-cli-e2e job.

Suggested labels

CI/CD

Suggested reviewers

  • jyaunches
  • ericksoa
  • cv

Poem

🐰 A rabbit's ode to session management
The sessions bloom in .jsonl files,
Reset gateways sweep away the beguiles,
Export trajectories to the host so fast,
Validators guard the paths to keep them cast,
Nightly E2E hums to prove the changes last.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(cli): add sandbox sessions subcommand group' accurately summarizes the main change—adding a new sessions subcommand group to the CLI.
Linked Issues check ✅ Passed The PR meets requirements from #834 (sessions reset via CLI), #3978 (session storage documentation), and #3979 (export/download sessions from running sandbox).
Out of Scope Changes check ✅ Passed All changes are focused on implementing sandbox sessions management (list, cleanup, reset, export-trajectory, download), documentation, tests, and CI integration—all within the stated PR scope.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-sessions-cli-group

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

@laitingsheng laitingsheng added NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). enhancement: feature Use this label to identify requests for new capabilities in NemoClaw. Sandbox Use this label to identify issues related to the NemoClaw isolated environment based on OpenShell. labels May 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 30, 2026

E2E Advisor Recommendation

Required E2E: sessions-cli-e2e
Optional E2E: docs-validation-e2e

Dispatch hint: sessions-cli-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • sessions-cli-e2e (medium: live sandbox onboard plus session seed and session-management commands): Directly covers the changed live user flow: onboard a sandbox, seed an OpenClaw session, run sessions list, sessions cleanup --dry-run, sessions export-trajectory --save-host, and gateway-delegated sessions reset. This is required because the PR changes sandbox lifecycle-adjacent runtime commands, OpenShell exec/download wiring, and OpenClaw gateway RPC routing for real assistant session state.

Optional E2E

  • docs-validation-e2e (low): Useful because the PR adds a new docs page, updates docs navigation, and expands command reference links. This should not be merge-blocking for the runtime session-management changes, but it can catch broken docs build/navigation/link issues.

New E2E recommendations

  • sandbox raw file download command (medium): The new sessions-cli-e2e exercises openshell sandbox download indirectly through sessions export-trajectory --save-host, but it does not explicitly invoke the new top-level nemoclaw <name> download <sandbox-path> [host-dest] command for arbitrary files/directories and destination semantics.
    • Suggested test: Add a direct sandbox download E2E case that creates a known file and directory in a live sandbox, runs nemoclaw <name> download for each, and verifies host-side contents and destination layout.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: sessions-cli-e2e

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 30, 2026

E2E Scenario Advisor Recommendation

Required scenario E2E: ubuntu-repo-cloud-openclaw
Optional scenario E2E: None

Dispatch required scenario E2E:

  • gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required scenario E2E

  • ubuntu-repo-cloud-openclaw: Production CLI changes add sandbox download/sessions commands and update public command display. The scenario catalog has no sessions-specific validation suite, so the smallest dispatchable scenario that exercises repo-current install, CLI availability/help, OpenShell integration, and a live OpenClaw sandbox is ubuntu-repo-cloud-openclaw.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional scenario E2E

  • None.

Relevant changed files

  • src/commands/sandbox/download.ts
  • src/commands/sandbox/sessions.ts
  • src/commands/sandbox/sessions/cleanup.ts
  • src/commands/sandbox/sessions/export-trajectory.ts
  • src/commands/sandbox/sessions/list.ts
  • src/commands/sandbox/sessions/reset.ts
  • src/lib/actions/sandbox/download.ts
  • src/lib/actions/sandbox/sessions/export-trajectory.ts
  • src/lib/actions/sandbox/sessions/passthrough.ts
  • src/lib/actions/sandbox/sessions/paths.ts
  • src/lib/actions/sandbox/sessions/reset.ts
  • src/lib/cli/public-display-defaults.ts
  • src/lib/cli/public-display-layout.ts
  • src/lib/cli/public-display-sessions.ts

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 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 @.github/workflows/nightly-e2e.yaml:
- Around line 510-528: Add the new sessions-cli-e2e job to the workflow_dispatch
"Valid:" jobs list and update coderabbit path_instructions: update the
inputs.jobs.description block (the user-facing valid jobs list referenced around
inputs.jobs.description) to include "sessions-cli-e2e" so manual dispatch docs
reflect the selectable job, and add corresponding path_instructions for the
source files covered by sessions-cli-e2e in .coderabbit.yaml so the new E2E job
is represented in coderabbit's mapping.

In `@docs/reference/commands.mdx`:
- Around line 842-849: Replace the fenced block labeled as `console` with a
copyable shell fence (e.g., `bash` or `sh`) and remove the leading `$` prompt
characters from each example line so the examples like "nemoclaw my-assistant
sessions list", "nemoclaw my-assistant sessions cleanup --dry-run", "nemoclaw
my-assistant sessions rm main", "nemoclaw my-assistant sessions rm main
agent:main:telegram:thread", and "nemoclaw my-assistant sessions download main
--out ./out/" are plain, runnable commands inside the fence.
- Around line 834-841: Add explicit "###" markdown subcommand headings for the
sessions commands so the docs match the CLI help: create headings titled "###
sessions", "### sessions list", "### sessions cleanup", "### sessions rm", and
"### sessions download" and move or duplicate the corresponding table rows'
descriptions under each heading (preserve the forwarded flags and usage examples
like `sessions rm <agent> [<sessionKey>]` and `sessions download <agent>
[<sessionKey>] [--out <dir>]`) so the content under each heading exactly
documents the purpose and flags as in the table; ensure heading text matches the
subcommand names used in the help output.

In `@docs/reference/session-storage.mdx`:
- Around line 94-98: The final "## Related References" section should be
replaced with a standardized "## Next Steps" section; update the heading text
from "Related References" to "Next Steps" and keep the existing link items
(OpenClaw — Session management deep dive, Commands Reference: `nemoclaw <name>
sessions`, Best Practices: workspace and credential storage) under that new
heading so the page conforms to the guideline requiring a bottom "Next Steps"
section; ensure the link labels and destinations remain unchanged while only
changing the heading text and preserving list order.
- Around line 1-12: Add the required frontmatter fields and an explicit H1 that
matches the page title: update the top YAML/frontmatter to include topics, tags,
difficulty, audience, and status keys (populate with appropriate values for this
reference page) and add an H1 heading whose text exactly matches the title value
"Session Storage Layout" (e.g., "# Session Storage Layout") immediately after
the frontmatter; ensure the frontmatter title and the H1 are identical and that
the new keys follow the same naming/format conventions used by other docs pages.

In `@test/e2e/test-sessions-cli.sh`:
- Around line 209-214: The dependent session tests are not actually gated by
seed success; replace the current loose "seed_session || skip ..." logic with an
explicit conditional: run seed_session and if it succeeds then execute
test_sessions_list_json, test_sessions_cleanup_dry_run, test_sessions_download,
test_sessions_rm_agent, and test_sessions_list_empty_after_rm; otherwise call
skip with the existing message and do not run those test_* functions. Locate the
seed invocation (seed_session) and wrap the subsequent test function calls in an
if/then/else block (or equivalent short-circuit that prevents execution) to
ensure the dependent tests only run on successful seeding.
🪄 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: b5a923ef-f4f9-432d-990a-773c9bacb4d1

📥 Commits

Reviewing files that changed from the base of the PR and between 7da4960 and 72d3657.

📒 Files selected for processing (20)
  • .github/workflows/nightly-e2e.yaml
  • docs/index.yml
  • docs/reference/commands.mdx
  • docs/reference/session-storage.mdx
  • src/commands/sandbox/sessions.ts
  • src/commands/sandbox/sessions/cleanup.ts
  • src/commands/sandbox/sessions/download.ts
  • src/commands/sandbox/sessions/list.ts
  • src/commands/sandbox/sessions/rm.ts
  • src/lib/actions/sandbox/sessions/download.ts
  • src/lib/actions/sandbox/sessions/passthrough.ts
  • src/lib/actions/sandbox/sessions/paths.test.ts
  • src/lib/actions/sandbox/sessions/paths.ts
  • src/lib/actions/sandbox/sessions/rm.ts
  • src/lib/actions/sandbox/sessions/store.test.ts
  • src/lib/actions/sandbox/sessions/store.ts
  • src/lib/cli/command-registry.test.ts
  • src/lib/cli/public-display-defaults.ts
  • test/cli.test.ts
  • test/e2e/test-sessions-cli.sh

Comment thread .github/workflows/nightly-e2e.yaml
Comment thread docs/reference/commands.mdx
Comment thread docs/reference/commands.mdx Outdated
Comment thread docs/reference/session-storage.mdx
Comment thread docs/reference/session-storage.mdx Outdated
Comment thread test/e2e/test-sessions-cli.sh Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 30, 2026

PR Review Advisor

Findings: 3 needs attention, 5 worth checking, 0 nice ideas
Since last review: 3 prior items resolved, 5 still apply, 2 new items found

Review findings

🛠️ Needs attention

🔎 Worth checking

  • Source-of-truth review needed: Trajectory export JSON parsing and host download: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `parseTrajectoryExportSummary()` accepts any object with `outputDir` and `sessionId`; `exportSandboxSessionTrajectory()` downloads `ensureTrailingSlash(summary.outputDir)`.
  • `--save-host` trusts the sandbox-reported trajectory export path (src/lib/actions/sandbox/sessions/export-trajectory.ts:99): When `--save-host` is used, the host parses `outputDir` from the in-sandbox OpenClaw JSON summary and passes it directly to `openshell sandbox download`. If the in-sandbox command is mismatched or compromised, it can cause the host wrapper to download an arbitrary sandbox-readable path instead of the intended trajectory export bundle.
    • Recommendation: Validate that `summary.outputDir` is an expected absolute path under the trajectory export base, or have OpenClaw return a stable bundle identifier and reconstruct the allowed path on the host. Add a negative test for an out-of-tree `outputDir`.
    • Evidence: `parseTrajectoryExportSummary()` only requires string `outputDir` and `sessionId`; `exportSandboxSessionTrajectory()` then calls `runOpenshell(["sandbox", "download", sandboxName, remoteSource, saveHost])` using that returned path.
  • `sessions-cli-e2e` runs target-ref code with `NVIDIA_API_KEY` (.github/workflows/nightly-e2e.yaml:520): The new workflow job checks out `${{ inputs.target_ref || github.ref }}` and runs `test/e2e/test-sessions-cli.sh` with `nvidia_api_key: true`. Disabling `github_token` and pinning actions are good mitigations, but this still expands a provider-secret-bearing target-ref execution path.
    • Recommendation: Prefer a fake/local inference endpoint for this job, avoid passing real provider credentials to target-ref code, or gate/document this job so it only runs against trusted refs when secrets are present.
    • Evidence: `sessions-cli-e2e` sets `ref: ${{ inputs.target_ref || github.ref }}`, `script: test/e2e/test-sessions-cli.sh`, `nvidia_api_key: true`, and `github_token: false`; the reusable E2E runner executes `bash "$E2E_SCRIPT"` with `NVIDIA_API_KEY` in the environment.
  • Reset E2E claims archive/rebind coverage but only checks exit and JSON parsing (test/e2e/test-sessions-cli.sh:180): The E2E header says TC-SESS-04 verifies a `<sessionId>.reset.<ts>.jsonl` archive and TC-SESS-05 verifies the rebound key still surfaces, but the implementation only checks that reset exits zero and that a later list response is parseable JSON.
    • Recommendation: After reset, assert that `agent:main:main` is still present with a new session id and that the prior transcript archive exists, or weaken the test description to match the actual smoke coverage.
    • Evidence: TC-SESS-04 runs `nemoclaw "$SANDBOX_NAME" sessions reset main agent:main:main` and passes on exit zero; TC-SESS-05 calls `json.loads` on `sessions list --json` output but never checks the key, new session id, or reset archive.
  • Raw session download examples use a path that contradicts the documented storage layout (docs/reference/session-storage.mdx:73): The new storage page defines the canonical session root as `/sandbox/.openclaw/agents/<agentId>/sessions/`, but its raw-download example, and the commands reference example, use `/sandbox/.openclaw/sessions/main`. Users following the new docs may download a non-existent or wrong path.
    • Recommendation: Change the examples to the documented canonical layout, e.g. `/sandbox/.openclaw/agents/main/sessions/`, and keep `docs/reference/commands.mdx` in sync.
    • Evidence: `docs/reference/session-storage.mdx` shows `/sandbox/.openclaw/agents/<agentId>/sessions/` at the layout block, but later says `nemoclaw my-assistant download /sandbox/.openclaw/sessions/main ./sessions-out/`; `docs/reference/commands.mdx` has the same `/sandbox/.openclaw/sessions/main` example.

🌱 Nice ideas

  • None.
Since last review details

Current findings:

  • Source-of-truth review needed: Trajectory export JSON parsing and host download: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `parseTrajectoryExportSummary()` accepts any object with `outputDir` and `sessionId`; `exportSandboxSessionTrajectory()` downloads `ensureTrailingSlash(summary.outputDir)`.
  • [feature] nemoclaw <name> session export — copy agent session JSONL + trajectory out of a running sandbox #3979 raw session export contract is still not implemented (src/commands/sandbox/sessions/export-trajectory.ts:24): The linked [feature] nemoclaw <name> session export — copy agent session JSONL + trajectory out of a running sandbox #3979 request asks for a raw `nemoclaw <sandbox> session export` helper that defaults to copying all session JSONL/index files, excludes `*.trajectory.jsonl` by default, and supports `--session <id>`, `--include-trajectory`, `--out <dir>`, `--format <dir|tar>`, and `--json`. This PR instead adds a generic `download` command plus `sessions export-trajectory <agent> <sessionKey>`, which delegates to OpenClaw's redacted trajectory export and requires a specific session key.
  • Add openclaw sessions reset <session-id> subcommand #834 asks for `openclaw sessions reset`, not only a NemoClaw gateway wrapper (src/commands/sandbox/sessions/reset.ts:14): The linked Add openclaw sessions reset <session-id> subcommand #834 issue explicitly proposes an upstream `openclaw sessions reset <session-id>` subcommand with examples `openclaw sessions reset main` and `openclaw sessions reset tg-8689248858`. This PR adds `nemoclaw <name> sessions reset <agent> <sessionKey>` and invokes `openclaw gateway call sessions.reset`, but it does not add or verify the requested OpenClaw CLI command surface.
  • Documented root `nemoclaw <name> sessions [flags]` route appears to dispatch to help or a bogus child command (src/lib/cli/public-argv-translation.ts:161): `src/commands/sandbox/sessions.ts` implements a root sessions pass-through, and the docs say `nemoclaw <name> sessions` lists sessions while root flags are forwarded. However, public dispatch treats parent commands with children as help/error routes unless special-cased. There is a special case for bare `channels`, but not for `sessions`, so `nemoclaw alpha sessions` maps to `sandbox sessions --help`, and `nemoclaw alpha sessions --json` maps toward `sandbox:sessions:--json` instead of invoking the root pass-through.
    • Recommendation: Add a `sessions` dispatch special case analogous to `channels` that routes bare/root-flag invocations to `sandbox:sessions`, and add tests for `nemoclaw alpha sessions` and `nemoclaw alpha sessions --json`.
    • Evidence: `translatePublicSandboxArgv()` only special-cases `action === "channels" && actionArgs.length === 0`; parent commands with children and no/unknown child call `nativeSandboxParentArgv()`. Existing tests cover `sessions list --json`, but not the documented root form.
  • `--save-host` trusts the sandbox-reported trajectory export path (src/lib/actions/sandbox/sessions/export-trajectory.ts:99): When `--save-host` is used, the host parses `outputDir` from the in-sandbox OpenClaw JSON summary and passes it directly to `openshell sandbox download`. If the in-sandbox command is mismatched or compromised, it can cause the host wrapper to download an arbitrary sandbox-readable path instead of the intended trajectory export bundle.
    • Recommendation: Validate that `summary.outputDir` is an expected absolute path under the trajectory export base, or have OpenClaw return a stable bundle identifier and reconstruct the allowed path on the host. Add a negative test for an out-of-tree `outputDir`.
    • Evidence: `parseTrajectoryExportSummary()` only requires string `outputDir` and `sessionId`; `exportSandboxSessionTrajectory()` then calls `runOpenshell(["sandbox", "download", sandboxName, remoteSource, saveHost])` using that returned path.
  • `sessions-cli-e2e` runs target-ref code with `NVIDIA_API_KEY` (.github/workflows/nightly-e2e.yaml:520): The new workflow job checks out `${{ inputs.target_ref || github.ref }}` and runs `test/e2e/test-sessions-cli.sh` with `nvidia_api_key: true`. Disabling `github_token` and pinning actions are good mitigations, but this still expands a provider-secret-bearing target-ref execution path.
    • Recommendation: Prefer a fake/local inference endpoint for this job, avoid passing real provider credentials to target-ref code, or gate/document this job so it only runs against trusted refs when secrets are present.
    • Evidence: `sessions-cli-e2e` sets `ref: ${{ inputs.target_ref || github.ref }}`, `script: test/e2e/test-sessions-cli.sh`, `nvidia_api_key: true`, and `github_token: false`; the reusable E2E runner executes `bash "$E2E_SCRIPT"` with `NVIDIA_API_KEY` in the environment.
  • Reset E2E claims archive/rebind coverage but only checks exit and JSON parsing (test/e2e/test-sessions-cli.sh:180): The E2E header says TC-SESS-04 verifies a `<sessionId>.reset.<ts>.jsonl` archive and TC-SESS-05 verifies the rebound key still surfaces, but the implementation only checks that reset exits zero and that a later list response is parseable JSON.
    • Recommendation: After reset, assert that `agent:main:main` is still present with a new session id and that the prior transcript archive exists, or weaken the test description to match the actual smoke coverage.
    • Evidence: TC-SESS-04 runs `nemoclaw "$SANDBOX_NAME" sessions reset main agent:main:main` and passes on exit zero; TC-SESS-05 calls `json.loads` on `sessions list --json` output but never checks the key, new session id, or reset archive.
  • Raw session download examples use a path that contradicts the documented storage layout (docs/reference/session-storage.mdx:73): The new storage page defines the canonical session root as `/sandbox/.openclaw/agents/<agentId>/sessions/`, but its raw-download example, and the commands reference example, use `/sandbox/.openclaw/sessions/main`. Users following the new docs may download a non-existent or wrong path.
    • Recommendation: Change the examples to the documented canonical layout, e.g. `/sandbox/.openclaw/agents/main/sessions/`, and keep `docs/reference/commands.mdx` in sync.
    • Evidence: `docs/reference/session-storage.mdx` shows `/sandbox/.openclaw/agents/<agentId>/sessions/` at the layout block, but later says `nemoclaw my-assistant download /sandbox/.openclaw/sessions/main ./sessions-out/`; `docs/reference/commands.mdx` has the same `/sandbox/.openclaw/sessions/main` example.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

# Conflicts:
#	docs/reference/commands.mdx
#	src/lib/cli/command-registry.test.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/sessions/rm.ts (1)

60-76: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Lock check is racy because probe and delete happen in separate execs.

rm can still race an active writer if a lock appears after the probe but before the delete/update script runs. Move lock detection and deletion into one shell transaction (or re-check immediately before delete in the same script) so the refusal guarantee is actually enforced.

Suggested direction
- const probe = captureOpenshell([...lock probe...]);
- if (lockCount > 0 && !force) failOnActiveLocks(...);
- const result = captureOpenshell([...delete script...]);
+ const result = captureOpenshell([...single script that:
+   1) checks locks
+   2) exits non-zero when locks > 0 and !force
+   3) performs delete + sessions.json write
+   4) prints LOCKS and REMOVED for parsing
+ ...]);

Also applies to: 82-93, 127-139, 153-164

🤖 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/sessions/rm.ts` around lines 60 - 76, The current
two-step approach using captureOpenshell (probe) then a separate delete exec is
racy; move the lock detection and removal into a single shell invocation so
detection and delete happen atomically. Update the code that builds the sandbox
"exec" command (the captureOpenshell call that references sandboxName and
sessionsDir) to perform the existence check, count locks, re-check for locks
immediately before any rm/update, and abort if locks are present — i.e., combine
the probe and delete logic into one shell script (or use an atomic strategy like
creating a temp marker and using POSIX file tests/flock within the same command)
so the refusal guarantee is enforced.
🧹 Nitpick comments (1)
docs/reference/session-storage.mdx (1)

89-96: 💤 Low value

Use fewer em dashes in this paragraph (LLM pattern detected).

This paragraph now has multiple em dashes; use commas/periods for one of them.

As per coding guidelines, “Excessive em dashes. One per paragraph is fine; multiple per paragraph ... should be flagged.”

🤖 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 `@docs/reference/session-storage.mdx` around lines 89 - 96, The paragraph
describing `sessions rm` uses multiple em dashes; revise it to use at most one
em dash by replacing other dashes with commas or sentence breaks, e.g., keep a
single em dash for a parenthetical emphasis (if needed) and convert the clause
about refusing and exiting non-zero into a separate sentence or use commas,
ensure references like `sessions rm`, `*.jsonl.lock`, `--force`, and
`<sessionId>.reset.<timestamp>.jsonl` remain intact and unchanged while
correcting punctuation and sentence boundaries to remove the LLM-style excessive
em dashes.
🤖 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/sessions/rm.ts`:
- Around line 60-76: The current two-step approach using captureOpenshell
(probe) then a separate delete exec is racy; move the lock detection and removal
into a single shell invocation so detection and delete happen atomically. Update
the code that builds the sandbox "exec" command (the captureOpenshell call that
references sandboxName and sessionsDir) to perform the existence check, count
locks, re-check for locks immediately before any rm/update, and abort if locks
are present — i.e., combine the probe and delete logic into one shell script (or
use an atomic strategy like creating a temp marker and using POSIX file
tests/flock within the same command) so the refusal guarantee is enforced.

---

Nitpick comments:
In `@docs/reference/session-storage.mdx`:
- Around line 89-96: The paragraph describing `sessions rm` uses multiple em
dashes; revise it to use at most one em dash by replacing other dashes with
commas or sentence breaks, e.g., keep a single em dash for a parenthetical
emphasis (if needed) and convert the clause about refusing and exiting non-zero
into a separate sentence or use commas, ensure references like `sessions rm`,
`*.jsonl.lock`, `--force`, and `<sessionId>.reset.<timestamp>.jsonl` remain
intact and unchanged while correcting punctuation and sentence boundaries to
remove the LLM-style excessive em dashes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f18f44e7-6462-4be0-92d7-1d01a8d542ad

📥 Commits

Reviewing files that changed from the base of the PR and between a793e71 and f4f7d50.

📒 Files selected for processing (10)
  • .coderabbit.yaml
  • .github/workflows/nightly-e2e.yaml
  • docs/reference/commands.mdx
  • docs/reference/session-storage.mdx
  • src/commands/sandbox/sessions/rm.ts
  • src/lib/actions/sandbox/sessions/rm.ts
  • src/lib/cli/command-registry.test.ts
  • src/lib/cli/public-display-defaults.ts
  • test/cli.test.ts
  • test/e2e/test-sessions-cli.sh
✅ Files skipped from review due to trivial changes (1)
  • docs/reference/commands.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/workflows/nightly-e2e.yaml
  • src/commands/sandbox/sessions/rm.ts
  • src/lib/cli/public-display-defaults.ts
  • test/e2e/test-sessions-cli.sh

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/lib/actions/sandbox/sessions/rm.ts (1)

35-51: Run the targeted sessions E2E workflow before merge.

Given this path controls lock detection and sessions.json rewrites, run sessions-cli-e2e to validate end-to-end safety (list, cleanup --dry-run, download, rm) on this branch.

As per coding guidelines: src/lib/actions/sandbox/sessions/** changes should run sessions-cli-e2e via gh workflow run nightly-e2e.yaml --ref <branch> -f jobs=sessions-cli-e2e.

🤖 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/sessions/rm.ts` around lines 35 - 51, This change to
rmSandboxSessions (and the related functions wipeWholeAgent and
removeSingleSession) affects lock detection and sessions.json rewrites for agent
sessions; before merging, run the sessions E2E workflow to validate
list/cleanup/download/rm flows by executing the sessions-cli-e2e job via GitHub
Actions: run gh workflow run nightly-e2e.yaml --ref <branch> -f
jobs=sessions-cli-e2e and verify the sessions-cli-e2e job passes to ensure
end-to-end safety for these session removal code paths.
🤖 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.

Nitpick comments:
In `@src/lib/actions/sandbox/sessions/rm.ts`:
- Around line 35-51: This change to rmSandboxSessions (and the related functions
wipeWholeAgent and removeSingleSession) affects lock detection and sessions.json
rewrites for agent sessions; before merging, run the sessions E2E workflow to
validate list/cleanup/download/rm flows by executing the sessions-cli-e2e job
via GitHub Actions: run gh workflow run nightly-e2e.yaml --ref <branch> -f
jobs=sessions-cli-e2e and verify the sessions-cli-e2e job passes to ensure
end-to-end safety for these session removal code paths.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 46191472-17cc-4a21-8855-f0454171a453

📥 Commits

Reviewing files that changed from the base of the PR and between f4f7d50 and 0ab26c5.

📒 Files selected for processing (7)
  • .github/workflows/nightly-e2e.yaml
  • docs/reference/session-storage.mdx
  • src/lib/actions/sandbox/sessions/rm.ts
  • src/lib/cli/public-display-defaults.ts
  • src/lib/cli/public-display-layout.ts
  • src/lib/cli/public-display-sessions.ts
  • test/cli.test.ts
✅ Files skipped from review due to trivial changes (2)
  • src/lib/cli/public-display-layout.ts
  • docs/reference/session-storage.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/cli/public-display-defaults.ts
  • .github/workflows/nightly-e2e.yaml

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
docs/reference/commands.mdx (4)

885-885: ⚡ Quick win

Split table cell into separate sentences.

The description contains two independent clauses separated by a semicolon.
Split them for consistency with the one-sentence-per-line guideline.

Suggested rewrite
-| `--reason` | `reset` (default) keeps the archive trail bound to the key; `new` registers a brand-new session id without binding the prior transcript. |
+| `--reason` | `reset` (default) keeps the archive trail bound to the key. `new` registers a brand-new session id without binding the prior transcript. |

As per coding guidelines, "One sentence per line in source (makes diffs readable)."

🤖 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 `@docs/reference/commands.mdx` at line 885, The table cell describing
`--reason` currently uses a semicolon to join two independent clauses; replace
the single semicolon-separated sentence with two sentences so each clause is on
its own line. Locate the `--reason` table cell and change "reset (default) keeps
the archive trail bound to the key; new registers a brand-new session id without
binding the prior transcript." into two sentences like "reset (default) keeps
the archive trail bound to the key. new registers a brand-new session id without
binding the prior transcript." so each sentence is on its own line.

859-859: ⚡ Quick win

Use active voice.

"Files ... are not touched" is passive.
Rewrite with the command as the subject.

Active voice alternative
-Files whose names only share a string prefix with the resolved `sessionId` are not touched.
+The command does not touch files whose names only share a string prefix with the resolved `sessionId`.

As per coding guidelines, "Active voice required. Flag passive constructions."

🤖 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 `@docs/reference/commands.mdx` at line 859, Rewrite the passive sentence so the
command is the subject performing the action: change "Files whose names only
share a string prefix with the resolved `sessionId` are not touched." to an
active-voice sentence where `sessions download` (or "the command") explicitly
does not copy or touch those files; keep the same meaning and mention `sessions
download`, `sessionId`, and the filename patterns (`<sessionId>.*`,
`<sessionId>-topic-*`) so readers can locate the referenced symbols.

877-877: ⚡ Quick win

Split sentences onto separate lines.

Line 877 contains two sentences.
One sentence per line improves diff clarity.

Suggested split
-Goes through `openshell sandbox exec` to `openclaw gateway call sessions.reset`, so the gateway owns the archival of the prior transcript (`<sessionId>.reset.<timestamp>.jsonl`), the lock, and the lifecycle event emission. The NemoClaw host never edits `sessions.json`.
+Goes through `openshell sandbox exec` to `openclaw gateway call sessions.reset`, so the gateway owns the archival of the prior transcript (`<sessionId>.reset.<timestamp>.jsonl`), the lock, and the lifecycle event emission.
+The NemoClaw host never edits `sessions.json`.

As per coding guidelines, "One sentence per line in source (makes diffs readable)."

🤖 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 `@docs/reference/commands.mdx` at line 877, The paragraph "Goes through
`openshell sandbox exec` to `openclaw gateway call sessions.reset`, so the
gateway owns the archival of the prior transcript
(`<sessionId>.reset.<timestamp>.jsonl`), the lock, and the lifecycle event
emission. The NemoClaw host never edits `sessions.json`." should be split into
two separate lines—one containing the sentence about openshell/openclaw and
archival/lock/lifecycle, and the next line containing "The NemoClaw host never
edits `sessions.json`"—so each sentence is on its own line to improve diff
readability.

858-860: ⚡ Quick win

Split sentences onto separate lines.

Line 858 has two independent clauses separated by a semicolon, and line 859 contains two sentences.
One sentence per line makes diffs cleaner.

Suggested split

Line 858:

-`sessions reset` goes through `openshell sandbox exec` to `openclaw gateway call sessions.reset`, so the OpenClaw gateway performs the archive-then-recreate; the NemoClaw host never edits `sessions.json` directly. Sessions stay lock-safe because the gateway is the writer.
+`sessions reset` goes through `openshell sandbox exec` to `openclaw gateway call sessions.reset`, so the OpenClaw gateway performs the archive-then-recreate.
+The NemoClaw host never edits `sessions.json` directly.
+Sessions stay lock-safe because the gateway is the writer.

Line 859:

-`sessions download` reads `sessions.json` over `openshell sandbox exec`, resolves the supplied `<sessionKey>` to its current `sessionId`, and copies the owned filename shapes for that session: `<sessionId>.*` (transcript, trajectory, lock, reset archives) and `<sessionId>-topic-*` (topic transcripts). Files whose names only share a string prefix with the resolved `sessionId` are not touched.
+`sessions download` reads `sessions.json` over `openshell sandbox exec`, resolves the supplied `<sessionKey>` to its current `sessionId`, and copies the owned filename shapes for that session: `<sessionId>.*` (transcript, trajectory, lock, reset archives) and `<sessionId>-topic-*` (topic transcripts).
+The command does not touch files whose names only share a string prefix with the resolved `sessionId`.

As per coding guidelines, "One sentence per line in source (makes diffs readable). Flag paragraphs where multiple sentences appear on the same line."

🤖 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 `@docs/reference/commands.mdx` around lines 858 - 860, The paragraph describing
the behavior of the sessions commands contains multiple sentences on the same
lines; update the doc text so each sentence is on its own line for clearer
diffs: split the line that mentions "sessions reset goes through openshell
sandbox exec to openclaw gateway call sessions.reset, so the OpenClaw gateway
performs the archive-then-recreate; the NemoClaw host never edits sessions.json
directly." into separate lines for each independent clause and likewise split
the line describing "sessions download reads sessions.json over openshell
sandbox exec, resolves the supplied <sessionKey> to its current <sessionId>, and
copies..." into one sentence per line (preserve references to sessions reset,
openshell sandbox exec, openclaw gateway call sessions.reset, sessions download,
sessions.json, <sessionKey>, and <sessionId> so meaning is unchanged).
🤖 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/e2e/test-sessions-cli.sh`:
- Around line 185-199: The test_sessions_list_after_reset currently only checks
for valid JSON but does not assert that the rebound session key is present;
update the function test_sessions_list_after_reset (TC-SESS-05) to parse the
JSON output of the nemoclaw "$SANDBOX_NAME" sessions list --json command and
assert that the expected rebound session key (use the actual key name used
elsewhere, e.g., rebound_session_key or the variable holding it) is present in
at least one session object, failing the test and dumping $out if the key is
missing.

---

Nitpick comments:
In `@docs/reference/commands.mdx`:
- Line 885: The table cell describing `--reason` currently uses a semicolon to
join two independent clauses; replace the single semicolon-separated sentence
with two sentences so each clause is on its own line. Locate the `--reason`
table cell and change "reset (default) keeps the archive trail bound to the key;
new registers a brand-new session id without binding the prior transcript." into
two sentences like "reset (default) keeps the archive trail bound to the key.
new registers a brand-new session id without binding the prior transcript." so
each sentence is on its own line.
- Line 859: Rewrite the passive sentence so the command is the subject
performing the action: change "Files whose names only share a string prefix with
the resolved `sessionId` are not touched." to an active-voice sentence where
`sessions download` (or "the command") explicitly does not copy or touch those
files; keep the same meaning and mention `sessions download`, `sessionId`, and
the filename patterns (`<sessionId>.*`, `<sessionId>-topic-*`) so readers can
locate the referenced symbols.
- Line 877: The paragraph "Goes through `openshell sandbox exec` to `openclaw
gateway call sessions.reset`, so the gateway owns the archival of the prior
transcript (`<sessionId>.reset.<timestamp>.jsonl`), the lock, and the lifecycle
event emission. The NemoClaw host never edits `sessions.json`." should be split
into two separate lines—one containing the sentence about openshell/openclaw and
archival/lock/lifecycle, and the next line containing "The NemoClaw host never
edits `sessions.json`"—so each sentence is on its own line to improve diff
readability.
- Around line 858-860: The paragraph describing the behavior of the sessions
commands contains multiple sentences on the same lines; update the doc text so
each sentence is on its own line for clearer diffs: split the line that mentions
"sessions reset goes through openshell sandbox exec to openclaw gateway call
sessions.reset, so the OpenClaw gateway performs the archive-then-recreate; the
NemoClaw host never edits sessions.json directly." into separate lines for each
independent clause and likewise split the line describing "sessions download
reads sessions.json over openshell sandbox exec, resolves the supplied
<sessionKey> to its current <sessionId>, and copies..." into one sentence per
line (preserve references to sessions reset, openshell sandbox exec, openclaw
gateway call sessions.reset, sessions download, sessions.json, <sessionKey>, and
<sessionId> so meaning is unchanged).
🪄 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: e628be25-aea6-4611-b7a2-104a1cfbd477

📥 Commits

Reviewing files that changed from the base of the PR and between 0ab26c5 and 5d6237c.

📒 Files selected for processing (10)
  • .coderabbit.yaml
  • .github/workflows/nightly-e2e.yaml
  • docs/reference/commands.mdx
  • docs/reference/session-storage.mdx
  • src/commands/sandbox/sessions/reset.ts
  • src/lib/actions/sandbox/sessions/reset.ts
  • src/lib/cli/command-registry.test.ts
  • src/lib/cli/public-display-sessions.ts
  • test/cli.test.ts
  • test/e2e/test-sessions-cli.sh
✅ Files skipped from review due to trivial changes (2)
  • src/lib/cli/command-registry.test.ts
  • docs/reference/session-storage.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/cli/public-display-sessions.ts
  • .coderabbit.yaml
  • .github/workflows/nightly-e2e.yaml

Comment on lines +185 to +199
# ── TC-SESS-05: list after reset still surfaces the rebound key ──────────────
test_sessions_list_after_reset() {
section "TC-SESS-05: sessions list --json after reset still surfaces the key"
local out
out="$(nemoclaw "$SANDBOX_NAME" sessions list --json 2>&1)" || {
fail "TC-SESS-05: sessions list --json exited non-zero after reset"
info "$out"
return 1
}
if ! printf '%s' "$out" | python3 -c "import json,sys; v=json.loads(sys.stdin.read())" 2>/dev/null; then
fail "TC-SESS-05: sessions list --json after reset did not return parseable JSON"
info "$out"
return 1
fi
pass "TC-SESS-05: sessions list --json after reset returned valid JSON"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert the rebound session key in TC-SESS-05.

Line 194 only validates that output is JSON; it does not verify the rebound key is present, so this test can pass even if reset behavior regresses.

Suggested patch
 test_sessions_list_after_reset() {
   section "TC-SESS-05: sessions list --json after reset still surfaces the key"
   local out
+  local key="agent:main:main"
   out="$(nemoclaw "$SANDBOX_NAME" sessions list --json 2>&1)" || {
     fail "TC-SESS-05: sessions list --json exited non-zero after reset"
     info "$out"
     return 1
   }
-  if ! printf '%s' "$out" | python3 -c "import json,sys; v=json.loads(sys.stdin.read())" 2>/dev/null; then
-    fail "TC-SESS-05: sessions list --json after reset did not return parseable JSON"
+  if ! printf '%s' "$out" | python3 - "$key" <<'PY' 2>/dev/null
+import json, sys
+key = sys.argv[1]
+data = json.loads(sys.stdin.read())
+
+def has_key(node):
+    if isinstance(node, dict):
+        return node.get("key") == key or any(has_key(v) for v in node.values())
+    if isinstance(node, list):
+        return any(has_key(v) for v in node)
+    return False
+
+sys.exit(0 if has_key(data) else 1)
+PY
+  then
+    fail "TC-SESS-05: sessions list --json after reset did not include '${key}'"
     info "$out"
     return 1
   fi
-  pass "TC-SESS-05: sessions list --json after reset returned valid JSON"
+  pass "TC-SESS-05: sessions list --json after reset still includes '${key}'"
 }
🤖 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/e2e/test-sessions-cli.sh` around lines 185 - 199, The
test_sessions_list_after_reset currently only checks for valid JSON but does not
assert that the rebound session key is present; update the function
test_sessions_list_after_reset (TC-SESS-05) to parse the JSON output of the
nemoclaw "$SANDBOX_NAME" sessions list --json command and assert that the
expected rebound session key (use the actual key name used elsewhere, e.g.,
rebound_session_key or the variable holding it) is present in at least one
session object, failing the test and dumping $out if the key is missing.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/cli/public-display-sessions.ts`:
- Around line 35-36: Update the CLI command flags in public-display-sessions.ts
to use the canonical sessionKey placeholder: change the flags string that
currently reads "<agent> <session> [--reason new|reset]" to "<agent>
<sessionKey> [--reason new|reset]" so help text aligns with the rest of the docs
and the sessionKey terminology used elsewhere (look for the flags property on
the command definition in public-display-sessions.ts).
🪄 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: 2d59f907-879b-46fc-9a1c-3a25a8722a1b

📥 Commits

Reviewing files that changed from the base of the PR and between 0ab26c5 and 5d6237c.

📒 Files selected for processing (10)
  • .coderabbit.yaml
  • .github/workflows/nightly-e2e.yaml
  • docs/reference/commands.mdx
  • docs/reference/session-storage.mdx
  • src/commands/sandbox/sessions/reset.ts
  • src/lib/actions/sandbox/sessions/reset.ts
  • src/lib/cli/command-registry.test.ts
  • src/lib/cli/public-display-sessions.ts
  • test/cli.test.ts
  • test/e2e/test-sessions-cli.sh
🚧 Files skipped from review as they are similar to previous changes (7)
  • docs/reference/commands.mdx
  • .coderabbit.yaml
  • src/commands/sandbox/sessions/reset.ts
  • .github/workflows/nightly-e2e.yaml
  • src/lib/actions/sandbox/sessions/reset.ts
  • test/cli.test.ts
  • test/e2e/test-sessions-cli.sh

Comment thread src/lib/cli/public-display-sessions.ts Outdated
…host

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.coderabbit.yaml (1)

261-266: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Update the command list to match the actual implementation.

The description mentions rm|download (line 263-264), but the actual commands tested are reset and export-trajectory (lines 270-272). The PR objectives confirm that download was replaced with export-trajectory during development.

📝 Proposed fix
     - path: "src/lib/actions/sandbox/sessions/**"
       instructions: &e2e-sessions-cli |
         This file is part of the host-side OpenClaw sessions management
-        surface (`nemoclaw <name> sessions list|cleanup|rm|download`).
+        surface (`nemoclaw <name> sessions list|cleanup|reset|export-trajectory`).
         Changes affect session store reads, owned-shape file matching,
         write-lock detection, and `sessions.json` rewrites inside a running
🤖 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 @.coderabbit.yaml around lines 261 - 266, Update the documented command list
under the instructions anchor (&e2e-sessions-cli) so it reflects the actual
implemented commands: replace the stale "rm|download" mention with the current
"reset|export-trajectory" verbs (or otherwise enumerate the real commands used
by the implementation and tests) so the description matches what nemoclaw
implements and the PR objectives.
🧹 Nitpick comments (5)
docs/reference/commands.mdx (4)

859-859: ⚡ Quick win

Format commands and flags as inline code.

Line 859 mentions openclaw sessions export-trajectory --json, --save-host, and openshell sandbox download without backticks.

As per coding guidelines, "CLI commands, file paths, flags, parameter names, and values must use inline code formatting."

Suggested fix
-`sessions export-trajectory` runs `openclaw sessions export-trajectory --json` inside the sandbox so OpenClaw owns the redaction pipeline and the on-disk bundle shape; pass `--save-host <dir>` to additionally copy the resolved bundle directory to the host via `openshell sandbox download`.

(Note: Apply backticks to openclaw sessions export-trajectory --json, --save-host <dir>, and openshell sandbox download)

🤖 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 `@docs/reference/commands.mdx` at line 859, Update the documentation sentence
to format the CLI commands and flags as inline code: wrap "openclaw sessions
export-trajectory --json", the flag " --save-host <dir>", and the command
"openshell sandbox download" in backticks so they appear as inline code in the
docs; ensure spacing and punctuation remain correct after adding backticks.

846-846: ⚡ Quick win

Format command names as inline code.

The description mentions openclaw sessions export-trajectory and openshell sandbox download but they are not formatted with backticks.

As per coding guidelines, "CLI commands, file paths, flags, parameter names, and values must use inline code formatting."

Suggested fix
-| `sessions export-trajectory <agent> <sessionKey> [--output <name>] [--workspace <dir>] [--save-host <dir>] [--json]` | Run openclaw sessions export-trajectory inside the sandbox and (optionally) copy the resulting bundle to the host via openshell sandbox download. |
+| `sessions export-trajectory <agent> <sessionKey> [--output <name>] [--workspace <dir>] [--save-host <dir>] [--json]` | Run `openclaw sessions export-trajectory` inside the sandbox and (optionally) copy the resulting bundle to the host via `openshell sandbox download`. |
🤖 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 `@docs/reference/commands.mdx` at line 846, The command descriptions need
inline code formatting: edit the table row containing sessions export-trajectory
and wrap the two CLI commands `openclaw sessions export-trajectory` and
`openshell sandbox download` in backticks so they appear as inline code; ensure
any other CLI tokens in that same line (flags like --output, --workspace,
--save-host, --json are already formatted) remain unchanged.

897-897: ⚡ Quick win

Format file path as inline code.

The path .openclaw/trajectory-exports in the --output flag description should use backticks.

As per coding guidelines, "CLI commands, file paths, flags, parameter names, and values must use inline code formatting."

Suggested fix
-| `--output <name>` | Bundle directory name inside the sandbox's .openclaw/trajectory-exports. OpenClaw assigns a name when omitted. |
+| `--output <name>` | Bundle directory name inside the sandbox's `.openclaw/trajectory-exports`. OpenClaw assigns a name when omitted. |
🤖 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 `@docs/reference/commands.mdx` at line 897, Update the `--output <name>` flag
description so the file path `.openclaw/trajectory-exports` is formatted as
inline code; locate the text containing `--output <name>` and replace the plain
path with backticked ` .openclaw/trajectory-exports ` (i.e.,
`\`.openclaw/trajectory-exports\``) so it follows the guideline that CLI
commands, file paths, flags, parameter names, and values use inline `code`
formatting.

889-889: ⚡ Quick win

Format commands and paths as inline code.

Line 889 mentions openclaw sessions export-trajectory --json, openshell sandbox exec, and the bundle layout .openclaw/trajectory-exports/ without backticks.

As per coding guidelines, "CLI commands, file paths, flags, parameter names, and values must use inline code formatting."

Suggested fix
-Runs `openclaw sessions export-trajectory --json` inside the sandbox via `openshell sandbox exec`, so OpenClaw owns the redaction pipeline and the bundle layout (`.openclaw/trajectory-exports/<output>/`).

(Note: The command, exec wrapper, and path should all have backticks)

🤖 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 `@docs/reference/commands.mdx` at line 889, Update the documentation sentence
to format CLI commands, wrapper and path as inline code: wrap openclaw sessions
export-trajectory --json, openshell sandbox exec, and the bundle path
.openclaw/trajectory-exports/<output>/ in backticks so they appear as `openclaw
sessions export-trajectory --json`, `openshell sandbox exec`, and
`/.openclaw/trajectory-exports/<output>/` respectively; ensure flags and the
path segment `<output>` remain inside the code spans to satisfy the guideline
for commands, flags, and file paths.
src/lib/cli/public-display-sessions.ts (1)

43-43: ⚡ Quick win

Use canonical <sessionKey> placeholder.

Line 43 uses <session>, but the CLI and docs describe this argument as a canonical sessionKey (per line 838 in commands.mdx: "sessionKey arguments are the canonical keys as stored in sessions.json"). Use <sessionKey> to keep help text consistent with the rest of the sessions command surface.

Suggested fix
-      flags: "<agent> <session> [--output <name>] [--workspace <dir>] [--save-host <dir>] [--json]",
+      flags: "<agent> <sessionKey> [--output <name>] [--workspace <dir>] [--save-host <dir>] [--json]",
🤖 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/cli/public-display-sessions.ts` at line 43, Update the CLI help flags
in src/lib/cli/public-display-sessions.ts to use the canonical placeholder
'<sessionKey>' instead of '<session>' so the flags string (the flags property
for the public-display-sessions command) matches the rest of the sessions
surface and the documentation (commands.mdx); locate the flags definition where
it reads 'flags: "<agent> <session> [...]"' and replace the '<session>' token
with '<sessionKey>' preserving the rest of the flag syntax and options.
🤖 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 @.coderabbit.yaml:
- Around line 261-266: Update the documented command list under the instructions
anchor (&e2e-sessions-cli) so it reflects the actual implemented commands:
replace the stale "rm|download" mention with the current
"reset|export-trajectory" verbs (or otherwise enumerate the real commands used
by the implementation and tests) so the description matches what nemoclaw
implements and the PR objectives.

---

Nitpick comments:
In `@docs/reference/commands.mdx`:
- Line 859: Update the documentation sentence to format the CLI commands and
flags as inline code: wrap "openclaw sessions export-trajectory --json", the
flag " --save-host <dir>", and the command "openshell sandbox download" in
backticks so they appear as inline code in the docs; ensure spacing and
punctuation remain correct after adding backticks.
- Line 846: The command descriptions need inline code formatting: edit the table
row containing sessions export-trajectory and wrap the two CLI commands
`openclaw sessions export-trajectory` and `openshell sandbox download` in
backticks so they appear as inline code; ensure any other CLI tokens in that
same line (flags like --output, --workspace, --save-host, --json are already
formatted) remain unchanged.
- Line 897: Update the `--output <name>` flag description so the file path
`.openclaw/trajectory-exports` is formatted as inline code; locate the text
containing `--output <name>` and replace the plain path with backticked `
.openclaw/trajectory-exports ` (i.e., `\`.openclaw/trajectory-exports\``) so it
follows the guideline that CLI commands, file paths, flags, parameter names, and
values use inline `code` formatting.
- Line 889: Update the documentation sentence to format CLI commands, wrapper
and path as inline code: wrap openclaw sessions export-trajectory --json,
openshell sandbox exec, and the bundle path
.openclaw/trajectory-exports/<output>/ in backticks so they appear as `openclaw
sessions export-trajectory --json`, `openshell sandbox exec`, and
`/.openclaw/trajectory-exports/<output>/` respectively; ensure flags and the
path segment `<output>` remain inside the code spans to satisfy the guideline
for commands, flags, and file paths.

In `@src/lib/cli/public-display-sessions.ts`:
- Line 43: Update the CLI help flags in src/lib/cli/public-display-sessions.ts
to use the canonical placeholder '<sessionKey>' instead of '<session>' so the
flags string (the flags property for the public-display-sessions command)
matches the rest of the sessions surface and the documentation (commands.mdx);
locate the flags definition where it reads 'flags: "<agent> <session> [...]"'
and replace the '<session>' token with '<sessionKey>' preserving the rest of the
flag syntax and options.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ebb3500b-7a7d-4195-9e91-228638a6725e

📥 Commits

Reviewing files that changed from the base of the PR and between 5d6237c and 1483b37.

📒 Files selected for processing (12)
  • .coderabbit.yaml
  • .github/workflows/nightly-e2e.yaml
  • docs/reference/commands.mdx
  • docs/reference/session-storage.mdx
  • src/commands/sandbox/sessions/export-trajectory.ts
  • src/lib/actions/sandbox/sessions/export-trajectory.ts
  • src/lib/actions/sandbox/sessions/paths.test.ts
  • src/lib/actions/sandbox/sessions/paths.ts
  • src/lib/cli/command-registry.test.ts
  • src/lib/cli/public-display-sessions.ts
  • test/cli.test.ts
  • test/e2e/test-sessions-cli.sh
💤 Files with no reviewable changes (2)
  • src/lib/actions/sandbox/sessions/paths.test.ts
  • src/lib/actions/sandbox/sessions/paths.ts
✅ Files skipped from review due to trivial changes (1)
  • docs/reference/session-storage.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/cli/command-registry.test.ts
  • test/e2e/test-sessions-cli.sh

…ndling

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Comment thread test/cli.test.ts
expect(fs.existsSync(hostDest)).toBe(true);
const calls = fs.readFileSync(openshellLog, "utf8");
expect(calls).toMatch(
new RegExp(`sandbox download alpha /sandbox/workspace/notes\\.md ${hostDest.replace(/\//g, "\\/")}`),
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/download.ts`:
- Around line 24-27: The helper downloadFromSandbox should not terminate the
process; replace the current process.exit call in the sandbox path validation
with a thrown error so the CLI boundary can decide exit behavior. In the
downloadFromSandbox function, where you check opts.sandboxPath (the block that
currently does console.error(...) then process.exit(1)), remove the
process.exit(1) and instead throw a descriptive Error (e.g., "No sandbox path
provided; refusing to invoke `openshell sandbox download`.") so upstream callers
can handle/log/exit as appropriate.
🪄 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: 3c06c355-6484-4456-af4f-c54e569ff41e

📥 Commits

Reviewing files that changed from the base of the PR and between 1483b37 and 7485962.

📒 Files selected for processing (11)
  • .coderabbit.yaml
  • docs/reference/commands.mdx
  • docs/reference/session-storage.mdx
  • src/commands/sandbox/download.ts
  • src/lib/actions/sandbox/download.ts
  • src/lib/actions/sandbox/sessions/paths.ts
  • src/lib/actions/sandbox/sessions/reset.ts
  • src/lib/cli/command-registry.test.ts
  • src/lib/cli/public-display-defaults.ts
  • src/lib/cli/public-display-sessions.ts
  • test/cli.test.ts
✅ Files skipped from review due to trivial changes (2)
  • docs/reference/commands.mdx
  • docs/reference/session-storage.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
  • .coderabbit.yaml
  • src/lib/cli/public-display-sessions.ts
  • src/lib/actions/sandbox/sessions/reset.ts
  • src/lib/cli/public-display-defaults.ts

Comment on lines +24 to +27
if (!opts.sandboxPath || opts.sandboxPath.trim().length === 0) {
console.error(" No sandbox path provided; refusing to invoke `openshell sandbox download`.");
process.exit(1);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid terminating the process inside the action helper.

downloadFromSandbox() should throw and let the CLI boundary decide exit behavior; process.exit(1) here can short-circuit upstream handling and reuse paths.

Proposed fix
   if (!opts.sandboxPath || opts.sandboxPath.trim().length === 0) {
-    console.error("  No sandbox path provided; refusing to invoke `openshell sandbox download`.");
-    process.exit(1);
+    throw new Error("No sandbox path provided; refusing to invoke `openshell sandbox download`.");
   }

Based on learnings: "avoid adding defensive error handling around internal helper logic... only add validation/error handling at system boundaries."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!opts.sandboxPath || opts.sandboxPath.trim().length === 0) {
console.error(" No sandbox path provided; refusing to invoke `openshell sandbox download`.");
process.exit(1);
}
if (!opts.sandboxPath || opts.sandboxPath.trim().length === 0) {
throw new Error("No sandbox path provided; refusing to invoke `openshell sandbox download`.");
}
🤖 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/download.ts` around lines 24 - 27, The helper
downloadFromSandbox should not terminate the process; replace the current
process.exit call in the sandbox path validation with a thrown error so the CLI
boundary can decide exit behavior. In the downloadFromSandbox function, where
you check opts.sandboxPath (the block that currently does console.error(...)
then process.exit(1)), remove the process.exit(1) and instead throw a
descriptive Error (e.g., "No sandbox path provided; refusing to invoke
`openshell sandbox download`.") so upstream callers can handle/log/exit as
appropriate.

@laitingsheng laitingsheng deleted the feat-sessions-cli-group branch May 31, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement: feature Use this label to identify requests for new capabilities in NemoClaw. NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). Sandbox Use this label to identify issues related to the NemoClaw isolated environment based on OpenShell.

Projects

None yet

2 participants