test(policy): align command coverage loader - #10408
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe policy command test replaces its static module import with a typed CommonJS ChangesPolicy test import
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This change only updates policy test loading and does not alter production behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
<!-- markdownlint-disable MD041 --> ## Summary Fixes the five legacy-upgrade regressions introduced by #10132 and exposed by the OpenShell 0.0.106 missing-gateway diagnostic. During an upgrade the installer intentionally removes the old `nemoclaw` gateway; `Unknown gateway 'nemoclaw'` now enters the existing `transport/unreachable` recovery path. The same live lanes exposed two later ordering/identity regressions already present on main. The hotfix now lets the intentional restart-safe recreation reconnect before revalidating the exact sandbox identity, and scopes #10044's sole-container proof to the exact replacement's OpenShell gateway namespace. A same-name orphan from the retired legacy gateway no longer invalidates the current gateway's replacement. This restores the previously supported missing-gateway recovery path. It does not weaken identity-mismatch, authentication, policy-authority, exact-container-ID, Ready, sandbox-exec, or same-gateway ambiguity fail-closed behavior. Affected lanes: [run 32909322579](https://github.com/NVIDIA/NemoClaw/actions/runs/32909322579). ## Changes - Classify `Unknown gateway` as `transport/unreachable` in the existing CLI observer. - Exercise existing named-gateway recovery and successful retry. - Preserve `handshake verification failed` as `identity_mismatch` without recovery. - Wait for the intentionally recreated supervisor to reconnect before exact lifecycle revalidation. - Keep #10044's exact-ID and sole-container proof, scoped to the replacement's `openshell.ai/sandbox-namespace` so retired-gateway orphans do not create false ambiguity. - Update the shared onboarding test fixture to model the namespace proof. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: exact-head advisor specialists, CodeRabbit, identity-bound unit coverage, local live Arm64 upgrade, and managed-image qualification passed. - [x] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: [CLI coverage aggregate waiver](#10319 (comment)); every CLI shard passed, and the only aggregate failure is byte-identical current-main `src/lib/policy/commands.ts` coverage from #10408. ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; legacy Docker gateway upgrade lanes are CPU-host qualifications. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: [five-lane E2E run 32996253520](https://github.com/NVIDIA/NemoClaw/actions/runs/32996253520). ## 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 - [x] Targeted behavior tests passed - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed Exact tests and checks run: - `npx vitest run --project cli src/lib/onboard/docker-gpu-patch-finalize.test.ts src/lib/onboard/docker-gpu-supervisor-reconnect.test.ts src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts src/lib/adapters/openshell/sandbox-observer-cli.test.ts src/lib/openshell-sandbox-list.test.ts` — 5 files, 88 tests passed. - `npx vitest run src/lib/onboard/docker-gpu-patch-finalize.test.ts src/lib/onboard/openshell-docker-sandbox-containers.test.ts src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts src/lib/onboard/sandbox-gpu-create-identity-gate.test.ts src/lib/onboard/sandbox-gpu-create-flow.test.ts src/lib/onboard/sandbox-recreate-transaction.test.ts` — 6 files, 189 tests passed. - `npx vitest run test/onboarding/onboard-custom-dockerfile.test.ts test/onboarding/onboard-messaging.test.ts` — 2 files, 25 tests passed. - `npx vitest run --project integration test/automation/pull-requests/growth-guardrails.test.ts` — 1 file, 32 tests passed. - `npm run typecheck:cli` — passed. - `npm run build:cli` — passed. - `npm run checks:repository` — passed. - `npx oxfmt --check` on changed production/test files — passed. - `npx oxlint --type-aware --type-check` on changed production/test files — passed. - `git diff --check` — passed. - Local isolated Ubuntu 24.04 ARM64 live `openshell-gateway-upgrade-v0-0-55-aarch64` on production head `5ff730b3b9ffb393b2648f86cb88cdfce5fc7829` — 4/4 tests passed. - Exact production five-lane legacy-upgrade E2E — [run 32996253520](https://github.com/NVIDIA/NemoClaw/actions/runs/32996253520), 5/5 passed. - Exact-head managed images — [run 32999149006](https://github.com/NVIDIA/NemoClaw/actions/runs/32999149006), 10/10 jobs passed. - Exact-head CI — all 12 CLI shards passed; aggregate non-success is covered by the maintainer waiver above. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
## Summary Ordinary fresh OpenClaw onboarding now has one initial scope-upgrade approval owner. The host produces and observes the canonical request while the in-sandbox watcher alone approves it, so startup timing no longer decides which concurrent approver wins. ## Related Issue Fixes #10269 ## Changes - Remove host-side ordinary onboarding approval and keep finalization read-only after one bounded request-producing warm-up. - Require the same canonical CLI device to reach the exact baseline scopes with no pending same-device request before onboarding succeeds. - Keep the watcher on its bounded fast cadence until canonical settlement, then switch to the existing slow cadence. - Add fixed, redacted warm-up and terminal failure classifications without exposing device identifiers, request identifiers, keys, tokens, or child output. - Preserve the separate restored-clone approval path and document the ordinary onboarding authority boundary. - Add focused coverage for delayed publication, watcher settlement, persistent pending state, identity drift, rejected approval, and fast-to-slow polling. - Ratchet the legacy `nemoclaw-start.test.ts` file-size allowance down after moving shared settlement fixtures. ## 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 completed the nine-category review at commit under review `d032f18e773e1a798fb6ef5835712538b9cb7edd`. Result: PASS. Ordinary approval remains owned only by the in-sandbox watcher. Success requires the exact local CLI identity, canonical roles, scopes, active token state, no same-device pending request, and current runtime and policy authority. State reads are descriptor-pinned and fail closed on filesystem, identity, target, or policy drift. Host-visible results are fixed and redacted. Request production, observation, and settlement are bounded. Current main adds only the test loader correction from #10408 outside this effective diff. No injection, authorization, credential-custody, network, filesystem, dependency, logging, or denial-of-service blocker was found. Independent approval remains required before merge. - [ ] Non-success, skipped, or missing CI check accepted by maintainer: check name, approval link, and follow-up issue: None accepted. The previous commit's twelve CLI shards passed, but its aggregate failed because base-owned `src/lib/policy/commands.ts` reported 88.88% against its 100% threshold. Current main `803a587752aef1896c659f0ed7373d501b3b4b24` includes #10408's two-line test loader correction for that root. Automatic CI for commit `d032f18e7` is the required proof. ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable - Supporting evidence: Not applicable ## 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: the stable 13-file patch passed 125 CLI tests and 185 integration tests. The signed current-main merge changes no PR-owned behavior, and exact `npm run validate:pr` passes at `d032f18e7`. `git diff --check` passes. - [ ] Applicable broad gate passed: Automatic CI for commit `d032f18e7` is pending. No failure is accepted. - [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): Fern reported 0 errors and 2 existing warnings; route validation passed. - [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] A separate documentation writer reviewed the completed changes - Result: `docs-updated` - Evidence: Reviewed the complete 13-file effective diff at signed merge commit `d032f18e773e1a798fb6ef5835712538b9cb7edd` against exact current main `803a587752aef1896c659f0ed7373d501b3b4b24`. Its stable patch ID exactly matches the previously reviewed `77c2` root2 diff, and current main's policy command test loader correction is absent from the effective diff. The updated OpenClaw documentation accurately assigns ordinary onboarding approval to the in-sandbox watcher, limits the host to bounded warm-up and observation, requires exact same-device settlement, and describes redacted failure diagnostics. Exact PR validation, 125 CLI tests, 185 integration tests, documentation route and build validation, variant synchronization, prior per-shard tests, and `git diff --check` passed. - Agent: Codex documentation writer (`/root/docs_review_10270`) <!-- docs-review-head-sha: d032f18 --> <!-- docs-review-agents-blob-sha: 281e79f --> --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> --------- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Signed-off-by: Rebecca Sliter <rsliter@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Summary
Align the policy command builder test with the CommonJS-compatible loader used by policy and Shields tests. This prevents Vitest V8 coverage from splitting the same source across loader identities and dropping strict per-file coverage after shard rebalancing.
Changes
Type of Change
Quality Gates
src/lib/policy/commands.test.tschanges; production policy behavior is unchanged. CI will run the strict merged coverage gate.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable — Normal pre-commit and commit-msg hooks passed during commit and pre-push hooks passed during publication.commands.tsfunction covered.npm run typecheck:cliandnpm run checks:repositorypassed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: The authoritative 12-shard merged coverage gate will run in PR CI; local full-shard experiments were not completed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit