Repository navigation
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
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:
📝 WalkthroughWalkthroughRenames live E2E coverage to feature-scoped OpenClaw and Hermes channel names, adds shared lifecycle and pairing helpers, introduces conflict-guard coverage, and updates workflow dispatching, boundary validation, regression wiring, docs, and support tests to match the renamed suites and jobs. ChangesOpenClaw channels E2E refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6053.docs.buildwithfern.com/nemoclaw |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
Contributor
PR Review Advisor (Nemotron Ultra) — BlockedMerge posture: Do not merge until addressed Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Signed-off-by: San Dang <sdang@nvidia.com>
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28444218181
|
![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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/e2e/support/e2e-workflow.test.ts (1)
1207-1257: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestore the Telegram fake-token assertion in this drift case.
.github/workflows/e2e.yamlstill hardcodes a fakeTELEGRAM_BOT_TOKENforopenclaw-channels-stop-start, but this boundary test now only proves Discord and Teams token enforcement. IfvalidateE2eWorkflowBoundary()stops checking the Telegram token, this suite still passes and a real Telegram credential can slip into the job unnoticed. As per path instructions,**/*.test.{ts,js,mts,mjs,cts,cjs}: 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/e2e-workflow.test.ts` around lines 1207 - 1257, The boundary test for the openclaw-channels-stop-start workflow no longer verifies the fake Telegram credential, so add back an assertion that validateE2eWorkflowBoundary() reports the expected TELEGRAM_BOT_TOKEN check alongside the existing token validations. Update the expectations in e2e-workflow.test.ts around the openclaw-channels-stop-start case to ensure the job still rejects real Telegram credentials, using the existing runStep and validateE2eWorkflowBoundary symbols to locate the drift case.Source: Path instructions
.github/workflows/regression-e2e.yaml (1)
323-354: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep this regression lane scoped to the WhatsApp QR guard.
The job is documented as a hermetic WhatsApp compact-QR regression, but it now runs
test/e2e/live/openclaw-channels-pairing.test.ts, which this PR’s context describes as the consolidated Discord/Slack/WhatsApp pairing suite. That broadens a focused regression lane into a multi-channel live suite and can pull in unrelated failures or setup requirements. Point this job back to a dedicated WhatsApp QR entrypoint, or at least add a focused Vitest filter for the WhatsApp case.🤖 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 @.github/workflows/regression-e2e.yaml around lines 323 - 354, The regression job is using the consolidated openclaw-channels-pairing.test.ts suite instead of staying scoped to the WhatsApp QR guard. Update the openclaw-channels-pairing-whatsapp-qr-e2e workflow step to run only the WhatsApp pairing path, either by pointing the npx vitest run command at a dedicated WhatsApp QR test entrypoint or by adding a Vitest filter that targets the WhatsApp case in openclaw-channels-pairing.test.ts.
🧹 Nitpick comments (1)
test/e2e/live/openclaw-channels-pairing-discord.ts (1)
97-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid coupling this helper to the private sandbox config layout.
This block shells into
/sandbox/.openclaw/openclaw.jsonand asserts nestedchannels.discord.accounts.defaultkeys directly. That makes the pairing suite fail on internal config reshuffles even when the public Discord behavior is still correct. The provider lookup, fake-gateway proof, and pairing approval below already cover the observable contract.🤖 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/openclaw-channels-pairing-discord.ts` around lines 97 - 116, This Discord pairing test is too tightly coupled to the internal sandbox config structure by reading /sandbox/.openclaw/openclaw.json and asserting nested channels.discord.accounts.default fields directly. Remove that helper block from openclaw-channels-pairing-discord.ts and rely on the existing observable checks in the pairing flow, such as the provider lookup, fake-gateway proof, and pairing approval assertions, so the test validates behavior instead of private config layout.
🤖 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/openclaw-channels-conflict-guard.test.ts`:
- Around line 126-136: The commandEnv() helper is pulling in unrelated messaging
credentials from process.env, which can make the
openclaw-channels-conflict-guard test depend on CI/local state instead of only
the Telegram duplicate being exercised. Update commandEnv() to build a minimal
child environment and explicitly remove or omit unrelated messaging variables
such as DISCORD_*, SLACK_*, and any other messaging creds that could affect this
single-channel conflict case, while keeping the Telegram values needed by the
test.
In `@test/e2e/support/e2e-workflow.test.ts`:
- Around line 601-630: The add/remove workflow coverage is split across selector
modes, so each free-standing job should be asserted through both `targets` and
`jobs` in `evaluateE2eWorkflowDispatchSelectors`. Update the contract tests in
`e2e-workflow.test.ts` to mirror the existing loop used for
`gateway-health-honest`, `concurrent-gateway-ports`, and
`openclaw-channels-conflict-guard`, and add both selector assertions for
`openclaw-channels-add-remove` and `hermes-channels-add-remove` to catch typos
in either branch.
In `@test/e2e/support/openclaw-channels-pairing-workflow-boundary.test.ts`:
- Around line 55-60: Update the boundary test in
openclaw-channels-pairing-workflow-boundary to cover the Slack bot token secret
drift as well: in the live step setup and matching assertions, mutate and verify
SLACK_BOT_TOKEN alongside DISCORD_BOT_TOKEN and SLACK_APP_TOKEN. Use the
existing pairingJob.steps lookup for "Run OpenClaw channels pairing live tests"
and the installOpenShell-related assertions so the test fails if SLACK_BOT_TOKEN
is no longer enforced.
In `@test/regression-e2e-workflow.test.ts`:
- Around line 59-63: The regression test is only asserting absence of the env
var in the generated shell command text, which can miss changes to the workflow
contract. Update test/regression-e2e-workflow.test.ts to inspect the parsed
workflow objects for the relevant job/step env maps, using the job and step
structures around the existing runText assertions. Keep the existing checks for
the e2e-live command and target test path, but replace the NEMOCLAW_RUN_LIVE_E2E
source-text assertion with a direct expectation on job.env or step.env so the
boundary is validated behaviorally.
In `@tools/e2e/workflow-boundary.mts`:
- Around line 4272-4289: The step validation in workflow-boundary should also
block unexpected GITHUB_TOKEN exposure for these job steps, since the current
checks in the step loop only cover NVIDIA_INFERENCE_API_KEY and Docker Hub
secrets. Update the same validation logic around asSteps/job.steps and the
per-step loop to call the existing secret-exposure guard for GITHUB_TOKEN on
every step except any explicitly allowed case, matching the neighboring
validators’ coverage so credential-rewrite and telegram-injection-safety jobs
cannot pass with `${{ github.token }}` in env or run content.
---
Outside diff comments:
In @.github/workflows/regression-e2e.yaml:
- Around line 323-354: The regression job is using the consolidated
openclaw-channels-pairing.test.ts suite instead of staying scoped to the
WhatsApp QR guard. Update the openclaw-channels-pairing-whatsapp-qr-e2e workflow
step to run only the WhatsApp pairing path, either by pointing the npx vitest
run command at a dedicated WhatsApp QR test entrypoint or by adding a Vitest
filter that targets the WhatsApp case in openclaw-channels-pairing.test.ts.
In `@test/e2e/support/e2e-workflow.test.ts`:
- Around line 1207-1257: The boundary test for the openclaw-channels-stop-start
workflow no longer verifies the fake Telegram credential, so add back an
assertion that validateE2eWorkflowBoundary() reports the expected
TELEGRAM_BOT_TOKEN check alongside the existing token validations. Update the
expectations in e2e-workflow.test.ts around the openclaw-channels-stop-start
case to ensure the job still rejects real Telegram credentials, using the
existing runStep and validateE2eWorkflowBoundary symbols to locate the drift
case.
---
Nitpick comments:
In `@test/e2e/live/openclaw-channels-pairing-discord.ts`:
- Around line 97-116: This Discord pairing test is too tightly coupled to the
internal sandbox config structure by reading /sandbox/.openclaw/openclaw.json
and asserting nested channels.discord.accounts.default fields directly. Remove
that helper block from openclaw-channels-pairing-discord.ts and rely on the
existing observable checks in the pairing flow, such as the provider lookup,
fake-gateway proof, and pairing approval assertions, so the test validates
behavior instead of private config layout.
🪄 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: 03b9ebae-9958-4cb1-872d-3529df518151
📒 Files selected for processing (42)
.github/workflows/brev-nightly-e2e.yaml.github/workflows/e2e-branch-validation.yaml.github/workflows/e2e.yaml.github/workflows/regression-e2e.yamldocs/manage-sandboxes/messaging-channels.mdxdocs/reference/troubleshooting.mdxsrc/lib/actions/sandbox/policy-explain.tstest/e2e-advisor-targets.test.tstest/e2e-release-gate-workflow.test.tstest/e2e/brev-e2e.test.tstest/e2e/live/channels-add-remove.test.tstest/e2e/live/channels-lifecycle-helpers.tstest/e2e/live/channels-stop-start-helpers.tstest/e2e/live/channels-stop-start-safety.tstest/e2e/live/channels-stop-start.test.tstest/e2e/live/hermes-channels-add-remove.test.tstest/e2e/live/hermes-channels-stop-start.test.tstest/e2e/live/hermes-discord.test.tstest/e2e/live/hermes-slack-e2e-helpers.tstest/e2e/live/hermes-slack-e2e.test.tstest/e2e/live/openclaw-channels-add-remove.test.tstest/e2e/live/openclaw-channels-conflict-guard.test.tstest/e2e/live/openclaw-channels-credential-rewrite-helpers.tstest/e2e/live/openclaw-channels-credential-rewrite.test.tstest/e2e/live/openclaw-channels-pairing-discord.tstest/e2e/live/openclaw-channels-pairing-slack.tstest/e2e/live/openclaw-channels-pairing-whatsapp-qr.tstest/e2e/live/openclaw-channels-pairing.test.tstest/e2e/live/openclaw-channels-stop-start.test.tstest/e2e/live/openclaw-channels-telegram-injection-safety.test.tstest/e2e/live/openclaw-channels-token-rotation.test.tstest/e2e/live/openclaw-discord-pairing.test.tstest/e2e/live/openclaw-pairing-helpers.tstest/e2e/live/openclaw-slack-pairing.test.tstest/e2e/live/phase6-messaging-helpers.tstest/e2e/live/rebuild-openclaw.test.tstest/e2e/support/e2e-workflow.test.tstest/e2e/support/openclaw-channels-pairing-workflow-boundary.test.tstest/e2e/support/openclaw-discord-workflow-boundary.test.tstest/regression-e2e-workflow.test.tstest/whatsapp-qr-compact.test.tstools/e2e/workflow-boundary.mts
💤 Files with no reviewable changes (10)
- test/e2e/live/openclaw-slack-pairing.test.ts
- test/e2e/live/openclaw-discord-pairing.test.ts
- test/e2e/support/openclaw-discord-workflow-boundary.test.ts
- test/e2e/live/channels-stop-start.test.ts
- test/e2e/live/hermes-slack-e2e.test.ts
- test/e2e/live/channels-stop-start-safety.ts
- test/e2e/live/hermes-slack-e2e-helpers.ts
- test/e2e/live/channels-add-remove.test.ts
- test/e2e/live/hermes-discord.test.ts
- test/e2e/live/channels-stop-start-helpers.ts
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28445695305
|
Signed-off-by: San Dang <sdang@nvidia.com>
Contributor
Vitest E2E Target Results —
|
| Job | Result |
|---|---|
| hermes-channels-add-remove | |
| openclaw-channels-add-remove | ✅ success |
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
sandl99
added
area: messaging
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28455933667
|
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28459843100
|
prekshivyas
approved these changes
Jun 30, 2026
prekshivyas
left a comment
prekshivyas
left a comment
Collaborator
There was a problem hiding this comment.
Pure rename/restructure — no assertions dropped, no if-statements in test bodies, add-remove jobs expand channel coverage. LGTM.
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28491284015
|
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28492704625
|
10 of 21 tasks
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28508508090
|
Contributor
Vitest E2E Target Results —
|
| Job | Result |
|---|---|
| hermes-channels-add-remove | |
| hermes-channels-stop-start | |
| network-policy | |
| openclaw-channels-add-remove | |
| openclaw-channels-conflict-guard | ✅ success |
| openclaw-channels-credential-rewrite | |
| openclaw-channels-pairing | |
| openclaw-channels-stop-start | |
| openclaw-channels-telegram-injection-safety | |
| openclaw-channels-token-rotation |
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28510840868
|
Contributor
Vitest E2E Target Results —
|
| Job | Result |
|---|---|
| hermes-channels-add-remove | |
| hermes-channels-stop-start | |
| network-policy | |
| openclaw-channels-add-remove | |
| openclaw-channels-conflict-guard | ✅ success |
| openclaw-channels-credential-rewrite | |
| openclaw-channels-pairing | |
| openclaw-channels-stop-start | |
| openclaw-channels-telegram-injection-safety | ✅ success |
| openclaw-channels-token-rotation |
Contributor
Vitest E2E Target Results —
|
| Job | Result |
|---|---|
| hermes-channels-add-remove | |
| hermes-channels-stop-start | |
| network-policy | |
| openclaw-channels-add-remove | |
| openclaw-channels-conflict-guard | ✅ success |
| openclaw-channels-credential-rewrite | |
| openclaw-channels-pairing | |
| openclaw-channels-stop-start | |
| openclaw-channels-telegram-injection-safety | |
| openclaw-channels-token-rotation |
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28514977239
|
This was referenced Jul 1, 2026
ericksoa
added a commit
that referenced
this pull request
Jul 2, 2026
## Summary Routes ordinary native CDI Linux through OpenShell 0.0.71's native `--gpu` path instead of the legacy Docker container swap. The legacy path remains available for WSL, Jetson, and explicit `NEMOCLAW_DOCKER_GPU_PATCH=1`, with its OpenShell supervisor command boundary and rollback diagnostics hardened. ## Related Issue Related to #6110 ## Changes - Use native OpenShell GPU injection by default on ordinary native CDI Linux. - Document the `NEMOCLAW_DOCKER_GPU_PATCH` auto, forced-legacy, and native-routing behavior for ordinary native Linux, Docker Desktop WSL, and Jetson/Tegra. - Keep `NEMOCLAW_DOCKER_GPU_PATCH=1` as the explicit legacy-swap force control and `=0` as the existing native opt-out; Docker Desktop WSL still ignores `=0`, and Jetson keeps its compatibility default. - Preserve OpenShell's supervisor entrypoint on the legacy swap: Docker receives no command tail, while the workload stays in `OPENSHELL_SANDBOX_COMMAND`. - Validate legacy startup tokens before stopping or renaming the original container and serialize extra-placeholder keys as one comma-delimited token. - Defer every legacy recreate through the same supervisor-wait/finalize boundary so failed clones are captured before rollback on both create timing paths. - Persist only allowlisted/redacted failed-clone topology, state, process, network, and log evidence, with a 10-second total / 2-second per-call budget so diagnostics cannot materially delay rollback. - Permit only the canonical OpenShell Docker/Podman TLS key path in the Hermes runtime environment; arbitrary values and persisted `.env` entries remain rejected. - Refresh the Dockerfile integrity pin for the changed validator so production Hermes images fail closed on any later digest drift. - Prove native and legacy Docker command boundaries separately, including Ready/CUDA status, supervisor PID 1, placeholder transport, config hashes, no backup-container leak, and inference requests. ## 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] Docs updated for user-facing behavior changes - [ ] Docs 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: independent exact-diff review found no remaining substantive issue after startup-envelope secret redaction, bounded pre-rollback capture, unified finalize ordering, and route-specific live assertions - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [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) - [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) Local exact-candidate verification at `d76f1647a5f354ba04737a6a049b82bfbf6d5454`: - CLI build and typecheck passed. - Hermes GPU support/client/workflow coverage passed 48/48; the built gateway-cleanup module resolves through Node, and runtime cleanup, registration removal, and bind availability remain fail-closed. - The shared Docker GPU diagnostic collector owns redaction for every text/JSON artifact and returned summary; direct conventional `*_KEY` and custom-placeholder canaries, JSON-validity, inspect-before-write, exhausted-budget, and collector-owned top regressions pass. - All 12 Docker GPU suites pass 126/126; the exact-head focused E2E-support set passes 39/39, including six process-token self-match regressions, the scrubbed integrity-proof environment, and total forbidden-marker count. - Conditional scan, source-shape, test-size, Biome, repository checks, commit hooks, and push typecheck passed for the final harness correction; only the documented macOS-invalid full CLI hook lane was excluded. - The forbidden-marker request sensor support suite passed 14/14 and records counts only, never raw request bodies or marker values. - OpenShell transport-boundary coverage passed 4/4; Docker GPU command-envelope coverage passed 6/6; extra-placeholder parsing coverage passed 16/16. - Hermes validator and wrapper integrity pins match their source SHA256 digests; hadolint and diff checks pass. - Hermes startup/boundary coverage passed 43/43 locally; the Linux-only wrapper cases are delegated to exact-head CI. - The full local CLI hook is not a valid gate on this macOS Node 22 host: unchanged `a1fc52c7` TypeScript entrypoints fail through the CommonJS preload with `ERR_UNKNOWN_FILE_EXTENSION`; exact-head Linux CI remains required. - Hermes runtime-guard plus current-main docs regression tests: 24/24 passed. - Hermes workflow-boundary test passed. - Project-boundary, project-membership, test-title, source-shape, test-size, targeted Biome, and diff checks passed. - `npm run docs:sync-agent-variants`, `npm run docs:check-agent-variants`, and `npm run docs` pass; Fern reports 0 errors and 2 existing warnings. Live A/B evidence: - Forced legacy swap at `97e3e7e1`: [run 28554699811](https://github.com/NVIDIA/NemoClaw/actions/runs/28554699811) reproduced sustained OpenShell `Error` followed by safe rollback. It also exposed and now closes a lifecycle instrumentation gap: the post-create `ensureApplied()` branch bypassed pre-rollback capture. - Native OpenShell route at signed diagnostic SHA `15f50182`: [run 28555110558](https://github.com/NVIDIA/NemoClaw/actions/runs/28555110558) completed onboarding with exit 0, reached `Phase: Ready`, reported `CUDA verified`, and sent authenticated Hermes chat-completions requests to the hermetic inference endpoint. The job's only failure was the now-fixed test regex not stripping ANSI around `Ready`. - Prior native evidence at `7cb219d9`: [full run 28559814959](https://github.com/NVIDIA/NemoClaw/actions/runs/28559814959) and [second pass 28559816026](https://github.com/NVIDIA/NemoClaw/actions/runs/28559816026) both reached Ready/CUDA with clean runtime and teardown; their Hermes proof stopped on the now-fixed ANSI matcher before downstream assertions. - Prior forced-legacy diagnostic on production parent `a1fc52c7` plus workflow-only child `6d0cf6a5`: [run 28561607207](https://github.com/NVIDIA/NemoClaw/actions/runs/28561607207) selected the legacy swap and rolled back cleanly, but failed because the Hermes boundary rejected the driver-owned OpenShell `OPENSHELL_TLS_KEY` path. Candidate `54cf259d` adds an exact runtime-only allowance with negative boundary tests. - Prior exact-head set at `a1fc52c7`: [run 28561548749](https://github.com/NVIDIA/NemoClaw/actions/runs/28561548749) proved the GPU and security companion jobs, while Hermes GPU stopped at a sandbox-user `/proc` permission probe after Ready/CUDA. Candidate `54cf259d` keeps the same proof but runs it as root and restricts the match to the exact `nemoclaw-start` process. - Prior second native pass at `a1fc52c7`: [run 28561555945](https://github.com/NVIDIA/NemoClaw/actions/runs/28561555945) reproduced only the same harness permission failure after Ready/CUDA, correct PID 1 topology, authenticated inference, zero forbidden-marker matches, and clean teardown. - Superseded six-job set at `54cf259d`: [run 28564960504](https://github.com/NVIDIA/NemoClaw/actions/runs/28564960504) exposed the stale Dockerfile validator digest and was canceled before runtime proof. Candidate `970803a4` updates the integrity pin, retained by final head `c5a67c4c`. - Superseded second native pass at `54cf259d`: [run 28564973806](https://github.com/NVIDIA/NemoClaw/actions/runs/28564973806) was canceled during pre-cleanup and supplies no acceptance evidence. - Superseded forced-legacy proof on production parent `54cf259d` plus child `69f4e1b2`: [run 28564983760](https://github.com/NVIDIA/NemoClaw/actions/runs/28564983760) was canceled during pre-cleanup and supplies no acceptance evidence. - Superseded six-job set at `970803a4`: [run 28565197328](https://github.com/NVIDIA/NemoClaw/actions/runs/28565197328) was canceled before acceptance execution when the canonical placeholder-format advisor fix advanced the head. - Superseded second native pass at `970803a4`: [run 28565207911](https://github.com/NVIDIA/NemoClaw/actions/runs/28565207911) was canceled before runner assignment and supplies no acceptance evidence. - Superseded forced-legacy proof on production parent `970803a4` plus child `b4d5679e`: [run 28565222881](https://github.com/NVIDIA/NemoClaw/actions/runs/28565222881) was canceled before runner assignment and supplies no acceptance evidence. - Superseded six-job set at `7335903b`: [run 28565576066](https://github.com/NVIDIA/NemoClaw/actions/runs/28565576066) was intentionally canceled when the documentation gap advanced the candidate; the root-entrypoint smoke passed, but the remaining lanes provide no complete acceptance proof. - Superseded second native pass at `7335903b`: [run 28565587094](https://github.com/NVIDIA/NemoClaw/actions/runs/28565587094) was canceled before acceptance execution and supplies no acceptance evidence. - Superseded forced-legacy proof on production parent `7335903b` plus child `091e16fd`: [run 28565603460](https://github.com/NVIDIA/NemoClaw/actions/runs/28565603460) was canceled before acceptance execution and supplies no acceptance evidence. - Superseded six-job set at `c5a67c4c`: [run 28566083673](https://github.com/NVIDIA/NemoClaw/actions/runs/28566083673) reached the native GPU/runtime proofs before the obsolete raw strict-hash assertion failed; messaging independently hit the process-probe self-match fixed by merged #6167. GPU, root-entrypoint, secret-boundary, and credential companion lanes passed. - Superseded second native pass at `c5a67c4c`: [run 28566083641](https://github.com/NVIDIA/NemoClaw/actions/runs/28566083641) proved native routing, Ready/CUDA, `nvidia-smi`, `/proc`, `cuInit(0)=0`, PID 1, authenticated inference, and cleanup, then failed only the obsolete raw strict-hash assertion. - Superseded forced-legacy proof on production parent `c5a67c4c` plus child `48c46a7f`: [run 28566083589](https://github.com/NVIDIA/NemoClaw/actions/runs/28566083589) proved the same runtime boundary on the legacy route, then failed only the obsolete raw strict-hash assertion. - Superseded six-job set at `a04a70ac`: [run 28568069499](https://github.com/NVIDIA/NemoClaw/actions/runs/28568069499) exposed a pre-onboarding harness defect: direct Vitest import of the production cleanup helper could not resolve its lazy CommonJS TypeScript dependencies. Companion results do not count as final-head evidence. - Superseded second native pass at `a04a70ac`: [run 28568069558](https://github.com/NVIDIA/NemoClaw/actions/runs/28568069558) failed at the same pre-onboarding cleanup boundary and supplies no runtime acceptance evidence. - Superseded forced-legacy proof on production parent `a04a70ac` plus child `085a3b7d`: [run 28568069530](https://github.com/NVIDIA/NemoClaw/actions/runs/28568069530) failed at the same pre-onboarding cleanup boundary and supplies no runtime acceptance evidence. - Superseded six-job set at `6ac4ebc8`: [run 28568490864](https://github.com/NVIDIA/NemoClaw/actions/runs/28568490864) exposed a clean-runner preinstall edge: the compiled cleanup child was invoked before OpenShell existed and failed before onboarding. Companion results do not count as final-head evidence. - Superseded second native pass at `6ac4ebc8`: [run 28568494928](https://github.com/NVIDIA/NemoClaw/actions/runs/28568494928) failed at the same pre-onboarding cleanup boundary and supplies no runtime acceptance evidence. - Superseded forced-legacy proof on production parent `6ac4ebc8` plus child `5d3742a7`: [run 28568501430](https://github.com/NVIDIA/NemoClaw/actions/runs/28568501430) failed at the same pre-onboarding cleanup boundary and supplies no runtime acceptance evidence. - Superseded six-job set at `65b06d64`: [run 28568954862](https://github.com/NVIDIA/NemoClaw/actions/runs/28568954862) passed all six jobs, but the candidate advanced to close the advisor-confirmed shared diagnostic-redaction boundary and two proof-hardening review threads. - Superseded second native pass at `65b06d64`: [run 28568959028](https://github.com/NVIDIA/NemoClaw/actions/runs/28568959028) passed the full native runtime proof but is not final-head evidence. - Superseded forced-legacy proof on production parent `65b06d64` plus child `2c6dca1b`: [run 28568966557](https://github.com/NVIDIA/NemoClaw/actions/runs/28568966557) passed but is not final-parent evidence. - Final six-job exact-head set at `d76f1647`: [run 28601346031](https://github.com/NVIDIA/NemoClaw/actions/runs/28601346031) passed all six requested jobs. Native GPU, Hermes startup, root-entrypoint, secret-boundary, credential-sanitization, and messaging proofs are green; all 21 messaging raw-token surface probes are `ABSENT`, and every cleanup record has zero failures. - Final second native Hermes GPU pass at `d76f1647`: [run 28601348023](https://github.com/NVIDIA/NemoClaw/actions/runs/28601348023) passed 9/9 assertions. Artifact `8043702467` (`sha256:0ef929fa2478f5c9579ad2f282c46de8fe4d6eefb04acb062de1af29b4eb002c`) proves native routing, Ready/CUDA, `nvidia-smi`, `/proc`, successful `cuInit(0)`, OpenShell PID 1, one container/no backup, authenticated inference with zero forbidden-marker matches, and clean teardown. - Final failed-clone rollback proof checks out exact production SHA `d76f1647` from signed workflow-only child `4c49b5bc`: [run 28602166456](https://github.com/NVIDIA/NemoClaw/actions/runs/28602166456) passed. Artifact `8044114375` (`sha256:b5cff9cc7e7cf361f55e074a265c8cfefd3e6dc21ad0d300b140d2a749cde00b`) records clone exit 137 with `failure_kind=patched_container_failed` and `rolled_back=no` before finalize, then `rolled_back=yes`, exactly one running original container, no backup leak, guard-observed clone removal, clean canary scans, and clean fixture teardown. - Final forced-legacy success proof on exact production parent `d76f1647` plus signed workflow-only child `078a372d`: [run 28603335692](https://github.com/NVIDIA/NemoClaw/actions/runs/28603335692) passed 9/9 assertions. Artifact `8044550700` (`sha256:56d97c5caa8536ebff735ff600399ac7fafa2fed71206de1f5470e7d15549f6f`) proves `gpuRoute=legacy-patch`, `--device nvidia.com/gpu=all`, Ready/CUDA with all three GPU probes, correct OpenShell PID 1/command envelope, one container/no backup, integrity and negative guard checks, two authenticated inference requests with zero forbidden-marker matches, a clean artifact canary scan, and clean teardown. Source-of-truth review for the retained compatibility path: - **Invalid state:** the legacy swap temporarily leaves a stopped backup and running clone with the same OpenShell sandbox ID. - **Source boundary:** OpenShell's Docker driver reconciles container summaries into a map keyed only by sandbox ID; 0.0.71 can let the stopped backup overwrite the running clone and drive the gateway into terminal `Error`. - **Source-fix constraint:** the NemoClaw-supported OpenShell release does not contain deterministic active-container selection. The focused source fix is open as [NVIDIA/OpenShell#2116](NVIDIA/OpenShell#2116) and passes 96/96 Docker-driver tests, strict clippy, and formatting, but is not yet released or pinned here. - **Regression coverage:** routing tests pin native auto / forced legacy / WSL / Jetson behavior; recreate tests pin capture-before-finalize on both create timing paths; the secret canary uses the actual single `OPENSHELL_SANDBOX_COMMAND=env ...` envelope; the live test pins native and legacy runtime topology separately. - **Removal condition:** remove the legacy swap and its rollback/diagnostic modules after WSL and Jetson are proven on native OpenShell GPU injection and the supported OpenShell floor contains deterministic duplicate-container reconciliation. - **WSL boundary:** Docker Desktop WSL does not expose a usable native CDI route to this flow, so WSL retains the compatibility path and ignores `NEMOCLAW_DOCKER_GPU_PATCH=0`; routing tests lock that behavior. Remove it when Docker Desktop exposes usable `nvidia.com/gpu` CDI devices to the WSL distro. - **Jetson boundary:** Tegra `/dev/nvmap` and `/dev/nvhost-*` device ownership requires host group propagation for the non-root sandbox user; group-add tests lock that behavior. Remove it only when the platform/runtime supplies equivalent access without the compatibility recreate. Diagnostic redaction boundary: - **Source-of-truth invariant:** `collectDockerGpuPatchDiagnostics()` constructs the trusted per-bundle redactor, discovers conventional and custom-placeholder values from every known/discovered full inspect before writing, recursively redacts JSON values, and publishes every summary, Docker, OpenShell, and pre-rollback top artifact through that boundary. - **Bounded pre-rollback path:** the caller contributes only additive values discovered from the failed clone before snapshot capture so the shared 10-second budget cannot hide opaque values; the collector still performs its own discovery and owns every write. Direct raw-caller and exhausted-budget regressions scan all artifacts and returned summaries. - **Removal condition:** remove the additive pre-rollback discovery only when the shared collector can own snapshot capture inside the same budget without delaying rollback. Advisor architecture and follow-up rationale: - `docker-gpu-patch.ts` grows 58 lines to keep token validation before container mutation and bounded failed-clone capture before rollback. Splitting this security-critical ordering during the release-blocker fix would add cross-module state transfer; extract it when the legacy swap is retired after WSL and Jetson native proof. - `docker-gpu-local-inference.test.ts` grows 32 lines so bridge-probe routing assertions stay beside the behavior under test. Extract a bridge-probe module and focused test file if that surface grows again. - `docker-gpu-local-inference.ts` grows 15 lines to keep the bridge-probe/host-network decision beside its caller-facing contract; extract it with the tests if that surface grows again. - `docker-gpu-patch.test.ts` grows 13 lines and remains below 1,350 lines; split the mode-routing cases on the next growth. - The live fixture now supplies the canonical comma-delimited placeholder transport. Whitespace compatibility remains covered by parser unit tests and the messaging-provider scenario; the live proof intentionally matches the exact canonical startup environ token. - Dedicated `OPENSHELL_TLS_KEY` tests prove exact runtime acceptance, arbitrary/PEM/relative/near-miss rejection, persisted `.env` rejection, and continued rejection of supervisor identity tokens. The exact allowed path is sourced from `NVIDIA/OpenShell@v0.0.71` (`a242f84bb367d6df7d4d133e95a93857406c67f7`), where `driver_utils.rs::TLS_KEY_MOUNT_PATH` defines `/etc/openshell/tls/client/tls.key` and the Docker/Podman drivers inject it. This PR does not claim #6110 resolved until the reporter-class DGX Spark aarch64 or DGX Station GB300 NVIDIA Endpoints path passes. No such runner is declared in this repository, and organization runner inventory is not visible with the current permissions. Missing reporter hardware is an external acceptance blocker, not a passing result. The #6155 docs regression fix and current `main` through `9fe45362` are integrated. A refreshed pairwise merge-tree audit at `d76f1647` is clean with #5595 and #6153. #5876 directly conflicts in `e2e.yaml` and related Hermes/docs/workflow-boundary files; its resolution must union `hermes-gpu-startup` and `mcp-bridge-dev` selectors/result summaries and recompute uploader validation. #6020 already conflicts with current `main` and also overlaps #6142 outside `e2e.yaml`; #6053 is mergeable with `main` but conflicts pairwise in the uploader boundary. Those later branches must preserve #6142's explicit-only inventory and artifact contracts during retargeting; #6142 itself remains mergeable/CLEAN. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Strengthened Hermes GPU startup proof and managed startup integrity assertions, plus added a skipped-by-default GPU live E2E run for startup readiness. * **Bug Fixes** * Tightened PID 1 identity validation to block runtime mutation under a foreign PID 1. * Improved Docker GPU/OpenShell sandbox command and placeholder handling; enhanced GPU failure diagnostics with safer redaction. * **Documentation** * Refined GPU passthrough and `NEMOCLAW_DOCKER_GPU_PATCH` guidance across native Linux, Docker Desktop WSL, and Jetson/Tegra. * **Tests** * Expanded unit/E2E coverage for placeholder parsing, readiness refusal, env/secret boundary enforcement, and diagnostic redaction. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Routes ordinary native CDI Linux through OpenShell 0.0.71's native `--gpu` path instead of the legacy Docker container swap. The legacy path remains available for WSL, Jetson, and explicit `NEMOCLAW_DOCKER_GPU_PATCH=1`, with its OpenShell supervisor command boundary and rollback diagnostics hardened. ## Related Issue Related to NVIDIA#6110 ## Changes - Use native OpenShell GPU injection by default on ordinary native CDI Linux. - Document the `NEMOCLAW_DOCKER_GPU_PATCH` auto, forced-legacy, and native-routing behavior for ordinary native Linux, Docker Desktop WSL, and Jetson/Tegra. - Keep `NEMOCLAW_DOCKER_GPU_PATCH=1` as the explicit legacy-swap force control and `=0` as the existing native opt-out; Docker Desktop WSL still ignores `=0`, and Jetson keeps its compatibility default. - Preserve OpenShell's supervisor entrypoint on the legacy swap: Docker receives no command tail, while the workload stays in `OPENSHELL_SANDBOX_COMMAND`. - Validate legacy startup tokens before stopping or renaming the original container and serialize extra-placeholder keys as one comma-delimited token. - Defer every legacy recreate through the same supervisor-wait/finalize boundary so failed clones are captured before rollback on both create timing paths. - Persist only allowlisted/redacted failed-clone topology, state, process, network, and log evidence, with a 10-second total / 2-second per-call budget so diagnostics cannot materially delay rollback. - Permit only the canonical OpenShell Docker/Podman TLS key path in the Hermes runtime environment; arbitrary values and persisted `.env` entries remain rejected. - Refresh the Dockerfile integrity pin for the changed validator so production Hermes images fail closed on any later digest drift. - Prove native and legacy Docker command boundaries separately, including Ready/CUDA status, supervisor PID 1, placeholder transport, config hashes, no backup-container leak, and inference requests. ## 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] Docs updated for user-facing behavior changes - [ ] Docs 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: independent exact-diff review found no remaining substantive issue after startup-envelope secret redaction, bounded pre-rollback capture, unified finalize ordering, and route-specific live assertions - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [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) - [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) Local exact-candidate verification at `d76f1647a5f354ba04737a6a049b82bfbf6d5454`: - CLI build and typecheck passed. - Hermes GPU support/client/workflow coverage passed 48/48; the built gateway-cleanup module resolves through Node, and runtime cleanup, registration removal, and bind availability remain fail-closed. - The shared Docker GPU diagnostic collector owns redaction for every text/JSON artifact and returned summary; direct conventional `*_KEY` and custom-placeholder canaries, JSON-validity, inspect-before-write, exhausted-budget, and collector-owned top regressions pass. - All 12 Docker GPU suites pass 126/126; the exact-head focused E2E-support set passes 39/39, including six process-token self-match regressions, the scrubbed integrity-proof environment, and total forbidden-marker count. - Conditional scan, source-shape, test-size, Biome, repository checks, commit hooks, and push typecheck passed for the final harness correction; only the documented macOS-invalid full CLI hook lane was excluded. - The forbidden-marker request sensor support suite passed 14/14 and records counts only, never raw request bodies or marker values. - OpenShell transport-boundary coverage passed 4/4; Docker GPU command-envelope coverage passed 6/6; extra-placeholder parsing coverage passed 16/16. - Hermes validator and wrapper integrity pins match their source SHA256 digests; hadolint and diff checks pass. - Hermes startup/boundary coverage passed 43/43 locally; the Linux-only wrapper cases are delegated to exact-head CI. - The full local CLI hook is not a valid gate on this macOS Node 22 host: unchanged `a1fc52c7` TypeScript entrypoints fail through the CommonJS preload with `ERR_UNKNOWN_FILE_EXTENSION`; exact-head Linux CI remains required. - Hermes runtime-guard plus current-main docs regression tests: 24/24 passed. - Hermes workflow-boundary test passed. - Project-boundary, project-membership, test-title, source-shape, test-size, targeted Biome, and diff checks passed. - `npm run docs:sync-agent-variants`, `npm run docs:check-agent-variants`, and `npm run docs` pass; Fern reports 0 errors and 2 existing warnings. Live A/B evidence: - Forced legacy swap at `97e3e7e1`: [run 28554699811](https://github.com/NVIDIA/NemoClaw/actions/runs/28554699811) reproduced sustained OpenShell `Error` followed by safe rollback. It also exposed and now closes a lifecycle instrumentation gap: the post-create `ensureApplied()` branch bypassed pre-rollback capture. - Native OpenShell route at signed diagnostic SHA `15f50182`: [run 28555110558](https://github.com/NVIDIA/NemoClaw/actions/runs/28555110558) completed onboarding with exit 0, reached `Phase: Ready`, reported `CUDA verified`, and sent authenticated Hermes chat-completions requests to the hermetic inference endpoint. The job's only failure was the now-fixed test regex not stripping ANSI around `Ready`. - Prior native evidence at `7cb219d9`: [full run 28559814959](https://github.com/NVIDIA/NemoClaw/actions/runs/28559814959) and [second pass 28559816026](https://github.com/NVIDIA/NemoClaw/actions/runs/28559816026) both reached Ready/CUDA with clean runtime and teardown; their Hermes proof stopped on the now-fixed ANSI matcher before downstream assertions. - Prior forced-legacy diagnostic on production parent `a1fc52c7` plus workflow-only child `6d0cf6a5`: [run 28561607207](https://github.com/NVIDIA/NemoClaw/actions/runs/28561607207) selected the legacy swap and rolled back cleanly, but failed because the Hermes boundary rejected the driver-owned OpenShell `OPENSHELL_TLS_KEY` path. Candidate `54cf259d` adds an exact runtime-only allowance with negative boundary tests. - Prior exact-head set at `a1fc52c7`: [run 28561548749](https://github.com/NVIDIA/NemoClaw/actions/runs/28561548749) proved the GPU and security companion jobs, while Hermes GPU stopped at a sandbox-user `/proc` permission probe after Ready/CUDA. Candidate `54cf259d` keeps the same proof but runs it as root and restricts the match to the exact `nemoclaw-start` process. - Prior second native pass at `a1fc52c7`: [run 28561555945](https://github.com/NVIDIA/NemoClaw/actions/runs/28561555945) reproduced only the same harness permission failure after Ready/CUDA, correct PID 1 topology, authenticated inference, zero forbidden-marker matches, and clean teardown. - Superseded six-job set at `54cf259d`: [run 28564960504](https://github.com/NVIDIA/NemoClaw/actions/runs/28564960504) exposed the stale Dockerfile validator digest and was canceled before runtime proof. Candidate `970803a4` updates the integrity pin, retained by final head `c5a67c4c`. - Superseded second native pass at `54cf259d`: [run 28564973806](https://github.com/NVIDIA/NemoClaw/actions/runs/28564973806) was canceled during pre-cleanup and supplies no acceptance evidence. - Superseded forced-legacy proof on production parent `54cf259d` plus child `69f4e1b2`: [run 28564983760](https://github.com/NVIDIA/NemoClaw/actions/runs/28564983760) was canceled during pre-cleanup and supplies no acceptance evidence. - Superseded six-job set at `970803a4`: [run 28565197328](https://github.com/NVIDIA/NemoClaw/actions/runs/28565197328) was canceled before acceptance execution when the canonical placeholder-format advisor fix advanced the head. - Superseded second native pass at `970803a4`: [run 28565207911](https://github.com/NVIDIA/NemoClaw/actions/runs/28565207911) was canceled before runner assignment and supplies no acceptance evidence. - Superseded forced-legacy proof on production parent `970803a4` plus child `b4d5679e`: [run 28565222881](https://github.com/NVIDIA/NemoClaw/actions/runs/28565222881) was canceled before runner assignment and supplies no acceptance evidence. - Superseded six-job set at `7335903b`: [run 28565576066](https://github.com/NVIDIA/NemoClaw/actions/runs/28565576066) was intentionally canceled when the documentation gap advanced the candidate; the root-entrypoint smoke passed, but the remaining lanes provide no complete acceptance proof. - Superseded second native pass at `7335903b`: [run 28565587094](https://github.com/NVIDIA/NemoClaw/actions/runs/28565587094) was canceled before acceptance execution and supplies no acceptance evidence. - Superseded forced-legacy proof on production parent `7335903b` plus child `091e16fd`: [run 28565603460](https://github.com/NVIDIA/NemoClaw/actions/runs/28565603460) was canceled before acceptance execution and supplies no acceptance evidence. - Superseded six-job set at `c5a67c4c`: [run 28566083673](https://github.com/NVIDIA/NemoClaw/actions/runs/28566083673) reached the native GPU/runtime proofs before the obsolete raw strict-hash assertion failed; messaging independently hit the process-probe self-match fixed by merged NVIDIA#6167. GPU, root-entrypoint, secret-boundary, and credential companion lanes passed. - Superseded second native pass at `c5a67c4c`: [run 28566083641](https://github.com/NVIDIA/NemoClaw/actions/runs/28566083641) proved native routing, Ready/CUDA, `nvidia-smi`, `/proc`, `cuInit(0)=0`, PID 1, authenticated inference, and cleanup, then failed only the obsolete raw strict-hash assertion. - Superseded forced-legacy proof on production parent `c5a67c4c` plus child `48c46a7f`: [run 28566083589](https://github.com/NVIDIA/NemoClaw/actions/runs/28566083589) proved the same runtime boundary on the legacy route, then failed only the obsolete raw strict-hash assertion. - Superseded six-job set at `a04a70ac`: [run 28568069499](https://github.com/NVIDIA/NemoClaw/actions/runs/28568069499) exposed a pre-onboarding harness defect: direct Vitest import of the production cleanup helper could not resolve its lazy CommonJS TypeScript dependencies. Companion results do not count as final-head evidence. - Superseded second native pass at `a04a70ac`: [run 28568069558](https://github.com/NVIDIA/NemoClaw/actions/runs/28568069558) failed at the same pre-onboarding cleanup boundary and supplies no runtime acceptance evidence. - Superseded forced-legacy proof on production parent `a04a70ac` plus child `085a3b7d`: [run 28568069530](https://github.com/NVIDIA/NemoClaw/actions/runs/28568069530) failed at the same pre-onboarding cleanup boundary and supplies no runtime acceptance evidence. - Superseded six-job set at `6ac4ebc8`: [run 28568490864](https://github.com/NVIDIA/NemoClaw/actions/runs/28568490864) exposed a clean-runner preinstall edge: the compiled cleanup child was invoked before OpenShell existed and failed before onboarding. Companion results do not count as final-head evidence. - Superseded second native pass at `6ac4ebc8`: [run 28568494928](https://github.com/NVIDIA/NemoClaw/actions/runs/28568494928) failed at the same pre-onboarding cleanup boundary and supplies no runtime acceptance evidence. - Superseded forced-legacy proof on production parent `6ac4ebc8` plus child `5d3742a7`: [run 28568501430](https://github.com/NVIDIA/NemoClaw/actions/runs/28568501430) failed at the same pre-onboarding cleanup boundary and supplies no runtime acceptance evidence. - Superseded six-job set at `65b06d64`: [run 28568954862](https://github.com/NVIDIA/NemoClaw/actions/runs/28568954862) passed all six jobs, but the candidate advanced to close the advisor-confirmed shared diagnostic-redaction boundary and two proof-hardening review threads. - Superseded second native pass at `65b06d64`: [run 28568959028](https://github.com/NVIDIA/NemoClaw/actions/runs/28568959028) passed the full native runtime proof but is not final-head evidence. - Superseded forced-legacy proof on production parent `65b06d64` plus child `2c6dca1b`: [run 28568966557](https://github.com/NVIDIA/NemoClaw/actions/runs/28568966557) passed but is not final-parent evidence. - Final six-job exact-head set at `d76f1647`: [run 28601346031](https://github.com/NVIDIA/NemoClaw/actions/runs/28601346031) passed all six requested jobs. Native GPU, Hermes startup, root-entrypoint, secret-boundary, credential-sanitization, and messaging proofs are green; all 21 messaging raw-token surface probes are `ABSENT`, and every cleanup record has zero failures. - Final second native Hermes GPU pass at `d76f1647`: [run 28601348023](https://github.com/NVIDIA/NemoClaw/actions/runs/28601348023) passed 9/9 assertions. Artifact `8043702467` (`sha256:0ef929fa2478f5c9579ad2f282c46de8fe4d6eefb04acb062de1af29b4eb002c`) proves native routing, Ready/CUDA, `nvidia-smi`, `/proc`, successful `cuInit(0)`, OpenShell PID 1, one container/no backup, authenticated inference with zero forbidden-marker matches, and clean teardown. - Final failed-clone rollback proof checks out exact production SHA `d76f1647` from signed workflow-only child `4c49b5bc`: [run 28602166456](https://github.com/NVIDIA/NemoClaw/actions/runs/28602166456) passed. Artifact `8044114375` (`sha256:b5cff9cc7e7cf361f55e074a265c8cfefd3e6dc21ad0d300b140d2a749cde00b`) records clone exit 137 with `failure_kind=patched_container_failed` and `rolled_back=no` before finalize, then `rolled_back=yes`, exactly one running original container, no backup leak, guard-observed clone removal, clean canary scans, and clean fixture teardown. - Final forced-legacy success proof on exact production parent `d76f1647` plus signed workflow-only child `078a372d`: [run 28603335692](https://github.com/NVIDIA/NemoClaw/actions/runs/28603335692) passed 9/9 assertions. Artifact `8044550700` (`sha256:56d97c5caa8536ebff735ff600399ac7fafa2fed71206de1f5470e7d15549f6f`) proves `gpuRoute=legacy-patch`, `--device nvidia.com/gpu=all`, Ready/CUDA with all three GPU probes, correct OpenShell PID 1/command envelope, one container/no backup, integrity and negative guard checks, two authenticated inference requests with zero forbidden-marker matches, a clean artifact canary scan, and clean teardown. Source-of-truth review for the retained compatibility path: - **Invalid state:** the legacy swap temporarily leaves a stopped backup and running clone with the same OpenShell sandbox ID. - **Source boundary:** OpenShell's Docker driver reconciles container summaries into a map keyed only by sandbox ID; 0.0.71 can let the stopped backup overwrite the running clone and drive the gateway into terminal `Error`. - **Source-fix constraint:** the NemoClaw-supported OpenShell release does not contain deterministic active-container selection. The focused source fix is open as [NVIDIA/OpenShell#2116](NVIDIA/OpenShell#2116) and passes 96/96 Docker-driver tests, strict clippy, and formatting, but is not yet released or pinned here. - **Regression coverage:** routing tests pin native auto / forced legacy / WSL / Jetson behavior; recreate tests pin capture-before-finalize on both create timing paths; the secret canary uses the actual single `OPENSHELL_SANDBOX_COMMAND=env ...` envelope; the live test pins native and legacy runtime topology separately. - **Removal condition:** remove the legacy swap and its rollback/diagnostic modules after WSL and Jetson are proven on native OpenShell GPU injection and the supported OpenShell floor contains deterministic duplicate-container reconciliation. - **WSL boundary:** Docker Desktop WSL does not expose a usable native CDI route to this flow, so WSL retains the compatibility path and ignores `NEMOCLAW_DOCKER_GPU_PATCH=0`; routing tests lock that behavior. Remove it when Docker Desktop exposes usable `nvidia.com/gpu` CDI devices to the WSL distro. - **Jetson boundary:** Tegra `/dev/nvmap` and `/dev/nvhost-*` device ownership requires host group propagation for the non-root sandbox user; group-add tests lock that behavior. Remove it only when the platform/runtime supplies equivalent access without the compatibility recreate. Diagnostic redaction boundary: - **Source-of-truth invariant:** `collectDockerGpuPatchDiagnostics()` constructs the trusted per-bundle redactor, discovers conventional and custom-placeholder values from every known/discovered full inspect before writing, recursively redacts JSON values, and publishes every summary, Docker, OpenShell, and pre-rollback top artifact through that boundary. - **Bounded pre-rollback path:** the caller contributes only additive values discovered from the failed clone before snapshot capture so the shared 10-second budget cannot hide opaque values; the collector still performs its own discovery and owns every write. Direct raw-caller and exhausted-budget regressions scan all artifacts and returned summaries. - **Removal condition:** remove the additive pre-rollback discovery only when the shared collector can own snapshot capture inside the same budget without delaying rollback. Advisor architecture and follow-up rationale: - `docker-gpu-patch.ts` grows 58 lines to keep token validation before container mutation and bounded failed-clone capture before rollback. Splitting this security-critical ordering during the release-blocker fix would add cross-module state transfer; extract it when the legacy swap is retired after WSL and Jetson native proof. - `docker-gpu-local-inference.test.ts` grows 32 lines so bridge-probe routing assertions stay beside the behavior under test. Extract a bridge-probe module and focused test file if that surface grows again. - `docker-gpu-local-inference.ts` grows 15 lines to keep the bridge-probe/host-network decision beside its caller-facing contract; extract it with the tests if that surface grows again. - `docker-gpu-patch.test.ts` grows 13 lines and remains below 1,350 lines; split the mode-routing cases on the next growth. - The live fixture now supplies the canonical comma-delimited placeholder transport. Whitespace compatibility remains covered by parser unit tests and the messaging-provider scenario; the live proof intentionally matches the exact canonical startup environ token. - Dedicated `OPENSHELL_TLS_KEY` tests prove exact runtime acceptance, arbitrary/PEM/relative/near-miss rejection, persisted `.env` rejection, and continued rejection of supervisor identity tokens. The exact allowed path is sourced from `NVIDIA/OpenShell@v0.0.71` (`a242f84bb367d6df7d4d133e95a93857406c67f7`), where `driver_utils.rs::TLS_KEY_MOUNT_PATH` defines `/etc/openshell/tls/client/tls.key` and the Docker/Podman drivers inject it. This PR does not claim NVIDIA#6110 resolved until the reporter-class DGX Spark aarch64 or DGX Station GB300 NVIDIA Endpoints path passes. No such runner is declared in this repository, and organization runner inventory is not visible with the current permissions. Missing reporter hardware is an external acceptance blocker, not a passing result. The NVIDIA#6155 docs regression fix and current `main` through `9fe45362` are integrated. A refreshed pairwise merge-tree audit at `d76f1647` is clean with NVIDIA#5595 and NVIDIA#6153. NVIDIA#5876 directly conflicts in `e2e.yaml` and related Hermes/docs/workflow-boundary files; its resolution must union `hermes-gpu-startup` and `mcp-bridge-dev` selectors/result summaries and recompute uploader validation. NVIDIA#6020 already conflicts with current `main` and also overlaps NVIDIA#6142 outside `e2e.yaml`; NVIDIA#6053 is mergeable with `main` but conflicts pairwise in the uploader boundary. Those later branches must preserve NVIDIA#6142's explicit-only inventory and artifact contracts during retargeting; NVIDIA#6142 itself remains mergeable/CLEAN. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Strengthened Hermes GPU startup proof and managed startup integrity assertions, plus added a skipped-by-default GPU live E2E run for startup readiness. * **Bug Fixes** * Tightened PID 1 identity validation to block runtime mutation under a foreign PID 1. * Improved Docker GPU/OpenShell sandbox command and placeholder handling; enhanced GPU failure diagnostics with safer redaction. * **Documentation** * Refined GPU passthrough and `NEMOCLAW_DOCKER_GPU_PATCH` guidance across native Linux, Docker Desktop WSL, and Jetson/Tegra. * **Tests** * Expanded unit/E2E coverage for placeholder parsing, readiness refusal, env/secret boundary enforcement, and diagnostic redaction. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Carlos Villela <cvillela@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
2 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
Refactors the live E2E messaging tests around channel-oriented suites so OpenClaw and Hermes lifecycle coverage share one manifest-aligned helper. Adds Microsoft Teams to the shared lifecycle matrix and adds a hermetic OpenClaw conflict guard target for duplicate channel credentials.
Related Issue
Fixes #6022
Changes
openclaw-channels-*andhermes-channels-*lifecycle suites backed bychannels-lifecycle-helpers.ts.openclaw-channels-conflict-guardto prove duplicate Telegram credentials abort without registry mutation or secret leakage.Type of Change
Quality Gates
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Targeted verification run:
npm run build:clinpm run typecheck:clinpx @biomejs/biome lint test/e2e/live/channels-lifecycle-helpers.ts test/e2e/live/openclaw-channels-conflict-guard.test.ts tools/e2e/workflow-boundary.mts test/e2e/support/e2e-workflow.test.tsenv NEMOCLAW_RUN_LIVE_E2E=1 npx vitest list --project e2e-live test/e2e/live/openclaw-channels-add-remove.test.ts test/e2e/live/hermes-channels-add-remove.test.ts test/e2e/live/openclaw-channels-stop-start.test.ts test/e2e/live/hermes-channels-stop-start.test.ts test/e2e/live/openclaw-channels-conflict-guard.test.tsenv NEMOCLAW_RUN_LIVE_E2E=1 npx vitest run --project e2e-live test/e2e/live/openclaw-channels-conflict-guard.test.ts --silent=false --reporter=defaultnpx vitest run --project e2e-support test/e2e/support/e2e-workflow.test.ts test/e2e/support/openclaw-channels-pairing-workflow-boundary.test.ts --silent=false --reporter=defaultnpx vitest run test/brev-nightly-workflow.test.ts test/e2e-advisor-targets.test.ts test/e2e-release-gate-workflow.test.ts test/regression-e2e-workflow.test.ts --silent=false --reporter=defaultnpm run test:projects:checknpm run test:titles:checknpm run test:imports:checknpm run test-size:checknpm run docspassed with 2 pre-existing Fern warningsgit diff --checkCommit pre-hook note: the initial commit hook was not used for the final commit after broad local
test-cliand unrelated integration checks failed in this environment; the targeted checks above passed.Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit