feat(cli): add Hermes portable lifecycle authority - #9424
Merged
Conversation
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
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 schema-5 portable lifecycle support now spans onboarding, durable receipt authority, rootless Podman and OpenShell validation, lifecycle fencing, command admission, sandbox operations, status reporting, cleanup, and integration tests. ChangesHermes portable lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟡 Moderate · up to This PR adds Hermes lifecycle authority, but the current head cannot complete onboarding because the authoritative source checkout is rejected due to an invalid Dockerfile permission, and several policy and lifecycle correctness issues could cause incorrect reporting, rejected configurations, or mismatched start/stop behavior. Merge should wait for a repaired commit, required CI classification, and successful requalification. Sequence Diagram(s)sequenceDiagram
participant CLI
participant LifecycleLock
participant HermesLifecycle
participant OpenShell
participant Podman
participant Registry
CLI->>LifecycleLock: acquire lifecycle fence
LifecycleLock->>HermesLifecycle: inspect receipt and command authority
HermesLifecycle->>OpenShell: validate executable and sandbox identity
HermesLifecycle->>Podman: validate socket, executable, container, and health
HermesLifecycle->>Registry: validate lifecycle metadata
HermesLifecycle-->>CLI: return portable lifecycle result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit dbb9d2e in the TypeScript / code-coverage/cliThe overall coverage in commit dbb9d2e in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> # Conflicts: # src/lib/actions/sandbox/destroy-flow.test.ts # src/lib/onboard.ts
prekshivyas
previously requested changes
Aug 18, 2026
prekshivyas
left a comment
prekshivyas
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed commit de6641009a95c550337b14354c1f64437c4c57ee, including all 97 changed files with security and lifecycle files prioritized. The focused Hermes lifecycle suite passed 27 tests, but it does not exercise the production Podman dependency factory or the transitional stop state described in the inline findings.
Security review: FAIL overall. Secrets and credentials: PASS. Input validation: PASS. Authentication and authorization: PASS. Command execution: FAIL because production mutations lack admitted executable authority. Error handling: PASS. Network security: PASS. Cryptography: PASS. Concurrency and resource management: FAIL because stop can return during Podman's transitional state. Dependencies: PASS.
The two inline findings are blocking.
prekshivyas
reviewed
Aug 18, 2026
prekshivyas
reviewed
Aug 18, 2026
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
senthilr-nv
marked this pull request as ready for review
August 18, 2026 15:09
Collaborator
Author
|
Addressed the two blocking security findings from review of de66410 in exact replacement head ffd482c.
Validation on the replacement bytes: 53 focused lifecycle/authority tests and CLI type-check passed; the independent nine-category security re-review passed; the affected suite recorded 10,358 passes, and its two resource-contention timeouts each passed unchanged in isolation (5/5 and 11/11). Exact-head CI and Brev qualification remain in progress, so the PR stays draft. |
senthilr-nv
dismissed
prekshivyas’s stale review
August 18, 2026 15:10
Addressed in exact replacement head ffd482c: admitted canonical Podman executable authority and bounded transitional-stop reconciliation are implemented and independently security-reviewed. PR remains draft pending exact-head CI and Brev qualification.
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
19 tasks
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
20 tasks
20 tasks
45 tasks
19 tasks
prekshivyas
pushed a commit
that referenced
this pull request
Aug 19, 2026
<!-- markdownlint-disable MD041 --> ## Summary This pull request (PR) fixes typed live E2E artifact lookup after semantic test titles were introduced. Registry targets now bind the artifact fixture to their stable target ID. LangChain Deep Agents Code reads base image publication evidence from the directory that the trusted workflow writes and uploads. ## Confirmed E2E Root `typed E2E titles / ArtifactSink root identity / DCode publication evidence written by stable target ID but read from semantic-title slug` - Source workflow: [run 32204372503](https://github.com/NVIDIA/NemoClaw/actions/runs/32204372503), attempt 1, at `ee6762b9941777d64dad832994b03ca2a572d4c9`. - Failed target: [job 95930234625](https://github.com/NVIDIA/NemoClaw/actions/runs/32204372503/job/95930234625), LangChain Deep Agents Code on GitHub Actions. - Failure: phase 1 stopped in 19 ms at `loadDcodeBaseImagePublicationEvidence:103` with `Deep Agents Code GitHub Actions run is missing published base evidence`. No onboarding or runtime phase ran. - The workflow validated the exact candidate checkout, CLI artifact, base image publication index, linux/amd64 child digest, and stripped-base negative import gate. Sanitization, evidence upload, Docker authentication cleanup, and workspace cleanup passed. PR #9514, merged as `1acc902896e6324f773df9dbcc32a761118c6f05`, changed typed live test titles from a stable target ID to `<target-id>: <semantic test title>`. The workflow continued to write `dcode-base-image.json` below `${TARGET_ID}`. The E2E artifact fixture derived its directory from the complete semantic test title. ## Changes - Add typed `e2eArtifactRootId` test metadata. The stateful E2E artifact fixture uses it before the existing `task.name` fallback. - Bind both supported and skipped registry target registrations to the already validated `target.id`. - Reuse one Deep Agents Code base image publication evidence fixture across the parser tests and artifact-root regression test. - Keep one nested Vitest regression test. It writes evidence below the stable target ID, asserts that ID as the artifact-root basename, and confirms no directory is derived from the semantic test title. - Leave `createArtifactSink`, the workflow fixture, workflow publication and upload paths, credentials, redaction, and cleanup unchanged. ## Base SHA Reconciliation Latest PR commit `69e46712823e50d0d009e8300f91ee519098649d` is a normal GitHub-Verified merge with ordered parents [`a5cbade3e7d375c14a515d9ff6950e4a7af0e647`, `7afe39541e81f70d9e1aa39c49415084d8276524`]. PR base SHA `7afe39541e81f70d9e1aa39c49415084d8276524` adds #9551, #9434, #9564, and #9566 after previous base SHA `cc45d243dcc256aba7b8d6a761c75d771148ead5`. None changes the six files in this PR, `ArtifactSink`, or trusted E2E workflow files. #9566 changes only `test/package-contract/cli/credentials-cli-command.test.ts` and corrects the inherited provider-reservation assertion that caused pre-reconciliation `build-typecheck` to fail. The base-composition tests below continue to exercise the #9424 shared onboarding paths. The net PR diff contains six files: the stateful E2E fixture, registry target test, two E2E-support tests, and two E2E-support fixtures. ## 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) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent exact-69 correctness review, nine-category security review, and documentation writer review passed. Exact-69 CodeRabbit and PR Review Advisor checks passed; maintainer approval remains pending. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Security and Documentation Review - Independent nine-category security review of latest PR commit `69e46712823e50d0d009e8300f91ee519098649d` passed. Stable target identity flows from `target.id` through `e2eArtifactRootId` to the existing `ArtifactSink`. The regression test writes publication evidence only below the stable target ID. It confirms that the stateful fixture selects that artifact root and does not create a directory from the semantic test title. - Exact-69 [PR Review Advisor run 32214387059](https://github.com/NVIDIA/NemoClaw/actions/runs/32214387059) completed successfully with both model lanes and the publisher. CodeRabbit status on `69e46712823e50d0d009e8300f91ee519098649d` is successful and produced no new actionable comment. - No documentation change is required. The existing E2E guides already define stable target IDs as artifact identities, `e2e-artifacts/live/<target-id>` as the standard layout, and the semantic suffix as display text. - The blocking [LOC Reduction / Codebase Simplicity Review](#9562 (comment)) is addressed in `a5cbade3e7d375c14a515d9ff6950e4a7af0e647`: the parser and artifact-root tests now share one publication evidence fixture, and the duplicated second nested Vitest process was removed. ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this PR does not change `scripts/prepare-dgx-station-host.sh`. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub — GitHub reports all four PR commits as Verified, and [exact-69 DCO job 95953058384](https://github.com/NVIDIA/NemoClaw/actions/runs/32214389535/job/95953058384) passed. - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable — `npm run validate:pr` passed on `69e46712823e50d0d009e8300f91ee519098649d` after reconciliation to base `7afe39541e81f70d9e1aa39c49415084d8276524`. - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: - Before the fixture correction, the regression test was added to a working tree based on base SHA `164cb284fb1efebf3b038beffed4de9870543c44`. `npm exec -- vitest run --project e2e-support test/e2e/support/e2e-artifact-root.test.ts` failed 1/1 with `Deep Agents Code GitHub Actions run is missing published base evidence`. - On latest PR commit `69e46712823e50d0d009e8300f91ee519098649d`, the focused E2E-support command below passed 108/108 tests: ```shell npm exec -- vitest run --project e2e-support \ test/e2e/support/e2e-artifact-root.test.ts \ test/e2e/support/e2e-fixture-context.test.ts \ test/e2e/support/dcode-base-image-runtime-evidence.test.ts \ test/e2e/support/base-image-publication-workflow-boundary.test.ts \ test/e2e/support/e2e-live-skip-name-contract.test.ts \ test/e2e/support/e2e-live-registry-discovery.test.ts \ test/e2e/support/e2e-registry.test.ts \ test/e2e/support/upload-e2e-artifacts-workflow-boundary.test.ts ``` - The PR-base-bound command below passed 498/498 selected tests, with 8 expected skips: ```shell npm exec -- vitest run --changed=7afe39541e81f70d9e1aa39c49415084d8276524 \ --project cli --project plugin --project e2e-support ``` - On `69e46712823e50d0d009e8300f91ee519098649d`, after `npm run build:cli`, `npm exec -- vitest run --project package-contract test/package-contract/cli/credentials-cli-command.test.ts --testTimeout=30000` did not pass: 15/25 tests passed and 10/25 failed before the expected mocked CLI calls because this macOS checkout could not revalidate gateway lifecycle authority. stderr also reported missing development packages `@oclif/plugin-help` and `@oclif/plugin-plugins` from the shared host `node_modules`; the changed rollback case recorded no lifecycle calls. Exact-69 Linux [`build-typecheck` job 95953099102](https://github.com/NVIDIA/NemoClaw/actions/runs/32214389601/job/95953099102) passed. - `npm run test:e2e-phases:check` passed with 131 semantic E2E phase plans across 86 files. - The grouped 15-file CLI base-composition command below did not pass: 14 files and 274 tests passed, while two tests in `src/commands/credentials.test.ts` hit the existing 5-second timeout under concurrent load: ```shell npm exec -- vitest run --project cli \ src/lib/onboard/experimental/hermes-portable-contract.test.ts \ src/lib/onboard/experimental/hermes-portable-lifecycle.test.ts \ src/lib/onboard/experimental/hermes-portable-podman-authority.test.ts \ src/lib/onboard/experimental/hermes-portable-policy-authority.test.ts \ src/lib/onboard/experimental/hermes-portable-receipt.test.ts \ src/lib/onboard/experimental/portable-agent-lifecycle.test.ts \ src/lib/onboard/managed-workload/onboard-orchestration.test.ts \ src/lib/onboard/created-sandbox-finalization.test.ts \ src/lib/actions/uninstall/run-plan-nvm-leftovers.test.ts \ src/lib/actions/uninstall/run-plan.test.ts \ src/commands/credentials.test.ts \ src/lib/actions/global.test.ts \ src/lib/actions/sandbox/mcp-bridge-input-targets.test.ts \ src/lib/actions/sandbox/mcp-bridge-provider.test.ts \ src/lib/state/registry-normalization.test.ts ``` - The isolated credentials command then passed 7/7: ```shell npm exec -- vitest run --project cli src/commands/credentials.test.ts ``` - This MCP integration command passed 93/93 tests across five files: ```shell npm exec -- vitest run --project integration \ test/cli/credentials-command.test.ts \ test/mcp-add-crash-consistency.test.ts \ test/mcp-destroy-lifecycle.test.ts \ test/mcp-policy-key-ownership.test.ts \ test/mcp-restart-policy-order.test.ts ``` - Fresh pre-commit, commit-msg, and pre-push hooks passed before reconciliation. `npm run validate:pr` passed after reconciliation. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: on pre-reconciliation commit `ba8560a78551a9c40a5fe00fb1ddf4db7443cb1f`, `npm exec -- vitest run --project e2e-support` did not pass locally: 2,983 tests passed, 38 skipped, and 42 failed. The failures reported host-wide subprocess contention or a macOS/GNU `find` mismatch. The focused and changed-test commands passed. Pre-reconciliation [build-typecheck job 95950501960](https://github.com/NVIDIA/NemoClaw/actions/runs/32213454369/job/95950501960) and [exact-base main job 95940499627](https://github.com/NVIDIA/NemoClaw/actions/runs/32209943161/job/95940499627) failed the stale provider-reservation assertion. #9566 corrected that package contract on base `7afe39541e81f70d9e1aa39c49415084d8276524`. Exact-69 [build-typecheck job 95953099102](https://github.com/NVIDIA/NemoClaw/actions/runs/32214389601/job/95953099102) passed and supersedes both stale failures; remaining exact-69 CI is pending. - [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) - [ ] 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) No live E2E workflow was dispatched for this PR. --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added end-to-end coverage for resolving publication evidence from stable target-based artifact directories. * Added validation that the stateful fixture selects the artifact root for the stable target ID and does not create a directory from the semantic test title. * Reused one publication-evidence fixture across the existing platform-reference, image-index, stale-candidate, and metadata-validation tests. * Added deterministic publication-evidence fixtures and metadata support for associating end-to-end artifacts with stable target identifiers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
This was referenced Aug 19, 2026
8 tasks
This was referenced Aug 19, 2026
20 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds an incremental Hermes Portable lifecycle authority under Epic #9211. It introduces schema-5 pending, configuring, and active receipts for receipt-owned Podman onboarding, start, stop, connect, probe, and recovery. Schema-4 OpenClaw and receiptless Docker behavior remain unchanged.
Related Issue
Part of #9203 under Epic #9211. This PR does not close #9203.
Changes
Readyverification, sandbox registry publication, and active receipt publication.sandbox:statusandsandbox:doctorcommands. Reject deferred mutation commands before effects.personal-open-internet, and bind the effective policy through scoped OpenShell policy evidence and sandbox registry readback.Scope and qualification status
ffd482c91b2e1cc3223309441db4f56590a3ad86. The review closure comment records the two repaired findings.no-docs-neededbecause this PR does not change the supported documentation surface.agents/hermes/Dockerfilehad mode0600. A candidate repair and requalification of the next latest PR commit are required.The PR is open for review. It is not approval-ready until a replacement latest PR commit passes required CI and the fresh Brev qualification.
Type of Change
Quality Gates
ffd482c91b2e1cc3223309441db4f56590a3ad86; closure evidenceDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passednpm run typecheck:clipassednpm run docsbuilds without warnings (doc changes only)Signed-off-by: Senthil Ravichandran senthilr@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests