Repository navigation
ci(e2e): renew model-drift budget after restoration probes - #12871
Merged
Merged
Conversation
|
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. |
Contributor
|
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; 7 remain after this review. 📝 WalkthroughWalkthroughThe assertion-growth exception comment now records the accounting included from merged PR ChangesAssertion-growth exception
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~4 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to The exception remains narrowly scoped, with no demonstrated guardrail bypass. The candidate budget needed to independently confirm its head digest was unavailable, but no current-head failure is established. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit f1d7600 in the Show a line coverage summary of the most impacted files.
TypeScript / code-coverage/cliThe overall line coverage in commit f1d7600 in the Show a line coverage summary of the most impacted files.
Updated |
rsliter
marked this pull request as ready for review
October 9, 2026 03:48
Contributor
|
PR Review Advisor finished for commit Request review only when Require no Advisor blockers is green. |
jyaunches
approved these changes
Oct 9, 2026
prekshivyas
pushed a commit
that referenced
this pull request
Oct 9, 2026
## Outcome Fresh onboarding with an existing Ready sandbox now checks the requested model and provider before deciding whether to reuse it. A changed selection reaches the existing recreation flow, including interactive confirmation; unchanged selections retain reuse behavior. After fresh, non-forced managed OpenClaw recreation restores native state, a verified provider/model change is applied before gateway restart and registry publication. Unrelated native settings remain preserved. ## Reason Provider review records the selected sandbox name in a new checkpoint. Crash recovery previously treated that identity as evidence for reuse even when the run was not resuming and no create receipt existed, bypassing selection-drift detection. Live validation also exposed a second bypass: a fresh route reservation could open a recreation journal on a Ready sandbox before selection checks, forcing replacement even when the model and provider were unchanged. Once that reuse path was restored, its route reservation remained pending and OpenClaw correctly rejected finalization. The handler now publishes the owned reservation after recording successful sandbox effects. The next live run reached model-change recreation and exposed an inherited ordering problem: complete native snapshot restoration overwrote the newly selected model. Rebecca accepted a bounded reconciliation repair for this path. ### Related issues Fixes #12667. ## Changes - Leave fresh Ready route reservations with the existing selection-aware create owner. Missing-sandbox creation and resumed or explicit recreation retain their journal paths. A focused regression protects this boundary. - Publish the successful create-owner route reservation through the existing session-checked finalizer after metadata and effect recording. Pending-create identity and pairing identity guards remain intact. - Preserve verified selection changes when a fresh non-forced run opens a journal for a live not-ready sandbox. The transaction owner records whether a prior journal existed; only the current fresh run receives a process-local handoff. The existing atomic delete transition persists the selection marker after session, source identity, registry, gateway, and target checks. Legacy and resumed handoffs cannot gain this authority. Handler, owner, source-disappearance, and orchestration regressions protect this boundary. - Document the bounded native-configuration exception in the OpenClaw runtime guide, including resume of a recreation that already recorded the verified selection. - Distinguish planned recreation from an explicit CLI/environment force request when recording reconciliation authority. The derived recreate flag also covers automatic drift and cannot identify forced recreation. - Reconcile only known provider/model changes on managed OpenClaw. An optional true-only marker on the existing recreation journal retains this decision through resume. Use the existing native inference writer inside the restore maintenance window; failed writes or identity checks prevent registry publication. Forced rebuilds, custom images, Hermes, unknown/unchanged selections, and generic snapshot preservation keep their existing behavior. - Restrict checkpoint crash recovery to resumed sessions, preserving lost-receipt recovery for real resumes. - Extend existing regression cases for fresh and ordinary onboarding, model/provider changes, unchanged selections, and a missing sandbox. - Extend the recreation fixture to cover interactive acceptance, interactive refusal, and noninteractive recreation, asserting sandbox identity and mutation outcomes. - Extend the existing double-onboard E2E with same-name OpenClaw model-change recreation, checking replacement identity, native primary model, and registry selection. Remove the redundant entrypoint-existence and sibling-list assertions while retaining ownership and cleanup checks. - Apply the exact assertion-budget transition renewed by merged prerequisite #12871, preserving main’s existing assertion reductions and newer live-file/probe accounting. The changed-model operation has its own semantic progress phase; Hermes retains the existing skipped-phase behavior. ## Verification - The marked-recreation recovery integration now persists the journal before calling production creation and completion. It verifies native primary model before restart and after restore, unrelated native settings, registry selection, and replacement identity. Forced and unmarked recovery retain the old restored model. All 18 recreation integration cases pass. Removing the production marker handoff locally makes only the marked-recovery case fail; production bytes were restored. This is deterministic integration evidence with mocked sandbox transport, not a live E2E result. Test-only repair `7305b0bb0c81f254837e9fcb31ee6f4a74e27445` passed commit hooks, normal pre-push publication validation, and CLI type-checking; plugin and checked-JavaScript checks legitimately skipped. All 16 PR commits are GitHub Verified. - Documentation correction `76d0464459a81e60c109ccd71283dbc181448f58` clarifies that an already-authorized recreation record retains its selection through resume. `npm run docs` passed with zero errors and two warnings; independent review of that commit found no blockers or suggestions. Only the OpenClaw generated variant changes. Commit hooks and normal pre-push publication validation passed; all compiler checks legitimately skipped for the documentation-only commit. All 15 PR commits are GitHub Verified. - After integrating merged prerequisite #12871, focused journal, finalization, orchestration, and create-plan suites pass 198 tests. All 16 recreation integration cases pass, including fresh, legacy, and forced journal boundaries. The first isolated integration attempt was blocked by loopback fixture permissions; the rerun allowed only localhost connections while retaining credential and external-network isolation. The prepared progress repair passed all 29 phase/progress support tests, and the documentation build and independent documentation review passed. The two accepted review repairs are published in `9f2f2f5fd241b1d5f6fcb1a082fd99274e4c5796`. Normal pre-push publication validation and CLI type-checking passed; plugin and checked-JavaScript checks legitimately skipped. All 14 PR commits are GitHub Verified. The unchanged installed hook ran with its verified credential isolation. - Fresh-journal regression reproduced the missing marker before the fix (one failure, two exclusions passing). The repaired orchestration passes all 16 recreation integration cases, including fresh, legacy, and forced pre-opened journals. Transaction/journal/finalization suites passed 183 tests before the test was moved to its owning suite for the file-size limit; final handler/owner suites passed 60 tests before the additional source-disappearance case. All four final marker cases pass. A final owner-suite run passes 39 tests and times out in an unchanged channel-recovery test; unchanged published `6dac1ab5717a8984e5808311db41ff78ec51e905` reproduces the same five-second timeout. No timeout was increased, and this case is not claimed passing. Growth guardrails, focused lint, and documentation build pass. Independent documentation review has no blockers. Normal pre-push publication validation and CLI type-checking passed for `1e2a33e80e75417f21408d18b7928128f693455a`; plugin and checked-JavaScript checks legitimately skipped. The unchanged installed hook and its children ran with a sanitized environment and credential-denied filesystem isolation. All 13 published commits are GitHub Verified. Candidate validation does not replace independent CI or review. - The force-admission regression failed before this repair: planned recreation omitted its reconciliation marker while explicit force correctly stayed excluded. With the one-line repair, all 13 recreation integration tests and 74 journal/finalization tests pass. All seven growth guardrails also pass after flattening the regression assertions. The regression reads the durable journal before the mocked deletion and verifies source retention. Formatting, lint, and diff checks pass. Normal pre-push publication validation and CLI type-checking passed for `6dac1ab5717a8984e5808311db41ff78ec51e905`; plugin and checked-JavaScript checks legitimately skipped. All 12 PR commits are GitHub Verified. - The restore-handoff regression fails without the repair. Focused selection, native preservation, journal resume, checkpoint parsing, and finalization tests pass: 144 tests across five files, plus the added identity-denial case. After integrating the required canonical validator dependency, all 34 finalization/GPU-finalization tests pass. - Existing onboarding integration tests pass (11 tests), and all seven growth guardrails pass. Four additional transaction, identity, and final-flow suites pass (144 tests). - Seven supplemental resume tests encounter permission failures accessing the personal runtime directory. An unchanged `73db8b8974` baseline reproduces the exact seven failures before the changed code. These tests are not claimed passing, and no personal runtime access was granted. - After integrating the merged prerequisite and preserved remote bot history, all 129 core repair tests pass. Supplemental registration, intent, and journal tests pass eight cases and encounter 30 personal-runtime permission failures. Unchanged main reproduces 29 matching failures; the additional PR regression reaches the same host-lock permission failure before its assertion. These cases are not claimed passing. - Formatting, lint, and diff checks pass. Normal pre-push publication validation and CLI type-checking passed for `a610dfd8295ea775075a28bb2dd67d2fffed6524`; plugin and checked-JavaScript checks legitimately skipped. All 11 commits published at that head were GitHub Verified. - The accepted double-onboard increase remains four assertion points and six expectations. The complete integrated budget hashes match merged prerequisite #12871. No credentials are introduced. ## Review notes Candidate `9f2f2f5fd241b1d5f6fcb1a082fd99274e4c5796` passed CI and managed-image validation. All nine Advisor specialists and all 27 receipts from run 37936255306 were read. The sole finding was documentation that incorrectly excluded resume of an already-authorized recreation record; this update corrects that wording without changing runtime behavior. The other eight interests, including migration, reported no findings. CodeRabbit generated no new actionable comments and all three threads remain resolved, but its summary still mentions a journal-marker gap. The previously concrete missing-source and fresh-journal findings were respectively withdrawn and accepted as addressed; the generic summary remains subject to bot reassessment. No review gate is claimed clear for this new commit. Fresh CI, automated reviews, and focused live validation remain required. This PR changes sensitive onboarding paths in NVIDIA/NemoClaw (`src/lib/onboard/**`). Local source review checked fresh reuse, selection drift, explicit recreation, resumed journals, journal-before-deletion ordering, native-state preservation, and reconciliation before registry publication. A write or identity failure aborts the offline window without claiming completion; the recorded selection decision survives resume. The existing session ownership, pending-create, and runtime identity checks remain intact. CI and managed-image validation passed for `a610dfd8295ea775075a28bb2dd67d2fffed6524`. All nine Advisor reports and 27 receipts from attempt 3 were read; its blocker gate failed on the explicitly deferred legacy-journal limitation. CodeRabbit withdrew its inherited missing-source backup finding and resolved the thread. The earlier operability report identified the derived-recreate versus explicit-force distinction; source verification and the new failing regression confirmed that defect, and this repair corrects the admission argument. Rebecca accepted deferring unreadable-state recovery and retaining the bounded verified-change scope. Forced recreation, missing-source backup selection inference, and automatic migration of unmarked older journals remain excluded inherited limitations. No warning-and-stop recovery guard was added because it lacks a verified recovery procedure. These dispositions do not clear the Advisor gate or claim reviewer agreement. No human approval or waiver is claimed. Historical candidate `73db8b897401331396822bd95b364583041adeb0` passed CI, managed-image publication, and all nine [Advisor specialists](https://github.com/NVIDIA/NemoClaw/actions/runs/37721068666); all 27 receipts were reviewed. CodeRabbit reported no actionable finding for that head. Its [focused live run](https://github.com/NVIDIA/NemoClaw/actions/runs/37722627433) passed Hermes and OpenClaw same-selection reuse, identity, and dashboard checks. OpenClaw then detected the requested model drift and recreated the sandbox, but failed because restored native configuration still selected `inference/test-model` instead of `inference/changed-model`. All 19 cleanup actions passed. Exact dispatch/candidate evidence and the full redacted failure-log audit were reviewed. The prior a610 candidate passed CI and image checks. The force-admission repair subsequently passed CI and image checks; the fresh-journal repair subsequently passed CI and image checks. The current integration and review repairs still require fresh CI and reviews, followed by focused live validation. Historical candidate `34ad182ad36f429fc3351ada1a5fc2c549badf8c` passed [CI](https://github.com/NVIDIA/NemoClaw/actions/runs/37708708337), [managed-image publication](https://github.com/NVIDIA/NemoClaw/actions/runs/37708708304), and all nine [Advisor specialists on approved attempt 2](https://github.com/NVIDIA/NemoClaw/actions/runs/37710339067/attempts/2). All 27 receipts were reviewed. CodeRabbit's optional actual-inference coverage suggestion was dispositioned against the accepted identity/configuration/registry scope; successful inference is not claimed. Its [focused double-onboard run](https://github.com/NVIDIA/NemoClaw/actions/runs/37716430958) passed Hermes. OpenClaw reused the same-selection sandbox but failed finalization because the route reservation remained pending. The changed-model phase was not reached. All nine OpenClaw and ten Hermes cleanups passed. The dispatch receipt, artifact identities, and full redacted failure-log audit were reviewed. The reservation repair subsequently passed this same-selection path. The earlier [run on `cf4de943`](https://github.com/NVIDIA/NemoClaw/actions/runs/37704984173) failed both same-selection ID checks because a premature journal forced recreation. That failure was repaired before the subsequent run established successful reuse. The authorized focused selector includes OpenClaw and Hermes; the added changed-model phase applies only to OpenClaw. Focused evidence does not claim the broader advisory coverage plan. The original DGX Spark Super-to-Ultra generated configuration and TUI reproduction remain unverified on hardware. The PR remains WIP until final-head validation and automated reviews settle, with eligible human approval required for merge. --- 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 * **New Features** * Re-onboarding with a changed provider or model now recreates the managed OpenClaw sandbox and applies the updated selection after restoring its backup. * The restored selection updates OpenClaw’s primary model while preserving unrelated native configuration. * **Bug Fixes** * Re-onboarding with the same provider and model continues to reuse a ready sandbox. Fresh onboarding creates a sandbox with the requested provider and model when one is missing. <!-- 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> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
prekshivyas
pushed a commit
that referenced
this pull request
Oct 9, 2026
## Outcome Remove the obsolete assertion-growth exception for merged PR #12769. ## Reason PR #12769 merged as 0754ac5. Its reviewed live assertions are now part of the main budget, so the temporary transition exception is no longer needed. ### Related issues Refs #12667. Follow-up to #12769 and the prerequisite policy renewal #12871. ## Changes Remove the #12769 exact digest pair and its explanatory paragraph from `ci/e2e-assertion-growth-exceptions.json`. Preserve the six other exception entries, the live assertion budget, and all test coverage. ## Verification - Existing growth-guardrail parser suite: 58 tests passed. - Structural comparison: exactly the #12769 entry and its paragraph were removed; all other policy values are unchanged. - Normal signed-commit hooks, publication validation, and CLI/plugin/JavaScript compiler checks: passed. - Reviewed the complete one-file change. It contains no secrets, API keys, or credentials. --- 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 * **Chores** * Updated CI assertion-budget exception records, including refreshed approval and budget details. * Removed an existing exception; other exception records remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
wscurran
added
area: ci
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.
Outcome
PR #12769 can retain its approved model-change assertions after integrating the restoration-probe accounting merged in #12767. This prerequisite renews only its exact budget digest pair.
Reason
The merged probe accounting changed the base budget bytes, invalidating the exception renewed in #12849. The independent growth check requires matching trusted base policy.
Related issues
Refs #12667. Prerequisite for #12769.
Changes
Update only the #12769 exception and its explanation. Preserve the concurrent-gateway assertion reduction, the new live-file and restoration-probe metrics, and every other PR entry. The model-change transition remains four additional assertion points and six additional expectations. This PR changes no tests, budget limits, or enforcement code.
Verification
Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com
Summary by CodeRabbit