Repository navigation
ci(e2e): authorize shared restoration budget for PR 12767 - #12833
Conversation
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe policy comment documents the approved restoration-validation transition and exception conditions for PR ChangesAssertion Budget Exception
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~3 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to The exception is narrowly bound to PR 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit 3c4688d in the Show a line coverage summary of the most impacted files.
TypeScript / code-coverage/cliThe overall line coverage in commit 3c4688d in the Show a line coverage summary of the most impacted files.
Updated |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
|
@coderabbitai full review Please review the updated prerequisite at commit 55bf5d2. The approved budget pair now preserves the upstream reduction. |
✅ Action performedFull review finished. |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
|
@coderabbitai full review Please review the complete current candidate |
✅ Action performedFull review finished. |
|
PR Review Advisor finished for commit Request review only when Require no Advisor blockers is green. |
## Outcome Refresh the exception for #12382 so its reviewed runtime assertions can pass the independent growth check after #12608 reduced main's gateway assertions. ## Reason #12608 merged before #12718 and changed the base budget bytes. The exception adopted by #12718 therefore matches neither the new base budget nor #12382's prepared integration. ### Related issues Refs #12382, #12718, #12608. ## Changes Replace only the two budget digests for #12382 and add a scoped explanation. Preserve every other exception and all enforcement code. | Budget | SHA-256 | |---|---| | Base | `53c51c4229d387633165771730f8f1a8373f84e9f3303b2144280027a56e3441` | | Runtime candidate | `8162408384d7a6b97ef0062042c73d4bc9486f54d2dfa6c0c5732e42d1060e2a` | The runtime transition retains native admin approval before survival/rebuild mutations and the plugin-install success check. It adds one unique assertion point and one generated probe condition over the reduced main budget. It adds no live scenario or matrix lane. ## Verification - Canonical `e2eAssertionBudgetGrowthViolations` consumer: accepts the proposed trusted-base transition; rejects a different PR, changed base bytes, changed candidate bytes, and candidate-only policy. - Assertion census: the prepared runtime source fits the proposed budget. - All 58 growth parser tests passed after resolving the shared-policy conflict. Existing census evidence remains valid because the budgets and runtime source did not change. - Signed commit and normal publication hooks passed, including plugin and checked-JavaScript type checks. - Reviewed the complete one-file diff. No secrets, API keys, or credentials. ## Review notes The branch incorporates #12837, which repairs the inherited Ubuntu mirrorlist installation failure. The branch also incorporates #12833 and preserves its #12767 exception. The #12382 budget digests are unchanged. The policy comment now cites the maintainer acceptance recorded below; no exception value or enforcement code changed. Signed commit `71535b2` passed normal commit and publication hooks. Fresh CI and Advisor checks for this comment-only update are pending. [Senthil approved the PR](#12840 (review)), confirming that the recorded acceptance resolves his earlier change request. Maintainer Prekshi Vyas (@prekshivyas) [accepted the exact replacement budget pair](#12840 (comment)), including one additional unique assertion point and one generated probe condition. Independent enforcement is not waived: adoption on main remains required before #12382 can use this exception. The approval records the exact replacement pair and its bounded assertion increase. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated internal CI records with the approved exception’s refreshed verification values. This does not change end-user functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…12767) ## Outcome Installer recovery attempts an explicitly confirmed legacy OpenClaw sandbox when the gateway reports Ready but its actual agent-version probe fails. Previously, `upgrade-sandboxes --auto` skipped this sandbox without using its prepared backup. ## Reason A pre-fingerprint entry cannot establish image drift, and a failed probe supplies no version for stale-version classification. The existing selection discarded the installer's managed-image confirmation before backup validation or rebuild. ### Related issues Refs #7475. Prerequisite assertion policy #12833 is merged. ## Changes - Include explicitly confirmed legacy OpenClaw entries after an actual failed probe in prepared-recovery selection. Keep sandbox-name confirmation, installer restore intent, recorded gateway, valid backup, and custom-image rejection mandatory. - Preserve selection behavior for successful or skipped probes, current managed fingerprints, missing expected versions, and other agents. Add no flags or persisted authorization. - Document the eligible Ready/failed-probe path in the quickstart, update guide, and command reference. - Share one restoration command builder between the existing rebuild live test and its executed-shell regression tests. Keep it in scanned live sources so the assertion census retains every predicate. Remove the AST extraction and command rewriting. - Fail restoration immediately on missing or mismatched state and failed commands. Require the exact expected plugin ID with loaded status. Cover invalid reports and incorrect command arguments in the existing support owner. - Propagate marker read failures even when the expected marker is empty, and register the shared helper as an owning path for rebuild selection. - Apply the exact maintainer-approved budget, preserving the upstream seven-assertion reduction. The exception policy is inherited unchanged from merged #12833. ## Verification - Recovery owner: 68 tests passed, including production version-command wrapping, parsing, and selection with a disposable local execution adapter. Confirmed probe failure selects rebuild; unconfirmed failure and successful current-version controls do not. The positive regression fails without the selection fix. - Restoration, workflow selection, and growth guardrails: 27 plus 138 plus seven tests passed after the latest repair. Both new defect regressions failed before the fixes; positive empty-marker restoration also passes. Shell cases cover restored state, missing or mismatched markers, config/plugin command failures, invalid plugin reports, and five incorrect-argument mutations. Fixture paths include spaces and an apostrophe. - Installer severity integration: five tests passed after integration. - `npm run e2e:assertions:check`: passed with 1,234 direct expect calls across 77 test files. All existing unique restoration predicates remain counted. - `NODE_OPTIONS=--max-old-space-size=8192 npm run typecheck:cli`: passed after canonical dependency synchronization. - Documentation build previously passed with zero errors and two warnings; this repair does not change those documentation edits. - Signed repair `61361a7ef3716f1ef3a31f65dcc1043911c54cf2` passed normal commit hooks. Normal pre-push publication validation and CLI TypeScript passed; plugin and JS-config checks were legitimately skipped. All eleven published commits are GitHub Verified. - The diff contains no secrets, API keys, or credentials. ## Review notes The shared-builder, marker-read, and helper-selection findings are repaired with regression evidence. On current head `61361a7ef3716f1ef3a31f65dcc1043911c54cf2`, [CI passed](https://github.com/NVIDIA/NemoClaw/actions/runs/37852518666), both independent growth checks passed, and CodeRabbit completed review across all 12 files with no actionable findings. [Advisor run 37854488215](https://github.com/NVIDIA/NemoClaw/actions/runs/37854488215) completed all nine specialists: eight were clear; verification raised a P1 for missing real OpenShell Ready/failed-probe live coverage. Its aggregate remains red. [The recorded disposition](#12767 (comment)) retains the production-command local regression and existing live upgrade/rebuild coverage, defers artificial fault injection, and challenges the P1 severity. This is not a green Advisor result or human review approval. [Rebecca approved the isolated validation procedure](#12767 (comment)) for the exact four-file repair. The canonical semantic checker passed 102 tests across 78 files in a credential-isolated environment with network and contributor-home access denied, refreshed against main `929d9f5287178d9bc2d744425c7bfb40d028ea37`. The candidate selector regression separately verifies its new mapping. Normal publication hooks remain required. [Rebecca approved the exact revised budget](#12767 (comment)): base `53c51c4229d387633165771730f8f1a8373f84e9f3303b2144280027a56e3441` to candidate `1653f4c6c0292d2853727c39ddb36aa55361f749df3db32aa3847c3d4a7f30cb`. This approval covers the shared restoration owner and preserves upstream reductions. The policy landed through #12833; it is not a waiver of the new candidate's checks or a PR review approval. [Focused E2E run 37857531326, attempt 1](https://github.com/NVIDIA/NemoClaw/actions/runs/37857531326) passed both upgrade and rebuild, plus Relevant E2E, on current head `61361a7ef3716f1ef3a31f65dcc1043911c54cf2`, base `c518e121d23c616da1b083c44c9eb504fb419ebd`, and trusted workflow `929d9f5287178d9bc2d744425c7bfb40d028ea37`. Correlation: `1e410023-8218-4b2f-a95f-e2eea8cb065d`. Selectors: `openshell-gateway-upgrade-v0-0-89-x86-64,rebuild-openclaw`; inference mode: `mock`. Dispatch and product evidence identities were verified. The 94 upgrade and 70 rebuild product files substantiate preserved state, configuration, exact loaded plugin, native readiness, authenticated mock inference and credential custody. Both cleanup reports have zero failures. Prior run 37700645111 remains evidence only for its original `63c0dc1` identity. The live upgrade exercised a stopped/stale survivor, not a real OpenShell Ready/failed-probe fault. The production probe-to-selection regression covers that selection boundary with a local execution adapter. Historical OpenShell v0.0.55 remains unverified; this PR does not establish the historical token-source failure's root cause. --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Installer recovery can rebuild a confirmed legacy OpenClaw sandbox marked Ready when its agent version check fails and no managed-image fingerprint is recorded, provided a validated latest backup is available. The sandbox must belong to the selected gateway and have no recorded custom-image evidence. * Recovery requires a validated latest backup for every sandbox selected for rebuilding. * **Documentation** * Updated sandbox management, command reference, and quickstart guidance to describe the recovery conditions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Outcome
The independent E2E growth check can accept the approved budget for #12767 after this prerequisite policy merges into
main.Reason
Independent enforcement reads exceptions from the trusted base. A candidate-only entry cannot authorize its own budget change.
Related issues
Refs #7475. Prerequisite for #12767.
Changes
Add one exception bound to PR #12767 and both budget SHA-256 hashes. Preserve all existing exception entries. Remove the temporary entry after #12767 merges.
The approved repair moves restoration validation into one builder within the scanned live source set. It preserves every restoration predicate, including plugin validation, and retains the seven-assertion reduction from #12608. The budget changes from
53c51c4229d387633165771730f8f1a8373f84e9f3303b2144280027a56e3441to1653f4c6c0292d2853727c39ddb36aa55361f749df3db32aa3847c3d4a7f30cb.Verification
3c4688d37d17f3a8e3aa96a933bb073924430ce2.The previous candidate passed growth enforcement and CodeRabbit. Its CI failed before tests in the inherited Ubuntu mirrorlist installer. This candidate integrates the landed fix from #12837 and resolves the independent policy-entry conflict while preserving both entries. CI run 37809262635 passed for the integrated candidate, including all twelve CLI shards. Independent growth enforcement passed. No live E2E is required for this policy-only diff; all nine specialists concurred.
Review notes
Rebecca approved this bounded relocation, budget transition, and prerequisite PR. Her maintainer permission and the record were verified. This authorization is not a PR review approval or an Advisor waiver. CodeRabbit completed its exact-head review with no actionable findings. All nine Advisor specialists reported clear, and the blocker gate passed in Advisor run 37811789238.
The policy PR changes no executable, live test, or budget file. Its parser continues to enforce PR identity and both hashes. The main repair remains local until this dependency lands. Local Docker and CLI exposure are not required for these policy checks.
Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com
Summary by CodeRabbit