Skip to content

fix(sandbox/sessions): route export by agent kind - #5526

Merged
cv merged 9 commits into
mainfrom
fix/sessions-export-route-by-agent-kind
Jun 24, 2026
Merged

fix(sandbox/sessions): route export by agent kind#5526
cv merged 9 commits into
mainfrom
fix/sessions-export-route-by-agent-kind

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

nemohermes <name> sessions export aborted with exit 127 because the wrapper unconditionally invoked the in-sandbox openclaw sessions list regardless of the sandbox's agent kind. The wrapper now reads sb.agent from the registry and routes Hermes sandboxes to hermes sessions export, downloading the resulting single JSONL stream to the host.

Related Issue

Fixes #5515

Changes

  • exportSandboxSessions dispatches on registry.getSandbox(name)?.agent; on hermes, runs umask 077 && hermes sessions export <staging> && chmod 600 <staging> in the sandbox, downloads the JSONL to the host, and best-effort cleans up the staging file.
  • OpenClaw-only flags (--agent, positional keys, --include-trajectory, --format tar) raise a clear error on a Hermes sandbox instead of producing a misleading half-export.
  • Host destination on Hermes defaults to ./sessions-<sandbox>.jsonl; --out picks a different path.
  • Command help text and docs/reference/commands.mdx document both branches; the nemohermes docs variant is regenerated.
  • Unit + CLI E2E tests cover the Hermes wire calls (exec sh -c, download, rm -f cleanup), option-rejection, and the manifest shape.

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: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • New Features

    • Sessions export now routes by sandbox agent kind: OpenClaw exports per-session JSONL files (optionally with trajectories) or a .tgz; Hermes exports a single JSONL stream produced inside the sandbox.
    • Hermes output defaults and option handling are now agent-specific, with stronger input/flag validation.
  • Documentation

    • Updated sessions export reference docs and CLI help to reflect OpenClaw vs Hermes flows and clarified security/permissions (owner-only files) plus staging and cleanup behavior.
  • Tests

    • Added/expanded Hermes-focused tests for routing, transfer/rename, and failure/cleanup scenarios.

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

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds Hermes-specific routing to exportSandboxSessions: when the sandbox's agent is "hermes", the new exportHermesSessions function runs hermes sessions export inside the sandbox, downloads the resulting JSONL to the host with owner-only permissions, and cleans up the staging file. OpenClaw-only options are rejected for Hermes sandboxes. SessionsExportFormat gains a "jsonl" variant, CLI metadata and reference docs are updated accordingly, and new unit and CLI-level tests cover the Hermes path.

Changes

Hermes sessions export routing

Layer / File(s) Summary
SessionsExportFormat type and Hermes dispatch
src/lib/actions/sandbox/sessions/export.ts
Adds "jsonl" to the SessionsExportFormat union, imports registry, and inserts an early-return Hermes dispatch in exportSandboxSessions that checks sandbox.agent === "hermes" and calls exportHermesSessions.
Hermes export implementation and helpers
src/lib/actions/sandbox/sessions/export.ts
Implements exportHermesSessions: validates and rejects OpenClaw-only options, generates a randomized staging path with umask 077, runs hermes sessions export in-sandbox, downloads to the host (default ./sessions-<name>.jsonl), applies chmod 600 host permissions, stats size, and best-effort removes the staging file in a finally block. Includes helper functions for option validation, staging path generation, shell command construction, and host destination resolution.
CLI command metadata and display layout
src/commands/sandbox/sessions/export.ts, src/lib/cli/public-display-sessions.ts
Updates SandboxSessionsExportCommand.summary and static description to describe agent-kind routing and Hermes-specific behavior. Updates SANDBOX_SESSIONS_DISPLAY_LAYOUT entry to annotate `--format <dir
Unit tests for Hermes export path
src/lib/actions/sandbox/sessions/export.test.ts
Mocks the registry module and adds a describe("exportSandboxSessions (hermes sandbox)") block covering command routing, staging filename patterns, download destination handling with out option, file cleanup across success/failure scenarios, chmodSync error handling, and validation that OpenClaw-only flags are rejected without invoking the runner.
CLI-level integration test
test/sandbox-sessions-export-cli.test.ts
Adds end-to-end Vitest case verifying the CLI invokes hermes sessions export (not openclaw sessions list), asserts umask 077/chmod 600 in the shell invocation, validates sandbox download destination, verifies staging cleanup, and confirms the emitted JSON manifest includes agent: "hermes", format: "jsonl", and the correct resolved filename.
Reference documentation
docs/reference/commands.mdx, docs/reference/commands-nemohermes.mdx
Documents split export behavior per agent kind, annotates OpenClaw-only flags, updates per-agent --out defaults, and adds security guidance for owner-only host output (0600), in-sandbox staging with umask 077, and cleanup timing.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as sessions export CLI
    participant registry as registry.getSandbox
    participant sandbox as Sandbox (Hermes)
    participant host as Host Filesystem

    CLI->>registry: getSandbox(sandboxName)
    registry-->>CLI: { agent: "hermes", ... }
    CLI->>CLI: validate: reject --agent (non-hermes), --include-trajectory, --format tar
    CLI->>sandbox: shell "umask 077 && mkdir -p /sandbox/.nemoclaw-staging && hermes sessions export > /sandbox/.nemoclaw-staging/sessions-export-hermes-<rand>.jsonl && chmod 600"
    sandbox-->>CLI: exit 0
    CLI->>sandbox: sandbox download to host staging
    sandbox-->>host: JSONL file transferred
    CLI->>host: chmod 600 and rename to final destination
    CLI->>sandbox: rm -f /sandbox/.nemoclaw-staging/sessions-export-hermes-<rand>.jsonl (finally)
    CLI-->>CLI: return { agent:"hermes", format:"jsonl", resolvedFiles:[...] }
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

documentation

Poem

🐇 Hop hop, the Hermes path is clear,
No openclaw binary to fear!
umask 077 keeps secrets tight,
A JSONL stream downloaded right.
/sandbox/.nemoclaw-staging cleaned,
The finest export the sandbox has seen! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% 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 'fix(sandbox/sessions): route export by agent kind' clearly and concisely summarizes the main change: implementing agent-aware routing for the sessions export command.
Linked Issues check ✅ Passed The PR directly addresses issue #5515 by implementing agent-aware routing so nemohermes sessions export works on Hermes sandboxes, achieving feature parity with OpenClaw.
Out of Scope Changes check ✅ Passed All changes (export.ts logic, tests, docs, CLI display) are directly related to implementing Hermes sandbox support and agent-aware routing for sessions export.

✏️ 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 fix/sessions-export-route-by-agent-kind

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

@github-actions

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/sessions-export-... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/sessions-export-... fd6666f +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the fix/sessions-export-... branch is 46%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/sessions-export-... fd6666f +/-
src/lib/state/o...oard-session.ts 91%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 67%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/onboard...er-gpu-patch.ts 50%
src/lib/policy/index.ts 49%
src/lib/onboard.ts 18%

Updated June 23, 2026 14:10 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@laitingsheng laitingsheng added integration: openclaw OpenClaw integration behavior integration: hermes Hermes integration behavior area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression labels Jun 17, 2026
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: sessions-agents-cli-vitest, hermes-e2e-vitest
Optional E2E: macos-e2e

Dispatch hint: sessions-agents-cli-vitest,hermes-e2e-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • sessions-agents-cli-vitest (high): Closest existing live E2E for the affected OpenClaw sessions host CLI path. It onboards a real OpenClaw sandbox, exercises session listing/reset/delete and agent session flows through the public CLI and OpenShell gateway, covering the same command family and live sandbox boundary changed by this PR.
  • hermes-e2e-vitest (high): The PR adds Hermes-specific routing for session export based on registry agent kind. Existing Hermes live E2E validates a real Hermes sandbox install/start/user-flow path and helps catch regressions in the registry, Hermes CLI availability, and live sandbox assumptions used by the new export branch.

Optional E2E

  • macos-e2e (high): Optional cross-platform confidence for host-side path, chmod, rename, and CLI build behavior affected by the new local staging/finalization code. It is not merge-blocking because the changed export path is not directly covered by this full macOS flow.

New E2E recommendations

  • openclaw-session-export-live (high): Existing live sessions E2E covers sessions list/reset/delete but does not appear to invoke nemoclaw <sandbox> sessions export. Add live coverage that seeds an OpenClaw session, exports dir and tar formats, verifies downloaded JSONL contents/0600 permissions, confirms trajectory filtering, and checks staging cleanup.
    • Suggested test: Add an OpenClaw live sessions-export scenario or extend sessions-agents-cli-vitest with sessions export assertions.
  • hermes-session-export-live (high): The new Hermes branch is covered by unit and stub CLI tests, but there is no existing live E2E proving hermes sessions export works inside a real Hermes sandbox and downloads through OpenShell with safe permissions and cleanup.
    • Suggested test: Add a Hermes live sessions-export scenario that creates a Hermes conversation, runs nemoclaw <sandbox> sessions export --json, verifies the JSONL download, rejects OpenClaw-only flags, and asserts remote staging cleanup.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: sessions-agents-cli-vitest,hermes-e2e-vitest

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: sessions-agents-cli-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required Vitest E2E scenarios

  • None.

Optional Vitest E2E scenarios

  • sessions-agents-cli-vitest: Adjacent live Vitest coverage for the host-side sessions/agents CLI surface. This PR changes sessions export, which is not directly covered by the current live job, but the job still exercises the same sandbox sessions command family and OpenClaw sandbox routing.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=sessions-agents-cli-vitest

Relevant changed files

  • src/commands/sandbox/sessions/export.ts
  • src/lib/actions/sandbox/sessions/export.ts
  • src/lib/cli/public-display-sessions.ts
  • test/sandbox-sessions-export-cli.test.ts

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 3 test follow-ups
Since last review: 0 prior items resolved · 3 still apply · 0 new items found

Action checklist

  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Runtime validation: run `nemohermes <hermes-sandbox> sessions export --agent hermes --out <temp>.jsonl --json` on a live Hermes sandbox, then verify the host JSONL exists, is owner-only (`0600`), contains exported JSONL session records, and the manifest reports `format: "jsonl"`.. Static unit and stub-CLI coverage is strong, but the changed behavior crosses a real Hermes/OpenShell boundary: the actual Hermes CLI must accept the output-file form, OpenShell must download the `/sandbox/.nemoclaw-staging` path, and the public `nemohermes` alias should be validated against a live or direct-stub invocation.
  • PRA-T2 Runtime validation — Runtime validation: in a live Hermes sandbox, verify `hermes sessions export /sandbox/.nemoclaw-staging/<file>.jsonl` creates a downloadable file and `openshell sandbox download <sandbox> /sandbox/.nemoclaw-staging/<file>.jsonl <host-path>` succeeds.. Static unit and stub-CLI coverage is strong, but the changed behavior crosses a real Hermes/OpenShell boundary: the actual Hermes CLI must accept the output-file form, OpenShell must download the `/sandbox/.nemoclaw-staging` path, and the public `nemohermes` alias should be validated against a live or direct-stub invocation.
  • PRA-T3 Runtime validation — Static CLI coverage improvement: add or identify a stub test that launches `bin/nemohermes.js` for `sessions export --agent hermes`, so the exact linked-issue command alias is covered in addition to the existing `bin/nemoclaw.js` public-dispatch path.. Static unit and stub-CLI coverage is strong, but the changed behavior crosses a real Hermes/OpenShell boundary: the actual Hermes CLI must accept the output-file form, OpenShell must download the `/sandbox/.nemoclaw-staging` path, and the public `nemohermes` alias should be validated against a live or direct-stub invocation.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🧹 Nitpick comments (2)
docs/reference/commands.mdx (1)

1355-1361: ⚡ Quick win

Remove decorative bold and rewrite the passive sentence in active voice.

Lines 1355 and 1360 use bold for routine section labels, and Line 1361 uses passive voice (“are refused”).
LLM pattern detected.

As per coding guidelines, avoid unnecessary bold in prose (reserved for UI labels/real warnings) and require active voice in documentation.

🤖 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 1355 - 1361, Remove the bold
formatting from the section labels "OpenClaw sandbox" and "Hermes sandbox" on
lines 1355 and 1360 respectively, as these are routine section headers and bold
should be reserved only for UI labels and real warnings. Additionally, convert
the passive voice construction "are refused" on line 1361 to active voice by
rewriting it so that the subject (the system or command handler) actively
performs the refusal action instead of the options being passively refused.

Source: Coding guidelines

docs/reference/commands-nemohermes.mdx (1)

1099-1099: ⚡ Quick win

Remove bold lead-ins in routine prose.

Line 1099 and Line 1104 use bold for standard explanatory text, which the style guide flags as an LLM pattern. Use plain prose (or proper headings) instead. LLM pattern detected.

Suggested edit
-**OpenClaw sandbox.** Enumerates the session store through `openclaw sessions list --agent <id> --json` and copies only the matching `<sessionId>.jsonl` (plus optional `<sessionId>.trajectory.jsonl`) files, so the export never picks up `sessions.json`, stale `.jsonl.lock` files, or other store bookkeeping.
+For an OpenClaw sandbox, the command enumerates the session store through `openclaw sessions list --agent <id> --json` and copies only the matching `<sessionId>.jsonl` (plus optional `<sessionId>.trajectory.jsonl`) files, so the export never picks up `sessions.json`, stale `.jsonl.lock` files, or other store bookkeeping.
@@
-**Hermes sandbox.** Invokes the in-sandbox `hermes sessions export` against a staging path, then downloads the resulting single JSONL stream to the host.
+For a Hermes sandbox, the command invokes the in-sandbox `hermes sessions export` against a staging path, then downloads the resulting single JSONL stream to the host.

As per coding guidelines, “Unnecessary bold on routine instructions … flag every occurrence.”

Also applies to: 1104-1104

🤖 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-nemohermes.mdx` at line 1099, Remove bold formatting
from the lead-in text at lines 1099 and 1104 in the documentation file. The
phrases marked with double asterisks (like **OpenClaw sandbox.** and the similar
bold text at line 1104) should be converted to plain prose without bold
formatting. Replace the bold formatting with regular text to comply with the
style guide that flags bold lead-ins in routine explanatory prose as an LLM
pattern.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/actions/sandbox/sessions/export.test.ts`:
- Around line 471-484: The vi.spyOn call for fs.chmodSync in the test "honours
--out for the host destination on a hermes sandbox" creates a spy that is never
restored, which causes mocked behavior to leak into subsequent tests. Store the
spy result from the vi.spyOn(fs, "chmodSync") call and call .mockRestore() on it
after all the expect statements in the test to ensure the original function
behavior is restored before the next test runs.

---

Nitpick comments:
In `@docs/reference/commands-nemohermes.mdx`:
- Line 1099: Remove bold formatting from the lead-in text at lines 1099 and 1104
in the documentation file. The phrases marked with double asterisks (like
**OpenClaw sandbox.** and the similar bold text at line 1104) should be
converted to plain prose without bold formatting. Replace the bold formatting
with regular text to comply with the style guide that flags bold lead-ins in
routine explanatory prose as an LLM pattern.

In `@docs/reference/commands.mdx`:
- Around line 1355-1361: Remove the bold formatting from the section labels
"OpenClaw sandbox" and "Hermes sandbox" on lines 1355 and 1360 respectively, as
these are routine section headers and bold should be reserved only for UI labels
and real warnings. Additionally, convert the passive voice construction "are
refused" on line 1361 to active voice by rewriting it so that the subject (the
system or command handler) actively performs the refusal action instead of the
options being passively refused.
🪄 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: 458f9bc9-19cf-4c83-a35d-269e1da95595

📥 Commits

Reviewing files that changed from the base of the PR and between a6e664d and b859a61.

📒 Files selected for processing (6)
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx
  • src/commands/sandbox/sessions/export.ts
  • src/lib/actions/sandbox/sessions/export.test.ts
  • src/lib/actions/sandbox/sessions/export.ts
  • test/sandbox-sessions-export-cli.test.ts

Comment thread src/lib/actions/sandbox/sessions/export.test.ts

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/sandbox-sessions-export-cli.test.ts`:
- Around line 272-274: The `home` variable is being interpolated directly into
the RegExp pattern without escaping special regex characters, which can cause
the assertion to be flaky or over-permissive if the path contains regex
operators like dots or brackets. Before embedding the `home` value into the
template string within the RegExp constructor call in the expect statement with
toMatch, escape any special regex characters in the `home` string using a
utility function or by replacing regex metacharacters. This ensures the path is
treated as literal characters in the regex pattern rather than as regex
operators.
🪄 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: fd5db9ca-bffa-4eb7-baff-ac52e832d2ea

📥 Commits

Reviewing files that changed from the base of the PR and between b859a61 and c42b88d.

📒 Files selected for processing (4)
  • src/lib/actions/sandbox/sessions/export.test.ts
  • src/lib/actions/sandbox/sessions/export.ts
  • src/lib/cli/public-display-sessions.ts
  • test/sandbox-sessions-export-cli.test.ts
✅ Files skipped from review due to trivial changes (1)
  • src/lib/cli/public-display-sessions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/actions/sandbox/sessions/export.ts

Comment thread test/sandbox-sessions-export-cli.test.ts
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

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.

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 43-44: The flags string for the public-display-sessions command at
line 43-44 is missing the required `<name>` positional argument that represents
the sandbox name. Add `<name>` as the first positional argument before
`[keys...]` in the flags string to match the actual command usage and maintain
consistency with other command entries in this file (such as those at lines 27
and 35) which correctly include their required positionals.
🪄 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: 379d545d-2cb4-43f1-857a-6eff3b60eb3f

📥 Commits

Reviewing files that changed from the base of the PR and between c42b88d and 698df35.

📒 Files selected for processing (4)
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx
  • src/lib/cli/public-display-sessions.ts
  • test/sandbox-sessions-export-cli.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx

Comment thread src/lib/cli/public-display-sessions.ts
@cv

cv commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

@laitingsheng some interesting findings in #5526 (comment); mind addressing them before we merge?

@prekshivyas prekshivyas self-assigned this Jun 22, 2026

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Correct and well-structured. Dispatch at the exportSandboxSessions entry point is clean; rejectOpenClawOnlyOptions fires before any sandbox I/O. The umask 077 && hermes sessions export <staging> && chmod 600 <staging> invocation is safe — shellQuote is used on the staging path, and the random hex suffix prevents collisions. Staging cleanup is in a finally block and the chmod-before-rename ordering prevents a world-readable window at the final path.

One minor gap for a follow-up: if registry.getSandbox(name)?.agent is something other than "hermes" or nullish (a future third agent kind), the code silently falls through to the OpenClaw path rather than failing explicitly. Not a blocker for this PR.

Test coverage is solid: six targeted unit cases and the CLI integration test confirm the routing at the call-log level.

Approve.

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Correcting my earlier approval — I missed the PR Review Advisor status.

The advisor has this PR marked Blocked with two required items that must be resolved before merge:

  • PRA-3: hermesStagingPath() returns /tmp/…, but openshell sandbox download refuses sources outside /sandbox. The Hermes staging path needs to move to /sandbox/.nemoclaw-staging/sessions-export-hermes-*.jsonl (matching the STAGING_DIR_IN_SANDBOX comment at line 100–106), with 0700 perms and umask 077 on write.
  • PRA-4: rejectOpenClawOnlyOptions() rejects --agent hermes before executing anything. Issue #5515 Step 5 expects nemohermes … sessions export --agent hermes to succeed on a Hermes sandbox — treat it as a no-op alias rather than an error.

