fix(sessions): hide onboard warm-up session from list and export by rluo8 · Pull Request #5533 · NVIDIA/NemoClaw

rluo8 · 2026-06-17T05:47:11Z

Summary

nemoclaw onboard runs a throwaway scope-upgrade warm-up that persists an internal nemoclaw-onboard-warmup-* session. On a freshly-onboarded sandbox that session is the only entry, so nemoclaw <name> sessions list reported 1 conversation and sessions export bundled a "ping" the user never sent. This filters that internal session out of the NemoClaw user-facing surfaces — sessions list, sessions list --json, and sessions export.

Related Issue

Fixes #5511

Changes

src/lib/actions/sandbox/warmup-session.ts (new): single source of truth — WARMUP_SESSION_ID_PREFIX + isWarmupSessionId.
src/lib/actions/sandbox/sessions/passthrough.ts: capture sessions list and filter internal warm-up sessions from both human-table output (filterWarmupSessionsListText, anchored on the id: field) and --json (filterWarmupSessionsListJson, tolerant balanced-JSON parse reusing parseSessionIndex; fails closed — errors rather than emitting unfiltered output if it can't parse but a warm-up id is present). stdout is filtered; OpenClaw's stderr is preserved on its own stream.
src/lib/actions/sandbox/sessions/export.ts: exclude warm-up sessions from "export all" (explicit --keys still honored); harden parseSessionIndex with balanced-JSON extraction for multi-line payloads.
src/lib/adapters/openshell/client.ts + runtime.ts: opt-in includeStreams option exposing separate stdout/stderr on the capture result (additive, backward-compatible).
src/lib/actions/sandbox/auto-pair-warmup.ts: reference the shared prefix in --session-id only — no change to warm-up timing/behavior.
Tests: passthrough.test.ts (new), client.test.ts (new), and updates to export.test.ts / auto-pair-warmup.test.ts.

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

[√] PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
[√] Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
[√] Targeted tests pass for changed behavior
Full npm test passes (broad runtime changes only)
[√] 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: rluo8 ruluo@nvidia.com

Summary by CodeRabbit

New Features
- Warm-up sessions are now automatically hidden from default session listing and export-all, while still retrievable when explicitly requested.
- Improved robustness for parsing sessions list JSON output, including noisy or wrapped responses.
Bug Fixes
- Export and passthrough filtering now reliably exclude warm-up sessions.
- Session list counts are adjusted correctly after warm-up entries are removed.
Documentation
- Updated CLI help text to reflect warm-up hiding behavior.
Tests
- Added/expanded coverage for warm-up tagging, filtering, export behavior, and tolerant noisy JSON handling.

Signed-off-by: Rui Luo <ruluo@nvidia.com>

coderabbitai · 2026-06-17T05:47:27Z

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Shared warmup-session prefixes now drive warm-up tagging, export and sessions list output hide warm-up sessions, and the related CLI text and OpenShell wrapper were updated.

Changes

Warmup Session Filtering

Layer / File(s)	Summary
Warmup session ID constant and script `src/lib/actions/sandbox/warmup-session.ts`, `src/lib/actions/sandbox/auto-pair-warmup.ts`, `src/lib/actions/sandbox/auto-pair-warmup.test.ts`	Adds `WARMUP_SESSION_ID_PREFIX` and `isWarmupSessionId()`, uses the shared prefix in `WARMUP_SCRIPT`, and updates tests for the shared prefix and shell-dependent exec round trip.
Balanced JSON parsing and export filtering `src/lib/actions/sandbox/sessions/export.ts`, `src/lib/actions/sandbox/sessions/export.test.ts`	`parseSessionIndex` now derives candidates from balanced JSON substrings, export-all skips warm-up sessions, and export tests cover noisy JSON parsing, wrapper rejection, and warm-up export selection.
Sessions passthrough warmup filtering `src/lib/actions/sandbox/sessions/passthrough.ts`, `src/lib/actions/sandbox/sessions/passthrough.test.ts`	`runSessionsPassthrough` captures `sessions` and `sessions list` output, filters warm-up rows from JSON or text results, and exits on unsafe JSON parse failures; tests cover JSON, text, shorthand, and non-zero exit handling.
CLI descriptions and OpenShell streams `src/commands/sandbox/sessions.ts`, `src/commands/sandbox/sessions/export.ts`, `src/commands/sandbox/sessions/list.ts`, `src/lib/adapters/openshell/runtime.ts`, `test/sandbox-sessions-export-cli.test.ts`	Updates sessions CLI descriptions to mention warm-up hiding, forwards `includeStreams` in the OpenShell runtime wrapper, and adds CLI coverage for warm-up filtering with stderr preserved.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

NVIDIA/NemoClaw#5526: Also updates src/lib/actions/sandbox/sessions/export.ts, touching the same export selection path.

Suggested labels

area: onboarding, area: sandbox

Suggested reviewers

cv
prekshivyas
sandl99

Poem

A bunny tagged a warmup with a prefix bright,
Then hid it from the list and export sight.
Balanced JSON hopped through the gate,
While stderr still could safelyేట్?
🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.90% 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 title accurately summarizes the main change: hiding onboard warm-up sessions from sessions list and export.
Linked Issues check	✅ Passed	The changes address `#5511` by filtering warm-up sessions from list, list --json, and export-all, with tests covering the required behavior.
Out of Scope Changes check	✅ Passed	No clearly unrelated code changes stand out; the extra helper, capture option, and help text updates support the warm-up filtering work.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

prekshivyas

Correct. captureOpenshell is synchronous so the call at line 782 needs no await. filterWarmupSessionsListJson failing closed when it can't parse but a warm-up id is present is the right safety invariant. The includeStreams opt-in on the openshell client is additive and backward-compatible.

Two minor notes: (1) filterWarmupSessionsListPayload probes sessions, entries, and items but the tests only cover the sessions key — low risk since that's the live key, but a quick fixture for the other two paths would guard future format changes. (2) filterWarmupSessionsListText anchors on id:${WARMUP_SESSION_ID_PREFIX} — this is correct for the current OpenClaw text format but is format-fragile; a comment noting the dependency on the id: field label would help future maintainers.

Otherwise coverage is solid: fail-closed path, noisy-output tolerance, explicit-key bypass, and text count adjustment are all exercised.

Approve.

…anup Signed-off-by: Rui Luo <ruluo@nvidia.com> # Conflicts: # src/lib/actions/sandbox/sessions/export.test.ts # src/lib/actions/sandbox/sessions/export.ts

…anup Signed-off-by: Rui Luo <ruluo@nvidia.com> # Conflicts: # src/lib/actions/sandbox/sessions/export.test.ts

coderabbitai

🧹 Nitpick comments (1)

test/sandbox-sessions-export-cli.test.ts (1)

71-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid keying JSON extraction to a hard-coded warning prefix.

Line 73 only parses correctly while stderr begins with warning:. If the non-fatal message text changes, indexOf("\nwarning:") becomes -1 and this test fails for the wrong reason. Parse the JSON portion independently of the warning text instead.

Suggested change

-      expect(result.out).toContain("warning: noisy but non-fatal");
-      expect(result.out).not.toContain("nemoclaw-onboard-warmup-");
-      expect(JSON.parse(result.out.slice(0, result.out.indexOf("\nwarning:")))).toEqual({
+      expect(result.out).toContain("warning: noisy but non-fatal");
+      expect(result.out).not.toContain("nemoclaw-onboard-warmup-");
+      const jsonLine = result.out
+        .split("\n")
+        .find((line) => line.trimStart().startsWith("{"));
+      expect(jsonLine).toBeDefined();
+      expect(JSON.parse(jsonLine!)).toEqual({
         count: 1,
         totalCount: 1,
         sessions: [{ key: "agent:main:explicit:real", sessionId: "sid-real" }],
       });

🤖 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/sandbox-sessions-export-cli.test.ts` around lines 71 - 73, The test in
sandbox-sessions-export-cli.test.ts is too tightly coupled to the warning text
by slicing stdout at a hard-coded warning prefix; update the JSON extraction in
the sandbox-sessions export CLI test to locate the JSON payload independently of
the warning message so it still parses correctly if the warning text changes.
Use the existing result.out assertions in this test and replace the
indexOf("\nwarning:") approach with a delimiter or structure based on the JSON
output itself.

🤖 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 `@test/sandbox-sessions-export-cli.test.ts`:
- Around line 71-73: The test in sandbox-sessions-export-cli.test.ts is too
tightly coupled to the warning text by slicing stdout at a hard-coded warning
prefix; update the JSON extraction in the sandbox-sessions export CLI test to
locate the JSON payload independently of the warning message so it still parses
correctly if the warning text changes. Use the existing result.out assertions in
this test and replace the indexOf("\nwarning:") approach with a delimiter or
structure based on the JSON output itself.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6fd25d55-a0e8-4715-a08e-06d1c9379bd0

📥 Commits

Reviewing files that changed from the base of the PR and between abb6f15 and 8b5a71d.

📒 Files selected for processing (7)

src/commands/sandbox/sessions.ts
src/commands/sandbox/sessions/export.ts
src/commands/sandbox/sessions/list.ts
src/lib/actions/sandbox/sessions/passthrough.test.ts
src/lib/actions/sandbox/sessions/passthrough.ts
src/lib/actions/sandbox/warmup-session.ts
test/sandbox-sessions-export-cli.test.ts

✅ Files skipped from review due to trivial changes (3)

src/commands/sandbox/sessions/export.ts
src/commands/sandbox/sessions.ts
src/commands/sandbox/sessions/list.ts

🚧 Files skipped from review as they are similar to previous changes (3)

src/lib/actions/sandbox/warmup-session.ts
src/lib/actions/sandbox/sessions/passthrough.test.ts
src/lib/actions/sandbox/sessions/passthrough.ts

cv · 2026-06-26T01:16:26Z

Manual PR Review Advisor result

This PR Review Advisor analysis was run manually via workflow_dispatch, so the workflow did not post its usual sticky comment. Posting the advisor summary here to populate the PR with advisor feedback.

Run: https://github.com/NVIDIA/NemoClaw/actions/runs/28210598269

Recommendation: merge_as_is (high confidence; findings: 0)

The diff addresses the linked warm-up session visibility bug with focused filtering, security-sensitive command boundaries remain constrained, and the changed behavior has targeted unit plus CLI-stub coverage.

PR Review Advisor

The diff addresses the linked warm-up session visibility bug with focused filtering, security-sensitive command boundaries remain constrained, and the changed behavior has targeted unit plus CLI-stub coverage.

Required before merge

None.

Resolve or justify before merge

None.

In-scope improvements

None.

Test follow-ups to resolve or justify

Runtime validation — Freshly onboarded sandbox with only the warm-up session: sessions list, parent sessions, sessions list --json, and parent sessions --json show zero non-internal sessions and preserve non-fatal stderr separately.. The changed behavior is well covered by unit and existing CLI-stub tests, but it depends on real OpenShell/OpenClaw stdout, stderr, table formatting, and sandbox download behavior, so targeted runtime validation would increase confidence.
Runtime validation — Freshly onboarded sandbox with only the warm-up session: sessions export --out <dir> exits nonzero with the no-sessions message and leaves no output directory or tarball behind.. The changed behavior is well covered by unit and existing CLI-stub tests, but it depends on real OpenShell/OpenClaw stdout, stderr, table formatting, and sandbox download behavior, so targeted runtime validation would increase confidence.
Runtime validation — Sandbox containing one real session plus one nemoclaw-onboard-warmup-* session: default export copies only the real session JSONL and sessions list reports only the real row.. The changed behavior is well covered by unit and existing CLI-stub tests, but it depends on real OpenShell/OpenClaw stdout, stderr, table formatting, and sandbox download behavior, so targeted runtime validation would increase confidence.
Runtime validation — Large sessions-list output exceeding the capture buffer returns the ENOBUFS guidance without hanging and without emitting partial unfiltered warm-up rows.. The changed behavior is well covered by unit and existing CLI-stub tests, but it depends on real OpenShell/OpenClaw stdout, stderr, table formatting, and sandbox download behavior, so targeted runtime validation would increase confidence.

What looks good

The parser extraction shrinks the existing export implementation while moving OpenClaw JSON compatibility rules into a focused, tested module.
Security-sensitive export behavior remains constrained: resolved filenames are validated, tar operands stay behind --, and sensitive session artifacts keep owner-only permissions.
The change preserves explicit warm-up-session export for debugging while fixing only the default user-facing list/export-all surfaces requested by the issue.
The new pass-through tests include both fail-closed and false-positive cases, which is important for a localized text/JSON filtering workaround.

…DIA#5533)  ## Summary  `nemoclaw onboard` runs a throwaway scope-upgrade warm-up that persists an internal `nemoclaw-onboard-warmup-*` session. On a freshly-onboarded sandbox that session is the only entry, so `nemoclaw <name> sessions list` reported 1 conversation and `sessions export` bundled a `"ping"` the user never sent. This filters that internal session out of the NemoClaw user-facing surfaces — `sessions list`, `sessions list --json`, and `sessions export`. ## Related Issue  Fixes NVIDIA#5511 ## Changes  - **`src/lib/actions/sandbox/warmup-session.ts`** (new): single source of truth — `WARMUP_SESSION_ID_PREFIX` + `isWarmupSessionId`. - **`src/lib/actions/sandbox/sessions/passthrough.ts`**: capture `sessions list` and filter internal warm-up sessions from both human-table output (`filterWarmupSessionsListText`, anchored on the `id:` field) and `--json` (`filterWarmupSessionsListJson`, tolerant balanced-JSON parse reusing `parseSessionIndex`; **fails closed** — errors rather than emitting unfiltered output if it can't parse but a warm-up id is present). stdout is filtered; OpenClaw's stderr is preserved on its own stream. - **`src/lib/actions/sandbox/sessions/export.ts`**: exclude warm-up sessions from "export all" (explicit `--keys` still honored); harden `parseSessionIndex` with balanced-JSON extraction for multi-line payloads. - **`src/lib/adapters/openshell/client.ts` + `runtime.ts`**: opt-in `includeStreams` option exposing separate `stdout`/`stderr` on the capture result (additive, backward-compatible). - **`src/lib/actions/sandbox/auto-pair-warmup.ts`**: reference the shared prefix in `--session-id` only — **no change to warm-up timing/behavior**. - Tests: `passthrough.test.ts` (new), `client.test.ts` (new), and updates to `export.test.ts` / `auto-pair-warmup.test.ts`. ## 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  - [√] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [√] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [√] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [√] 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](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [√] New doc pages include SPDX header and frontmatter (new pages only) ---  Signed-off-by: rluo8 <ruluo@nvidia.com>  ## Summary by CodeRabbit * **New Features** * Warm-up sessions are now automatically hidden from default session listing and export-all, while still retrievable when explicitly requested. * Improved robustness for parsing `sessions list` JSON output, including noisy or wrapped responses. * **Bug Fixes** * Export and passthrough filtering now reliably exclude warm-up sessions. * Session list counts are adjusted correctly after warm-up entries are removed. * **Documentation** * Updated CLI help text to reflect warm-up hiding behavior. * **Tests** * Added/expanded coverage for warm-up tagging, filtering, export behavior, and tolerant noisy JSON handling.  --------- Signed-off-by: Rui Luo <ruluo@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>

fix(sessions): hide onboard warm-up session from list and export

28da5be

Signed-off-by: Rui Luo <ruluo@nvidia.com>

Merge branch 'main' into fix/5511-warmup-cleanup

3e32dbc

rluo8 added the v0.0.66 label Jun 17, 2026

cv approved these changes Jun 17, 2026

View reviewed changes

Merge branch 'main' into fix/5511-warmup-cleanup

ba13646

prekshivyas self-assigned this Jun 22, 2026

prekshivyas approved these changes Jun 22, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/main' into fix/5511-warmup-cle…

87a12ab

…anup Signed-off-by: Rui Luo <ruluo@nvidia.com> # Conflicts: # src/lib/actions/sandbox/sessions/export.test.ts # src/lib/actions/sandbox/sessions/export.ts

cv added v0.0.67 and removed v0.0.66 labels Jun 23, 2026

wscurran added area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression labels Jun 23, 2026

jyaunches added v0.0.68 and removed v0.0.67 labels Jun 24, 2026

Merge remote-tracking branch 'upstream/main' into fix/5511-warmup-cle…

2d43aae

…anup Signed-off-by: Rui Luo <ruluo@nvidia.com> # Conflicts: # src/lib/actions/sandbox/sessions/export.test.ts

rluo8 force-pushed the fix/5511-warmup-cleanup branch from daafce2 to 2d43aae Compare June 25, 2026 01:18

cv and others added 2 commits June 24, 2026 19:17

Merge branch 'main' into fix/5511-warmup-cleanup

9da76be

Merge branch 'main' into fix/5511-warmup-cleanup

65d67da

jyaunches mentioned this pull request Jun 25, 2026

Epic: Restore Vitest parity for post-migration bash E2E changes #5800

Closed

5 tasks

jyaunches added v0.0.69 and removed v0.0.68 labels Jun 25, 2026

jyaunches added 2 commits June 25, 2026 19:11

Merge branch 'main' into fix/5511-warmup-cleanup

abb6f15

fix(sessions): address warmup filtering review

8b5a71d

coderabbitai Bot reviewed Jun 25, 2026

View reviewed changes

jyaunches added 3 commits June 25, 2026 19:51

style(sessions): apply formatter

6540f27

fix(sessions): address advisor follow-ups

4f69f79

docs(sessions): sync agent command reference

e72b4f9

wscurran added the NV QA Bugs found by the NVIDIA QA Team label Jun 26, 2026

fix(sessions): harden warmup filtering coverage

288ed35

jyaunches added 2 commits June 25, 2026 20:59

refactor(sessions): extract session index parser

6b99df8

Merge remote-tracking branch 'origin/main' into pr-5533-ci-loop

70618b1

Merge branch 'main' into fix/5511-warmup-cleanup

69075c1

cv merged commit ffa8967 into NVIDIA:main Jun 26, 2026
31 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sessions): hide onboard warm-up session from list and export#5533

fix(sessions): hide onboard warm-up session from list and export#5533
cv merged 16 commits into
NVIDIA:mainfrom
rluo8:fix/5511-warmup-cleanup

rluo8 commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

prekshivyas left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

cv commented Jun 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

rluo8 commented Jun 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Type of Change

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

prekshivyas left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cv commented Jun 26, 2026

Manual PR Review Advisor result

PR Review Advisor

Required before merge

Resolve or justify before merge

In-scope improvements

Test follow-ups to resolve or justify

What looks good

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rluo8 commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading