perf(ci): reuse trusted registry build caches - #6891
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughBase-image workflows now publish registry-backed Buildx caches and track additional image inputs. E2E workflows configure and route dedicated builders, warm current caches, and validate cache wiring, action pins, environment propagation, target scoping, and execution order. ChangesBuildx cache publication and input tracking
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant E2EWorkflow
participant Buildx
participant DockerBuild
participant BoundaryValidator
participant E2ETest
E2ETest->>E2EWorkflow: Load workflow configuration
E2EWorkflow->>Buildx: Configure dedicated builder
E2EWorkflow->>DockerBuild: Warm current base-image registry cache
E2EWorkflow->>DockerBuild: Run routed Docker build
E2ETest->>BoundaryValidator: Validate cache wiring and step order
BoundaryValidator-->>E2ETest: Return boundary validation results
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / medium confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 4 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/dcode-base-image-workflow.test.ts (1)
73-86: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
ghaCacheEntriesassumestype=ghais always the first CSV field.
.split(",").slice(1)drops whichever field is first, not thetype=ghatoken specifically. If a cache string ever placestype=ghain a non-leading position (e.g.scope=x,type=gha), the realscope=/mode=field would be dropped instead, and the validator would falsely flag a valid config. Currently safe only because every cache string in the workflows putstype=ghafirst.♻️ Proposed fix: filter out the `type=gha` token explicitly instead of assuming position
.map((entry) => Object.fromEntries( entry .split(",") - .slice(1) + .filter((field) => field !== "type=gha") .map((field) => field.split("=", 2) as [string, string]), ), );🤖 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 `@test/dcode-base-image-workflow.test.ts` around lines 73 - 86, Update ghaCacheEntries to remove the type=gha field explicitly before constructing the entry object, rather than dropping the first CSV field with slice(1). Preserve all other fields, including scope and mode, regardless of their position.
🤖 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/e2e/support/e2e-workflow.test.ts`:
- Around line 39-65: Clone the four steps selected by setupBuildx, routeBuilds,
warmCurrent, and warmOld before mutating their with/env fields, then replace the
corresponding entries in rebuild-hermes.steps with those clones. Keep the
synthetic drift isolated to rebuild-hermes so shared steps used by
rebuild-hermes-stale-base remain unchanged.
---
Nitpick comments:
In `@test/dcode-base-image-workflow.test.ts`:
- Around line 73-86: Update ghaCacheEntries to remove the type=gha field
explicitly before constructing the entry object, rather than dropping the first
CSV field with slice(1). Preserve all other fields, including scope and mode,
regardless of their position.
🪄 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: 8e33d1e0-04de-47cb-a2d1-4d5bd955b905
📒 Files selected for processing (5)
.github/workflows/base-image.yaml.github/workflows/e2e.yamltest/dcode-base-image-workflow.test.tstest/e2e/support/e2e-workflow.test.tstools/e2e/workflow-boundary.mts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
There was a problem hiding this comment.
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/e2e/support/e2e-workflow.test.ts`:
- Around line 55-61: Update cloneStep to validate the result of steps.findIndex
before accessing or cloning steps[index]. When the named step is absent, throw a
descriptive error identifying the missing step; only perform structuredClone and
replacement after the index is confirmed valid.
🪄 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: a89fff99-d9ff-4fca-b8a6-154190ad8dc5
📒 Files selected for processing (2)
test/dcode-base-image-workflow.test.tstest/e2e/support/e2e-workflow.test.ts
wscurran
added
area: ci
cv
changed the title
Contributor
There was a problem hiding this comment.
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 `@tools/e2e/workflow-boundary.mts`:
- Around line 3941-4001: Remove the redundant cache-to/push validation from the
warmDcodeInputs block in the DCode workflow checks. Keep
requireReadOnlyBuildCacheImports(errors, steps, "live DCode cache warm") as the
sole validation for job-local PR-controlled layers, while preserving the
existing builder, context, file, and cache-from checks.
🪄 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: db5b053c-5e6e-473f-8a96-a81946162bb1
📒 Files selected for processing (13)
.github/workflows/base-image.yaml.github/workflows/e2e.yamlsrc/lib/onboard/sandbox-prebuild.test.tssrc/lib/onboard/sandbox-prebuild.tssrc/lib/sandbox-base-image/resolution-key.test.tssrc/lib/sandbox-base-image/source-identity.test.tssrc/lib/sandbox-base-image/source-identity.tstest/dcode-base-image-workflow.test.tstest/e2e/fixtures/availability-env.tstest/e2e/support/e2e-workflow.test.tstest/e2e/support/hosted-inference.test.tstest/install-openshell-version-check.test.tstools/e2e/workflow-boundary.mts
Collaborator
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
This was referenced Jul 15, 2026
cv
added a commit
that referenced
this pull request
Jul 15, 2026
<!-- markdownlint-disable MD041 --> ## Summary Repair the rebuild E2E cache regressions introduced by #6891. The rebuild jobs now keep their sequential production Docker builds on the Docker engine cache; trusted base-image publisher jobs continue to import and export their registry caches unchanged. ## Changes - Remove the separate Buildx setup and base-cache warm from `rebuild-openclaw`, `rebuild-hermes`, and `rebuild-hermes-stale-base`. Cache metadata created in the isolated container builder is not available to the later Docker engine builds, so these steps only add work. - Keep the DCode profile-import gate on the Docker engine. Its three negative builds communicate through temporary local image tags that are unavailable to a containerized builder. - Remove the now-unused `BUILDX_BUILDER` propagation through fixture and sandbox-prebuild child environments. - Preserve the trusted base-image publisher cache imports/exports, provenance, permissions, and registry ownership from #6891. - Add workflow-boundary mutations that reject separate Buildx setup/cache-warm actions and persistent builder selection in rebuild and DCode jobs. Runtime measurements confirmed the split: - DCode passed end to end on the Docker engine in [run 29378936022](https://github.com/NVIDIA/NemoClaw/actions/runs/29378936022). Its target job took 8m37s versus an 8m48s pre-change average (11s faster, effectively unchanged). - An isolated OpenClaw warm plus Docker-engine live test took 9m53s in [run 29379468825](https://github.com/NVIDIA/NemoClaw/actions/runs/29379468825), versus a 9m06s pre-change live-test average. The warm cost 34s, while the current-base build was still 41s with 10 cached steps; the registry warm did not transfer cache metadata into the engine. - Routing the whole OpenClaw test through the isolated builder was worse: the live test took 9m54s after a 28s warm in [run 29378572904](https://github.com/NVIDIA/NemoClaw/actions/runs/29378572904), and the later production rebuild retained only 15 cached steps instead of the Docker engine baseline of 79. ## 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: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: Documentation review confirmed this is internal CI cache topology with no user-facing command, configuration, image contract, or supported product behavior change. - [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: Publisher provenance, cache writers, registry ownership, and permissions are unchanged. Rebuild jobs stop selecting an isolated PR-job builder and retain their existing Docker engine execution boundary. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes a `Signed-off-by:` line 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 — `npx vitest run --project cli --project e2e-support src/lib/onboard/sandbox-prebuild.test.ts test/e2e/support/e2e-workflow.test.ts test/e2e/support/dcode-profile-import-gate-workflow-boundary.test.ts test/e2e/support/hosted-inference.test.ts` (4 files, 75 tests); `npx vitest run --project installer-integration test/install-openshell-version-check.test.ts -t "preserves the rebuild Hermes requested channel through the real installer boundary"` (1 passed); `npm run source-shape:check` (passed) - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
21 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
3 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
Moves base-image BuildKit cache storage from quota-constrained GitHub Actions caches to per-image GHCR registry cache artifacts. OpenClaw and Hermes rebuild E2Es plus the Deep Agents Code profile gate import those trusted caches through job-local Buildx builders without exporting PR-controlled layers, and the selected builder now reaches the Docker builds spawned by the fixtures and CLI.
Changes
:buildcacheregistry artifact beside each OpenClaw, Hermes, and Deep Agents Code base image, usingmode=maxonly in the trusted publisher workflow.BUILDX_BUILDERacross the fixture and sandbox-prebuild environment boundaries so direct test builds and NemoClaw prebuilds use the warmed builder.Dockerfile.baseCOPYinput.full-e2epath and the existing build-once/artifact-sharing sandbox-image workflows unchanged.Type of Change
Quality Gates
cache-toorpushinputs. Existing credential isolation and immutable action pins remain intact.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 unavailablenpx vitest run --project cli --project integration --project e2e-support src/lib/onboard/sandbox-prebuild.test.ts src/lib/sandbox-base-image/source-identity.test.ts src/lib/sandbox-base-image/resolution-key.test.ts test/dcode-base-image-workflow.test.ts test/e2e/support/e2e-workflow.test.ts test/e2e/support/hosted-inference.test.ts(6 files, 88 tests);npx vitest run --project installer-integration test/install-openshell-version-check.test.ts -t "preserves rebuild Hermes workflow inputs through the real installer boundary"(1 passed);npm run source-shape:check(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)Additional runtime evidence from the original change set: a local container-driver Buildx smoke confirmed that a subsequent plain
docker buildselected the named builder, reused the warmed layer asCACHED, and auto-loaded the resulting image.Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
mode=max) across relevant build-and-push jobs.BUILDX_BUILDER, plus sandbox base-image identity/resolution-key test updates.