Additionally, three items to resolve/justify: PRA-1/PRA-2 (architecture source-of-truth), PRA-5 (silent suppression of cleanup errors for sensitive session artifacts).

cc @cv — this aligns with your comment asking @laitingsheng to address the PRA findings first.

…staging and accepts --agent hermes

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
… + document hermes source boundary

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

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/actions/sandbox/sessions/export.test.ts`:
- Around line 613-632: The test at lines 613-632 for "warns about a non-zero
in-sandbox cleanup exit" will fail because the mocked download call does not
actually create a file, but the exportSandboxSessions implementation attempts to
call fs.chmodSync on the hostStagingPath which does not exist, causing an ENOENT
error before the test can verify the console warning. Mock the fs.chmodSync
operation (and any other fs operations that interact with the staged file) to
prevent this error and allow the test to proceed to the consoleWarnSpy
assertion, following the same pattern used in the test at line 473 that
addresses this isolation issue.
🪄 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: 6cba9bfe-74dd-4895-b655-58fa665d72cc

📥 Commits

Reviewing files that changed from the base of the PR and between 7f38d0c and 26915c5.

📒 Files selected for processing (2)
  • src/lib/actions/sandbox/sessions/export.test.ts
  • src/lib/actions/sandbox/sessions/export.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/actions/sandbox/sessions/export.ts

Comment thread src/lib/actions/sandbox/sessions/export.test.ts
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@cv
cv merged commit 287e439 into main Jun 24, 2026
42 checks passed
@cv
cv deleted the fix/sessions-export-route-by-agent-kind branch June 24, 2026 21:00
@miyoungc miyoungc mentioned this pull request Jun 25, 2026
21 tasks
cv pushed a commit that referenced this pull request Jun 25, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Refreshes the v0.0.68 docs with release notes, Microsoft Teams messaging
guidance, and generated support-matrix coverage.
Also corrects the Hermes generated command reference so `sessions
export` examples match Hermes-only behavior while keeping the shared
command heading compatible with CLI parity checks.

## Changes
- #5585 -> `docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/platform-support.mdx`: Documents experimental Microsoft
Teams channel setup, Bot Framework credentials, webhook forwarding,
local `MSTEAMS_PORT` conflicts, and the generated integration support
row.
- #5526 -> `docs/reference/commands.mdx`,
`docs/reference/commands-nemohermes.mdx`: Keeps Hermes `sessions export`
examples on the supported single-JSONL export path while preserving the
canonical shared CLI heading.
- #5044 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for installer onboarding failure propagation.
- #5641 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for fresh recovery after pre-sandbox installer interruption.
- #5673 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for explicit deny-all messaging manifests.
- #5743 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for unsupported-agent channel-add rejection.
- #5252 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for Hermes `gateway-token` dashboard guidance.
- #5659 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for local OpenClaw `agent` wrapper help.
- #5661 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for default extra-agent paths.
- #5669 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for Deep Agents Code branding.
- #5672 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for hosted-compatible default model ID preservation.
- #5725 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for Deep Agents Code sandbox liveness.
- #5644 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for DGX Spark managed-vLLM express install defaults.
- #5712 -> `docs/about/release-notes.mdx`,
`docs/reference/platform-support.mdx`: Adds v0.0.68 release-note
coverage for the canonical support matrix and updates the matrix source
with Teams.

## Type of Change

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

## Quality Gates
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: docs-only release refresh;
no runtime code changed.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] 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)
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — `npm
run docs` passed with 0 errors; Fern reported the existing light-mode
accent contrast warning.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

Additional verification:
- `python3 scripts/generate-platform-docs.py --check` passed.
- `npm run docs:sync-agent-variants` passed.
- `bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-cli`
passed.
- `npm run docs` passed with 0 errors and one Fern theme warning:
light-mode accent contrast ratio is 2.41:1 and should be at least 3:1.
- `npm run build:cli` refreshed local untracked `dist/` artifacts after
rebase; no tracked files changed.
- `npm run typecheck:cli` passed.
- Normal commit and push hooks passed after the local CLI rebuild.

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added **Microsoft Teams** as an experimental messaging channel,
including manifest-first onboarding, local port routing/conflict
handling, sandbox delivery controls, and policy preset support.

* **Documentation**
* Expanded messaging-channel setup for Teams (prerequisites,
credential/webhook setup, wizard flow, and add/remove commands).
* Updated reference docs for **agent-specific** session export examples
(OpenClaw vs Hermes).
* Refreshed platform support guidance and added the latest release-notes
entry.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@wscurran wscurran added the NV QA Bugs found by the NVIDIA QA Team label Jun 26, 2026
cv pushed a commit that referenced this pull request Jun 26, 2026
…k-gates.ts (#5601)

## Summary

- Adds a fifth gate (`prAdvisor`) to `check-gates.ts` that fetches the
PR Review Advisor sticky comment, parses the `recommendation:` field
from its embedded HTML metadata, and sets `allPass: false` when the
value is `blocked`
- Extracts all PRA parsing and provenance logic into a new pure module
`pra-gate.ts` (no shell calls) so it can be unit-tested independently
- Adds unit tests in `test/skills/check-gates-pra.test.ts` covering
trusted comments, spoofed comments, stale head SHA, missing metadata,
all recommendation values, NDJSON parsing, `allPass` propagation, run
provenance validation, and the fail-closed no-trusted-comment path
- Validates comment provenance before trusting any recommendation:
requires `user.login === github-actions[bot]`, verifies `comment_id`
matches the actual GitHub comment id, and verifies `head_sha` matches
the current PR head
- Switches from a blocklist (`!== "blocked"`) to an explicit allowlist
(`{merge_as_is}`); `merge_after_fixes`, `needs_rework`, `blocked`, and
unknown values all fail the gate
- Uses `--jq ".[]"` to emit NDJSON instead of relying on `gh --paginate`
array concatenation, which is ambiguous on multi-page results
- Fails closed on API errors — consistent with the CodeRabbit gate
- Updates `triage.ts` with a comment making it explicit that CodeRabbit
and PRA are both skipped there for performance, and that a `merge-now`
bucket assignment does not mean `check-gates.ts` can be skipped
- Updates `MERGE-GATE.md`: removes the "manual review step" caveat and
documents the gate as automated

## Motivation

PR #5526 was approved despite the PR Review Advisor posting a
**Blocked** status with two required fixes. The advisor check was
documented as "manual" in `MERGE-GATE.md`, making it easy to skip.
Making it a programmatic gate means `allPass` will be `false` on any
blocked advisor comment, preventing the approval flow from proceeding.

## Test evidence

```
$ node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts 5526
{
  "prAdvisor": {
    "pass": false,
    "details": "PR Review Advisor: blocked (2 required item(s))",
    "recommendation": "blocked",
    "openRequired": 2
  },
  "allPass": false
}

$ npx vitest run test/skills/check-gates-pra.test.ts
✓ test/skills/check-gates-pra.test.ts (21 tests)
```

## Test plan

- [ ] Run `check-gates.ts` against a PR where the advisor is blocked —
`prAdvisor.pass` should be `false` and `allPass` should be `false`
- [ ] Run `check-gates.ts` against a PR with no advisor comment —
`prAdvisor.pass` should be `false` (fail-closed)
- [ ] Verify a spoofed comment (non-bot user, mismatched `comment_id`,
or stale `head_sha`) is rejected

Signed-off-by: Preksha Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Preksha Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary

`nemohermes <name> sessions export` aborted with exit 127 because the
wrapper unconditionally invoked the in-sandbox `openclaw sessions list`
regardless of the sandbox's agent kind. The wrapper now reads `sb.agent`
from the registry and routes Hermes sandboxes to `hermes sessions
export`, downloading the resulting single JSONL stream to the host.

## Related Issue

Fixes NVIDIA#5515

## Changes

- `exportSandboxSessions` dispatches on
`registry.getSandbox(name)?.agent`; on `hermes`, runs `umask 077 &&
hermes sessions export <staging> && chmod 600 <staging>` in the sandbox,
downloads the JSONL to the host, and best-effort cleans up the staging
file.
- OpenClaw-only flags (`--agent`, positional keys,
`--include-trajectory`, `--format tar`) raise a clear error on a Hermes
sandbox instead of producing a misleading half-export.
- Host destination on Hermes defaults to `./sessions-<sandbox>.jsonl`;
`--out` picks a different path.
- Command help text and `docs/reference/commands.mdx` document both
branches; the nemohermes docs variant is regenerated.
- Unit + CLI E2E tests cover the Hermes wire calls (`exec sh -c`,
`download`, `rm -f` cleanup), option-rejection, and the manifest shape.

## Type of Change

- [x] 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

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [x] Docs updated for user-facing behavior changes
- [ ] `npm run docs` builds without warnings (doc changes only)
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

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


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Sessions export now routes by sandbox agent kind: OpenClaw exports
per-session JSONL files (optionally with trajectories) or a `.tgz`;
Hermes exports a single JSONL stream produced inside the sandbox.
* Hermes output defaults and option handling are now agent-specific,
with stronger input/flag validation.

* **Documentation**
* Updated `sessions export` reference docs and CLI help to reflect
OpenClaw vs Hermes flows and clarified security/permissions (owner-only
files) plus staging and cleanup behavior.

* **Tests**
* Added/expanded Hermes-focused tests for routing, transfer/rename, and
failure/cleanup scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Refreshes the v0.0.68 docs with release notes, Microsoft Teams messaging
guidance, and generated support-matrix coverage.
Also corrects the Hermes generated command reference so `sessions
export` examples match Hermes-only behavior while keeping the shared
command heading compatible with CLI parity checks.

## Changes
- NVIDIA#5585 -> `docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/platform-support.mdx`: Documents experimental Microsoft
Teams channel setup, Bot Framework credentials, webhook forwarding,
local `MSTEAMS_PORT` conflicts, and the generated integration support
row.
- NVIDIA#5526 -> `docs/reference/commands.mdx`,
`docs/reference/commands-nemohermes.mdx`: Keeps Hermes `sessions export`
examples on the supported single-JSONL export path while preserving the
canonical shared CLI heading.
- NVIDIA#5044 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for installer onboarding failure propagation.
- NVIDIA#5641 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for fresh recovery after pre-sandbox installer interruption.
- NVIDIA#5673 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for explicit deny-all messaging manifests.
- NVIDIA#5743 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for unsupported-agent channel-add rejection.
- NVIDIA#5252 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for Hermes `gateway-token` dashboard guidance.
- NVIDIA#5659 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for local OpenClaw `agent` wrapper help.
- NVIDIA#5661 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for default extra-agent paths.
- NVIDIA#5669 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for Deep Agents Code branding.
- NVIDIA#5672 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for hosted-compatible default model ID preservation.
- NVIDIA#5725 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for Deep Agents Code sandbox liveness.
- NVIDIA#5644 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note
coverage for DGX Spark managed-vLLM express install defaults.
- NVIDIA#5712 -> `docs/about/release-notes.mdx`,
`docs/reference/platform-support.mdx`: Adds v0.0.68 release-note
coverage for the canonical support matrix and updates the matrix source
with Teams.

## Type of Change

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

## Quality Gates
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: docs-only release refresh;
no runtime code changed.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] 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)
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — `npm
run docs` passed with 0 errors; Fern reported the existing light-mode
accent contrast warning.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

Additional verification:
- `python3 scripts/generate-platform-docs.py --check` passed.
- `npm run docs:sync-agent-variants` passed.
- `bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-cli`
passed.
- `npm run docs` passed with 0 errors and one Fern theme warning:
light-mode accent contrast ratio is 2.41:1 and should be at least 3:1.
- `npm run build:cli` refreshed local untracked `dist/` artifacts after
rebase; no tracked files changed.
- `npm run typecheck:cli` passed.
- Normal commit and push hooks passed after the local CLI rebuild.

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added **Microsoft Teams** as an experimental messaging channel,
including manifest-first onboarding, local port routing/conflict
handling, sandbox delivery controls, and policy preset support.

* **Documentation**
* Expanded messaging-channel setup for Teams (prerequisites,
credential/webhook setup, wizard flow, and add/remove commands).
* Updated reference docs for **agent-specific** session export examples
(OpenClaw vs Hermes).
* Refreshed platform support guidance and added the latest release-notes
entry.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…k-gates.ts (NVIDIA#5601)

## Summary

- Adds a fifth gate (`prAdvisor`) to `check-gates.ts` that fetches the
PR Review Advisor sticky comment, parses the `recommendation:` field
from its embedded HTML metadata, and sets `allPass: false` when the
value is `blocked`
- Extracts all PRA parsing and provenance logic into a new pure module
`pra-gate.ts` (no shell calls) so it can be unit-tested independently
- Adds unit tests in `test/skills/check-gates-pra.test.ts` covering
trusted comments, spoofed comments, stale head SHA, missing metadata,
all recommendation values, NDJSON parsing, `allPass` propagation, run
provenance validation, and the fail-closed no-trusted-comment path
- Validates comment provenance before trusting any recommendation:
requires `user.login === github-actions[bot]`, verifies `comment_id`
matches the actual GitHub comment id, and verifies `head_sha` matches
the current PR head
- Switches from a blocklist (`!== "blocked"`) to an explicit allowlist
(`{merge_as_is}`); `merge_after_fixes`, `needs_rework`, `blocked`, and
unknown values all fail the gate
- Uses `--jq ".[]"` to emit NDJSON instead of relying on `gh --paginate`
array concatenation, which is ambiguous on multi-page results
- Fails closed on API errors — consistent with the CodeRabbit gate
- Updates `triage.ts` with a comment making it explicit that CodeRabbit
and PRA are both skipped there for performance, and that a `merge-now`
bucket assignment does not mean `check-gates.ts` can be skipped
- Updates `MERGE-GATE.md`: removes the "manual review step" caveat and
documents the gate as automated

## Motivation

PR NVIDIA#5526 was approved despite the PR Review Advisor posting a
**Blocked** status with two required fixes. The advisor check was
documented as "manual" in `MERGE-GATE.md`, making it easy to skip.
Making it a programmatic gate means `allPass` will be `false` on any
blocked advisor comment, preventing the approval flow from proceeding.

## Test evidence

```
$ node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts 5526
{
  "prAdvisor": {
    "pass": false,
    "details": "PR Review Advisor: blocked (2 required item(s))",
    "recommendation": "blocked",
    "openRequired": 2
  },
  "allPass": false
}

$ npx vitest run test/skills/check-gates-pra.test.ts
✓ test/skills/check-gates-pra.test.ts (21 tests)
```

## Test plan

- [ ] Run `check-gates.ts` against a PR where the advisor is blocked —
`prAdvisor.pass` should be `false` and `allPass` should be `false`
- [ ] Run `check-gates.ts` against a PR with no advisor comment —
`prAdvisor.pass` should be `false` (fail-closed)
- [ ] Verify a spoofed comment (non-bot user, mismatched `comment_id`,
or stale `head_sha`) is rejected

Signed-off-by: Preksha Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Preksha Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior integration: openclaw OpenClaw integration behavior NV QA Bugs found by the NVIDIA QA Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ubuntu 24.04][CLI&UX] nemohermes <name> sessions export fails on Hermes sandbox — exits 1 with exit 127 (command not found inside sandbox)

5 participants