fix(onboard): diagnose incomplete custom plugin images#6250
Closed
ericksoa wants to merge 85 commits into
Closed
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
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:
📝 WalkthroughWalkthroughThis PR adds custom OpenClaw runtime diagnosis, threads OpenClaw plugin provenance through restore and rebuild flows, updates managed-extension and config-merge behavior, refreshes OpenClaw docs, and expands the weather fixture plus E2E regression coverage. ChangesCustom OpenClaw runtime, provenance restore, docs, and lifecycle tests
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6250.docs.buildwithfern.com/nemoclaw |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 76%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
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
|
Contributor
PR Review Advisor — BlockedMerge posture: Do not merge until addressed Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
PR Review Advisor (Nemotron Ultra) — BlockedMerge posture: Do not merge until addressed Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/live/openclaw-plugin-runtime-exdev.test.ts (1)
142-146: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
plugins.load.pathsset overwrites rather than extends the array.
openclaw config set plugins.load.paths '["/sandbox/.openclaw/extensions/weather"]'replaces the entire load-paths array. If the managed base image ships any default plugin paths, they'd be dropped in this custom image. Likely fine for this narrow fixture, but worth a quick sanity check that the base image doesn't rely on other defaultplugins.load.pathsentries.🤖 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/e2e/live/openclaw-plugin-runtime-exdev.test.ts` around lines 142 - 146, The `plugins.load.paths` update in the `openclaw` setup currently replaces the entire array instead of extending it, which can drop any default plugin paths from the base image. In the `openclaw config set plugins.load.paths` step, verify the existing load paths first and append the weather plugin path rather than overwriting the full list, keeping the fixture compatible with any default entries used by the image.
🤖 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 `@test/e2e/live/openclaw-plugin-runtime-exdev.test.ts`:
- Around line 142-146: The `plugins.load.paths` update in the `openclaw` setup
currently replaces the entire array instead of extending it, which can drop any
default plugin paths from the base image. In the `openclaw config set
plugins.load.paths` step, verify the existing load paths first and append the
weather plugin path rather than overwriting the full list, keeping the fixture
compatible with any default entries used by the image.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c4d0e597-496f-47ca-8d3a-07a8da6e7474
⛔ Files ignored due to path filters (1)
test/e2e/fixtures/plugins/weather/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.github/workflows/regression-e2e.yamldocs/deployment/install-openclaw-plugins.mdxdocs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxdocs/reference/troubleshooting.mdxsrc/lib/onboard.tssrc/lib/verify-deployment.test.tssrc/lib/verify-deployment.tstest/e2e/fixtures/plugins/weather/.gitignoretest/e2e/fixtures/plugins/weather/openclaw.plugin.jsontest/e2e/fixtures/plugins/weather/package.jsontest/e2e/fixtures/plugins/weather/src/index.tstest/e2e/fixtures/plugins/weather/tsconfig.jsontest/e2e/live/openclaw-plugin-runtime-exdev.test.tstest/regression-e2e-workflow.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28668378954
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results —
|
| Job | Result |
|---|---|
| cloud-onboard |
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/regression-e2e-workflow.test.ts`:
- Around line 108-123: The contract test in regression-e2e-workflow.test.ts is
asserting against raw source text from openclaw-plugin-runtime-exdev.test.ts
instead of an observable outcome. Update hardens the generated custom-plugin
image contract to verify behavior through a shared exported constant/module for
the Dockerfile and plugin-install command, or by checking the generated artifact
itself, and avoid reading the sibling test file’s source. Use the existing
identifiers readFileSync, openclaw-plugin-runtime-exdev.test.ts, and the hardens
the generated custom-plugin image contract test to relocate these assertions to
a shared boundary.
🪄 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: fc108fb5-d1d1-45f8-a8ea-fc09393c9dec
📒 Files selected for processing (3)
docs/deployment/install-openclaw-plugins.mdxtest/e2e/live/openclaw-plugin-runtime-exdev.test.tstest/regression-e2e-workflow.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/deployment/install-openclaw-plugins.mdx
- test/e2e/live/openclaw-plugin-runtime-exdev.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Author
|
Advisor follow-up for the remaining EXDEV
I accept that bounded test-only policy broadening for this PR; the production source files and runtime policy remain unchanged. |
Contributor
Author
|
Nemotron advisor disposition for the remaining nominal blockers/warnings:
No additional refactor is warranted for these items; the focused GPT-5.5 review has zero required fixes, and the exact-head repository guards are green. |
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28670144442
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results —
|
| Job | Result |
|---|---|
| cloud-onboard |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28672329509
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28673496126
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28674366019
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28676048797
|
Collaborator
Final exact-head handoff —
|
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28899273118
|
Collaborator
Refreshed exact-head handoff —
|
21 tasks
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28904067877
|
Collaborator
Exact-head handoff —
|
21 tasks
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28910070104
|
Collaborator
Refreshed exact-head handoff —
|
# Conflicts: # ci/test-file-size-budget.json Signed-off-by: cjagwani <cjagwani@nvidia.com>
Collaborator
|
Maintainer refresh at exact signed head
Local verification: CLI build and typecheck; 83 focused restore/config/rebuild/dependency-review tests; repository checks; test-size budget; docs variant/route checks; Biome; and |
Collaborator
|
Trusted acceptance-source map for exact head
Security disposition: today’s audit records the exact 9-package / 27-advisory release-pinned graph and lock digest. The fixture remains secret-free, read-only, integrity-locked, installed with lifecycle scripts disabled, dev/peer-pruned before runtime staging, and runs only after Docker credentials are removed. Replacing OpenClaw The Nemotron same-ID collision proposal is not applicable: a fresh and previous image carrying the same plugin ID at different paths is the intended image-owned upgrade case. Fresh config ownership wins and the previous directory is excluded from backup restore; the new regression proves that. At the pre-delete boundary, All exact-head automated gates are green. Because I contributed the merge/test clarification commit, I am leaving the final approval to the independently requested reviewer. |
# Conflicts: # src/lib/actions/sandbox/rebuild-backup-phase.ts Signed-off-by: cjagwani <cjagwani@nvidia.com>
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Collaborator
|
Current-main refresh pushed at signed head |
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Collaborator
|
Growth-guard follow-up at signed head |
cv
requested changes
Jul 8, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Current head e4c712ec5 remains blocked by the hard contributor-compliance gate: GitHub does not mark six commits Verified (60c031a4, 00d2883f, 7eadce16, 3927277d, 5c2f9f2b, and 9f3df598). Their Signed-off-by trailers satisfy DCO, but they do not satisfy the separate Verified-commit requirement, and maintainer waiver/repair of contributor history is not permitted.
The current ci/test-file-size-budget.json conflict is mechanically resolvable, but merging main would not cure this gate. Please replace the branch history with a fresh compliant branch/PR containing only GitHub-Verified commits, then rerun exact-head CI and review. Do not force-push if the published branch rules prohibit it.
Collaborator
|
Superseded by #6490. The reviewed implementation is preserved there on current main as one fresh, signed commit with the original contributors credited. This replacement is necessary because six historical commits in #6250 cannot satisfy the repository's GitHub-Verified requirement and cannot be waived under the current release policy. Full CI and applicable live evidence will rerun on #6490's exact head. |
apurvvkumaria
added a commit
that referenced
this pull request
Jul 8, 2026
<!-- markdownlint-disable MD041 --> ## Summary Fresh verified-history replacement for #6250. This PR preserves the reviewed net diff on current `main` while replacing the non-compliant historical commit chain with three newly signed, GitHub-Verified commits. Aaron Erickson remains the original contributor and author of superseded PR #6250; cjagwani authored this replacement PR. Apurv Kumaria contributed the review-driven follow-up fixes. Both Aaron and Apurv are credited as co-authors on the replacement commit. ## Related - Supersedes #6250 - [Discussion #6108](#6108) - Regression coverage for #3513 and #3127 - The no-source-checkout managed plugin lifecycle remains tracked by #5998 ## Changes - Diagnose custom OpenClaw images that contain only the intermediate `sandbox-base` instead of the complete managed runtime. - Record authoritative image-plugin provenance and reconcile removed, renamed, and same-ID upgraded plugins during recreation and rebuild. - Preserve user-owned plugin, channel, and tool state while removing stale image-owned configuration; fail closed on incomplete provenance. - Add deterministic release lifecycle coverage for onboarding, restart, recreation, rebuild, workspace preservation, and EXDEV-safe runtime dependency replacement. - Authenticate only for the exact digest-pinned builder pre-pull, then remove Docker credentials before release-pinned fixture code runs. - Document the full-image plugin recipe, troubleshooting guidance, dependency risk controls, and the explicit `REMOVE-WHEN` condition. ## Verification - 126 focused rebuild, restore, diagnosis, and custom-Dockerfile tests passed. - `npm run build:cli` passed. - Node 22 CLI typecheck passed. - Test-file size budget passed across 1,372 files. - Biome passed all 64 changed supported files. - `git diff --cached --check` passed before commit. - Replacement history contains three commits, all GitHub Verified with good ED25519 Git signatures. - Full standard CI and applicable live E2E are intentionally running again on this new exact head. ## Provenance The implementation and tests are the reviewed work from #6250, replayed onto current `main` because six historical commits in that PR could not satisfy the repository's GitHub-Verified requirement. This replacement does not claim those historical objects are verified; it provides a new, attributable, signed history containing their reviewed net result. Signed-off-by: cjagwani <cjagwani@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added OpenClaw weather plugin E2E fixtures and expanded end-to-end plugin lifecycle coverage, including a new runtime-deps EXDEV lane. * **Bug Fixes** * Strengthened onboarding/recovery to fail closed when OpenClaw image-plugin provenance is incomplete, and tightened rebuild/restore gating and hints. * Improved handling of OpenClaw managed extensions and restore planning with stricter symlink and identity checks. * **Documentation** * Updated installation, lifecycle, quickstart, commands, and troubleshooting docs with safer recovery and pinned sandbox workflows. * **Tests/CI** * Updated E2E/regression workflows and added provenance-related rebuild/restore test coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: cjagwani <cjagwani@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Merged
21 tasks
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Fresh verified-history replacement for NVIDIA#6250. This PR preserves the reviewed net diff on current `main` while replacing the non-compliant historical commit chain with three newly signed, GitHub-Verified commits. Aaron Erickson remains the original contributor and author of superseded PR NVIDIA#6250; cjagwani authored this replacement PR. Apurv Kumaria contributed the review-driven follow-up fixes. Both Aaron and Apurv are credited as co-authors on the replacement commit. ## Related - Supersedes NVIDIA#6250 - [Discussion NVIDIA#6108](NVIDIA#6108) - Regression coverage for NVIDIA#3513 and NVIDIA#3127 - The no-source-checkout managed plugin lifecycle remains tracked by NVIDIA#5998 ## Changes - Diagnose custom OpenClaw images that contain only the intermediate `sandbox-base` instead of the complete managed runtime. - Record authoritative image-plugin provenance and reconcile removed, renamed, and same-ID upgraded plugins during recreation and rebuild. - Preserve user-owned plugin, channel, and tool state while removing stale image-owned configuration; fail closed on incomplete provenance. - Add deterministic release lifecycle coverage for onboarding, restart, recreation, rebuild, workspace preservation, and EXDEV-safe runtime dependency replacement. - Authenticate only for the exact digest-pinned builder pre-pull, then remove Docker credentials before release-pinned fixture code runs. - Document the full-image plugin recipe, troubleshooting guidance, dependency risk controls, and the explicit `REMOVE-WHEN` condition. ## Verification - 126 focused rebuild, restore, diagnosis, and custom-Dockerfile tests passed. - `npm run build:cli` passed. - Node 22 CLI typecheck passed. - Test-file size budget passed across 1,372 files. - Biome passed all 64 changed supported files. - `git diff --cached --check` passed before commit. - Replacement history contains three commits, all GitHub Verified with good ED25519 Git signatures. - Full standard CI and applicable live E2E are intentionally running again on this new exact head. ## Provenance The implementation and tests are the reviewed work from NVIDIA#6250, replayed onto current `main` because six historical commits in that PR could not satisfy the repository's GitHub-Verified requirement. This replacement does not claim those historical objects are verified; it provides a new, attributable, signed history containing their reviewed net result. Signed-off-by: cjagwani <cjagwani@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added OpenClaw weather plugin E2E fixtures and expanded end-to-end plugin lifecycle coverage, including a new runtime-deps EXDEV lane. * **Bug Fixes** * Strengthened onboarding/recovery to fail closed when OpenClaw image-plugin provenance is incomplete, and tightened rebuild/restore gating and hints. * Improved handling of OpenClaw managed extensions and restore planning with stricter symlink and identity checks. * **Documentation** * Updated installation, lifecycle, quickstart, commands, and troubleshooting docs with safer recovery and pinned sandbox workflows. * **Tests/CI** * Updated E2E/regression workflows and added provenance-related rebuild/restore test coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: cjagwani <cjagwani@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
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
Extends Aaron Erickson's original custom-plugin diagnosis and lifecycle work so OpenClaw image plugins remain correct across onboarding, restart, recreation, and rebuild. The follow-up hardens provenance/restore boundaries, adds release-matched live coverage, and keeps the historical fixture isolated from Docker credentials.
Aaron remains the original author and PR author; Apurv Kumaria contributed the review-driven follow-up fixes.
Related Issue
Changes
sandbox-baseinstead of the complete managed runtime.REMOVE-WHENcondition.Type of Change
Quality Gates
Verification
Verifiedin GitHub — DCO is green; six historical Aaron commits remain unsigned under the explicit maintainer waiver, while both new contributor merge commits are GitHub Verified.pre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablesandbox-operationsexact-head run also passednpm run docsbuilds without warnings (doc changes only) — completed with 0 errors and 2 pre-existing warningsSigned-off-by: Aaron Erickson aerickson@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit