Skip to content

fix(cli): restore fresh onboarding device pairing - #7997

Closed
sandl99 wants to merge 6 commits into
mainfrom
fix/fresh-onboard-device-pairing
Closed

fix(cli): restore fresh onboarding device pairing#7997
sandl99 wants to merge 6 commits into
mainfrom
fix/fresh-onboard-device-pairing

Conversation

@sandl99

@sandl99 sandl99 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

The restored-clone pairing path from #7834 made ordinary force pairing require clone descriptors, so fresh onboarding could finish without pairing its CLI device. The restored-clone warm-up could also return before OpenClaw published its pending scope request. This PR separates the two identity modes and publishes one write-scoped request before #7834's one-shot approval. The restored-clone provoke command removes runtime shared auth so OpenClaw uses the clone's stored device credential to publish that request.

Related Issue

Regression follow-up to #7834.

Changes

  • Add the internal NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING mode for the restored-clone approval child. Ordinary force pairing continues to use the default device identity.
  • Migrate OpenClaw runtimes that contain the fix(cli): stabilize restored snapshot pairing verification #7834 force-pairing patches. Exact-literal migration tests protect the supported legacy patch shapes.
  • Tolerate atomic replacement of OpenClaw pairing-state files while retaining descriptor-backed validation.
  • Add fresh-onboarding regressions and map the pairing invariant to issue-4462-scope-upgrade-approval for deterministic E2E selection.
  • Publish the restored clone's operator.write pairing request with gateway call sessions.create before approval. Remove shared gateway credentials only from that provoke child so OpenClaw uses the clone's stored device credential. The coordinator then uses fix(cli): stabilize restored snapshot pairing verification #7834's one strict approval and authenticated verification; it does not add an approval retry or broader scope.

Requested Live E2E

  • Target: snapshot-commands
  • Reason: This target exercises cross-sandbox restore and previously returned restored-pairing-unverified after the strict approval could not list a pending request.
  • Acceptance: The targeted clone restore returns restored, verifies gateway pairing, and completes the no-leak lifecycle.

Type of Change

  • 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

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: This repairs the implementation of the documented restore and onboarding contracts. It does not change a public command, configuration, default, output, procedure, or supported surface.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Codex CLI reviewed all nine security categories at head 4979e10b1 and reported PASS with no findings. Ordinary pairing cannot enter the descriptor-backed clone path. Restored-clone pairing still requires the expected device identity, pinned loopback URL, descriptor-backed state, exact pairing transition, and scoped token. The warm-up removes shared credentials from the provoke child, can publish a request, but cannot approve it. The strict approval and authenticated verifier remain the authorization and success conditions.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: Independently reviewed the completed diff at head 4979e10b1c4de760f1c39bad69a260f77aa181a0 against AGENTS.md blob c052d60aa2b486bac1235643a56c2d2b1e5c13d5. No findings. Removing shared gateway credentials from this internal provoke child does not change the CLI, configuration, defaults, output, recovery procedure, or supported behavior. Existing restore documentation remains accurate. The reviewer examined 35 focused restore/approval tests, 10 changed tests, CLI typecheck, Biome, and normal hooks.
  • Agent: Codex CLI documentation-writer subagent

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable; scripts/prepare-dgx-station-host.sh is unchanged.
  • Station profile/scenario: Not applicable.
  • Result: Not applicable.
  • Supporting evidence: Not applicable.

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • 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
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project cli for the six restored-pairing warm-up/approval suites passed 35 tests; npm run test:changed passed 10 tests; the focused OpenClaw patch integration run on the preceding PR head passed 71 tests with one opt-in real-distribution test skipped; npm run typecheck:cli, Biome, and normal pre-commit/commit-msg/pre-push hooks passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: An earlier npm test run reached the 20-minute local bound after failures in untouched host/runtime-dependent gateway, Docker, Hermes image, package-link, and E2E workflow tests. No maintainer waiver is claimed. The changed suites and normal hooks pass, and CI remains authoritative.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

  • New Features

    • Added dedicated restored-clone pairing support for device identity loading, paired-token selection, and upgrade flows.
    • Preserved compatibility with existing forced-pairing behavior.
  • Bug Fixes

    • Improved handling of restored pairing state during failures, retries, and patch upgrades.
    • Increased resilience when device metadata files are replaced during approval.
    • Added recovery for pending scope-upgrade pairing approvals.
    • Ensured fresh onboarding pairs exactly one CLI identity without pending requests.
  • Tests

    • Added regression coverage for restored-clone pairing, identity resolution, token selection, upgrades, retries, and concurrent file replacement.

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 added the area: security Security controls, permissions, secrets, or hardening label Jul 31, 2026
@sandl99 sandl99 self-assigned this Jul 31, 2026
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds restored-clone pairing support across patching, warm-up, approval, identity loading, CLI token selection, fixtures, and regression tests.

Changes

Restored-Clone Pairing Support

Layer / File(s) Summary
Patch script identity and CLI migration
scripts/patch-openclaw-device-self-approval.mts
Adds restored-clone identity conditions and migrates legacy forced-pairing markers.
Restored-clone warm-up and gateway pairing
src/lib/actions/sandbox/auto-pair-warmup.ts, src/lib/actions/sandbox/restore-gateway-pairing.ts, src/lib/actions/sandbox/*test.ts
Adds direct sessions.create warm-up execution and wires it into restored gateway pairing.
Approval state and pairing flags
src/lib/actions/sandbox/auto-pair-approval.ts, src/lib/actions/sandbox/auto-pair-approval.test.ts, src/lib/actions/sandbox/auto-pair-approval-script.test.ts
Uses the restored-clone flag and clears both pairing flags across approval paths.
Identity fixtures and regression contracts
test/helpers/*, test/openclaw-device-self-approval-*.test.ts, test/e2e/live/issue-4462-scope-upgrade-approval.test.ts
Validates identity resolution, patch upgrades, proof execution, and fresh onboarding state.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AutoPairApproval
  participant SandboxWarmup
  participant OpenClawGateway
  participant PatchScript
  participant DevicesCLI
  AutoPairApproval->>SandboxWarmup: run restored-clone warm-up
  SandboxWarmup->>OpenClawGateway: call sessions.create
  PatchScript->>OpenClawGateway: apply restored identity handling
  PatchScript->>DevicesCLI: select restored paired token
  DevicesCLI-->>AutoPairApproval: return paired token
Loading

Possibly related PRs

Suggested labels: area: onboarding, area: e2e, bug-fix

Suggested reviewers: cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: restoring fresh onboarding device pairing.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/fresh-onboard-device-pairing

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 4979e10 in the fix/fresh-onboard-de... branch remains at 96%, unchanged from commit 7eb0369 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 4979e10 in the fix/fresh-onboard-de... branch remains at 81%, unchanged from commit 7eb0369 in the main branch.

Show a code coverage summary of the most impacted files.
File main 7eb0369 fix/fresh-onboard-de... 4979e10 +/-
src/lib/onboard/docker-cdi.ts 80% 70% -10%
src/lib/onboard...host-anchors.ts 94% 90% -4%
src/lib/onboard/preflight.ts 80% 80% 0%
src/lib/actions...orchestrator.ts 83% 87% +4%
src/lib/onboard...box-gpu-mode.ts 92% 97% +5%
src/lib/domain/.../connect-env.ts 89% 97% +8%
src/lib/actions...confirmation.ts 69% 79% +10%
src/lib/actions...ocker-health.ts 65% 82% +17%
src/lib/onboard...box-prebuild.ts 74% 92% +18%
src/lib/onboard...flight-ports.ts 33% 67% +34%

Updated July 31, 2026 14:16 UTC

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 1 warning · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Failed after a partial review · low confidence · 2 blockers · 1 warning · 0 suggestions

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: issue-4462-scope-upgrade-approval, onboard-repair, onboard-resume, rebuild-openclaw, snapshot-commands, state-backup-restore

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Document the restored-clone warm-up workaround boundary

  • Location: src/lib/actions/sandbox/auto-pair-warmup.ts:130
  • Category: correctness
  • Problem: The restored-clone direct warm-up has only a brief behavior comment. It does not record the invalid state, creation point, upstream constraint, regression proof, or removal condition required for a workaround on the pairing path.
  • Impact: Future changes can preserve or broaden the direct gateway call without knowing the narrow state it repairs or when it is safe to remove.
  • Recommendation: Add a source-of-truth comment for the restored-clone warm-up that names the pre-approval state, its creation point, the unavailable upstream behavior, the checked-in regression coverage, and the removal condition.
  • Verification: Read src/lib/actions/sandbox/auto-pair-warmup.ts:130-141 and verify the comment identifies the invalid state, creation point, upstream limitation, regression proof, and removal condition.
  • Test coverage: Existing auto-pair-warmup tests should name the restored-clone transition that must create a pending write-scope request before local approval; retain that test when the workaround is removed.
  • Evidence: src/lib/actions/sandbox/auto-pair-warmup.ts:189 introduces runRestoredSandboxScopeWarmupRun with only “Publish a restored clone's write-scope request without an embedded fallback.” The detailed SOURCE_OF_TRUTH_REVIEW comment in src/lib/actions/sandbox/auto-pair-approval.ts applies to local pending-state approval, not the new direct warm-up call. src/lib/actions/sandbox/auto-pair-warmup.ts:130-141 defines RESTORED_CLONE_WARMUP_SCRIPT and describes its direct gateway call and credential handling. src/lib/actions/sandbox/auto-pair-warmup.ts:189-191 only exports the wrapper that invokes the script.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@sandl99
sandl99 marked this pull request as draft July 31, 2026 10:16
@sandl99
sandl99 marked this pull request as ready for review July 31, 2026 10:16
@copy-pr-bot

copy-pr-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown

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.

Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/auto-pair-approval.ts (1)

237-240: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Clear inherited pairing markers before ordinary approval.

gateway_approval_env removes only OPENCLAW_GATEWAY_* variables. The ordinary path can inherit both NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING and NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING. Clear both markers before invoking the approval command.

🤖 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 `@src/lib/actions/sandbox/auto-pair-approval.ts` around lines 237 - 240, Update
the ordinary approval path that assigns approve_env via gateway_approval_env to
remove both NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING and
NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING from the inherited environment before
invoking the approval command, matching the cleanup already performed in the
else branch.
🧹 Nitpick comments (1)
src/lib/actions/sandbox/auto-pair-approval.ts (1)

224-225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting the repeated pairing-flag reset into a helper.

The pattern approve_env.pop('NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING', None) followed by approve_env.pop('NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING', None) (or the corresponding set) repeats at Line 224-225, Line 238-239, Line 245-246, Line 260-261, and Line 278-279. If a future change adds a third pairing mode or another env var, a missed site can reintroduce mode-confusion between ordinary and restored-clone pairing, the exact class of regression this PR fixes.

Extract a small Python helper (e.g., def reset_pairing_flags(env): ...) generated once in the template and call it from each site instead of repeating the two-line pop pattern.

Also applies to: 237-246, 260-261, 278-279

🤖 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 `@src/lib/actions/sandbox/auto-pair-approval.ts` around lines 224 - 225,
Extract the repeated pairing-flag updates in the generated Python template into
a single helper, such as reset_pairing_flags(env), that removes both
NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING and
NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING. Define the helper once and replace
each repeated reset or corresponding set sequence at the affected pairing sites
with calls to it, preserving the existing ordinary and restored-clone mode
behavior.
🤖 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.

Outside diff comments:
In `@src/lib/actions/sandbox/auto-pair-approval.ts`:
- Around line 237-240: Update the ordinary approval path that assigns
approve_env via gateway_approval_env to remove both
NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING and
NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING from the inherited environment before
invoking the approval command, matching the cleanup already performed in the
else branch.

---

Nitpick comments:
In `@src/lib/actions/sandbox/auto-pair-approval.ts`:
- Around line 224-225: Extract the repeated pairing-flag updates in the
generated Python template into a single helper, such as
reset_pairing_flags(env), that removes both
NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING and
NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING. Define the helper once and replace
each repeated reset or corresponding set sequence at the affected pairing sites
with calls to it, preserving the existing ordinary and restored-clone mode
behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e719d4f5-32c3-4f4b-ad5e-4a02c0062c7d

📥 Commits

Reviewing files that changed from the base of the PR and between 172e846 and b282da9.

📒 Files selected for processing (3)
  • src/lib/actions/sandbox/auto-pair-approval-script.test.ts
  • src/lib/actions/sandbox/auto-pair-approval.test.ts
  • src/lib/actions/sandbox/auto-pair-approval.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/actions/sandbox/auto-pair-approval-script.test.ts
  • src/lib/actions/sandbox/auto-pair-approval.test.ts

Signed-off-by: San Dang <sdang@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/auto-pair-approval.test.ts (1)

524-528: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the pairing modes mutually exclusive in the child environment.

This test sets NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING, but it also inherits NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING from process.env. Since the restored-clone flag now controls forced-authentication detection, both flags can make this test exercise the wrong mode.

Remove the restored-clone flag before spawning the child process.

Proposed fix
+        const {
+          NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING: _restoredClonePairing,
+          ...baseEnv
+        } = process.env;
         return spawnSync("sh", {
           encoding: "utf-8",
           input: approvalScript,
           env: {
-            ...process.env,
+            ...baseEnv,
             PATH: `${tmpDir}:/usr/bin:/bin`,
             NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING: "1",
🤖 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 `@src/lib/actions/sandbox/auto-pair-approval.test.ts` around lines 524 - 528,
Remove NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING from the child process
environment in the test setup around the environment object containing
NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING. Ensure the spawned child inherits no
restored-clone flag so the test exercises forced device pairing exclusively.
🤖 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.

Outside diff comments:
In `@src/lib/actions/sandbox/auto-pair-approval.test.ts`:
- Around line 524-528: Remove NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING from the
child process environment in the test setup around the environment object
containing NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING. Ensure the spawned child
inherits no restored-clone flag so the test exercises forced device pairing
exclusively.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bb32f3e0-9445-4c20-86cf-734b3be0bd41

📥 Commits

Reviewing files that changed from the base of the PR and between b282da9 and 849323c.

📒 Files selected for processing (5)
  • src/lib/actions/sandbox/auto-pair-approval-script.test.ts
  • src/lib/actions/sandbox/auto-pair-approval.test.ts
  • src/lib/actions/sandbox/auto-pair-approval.ts
  • src/lib/actions/sandbox/restore-gateway-pairing.test.ts
  • src/lib/actions/sandbox/restore-gateway-pairing.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/actions/sandbox/auto-pair-approval-script.test.ts
  • src/lib/actions/sandbox/auto-pair-approval.ts

Signed-off-by: San Dang <sdang@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/restore-gateway-pairing.ts (1)

137-146: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

The restore entrypoint lacks a bounded retry wrapper for pairing failures.

The snapshot restore function (runSnapshotRestore) calls establishRestoredSandboxGatewayPairing exactly once (line 1178–1187 of snapshot.ts). When pairing fails, the restore throws an error and directs the user to run ${CLI_NAME} ${targetSandbox} connect to retry. This places the retry burden on the user, not on the restore operation itself.

establishRestoredSandboxGatewayPairing correctly removes internal retry (per its comment at line 137–138), but the public restore caller does not implement a bounded automatic retry loop. The connect command provides manual recovery through runConnectAutoPairApprovalPass, which is best-effort only and not a bounded pairing reestablishment.

Verify that transient pairing-state rotation during restore does not become a user-facing failure. If bounded automatic retries are required by the PR objective, add retry logic to the restore entrypoint or ensure the requirement is documented and deliberately deferred.

🤖 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 `@src/lib/actions/sandbox/restore-gateway-pairing.ts` around lines 137 - 146,
Update the runSnapshotRestore caller around
establishRestoredSandboxGatewayPairing to perform a bounded automatic retry for
transient pairing failures, while keeping
establishRestoredSandboxGatewayPairing’s single-attempt behavior unchanged.
Retry only the pairing operation, stop after the defined limit, and preserve the
existing user-facing failure path when all attempts are exhausted.

Source: Path instructions

🧹 Nitpick comments (1)
src/lib/actions/sandbox/auto-pair-warmup.test.ts (1)

73-82: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift

Add behavioral coverage through the public warm-up entrypoint.

This test checks RESTORED_CLONE_WARMUP_SCRIPT text only. It does not prove that runRestoredSandboxScopeWarmupRun passes this script to openshell sandbox exec or that the sandbox executes sessions.create. A wiring regression can therefore pass these assertions. Add a test through the public entrypoint with a controlled adapter boundary and assert the observable command payload. Keep the string checks as supplemental contract checks.

As per path instructions, tests must prefer observable outcomes and prove that public entrypoints reach the replacement path.

🤖 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 `@src/lib/actions/sandbox/auto-pair-warmup.test.ts` around lines 73 - 82, Add
behavioral coverage for the public runRestoredSandboxScopeWarmupRun entrypoint
using a controlled adapter boundary, asserting that openshell sandbox exec
receives the restored-clone warm-up script and its payload performs
sessions.create. Retain the existing RESTORED_CLONE_WARMUP_SCRIPT string
assertions as supplemental contract checks, while verifying the public
entrypoint reaches the replacement execution path.

Source: Path instructions

🤖 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 `@src/lib/actions/sandbox/auto-pair-warmup.ts`:
- Around line 142-143: Update the restored-clone warm-up command in the
auto-pair flow to export NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING=1 alongside
NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING=1, ensuring the descriptor-backed clone
identity is used. Add a regression assertion verifying this environment variable
is present for the warm-up subprocess.

---

Outside diff comments:
In `@src/lib/actions/sandbox/restore-gateway-pairing.ts`:
- Around line 137-146: Update the runSnapshotRestore caller around
establishRestoredSandboxGatewayPairing to perform a bounded automatic retry for
transient pairing failures, while keeping
establishRestoredSandboxGatewayPairing’s single-attempt behavior unchanged.
Retry only the pairing operation, stop after the defined limit, and preserve the
existing user-facing failure path when all attempts are exhausted.

---

Nitpick comments:
In `@src/lib/actions/sandbox/auto-pair-warmup.test.ts`:
- Around line 73-82: Add behavioral coverage for the public
runRestoredSandboxScopeWarmupRun entrypoint using a controlled adapter boundary,
asserting that openshell sandbox exec receives the restored-clone warm-up script
and its payload performs sessions.create. Retain the existing
RESTORED_CLONE_WARMUP_SCRIPT string assertions as supplemental contract checks,
while verifying the public entrypoint reaches the replacement execution path.
🪄 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: 67763668-42f3-4ac5-814e-add4fd7614b6

📥 Commits

Reviewing files that changed from the base of the PR and between 849323c and 5b3f971.

📒 Files selected for processing (5)
  • src/lib/actions/sandbox/auto-pair-approval.ts
  • src/lib/actions/sandbox/auto-pair-warmup.test.ts
  • src/lib/actions/sandbox/auto-pair-warmup.ts
  • src/lib/actions/sandbox/restore-gateway-pairing.test.ts
  • src/lib/actions/sandbox/restore-gateway-pairing.ts
💤 Files with no reviewable changes (1)
  • src/lib/actions/sandbox/restore-gateway-pairing.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/actions/sandbox/auto-pair-approval.ts

Comment on lines +142 to +143
NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING=1 \\
openclaw gateway call sessions.create --params "$params" --json >/dev/null 2>&1 || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 6 \
  'NEMOCLAW_OPENCLAW_(FORCE_DEVICE_PAIRING|RESTORED_CLONE_PAIRING)|RESTORED_CLONE_WARMUP_SCRIPT|runRestoredSandboxScopeWarmupRun' \
  .

Repository: NVIDIA/NemoClaw

Length of output: 50371


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- auto-pair-warmup outline ---'
ast-grep outline src/lib/actions/sandbox/auto-pair-warmup.ts
printf '%s\n' '--- warmup implementation ---'
sed -n '1,215p' src/lib/actions/sandbox/auto-pair-warmup.ts
printf '%s\n' '--- restore pairing callers and environment setup ---'
rg -n -C 10 \
  'warmupScopeUpgrade|runRestoredSandboxScopeWarmupRun|NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING|runSandboxWarmupScript|openshell.*run|sandbox exec' \
  src/lib/actions/sandbox test \
  | head -n 500
printf '%s\n' '--- warmup tests ---'
rg -n -C 12 \
  'RESTORED_CLONE_WARMUP_SCRIPT|runRestoredSandboxScopeWarmupRun|warmupScopeUpgrade|auto-pair-warmup' \
  src test

Repository: NVIDIA/NemoClaw

Length of output: 50372


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- warmup tests ---'
sed -n '1,260p' src/lib/actions/sandbox/auto-pair-warmup.test.ts

printf '%s\n' '--- restore pairing action ---'
sed -n '1,240p' src/lib/actions/sandbox/restore-gateway-pairing.ts

printf '%s\n' '--- exact warmup call sites ---'
rg -n -C 18 \
  'warmupScopeUpgrade|runRestoredSandboxScopeWarmupRun|runSandboxScopeWarmupRun' \
  src/lib --glob '*.ts' --glob '!**/*.test.ts'

printf '%s\n' '--- restored-clone environment construction ---'
rg -n -C 14 \
  'NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING|NEMOCLAW_OPENCLAW_IDENTITY_FD|buildAutoPairApprovalScript|approveRestoredClonePairing' \
  src/lib/actions/sandbox/restore-gateway-pairing.ts \
  src/lib/actions/sandbox/auto-pair-approval.ts \
  src/lib/actions/sandbox/*.ts \
  | head -n 500

Repository: NVIDIA/NemoClaw

Length of output: 50371


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- all production assignments and exports ---'
rg -n \
  '(^|[[:space:];])export[[:space:]]+NEMOCLAW_OPENCLAW_(FORCE_DEVICE_PAIRING|RESTORED_CLONE_PAIRING)|NEMOCLAW_OPENCLAW_(FORCE_DEVICE_PAIRING|RESTORED_CLONE_PAIRING)[[:space:]]*=' \
  src scripts \
  --glob '*.ts' --glob '*.mts' --glob '*.sh' --glob '*.py'

printf '%s\n' '--- sandbox environment and startup wiring ---'
rg -n -C 10 \
  'proxy-env|PROXY_ENV|sandbox.*env|environment|env=|OPENCLAW_STATE_DIR|NEMOCLAW_OPENCLAW_' \
  src/lib/adapters src/lib/actions src/lib/onboard scripts \
  --glob '*.ts' --glob '*.mts' --glob '*.py' --glob '*.sh' \
  | head -n 700

printf '%s\n' '--- patched identity resolution semantics ---'
sed -n '89,185p' scripts/patch-openclaw-device-self-approval.mts
sed -n '490,525p' scripts/patch-openclaw-device-self-approval.mts

Repository: NVIDIA/NemoClaw

Length of output: 50371


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- identity resolver patch ---'
sed -n '115,155p' scripts/patch-openclaw-device-self-approval.mts
sed -n '160,205p' scripts/patch-openclaw-device-self-approval.mts

printf '%s\n' '--- paired-token context patch ---'
sed -n '495,535p' scripts/patch-openclaw-device-self-approval.mts

printf '%s\n' '--- production restored-clone environment evidence ---'
rg -n \
  'NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING|NEMOCLAW_OPENCLAW_IDENTITY_FD|NEMOCLAW_OPENCLAW_EXPECTED_DEVICE_ID' \
  src scripts \
  --glob '*.ts' --glob '*.mts' --glob '*.sh' --glob '*.py' \
  | grep -v '/test' \
  | head -n 250

Repository: NVIDIA/NemoClaw

Length of output: 11528


Set NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING=1 for the restored-clone warm-up. The host process does not export this variable, and the later approval subprocess cannot affect the earlier warm-up. With only NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING=1, OpenClaw resolves the default identity instead of the descriptor-backed clone identity. Add a regression assertion for the environment.

🤖 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 `@src/lib/actions/sandbox/auto-pair-warmup.ts` around lines 142 - 143, Update
the restored-clone warm-up command in the auto-pair flow to export
NEMOCLAW_OPENCLAW_RESTORED_CLONE_PAIRING=1 alongside
NEMOCLAW_OPENCLAW_FORCE_DEVICE_PAIRING=1, ensuring the descriptor-backed clone
identity is used. Add a regression assertion verifying this environment variable
is present for the warm-up subprocess.

Signed-off-by: San Dang <sdang@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

Superseded by #8015. The replacement preserves all six commits from this PR with San Dang's authorship and Signed-off-by trailers, then adds the bounded restored-clone recovery needed to protect both fresh onboarding and snapshot restore. Thank you @sandl99 for the diagnosis and original repair.

cv pushed a commit that referenced this pull request Jul 31, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Fresh onboarding and restored snapshots now use separate OpenClaw
pairing modes. Fresh onboarding keeps the default generated identity
path, while restored clones use descriptor-pinned identity and pairing
state plus one narrowly bounded recovery when the first verifier
publishes the missing scope-upgrade request.

This replacement carries forward and credits all six signed commits from
#7997, then adds the bounded recovery needed to keep the fresh-onboard
and restored-clone E2E paths from regressing one another.

## Related Issue

Supersedes #7997 and follows up the pairing regression introduced while
fixing #7834. The regression is exercised by
`issue-4462-scope-upgrade-approval` and `snapshot-e2e`.

## Changes

- Separate ordinary forced pairing from restored-clone descriptor-backed
pairing.
- Use clone device authentication for the restored-clone write-scope
warm-up.
- Distinguish an absent clone pending file from malformed or unreadable
pairing state.
- Retry approval exactly once only after `list-pending-unavailable` and
an authenticated `scope-upgrade-pending` verification result.
- Keep malformed, ambiguous, mismatched, or unreadable state fail
closed.
- Add focused unit, patch-upgrade, and live E2E contract coverage for
both pairing modes.

## 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: This repairs internal
OpenClaw pairing orchestration without changing a supported command,
configuration, default, or documented workflow.
- [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: Codex Desktop reviewed
the exact head against all nine security categories and found no
blocking or non-blocking findings:
#8015 (comment)
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `no-docs-needed`
- Evidence: The completed diff changes internal OpenClaw pairing repair
behavior only; no public command, configuration, default, error
contract, or documentation workflow changed.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 5be51ee -->
<!-- docs-review-agents-blob-sha: c052d60 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit:
- Station profile/scenario:
- Result:
- Supporting evidence:

## 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 — 35/35 exact-head focused tests passed;
the affected-test run passed 224/224 before the unchanged rebase; `npm
run validate:pr` passed on the current head.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: `npm test` built
successfully and passed 23,255 tests locally, with 113 host-dependent
failures on macOS/Node 25; required Linux CI and trusted live E2E are
authoritative and pending.
- [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: Prekshi Vyas <prekshiv@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Improved restored-device pairing with dedicated handling for cloned
environments.
  * Added a warm-up step to request required permissions before pairing.
* Added clearer status reporting when pending pairing information is
unavailable.
* **Bug Fixes**
* Pairing can now retry when an additional permission approval is
needed.
* Preserved authentication appropriately across restored-device pairing
flows.
* Improved compatibility when upgrading existing pairing configurations.
* **Tests**
* Added coverage for restored-device pairing, retries, unavailable
states, and legacy configuration upgrades.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: San Dang <sdang@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: security Security controls, permissions, secrets, or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants