-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(onboard): warm up scope upgrade so first agent run avoids fallback #5323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
9e0b7f7
fix(onboard): warm up scope upgrade so first agent run avoids fallback
TonyLuo-NV 62670c5
Merge remote-tracking branch 'upstream/main' into fix/4504-v2-onboard…
TonyLuo-NV 3ede0cb
fix(onboard): keep scope warm-up best-effort when OpenShell is missing
TonyLuo-NV a422d46
fix(onboard): trim scope warm-up poll budget under the outer timeout
TonyLuo-NV a0a8e66
fix(onboard): short-circuit scope warm-up poll when write already gra…
TonyLuo-NV ae22d53
Merge branch 'main' into fix/4504-v2-onboard-scope-warmup
cv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { spawnSync } from "node:child_process"; | ||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| import { wrapSandboxShellScript } from "./auto-pair-approval"; | ||
| import { WARMUP_TIMEOUT_MS } from "./auto-pair-warmup"; | ||
|
|
||
| // NOTE on coverage shape (#4504-v2): `runSandboxScopeWarmupRun` is not exercised | ||
| // in-process here. Like its sibling `runSandboxAutoPairApprovalPass`, the leaf | ||
| // lazily does a raw `require("../../adapters/openshell/runtime")` — a native | ||
| // CJS require of a relative `.ts` path that Vitest's module-mock registry does | ||
| // not intercept (mocking `node:child_process` to inspect the spawn args makes | ||
| // the source resolve that require through native Node, which then fails with | ||
| // "Cannot find module"). The same constraint is why | ||
| // `auto-pair-approval.test.ts` only unit-tests the pure exports and leaves the | ||
| // spawn/wiring path to the `test/sandbox-connect-inference/` integration | ||
| // harness (real compiled CLI + fake openshell on PATH). These cases therefore | ||
| // pin the contract surface that IS testable in-process — the timeout bound and | ||
| // the OpenShell-exec wrapping the leaf depends on — and the | ||
| // finalization.test.ts ordering tests pin the provoke→approve wiring. | ||
|
|
||
| describe("scope-upgrade warm-up timeout bound (#4504-v2)", () => { | ||
| it("uses a fixed 30s outer cap so a wedged warm-up can never block onboard", () => { | ||
| // The `-m "ping"` one-shot returns fast even when it falls back to embedded | ||
| // mode; 30s covers gateway-connect + the scope-upgrade request plus | ||
| // shell/agent startup while still bounding a hung sandbox. The constant is a | ||
| // dependency-free export so this assertion stays in-process. | ||
| expect(WARMUP_TIMEOUT_MS).toBe(30_000); | ||
| expect(typeof WARMUP_TIMEOUT_MS).toBe("number"); | ||
| expect(WARMUP_TIMEOUT_MS).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it("stays within the bounds the contract budgeted for finalization latency", () => { | ||
| // The architect budgeted worst-case added finalization latency at the | ||
| // warm-up cap (<=30s) plus the existing 15s approval pass. Guard that the | ||
| // warm-up cap has not crept past its 30s ceiling — anything larger would | ||
| // blow the budget the contract signed off on for a one-time onboard. | ||
| expect(WARMUP_TIMEOUT_MS).toBeLessThanOrEqual(30_000); | ||
| }); | ||
| }); | ||
|
|
||
| describe("warm-up payload survives OpenShell exec (#4504-v2)", () => { | ||
| // The leaf wraps its in-sandbox script with the shared `wrapSandboxShellScript` | ||
| // (OpenShell exec rejects newline-bearing args). These cases pin that wrapper | ||
| // contract — the exact mechanism the warm-up exec relies on — without needing | ||
| // the un-mockable lazy require. | ||
| it("encodes a multi-line warm-up-shaped payload onto a single newline-free line", () => { | ||
| const warmupShaped = [ | ||
| "command -v openclaw >/dev/null 2>&1 || exit 0", | ||
| 'openclaw agent --agent main -m "ping" \\', | ||
| ' --session-id "nemoclaw-onboard-warmup-$$-$(date +%s)" >/dev/null 2>&1 || true', | ||
| "exit 0", | ||
| "", | ||
| ].join("\n"); | ||
| const wrapped = wrapSandboxShellScript(warmupShaped); | ||
| expect(wrapped).not.toMatch(/[\n\r]/); | ||
| expect(wrapped).toContain("base64 -d"); | ||
| expect(wrapped).toContain("mktemp"); | ||
| }); | ||
|
|
||
| it("round-trips a warm-up-shaped payload and preserves its exit-0 status when run", () => { | ||
| // Mirror the real warm-up: the provoke command itself may "fail" (the agent | ||
| // falls back to embedded mode), but `|| true` + trailing `exit 0` mean the | ||
| // wrapped script always exits 0 — so a failed provoke never surfaces as a | ||
| // nonzero status to the onboard path. Use `false` to stand in for the failing | ||
| // openclaw run. | ||
| const inner = ["false || true", "exit 0", ""].join("\n"); | ||
| const wrapped = wrapSandboxShellScript(inner); | ||
| const result = spawnSync("sh", ["-c", wrapped], { encoding: "utf-8", timeout: 10_000 }); | ||
| expect(result.status).toBe(0); | ||
| }); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| /** | ||
| * Onboard scope-upgrade warm-up (#4504-v2). | ||
| * | ||
| * The connect-time approval pass (`auto-pair-approval.ts`) is purely | ||
| * request-driven: it can only approve a scope upgrade that is already PENDING. | ||
| * During fresh onboard the device is auto-paired with `operator.pairing` only; | ||
| * the `operator.write` upgrade is not requested until the user's *first* real | ||
| * `openclaw agent` run — which happens *after* onboard finalization's approval | ||
| * pass already found nothing pending. The result is one silent embedded | ||
| * fallback on that first run, then `connect`/`recover` fixes it. | ||
| * | ||
| * This warm-up provokes the upgrade ourselves: it runs a single, throwaway, | ||
| * bounded `openclaw agent --agent main -m "ping"` inside the sandbox during | ||
| * finalization. That connects to the gateway exactly as the user's first run | ||
| * will, triggers the identical `operator.write` scope-upgrade request, and | ||
| * makes it PENDING. The existing `runConnectAutoPairApprovalPass` (run | ||
| * immediately after) then approves it, so `operator.write` is persisted before | ||
| * handoff and the user's first run connects clean. | ||
| * | ||
| * Contract: best-effort, non-blocking, idempotent. The warm-up run will itself | ||
| * fall back to embedded mode on this first invocation (EXIT 0) — that is | ||
| * expected; its output is discarded. Any failure (exec timeout, gateway not up, | ||
| * agent error) is swallowed so finalization is never blocked; behavior then | ||
| * degrades to the v1 first-run-falls-back-then-recover path, strictly no worse | ||
| * than today. On re-onboard where `operator.write` is already paired the run | ||
| * connects clean (no new pending) and the approval pass is a no-op. | ||
| * | ||
| * Workaround boundary (NemoClaw#4462): OpenClaw owns device-pairing semantics | ||
| * and exposes only `devices list/get/approve` — there is no way to pre-grant a | ||
| * scope the device has not requested. Remove this warm-up when OpenClaw can | ||
| * pre-approve the full scope set at pairing time. | ||
| */ | ||
|
|
||
| import { spawnSync } from "node:child_process"; | ||
|
|
||
| import { ROOT } from "../../state/paths"; | ||
| import { wrapSandboxShellScript } from "./auto-pair-approval"; | ||
|
|
||
| // Outer spawnSync cap (ms) for the throwaway warm-up agent run. The `-m` | ||
| // one-shot prompt ("ping") returns fast even when it falls back to embedded | ||
| // mode, so 30s comfortably covers gateway-connect + scope-upgrade request, the | ||
| // bounded pending-upgrade poll below, plus shell/agent startup, while never | ||
| // letting a wedged sandbox block onboard. | ||
| export const WARMUP_TIMEOUT_MS = 30_000; | ||
|
|
||
| // Bounded in-sandbox poll for the pending scope upgrade after the provoke run. | ||
| // Worst case = WARMUP_POLL_ATTEMPTS × WARMUP_POLL_LIST_TIMEOUT_S list calls plus | ||
| // (WARMUP_POLL_ATTEMPTS - 1) inter-attempt 1s sleeps = 5×2 + 4×1 = 14s, which | ||
| // leaves clear headroom under WARMUP_TIMEOUT_MS (30s) for shell startup and the | ||
| // throwaway agent run that runs first. The gateway persists the upgrade | ||
| // requestId once created (#4504 evidence), so once the poll sees it pending the | ||
| // downstream approval pass deterministically finds and approves it before | ||
| // handoff — making "very first real run, zero fallback" deterministic even on | ||
| // slow/contended gateways. | ||
| export const WARMUP_POLL_ATTEMPTS = 5; | ||
| export const WARMUP_POLL_LIST_TIMEOUT_S = 2; | ||
|
|
||
| // Best-effort in-sandbox warm-up script. Always exits 0. It connects to the | ||
| // gateway and provokes the `operator.write` scope-upgrade so the request is | ||
| // PENDING, then POLLS `devices list` until that allowlisted upgrade is visible | ||
| // (or the bounded deadline elapses) before returning — closing the race where | ||
| // the approval pass that runs immediately after could otherwise list devices | ||
| // before the gateway has registered the upgrade. The poll bounds are | ||
| // interpolated so the cap is asserted on real values, not source text. | ||
| const WARMUP_SCRIPT = ` | ||
| PROXY_ENV=/tmp/nemoclaw-proxy-env.sh | ||
| [ -r "$PROXY_ENV" ] && . "$PROXY_ENV" | ||
| command -v openclaw >/dev/null 2>&1 || exit 0 | ||
| openclaw agent --agent main -m "ping" \\ | ||
| --session-id "nemoclaw-onboard-warmup-$$-$(date +%s)" >/dev/null 2>&1 || true | ||
| command -v python3 >/dev/null 2>&1 || exit 0 | ||
| OPENCLAW_BIN="$(command -v openclaw)" | ||
| i=0 | ||
| while [ "$i" -lt ${WARMUP_POLL_ATTEMPTS} ]; do | ||
| OPENCLAW_BIN="$OPENCLAW_BIN" python3 - <<'PYPOLL' | ||
| import json | ||
| import os | ||
| import subprocess | ||
| import sys | ||
|
|
||
| OPENCLAW = os.environ.get('OPENCLAW_BIN', 'openclaw') | ||
| try: | ||
| proc = subprocess.run( | ||
| [OPENCLAW, 'devices', 'list', '--json'], | ||
| capture_output=True, text=True, timeout=${WARMUP_POLL_LIST_TIMEOUT_S}, | ||
| ) | ||
| except (subprocess.TimeoutExpired, FileNotFoundError, OSError): | ||
| sys.exit(1) | ||
| if proc.returncode != 0 or not proc.stdout.strip(): | ||
| sys.exit(1) | ||
| try: | ||
| data = json.loads(proc.stdout) | ||
| except ValueError: | ||
| sys.exit(1) | ||
| if not isinstance(data, dict): | ||
| sys.exit(1) | ||
| # Terminal success = operator.write is satisfied, whether it is a PENDING | ||
| # upgrade (the approval pass will grant it next) or ALREADY GRANTED on a | ||
| # re-onboard (idempotent no-op — nothing left to do before handoff). Scan | ||
| # every device collection the response exposes (pending plus any granted/ | ||
| # approved/paired/devices list, and any other top-level list of device dicts) | ||
| # rather than only 'pending', so the already-paired path short-circuits | ||
| # immediately instead of burning the whole poll budget. | ||
| devices = [] | ||
| for value in data.values(): | ||
| if isinstance(value, list): | ||
| devices.extend(d for d in value if isinstance(d, dict)) | ||
| for device in devices: | ||
| scopes = device.get('scopes') or device.get('requestedScopes') | ||
| if isinstance(scopes, str): | ||
| scopes = scopes.replace(',', ' ').split() | ||
| if isinstance(scopes, list) and 'operator.write' in scopes: | ||
| sys.exit(0) | ||
| sys.exit(1) | ||
| PYPOLL | ||
| if [ "$?" -eq 0 ]; then | ||
| break | ||
| fi | ||
| i=$((i + 1)) | ||
| [ "$i" -lt ${WARMUP_POLL_ATTEMPTS} ] && sleep 1 | ||
| done | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| exit 0 | ||
| `; | ||
|
|
||
| /** | ||
| * Run the bounded, throwaway scope-upgrade warm-up inside the named sandbox via | ||
| * `openshell sandbox exec`. All failure modes (timeout, sandbox-exec errors, | ||
| * missing openclaw, gateway unreachable) are swallowed: this is best-effort and | ||
| * must never throw — onboard finalization must not be blocked. | ||
| */ | ||
| export function runSandboxScopeWarmupRun(sandboxName: string): void { | ||
| // Lazy require: `adapters/openshell/resolve` pulls in `runner`, whose | ||
| // load-time `require("./platform")` cannot be resolved by the Vitest TS | ||
| // loader. Importing it here keeps this module unit-testable in-process. | ||
| // Use `resolveOpenshell` (returns null) rather than `getOpenshellBinary`, | ||
| // which `process.exit(1)`s when the CLI is missing — that fail-fast escapes | ||
| // this try/catch and would turn the best-effort warm-up into a hard onboard | ||
| // exit. A missing OpenShell here is a no-op instead. | ||
| const { resolveOpenshell } = | ||
| require("../../adapters/openshell/resolve") as typeof import("../../adapters/openshell/resolve"); | ||
| try { | ||
| const openshellBinary = resolveOpenshell(); | ||
| if (!openshellBinary) return; | ||
| spawnSync( | ||
| openshellBinary, | ||
| [ | ||
| "sandbox", | ||
| "exec", | ||
| "--name", | ||
| sandboxName, | ||
| "--", | ||
| "sh", | ||
| "-c", | ||
| wrapSandboxShellScript(WARMUP_SCRIPT), | ||
| ], | ||
| { | ||
| cwd: ROOT, | ||
| env: process.env, | ||
| stdio: ["ignore", "ignore", "ignore"], | ||
| timeout: WARMUP_TIMEOUT_MS, | ||
| }, | ||
| ); | ||
| } catch { | ||
| /* defense-in-depth — never throw from the onboard finalization path */ | ||
| } | ||
| } | ||
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
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.