feat(cua): add gated candidate install readiness - #8484
Open
jyaunches wants to merge 19 commits into
Open
Conversation
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
added
area: docs
|
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
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds the CUA lifecycle contract, runtime manifest validation, readiness authority, target/security/task commands, reconciliation and registry persistence, isolated qualification execution, GPU bootstrap, public status projections, documentation, and extensive unit, integration, and end-to-end coverage. ChangesCUA lifecycle and qualification
Estimated code review effort: 5 (Critical) | ~180 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
marked this pull request as ready for review
August 6, 2026 16:25
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
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/inference-set.test-support.ts (1)
176-193: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse distinct spies for generic and route-specific registry updates.
createDepsassigns both methods to onevi.fn, so a call toupdateSandboxsatisfies the route-specific assertion. Create separate spies and assert thatupdateSandboxis not called ininference-set-openclaw-run.test.ts.🤖 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/inference-set.test-support.ts` around lines 176 - 193, In src/lib/actions/inference-set.test-support.ts:176-193, update createDeps to create distinct vi.fn spies for generic updateSandbox and route-specific updateSandboxInferenceRoute instead of assigning both to updateSandboxInferenceRoute; preserve each option’s configured fallback behavior. In src/lib/actions/inference-set-openclaw-run.test.ts:54-61, add an assertion that the generic updateSandbox spy is not called, while retaining the route-specific assertion.
🟡 Minor comments (13)
src/lib/actions/sandbox/destroy.ts-466-467 (1)
466-467: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the CUA recovery commands.
The displayed commands do not match the
sandbox:cua:*command hierarchy. They omitsandboxand place the sandbox name before the command. Users cannot follow this recovery path as written.Proposed fix
- ` Run '${CLI_NAME} ${sandboxName} cua target health', then cancel any observed task and run target destroy before destroying the sandbox.`, + ` Run '${CLI_NAME} sandbox cua target health ${sandboxName}', then cancel any observed task and run '${CLI_NAME} sandbox cua target destroy ${sandboxName}' before destroying the sandbox.`,🤖 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/destroy.ts` around lines 466 - 467, The recovery guidance in the destroy error message should use the correct sandbox:cua command hierarchy. Update the command text near CLI_NAME to include sandbox and place sandboxName after the appropriate CUA command, for both the target health and target destroy instructions.src/lib/actions/sandbox/snapshot-restore-lifecycle.test.ts-144-161 (1)
144-161: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAssert that the restore actually ran.
The
?? Number.POSITIVE_INFINITYfallback at Line 159 makes the ordering assertion pass whenrestoreSandboxStateMockwas never called. The test then proves ordering without proving that the restore proceeded. Add an explicit call assertion so the claim in the title holds.💚 Proposed fix to remove the vacuous pass
+ expect(f.restoreSandboxStateMock).toHaveBeenCalled(); expect(f.updateSandboxMock.mock.invocationCallOrder[0]).toBeLessThan( - f.restoreSandboxStateMock.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY, + f.restoreSandboxStateMock.mock.invocationCallOrder[0], );🤖 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/snapshot-restore-lifecycle.test.ts` around lines 144 - 161, Add an explicit assertion in the “invalidates readiness-only CUA authority before a snapshot restore” test that restoreSandboxStateMock was called, then retain the invocation-order assertion without relying on the Number.POSITIVE_INFINITY fallback. Ensure the test verifies both that restore proceeded and that updateSandboxMock ran first.src/lib/actions/sandbox/doctor.ts-614-616 (1)
614-616: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRemove the non-null assertion; it can place
nullinto the check list.
cuaReconciliationDoctorCheckreturnsnullat Line 549 whengetCuaReconciliationForProjectionyields a falsy record. A truthysb.cuaReconciliationdoes not guarantee a projectable record, so the assertion can addnulltoDoctorCheck[]. Downstream renderers then dereference a null check.🛡️ Proposed fix
- if (sb?.cuaReconciliation) { - return [cuaReconciliationDoctorCheck(sandboxName, sb)!]; - } + if (sb?.cuaReconciliation) { + const reconciliation = cuaReconciliationDoctorCheck(sandboxName, sb); + if (reconciliation) return [reconciliation]; + }🤖 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/doctor.ts` around lines 614 - 616, Remove the non-null assertion from the cuaReconciliationDoctorCheck call in the sb?.cuaReconciliation branch, and only return an array containing the check when the function returns a non-null result; otherwise return an empty check list. Preserve the existing behavior when no reconciliation configuration is present.test/cua-task-cli.test.ts-406-456 (1)
406-456: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that the adapter did not run in the three input-validation tests.
The three test titles claim rejection happens "before invoking the adapter", but the assertions only check the exit code and failure family. A regression that invokes the adapter first and fails afterward would still pass. The fixture adapter writes
${HOME}/.cua-task-fixture-state.jsonontask.start, so its absence is an observable proof of the claim.Apply the same assertion in the invalid-UTF-8 test (lines 406-421), the symbolic-link test (lines 423-439), and the oversized-input test (lines 441-456).
💚 Proposed assertion
expect(started.status).toBe(2); expect(JSON.parse(started.stdout)).toMatchObject({ kind: "failure", family: "validation_failed", }); + expect(fs.existsSync(path.join(home, ".cua-task-fixture-state.json"))).toBe(false); });As per path instructions: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."
🤖 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 `@test/cua-task-cli.test.ts` around lines 406 - 456, Update the three input-validation tests around the invalid UTF-8, symbolic-link, and oversized-input cases to assert that the fixture adapter state file does not exist after running the command. Use the fixture’s `${HOME}/.cua-task-fixture-state.json` path and keep the existing status and failure-family assertions unchanged.Source: Path instructions
test/e2e/live/cua-gpu-qualification-onboard.ts-98-122 (1)
98-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the provider lookups against inherited
Object.prototypekeys.
PROVIDER_SELECTORacceptsconstructor,toString,hasOwnProperty, and every otherObject.prototypemember name. For those values,PROVIDER_ALIASES[normalized]at line 104 resolves through the prototype chain and returns aFunction, so the??fallback never applies andnormalizedProviderreturns a non-string despite its declared type. The subsequent lookup at line 112 then coerces that function to a string key, so the caller receives the misleading message "has no qualification credential mapping" instead of an invalid-provider error.The same prototype exposure applies to
PROVIDER_SECRET_ENV_KEYS[providerKey]at lines 112 and 148. Note thatassertCuaQualificationLocalRegistryAbsentalready usesObject.prototype.hasOwnProperty.callat line 257 for exactly this reason.🛡️ Proposed fix using prototype-free records
-const PROVIDER_ALIASES: Readonly<Record<string, string>> = { +const PROVIDER_ALIASES: ReadonlyMap<string, string> = new Map([ ... -}; +]);A smaller change keeps the literals and adds own-property checks:
const normalized = provider.toLowerCase(); - return PROVIDER_ALIASES[normalized] ?? normalized; + return Object.prototype.hasOwnProperty.call(PROVIDER_ALIASES, normalized) + ? PROVIDER_ALIASES[normalized]! + : normalized; }const providerKey = normalizedProvider(provider); - const allowedKeys = PROVIDER_SECRET_ENV_KEYS[providerKey]; - if (!allowedKeys) { + const allowedKeys = Object.prototype.hasOwnProperty.call(PROVIDER_SECRET_ENV_KEYS, providerKey) + ? PROVIDER_SECRET_ENV_KEYS[providerKey] + : undefined; + if (!Array.isArray(allowedKeys)) { throw new Error(`NEMOCLAW_PROVIDER '${provider}' has no qualification credential mapping`); }Apply the same own-property guard at line 148.
🤖 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 `@test/e2e/live/cua-gpu-qualification-onboard.ts` around lines 98 - 122, Guard both provider record lookups against inherited Object.prototype keys: update normalizedProvider to use PROVIDER_ALIASES only when normalized is an own property, and update collectCuaQualificationOnboardSecretEnv’s PROVIDER_SECRET_ENV_KEYS lookup similarly. Reuse the existing Object.prototype.hasOwnProperty.call pattern so prototype member names fall through to the invalid-provider handling instead of being treated as mappings.test/e2e/support/cua-qualification-receipt.test.ts-1494-1512 (1)
1494-1512: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore the
fs.readSyncspy in this test.The spy stays installed after the test ends, so every later test in this file runs against a patched
fs.readSync. Restore it in afinallyblock, as done forchmodSpyat Line 1745.♻️ Proposed fix
const realReadSync = fs.readSync.bind(fs); let tampered = false; - vi.spyOn(fs, "readSync").mockImplementation((( + const readSpy = vi.spyOn(fs, "readSync").mockImplementation((( fd: number, buffer: Buffer, offset: number, length: number, position: number | null, ) => { const read = realReadSync(fd, buffer, offset, length, position); if (!tampered) { tampered = true; fs.appendFileSync(tamperedPath, " "); } return read; }) as typeof fs.readSync); - expect(() => readBoundedCuaQualificationJson(tamperedPath)).toThrow( - /changed during bounded validation/, - ); + try { + expect(() => readBoundedCuaQualificationJson(tamperedPath)).toThrow( + /changed during bounded validation/, + ); + } finally { + readSpy.mockRestore(); + }As per coding guidelines: "In deterministic tests, clear mock calls, restore spies, undo environment and global stubs, and explicitly reset mock implementations when needed."
🤖 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 `@test/e2e/support/cua-qualification-receipt.test.ts` around lines 1494 - 1512, Restore the fs.readSync spy created in this test after the assertion, using a try/finally structure so cleanup runs whether readBoundedCuaQualificationJson succeeds or throws. Keep the existing tampering behavior and expectation unchanged, and follow the cleanup pattern used by chmodSpy.Source: Coding guidelines
src/lib/cua/runtime-readiness.test.ts-265-277 (1)
265-277: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the rejection reason for credential-shaped model selectors.
Line 276 uses
toThrow()with no matcher. The assertion passes on any error, including aTypeErrorraised by a defect insidegetCuaInferenceRouteIdentity. The test then cannot prove that each model value is rejected as a coordinate- or credential-shaped selector.The adjacent provider loop at lines 261-263 already asserts the message. Use the same matcher for the model loop.
💚 Proposed fix
- expect(() => getCuaInferenceRouteIdentity({ provider: "nvidia", model })).toThrow(); + expect(() => getCuaInferenceRouteIdentity({ provider: "nvidia", model })).toThrow( + /coordinate- and credential-free/, + );🤖 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/cua/runtime-readiness.test.ts` around lines 265 - 277, Update the model loop in the runtime-readiness test to assert the specific rejection message, matching the existing provider loop’s matcher, instead of using an unqualified toThrow(). Ensure each coordinate- or credential-shaped model selector is verified to fail for the intended reason through getCuaInferenceRouteIdentity.Source: Path instructions
test/onboard-sandbox-name.test.ts-68-74 (1)
68-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTest the no-inner-sandbox requirement through the onboarding boundary.
This test only verifies naming helper output. It does not prove that NemoCUA creates exactly one sandbox. Add a public onboarding assertion that observes one sandbox creation, or remove the no-inner-sandbox claim from the title.
🤖 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 `@test/onboard-sandbox-name.test.ts` around lines 68 - 74, Update the test around formatSandboxAgentName, getDefaultSandboxNameForAgent, and getRequestedSandboxAgentName to either exercise the public onboarding flow and assert that NemoCUA creates exactly one sandbox, or rename the test to remove the no-inner-sandbox claim; keep the existing naming assertions unchanged.Sources: Coding guidelines, Path instructions
src/lib/cua/contract.test.ts-315-322 (1)
315-322: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThis test cannot fail for the reason it claims.
runtimeReadiness()already setstaskOperations: [...CUA_TASK_OPERATIONS]at line 93. Line 318 reassigns the same value, so the test validates a record identical to the one already covered at lines 260-281. Both the fixture and the expectation derive fromCUA_TASK_OPERATIONS, so the test stays green no matter which operations that constant advertises. To exercise the claim, assert the expected operation list literally, and assert that a readiness record advertising an extra or missing operation is rejected.💚 Proposed assertion
it("advertises exactly the browser-slice task operations (`#7755`)", () => { const validate = createValidator(); const readiness = runtimeReadiness(); - readiness.taskOperations = [...CUA_TASK_OPERATIONS]; expect(validate(readiness), JSON.stringify(validate.errors)).toBe(true); expect(getCuaLifecycleSemanticErrors(readiness)).toEqual([]); + expect(readiness.taskOperations).toEqual([ + // the exact browser-slice operations this schema major promises + ]); + + const extra = runtimeReadiness(); + extra.taskOperations = [...CUA_TASK_OPERATIONS, "task.shell" as never]; + expect(validate(extra)).toBe(false);Based on path instructions for
**/*.test.{ts,js,mts,mjs,cts,cjs}: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."🤖 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/cua/contract.test.ts` around lines 315 - 322, Update the test named “advertises exactly the browser-slice task operations (`#7755`)” so its expected operations are an explicit literal list rather than derived from CUA_TASK_OPERATIONS. Add assertions covering readiness records with an extra operation and with a missing operation, verifying both are rejected while the exact list remains valid.Source: Path instructions
src/lib/cua/target-lifecycle.test.ts-377-397 (1)
377-397: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClean up the manifest temp directories even when an assertion fails.
Both tests call
fs.rmSyncas the last statement of the test body. Ifexpect(...).toThrow(...)fails, the directory underos.tmpdir()survives the run. Usetry/finally, or collect the directories and remove them in anafterEachhook assrc/lib/adapters/cua-security.test.tsdoes.🧹 Proposed cleanup
it("rejects a symlinked target manifest before parsing it", () => { const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cua-target-manifest-")); - const target = path.join(directory, "target.json"); - const link = path.join(directory, "manifest.json"); - fs.writeFileSync(target, JSON.stringify(manifest)); - fs.symlinkSync(target, link); - - expect(() => readCuaTargetManifest(link)).toThrow(); - - fs.rmSync(directory, { recursive: true, force: true }); + try { + const target = path.join(directory, "target.json"); + const link = path.join(directory, "manifest.json"); + fs.writeFileSync(target, JSON.stringify(manifest)); + fs.symlinkSync(target, link); + + expect(() => readCuaTargetManifest(link)).toThrow(); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } }); it("rejects an oversized target manifest before parsing it", () => { const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cua-target-manifest-")); - const oversized = path.join(directory, "manifest.json"); - fs.writeFileSync(oversized, "x".repeat(64 * 1024 + 1)); - - expect(() => readCuaTargetManifest(oversized)).toThrow(/regular file/); - - fs.rmSync(directory, { recursive: true, force: true }); + try { + const oversized = path.join(directory, "manifest.json"); + fs.writeFileSync(oversized, "x".repeat(64 * 1024 + 1)); + + expect(() => readCuaTargetManifest(oversized)).toThrow(/regular file/); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } });🤖 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/cua/target-lifecycle.test.ts` around lines 377 - 397, Ensure the temporary directories created in the symlink and oversized-manifest tests are removed even when their readCuaTargetManifest assertions fail. Wrap each test body’s setup and expectation in try/finally, or reuse the existing afterEach cleanup pattern from the related security tests, while preserving the current assertions.scripts/brev-launchable-cua-gpu.sh-1120-1123 (1)
1120-1123: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winThe extra-content check depends on a trailing newline.
readreturns non-zero when the third line has no terminating newline, even though it assigns the partial line topublished_sentinel_extra. A sentinel with unterminated extra content then passes this gate. Test the captured value instead.🛡️ Proposed fix
-if IFS= read -r published_sentinel_extra < <("$SED_BINARY" -n '3p' "$CUA_SENTINEL"); then - : "$published_sentinel_extra" - fail "the published CUA readiness authority has extra content" -fi +IFS= read -r published_sentinel_extra < <("$SED_BINARY" -n '3p' "$CUA_SENTINEL") || true +[[ -z "$published_sentinel_extra" ]] \ + || fail "the published CUA readiness authority has extra content"🤖 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 `@scripts/brev-launchable-cua-gpu.sh` around lines 1120 - 1123, Update the extra-content check following the third-line read in the CUA sentinel validation to test whether published_sentinel_extra contains content, rather than depending on read’s success status. Ensure unterminated third-line content triggers fail with the existing message, while an empty third line remains valid.src/lib/cua/lifecycle-readiness.ts-105-111 (1)
105-111: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
config_revisionis checked for uniqueness but never bound into the identity.The returned
CuaAppliedPolicyIdentitycontains onlyrevisionanddigest. The raw-text scan on Line 106 only proves"config_revision"appears once. Ifconfig_revisioncan change whileversionandhashstay equal,assertCuaLiveAppliedPolicyUnchangedwill not detect that drift. Confirm the intent. Ifconfig_revisionis part of effective policy identity, include it in the returned identity.#!/bin/bash # Description: Inspect how config_revision is produced and consumed across CUA policy identity code. set -euo pipefail rg -n 'config_revision' -C 5 rg -n 'CuaAppliedPolicyIdentity' -C 3 src | head -60🤖 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/cua/lifecycle-readiness.ts` around lines 105 - 111, The config_revision value is validated for uniqueness in the lifecycle-readiness flow but is omitted from the returned CuaAppliedPolicyIdentity. Update the identity construction around the config_revision scan and the return value to parse and include config_revision when present, and update the identity type and assertCuaLiveAppliedPolicyUnchanged comparisons accordingly so changes to it detect policy drift.src/lib/adapters/openshell/runtime.ts-86-86 (1)
86-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
path.isAbsolutefor the absolute-path check.
startsWith("/")rejects a valid Windows absolute path such asC:\tools\openshell.exe.src/lib/cua/openshell-authority.tsusespath.isAbsolutefor the same check, so the two paths disagree.captureResolvedOpenshellis a general adapter function, not a CUA-only helper.🐛 Proposed fix
- if (!openshell.startsWith("/")) throw new Error("OpenShell executable must be absolute"); + if (!path.isAbsolute(openshell)) throw new Error("OpenShell executable must be absolute");Add the import if it is absent:
+import path from "node: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/adapters/openshell/runtime.ts` at line 86, Update captureResolvedOpenshell to validate the openshell executable with path.isAbsolute instead of startsWith("/"), adding the path import if needed. Preserve the existing error behavior while accepting valid absolute paths on all supported platforms.
🤖 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 `@scripts/brev-launchable-cua-gpu.sh`:
- Around line 143-163: Reset or clear VALIDATED_ROOT_AUTHORITY_DIRECTORIES
immediately before the pre-publication authority validation call so
assert_root_authority_ancestors rechecks every path ancestor. Preserve the cache
for normal repeated validations, but ensure the pass around the call near the
authority publication flow cannot short-circuit on previously validated
directories.
In `@scripts/cua-qualification-artifact-runner.sh`:
- Around line 190-203: Derive the original caller UID and GID from sudo-provided
SUDO_UID and SUDO_GID rather than trusting the --root-caller argv values in the
root escalation flow. Validate that the environment values are present and
numeric, reject any mismatch with the supplied identity, and ensure
caller_uid/caller_gid used by assert_artifact_source_file and the task-input
gate reflect the verified sudo identity.
In `@src/lib/actions/sandbox/snapshot.ts`:
- Around line 136-159: Update invalidateCuaAuthorityBeforeSnapshotRestore in
src/lib/actions/sandbox/snapshot.ts (lines 136-159) to clear
cuaRuntimeReadiness, cuaTarget, cuaSecurityAttestation, and cuaTaskResults
together in registry.updateSandbox, matching the clone path; leave
cuaReconciliation governed by the existing reconciliation check. Add a test in
src/lib/actions/sandbox/snapshot-restore-lifecycle.test.ts (lines 144-161) with
cuaTarget and cuaSecurityAttestation set and
requireCuaReconciliationBeforeSandboxMutationMock returning false, asserting
every CUA authority field is cleared before restoreSandboxStateMock runs.
In `@src/lib/adapters/cua-task.ts`:
- Line 9: Update the CUA_TASK_OPERATIONS import in cua-task.ts by removing the
type-only modifier, so its value meaning is available for the typeof
CUA_TASK_OPERATIONS expression while preserving the existing type usage.
In `@src/lib/cua/lifecycle-registry-persistence.test.ts`:
- Around line 4-15: Make the test imports hermetic by using the repository’s
existing temporary-state isolation helper instead of mutating process.env.HOME
directly; update the setup around originalHome, testHome, and the dynamic
persistence/registryLock imports so registry paths are redirected before any CUA
or registry modules load, while preserving cleanup and existing assertions.
In `@src/lib/cua/qualification-evidence.ts`:
- Around line 14-17: The CUA primitives are duplicated and have diverging
behavior; create one shared module containing SENSITIVE_VALUE, the stricter
HOST_COORDINATE from qualification-evidence.ts, canonicalize, and
canonicalJsonSha256. Replace the copies in qualification-evidence.ts,
contract.ts, runtime-manifest.ts, runtime-readiness.ts, and
runtime-test-fixture.ts with imports, preserving the stricter IPv6/ip6-localhost
rejection and using code-unit key comparison instead of localeCompare so digest
generation remains identical across all callers.
In `@src/lib/cua/runtime-manifest.ts`:
- Around line 161-187: Update assertCuaAuthorityFileOwnership to throw an
explicit error when process.platform is not Linux instead of returning early.
Preserve the existing file and parent ownership, type, and write-permission
checks on Linux, ensuring unsupported hosts fail closed before accepting CUA
authority artifacts.
In `@src/lib/cua/runtime-readiness.ts`:
- Around line 415-426: Replace the order-sensitive JSON.stringify comparisons
for inference and components in the runtime identity validation with digestJson
comparisons, preserving the existing semantic equality checks. Also update the
receipt validation around the inference comparison to use digestJson for both
values, ensuring key-order-independent matching while leaving the surrounding
identity checks unchanged.
In `@src/lib/cua/security-lifecycle.ts`:
- Line 350: Update invokeAdapter so it passes immutable copies of runtime and
target to input.adapter.execute, cloning both immediately before the adapter
call. Preserve the existing adapter invocation behavior and keep the attestation
cloning unchanged.
In `@src/lib/state/registry.ts`:
- Around line 331-396: Update updateSandbox so its result distinguishes a
missing sandbox, an active-reconciliation rejection, and a readiness replacement
that quarantines authority and persists via save(data). Replace the ambiguous
boolean return with a discriminated result (or expose an equivalent quarantine
outcome accessor), and ensure callers can identify the persisted-quarantine case
and provide the required next step.
In `@test/e2e/live/cua-gpu-qualification.test.ts`:
- Around line 1643-1668: Update the cleanup logic in the finally block around
nemoclaw so it never throws directly from finally. Capture any nonzero cleanup
result, preserve the original qualification error when the try body failed, and
only raise the cleanup failure when the body completed successfully or attach it
as an AggregateError containing both errors.
In `@tools/e2e/cua-qualification-receipt.mts`:
- Around line 1082-1097: Update canonicalQualificationValue to sort object keys
with a locale-independent code-unit comparison instead of localeCompare,
ensuring identical key sets produce the same ordering and
qualificationObservationDigest across runtimes.
---
Outside diff comments:
In `@src/lib/actions/inference-set.test-support.ts`:
- Around line 176-193: In src/lib/actions/inference-set.test-support.ts:176-193,
update createDeps to create distinct vi.fn spies for generic updateSandbox and
route-specific updateSandboxInferenceRoute instead of assigning both to
updateSandboxInferenceRoute; preserve each option’s configured fallback
behavior. In src/lib/actions/inference-set-openclaw-run.test.ts:54-61, add an
assertion that the generic updateSandbox spy is not called, while retaining the
route-specific assertion.
---
Minor comments:
In `@scripts/brev-launchable-cua-gpu.sh`:
- Around line 1120-1123: Update the extra-content check following the third-line
read in the CUA sentinel validation to test whether published_sentinel_extra
contains content, rather than depending on read’s success status. Ensure
unterminated third-line content triggers fail with the existing message, while
an empty third line remains valid.
In `@src/lib/actions/sandbox/destroy.ts`:
- Around line 466-467: The recovery guidance in the destroy error message should
use the correct sandbox:cua command hierarchy. Update the command text near
CLI_NAME to include sandbox and place sandboxName after the appropriate CUA
command, for both the target health and target destroy instructions.
In `@src/lib/actions/sandbox/doctor.ts`:
- Around line 614-616: Remove the non-null assertion from the
cuaReconciliationDoctorCheck call in the sb?.cuaReconciliation branch, and only
return an array containing the check when the function returns a non-null
result; otherwise return an empty check list. Preserve the existing behavior
when no reconciliation configuration is present.
In `@src/lib/actions/sandbox/snapshot-restore-lifecycle.test.ts`:
- Around line 144-161: Add an explicit assertion in the “invalidates
readiness-only CUA authority before a snapshot restore” test that
restoreSandboxStateMock was called, then retain the invocation-order assertion
without relying on the Number.POSITIVE_INFINITY fallback. Ensure the test
verifies both that restore proceeded and that updateSandboxMock ran first.
In `@src/lib/adapters/openshell/runtime.ts`:
- Line 86: Update captureResolvedOpenshell to validate the openshell executable
with path.isAbsolute instead of startsWith("/"), adding the path import if
needed. Preserve the existing error behavior while accepting valid absolute
paths on all supported platforms.
In `@src/lib/cua/contract.test.ts`:
- Around line 315-322: Update the test named “advertises exactly the
browser-slice task operations (`#7755`)” so its expected operations are an
explicit literal list rather than derived from CUA_TASK_OPERATIONS. Add
assertions covering readiness records with an extra operation and with a missing
operation, verifying both are rejected while the exact list remains valid.
In `@src/lib/cua/lifecycle-readiness.ts`:
- Around line 105-111: The config_revision value is validated for uniqueness in
the lifecycle-readiness flow but is omitted from the returned
CuaAppliedPolicyIdentity. Update the identity construction around the
config_revision scan and the return value to parse and include config_revision
when present, and update the identity type and
assertCuaLiveAppliedPolicyUnchanged comparisons accordingly so changes to it
detect policy drift.
In `@src/lib/cua/runtime-readiness.test.ts`:
- Around line 265-277: Update the model loop in the runtime-readiness test to
assert the specific rejection message, matching the existing provider loop’s
matcher, instead of using an unqualified toThrow(). Ensure each coordinate- or
credential-shaped model selector is verified to fail for the intended reason
through getCuaInferenceRouteIdentity.
In `@src/lib/cua/target-lifecycle.test.ts`:
- Around line 377-397: Ensure the temporary directories created in the symlink
and oversized-manifest tests are removed even when their readCuaTargetManifest
assertions fail. Wrap each test body’s setup and expectation in try/finally, or
reuse the existing afterEach cleanup pattern from the related security tests,
while preserving the current assertions.
In `@test/cua-task-cli.test.ts`:
- Around line 406-456: Update the three input-validation tests around the
invalid UTF-8, symbolic-link, and oversized-input cases to assert that the
fixture adapter state file does not exist after running the command. Use the
fixture’s `${HOME}/.cua-task-fixture-state.json` path and keep the existing
status and failure-family assertions unchanged.
In `@test/e2e/live/cua-gpu-qualification-onboard.ts`:
- Around line 98-122: Guard both provider record lookups against inherited
Object.prototype keys: update normalizedProvider to use PROVIDER_ALIASES only
when normalized is an own property, and update
collectCuaQualificationOnboardSecretEnv’s PROVIDER_SECRET_ENV_KEYS lookup
similarly. Reuse the existing Object.prototype.hasOwnProperty.call pattern so
prototype member names fall through to the invalid-provider handling instead of
being treated as mappings.
In `@test/e2e/support/cua-qualification-receipt.test.ts`:
- Around line 1494-1512: Restore the fs.readSync spy created in this test after
the assertion, using a try/finally structure so cleanup runs whether
readBoundedCuaQualificationJson succeeds or throws. Keep the existing tampering
behavior and expectation unchanged, and follow the cleanup pattern used by
chmodSpy.
In `@test/onboard-sandbox-name.test.ts`:
- Around line 68-74: Update the test around formatSandboxAgentName,
getDefaultSandboxNameForAgent, and getRequestedSandboxAgentName to either
exercise the public onboarding flow and assert that NemoCUA creates exactly one
sandbox, or rename the test to remove the no-inner-sandbox claim; keep the
existing naming assertions unchanged.
---
Nitpick comments:
In `@ci/test-file-size-budget.json`:
- Around line 8-10: Split the new test suites represented by
test/brev-launchable-cua-gpu.test.ts,
test/e2e/live/cua-gpu-qualification.test.ts, and
test/e2e/support/cua-qualification-receipt.test.ts into focused files by
concern, keeping each within the default size budget, and remove their entries
from legacyMaxLines. If retaining the current sizes, document the rationale and
a retirement path in the linked issue instead.
In `@schemas/cua-target-manifest.schema.json`:
- Around line 38-45: Update the capabilities array schema to set uniqueItems to
true, matching the duplicate-rejection behavior of parseCuaTargetManifest and
the lifecycle schema while preserving its existing three-item constraints.
In `@src/lib/adapters/cua-security.ts`:
- Around line 155-165: Replace the message-suffix check in the
CuaSecurityAdapter catch block with a stable typed-error or error-code check
from snapshotBoundedExecutable. Export and use the discriminated digest-mismatch
error contract in bounded-file.ts, preserving the existing readiness message for
digest mismatches and unavailable message for other failures.
In `@src/lib/adapters/cua-task.test.ts`:
- Around line 221-239: Update the incomplete lifecycle record test around
ProcessCuaTaskAdapter.execute so it isolates the missing-proof condition from
unrelated validation failures: include the required evidence mediaType, then
split or parameterize cases for empty verification.checkIds,
verification.evidenceDigests, and receipts, asserting the completeness-specific
validation message or otherwise proving each case reaches that rule.
In `@src/lib/adapters/cua-task.ts`:
- Around line 184-219: Extract the duplicated bounded adapter process invocation
into a shared helper near snapshotBoundedExecutable, centralizing the runner
arguments, spawn options, cleanup, and ETIMEDOUT handling. Update
src/lib/adapters/cua-task.ts:184-219 to call it with the task label, digest
source, and failure mapping; update src/lib/adapters/cua-target.ts:176-211
likewise while preserving target_unreachable/lifecycle_unavailable mapping
locally; update src/lib/adapters/cua-security.ts:167-201 likewise while
preserving its retryable-timeout classification locally.
In `@src/lib/adapters/openshell/runtime.test.ts`:
- Around line 13-19: Update the executable helper to quote the interpolated
output value in the generated shell script, ensuring fixture values containing
spaces or shell metacharacters are passed to printf as a single literal
argument.
- Around line 27-41: Extend the captureResolvedOpenshell test suite with
public-function assertions for both guards: verify a missing binary reports
“OpenShell is unavailable” and a relative binary path reports “OpenShell
executable must be absolute.” Keep the tests focused on the returned failure
status and error output, using inputs that trigger each validation path
directly.
In `@src/lib/adapters/openshell/runtime.ts`:
- Around line 82-99: Extract the duplicated capture-option object from
captureOpenshell and captureResolvedOpenshell into a shared helper that builds
the options while accepting the executable from its caller. Update both
functions to use this helper, preserving their existing binary-resolution
behavior and all option forwarding.
In `@src/lib/cua/bounded-file.test.ts`:
- Around line 20-25: Remove vi.restoreAllMocks() from the afterEach teardown in
the bounded-file test, leaving only temporaryDirectories cleanup. If vi has no
other uses in the file, remove its import as well.
- Around line 27-38: Add two tests alongside the existing “bounded regular file
reads” case to cover readBoundedRegularFile’s byte limits: verify a file larger
than maxBytes is rejected and a file smaller than minBytes is rejected. Use the
existing temporaryFile fixture and assert the established error behavior for
each boundary violation.
In `@src/lib/cua/build-identity.ts`:
- Around line 134-135: In the LFS pointer parsing flow around GIT_LFS_POINTER,
only convert authoritative to an ASCII string when its byte length is within a
small bound sufficient for a Git LFS pointer; otherwise treat lfsPointer as
absent. Preserve the existing lfsSize null behavior for non-pointers and avoid
decoding large blobs.
In `@src/lib/cua/command-adapter-binding.test.ts`:
- Line 254: Replace every hardcoded schemaVersion value in the fixture records
of command-adapter-binding.test.ts with the imported
CUA_LIFECYCLE_SCHEMA_VERSION constant, including the records near the referenced
locations. Ensure all fixtures use this single source of truth consistently.
In `@src/lib/cua/contract.test.ts`:
- Around line 283-302: Reduce the assertions in the test case around loadAgent
and getAgentChoices so it no longer compares the full runtime object or
smoke_commands for langchain-deepagents-code. Retain assertions for
agent.runtime.kind, interactive_command, headless_command, versionCommand, and
both getTerminalCommand results to verify ordinary terminal manifest discovery,
leaving smoke-command text to the agent-specific tests.
In `@src/lib/cua/contract.ts`:
- Around line 212-228: Extract the duplicated canonicalize logic into one shared
module, exporting canonicalize and canonicalJsonSha256. Update contract.ts,
runtime-readiness.ts, and runtime-test-fixture.ts to import and reuse these
helpers for all digest calculations, replacing localeCompare with deterministic
code-unit key ordering; preserve the existing digest format and readiness
validation behavior.
- Around line 394-414: Remove the unused module-private requiredSetErrors
helper; do not add a caller or retain duplicate validation logic, since no call
site uses it and exactSetErrors is the intended neighboring helper.
In `@src/lib/cua/lifecycle-readiness.test.ts`:
- Around line 229-231: Update the no-spawn test assertion around
observeCuaLiveInference to require the exact authority-validation error message
instead of accepting any thrown error. Preserve the marker absence assertion and
ensure the check still exercises the intended validation path.
In `@src/lib/cua/lifecycle-registry-persistence.test.ts`:
- Around line 62-70: Add a test around the persistence transaction using the
existing conflict callback to mutate the on-disk lifecycleGeneration between
checkpoint() and the final save, simulating a stale row-level CAS update. Assert
the operation returns "rejected" and verify no partial state or
cuaReconciliation is persisted in persistence.load().sandboxes.alpha.
In `@src/lib/cua/qualification-artifact-runner.test.ts`:
- Around line 33-40: The test title overstates coverage because it only verifies
exported constant values. Rename the test to describe the pinned path contract,
or update it to exercise resolveCuaQualificationArtifactRunner with an alternate
configured path and assert that the path is rejected.
- Around line 17-31: Extend the test around
resolveCuaQualificationArtifactRunner to include the exact
CUA_QUALIFICATION_ARTIFACT_RUNNER_PATH value and assert the distinct “authority
is unsafe” error. Keep the existing missing and mismatched runner assertions,
ensuring the new case verifies execution passes the environment-value gate and
reaches filesystem authority validation.
In `@src/lib/cua/qualification-evidence.test.ts`:
- Around line 135-143: Update the undeclared-field parameterized test around the
“rejects an undeclared” case to include the specific parser for each table row,
invoke only that parser, and assert the thrown error message identifies the
undeclared field. Preserve the existing mutation setup while ensuring
receipt-only cases cannot pass because the environment parser throws first.
In `@src/lib/cua/runtime-manifest.ts`:
- Line 23: The js-yaml dependency is loaded with CommonJS require and a
handwritten type instead of the project’s ESM import pattern. After confirming
the CLI module target and declared js-yaml version, update the yaml declaration
in runtime-manifest.ts to use an ESM import from js-yaml while preserving
validateExternalCuaAgentManifest and its unknown return handling.
In `@src/lib/cua/runtime-readiness.ts`:
- Around line 104-124: Remove the dead conditional argument from
canonicalEndpoint’s final replace call, preserving the existing normalized URL
output and routeDigest behavior while making the unconditional trailing-slash
removal explicit.
In `@src/lib/cua/schema.test.ts`:
- Around line 14-16: Update the targetManifest() fixture to use an explicit
valid target-manifest schema version matching the pinned 1.x pattern, rather
than CUA_LIFECYCLE_SCHEMA_VERSION. Leave CUA_LIFECYCLE_SCHEMA_VERSION unchanged
in the securityAttestation fixture.
- Around line 190-201: Extend the test around parseCuaSecurityAttestation in
“rejects missing denials and authority-bearing fields” with a case that
preserves the required array length and schema-valid values while violating an
exact-set semantic contract, such as artifacts.materials or another listed
field. Assert the semantic contract error rather than the generic schema error,
and avoid duplicate or enum-invalid substitutions that would be rejected before
getCuaLifecycleSemanticErrors runs.
In `@src/lib/cua/security-command.ts`:
- Around line 102-116: The bare catch blocks in the security command flow should
preserve diagnostics while continuing to return the unchanged
runtime_unavailable failure. Update each catch around the visible
lifecycle/adapter handling and outer command execution to accept the error, log
a redacted diagnostic through the existing logger at debug level, and then call
commandFailure as before.
In `@src/lib/cua/security-lifecycle.test.ts`:
- Around line 160-187: Update the test harness so load returns a clone of the
committed registry and save replaces the committed registry with its argument,
rather than using the live object and inert vi.fn(). Adjust setup cases that
mutate registry.sandboxes.alpha directly before lifecycle calls to modify state
through the persistence accessor, while keeping assertions against committed
registry state.
- Around line 479-490: Deep-clone the module-level runtime fixture when creating
changedRuntime at both affected sites, using structuredClone(runtime) before
applying the targeted component changes. Preserve the existing modifications to
targetAdapter and related runtime readiness updates while ensuring nested
inference, qualification, and components entries are not shared with runtime.
In `@src/lib/cua/state.ts`:
- Around line 14-21: Move the shared pure predicate
cuaSecurityAttestationMatches out of security-lifecycle.ts into a domain-style
module, then update both security-lifecycle.ts and state.ts to import it from
that module. Keep lifecycle orchestration and host-boundary calls out of the
domain module, and remove the old predicate ownership/import path to avoid
duplicate sources of truth or import cycles.
In `@src/lib/cua/task-lifecycle.test.ts`:
- Around line 262-273: Update the test harness factory around the registry
`load` and `save` dependencies to clone the registry when loading and apply the
lifecycle’s persisted value during saving, matching the durable harness pattern
in `target-lifecycle.test.ts`. Ensure assertions inspect the original registry
independently of the loaded working copy, and remove the now-redundant manual
restart simulation near the restart test.
In `@src/lib/inference/gateway-route-compatibility.ts`:
- Around line 8-9: Remove the redundant resolver alias chain: delete
resolveLiveInferenceGatewayName from gateway-route-compatibility.ts, remove its
re-export in src/lib/actions/sandbox/connect-inference-gateway.ts at line 14,
and update src/lib/actions/sandbox/status-snapshot.ts at line 42 to import
resolveSandboxGatewayName directly from gateway-runtime-action without renaming
it.
In `@src/lib/onboard/sandbox-agent.test.ts`:
- Line 11: Append the linked issue suffix “(`#7755`)” to the parent describe
titles for the CUA onboarding reconciliation suite in
src/lib/onboard/sandbox-agent.test.ts:11-11, the CUA lifecycle reconciliation
suite in src/lib/cua/reconciliation.test.ts:48-48, and the CUA OpenShell
executable authority suite in src/lib/cua/openshell-authority.test.ts:38-38.
In `@src/lib/state/registry.ts`:
- Around line 398-406: Remove the behaviorless updateSandboxInferenceRoute
wrapper and update its callers to invoke updateSandbox directly, unless the
wrapper is narrowed to route-specific fields and provides distinct validation.
Eliminate the redundant documentation and avoid retaining a forwarding layer
that adds no enforcement.
- Around line 343-362: Update policyAuthorityChanges to derive its fields from
the typed CUA_AUTHORITY_INPUT_FIELDS constant instead of maintaining a separate
string list. Reuse the typed field access pattern used by authorityInputChanges
and remove the unknown Record casts, preserving the existing deep-equality
change detection behavior.
In `@test/e2e/live/cua-gpu-qualification.test.ts`:
- Around line 804-813: Replace the fixed 1,200 ms delay before the cleanup
assertions with a bounded polling loop that repeatedly checks
hostProcessesUsingIdentity, cuaArtifactCgroups, and listCuaArtifactUnits until
all three are empty or a defined deadline expires. Preserve the existing final
empty-state assertions and make the poll wait briefly between attempts without
exceeding the deadline.
In `@test/e2e/support/cua-gpu-qualification-onboard.test.ts`:
- Around line 205-207: Update the assertion around
assertCuaQualificationLocalRegistryAbsent to require the exact symlink-rejection
error message, matching the message-binding style used by the other cases in
this test file rather than accepting any thrown error.
In `@test/e2e/support/cua-qualification-receipt.test.ts`:
- Around line 1182-1190: Remove the unused sourceTaskInputPath and taskInputPath
statements from the test setup, or prefix any intentionally retained binding
with an underscore. Keep the input-directory setup and subsequent
artifact-environment assertions unchanged.
In `@test/e2e/support/fixtures/cua-qualification-artifact-boundary-probe.sh`:
- Around line 204-213: The stdin branch must explicitly handle an unset TMPDIR
before expanding "$TMPDIR/stdin"; add a guard that exits with the documented
boundary-violation probe code, while preserving the existing stdin capture and
hashing flow when TMPDIR is available.
- Around line 105-108: Use distinct exit codes for each probe invariant: in
test/e2e/support/fixtures/cua-qualification-artifact-boundary-probe.sh, update
the namespace-identity validation loop to use an unused code instead of 58; in
tools/e2e/cua-qualification-isolation-probe.sh, assign three separate unused
codes to the PID-namespace check, unexpected-variable check, and completeness
check currently using 35.
In `@test/helpers/cua-cli-runtime.ts`:
- Around line 111-122: Update createCuaRuntimeTestFixture’s returned
CuaCliRuntimeFixture object to forward the existing cleanup callback. Preserve
the current fixture fields and expose cleanup so CLI test suites can invoke it
during afterEach to remove runtime.root.
In `@tools/e2e/cua-qualification-receipt.mts`:
- Around line 1419-1424: Rename the exactOperations helper to a generic name
such as sameStringSet, reflecting that it compares arbitrary string sets rather
than only operation lists. Update both evidence-digest call sites around lines
1781 and 1806 to use the new name, preserving the comparison behavior.
🪄 Autofix
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: ff4f687d-dfa7-4a2f-baf7-bc761707d937
📒 Files selected for processing (144)
ci/source-architecture-budget.jsonci/source-shape-test-budget.jsonci/test-file-size-budget.jsondocs/reference/commands.mdxpackage.jsonschemas/cua-lifecycle.schema.jsonschemas/cua-target-manifest.schema.jsonscripts/brev-launchable-cua-gpu.shscripts/cua-qualification-artifact-runner.shscripts/cua-qualification-target-channel-probe.tssrc/commands/sandbox/cua/security/status.tssrc/commands/sandbox/cua/security/verify.tssrc/commands/sandbox/cua/target/attach.tssrc/commands/sandbox/cua/target/destroy.tssrc/commands/sandbox/cua/target/detach.tssrc/commands/sandbox/cua/target/health.tssrc/commands/sandbox/cua/target/reset.tssrc/commands/sandbox/cua/target/status.tssrc/commands/sandbox/cua/task/cancel.tssrc/commands/sandbox/cua/task/events.tssrc/commands/sandbox/cua/task/guide.tssrc/commands/sandbox/cua/task/logs.tssrc/commands/sandbox/cua/task/pause.tssrc/commands/sandbox/cua/task/plans.tssrc/commands/sandbox/cua/task/respond.tssrc/commands/sandbox/cua/task/result.tssrc/commands/sandbox/cua/task/start.tssrc/commands/sandbox/cua/task/status.tssrc/lib/actions/inference-get.tssrc/lib/actions/inference-set-openclaw-run.test.tssrc/lib/actions/inference-set.test-support.tssrc/lib/actions/inference-set.tssrc/lib/actions/sandbox/connect-inference-gateway.tssrc/lib/actions/sandbox/cua-target-status.test.tssrc/lib/actions/sandbox/destroy-flow.test.tssrc/lib/actions/sandbox/destroy.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/snapshot-restore-lifecycle.test.tssrc/lib/actions/sandbox/snapshot-restore-test-fixture.tssrc/lib/actions/sandbox/snapshot.test.tssrc/lib/actions/sandbox/snapshot.tssrc/lib/actions/sandbox/status-snapshot.tssrc/lib/actions/update.test.tssrc/lib/actions/update.tssrc/lib/adapters/cua-security.test.tssrc/lib/adapters/cua-security.tssrc/lib/adapters/cua-target.test.tssrc/lib/adapters/cua-target.tssrc/lib/adapters/cua-task.test.tssrc/lib/adapters/cua-task.tssrc/lib/adapters/openshell/resolve-shared.tssrc/lib/adapters/openshell/runtime.test.tssrc/lib/adapters/openshell/runtime.tssrc/lib/agent/aliases.tssrc/lib/agent/base-image.test.tssrc/lib/agent/base-image.tssrc/lib/agent/defs.test.tssrc/lib/agent/defs.tssrc/lib/agent/onboard-cua.test.tssrc/lib/agent/onboard.tssrc/lib/cli/branding.test.tssrc/lib/cli/branding.tssrc/lib/cli/public-display-defaults.tssrc/lib/core/generate-build-identity.tssrc/lib/cua/bounded-file.test.tssrc/lib/cua/bounded-file.tssrc/lib/cua/build-identity.test.tssrc/lib/cua/build-identity.tssrc/lib/cua/command-adapter-binding.test.tssrc/lib/cua/command-route-lock.test.tssrc/lib/cua/command-route-lock.tssrc/lib/cua/contract.mdsrc/lib/cua/contract.test.tssrc/lib/cua/contract.tssrc/lib/cua/feature.test.tssrc/lib/cua/feature.tssrc/lib/cua/lifecycle-readiness.test.tssrc/lib/cua/lifecycle-readiness.tssrc/lib/cua/lifecycle-registry-persistence.test.tssrc/lib/cua/lifecycle-registry-transaction.test.tssrc/lib/cua/lifecycle-registry-transaction.tssrc/lib/cua/onboard-runtime.tssrc/lib/cua/openshell-authority.test.tssrc/lib/cua/openshell-authority.tssrc/lib/cua/qualification-artifact-runner.test.tssrc/lib/cua/qualification-artifact-runner.tssrc/lib/cua/qualification-evidence.test.tssrc/lib/cua/qualification-evidence.tssrc/lib/cua/reconciliation.test.tssrc/lib/cua/reconciliation.tssrc/lib/cua/runtime-manifest.test.tssrc/lib/cua/runtime-manifest.tssrc/lib/cua/runtime-readiness.test.tssrc/lib/cua/runtime-readiness.tssrc/lib/cua/runtime-test-fixture.tssrc/lib/cua/schema.test.tssrc/lib/cua/schema.tssrc/lib/cua/security-command.tssrc/lib/cua/security-lifecycle.test.tssrc/lib/cua/security-lifecycle.tssrc/lib/cua/state.tssrc/lib/cua/target-command.tssrc/lib/cua/target-lifecycle.test.tssrc/lib/cua/target-lifecycle.tssrc/lib/cua/task-cli-definitions.tssrc/lib/cua/task-command.tssrc/lib/cua/task-lifecycle.test.tssrc/lib/cua/task-lifecycle.tssrc/lib/gateway-runtime-action.tssrc/lib/inference/gateway-route-compatibility.tssrc/lib/inference/live.tssrc/lib/onboard.tssrc/lib/onboard/sandbox-agent.test.tssrc/lib/onboard/sandbox-agent.tssrc/lib/onboard/tool-disclosure-flow.test.tssrc/lib/onboard/tool-disclosure-flow.tssrc/lib/state/registry-cua.test.tssrc/lib/state/registry.tssrc/lib/state/registry/persistence.tssrc/lib/state/registry/types.tstest/brev-launchable-cua-gpu.test.tstest/cua-qualification-target-channel-probe.test.tstest/cua-security-cli.test.tstest/cua-target-cli.test.tstest/cua-task-cli.test.tstest/e2e/README.mdtest/e2e/fixtures/artifacts.tstest/e2e/live/cua-gpu-qualification-onboard.tstest/e2e/live/cua-gpu-qualification.test.tstest/e2e/support/cua-gpu-qualification-onboard.test.tstest/e2e/support/cua-qualification-artifact-runner.test.tstest/e2e/support/cua-qualification-receipt.test.tstest/e2e/support/e2e-artifact-permissions.test.tstest/e2e/support/fixtures/cua-qualification-artifact-boundary-probe.shtest/helpers/base-image-test-harness.tstest/helpers/cua-cli-runtime.tstest/helpers/cua-launchable-git-verifier.tstest/helpers/destroy-flow-test-harness.tstest/helpers/vitest-watch-triggers.tstest/onboard-sandbox-name.test.tstest/package-contract/cli/command-registry.test.tstest/vitest-watch-triggers.test.tstools/e2e/cua-qualification-isolation-probe.shtools/e2e/cua-qualification-receipt.mts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/e2e/support/cua-qualification-canonicalization.test.ts (1)
8-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert canonicalization with a fixed digest vector.
expect(localeCompare).not.toHaveBeenCalled()couples the test to the sorting implementation. Assert the expected digest instead, so the test checks the public contract and detects locale-dependent ordering. Thee2e-supportproject automatically restores mocks.🤖 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 `@test/e2e/support/cua-qualification-canonicalization.test.ts` around lines 8 - 17, Update the test around getCuaQualificationSandboxObservationDigest to assert the fixed expected sha256 digest for the given observation inputs instead of spying on String.prototype.localeCompare or checking that it was unused. Keep the existing digest-format assertion only if useful, and rely on the e2e-support mock restoration.Sources: Coding guidelines, 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/cua/shared-primitives.ts`:
- Around line 9-10: Update CUA_HOST_COORDINATE and its related
CUA_PROVIDER_IDENTITY validation to cover IPv6 addresses such as 2001:db8::1 and
hostnames such as provider.example.xyz. Use a grammar-based host check or expand
the suffix allowlist to include all accepted provider hostnames, and add tests
for both cases.
- Around line 19-23: Update the canonicalization flow in canonicalizeCuaJson so
numeric property names retain compareCodeUnits ordering rather than being
reordered by Object.fromEntries before JSON.stringify. Serialize the sorted,
recursively canonicalized entries directly, and add a regression test covering
numeric keys such as "10" and "2" through canonicalJsonSha256.
---
Nitpick comments:
In `@test/e2e/support/cua-qualification-canonicalization.test.ts`:
- Around line 8-17: Update the test around
getCuaQualificationSandboxObservationDigest to assert the fixed expected sha256
digest for the given observation inputs instead of spying on
String.prototype.localeCompare or checking that it was unused. Keep the existing
digest-format assertion only if useful, and rely on the e2e-support mock
restoration.
🪄 Autofix
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: 7d97140e-31bd-4783-b27f-f10e56abeace
📒 Files selected for processing (26)
ci/test-file-size-budget.jsondocs/reference/commands.mdxscripts/brev-launchable-cua-gpu.shscripts/cua-qualification-artifact-runner.shsrc/lib/actions/sandbox/snapshot-restore-lifecycle.test.tssrc/lib/actions/sandbox/snapshot.tssrc/lib/cua/contract.test.tssrc/lib/cua/contract.tssrc/lib/cua/lifecycle-registry-persistence.test.tssrc/lib/cua/qualification-evidence.tssrc/lib/cua/runtime-manifest.test.tssrc/lib/cua/runtime-manifest.tssrc/lib/cua/runtime-readiness.test.tssrc/lib/cua/runtime-readiness.tssrc/lib/cua/runtime-test-fixture.tssrc/lib/cua/security-lifecycle.test.tssrc/lib/cua/security-lifecycle.tssrc/lib/cua/shared-primitives.tstest/brev-launchable-cua-gpu.test.tstest/e2e/live/cua-gpu-qualification-inputs.tstest/e2e/live/cua-gpu-qualification.test.tstest/e2e/support/cua-qualification-artifact-runner.test.tstest/e2e/support/cua-qualification-canonicalization.test.tstest/e2e/support/cua-qualification-receipt.test.tstest/helpers/cua-launchable-fixture.tstools/e2e/cua-qualification-receipt.mts
🚧 Files skipped from review as they are similar to previous changes (19)
- ci/test-file-size-budget.json
- src/lib/actions/sandbox/snapshot.ts
- src/lib/actions/sandbox/snapshot-restore-lifecycle.test.ts
- src/lib/cua/security-lifecycle.test.ts
- src/lib/cua/lifecycle-registry-persistence.test.ts
- src/lib/cua/security-lifecycle.ts
- src/lib/cua/runtime-test-fixture.ts
- src/lib/cua/runtime-manifest.ts
- src/lib/cua/contract.ts
- test/e2e/live/cua-gpu-qualification.test.ts
- src/lib/cua/qualification-evidence.ts
- test/e2e/support/cua-qualification-receipt.test.ts
- scripts/brev-launchable-cua-gpu.sh
- src/lib/cua/runtime-readiness.ts
- docs/reference/commands.mdx
- tools/e2e/cua-qualification-receipt.mts
- src/lib/cua/contract.test.ts
- test/e2e/support/cua-qualification-artifact-runner.test.ts
- scripts/cua-qualification-artifact-runner.sh
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/cua/shared-primitives.ts`:
- Around line 19-30: Update canonicalizeCuaJson to track object and array values
on the active recursion path with a WeakSet, throwing TypeError when a value is
encountered again; remove each value in a finally block after serialization so
shared non-circular values remain valid. Add a regression test covering a
self-referential value and the expected TypeError.
In `@test/e2e/support/cua-qualification-receipt.test.ts`:
- Line 1511: Update the tamper assertion for readBoundedCuaQualificationJson to
match a stable tamper-specific error code or type; if only message matching is
available, require both the tampered path and the read operation in the matcher
instead of the broad “changed during” substring.
🪄 Autofix
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: f2e5e09e-0c68-4a45-bdbd-2b6cb7d2800d
📒 Files selected for processing (27)
ci/test-file-size-budget.jsondocs/reference/commands.mdxscripts/brev-launchable-cua-gpu.shsrc/lib/actions/inference-set-openclaw-run.test.tssrc/lib/actions/inference-set.test-support.tssrc/lib/actions/sandbox/destroy.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/snapshot-restore-lifecycle.test.tssrc/lib/actions/sandbox/snapshot.tssrc/lib/adapters/openshell/runtime.tssrc/lib/cua/contract.test.tssrc/lib/cua/contract.tssrc/lib/cua/qualification-evidence.tssrc/lib/cua/runtime-readiness.test.tssrc/lib/cua/runtime-readiness.tssrc/lib/cua/shared-primitives.test.tssrc/lib/cua/shared-primitives.tssrc/lib/cua/target-lifecycle.test.tssrc/lib/onboard/sandbox-agent.tstest/brev-launchable-cua-gpu.test.tstest/cua-task-cli.test.tstest/e2e/live/cua-gpu-qualification-onboard.tstest/e2e/support/cua-gpu-qualification-onboard.test.tstest/e2e/support/cua-qualification-canonicalization.test.tstest/e2e/support/cua-qualification-receipt.test.tstest/onboard-sandbox-name.test.tstools/e2e/cua-qualification-receipt.mts
🚧 Files skipped from review as they are similar to previous changes (22)
- src/lib/actions/sandbox/destroy.ts
- ci/test-file-size-budget.json
- test/onboard-sandbox-name.test.ts
- src/lib/actions/sandbox/snapshot-restore-lifecycle.test.ts
- src/lib/onboard/sandbox-agent.ts
- src/lib/cua/runtime-readiness.test.ts
- test/cua-task-cli.test.ts
- src/lib/adapters/openshell/runtime.ts
- src/lib/actions/inference-set-openclaw-run.test.ts
- src/lib/actions/inference-set.test-support.ts
- test/e2e/support/cua-qualification-canonicalization.test.ts
- src/lib/cua/runtime-readiness.ts
- test/e2e/support/cua-gpu-qualification-onboard.test.ts
- src/lib/cua/contract.test.ts
- scripts/brev-launchable-cua-gpu.sh
- test/e2e/live/cua-gpu-qualification-onboard.ts
- src/lib/cua/contract.ts
- src/lib/actions/sandbox/snapshot.ts
- src/lib/cua/qualification-evidence.ts
- docs/reference/commands.mdx
- test/brev-launchable-cua-gpu.test.ts
- tools/e2e/cua-qualification-receipt.mts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> # Conflicts: # ci/source-architecture-budget.json # src/lib/actions/sandbox/snapshot-restore-test-fixture.ts # src/lib/actions/sandbox/snapshot.test.ts
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-8484.docs.buildwithfern.com/nemoclaw |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 94f71bd in the TypeScript / code-coverage/cliThe overall coverage in commit 94f71bd in the Show a code coverage summary of the most impacted files.
Updated |
cv
requested changes
Aug 7, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
This PR exceeds the accepted #7755 slice. That issue explicitly limits the outcome to installing and inspecting one candidate worker and states that it does not attach a desktop target or run a task. The diff adds target attach/detach/reset/destroy and task start/pause/cancel/respond/result lifecycle commands, schemas, persistence, and qualification machinery. Remove the unaccepted target/task surface or link the accepted follow-on decisions that authorize it. Resolve the 24 current threads, including the two CodeQL filesystem race findings in qualification paths, then refresh onto current main and rerun the complete security and required-check set.
cv
requested changes
Aug 7, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Please review this PR description and diff for comms and documentation guidelines in WRITING.md and linked artifacts.
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
changed the title
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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.
Summary
Adds the issue #7755 candidate install-readiness slice behind exact
NEMOCLAW_CUA_ENABLED=1andNEMOCLAW_CUA_QUALIFICATION=1gates. CUA remains private and disabled by default. Required binaries and artifacts are supplied by the controlled Brev environment, not installed or exposed by ordinary NemoClaw flows.Related Issue
Closes #7755
Changes
candidatereadiness; finalavailablereadiness is impossible and target, security, and task operation arrays remain empty.nemocua interactiveandnemocua headlessexecution.Type of Change
Quality Gates
docs/,fern/, or user-guide routing files change.Documentation Writer Review
origin/mainthrough exact head94f71bd550394e3acc77d9ecfe88366f55be99f8, with focused review of the final test-only growth-guard fix.no-docs-needednode:assert/strictvalidation to satisfy the codebase-growth guard. The compared authority fields and assertions are unchanged. Across the full PR, CUA remains private and disabled by default behind exactNEMOCLAW_CUA_ENABLED=1andNEMOCLAW_CUA_QUALIFICATION=1gates, consumes controlled Brev-provided artifacts, remains candidate/install-and-inspect only, keeps target/security/task operation arrays empty, and exposes no dedicated lifecycle routes.git diff --check origin/main...HEADpassed; no trackeddocs/,fern/, user-guide routing, AGENTS, WRITING, README, or CONTRIBUTING files changed. Focused tests passed 36/36; CLI type-checking and build, repository checks (1,646 files, 4,927 edges, 0 cycles), the test-conditional scan, and normal pre-commit/commit-msg hooks passed. The last commit changes one test file only and leaves noifstatement in it. Public activation documentation would therefore be premature and would broaden the accepted surface.no-docs-neededc69aad4d563f774afb9924b33122b9485a48cdebDGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.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 unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run test:changed -- --maxWorkers=4passed 6,189 tests and reported 21 timeouts/shared-state failures in untouched Hermes rebuild, watcher, base-image, and messaging suites; focused changed behavior remains green. GitHub CI is authoritative and in progress.npm run docsbuilds without warnings (doc changes only) — not applicable; no documentation changesLive candidate acceptance depends on the external digest-pinned NemoCUA runtime manifest, payload, image, and host binaries supplied through the controlled Brev environment. Those inputs are intentionally absent from this repository.
Signed-off-by: Julie Yaunches jyaunches@nvidia.com