fix(onboard): admit pending OpenClaw pairing observation - #10062
Conversation
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
|
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 (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe settlement parser now accepts pending scope-upgrade observations. Related tests expect this state, and the Portable repair assertion uses the pending upgrade device identity hash. ChangesOpenClaw settlement qualification
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change admits the intended pending pairing state while preserving fail-closed repair handling and same-device verification; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: None Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
…real-device-proof
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
sandl99
left a comment
There was a problem hiding this comment.
Approved latest PR commit 6158163 after code and security review. The settlement parser now admits only the existing scope-upgrade-pending state while preserving exact-key, terminal-output, digest, repair-path, and qualification checks; the real-device proof compares the correct canonical pending observation. Local validation: plugin and CLI builds, CLI type-check, focused pairing qualification tests (40/40), and diff check passed. Security: PASS for secrets and credentials, input validation, authentication and authorization, dependencies, errors and logging, cryptography and data protection, configuration, security testing, and system security. Cross-issue scan found no related fixes or conflicts. CI is not used as an approval gate for this review per maintainer direction.
## Summary Portable OpenClaw onboarding now waits within its existing fixed lifecycle for the canonical CLI device to appear and for approved baseline scopes to persist. Invalid, unrelated, multiple-request, or changed-device state still fails closed, and the controller still runs at most one request producer and one approval. ## Related Issue Related to #9817. ## Changes - Give not-yet-visible or concurrently changing canonical state a typed retryable observation result while retaining terminal rejection for invalid state. - Reuse the accepted 60-second appearance window, 30-second final observation window, one-second polling interval, and existing total settlement cap in the Portable controller. - Require the final strict observation to retain the same device identity, no pending request, and exact baseline scopes. - Add deterministic delayed-appearance and delayed-persistence controller regressions, plus a pinned OpenClaw 2026.7.1 production-controller proof. - Preserve PR #10062 as the separate owner for the ordinary `scope-upgrade-pending` parser and proof baseline. ## 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: - [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: Issue #9817 and the maintainer decision fix the contract to one canonical CLI identity with exact baseline scopes and no pending request. The change preserves that conjunction, retries only typed absence or state change, and keeps unrelated, multiple, malformed, or changed-identity state terminal. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: local complete changed-test and pinned-runtime lanes reach the separate PR #10062 `scope-upgrade-pending` baseline after this PR's tests pass; no acceptance is claimed here. ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable - Supporting evidence: This change does not modify `scripts/prepare-dgx-station-host.sh`. ## 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 validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 27/27 Portable settlement tests; 2/2 focused observer tests; 29/29 finalization dependency tests; CLI typecheck; repository architecture checks; pinned OpenClaw 2026.7.1 proof crossed the new production Portable-controller phase. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — `npm run test:changed` passed 529/531 tests; the remaining two tests are the separate PR #10062 baseline. The full pinned harness likewise crossed this PR's phase and then stopped at that baseline. - [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: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved pairing readiness checks to tolerate temporary state changes and delayed device availability. * Added bounded retries for transient observation errors during pairing approval and settlement. * Rejects mismatched device identities and reports incomplete status when settlement times out or is declined. * Prevents unrelated pending requests from being mistaken for temporary pairing states. * **Tests** * Expanded coverage for delayed pairing, retries, timeouts, and final settlement validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Summary
OpenClaw ordinary pairing observation now accepts the typed
scope-upgrade-pendingstate that its observer already emits. The real-device proof also compares the Portable repair result with the current pending-upgrade observation instead of a removed variable.Related to #9211 and #9817.
Changes
scope-upgrade-pendingstate in the ordinary settlement parser.Type of Change
Quality Gates
DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project cli src/lib/actions/sandbox/launch-readiness/openclaw-pairing-qualification.test.ts(40/40);npm run test:changed(32/32 growth checks and 527/527 changed tests);npm run typecheck:cli;npm run checks:repository; CLI/plugin builds and source-map validationnpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Senthil Ravichandran senthilr@nvidia.com
Summary by CodeRabbit