fix(hermes): accept pinned base platform digest#6318
Conversation
Signed-off-by: Chengjie Wang <chengjiew@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 base-image resolution now carries pinned remote ref metadata through sandbox resolution, validates stale pinned hints, and accepts the tracked official pinned digest when Hermes final-image checks receive matching resolved provenance. ChangesHermes pinned base-image resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the branch is 96%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the branch is 73%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
PR Review Advisor (Nemotron Ultra) — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
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. |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/agent/base-image-hermes.test.ts (1)
69-80: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding boundary negative-case coverage.
To pin down the acceptance boundary introduced in
hermesFinalDockerfileAcceptsBase, consider adding two negative cases alongside this one: (1)source: "pinned"with a ref that doesn't match the official Hermes digest pattern (e.g., wrong image name), and (2) a matching official digest pattern with a non-"pinned"source (e.g.,"latest"). Both should still be rejected, and asserting this would guard against future regressions widening the trust boundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/agent/base-image-hermes.test.ts` around lines 69 - 80, The current base-image acceptance test only covers the happy path for hermesFinalDockerfileAcceptsBase, so add two negative assertions in base-image-related tests around ensureAgentBaseImage and resolveSandboxBaseImageMock: one where source is "pinned" but the ref does not match the official Hermes digest pattern, and another where the ref matches the Hermes digest pattern but source is not "pinned" (for example "latest"). Both cases should assert rejection to keep the trust boundary enforced.src/lib/agent/base-image.ts (1)
32-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the new constant to avoid duplicating the digest regex.
getHermesPinnedRemoteBaseRef(line 101, unchanged) hardcodes an identical inline regex literal for the same official-digest pattern now captured inHERMES_OFFICIAL_BASE_DIGEST_REF. Since this PR introduces the shared constant, consolidating avoids future drift between the two patterns.♻️ Proposed fix to reuse the shared constant
if ( !pinnedRef || - !/^ghcr\.io\/nvidia\/nemoclaw\/hermes-sandbox-base@sha256:[0-9a-f]{64}$/.test(pinnedRef) + !HERMES_OFFICIAL_BASE_DIGEST_REF.test(pinnedRef) ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/agent/base-image.ts` around lines 32 - 34, Reuse the shared digest pattern constant instead of duplicating the inline regex in getHermesPinnedRemoteBaseRef. Update the Hermes base-image logic in src/lib/agent/base-image.ts so the official digest check uses HERMES_OFFICIAL_BASE_DIGEST_REF rather than a separate regex literal, keeping the pinned-ref validation aligned with the constant already defined alongside HERMES_MCP_RUNTIME_PROBE_OK.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/agent/base-image-hermes.test.ts`:
- Around line 69-80: The current base-image acceptance test only covers the
happy path for hermesFinalDockerfileAcceptsBase, so add two negative assertions
in base-image-related tests around ensureAgentBaseImage and
resolveSandboxBaseImageMock: one where source is "pinned" but the ref does not
match the official Hermes digest pattern, and another where the ref matches the
Hermes digest pattern but source is not "pinned" (for example "latest"). Both
cases should assert rejection to keep the trust boundary enforced.
In `@src/lib/agent/base-image.ts`:
- Around line 32-34: Reuse the shared digest pattern constant instead of
duplicating the inline regex in getHermesPinnedRemoteBaseRef. Update the Hermes
base-image logic in src/lib/agent/base-image.ts so the official digest check
uses HERMES_OFFICIAL_BASE_DIGEST_REF rather than a separate regex literal,
keeping the pinned-ref validation aligned with the constant already defined
alongside HERMES_MCP_RUNTIME_PROBE_OK.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e43a32e2-ad0e-4c5d-a763-5ea7fdd8b2d8
📒 Files selected for processing (2)
src/lib/agent/base-image-hermes.test.tssrc/lib/agent/base-image.ts
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Maintainer follow-up for the current Advisor evidence request:
This is test/evidence closure only; no production behavior change or registry-risk waiver is needed. |
|
Maintainer accepted-scope decision for current-head The defect and changed crash boundary are narrow. Changed-boundary evidence is now explicit:
The Ready/create/registry ordering is unchanged and excludes a fresh-row leak from this failure:
Therefore a base-image validation exception occurs before OpenShell create and before every fresh registry writer. The issue report also describes clean failure/no registry pollution as observed v0.0.74 reproduction context, not behavior introduced or altered by this patch. For the v0.0.75 RC, I explicitly accept the focused aarch64 resolver/staging regression coverage as sufficient for the changed boundary. The broader onboard→Ready contract and registry ordering are unchanged and already have their canonical full-flow/runtime coverage. No additional full-onboard harness is required in this PR, and |
## Summary Add the v0.0.75 release-notes entry for the release train, summarizing the user-facing fixes merged since v0.0.74. Release-prep docs for the `nemoclaw-maintainer-cut-release-tag` gate. ## Related Issue Release prep for v0.0.75. Remove this section if none. ## Changes - `docs/about/release-notes.mdx`: add the `## v0.0.75` section (themed intro + grouped bullets with source-page links), matching the existing v0.0.74 style. ### Source summary (doc-impacting PRs → doc page) - #6370 -> `docs/about/release-notes.mdx`: prepared-backup recovery restores gateway state and defers the live route check to onboarding, so upgrade recovery no longer fails on an unset gateway route. - #6305 -> `docs/about/release-notes.mdx`: in-place upgrades recover gateway-orphaned sandboxes. - #6332 -> `docs/about/release-notes.mdx`: same-name `--fresh` re-onboard preserves fresh LangChain Deep Agents Code routing. - #6335 -> `docs/about/release-notes.mdx`: custom Anthropic-compatible inference uses the OpenAI frontend. - #6298 -> `docs/about/release-notes.mdx`: OpenAI-only agents keep the `/v1` base URL on Anthropic-compatible endpoints. - #6304 -> `docs/about/release-notes.mdx`: local docker-driver gateway credentials no longer expire. - #6261 -> `docs/about/release-notes.mdx`: Hermes runtime and managed MCP state reconcile after a runtime change. - #6318 -> `docs/about/release-notes.mdx`: Hermes installs accept a pinned base platform digest. - #6291 -> `docs/about/release-notes.mdx`: OpenClaw local CLI pairing restores its previous connection path. Test-performance, CI, and chore commits since v0.0.74 are excluded as non-user-facing. ## Type of Change - [x] Doc only (prose changes, no code sample modifications) ## Quality Gates - [x] Tests not applicable — justification: documentation-only change (release notes prose). - [x] Docs updated for user-facing behavior changes ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] `npm run docs` builds without warnings introduced by this change — command/result: "Found 0 errors and 2 warnings" (the 2 warnings pre-exist this change). - [x] Doc pages follow the style guide (active voice, no numbered/colon titles, correct NVIDIA/NemoClaw/OpenShell capitalization; skip-terms avoided). - [x] No secrets, API keys, or credentials committed --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new **v0.0.75** section to the release notes, highlighting improved sandbox upgrade hardening and prepared-backup recovery, updated inference routing for Anthropic-compatible endpoints, longer-lasting local gateway credential handling, and restored CLI pairing reconnection without re-pairing. Also includes cross-links to related NemoClaw CLI and documentation pages. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
<!-- markdownlint-disable MD041 --> ## Summary Hermes sandbox onboarding now accepts official pinned base-image resolutions when Docker normalizes the pulled image to an official platform manifest digest. This prevents `ensureAgentBaseImage` from rejecting the ghcr.io `hermes-sandbox-base` digest that the pinned resolver path just selected. ## Related Issue Fixes NVIDIA#6313 ## Changes - Updated `src/lib/agent/base-image.ts` so Hermes final-image validation accepts official `hermes-sandbox-base@sha256:*` refs only when they came from the pinned remote resolver source. - Kept local Hermes base allowlisting and explicit/moving-candidate digest checks strict. - Added a regression case in `src/lib/agent/base-image-hermes.test.ts` for a pinned official digest that differs from the Dockerfile ARG after `RepoDigests` normalization. ## 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 <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: internal Hermes base-image validation behavior only; no docs or command syntax changed. - [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: targeted sandbox base-image resolver tests, Hermes base-image tests, CLI build/typecheck, and diff-scoped repository checks passed locally; maintainer review requested via this PR. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested command/result, justification, normal hook evidence, or fallback evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `NPM_CONFIG_CACHE=/tmp/nemoclaw-issue-6313-npm-cache npx vitest run --project cli src/lib/agent/base-image-hermes.test.ts` passed; `NPM_CONFIG_CACHE=/tmp/nemoclaw-issue-6313-npm-cache npx vitest run --project cli src/lib/agent/base-image-hermes.test.ts src/lib/agent/base-image.test.ts src/lib/sandbox-base-image-resolution.test.ts` passed. - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: `NPM_CONFIG_CACHE=/tmp/nemoclaw-issue-6313-npm-cache npm run build:cli` passed; `NPM_CONFIG_CACHE=/tmp/nemoclaw-issue-6313-npm-cache npm run typecheck:cli` passed; `git diff --check` passed. - [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) --- <!-- 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: Chengjie Wang <chengjiew@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Strengthened Hermes base-image acceptance to validate the expected immutable pinned digest format and ensure resolved “pinned” details match. * Improved sandbox base-image resolution to better detect pinned digest refs, propagate `pinnedRemoteRef`, and fail with `pinned_ref_mismatch` when the pinned remote ref is stale or divergent. * Updated compatibility checks to work with either raw image refs or resolved image details. * **Tests** * Expanded Hermes and sandbox base image resolution tests for pinned refs, platform-digest resolver returns, stale pin rejection, and added a dedicated Hermes resolver integration test. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chengjie Wang <chengjiew@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
## Summary Add the v0.0.75 release-notes entry for the release train, summarizing the user-facing fixes merged since v0.0.74. Release-prep docs for the `nemoclaw-maintainer-cut-release-tag` gate. ## Related Issue Release prep for v0.0.75. Remove this section if none. ## Changes - `docs/about/release-notes.mdx`: add the `## v0.0.75` section (themed intro + grouped bullets with source-page links), matching the existing v0.0.74 style. ### Source summary (doc-impacting PRs → doc page) - NVIDIA#6370 -> `docs/about/release-notes.mdx`: prepared-backup recovery restores gateway state and defers the live route check to onboarding, so upgrade recovery no longer fails on an unset gateway route. - NVIDIA#6305 -> `docs/about/release-notes.mdx`: in-place upgrades recover gateway-orphaned sandboxes. - NVIDIA#6332 -> `docs/about/release-notes.mdx`: same-name `--fresh` re-onboard preserves fresh LangChain Deep Agents Code routing. - NVIDIA#6335 -> `docs/about/release-notes.mdx`: custom Anthropic-compatible inference uses the OpenAI frontend. - NVIDIA#6298 -> `docs/about/release-notes.mdx`: OpenAI-only agents keep the `/v1` base URL on Anthropic-compatible endpoints. - NVIDIA#6304 -> `docs/about/release-notes.mdx`: local docker-driver gateway credentials no longer expire. - NVIDIA#6261 -> `docs/about/release-notes.mdx`: Hermes runtime and managed MCP state reconcile after a runtime change. - NVIDIA#6318 -> `docs/about/release-notes.mdx`: Hermes installs accept a pinned base platform digest. - NVIDIA#6291 -> `docs/about/release-notes.mdx`: OpenClaw local CLI pairing restores its previous connection path. Test-performance, CI, and chore commits since v0.0.74 are excluded as non-user-facing. ## Type of Change - [x] Doc only (prose changes, no code sample modifications) ## Quality Gates - [x] Tests not applicable — justification: documentation-only change (release notes prose). - [x] Docs updated for user-facing behavior changes ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] `npm run docs` builds without warnings introduced by this change — command/result: "Found 0 errors and 2 warnings" (the 2 warnings pre-exist this change). - [x] Doc pages follow the style guide (active voice, no numbered/colon titles, correct NVIDIA/NemoClaw/OpenShell capitalization; skip-terms avoided). - [x] No secrets, API keys, or credentials committed --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new **v0.0.75** section to the release notes, highlighting improved sandbox upgrade hardening and prepared-backup recovery, updated inference routing for Anthropic-compatible endpoints, longer-lasting local gateway credential handling, and restored CLI pairing reconnection without re-pairing. Also includes cross-links to related NemoClaw CLI and documentation pages. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Hermes sandbox onboarding now accepts official pinned base-image resolutions when Docker normalizes the pulled image to an official platform manifest digest. This prevents
ensureAgentBaseImagefrom rejecting the ghcr.iohermes-sandbox-basedigest that the pinned resolver path just selected.Related Issue
Fixes #6313
Changes
src/lib/agent/base-image.tsso Hermes final-image validation accepts officialhermes-sandbox-base@sha256:*refs only when they came from the pinned remote resolver source.src/lib/agent/base-image-hermes.test.tsfor a pinned official digest that differs from the Dockerfile ARG afterRepoDigestsnormalization.Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailableNPM_CONFIG_CACHE=/tmp/nemoclaw-issue-6313-npm-cache npx vitest run --project cli src/lib/agent/base-image-hermes.test.tspassed;NPM_CONFIG_CACHE=/tmp/nemoclaw-issue-6313-npm-cache npx vitest run --project cli src/lib/agent/base-image-hermes.test.ts src/lib/agent/base-image.test.ts src/lib/sandbox-base-image-resolution.test.tspassed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:NPM_CONFIG_CACHE=/tmp/nemoclaw-issue-6313-npm-cache npm run build:clipassed;NPM_CONFIG_CACHE=/tmp/nemoclaw-issue-6313-npm-cache npm run typecheck:clipassed;git diff --checkpassed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Chengjie Wang chengjiew@nvidia.com
Summary by CodeRabbit
pinnedRemoteRef, and fail withpinned_ref_mismatchwhen the pinned remote ref is stale or divergent.