Repository navigation
Conversation
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.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. |
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit 4539d0f in the TypeScript / code-coverage/cliThe overall line coverage in commit 4539d0f in the Show a line coverage summary of the most impacted files.
Updated |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> # Conflicts: # ci/e2e-assertion-budget.json
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
9 of 20 tasks
prekshivyas
added a commit
that referenced
this pull request
Sep 25, 2026
## Outcome Ordinary sandbox commands, probes and diagnostics use native OpenShell execution. Failed or ambiguous commands do not retry through SSH or privileged local execution. Supported provider recovery, interactive SSH and file transfer retain their existing authority checks. ## Reason Multiple ordinary-command transports could run equivalent commands with different identities or repeat an ambiguous mutation through a more privileged path. ### Related issues Fixes #11263, part of #11255. Includes merged prerequisites #12214, #12222, #12236, #11911, #12258 and #12256. ## Changes - Remove ordinary SSH execution, compatibility wiring and privileged fallbacks. Preserve named-gateway targeting, runtime identity, filtered environments, timeouts and distinguishable failures. - Route status through the same native executor while preserving its deadline and nullable transport-failure behavior. The OpenClaw readiness probe also preserves unavailable transport evidence after integrating #12256. - Retire unsupported custom gateway SSH recovery after auditing shipped manifests. Keep supported recovery, interactive access and file transfer. - Prevent WeChat removal when orphaned physical-session cleanup cannot be confirmed; document that behavior even without a channel entry. - Refresh corporate-CA trust through native stop/start of the same sandbox, and bound/redact MCP diagnostics. - Avoid evaluating a sandbox-controlled shell file for ordinary commands, and test actual process boundaries to reject SSH retries. ## Verification Current candidate: `836320e005cdb041c28afe12a7986bd4b8f05a5a`, integrating canonical `main` at `350a9863cd83b5a8ee7932d4ab916393d7e69279`. The merge resolves the base conflict, preserves the newer bounded MCP HTTPS diagnostics, and repairs native-version CLI fixtures to emit the required sandbox-exec marker while rejecting SSH fallback. The final follow-up pins the Hermes diagnostics support test to its asserted runtime and restores the ambient environment after each case. Local evidence: 14 CLI integration tests, 94 E2E-support tests, and 72 focused transport/version/debug tests passed. CLI TypeScript passed with an 8 GiB heap ceiling; lint, formatting, all 18 repository checks, signed commit hooks, publication validation, and pre-push CLI/plugin TypeScript checks passed. Hosted CI on this exact head is green, including all 12 CLI shards, aggregate CLI, managed startup for OpenClaw/Hermes/Deep Agents Code, exact all-agent activation on Docker and rootless Podman, both Pi image builds, rootless lifecycle and portable profile, CodeQL, audits, docs, and static checks. The only red attempt was an external HTTP 429 fetching the pinned Hermes archive; its single rerun passed. The earlier managed-image failure at `ceca9ae56` was an external `ImagePullFailed` ("bytes remaining on stream"); fail-closed cleanup remained intact and the explicit OpenShell cleanup removed the sandbox. Evidence below that names another candidate or says current-head is historical for `836320e00`. Previous candidate: `3caba6799cc006bd6ee2a891719509d73f773d73`. This follows the conflict-resolution merge `908a0b4`, which integrated main `2e162f266f583d78392494feff44c360d1390ad0`, without another base integration. The follow-up preserves later diagnostics and archive creation when endpoint authority refuses sandbox-internals collection. Cancellation and unexpected errors still propagate. It replaces an obsolete nullable DeepAgents transport mock with typed failure coverage and adds public-create coverage proving corporate-CA refresh finishes before registration. All 94 tests across seven affected suites passed, along with CLI TypeScript, source-shape, lint and formatting. Independent review, signed commit hooks, isolated pre-push validation, container cleanup and actual push hooks passed. Current-head [required CI](https://github.com/NVIDIA/NemoClaw/actions/runs/35967107832), [all nine Advisor reports and aggregate](https://github.com/NVIDIA/NemoClaw/actions/runs/35968484852), substantive CodeRabbit review, [managed images](https://github.com/NVIDIA/NemoClaw/actions/runs/35967107774), and [portable rootless checks](https://github.com/NVIDIA/NemoClaw/actions/runs/35967107785) passed. Image qualification covered all three agents on Docker and rootless Podman, with 36 activation turns and 18 cleanup actions total. Five current-head manual runs passed: - [Native CPU startup](https://github.com/NVIDIA/NemoClaw/actions/runs/35969691815): all three agents on AMD64 and ARM64; cleanup verified. - [Docker onboarding](https://github.com/NVIDIA/NemoClaw/actions/runs/35971477529): three repair/resume scenarios; 28 cleanup actions. - [Standard Docker lifecycle](https://github.com/NVIDIA/NemoClaw/actions/runs/35972475425): eight scenarios; 38 cleanup actions. - [Docker MCP](https://github.com/NVIDIA/NemoClaw/actions/runs/35973673916): all three agents and the credential-generation-window test; 51 cleanup actions, including explicit successful removal of all three private relays. - [Hermes Docker lifecycle](https://github.com/NVIDIA/NemoClaw/actions/runs/35975851054): all eight phases, including restart, ACP and configuration integrity; seven cleanup actions. These results qualify the stated scopes only. Remaining Podman lifecycle prerequisites, development-runtime policy disposition and unexecuted provider/protected scopes still prevent a full review-readiness claim. At parent `908a0b4`, managed images activated all three agents on Docker and Podman: 18 turns and nine cleanup passes per runtime. Its portable rootless fixture failed an initial PID identity check before onboarding. The unchanged fixture passed ten isolated Linux cycles, but the hosted cause remains unresolved. Neither result qualifies this new candidate. ### Historical evidence The following results belong to earlier commits and do not qualify the current candidate. The transport repair moves ordinary execution and its environment wrapper into the sandbox transport adapter, retargets all callers, improves public health-outcome coverage, and gives the OpenClaw skill fixture an account-home workspace with immediate cleanup registration. - Local affected suites: 1,234 tests passed, with 14 skips; 943 additional integration tests passed, with 15 skips. The sole remaining affected-suite failure is the unchanged Hermes Python fixture on a host without PyYAML. The dependency-boundary follow-up passed 466 targeted tests. TypeScript, lint, formatting, mock/live parity, focused cleanup tests and the architecture census passed; four stale architecture limits were lowered, with no increases. Current-head hosted evidence remains required after publication. - At `81936456060c102fe1d88795ffe31f6830d39a6a`, CI passed and all nine Advisor reports were inspected. The architecture finding and CodeRabbit's startup-test duplication finding are addressed by this repair. - [Live validation at 8193645](https://github.com/NVIDIA/NemoClaw/actions/runs/35832734620) finished with 41 selected scenarios passing and 17 failing. Thirteen Podman scenarios reported incompatible or ambiguous gateway ownership; another failed rebuild preflight. The selected-gateway runtime fix is isolated in #12267 and has not yet qualified this main PR. - Other failures: Docker provider selection exceeded its 8-second budget; the protected amd64 OpenClaw normalization probe timed out; and the Docker skill fixture failed finalization. The unsafe skill workspace placement is corrected here, but the complete live failure cause has not been proved. No latency, timeout, or baseline waiver is claimed. - Of 64 ordinary cleanup receipts, 63 were clear. Skill CLI cleanup failed; its fallback sandbox deletion, gateway removal and home cleanup passed. Both protected qualification daemon-removal receipts passed. Historical results do not qualify the new commit. - At parent `85256b3`, CI and all nine Advisor reports passed. Exact managed images activated all three agents on Docker and Podman, with 18 turns and nine clean teardown actions per runtime. CodeRabbit identified two dead duplicate mock setups; the preceding test-only correction replaces them with fail-fast unexpected-call stubs and explicit zero-call assertions. All 52 focused/growth tests, source-shape and parity checks passed. The trusted matcher still requires fresh managed-image qualification for this candidate; no ancestor-image override is used. - At parent `4c4dc76`, CI and CodeRabbit passed. All nine Advisor reports were collected; one reduction finding identified an impossible null return in the native-command facade type. The current repair narrows that contract and removes unreachable direct-consumer branches, retaining explicit typed transport-error mappings and remote nonzero exit handling. 364 affected tests passed, 14 skipped; seven growth tests, TypeScript, parity, source-shape, lint and formatting passed. Docker and Podman core image activation passed at that parent, but its image workflow failed an upstream Pi Perl DNS test. No parent result clears the current commit. - Advisor additionally requires the OpenShell gateway-auth contract scenario. It remains part of the outstanding current-head E2E work. ## Review notes The merged Podman artifact renewal and full-install cleanup prerequisite are included. Remaining fixture prerequisites are #12267 (Hermes ACP Podman context) and #12269 (MCP cleanup). Full Podman lifecycle coverage remains pending. Protected GPU coverage needs the offline npm prerequisite. Observer coverage is tracked by #12238/#12197. The dev MCP lane conflicts with the supported installer channel and needs disposition. Real-provider messaging and exact staging Launchable coverage are not claimed. No human approval, gate waiver or merge-readiness claim is made. --- Signed-off-by: Deepak Jain <deepujain@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Sandbox version checks, diagnostics, and maintenance commands now use native OpenShell execution. * Managed sandbox onboarding refreshes corporate CA trust before completing setup when a CA is configured. * CLI recovery messages now direct you to relevant OpenShell commands. * **Reliability** * Channel removal stops when cleanup cannot be confirmed, rather than proceeding with an uncertain result. * Sandbox transport failures are reported without retrying through SSH or a local runtime. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Deepak Jain <deepujain@gmail.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
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 <571084+rsliter@users.noreply.github.com>
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 <571084+rsliter@users.noreply.github.com>
ericksoa
added a commit
that referenced
this pull request
Sep 27, 2026
## Outcome Ordinary OpenClaw agent turns use the native write scope. Privileged operations still require explicit approval, and the approving CLI confirms the rotated admin token through the same gateway before returning success. ## Reason Pinned OpenClaw 2026.9.1 forces admin scope for ordinary local turns. Removing that override restores the affected consumers, but the first minimum-scope connection after admin approval can cache narrower token metadata than readiness expects. A read-only post-approval confirmation completes that handoff while preserving the existing readiness and authorization checks. ## Changes - Delegate ordinary local agent calls to OpenClaw's method scope resolver. Model overrides and session resets retain admin scope. - After a successful admin approval whose device ID matches the calling CLI’s existing identity, confirm it with a read-only `device.pair.list` request using the same gateway options and admin scope. The confirmation uses the existing normal gateway transport so its token cache can be written; ordinary device-command reads remain read-only. Confirmation failure propagates before success output. Other-device approvals, absent identities, non-admin approvals, and missing requests are unchanged. An identity-read exception reports that approval completed but the handoff failed, with recovery guidance; it cannot report success. The real-distribution proof exercises the device wrapper, canonical transport, and native token-store policy, including preservation of read-only behavior for ordinary device queries. The exact-block patch remains audited and idempotent. - Inference switching explicitly approves the matching admin request before its required gateway restart. The llama.cpp GPU test now proves ordinary inference before approval and uses the existing admin-only cron operation and exact-request helper for denial, approval, and privileged-consumer proof. GPU, model, authentication, and cleanup checks remain live. - Select the existing six affected test families for patch changes: eleven Docker/Podman executions. Extend the existing planner and pinned-distribution proofs. Direct live expectations decrease from the main baseline of 1,381 to 1,378; the llama.cpp case's transitive assertion points remain 61. No budget exception. ## Verification - Latest repair: 157 focused Vitest tests passed across five files; all seven growth-guardrail tests passed. - Executed the real pinned distribution's approval command, device wrapper, CLI transport, gateway-client host dependencies, and client cache expression for seven approval outcomes, plus eight ordinary/admin agent scope cases. The handoff proof failed before the confirmation patch and passes afterward, including confirmation denial and preservation of approved metadata through a later ordinary call. - Patch audit passed all nine current-runtime targets. Older JSON-runtime scope/approval tests remain covered by the focused suite. - Assertion ratchet, semantic E2E phases, mock/live parity, source-shape budget, formatting, and diff checks passed. - Exact head `0be5e29e4b8c3a17a4116e866a9152a7d792e60a`: [focused qualification](https://github.com/NVIDIA/NemoClaw/actions/runs/36349722018) passed full OpenClaw on Docker and Podman plus llama.cpp GPU, with clean cleanup. The [complete supported Docker/Podman matrix through attempt 3](https://github.com/NVIDIA/NemoClaw/actions/runs/36350771086/attempts/3) finished with 119 passed and 14 failed: all 110 retained main passes and all 119 known passes are preserved. The remaining 14 failing cases were already red in the pre-repair baseline. All eight original scope-handoff cases and the three repaired cases pass. - [CI](https://github.com/NVIDIA/NemoClaw/actions/runs/36338962655), [growth](https://github.com/NVIDIA/NemoClaw/actions/runs/36356766705), [managed images and activation](https://github.com/NVIDIA/NemoClaw/actions/runs/36338962659), [Advisor](https://github.com/NVIDIA/NemoClaw/actions/runs/36339790443), and [automatic qualification after ready-for-review](https://github.com/NVIDIA/NemoClaw/actions/runs/36356769500) passed. - No local live E2E was run. The diff contains no secrets, API keys, or credentials. ## Review notes The runtime changes stay in the existing pinned OpenClaw patch. The new call happens only after explicit approval returns an admin grant and reuses the original gateway options. It does not alter pairing acceptance, signature or identity checks, automatic approval policy, cleanup authority, timeouts, or versions. Self-review covered absent approval, ordinary approval, other-device approval, unavailable local identity, denied confirmation, and ordinary turns after confirmation. All nine exact-head Advisor reports are clear. CodeRabbit completed with no blocking threads. Its two minor suggestions concern an additional explicit audit marker and a uniqueness guard for a future upstream layout; the qualified head is preserved. The pinned distribution has exactly one gateway-call module, and fresh/upgrade byte equality, nine-target audit, native token persistence, and denial paths were exercised. No dependency on #12238 or #12332. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Access and approvals** * Ordinary local agent turns now use standard write permissions rather than administrator access. Administrator permissions remain available for model overrides, session resets, and remote turns. * After an administrator approval, the CLI confirms the grant with the gateway only when the approved device matches the CLI’s identity. Identity lookup failures are reported as errors. * **Testing and documentation** * Updated end-to-end coverage and guidance for inference switching, administrator approvals, gateway restarts, and GPU inference. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
# Conflicts: # ci/e2e-assertion-budget.json # test/e2e/live/issue-4462-admin-approval-helper.ts # test/e2e/mock-parity.json # tools/e2e/target-catalogue.mts
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Contributor
|
PR Review Advisor finished for commit Request review only when Require no Advisor blockers is green. |
Collaborator
Author
|
Closing as superseded by the current main behavior. The exact |
wscurran
added
the
area: e2e
wscurran
added
area: security
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
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.
Outcome
The focused issue-4462 live target now proves that the first gateway-backed agent request succeeds before any admin approval. It then proves explicit request-bound
operator.adminapproval and successful cron add and run behavior while removing the obsolete state observer.Reason
The previous observer read retired JSON pairing files and could stop the test before the product behavior. Internal token and device counts duplicated deterministic authorization coverage and did not prove the user journey.
Related issues
Refs #12124
Changes
nemoclaw <sandbox> agentwrapper before the cron request. Cap retained command output at 64 KiB and require the expected reply; the wrapper fails embedded fallback before success.Verification
npx vitest run --project cli src/lib/actions/sandbox/agent/passthrough.test.ts --silent=false --reporter=default- 73 tests passed.npx vitest run --project e2e-support test/e2e/support/issue-4462-fixture-boundary.test.ts --silent=false --reporter=default- 2 tests passed.npx vitest run --project integration test/security/admin-approval-helper.test.ts --silent=false --reporter=default- 21 tests passed.npx vitest run --project integration test/inference/managed/managed-image-publication-workflow.test.ts --silent=false --reporter=default- 36 tests passed.npx vitest run --project integration test/automation/e2e/e2e-mock-parity.test.ts --silent=false --reporter=default- 54 tests passed.npx tsx scripts/checks/e2e-mock-parity.mts --base origin/main --head HEAD- passed against canonical main994f73fc9e.npm run test:changed- growth guardrail passed 7 tests; the changed cli, plugin, and e2e-support selector had no additional files after the merge topology.npm run e2e:assertions:check- passed with 1,389 direct expect calls across 79 live test files.npm run test:e2e-phases:check- passed for 102 tests across 80 files.npm run test:projects:check- exact membership passed for 2,702 candidate files across 7 projects.npx vitest run --project e2e-support test/e2e/support/workflow-plan.test.tsafter the latest Advisor repair - 121 tests passed.npm run test:changedafter the latest Advisor repair - 7 growth-guardrail and 930 selected tests passed.ea247e7819, including publication validation and CLI TypeScript checks.Review notes
This PR changes the sensitive paths
.github/workflows/managed-images.yamlandtest/e2e/live/**. Author self-review compared the complete diff with canonicalNVIDIA/NemoClawmain994f73fc9e; the workflow edit only removes the deleted observer path trigger, and the live-target edit is covered by the focused deterministic checks above. Exact-head Advisor runs 36247022050 and 36248689645 narrowed the transport evidence to the owned production boundary. Commitdec7149707routes the first turn through NemoClaw's existing non-JSON agent wrapper, which rejects embedded fallback before success, and removes the redundant custom classifier. Exact-head Advisor run 36255670783 identified one scope-local ownership gap in the shared gateway classifier. Commitea247e7819registers all three live consumers in the typed target catalogue and adds deterministic planner coverage. The draft awaits automated review of this latest repair commit.Dependency #12197 has merged into the integrated base. Earlier trusted manual PR E2E run 35777000382 exercised the exact
issue-4462-scope-upgrade-approvaltarget at commit2a5419b8a0. Phase 3 proved the first gateway-backed agent request succeeded without admin approval. Phase 4 returnedunexpected-successinstead ofapproval-requiredbecause that run's base did not yet contain #12197.The exact focused target must now pass at commit
ea247e7819before this draft is ready for human review.Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com