Skip to content

Commit 41bb231

Browse files
authored
fix(cli): repair post-merge validation and recovery (NVIDIA#10235)
<!-- markdownlint-disable MD041 --> ## Summary Repairs stale references left by NVIDIA#10219 so current validation selects the migrated tests instead of missing paths. It also prevents Vitest 4.1.9 from treating text inside `tsx` as a malformed inline source map after otherwise completed shards, and completes NVIDIA#10128's timeout recovery contract by preserving sandbox ownership state and documenting retry steps. ## Changes - Point the Ubuntu contract, DGX Station ownership test, validation selectors, lint scope, maintainer instructions, and internal security records at their migrated test paths. The affected selector tests and repository checks protect these mappings. - Install a test-runner-only compatibility hook for `convert-source-map` 2.0.0 because no stable Vitest 4 release contains the upstream fix. It returns no source map only when inline source-map parsing throws a `SyntaxError`; other failures still propagate. `test/repository/vitest-malformed-source-map-compat.test.ts` protects the `tsx` marker collision. - Propagate OpenShell workspace and sandbox deletion timeouts to `destroy`, refuse local-only `--force` cleanup when the remote result is unknown, and preserve registry, MCP, and shields recovery authority. Combined-state tests require gateway status, shields status, then a destroy retry only after recovery permits it when a timeout overlaps an active timer or failed pre-delete re-lock. - Keep the expanded destroy behavior coverage within the existing test-size budget by placing shared recovery assertions in `test/helpers/destroy-flow-test-assertions.ts`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior, justification: - [ ] Tests not applicable, justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded, reviewer/approval link/justification: Codex Desktop reviewed commit `63c508ba8`. PASS. Timeout results preserve local ownership and shields recovery state, the test-only source-map compatibility changes only malformed source-map handling and propagates other failure classes, and path-only security-record changes retain the reviewed boundaries. - [ ] Non-success, skipped, or missing CI check accepted by maintainer, check name, approval link, and follow-up issue: ## 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, command/result or justification: migrated-reference selection 361/361, E2E support 11/11, package contract 4/4, Vitest compatibility and create-require ratchets 38/38, focused destroy source tests 90/90, related destroy integration tests 27/27, and growth guardrails 32/32. CLI and plugin builds, CLI type-checking, repository checks, `npm run validate:pr`, Oxfmt, Oxlint, and `git diff --check` passed at `63c508ba8`. - [ ] Applicable broad gate passed, `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes, command/result: The exact shard 10 command completed blob-report finalization twice without the source-map parser crash. Remaining local failures require CI runner tools or host capabilities. Required GitHub checks remain mandatory. - [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). It passed with 0 errors and 2 existing Fern warnings. - [x] 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) ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Against current base `41a24b994`, the 26-path three-dot diff updates destroy timeout documentation and recovery behavior, migrated validation references, and test-only Vitest compatibility. The conflict-free merge-tree result equals candidate tree `e83a66e2d1b14f55ed12d05f871aadf82d643b50`. Focused source tests passed 90/90, integration tests passed 27/27, growth guardrails passed 32/32, the docs build passed with 0 errors and 2 existing warnings, CLI and plugin builds passed, CLI typecheck passed, repository checks passed, `npm run validate:pr` passed, and `git diff --check` passed. - Agent: Codex Desktop <!-- docs-review-head-sha: 63c508b --> <!-- docs-review-agents-blob-sha: becb5c5 --> --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved sandbox destruction handling for workspace-cleanup and sandbox-deletion timeouts. - Preserved sandbox records when cleanup cannot complete and provided clearer Shields recovery guidance. - Prioritized actionable recovery instructions for unreachable gateways and active cleanup timers. - **Documentation** - Updated `destroy` command documentation to explain timeout behavior, retained volumes, registry preservation, and recovery steps. - **Tests** - Added regression coverage for timeout recovery and malformed source-map handling. - Updated validation references and expectations for relocated test suites. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
1 parent d4522de commit 41bb231

21 files changed

Lines changed: 276 additions & 86 deletions

File tree

.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/review-and-merge.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ from an `origin/main` worktree. Remove the temporary worktree after the decision
5757
interrupted or denied run.
5858

5959
Use one supported `kind`: `ambiguous-write`, `fork-workflow-approval`, `review`, `merge`, or
60-
`post-merge-e2e`. The executable scenarios in `test/maintainer-fix-e2e-policy.test.ts` define each
60+
`post-merge-e2e`. The executable scenarios in `test/automation/pull-requests/maintainer-fix-e2e-policy.test.ts` define each
6161
required state field.
6262

6363
Perform only the entry returned in `allowedWrites`. An empty list denies the requested write.

.agents/skills/nemoclaw-maintainer-runtime-provider/references/implementation-and-review.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ npx vitest run --project cli \
5454
src/lib/onboard/runtime-provider/runtime-provider-contract.test.ts \
5555
src/lib/onboard/runtime-provider/activation.test.ts \
5656
src/lib/onboard/runtime-provider/<provider>.test.ts
57-
npx vitest run --project integration test/layer-import-boundaries.test.ts
57+
npx vitest run --project integration test/repository/layer-import-boundaries.test.ts
5858
npx vitest run --project e2e-support test/e2e/support/native-runtime-qualification.test.ts
5959
```
6060

.dsh/tools/infer_validation_for_changed_files/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ export default async function infer_validation_for_changed_files(input: {
5858
}
5959
if (/agents\/langchain-deepagents-code\/Dockerfile|corporate-ca/.test(file)) {
6060
[
61-
"test/corporate-ca-build-tls-anchor.test.ts",
62-
"test/corporate-ca-dockerfile-decode.test.ts",
61+
"test/security/corporate-ca-runtime-merge.test.ts",
62+
"test/install/corporate-ca-dockerfile-decode.test.ts",
6363
"src/lib/onboard/corporate-ca-host-anchors.test.ts",
6464
"src/lib/onboard/dockerfile-patch-corporate-ca.test.ts",
6565
].forEach((x) => tests.add(x));

.github/workflows/platform-vitest-main.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ jobs:
7575
npx vitest run --project cli \
7676
src/lib/onboard/preflight.test.ts \
7777
src/lib/onboard/preflight-ubuntu-2604.test.ts
78-
npx vitest run --project integration test/platform.test.ts
78+
npx vitest run --project integration test/e2e-runtime/platform.test.ts
7979
npx vitest run --project installer-integration \
8080
--testTimeout 30000 \
8181
test/installer-integration/install-preflight.test.ts

docs/reference/commands.mdx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2578,7 +2578,13 @@ If deletion fails after hardening, the command keeps the surviving sandbox's loc
25782578
By default, unattended final-sandbox destroys (`--yes`, `--force`, or `NEMOCLAW_NON_INTERACTIVE=1`) remove the shared NemoClaw gateway on macOS so the host listener is released, while Linux preserves it for reuse.
25792579
Pass `--cleanup-gateway` to force removal, or `--no-cleanup-gateway` to force preservation.
25802580
These flags always override both `NEMOCLAW_CLEANUP_GATEWAY` and the platform default.
2581-
If the pre-delete workspace wipe cannot run, use a different sandbox name for a clean start.
2581+
If the pre-delete workspace wipe completes with a nonzero status, `destroy` continues, but the retained volume may still contain old files.
2582+
Use a different sandbox name to avoid reusing that retained volume.
2583+
If workspace cleanup reaches its 60-second timeout, NemoClaw cannot confirm the remote result.
2584+
NemoClaw stops before provider cleanup and sandbox deletion, attempts to restore any prepared MCP state, and preserves the local registry entry.
2585+
Run `$$nemoclaw <name> status` to check or start the recorded OpenShell gateway.
2586+
If a shields auto-restore timer remains active, run `$$nemoclaw <name> shields status` to verify bounded recovery or follow its durable-containment guidance, then retry `destroy` only after shields recovery permits it.
2587+
If no timer remains active, retry after the recorded gateway is available.
25822588
Cleaning up the gateway after the last sandbox also purges the shared cluster volume that retains the per-name persistent volume.
25832589
If NemoClaw detects active SSH sessions before destroy, it warns that destroy terminates them with a Broken pipe error and lists their process IDs.
25842590
This warning prints before the confirmation prompt and when `--yes` or `--force` skips that prompt.
@@ -2593,9 +2599,14 @@ If the recorded PID is live, its command line must match the exact gateway name
25932599
If the recorded process has exited, `destroy` continues only after it verifies that the gateway port is free.
25942600
If a live PID does not prove gateway ownership or the port remains occupied, `destroy` exits non-zero and preserves the runtime evidence for inspection.
25952601
For any other service stop failure, `destroy` exits non-zero after sandbox and registry deletion, prints the status command for the service, and skips gateway and volume removal.
2596-
If the OpenShell gateway is unreachable and the sandbox has no managed MCP ownership state, `--force` removes only NemoClaw's local registry entry and local artifacts.
2602+
If the OpenShell command completes with a gateway transport error and the sandbox has no managed MCP ownership state, `--force` removes only NemoClaw's local registry entry and local artifacts.
25972603
Gateway-side deletion remains unconfirmed, shared host-service and gateway teardown are skipped, and the sandbox and retained volume may still exist if the gateway returns.
25982604
Start the gateway with `$$nemoclaw <name> status` and retry destroy when you need a confirmed deletion.
2605+
If the OpenShell sandbox deletion command reaches its 60-second timeout, NemoClaw cannot confirm whether OpenShell deleted the sandbox.
2606+
NemoClaw preserves the local registry entry under both `--yes` and `--force`.
2607+
Run `$$nemoclaw <name> status` to check or start the recorded OpenShell gateway.
2608+
If the preceding output also reports a failed pre-delete re-lock, run `$$nemoclaw <name> shields status` to verify recovery or follow its durable-containment guidance, then retry `destroy` only after shields recovery permits it.
2609+
Otherwise, retry after the recorded gateway is available.
25992610
Managed MCP ownership disables the local-only fallback because exact provider cleanup requires the retained ownership state, and other delete failures remain fatal.
26002611
A failed pre-delete re-lock also disables the local-only fallback, because the auto-restore timer is then the only authority that can lock the configuration again after the gateway returns.
26012612

internal/security-reviews/openclaw-2026.6.10-dependency-review.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ The published `@openclaw/msteams@2026.6.10` artifact was re-reviewed after integ
190190

191191
## Bundled Weather Skill Egress Review
192192

193-
The SRI-verified `openclaw@2026.6.10` artifact's `package/skills/weather/SKILL.md` has SHA-256 `62ab4821aa873949d1c1091836be1659a42b32caadce4bd145f5505a1ceaeec1`, unchanged from the reviewed 2026.6.9 artifact. The reviewed skill prefers `web_fetch` to HTTPS `wttr.in` paths and lists HTTPS `wttr.in` curl fallbacks using read-only requests; it mentions `wttr.is` only as an optional retry when the primary service is unreliable. NemoClaw's weather preset therefore continues to allow only GET/HEAD to `wttr.in` at that boundary and intentionally leaves `wttr.is` denied unless a future pinned runtime makes the fallback required. `test/weather-policy.test.ts` binds that host/method contract to the reviewed OpenClaw version.
193+
The SRI-verified `openclaw@2026.6.10` artifact's `package/skills/weather/SKILL.md` has SHA-256 `62ab4821aa873949d1c1091836be1659a42b32caadce4bd145f5505a1ceaeec1`, unchanged from the reviewed 2026.6.9 artifact. The reviewed skill prefers `web_fetch` to HTTPS `wttr.in` paths and lists HTTPS `wttr.in` curl fallbacks using read-only requests; it mentions `wttr.is` only as an optional retry when the primary service is unreliable. NemoClaw's weather preset therefore continues to allow only GET/HEAD to `wttr.in` at that boundary and intentionally leaves `wttr.is` denied unless a future pinned runtime makes the fallback required. `test/onboarding/effective-policy-contracts.test.ts` binds that host/method contract to the reviewed OpenClaw version.
194194

195195
## PR Review Follow-ups
196196

@@ -295,7 +295,7 @@ The pinned OpenClaw 2026.6.10 devices CLI normally requests the scopes it is try
295295

296296
At the host-caller boundary, NemoClaw no longer reads or writes device state during approval. Inside the reviewed compiled pairing module, Patch 8 writes a fixed-version, exact-schema `idle` / `prepared` / `committed` journal beside the pairing files with mode `0600` and a `0700` directory contract. Before publication it records the exact before/after snapshots and request/device identity in `prepared`; because those snapshots can contain device tokens, the journal is never logged, remains permission-bounded, and drops all snapshots when it returns to `idle`. The module waits for both canonical pending and paired writes with `Promise.allSettled`, records `committed` only after both succeed, and finally returns the journal to `idle`. Recovery runs from pairing-state loads under the module lock, rejects a malformed journal or any current file that is neither its exact before-image nor after-image, restores `prepared` transactions backward, completes `committed` transactions forward, and returns to `idle`, so a fresh process deterministically settles either one-sided publication direction. A synchronous publication failure uses the same prepared recovery before the approval error is returned.
297297

298-
`scripts/lib/openclaw_device_approval_policy.py` remains a pure allowlist/environment helper that requires the explicit `cli`, `openclaw-cli`, or `openclaw-control-ui` client identity and never accepts an unknown identity merely for claiming `cli` or `webchat` mode; the startup, interactive-shell, and connect-time callers count only an OpenClaw CLI exit status of zero. Invalid state: any caller without device-token auth, signed same-device identity, exact CLI/operator metadata, existing `operator.pairing`, or complete bounded non-admin scopes receives the self-approval exception; current pending state is not revalidated inside the pairing lock; a host caller reads or publishes `pending.json` / `paired.json`; a failed CLI result is counted as approved; concurrent canonical approvals lose an unrelated pending request or paired token; or an interrupted two-file publication cannot recover to the journal's exact before/after state. Source boundary: the reviewed OpenClaw CLI, session producer, canonical session-authz resolver, gateway dispatcher/device handler, pairing-state dist module and its fixed-version journal, the pure policy module, and the three host callers. Source-fix constraint: OpenClaw owns pairing state; keeping recovery inside its reviewed compiled module and existing lock is safer than a host-side writer, while native atomic/recoverable publication belongs upstream. Regression detection: `test/agents/openclaw/openclaw-device-self-approval-patch.test.ts`, `test/agents/openclaw/openclaw-device-approval-policy.test.ts`, `test/nemoclaw-start-scope-replacement.test.ts`, connect-time auto-pair tests, the exact-dist linkage/real-handler/concurrent-publication/restart-recovery proof, and the issue #4462/device-auth live lanes. Removal condition: delete Patch 8 when a reviewed OpenClaw release completes this bounded same-device flow natively, but only if that release also publishes the pending/paired transition atomically or with equivalent durable restart recovery; retain the no-admin live assertion and behavioral proof that host callers leave device state untouched.
298+
`scripts/lib/openclaw_device_approval_policy.py` remains a pure allowlist/environment helper that requires the explicit `cli`, `openclaw-cli`, or `openclaw-control-ui` client identity and never accepts an unknown identity merely for claiming `cli` or `webchat` mode; the startup, interactive-shell, and connect-time callers count only an OpenClaw CLI exit status of zero. Invalid state: any caller without device-token auth, signed same-device identity, exact CLI/operator metadata, existing `operator.pairing`, or complete bounded non-admin scopes receives the self-approval exception; current pending state is not revalidated inside the pairing lock; a host caller reads or publishes `pending.json` / `paired.json`; a failed CLI result is counted as approved; concurrent canonical approvals lose an unrelated pending request or paired token; or an interrupted two-file publication cannot recover to the journal's exact before/after state. Source boundary: the reviewed OpenClaw CLI, session producer, canonical session-authz resolver, gateway dispatcher/device handler, pairing-state dist module and its fixed-version journal, the pure policy module, and the three host callers. Source-fix constraint: OpenClaw owns pairing state; keeping recovery inside its reviewed compiled module and existing lock is safer than a host-side writer, while native atomic/recoverable publication belongs upstream. Regression detection: `test/agents/openclaw/openclaw-device-self-approval-patch.test.ts`, `test/agents/openclaw/openclaw-device-approval-policy.test.ts`, `test/agents/openclaw/runtime/nemoclaw-start-scope-replacement.test.ts`, connect-time auto-pair tests, the exact-dist linkage/real-handler/concurrent-publication/restart-recovery proof, and the issue #4462/device-auth live lanes. Removal condition: delete Patch 8 when a reviewed OpenClaw release completes this bounded same-device flow natively, but only if that release also publishes the pending/paired transition atomically or with equivalent durable restart recovery; retain the no-admin live assertion and behavioral proof that host callers leave device state untouched.
299299

300300
### Recovered Gateway Credential Boundary
301301

oxlint.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export default defineConfig({
6363
"scripts/**/*.js",
6464
"scripts/**/*.mjs",
6565
"test/**/*.js",
66-
"test/credentials-shim.test.ts",
66+
"test/package-contract/credentials-shim.test.ts",
6767
"test/e2e-runtime/runner-basic.test.ts",
6868
],
6969
rules: {

package-lock.json

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@
141141
"@j178/prek": "^0.3.6",
142142
"@types/node": "^25.5.2",
143143
"@vitest/coverage-v8": "^4.1.0",
144+
"convert-source-map": "2.0.0",
144145
"eslint-plugin-sonarjs": "4.2.0",
145146
"fast-check": "^4.8.0",
146147
"oxfmt": "0.63.0",

src/commands/sandbox/inference/set.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { sandboxNameArg } from "../../../lib/sandbox/command-support";
1515
// here — deep validation (provider allowlist, model id charset, custom endpoint
1616
// URL/credential/API normalization) is intentionally centralized in
1717
// runInferenceSet so the global and sandbox-first grammars share one
18-
// validation surface (covered by test/lib/actions/inference-set.test.ts).
18+
// validation surface (covered by the src/lib/actions/inference-set-*.test.ts suites).
1919
export default class SandboxInferenceSetCommand extends NemoClawCommand {
2020
static id = "sandbox:inference:set";
2121
static strict = true;

0 commit comments

Comments
 (0)