Skip to content

fix(hermes): remove uv build cache metadata - #7948

Closed
senthilr-nv wants to merge 1 commit into
mainfrom
codex/cleanup-nspect-git-metadata
Closed

senthilr-nv wants to merge 1 commit into
mainfrom
codex/cleanup-nspect-git-metadata

Conversation

@senthilr-nv

@senthilr-nv senthilr-nv commented Jul 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Hermes image builds can leave uv's build-only root cache in the published image even when uv runs with --no-cache. This change removes the cache after the final uv command and makes both image gates reject it.

Changes

  • Remove /root/.cache/uv after the final build-time uv command.
  • Reject the cache path in the Hermes base-image and final-image gates.
  • Verify the cache remains absent in the Hermes live E2E runtime image.

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)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: This change removes build-only cache metadata and does not change a user-visible surface.
  • 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: Maintainer security review passed all nine categories with no findings. The change removes a root-owned build cache and adds no input, dependency, credential, or privilege surface.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: The change removes /root/.cache/uv after the final build-time uv command and verifies that the path is absent from the base image, final image, and live E2E runtime image. No user-visible surface changed.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • 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
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npm run test:e2e-phases:check passed for 116 tests across 73 files; npx vitest run --project integration test/node-tar-dockerfile-contract.test.ts passed 13/13; Hadolint passed for both changed Dockerfiles.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • 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: Senthil Ravichandran senthilr@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved runtime image hygiene by preventing temporary uv build caches from remaining in published images.
    • Added safeguards to ensure build-only cache paths are removed and cannot appear as symlinks.
  • Tests

    • Expanded image verification checks to detect unintended uv cache remnants.

@senthilr-nv senthilr-nv added integration: hermes Hermes integration behavior platform: container Affects Docker, containerd, Podman, or images security labels Jul 30, 2026
@senthilr-nv senthilr-nv self-assigned this Jul 30, 2026
@coderabbitai

coderabbitai Bot commented Jul 30, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7e30b6ae-5cac-4aa5-91a4-9e99bbfd4560

📥 Commits

Reviewing files that changed from the base of the PR and between ef32617 and 22ec39e.

📒 Files selected for processing (3)
  • agents/hermes/Dockerfile
  • agents/hermes/Dockerfile.base
  • test/e2e/live/hermes-root-entrypoint-smoke.test.ts

📝 Walkthrough

Walkthrough

Changes

uv cache hygiene

Layer / File(s) Summary
Build cache cleanup and base-image validation
agents/hermes/Dockerfile.base
The base image documents and performs uv cache cleanup after dependency verification, then rejects /root/.cache/uv during build-only path scanning.
Final image and runtime validation
agents/hermes/Dockerfile, test/e2e/live/hermes-root-entrypoint-smoke.test.ts
Final image checks and runtime smoke tests verify that /root/.cache/uv is absent and not a symlink.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested labels: area: security, area: sandbox, area: e2e, bug-fix

Suggested reviewers: cv, jyaunches

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: removing Hermes uv build cache metadata from published images.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 codex/cleanup-nspect-git-metadata

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

@github-code-quality

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 22ec39e in the codex/cleanup-nspect... branch remains at 96%, unchanged from commit ef32617 in the main branch.

@senthilr-nv

Copy link
Copy Markdown
Collaborator Author

Superseded by #7949 with the same verified commit and a clearer branch name.

@senthilr-nv
senthilr-nv deleted the codex/cleanup-nspect-git-metadata branch July 30, 2026 22:31
@github-actions

github-actions Bot commented Jul 30, 2026 •

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections match; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-inference, cloud-onboard, full-e2e, hermes-e2e, security-posture

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@coderabbitai coderabbitai Bot mentioned this pull request Jul 30, 2026
11 of 23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration: hermes Hermes integration behavior platform: container Affects Docker, containerd, Podman, or images security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant