test(e2e): consolidate sandbox resource-limit coverage - #8070
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
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:
📝 WalkthroughWalkthroughThe standalone ChangesSandbox connect consolidation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant E2ESelector as E2E selector
participant WorkflowBoundary as workflow-boundary
participant SandboxOperations as sandbox-operations
participant ConnectedShell as connected shell
E2ESelector->>WorkflowBoundary: Select sandbox-rlimits-connect
WorkflowBoundary->>SandboxOperations: Resolve the selector alias
SandboxOperations->>ConnectedShell: Run connect resource-limit probes
ConnectedShell-->>SandboxOperations: Return limits and diagnostic status
SandboxOperations-->>E2ESelector: Return the consolidated E2E result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 84ed851 in the TypeScript / code-coverage/cliThe overall coverage in commit 84ed851 in the Show a code coverage summary of the most impacted files.
Updated |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/e2e/live/sandbox-operations.test.ts`:
- Around line 40-43: Remove raw connected-shell output from diagnostics and
artifacts in numericProbe, the connect assertion around expectExitZero, and the
code near line 740. Parse and retain only limit values, exit status, markers,
and bounded redacted, content-free metadata; do not include resultText(connect)
or output in assertion messages, logs, or artifacts.
🪄 Autofix (Beta)
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: 4ba287dc-ea2f-4f76-b849-198f083ad235
📒 Files selected for processing (16)
.github/workflows/e2e.yamltest/e2e/fixtures/resource-limit-diagnostics.tstest/e2e/live/sandbox-operations.test.tstest/e2e/live/sandbox-rlimits-connect.test.tstest/e2e/mock-parity.jsontest/e2e/support/e2e-live-target-gating.test.tstest/e2e/support/e2e-scorecard.test.tstest/e2e/support/e2e-workflow.test.tstest/e2e/support/resource-limit-diagnostics.test.tstest/e2e/support/sandbox-operations-workflow-boundary.test.tstest/e2e/support/workflow-plan.test.tstest/release-e2e-evidence.test.tstools/e2e/report-e2e-results.mtstools/e2e/sandbox-operations-workflow-boundary.mtstools/e2e/selector-aliases.mtstools/e2e/workflow-boundary.mts
💤 Files with no reviewable changes (1)
- test/e2e/live/sandbox-rlimits-connect.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/e2e/fixtures/resource-limit-diagnostics.ts`:
- Around line 6-7: Frame resource-limit diagnostics strictly across
test/e2e/fixtures/resource-limit-diagnostics.ts:6-7 by accepting only complete
standalone begin/end marker lines and emitting fields only while inside that
frame; update numericProbe in test/e2e/live/sandbox-operations.test.ts:41-46 to
require each expected field exactly once and reject missing, duplicate, or
out-of-frame fields; replace the embedded-marker test in
test/e2e/support/resource-limit-diagnostics.test.ts:38-58 with coverage
rejecting prefixed markers, out-of-frame fields, and duplicate fields.
🪄 Autofix (Beta)
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: d4b81d26-cb2e-40f2-b8a9-e03d7269f214
📒 Files selected for processing (3)
test/e2e/fixtures/resource-limit-diagnostics.tstest/e2e/live/sandbox-operations.test.tstest/e2e/support/resource-limit-diagnostics.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/support/resource-limit-diagnostics.test.ts (1)
79-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd incomplete-frame coverage.
The filter flags an open frame at input close as a protocol error. Add a test with a begin marker and no end marker. Assert
resource_limit_protocol_error=1.As per path instructions, review tests for behavioral confidence rather than implementation lock-in.
🤖 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/resource-limit-diagnostics.test.ts` around lines 79 - 115, Add an incomplete-frame test alongside the existing filterResourceLimitOutput protocol tests, passing input containing RESOURCE_LIMIT_CONNECT_BEGIN_MARKER without RESOURCE_LIMIT_CONNECT_END_MARKER. Assert the result contains "resource_limit_protocol_error=1\n", focusing on the externally observable behavior.Source: 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.
Nitpick comments:
In `@test/e2e/support/resource-limit-diagnostics.test.ts`:
- Around line 79-115: Add an incomplete-frame test alongside the existing
filterResourceLimitOutput protocol tests, passing input containing
RESOURCE_LIMIT_CONNECT_BEGIN_MARKER without RESOURCE_LIMIT_CONNECT_END_MARKER.
Assert the result contains "resource_limit_protocol_error=1\n", focusing on the
externally observable behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6fdea694-5920-48d4-92c1-de2fcb68bd9d
📒 Files selected for processing (3)
test/e2e/fixtures/resource-limit-diagnostics.tstest/e2e/live/sandbox-operations.test.tstest/e2e/support/resource-limit-diagnostics.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/live/sandbox-operations.test.ts
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical dated `v0.0.101` changelog entry that was missing when the release tag was cut. This post-release recovery records the shipped behavior on current `main` without changing or replacing the existing tag. ## Changes - Add `docs/changelog/2026-08-03.mdx` with the exact `## v0.0.101` heading, release summary, detailed behavior changes, support boundaries, and links to durable documentation. - [#7317](#7317) -> `docs/changelog/2026-08-03.mdx`: Records experimental OpenClaw Google Chat support and its restricted credential and webhook boundary. - [#7715](#7715) -> `docs/changelog/2026-08-03.mdx`: Records strict onboarding recovery state and authoritative resume identity. - [#7749](#7749) -> `docs/changelog/2026-08-03.mdx`: Records the provider-neutral policy seam and unchanged runtime support boundary. - [#7817](#7817) -> `docs/changelog/2026-08-03.mdx`: Records preserved Hermes home-channel assignments across rebuilds. - [#7820](#7820) -> `docs/changelog/2026-08-03.mdx`: Records the SSH-session status field correction. - [#7847](#7847) -> `docs/changelog/2026-08-03.mdx`: Records fail-closed credential filtering for migration and rebuild backups. - [#7870](#7870) -> `docs/changelog/2026-08-03.mdx`: Records sandbox-qualified in-sandbox host command hints. - [#7875](#7875) -> `docs/changelog/2026-08-03.mdx`: Records Microsoft Teams stop and start E2E coverage. - [#7885](#7885) -> `docs/changelog/2026-08-03.mdx`: Records Hermes managed gateway detection in status. - [#7889](#7889) -> `docs/changelog/2026-08-03.mdx`: Records policy-authenticated HTTPS Pin Runtime route revocation. - [#7891](#7891) -> `docs/changelog/2026-08-03.mdx`: Records default fallback for negative timeout and polling overrides. - [#7993](#7993) -> `docs/changelog/2026-08-03.mdx`: Records correct sibling detection during uninstall. - [#7995](#7995) -> `docs/changelog/2026-08-03.mdx`: Records absent configuration-hash handling before shields lock. - [#8001](#8001) -> `docs/changelog/2026-08-03.mdx`: Records the dormant atomic managed workload replacement foundation. - [#8029](#8029) -> `docs/changelog/2026-08-03.mdx`: Records repository terminology review in PR Review Advisor. - [#8031](#8031) -> `docs/changelog/2026-08-03.mdx`: Records provider-neutral managed snapshot authority. - [#8032](#8032) -> `docs/changelog/2026-08-03.mdx`: Records immutable managed clone handoff contracts. - [#8034](#8034) -> `docs/changelog/2026-08-03.mdx`: Records the dormant provider-owned clone transaction surface. - [#8035](#8035) -> `docs/changelog/2026-08-03.mdx`: Records the dormant Hermes managed clone broker boundary. - [#8036](#8036) -> `docs/changelog/2026-08-03.mdx`: Records the dormant transactional managed bootstrap boundary. - [#8037](#8037) -> `docs/changelog/2026-08-03.mdx`: Records dormant Docker bootstrap primitives and the unchanged provider support boundary. - [#8070](#8070) -> `docs/changelog/2026-08-03.mdx`: Records consolidated sandbox resource-limit E2E coverage. - [#8071](#8071) -> `docs/changelog/2026-08-03.mdx`: Records escaped and bounded CLI validation diagnostics. - [#8081](#8081) -> `docs/changelog/2026-08-03.mdx`: Records bounded linear snapshot Base64 validation. - [#8085](#8085) -> `docs/changelog/2026-08-03.mdx`: Records commit-bound workflow approval for eligible same-repository maintainers. - [#8088](#8088) -> `docs/changelog/2026-08-03.mdx`: Records Hermes managed-policy E2E selection. - [#8090](#8090) -> `docs/changelog/2026-08-03.mdx`: Records pinned CI search-tool provisioning. - [#8106](#8106) -> `docs/changelog/2026-08-03.mdx`: Records fallback from failed managed OpenShell gateway startup. - [#8107](#8107) -> `docs/changelog/2026-08-03.mdx`: Records Hermes adapter lifecycle E2E selection. - [#8128](#8128) -> `docs/changelog/2026-08-03.mdx`: Records the dormant transactional Docker bootstrap adapter and rollback authority. - [#8140](#8140) -> `docs/changelog/2026-08-03.mdx`: Records Slack conflict scope across independent OpenShell gateways. - [#8147](#8147) -> `docs/changelog/2026-08-03.mdx`: Records completion of durable v0.0.100 documentation audit follow-ups. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This documentation-only recovery does not change executable behavior. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Independently reviewed `docs/changelog/2026-08-03.mdx` at commit `0bebe1f568e3dc85cf410aac1dfb8f8830070b85`. Its blob is `82887920f9720eafd75db6b2271c35f7477edb9b`. The entry follows the writing guide, controlled terminology, changelog structure, MDX SPDX format, literal CLI-name rule, and root-absolute route requirements. It accurately records the `v0.0.100...v0.0.101` release range, Announcement #8162, accepted scope boundaries, and shipped security behavior. There are no code samples. Focused changelog tests and the documentation build pass for this commit. - Agent: Codex Desktop independent documentation writer <!-- docs-review-head-sha: 0bebe1f --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## Security Review - Result: `PASS` - Reviewed commit: `0bebe1f568e3dc85cf410aac1dfb8f8830070b85` - Base commit: `643a4ab8b5f583d8555192a37927268b26022c51` - Findings: None. - Secrets and credentials: `PASS`. No credential values or secret files are present. - Input validation and data sanitization: `PASS`. No executable input path changes. - Authentication and authorization: `PASS`. No identity or permission logic changes. - Dependencies and third-party libraries: `PASS`. No dependency changes. - Error handling and logging: `PASS`. No runtime path changes; diagnostic-security claims are precise. - Cryptography and data protection: `PASS`. No implementation changes. - Configuration and security controls: `PASS`. No configuration, container, port, or HTTP changes. - Security testing: `PASS`. No coverage is removed; the entry records shipped test and security behavior. - System security: `PASS`. No runtime control changes; dormant and non-activation boundaries are explicit. - Agent: Codex Desktop independent security reviewer ## Verification - [ ] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub — verification is pending after commit `0bebe1f568e3dc85cf410aac1dfb8f8830070b85` is pushed. - [ ] 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 — commit hooks passed; pre-push is pending. - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — tests are not applicable to this documentation-only recovery. - [x] Applicable broad gate passed — not applicable to this documentation-only recovery. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, credentials, or private keys are added by this diff. - [ ] `npm run docs` builds without warnings (doc changes only) — GitHub documentation checks are pending. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) — independent documentation review passed. - [x] New doc pages include SPDX header and frontmatter (new pages only) — the native changelog entry uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. GitHub CI is authoritative. Focused changelog tests and `npm run docs` passed after the merge refresh. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added experimental Google Chat support. * Improved runtime and session status visibility. * Added onboarding recovery and persistence safeguards. * Added snapshot validation and dormant managed-workload support. * **Bug Fixes** * Improved backup sanitization, route handling, and gateway reliability. * **Documentation** * Added the v0.0.101 changelog and related updates. * **Tests** * Expanded end-to-end coverage and strengthened trusted CI validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
Summary
This consolidates sandbox resource-limit validation into the broader sandbox-operations live E2E and replaces the destructive 5,000-process fork storm with a lightweight runtime probe. Fast support tests now protect resource-limit warning detection and a content-free framed artifact protocol, while the retired selector continues to route existing dispatches to the consolidated target.
Changes
nemoclaw connectresource-limit probe intosandbox-operations, covering login and interactive shell limits, denied raises, completion markers, and security-warning absence.sandbox-rlimits-connectas a selector alias because existing controllers and manual workflow dispatches can still submit it. A direct removal would reject or skip those raw dispatches; workflow-plan, workflow-boundary, and E2E workflow tests protect the route tosandbox-operations.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededDGX 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 unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm testwas attempted but stopped after unrelated host-dependent failures involving the local OpenShell gateway/TLS fixture, runner-label/platform fixture, SSH-agent subprocess cases, and DGX host detection. No changed test failed; all focused suites were rerun with the active SSH agent and passed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com