fix(hermes): preserve home channels across rebuilds - #7817
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHermes backups now capture allowlisted home-channel assignments and validate them in rebuild manifests. Recreate flows merge those assignments into the Hermes messaging plan, while documentation describes ChangesHermes home-channel preservation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant HermesSandbox
participant BackupState
participant RebuildPhase
participant MessagingSetup
HermesSandbox->>BackupState: capture allowlisted home-channel assignments
BackupState->>RebuildPhase: provide preservedEnv manifest entries
RebuildPhase->>MessagingSetup: write merged messaging plan
MessagingSetup->>HermesSandbox: apply preserved home-channel env-lines
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 52461d0 in the TypeScript / code-coverage/cliThe overall coverage in commit 52461d0 in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-7817.docs.buildwithfern.com/nemoclaw |
Signed-off-by: San Dang <sdang@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/manage-sandboxes/backup-restore.mdx`:
- Around line 146-153: Reword the sentence in the AgentOnly Hermes guidance from
“A current generated assignment overrides…” to “A newly generated assignment
overrides…” in docs/manage-sandboxes/backup-restore.mdx lines 146-153 and
docs/manage-sandboxes/workspace-files.mdx lines 124-149, preserving the rest of
both sections unchanged.
In `@src/lib/state/preserved-env/index.test.ts`:
- Around line 113-158: Remove the conditional throw in the test at
src/lib/state/preserved-env/index.test.ts lines 113-158 by using a
non-conditional assertion or typed helper to narrow currentRender to an
env-lines render. Also replace the planArg conditional throw in
src/lib/onboard/dockerfile-patch-preserved-env.test.ts lines 111-124 with a
non-conditional defined assertion and safe narrowing; both tests must retain
their existing validation while adding no if statements.
🪄 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: 0c53483c-0669-4427-be45-0e9396940bf8
📒 Files selected for processing (13)
agents/hermes/manifest.yamldocs/manage-sandboxes/backup-restore.mdxdocs/manage-sandboxes/messaging-channels.mdxdocs/manage-sandboxes/workspace-files.mdxsrc/lib/actions/sandbox/rebuild-recreate-observability.test.tssrc/lib/actions/sandbox/rebuild-recreate-phase.tssrc/lib/onboard/dockerfile-patch-preserved-env.test.tssrc/lib/onboard/dockerfile-patch.test.tssrc/lib/onboard/dockerfile-patch.tssrc/lib/state/preserved-env/index.test.tssrc/lib/state/preserved-env/index.tssrc/lib/state/sandbox.tstest/hermes-home-channel-snapshot.test.ts
💤 Files with no reviewable changes (1)
- src/lib/onboard/dockerfile-patch.test.ts
Signed-off-by: San Dang <sdang@nvidia.com>
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
4 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
5 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
cjagwani
left a comment
There was a problem hiding this comment.
Maintainer security review — exact head 598d45a99809cec0565c4b34f4a011254d1a489a against base 41d8d55f9f0632a69e07cbc172963543fe01bff1: PASS. No security findings.
- Secrets and credentials — PASS. Capture is limited to exact
*_HOME_CHANNEL,*_HOME_CHANNEL_NAME, and*_HOME_CHANNEL_THREAD_IDsuffixes. Credential keys such asSLACK_BOT_TOKENare rejected at capture, manifest-validation, and merge boundaries. - Input validation and sanitization — PASS. The parser bounds source and assignment sizes, rejects duplicates and control characters, validates keys and the single
.envinventory path, and rejects newline injection and unrecognized assignments. - Authentication and authorization — PASS. No auth provider, token, pairing, role, or permission behavior changes. Preserved data is non-secret routing metadata.
- Dependencies and third-party code — PASS. No dependency, lockfile, registry, image-source, or license change.
- Error handling and logging — PASS. Capture and validation failures fail the backup/rebuild safely; logs report paths and counts without values. A finalization failure leaves the existing sandbox untouched.
- Cryptography and data protection — PASS / not applicable. No cryptographic behavior changes. The preserved plan is canonical JSON encoded as base64 and remains inside the existing rebuild artifact boundary.
- Configuration and infrastructure — PASS. Current manifest renders override preserved values; inactive/non-Hermes plans do not receive the preserved render; the staged Dockerfile is re-fingerprinted after finalization.
- Security testing — PASS. Tests cover secret exclusion, duplicate and malformed assignments, newline injection, precedence, inactive channels, image finalization, pre-delete validation, snapshot capture, and rebuild handoff. Focused validation passed 151 tests; plugin build and
git diff --checkpassed. - Holistic security posture — PASS. The replacement image is finalized and verified before sandbox deletion, preserving the existing fail-closed destructive boundary. No network-policy, capability, filesystem-permission, SSRF, or credential-isolation boundary is weakened.
Files reviewed: all 28 changed files, with deep review of src/lib/state/preserved-env/index.ts, src/lib/state/sandbox.ts, src/lib/actions/sandbox/rebuild-custom-image-preflight.ts, src/lib/actions/sandbox/rebuild-pipeline.ts, src/lib/actions/sandbox/rebuild-recreate-phase.ts, src/lib/onboard/dockerfile-patch.ts, and their focused tests and documentation.
|
Exact-head review remains approved at |
|
Correction to my prior handoff: conflict-free base refreshes are explicitly waived. Please do not merge main solely for base currency; preserving exact-head CI/E2E and documentation receipts is preferred unless GitHub reports a real conflict or reviewed behavior requires a change. The existing approval/review evidence remains in force, and I will continue monitoring mergeability. This is a plain coordination comment, not Changes Requested. |
|
Exact-head follow-up for |
|
Exact-head babysitting update for The deterministic gate is now blocked only because |
Superseded by current head 05c866c; exact-head follow-up is waiting on required E2E backfill. No Changes Requested review is being submitted.
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
cjagwani
left a comment
There was a problem hiding this comment.
Reviewed exact head 52461d055727e241f5fb274a6a5f6af15ec1a151. Product scope is established by accepted regression #7803. Required CI, E2E, DCO, commit verification, and automated review gates are green. Security review is clean: preservation is restricted to validated Hermes home-channel keys, credential assignments are excluded, and the retained image context is revalidated before deletion.
|
Post-merge maintainer audit for head |
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical dated `v0.0.101` changelog entry that was missing when the release tag was cut. This post-release recovery records the shipped behavior on current `main` without changing or replacing the existing tag. ## Changes - Add `docs/changelog/2026-08-03.mdx` with the exact `## v0.0.101` heading, release summary, detailed behavior changes, support boundaries, and links to durable documentation. - [#7317](#7317) -> `docs/changelog/2026-08-03.mdx`: Records experimental OpenClaw Google Chat support and its restricted credential and webhook boundary. - [#7715](#7715) -> `docs/changelog/2026-08-03.mdx`: Records strict onboarding recovery state and authoritative resume identity. - [#7749](#7749) -> `docs/changelog/2026-08-03.mdx`: Records the provider-neutral policy seam and unchanged runtime support boundary. - [#7817](#7817) -> `docs/changelog/2026-08-03.mdx`: Records preserved Hermes home-channel assignments across rebuilds. - [#7820](#7820) -> `docs/changelog/2026-08-03.mdx`: Records the SSH-session status field correction. - [#7847](#7847) -> `docs/changelog/2026-08-03.mdx`: Records fail-closed credential filtering for migration and rebuild backups. - [#7870](#7870) -> `docs/changelog/2026-08-03.mdx`: Records sandbox-qualified in-sandbox host command hints. - [#7875](#7875) -> `docs/changelog/2026-08-03.mdx`: Records Microsoft Teams stop and start E2E coverage. - [#7885](#7885) -> `docs/changelog/2026-08-03.mdx`: Records Hermes managed gateway detection in status. - [#7889](#7889) -> `docs/changelog/2026-08-03.mdx`: Records policy-authenticated HTTPS Pin Runtime route revocation. - [#7891](#7891) -> `docs/changelog/2026-08-03.mdx`: Records default fallback for negative timeout and polling overrides. - [#7993](#7993) -> `docs/changelog/2026-08-03.mdx`: Records correct sibling detection during uninstall. - [#7995](#7995) -> `docs/changelog/2026-08-03.mdx`: Records absent configuration-hash handling before shields lock. - [#8001](#8001) -> `docs/changelog/2026-08-03.mdx`: Records the dormant atomic managed workload replacement foundation. - [#8029](#8029) -> `docs/changelog/2026-08-03.mdx`: Records repository terminology review in PR Review Advisor. - [#8031](#8031) -> `docs/changelog/2026-08-03.mdx`: Records provider-neutral managed snapshot authority. - [#8032](#8032) -> `docs/changelog/2026-08-03.mdx`: Records immutable managed clone handoff contracts. - [#8034](#8034) -> `docs/changelog/2026-08-03.mdx`: Records the dormant provider-owned clone transaction surface. - [#8035](#8035) -> `docs/changelog/2026-08-03.mdx`: Records the dormant Hermes managed clone broker boundary. - [#8036](#8036) -> `docs/changelog/2026-08-03.mdx`: Records the dormant transactional managed bootstrap boundary. - [#8037](#8037) -> `docs/changelog/2026-08-03.mdx`: Records dormant Docker bootstrap primitives and the unchanged provider support boundary. - [#8070](#8070) -> `docs/changelog/2026-08-03.mdx`: Records consolidated sandbox resource-limit E2E coverage. - [#8071](#8071) -> `docs/changelog/2026-08-03.mdx`: Records escaped and bounded CLI validation diagnostics. - [#8081](#8081) -> `docs/changelog/2026-08-03.mdx`: Records bounded linear snapshot Base64 validation. - [#8085](#8085) -> `docs/changelog/2026-08-03.mdx`: Records commit-bound workflow approval for eligible same-repository maintainers. - [#8088](#8088) -> `docs/changelog/2026-08-03.mdx`: Records Hermes managed-policy E2E selection. - [#8090](#8090) -> `docs/changelog/2026-08-03.mdx`: Records pinned CI search-tool provisioning. - [#8106](#8106) -> `docs/changelog/2026-08-03.mdx`: Records fallback from failed managed OpenShell gateway startup. - [#8107](#8107) -> `docs/changelog/2026-08-03.mdx`: Records Hermes adapter lifecycle E2E selection. - [#8128](#8128) -> `docs/changelog/2026-08-03.mdx`: Records the dormant transactional Docker bootstrap adapter and rollback authority. - [#8140](#8140) -> `docs/changelog/2026-08-03.mdx`: Records Slack conflict scope across independent OpenShell gateways. - [#8147](#8147) -> `docs/changelog/2026-08-03.mdx`: Records completion of durable v0.0.100 documentation audit follow-ups. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This documentation-only recovery does not change executable behavior. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Independently reviewed `docs/changelog/2026-08-03.mdx` at commit `0bebe1f568e3dc85cf410aac1dfb8f8830070b85`. Its blob is `82887920f9720eafd75db6b2271c35f7477edb9b`. The entry follows the writing guide, controlled terminology, changelog structure, MDX SPDX format, literal CLI-name rule, and root-absolute route requirements. It accurately records the `v0.0.100...v0.0.101` release range, Announcement #8162, accepted scope boundaries, and shipped security behavior. There are no code samples. Focused changelog tests and the documentation build pass for this commit. - Agent: Codex Desktop independent documentation writer <!-- docs-review-head-sha: 0bebe1f --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## Security Review - Result: `PASS` - Reviewed commit: `0bebe1f568e3dc85cf410aac1dfb8f8830070b85` - Base commit: `643a4ab8b5f583d8555192a37927268b26022c51` - Findings: None. - Secrets and credentials: `PASS`. No credential values or secret files are present. - Input validation and data sanitization: `PASS`. No executable input path changes. - Authentication and authorization: `PASS`. No identity or permission logic changes. - Dependencies and third-party libraries: `PASS`. No dependency changes. - Error handling and logging: `PASS`. No runtime path changes; diagnostic-security claims are precise. - Cryptography and data protection: `PASS`. No implementation changes. - Configuration and security controls: `PASS`. No configuration, container, port, or HTTP changes. - Security testing: `PASS`. No coverage is removed; the entry records shipped test and security behavior. - System security: `PASS`. No runtime control changes; dormant and non-activation boundaries are explicit. - Agent: Codex Desktop independent security reviewer ## Verification - [ ] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub — verification is pending after commit `0bebe1f568e3dc85cf410aac1dfb8f8830070b85` is pushed. - [ ] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable — commit hooks passed; pre-push is pending. - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — tests are not applicable to this documentation-only recovery. - [x] Applicable broad gate passed — not applicable to this documentation-only recovery. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, credentials, or private keys are added by this diff. - [ ] `npm run docs` builds without warnings (doc changes only) — GitHub documentation checks are pending. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) — independent documentation review passed. - [x] New doc pages include SPDX header and frontmatter (new pages only) — the native changelog entry uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. GitHub CI is authoritative. Focused changelog tests and `npm run docs` passed after the merge refresh. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added experimental Google Chat support. * Improved runtime and session status visibility. * Added onboarding recovery and persistence safeguards. * Added snapshot validation and dormant managed-workload support. * **Bug Fixes** * Improved backup sanitization, route handling, and gateway reliability. * **Documentation** * Added the v0.0.101 changelog and related updates. * **Tests** * Expanded end-to-end coverage and strengthened trusted CI validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
Summary
Hermes rebuilds previously regenerated
.envwithout retaining home-channel routing written by/sethome.This change preserves the approved home-channel assignments across rebuild while continuing to regenerate credentials and all other environment values.
Related Issue
Fixes #7803
Changes
*_HOME_CHANNEL,*_HOME_CHANNEL_NAME, and*_HOME_CHANNEL_THREAD_ID.The Hermes rebuild flow needs a selective migration because copying the complete
.envwould also copy credentials.src/lib/state/preserved-env/index.test.tsprotects wildcard matching, validation, precedence, and inactive-channel behavior.The finalized context is fingerprinted again, and current generated assignments remain authoritative when the same key exists.
Snapshot, Dockerfile-patch, retained-image, and rebuild-flow tests protect capture, image encoding, pre-delete validation, recreate handoff, and disposable-image cleanup.
/sethomerebuild-persistence contract.Type of Change
Quality Gates
Documentation Writer Review
docs-updateda2883aa1f; reviewed the writing rules and documentation style fordocs/manage-sandboxes/messaging-channels.mdx. The final PR documentation adds only the Hermes/sethomerebuild-persistence contract. Source and focused tests confirm the claim, and generated Hermes and OpenClaw variants render with the correct agent scope.npm run docs:sync-agent-variants,npx vitest run test/changelog-docs.test.ts(6/6),npm run docs(0 errors; 2 known unrelated warnings), andgit diff --checkpassed.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablemainmerge,npx vitest run --project cli src/lib/actions/sandbox/rebuild-flow.test.tspassed 111/111;npm run build:cli,git diff --check, and normal pre-commit, commit-msg, and pre-push hooks passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit
*_HOME_CHANNEL*) during backup/restore./sethomeinstead of editing generated files.