Skip to content

feat(mcp): add OpenShell-managed MCP servers#5876

Merged
apurvvkumaria merged 526 commits into
mainfrom
feat/mcp-bridge-vdr-566
Jul 3, 2026
Merged

feat(mcp): add OpenShell-managed MCP servers#5876
apurvvkumaria merged 526 commits into
mainfrom
feat/mcp-bridge-vdr-566

Conversation

@ericksoa

@ericksoa ericksoa commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds native OpenShell-managed authenticated HTTPS Streamable HTTP MCP server lifecycle for OpenClaw, Hermes, and LangChain Deep Agents Code. It exposes mcp add|list|status|restart|remove, integrates crash-safe rebuild and destroy recovery, and keeps raw credentials out of sandbox files and NemoClaw registry state. Stable OpenShell 0.0.72 is the shipping dependency for NemoClaw v0.0.74.

Related Issue

Closes #566.
Refs #6195.
Implements the accepted native OpenShell design decision.
Supersedes the closed host-proxy implementation in #565.

Changes

  • Support OpenClaw, Hermes, and LangChain Deep Agents Code with mcp add|list|status|restart|remove; normal sandbox rebuild and destroy participate in lifecycle reconciliation.
  • Accept only authenticated canonical HTTPS Streamable HTTP endpoints with exactly one host-side --env KEY credential reference per server.
  • Use native OpenShell protocol: mcp policy enforcement and provider-backed credential replacement. NemoClaw starts no MCP proxy, relay, listener, stdio adapter, or persistent secret-bearing host process.
  • Persist only credential names and opaque ownership metadata. Agent configuration contains Bearer openshell:resolve:env:KEY; raw values are supplied only to the allowlisted OpenShell provider subprocess.
  • Fail closed on private/local/special-use targets, unsupported host aliases, userinfo, query, fragment, percent characters, non-canonical paths, ambiguous provider/policy/adapter state, and ownership drift.
  • Serialize supported per-sandbox writers with one cross-process mutation lock and exact-check policy/provider state before and after mutation.
  • Preserve owned MCP intent across rebuild, recover interrupted add/remove/destroy transactions, and delete shared state only after exact ownership verification.
  • Add stable and explicit dev-channel live lanes, artifact credential scanning, workflow boundary tests, operator documentation, and focused security/lifecycle regression coverage.

Apurv review feedback addressed

  • Use the checked-in Hermes BASE_IMAGE pin: validation now reads the single immutable ARG BASE_IMAGE=...@sha256:... from the actual final Dockerfile, accepts only that official digest, and rejects a different digest. The obsolete NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST guard is removed.
  • Preserve falsey non-map YAML roots: both Hermes transaction paths normalize only None to {}. [], false, 0, and "" reach the object-root guard and fail without a write; tests prove byte preservation and zero writes.
  • Block rebuild while MCP destroy is incomplete: live rebuild and absent-sandbox onboarding reject both destroyPreparedAt and destroyPendingAt before policy, adapter, provider, gateway, pruning, or recreation work.
  • Reject every percent character and retain legacy cleanup: host and Hermes add validation reject %, %GG, %2, and valid percent escapes before side effects. Forced Hermes cleanup validates the action and server name, then removes the exact legacy key without re-rejecting its already-persisted URL.
  • Recheck exact policy content and serialize supported writers: the fresh policy read deep-compares the live key with the exact previously owned content and refuses changed or removed values before policy set; a post-set exact read gates ownership commit. Supported NemoClaw policy, channel, shields, inference, snapshot, onboarding, and config writers share the per-sandbox lock.
  • Keep credential revision proof host-side: the sandbox-writable revision snapshot is removed. Fresh OpenShell-mediated execs return only bounded absent, canonical, or vN observations; the host retains the prior value and requires a changed, non-absent revision after update.

The review fixes landed in signed commit 299efb5c1eaeec21f9b91d7d1ce9874b6768d8e8 and remain present on final exact head f9283fe624a479796ee1dcba35e5e95fde182ab2.

Release boundary and residual-risk disposition

  • Stable OpenShell 0.0.72 is the only supported and shipped OpenShell version for NemoClaw v0.0.74; installer, blueprint, workflow, and credential-boundary data are aligned to that version.
  • OpenShell 0.0.72 attributes some script clients to Node/Python interpreters rather than immutable package entrypoints. Exact destination, literal path, MCP method profile, request-size bound, public-IP pins, provider ownership, and runtime checks narrow that grant. Remove interpreter grants when OpenShell exposes stable package-entrypoint attribution.
  • Static provider credentials are sandbox-scoped rather than endpoint-exclusive. Every managed server requires a unique credential name and a least-privilege token; operators must not grant the same runtime a broader route capable of resolving that placeholder.
  • Supported NemoClaw writers are serialized, and external drift visible before or after mutation is rejected by exact reads. A direct external openshell policy set does not participate in NemoClaw's lock, and OpenShell 0.0.72 exposes no policy compare-and-swap/version precondition. If an external set lands in the narrow interval after the fresh read and loses the final write race, it can be overwritten. Closing that arbitrary-client race requires an upstream OpenShell CAS primitive; this PR does not claim atomicity beyond supported NemoClaw writers.
  • OpenShell policy does not bind URL scheme, HTTP Host, or query parameters. NemoClaw therefore accepts only canonical HTTPS URLs and rejects userinfo, query, fragment, percent characters, credential-shaped paths, and unsupported aliases.
  • Stable OpenShell cannot safely parse bracketed IPv6 literals in HTTPS CONNECT targets. Direct IPv6 literals are rejected; DNS AAAA destinations remain supported through resolve/validate/connect and exact public-address pins.
  • The explicit compatibility-only dev lane consumed mutable OpenShell 0.0.76-dev.3+g6461677c. OpenClaw and Deep Agents passed, but Hermes rebuild exited during pre-delete MCP preservation. The same exact NemoClaw head passed the identical Hermes rebuild with supported stable 0.0.72. Because the dev lane is non-default, unverified, outside the v0.0.74 support range, and not a required PR check, this PR does not add speculative compatibility code for the moving artifact; compatibility should be re-evaluated with OpenShell before NemoClaw widens its supported range.

Exact-head verification

  • PR head f9283fe624a479796ee1dcba35e5e95fde182ab2 is signed and GitHub-verified, labeled v0.0.74, approved by Apurv, and merged to main as bddbb3e2418766015b931829571202f443a788f1. Immediately before merge, main advanced by heading-only commit cc532e5; the overlap audit and merge-tree were clean.
  • The final restack integrates current-main DCode 0.1.30 runtime hardening while preserving managed MCP in interactive and headless paths. The complete sandbox-writable MCP file is validated fail-closed as HTTPS-only registry-owned shape before launch; stdio commands, extra headers, raw credentials, mismatched placeholders, unsafe ownership/mode, and unrelated top-level configuration are rejected.
  • Final local validation passed build, typecheck, all 44 config schemas, repository checks, commit/push hooks, 290/290 focused CLI tests, 192/192 MCP integration tests, 77/77 DCode image-contract tests, and the affected DCode direct-entrypoint, snapshot, rebuild, proxy, and legacy-lifecycle suites. The third-party-notice harness passes 14/14 DCode rebuild tests. Two Linux-only test fixtures needed adaptation after the current-main DCode wrapper added isolated managed-MCP validation: bf9100f stubs every isolated Python invocation in the empty-prompt fixture, and f9283fe uses the established exact validator stub in the identity fixture before its existing launch stub. Both fixes are test-only; formatting, repository growth, fixture-transform, commit, and push guards pass.
  • Exact-head CI / Pull Request run 28683600527 passed, including static checks, build/typecheck, all five CLI shards, aggregate cli-tests, and required checks; the standard PR check rollup completed with no failures.
  • Exact-head DCode live E2E run 28683611376 passed for ubuntu-repo-cloud-langchain-deepagents-code.
  • In exact-head selective run 28683610226, both requested runtime jobs passed: mcp-bridge and hermes-e2e. The workflow-level conclusion is failure only because the post-test report-to-pr job correctly refused to comment after PR feat(mcp): add OpenShell-managed MCP servers #5876 had already merged; the scorecard also passed.
  • Exact-head Sandbox Images and E2E run 28683612355 passed both image builds plus runtime-overrides, gateway-isolation, port-override, and sandbox E2E.
  • No required-check waiver is requested.

Apurv approved the exact final head after all six findings were addressed, then merged it to main; issue #566 closed with the merge.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification: focused coverage exercises adapter, policy, credential, ownership, race, crash/rebuild/destroy, Hermes transaction/reload, workflow, and live-runtime boundaries.
  • Tests not applicable — justification: not applicable; this adds security-sensitive credential and sandbox lifecycle behavior.
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: not applicable; MCP setup and stable limitations are user-facing.
  • 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: Apurv approved exact head f9283fe after all six findings were addressed; no waiver was used.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: no required-check waiver is needed; every required check passed. Exact-head MCP and Hermes runtime jobs passed, while their non-required workflow's post-test reporter failed only because the PR had already merged. The mutable dev-channel compatibility result disclosed above remains outside the v0.0.74 release boundary.

Verification

  • 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
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Aaron Erickson aerickson@nvidia.com

@copy-pr-bot

copy-pr-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR implements end-to-end MCP (Model Context Protocol) Streamable HTTP bridge support for NemoClaw sandboxed agents. It adds a mcp CLI subcommand, OpenShell provider/policy/adapter lifecycle, per-agent adapter implementations (mcporter for OpenClaw, hermes-config for Hermes, deepagents-config for Deep Agents Code), a Hermes transactional config utility with gateway reload, cross-process filesystem lifecycle locking, sandbox destroy/rebuild MCP integration, OpenShell 0.0.72 version gate with binary MCP feature detection, Docker gateway JWT material generation, mcporter version pinning with integrity verification, JSON Schema extensions for MCP/JSON-RPC protocols, and full E2E test and documentation coverage.

Changes

MCP Streamable HTTP Bridge

Layer / File(s) Summary
Agent MCP capability contract
agents/*/manifest.yaml, src/lib/agent/defs.ts, src/lib/agent/defs.test.ts, src/lib/agent/...fixtures.ts, src/lib/agent/onboard.test.ts, src/lib/agent/runtime.test.ts
OpenClaw, Hermes, and LangChain manifests declare mcp.support: bridge and mcp.adapter; defs.ts adds AgentMcpSupport/AgentMcpAdapter/AgentMcpCapability types, readMcpCapability parser, and mcpCapability getter; test fixtures and defs tests assert parsed values and reject invalid manifests.
Policy and blueprint JSON Schema extensions
schemas/policy-preset.schema.json, schemas/sandbox-policy.schema.json, schemas/blueprint.schema.json, test/validate-config-schemas.test.ts
Schemas are extended to accept json-rpc and mcp protocols, deny_rules, recursive l7Matcher/matcher/paramMatcher types, jsonRpcOptions/mcpOptions sub-schemas, tls: require, and credential_keys; validation tests cover all new accept/reject combinations.
MCP URL security and bridge input/contract validation
src/lib/security/mcp-url-target.ts, src/lib/adapters/openshell/runtime-capabilities.ts, src/lib/actions/sandbox/mcp-bridge-contracts.ts, src/lib/actions/sandbox/mcp-bridge-validation.ts, test/mcp-url-target.test.ts, src/lib/actions/sandbox/mcp-bridge-input.test.ts
mcp-url-target.ts blocks private/reserved/special-use hosts; runtime-capabilities.ts exports the sandbox MCP transport marker constant; mcp-bridge-contracts.ts defines McpBridgeError, McpBridgeStatus, and isAgentMcpAdapter; mcp-bridge-validation.ts normalizes/validates MCP server URLs, parses mcp add CLI args, resolves credential env refs, and builds deterministic provider names.
OpenShell provider lifecycle, generated MCP policy, bridge state, and output redaction
src/lib/actions/sandbox/mcp-bridge-output.ts, src/lib/actions/sandbox/mcp-bridge-provider.ts, src/lib/actions/sandbox/mcp-bridge-policy.ts, src/lib/actions/sandbox/mcp-bridge-state.ts, src/lib/actions/sandbox/mcp-bridge-provider.test.ts, src/lib/actions/sandbox/mcp-bridge-policy.test.ts
mcp-bridge-output.ts redacts bridge secrets; mcp-bridge-provider.ts implements provider inspect/upsert/attach/detach/delete with credential revision snapshotting; mcp-bridge-policy.ts builds MCP policy YAML and manages apply/remove with ownership tracking; mcp-bridge-state.ts reads/writes bridge registry state with collision detection and gateway selection guard.
Per-agent adapter command builders (mcporter, hermes-config, deepagents-config)
src/lib/actions/sandbox/mcp-bridge-adapters.ts, src/lib/actions/sandbox/mcp-bridge-adapters.test.ts
Generates and executes registration/unregistration/inspection commands for mcporter (embedded Node script with header ownership fingerprinting), Hermes (Python one-shot exec via privileged exec), and Deep Agents Code (JSON config with managed-by ownership); tests cover header drift, output redaction, and version pinning.
MCP bridge orchestration, status, and crash-safe lifecycle tests
src/lib/actions/sandbox/mcp-bridge.ts, src/lib/actions/sandbox/mcp-bridge-status.ts, src/commands/sandbox/mcp.ts, test/mcp-add-crash-consistency.test.ts, test/mcp-policy-key-ownership.test.ts, test/mcp-provider-ownership.test.ts, src/lib/actions/sandbox/mcp-bridge-status.test.ts
mcp-bridge.ts orchestrates add (provider+policy+adapter+registry with crash-safe rollback), restart, destroy/rebuild phase preparation/finalization, and remove; mcp-bridge-status.ts produces McpBridgeJsonSummary; SandboxMcpCommand dispatches subcommands; crash-consistency, policy-key-ownership, and provider-ownership tests validate recovery and ownership semantics.
Cross-process MCP lifecycle serialization lock
src/lib/state/mcp-lifecycle-lock.ts, test/mcp-lifecycle-lock.test.ts
Implements an exclusive per-sandbox lock using atomic hard-link publication, AsyncLocalStorage reentrancy, PID liveness checking, stale-lock reaping via quarantine-and-verify rename, and a polling acquisition loop; tests cover serialization, reentrancy, stale/dead-owner recovery, recycled PIDs, and token-change protection.
Registry MCP state, sandbox-lifecycle reconcile, registration, and create-failure diagnostics
src/lib/state/registry.ts, src/lib/onboard/sandbox-lifecycle.ts, src/lib/onboard/sandbox-registration.ts, src/lib/onboard/sandbox-create-failure.ts, test/registry.test.ts, src/lib/onboard/sandbox-registration.test.ts, test/onboard-sandbox-create-failure.test.ts
registry.ts adds McpBridgeEntry/SandboxMcpState with normalization for disk serialization; sandbox-lifecycle.ts replaces sandboxExistsInGateway with reconcileSandboxForCreate returning existingEntry + preservedMcpState + liveExists; sandbox-registration.ts carries preservedMcpState; sandbox-create-failure.ts adds gateway tail fallback and printSandboxCreateFailureDiagnostics.
Sandbox destroy, rebuild, and onboard MCP lifecycle integration
src/lib/actions/sandbox/destroy.ts, src/lib/actions/sandbox/rebuild.ts, src/lib/onboard.ts, src/lib/actions/sandbox/destroy-flow.test.ts, src/lib/actions/sandbox/rebuild-flow.test.ts, test/mcp-destroy-lifecycle.test.ts
destroySandbox wraps in withMcpLifecycleLock, classifies sandbox presence via sandbox list -o json, pins OPENSHELL_GATEWAY, and stages MCP teardown/finalization; rebuildSandbox similarly stages MCP preparation and restores after recreate; onboard.ts blocks generic recreate when MCP bridges exist; flow and destroy-lifecycle tests validate lifecycle ordering and crash recovery.
policy/index.ts non-fatal mode and preset gateway state comparison
src/lib/policy/index.ts
Switches to ES imports; assertOpenshellResolvable returns boolean with nonFatal option; removePreset and applyPresetContent gain nonFatal, skipRegistryUpdate, and allowedExistingNetworkPolicyKeys; getPresetContentGatewayState and presetContentMatchesGateway add deep-equality drift detection.
Hermes transactional MCP config, gateway reload loop, base-image MCP validation, and tests
agents/hermes/mcp-config-transaction.py, agents/hermes/start.sh, agents/hermes/Dockerfile, agents/hermes/Dockerfile.base, agents/hermes/hermes-wrapper.sh, .github/actions/resolve-hermes-base-image/action.yaml, src/lib/sandbox-base-image.ts, src/lib/agent/onboard.ts, src/lib/agent/base-image.test.ts, test/hermes-mcp-config-transaction.test.ts, test/hermes-mcp-runtime-capability.test.ts, test/hermes-start*.test.ts, scripts/update-hermes-agent.sh, test/sandbox-rlimit-hooks.test.ts
mcp-config-transaction.py implements transactional Hermes config mutation with URL/payload validation, hash management, SIGUSR1 reload, rollback, and root-separation enforcement; start.sh adds a reload loop on exit code 75 with preserve-forwarders mode; Hermes Dockerfiles add the mcp UV extra, MCP runtime capability build checks, and relocate the hermes binary; sandbox-base-image.ts and onboard.ts add a validateImage/hermesBaseImageSupportsMcp probe; GitHub Action validates mcp_runtime_ok.
Deep Agents Code MCP integration
agents/langchain-deepagents-code/dcode-wrapper.sh, agents/langchain-deepagents-code/patch-managed-deepagents-code.py, test/langchain-deepagents-code-image.test.ts, test/deepagents-code-tui-startup-check.test.ts, test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh
dcode-wrapper.sh conditionally routes --mcp-config to /sandbox/.deepagents/.mcp.json and blocks standalone --no-mcp; patch-managed-deepagents-code.py injects conditional MCP enablement; image tests and TUI startup check are updated for new MCP config wiring.
OpenShell 0.0.72 version gate, MCP feature detection, and version-pin tests
src/lib/onboard/openshell-feature-gate.ts, src/lib/onboard/openshell-install.ts, src/lib/onboard/openshell-pin.ts, src/lib/onboard/openshell-version.ts, scripts/install-openshell.sh, nemoclaw-blueprint/blueprint.yaml, scripts/brev-launchable-ci-cpu.sh, test/install-openshell-version-check.test.ts, test/onboard-openshell-version.test.ts, test/e2e/test-openshell-version-pin.sh, test/e2e-scenario/live/openshell-version-pin.test.ts, test/runner.test.ts
Version constants bumped to 0.0.72; openshell-feature-gate.ts scans binary byte strings for three required MCP markers plus a sandbox transport gate; openshell-install.ts adds min version bound, messaging feature gate, and fail-closed behavior; install-openshell.sh adds FORCE_INSTALL validation, aggregated binary string scanning, and select_sha_cmd; openshell-pin.ts skips pin resolution on the dev channel.
Docker gateway JWT key material and OPENSHELL_GATEWAY_CONFIG
src/lib/onboard/docker-driver-gateway-launch.ts, src/lib/onboard/docker-driver-gateway-env.ts, src/lib/onboard/docker-driver-gateway-launch.test.ts, src/lib/onboard/docker-driver-gateway-env.test.ts
docker-driver-gateway-launch.ts generates an ed25519 JWT key bundle with a random KID, embeds [openshell.gateway.auth] and [openshell.gateway.gateway_jwt] TOML sections, and sets OPENSHELL_GATEWAY_CONFIG; OPENSHELL_GATEWAY_CONFIG is added to the managed gateway env key list; tests assert TOML content, file permissions, PEM format, and idempotency.
mcporter version pinning and upgrade regression
Dockerfile, Dockerfile.base, scripts/generate-openclaw-config.mts, test/fetch-guard-patch-regression.test.ts, test/generate-openclaw-config.test.ts
Dockerfile and Dockerfile.base add MCPORTER_VERSION (default 0.7.3) with npm dist.integrity verification and conditional upgrade logic; generate-openclaw-config.mts removes default-disabled acpx/qqbot entries; fetch-guard regression tests validate stale mcporter installs are repaired.
Privileged exec label-based container selection
src/lib/sandbox/privileged-exec.ts, src/lib/sandbox/privileged-exec.test.ts, test/config-set.test.ts, test/vm-driver-privileged-exec-routing.test.ts, test/e2e/test-vm-driver-privileged-exec-routing.sh
privileged-exec.ts switches from name-prefix heuristics to Docker label filters with tab-delimited ID+Name output; selectDirectSandboxContainer validates label/name agreement and rejects ambiguous/mismatched results; tests and E2E scripts use tabular fixture rows and assert direct routing to expected container IDs.
CLI surface, lazy imports, runner, and redact
src/commands/sandbox/mcp.ts, src/lib/cli/command-display.ts, src/lib/cli/command-registry.ts, src/lib/cli/public-display-defaults.ts, src/lib/actions/onboard.ts, src/lib/gateway-runtime-action.ts, src/lib/runner.ts, src/lib/security/redact.ts
SandboxMcpCommand dispatches `mcp add
MCP bridge documentation and command references
docs/deployment/set-up-mcp-bridge.mdx, docs/reference/commands.mdx, docs/reference/commands-nemohermes.mdx, docs/index.yml, docs/get-started/quickstart-langchain-deepagents-code.mdx, docs/get-started/prerequisites.mdx, docs/reference/platform-support.mdx, ci/platform-matrix.json, ci/test-file-size-budget.json
Adds set-up-mcp-bridge.mdx covering architecture, security boundary, per-agent adapter behavior, operational commands, rebuild/destroy semantics, and troubleshooting; updates commands.mdx and commands-nemohermes.mdx with mcp subcommand reference; updates nav in docs/index.yml; minor line-number and metadata updates in other docs/CI files.
CI workflows, E2E test infrastructure, and install-preflight updates
.github/workflows/e2e-vitest-scenarios.yaml, .github/workflows/nightly-e2e.yaml, tools/e2e-scenarios/workflow-boundary.mts, test/e2e/setup-mcp-test-tls.sh, test/e2e-scenario/live/mcp-bridge-servers.ts, test/e2e-scenario/live/mcp-bridge.test.ts, test/mcp-bridge-servers.test.ts, test/install-preflight.test.ts, test/install-build-dependency-preflight.test.ts, vitest.config.ts, test/e2e-scenario/live/network-policy.test.ts, test/e2e/test-network-policy.sh, test/e2e-scenario/live/rebuild-hermes.test.ts, test/e2e/test-rebuild-hermes.sh, test/e2e-expect-fail-closed.test.ts, test/e2e-script-workflow.test.ts, test/e2e-scenario/support-tests/*
Adds mcp-bridge-vitest and mcp-bridge-e2e jobs with openshell_channel input and MCP TLS setup; validateMcpBridgeVitestJob in workflow-boundary validates job structure; mcp-bridge-servers.ts provides fake MCP HTTPS and compatible mock servers; live mcp-bridge.test.ts covers OpenClaw/Hermes/DeepAgents E2E flows including policy verification, secret nonleak, restart, rebuild, and remove; install-preflight tests gain better assertion messages, docker/openshell stubs, and a new build-dependency preflight test.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~180 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5197: Modifies agents/langchain-deepagents-code/dcode-wrapper.sh and Deep Agents Code MCP gating paths that this PR directly extends with conditional .deepagents/.mcp.json routing.
  • NVIDIA/NemoClaw#5455: Modifies src/lib/actions/sandbox/destroy.ts sandbox destroy flow ordering that this PR refactors with MCP lifecycle preparation/finalization.
  • NVIDIA/NemoClaw#5787: Modifies src/lib/actions/sandbox/rebuild.ts rebuild workflow that this PR extends with MCP bridge preparation and restore logic.

Suggested labels

feature, `area: cli`, `area: sandbox`, `area: security`, `integration: openclaw`, `integration: hermes`, `integration: dcode`, `v0.0.69`

Suggested reviewers

  • cjagwani
  • jyaunches
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.40% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes add sandbox-scoped MCP add/list/status/restart/remove flows with host-side secret storage and policy enforcement, matching the bridge objective.
Out of Scope Changes check ✅ Passed The diff is broad, but the extra schema, Docker, docs, and test updates all support the MCP bridge feature set.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding OpenShell-managed MCP servers.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/mcp-bridge-vdr-566

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the branch is 96%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File f9283fe +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the branch is 69%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File f9283fe +/-
src/lib/shields...nsition-lock.ts 87%
src/lib/actions...all/run-plan.ts 81%
src/lib/state/o...oard-session.ts 78%
src/lib/onboard/preflight.ts 77%
src/lib/state/sandbox.ts 74%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/shields/index.ts 68%
src/lib/actions...licy-channel.ts 60%
src/lib/policy/index.ts 60%
src/lib/onboard.ts 20%

Updated July 03, 2026 21:22 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: onboard-resume, onboard-repair, cloud-onboard, full-e2e, mcp-bridge, rebuild-openclaw, rebuild-hermes, sandbox-rebuild, hermes-e2e, inference-routing, cloud-inference, network-policy, credential-sanitization, launchable-smoke, messaging-compatible-endpoint
Optional E2E: mcp-bridge-dev, rebuild-hermes-stale-base, openshell-gateway-upgrade, security-posture, messaging-providers

Dispatch hint: onboard-resume,onboard-repair,cloud-onboard,full-e2e,mcp-bridge,rebuild-openclaw,rebuild-hermes,sandbox-rebuild,hermes-e2e,inference-routing,cloud-inference,network-policy,credential-sanitization,launchable-smoke,messaging-compatible-endpoint

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • onboard-resume (medium): Required by the onboarding resume rule because live onboarding state-machine orchestration, resume config, session bootstrap, and state transitions changed.
  • onboard-repair (medium): Required companion to onboard-resume for resume repair policy and recovery-path coverage after changes to onboarding state and sandbox lifecycle code.
  • cloud-onboard (high): Full hosted onboarding can be affected by provider-inference, OpenShell install/pin, sandbox registration, Docker gateway, image, and session bootstrap changes.
  • full-e2e (high): Validates the primary install → onboard → sandbox verify → hosted inference → CLI operations user journey after broad CLI, package, Dockerfile, OpenShell, and onboarding changes.
  • mcp-bridge (high): MCP bridge implementation, policy rendering, URL validation, provider adapters, runtime capabilities, cloudflared prerequisite, and MCP live tests changed; real OpenShell MCP connectivity and artifact redaction must be validated.
  • rebuild-openclaw (high): OpenClaw Dockerfile/base image, mcporter runtime, plugin installation, rebuild phases, policy state, backup/restore, and target config changed; live rebuild is required for sandbox lifecycle confidence.
  • rebuild-hermes (high): Hermes Dockerfile/base image resolution, Hermes manifest/startup/MCP runtime, provider auth, and rebuild orchestration changed; validate a real Hermes rebuild path.
  • sandbox-rebuild (high): Core sandbox rebuild/destroy/recreate/restore/resume phases and durable config changed, so the generic sandbox rebuild lifecycle should run in addition to agent-specific rebuild jobs.
  • hermes-e2e (high): Hermes agent runtime, image contract, provider auth, startup, manifest, and MCP support changed; run the real Hermes assistant flow.
  • inference-routing (medium): Inference selection, inference-set, hosted/provider inference handling, and provider auth changed; required to catch routing regressions before merge.
  • cloud-inference (medium): Hosted/cloud inference user flow can be affected by provider selection, model namespace, credentials, and onboard provider-handler changes.
  • network-policy (medium): Policy schemas, gateway state, DNS resolution, MCP URL validation, OpenShell policy mutation checks, and sandbox proxy behavior changed; validate live network policy enforcement.
  • credential-sanitization (medium): Credential add, rebuild credential preflight, MCP bridge credentials, artifact secret scanning, subprocess environment handling, and security docs/fixtures changed; validate no secrets leak across snapshots/artifacts/runtime.
  • launchable-smoke (medium): Install scripts, Brev launchable workflow/script, Docker/base images, package metadata, and public quickstart path changed; validate launchable install smoke behavior.
  • messaging-compatible-endpoint (medium): The PR adds/changes messaging-compatible-endpoint coverage in Brev/E2E workflows and changes OpenClaw image/runtime/network proxy pieces that can affect Telegram-compatible endpoint behavior.

Optional E2E

  • mcp-bridge-dev (high): Useful compatibility evidence for the new MCP bridge path against moving OpenShell dev artifacts, but explicit-only and not necessary as a merge blocker if stable mcp-bridge passes.
  • rebuild-hermes-stale-base (high): Good adjacent coverage for the Hermes base-image fallback/import-guard changes and stale GHCR base compatibility, but can be optional if rebuild-hermes and hermes-e2e pass.
  • openshell-gateway-upgrade (medium): Adjacent confidence for OpenShell pin/install and gateway/runtime compatibility changes.
  • security-posture (high): Broad defense-in-depth check for policy, credential, image, and sandbox hardening changes; useful if runtime/security reviewers want additional confidence.
  • messaging-providers (high): Provider/L7 proxy behavior is adjacent to the OpenClaw image, network proxy, and messaging workflow changes, but the narrower compatible-endpoint lane is the stronger merge-blocking signal here.

New E2E recommendations

  • Hermes MCP Streamable HTTP client import guard (medium): The PR adds build-time import guards for Hermes MCP client support, but the existing mcp-bridge job is primarily OpenShell/OpenClaw-provider oriented and hermes-e2e may not isolate authenticated Hermes MCP Streamable HTTP execution as its own assertion.
    • Suggested test: Add a dedicated Hermes MCP Streamable HTTP live target that provisions a Hermes sandbox, configures a fixture MCP server through the bridge, and asserts authenticated tool discovery/execution plus artifact redaction.
  • E2E workflow reusable branch validation security (medium): The PR substantially rewrites e2e-branch-validation permissions, reporting, tested-SHA validation, deletion, and matrix concurrency. Unit workflow-contract tests exist, but there is limited live coverage that the reusable workflow reports the correct SHA and cleans up Brev instances across failure modes.
    • Suggested test: Add a workflow-dispatched dry-run or mocked Brev integration E2E that exercises e2e-branch-validation reporting, tested-SHA mismatch handling, and cleanup without exposing secrets to target-branch code.

Dispatch hint

  • Workflow: e2e.yaml
  • jobs input: onboard-resume,onboard-repair,cloud-onboard,full-e2e,mcp-bridge,rebuild-openclaw,rebuild-hermes,sandbox-rebuild,hermes-e2e,inference-routing,cloud-inference,network-policy,credential-sanitization,launchable-smoke,messaging-compatible-endpoint

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: issue-4462-scope-upgrade-approval-vitest, launchable-smoke-vitest, mcp-bridge-vitest, network-policy-vitest, openshell-gateway-upgrade-vitest, openshell-version-pin-vitest, rebuild-hermes-stale-base-vitest, rebuild-hermes-vitest, e2e-scenarios-all, onboard-resume-vitest, onboard-repair-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=issue-4462-scope-upgrade-approval-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=launchable-smoke-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=mcp-bridge-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=network-policy-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openshell-gateway-upgrade-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openshell-version-pin-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=rebuild-hermes-stale-base-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=rebuild-hermes-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref>
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • issue-4462-scope-upgrade-approval-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/issue-4462-scope-upgrade-approval.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=issue-4462-scope-upgrade-approval-vitest
  • launchable-smoke-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/launchable-smoke.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=launchable-smoke-vitest
  • mcp-bridge-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/mcp-bridge.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=mcp-bridge-vitest
  • network-policy-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/network-policy.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=network-policy-vitest
  • openshell-gateway-upgrade-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/openshell-gateway-upgrade.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openshell-gateway-upgrade-vitest
  • openshell-version-pin-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/openshell-version-pin.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openshell-version-pin-vitest
  • rebuild-hermes-stale-base-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/rebuild-hermes.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=rebuild-hermes-stale-base-vitest
  • rebuild-hermes-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/rebuild-hermes.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=rebuild-hermes-vitest
  • e2e-scenarios-all: The PR changes shared Vitest scenario workflow machinery, free-standing job inventory behavior, shared fixtures/support tests, and multiple live Vitest surfaces; run the full Vitest scenario fan-out.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref>
  • onboard-resume-vitest: The PR changes src/lib/onboard/machine orchestration, onboarding transitions, resume config, and session bootstrap paths; onboarding resume compatibility coverage is mandatory.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • onboard-repair-vitest: The same onboarding state-machine and session bootstrap changes can affect repair/backstop execution from persisted sessions; repair coverage is required alongside resume.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • .github/actions/resolve-hermes-base-image/action.yaml
  • .github/workflows/e2e-vitest-scenarios.yaml
  • agents/hermes/Dockerfile
  • agents/hermes/Dockerfile.base
  • agents/hermes/manifest.yaml
  • agents/hermes/mcp-config-transaction.py
  • agents/hermes/start.sh
  • agents/langchain-deepagents-code/manifest.yaml
  • scripts/install-openshell.sh
  • src/commands/sandbox/mcp.ts
  • src/lib/actions/onboard.ts
  • src/lib/actions/sandbox/mcp-bridge-adapters.ts
  • src/lib/actions/sandbox/mcp-bridge-contracts.ts
  • src/lib/actions/sandbox/mcp-bridge-output.ts
  • src/lib/actions/sandbox/mcp-bridge-policy.ts
  • src/lib/actions/sandbox/mcp-bridge-provider.ts
  • src/lib/actions/sandbox/mcp-bridge-state.ts
  • src/lib/actions/sandbox/mcp-bridge-status.ts
  • src/lib/actions/sandbox/mcp-bridge-validation.ts
  • src/lib/actions/sandbox/mcp-bridge.ts
  • src/lib/actions/sandbox/rebuild.ts
  • src/lib/adapters/openshell/client.ts
  • src/lib/adapters/openshell/resolve.ts
  • src/lib/adapters/openshell/runtime-capabilities.ts
  • src/lib/adapters/openshell/runtime.ts
  • src/lib/agent/defs.ts
  • src/lib/agent/onboard.ts
  • src/lib/agent/runtime.ts
  • src/lib/onboard.ts
  • src/lib/onboard/machine/core-flow-phases.ts
  • src/lib/onboard/machine/flow-context.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts
  • src/lib/onboard/machine/handlers/sandbox.ts
  • src/lib/onboard/resume-config.ts
  • src/lib/onboard/session-bootstrap.ts
  • src/lib/onboard/types.ts
  • src/lib/policy/index.ts
  • src/lib/security/mcp-url-target.ts
  • src/lib/security/redact.ts
  • src/lib/state/mcp-lifecycle-lock.ts
  • src/lib/state/registry.ts
  • src/lib/state/sandbox.ts
  • test/e2e-scenario/fixtures/clients/sandbox.ts
  • test/e2e-scenario/fixtures/mcp-bridge-credentials.ts
  • test/e2e-scenario/fixtures/redaction.ts
  • test/e2e-scenario/live/issue-4462-scope-upgrade-approval.test.ts
  • test/e2e-scenario/live/launchable-smoke.test.ts
  • test/e2e-scenario/live/mcp-bridge-sandbox.ts
  • test/e2e-scenario/live/mcp-bridge-servers.ts
  • test/e2e-scenario/live/mcp-bridge.test.ts
  • test/e2e-scenario/live/network-policy.test.ts
  • test/e2e-scenario/live/openshell-gateway-upgrade.test.ts
  • test/e2e-scenario/live/openshell-version-pin.test.ts
  • test/e2e-scenario/live/rebuild-hermes-env.ts
  • test/e2e-scenario/live/rebuild-hermes.test.ts
  • test/e2e-scenario/support-tests/e2e-clients.test.ts
  • test/e2e-scenario/support-tests/e2e-live-project-config.test.ts
  • test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts
  • test/e2e-scenario/support-tests/hosted-inference.test.ts
  • test/e2e-scenario/support-tests/mcp-bridge-sandbox.test.ts
  • tools/e2e-scenarios/assert-mcp-artifact-secrets-absent.mts
  • tools/e2e-scenarios/workflow-boundary.mts

Comment thread src/mcp-proxy.ts Fixed
@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Stable OpenShell MCP attribution remains interpreter/provider-scoped.
Open items: 0 required · 1 warning · 0 suggestions · 7 test follow-ups
Since last review: 0 prior items resolved · 1 still applies · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Stable OpenShell MCP attribution remains interpreter/provider-scoped in src/lib/actions/sandbox/mcp-bridge-policy-render.ts:54
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Acceptance clause
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: Acceptance clause

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify security src/lib/actions/sandbox/mcp-bridge-policy-render.ts:54 Keep the accepted-risk disposition explicit for the v0.0.74/OpenShell 0.0.72 release boundary and avoid weakening the compensating controls. If maintainers no longer accept this residual risk, tighten the policy only after the shipping OpenShell dependency exposes stable script/package-entrypoint attribution and endpoint-exclusive credential attachment.
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Stable OpenShell MCP attribution remains interpreter/provider-scoped

  • Location: src/lib/actions/sandbox/mcp-bridge-policy-render.ts:54
  • Category: security
  • Problem: The generated MCP policies authorize script-based adapters through generic Node and Python interpreter binaries, and OpenShell 0.0.72 attaches static provider credentials at sandbox scope rather than binding one credential exclusively to one endpoint. The PR documents this as an accepted v0.0.74 release boundary and adds strong compensating controls: exact HTTPS destination, literal path, explicit MCP method profile, request-size limit, DNS pins, unique credential-key checks, provider ownership checks, non-allowlisted-binary denial controls, and artifact credential scans. However, the residual boundary is still active in the shipping stable path.
  • Impact: A compromised or unintended process already running under an allowlisted Node or Python interpreter inside the sandbox can attempt the configured MCP endpoint with the sandbox-scoped OpenShell provider placeholder if it stays within the generated destination, path, method, body-size, and DNS-pin envelope. This is much narrower than broad egress, but it is weaker than package-entrypoint attribution plus endpoint-exclusive provider credentials.
  • Recommended action: Keep the accepted-risk disposition explicit for the v0.0.74/OpenShell 0.0.72 release boundary and avoid weakening the compensating controls. If maintainers no longer accept this residual risk, tighten the policy only after the shipping OpenShell dependency exposes stable script/package-entrypoint attribution and endpoint-exclusive credential attachment.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `binariesForAdapter()` in `src/lib/actions/sandbox/mcp-bridge-policy-render.ts` and the `Stable OpenShell 0.0.72 Limitations` section in `docs/deployment/set-up-mcp-bridge.mdx`; then inspect `src/lib/actions/sandbox/mcp-bridge-policy.test.ts`, `test/e2e/support/mcp-bridge-sandbox.test.ts`, and `test/e2e/live/mcp-bridge.test.ts` for DNS pins, method/path constraints, non-allowlisted curl denial, and zero-upstream rebinding assertions.
  • Missing regression test: Existing coverage documents and constrains the accepted boundary in `src/lib/actions/sandbox/mcp-bridge-policy.test.ts`, `test/e2e/support/mcp-bridge-sandbox.test.ts`, and `test/e2e/live/mcp-bridge.test.ts`. If this risk is fixed instead of accepted, add a live regression named `unrelated Node and Python interpreter processes are denied while the real MCP adapters succeed under stable package-entrypoint attribution`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `binariesForAdapter()` in `src/lib/actions/sandbox/mcp-bridge-policy-render.ts` and the `Stable OpenShell 0.0.72 Limitations` section in `docs/deployment/set-up-mcp-bridge.mdx`; then inspect `src/lib/actions/sandbox/mcp-bridge-policy.test.ts`, `test/e2e/support/mcp-bridge-sandbox.test.ts`, and `test/e2e/live/mcp-bridge.test.ts` for DNS pins, method/path constraints, non-allowlisted curl denial, and zero-upstream rebinding assertions.
  • Evidence: `binariesForAdapter()` returns Node interpreter paths for `mcporter` and Python interpreter paths for Hermes and Deep Agents. `docs/deployment/set-up-mcp-bridge.mdx` states that OpenShell v0.0.72 attributes script-based clients through Node/Python and attaches provider credentials at sandbox scope.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Keep or identify the live MCP bridge runtime proof where OpenClaw, Hermes, and LangChain Deep Agents Code add an authenticated HTTPS MCP server, call a real tool, restart without the host secret exported, rotate the provider credential, rebuild without the host secret, remove the bridge, deny post-remove access, and scan sandbox files/registry/artifacts for fixture credentials.. This PR changes actual OpenShell MCP policy enforcement, provider credential replacement, Docker image composition, Hermes gateway reload, workflow secret boundaries, and live sandbox networking. The diff includes targeted runtime validation for the current accepted behavior; keep that evidence available because unit mocks alone are not sufficient for these trust boundaries.
  • PRA-T2 Runtime validation — Keep or identify workflow-boundary validation proving the Brev secret-bearing target-branch job has read-only permissions and `persist-credentials:false`, while the write-scoped reporter has no checkout, no Brev/inference secrets, and refuses to report stale PR-head evidence.. This PR changes actual OpenShell MCP policy enforcement, provider credential replacement, Docker image composition, Hermes gateway reload, workflow secret boundaries, and live sandbox networking. The diff includes targeted runtime validation for the current accepted behavior; keep that evidence available because unit mocks alone are not sufficient for these trust boundaries.
  • PRA-T3 Runtime validation — Keep or identify image/runtime validation proving final OpenClaw and Hermes images contain the pinned `mcporter`/Hermes MCP runtime, reject missing MCP imports at build time, and install `mcporter` from the committed lock with package lifecycle scripts disabled.. This PR changes actual OpenShell MCP policy enforcement, provider credential replacement, Docker image composition, Hermes gateway reload, workflow secret boundaries, and live sandbox networking. The diff includes targeted runtime validation for the current accepted behavior; keep that evidence available because unit mocks alone are not sufficient for these trust boundaries.
  • PRA-T4 Runtime validation — If maintainers choose to fix rather than accept the OpenShell 0.0.72 interpreter-attribution limitation, add a live regression proving unrelated Node and Python interpreter processes are denied while the real MCP adapters still succeed under stable package-entrypoint attribution.. This PR changes actual OpenShell MCP policy enforcement, provider credential replacement, Docker image composition, Hermes gateway reload, workflow secret boundaries, and live sandbox networking. The diff includes targeted runtime validation for the current accepted behavior; keep that evidence available because unit mocks alone are not sufficient for these trust boundaries.
  • PRA-T5 Acceptance clause — OpenShell 0.0.72 attributes script-based clients through their Node or Python interpreter. — add test evidence or identify existing coverage. The PR documents and constrains this accepted limitation but does not remove it. The remaining residual risk is tracked as the security finding in this review.
  • PRA-T6 Acceptance clause — Static provider credentials are sandbox-scoped rather than endpoint-exclusive. — add test evidence or identify existing coverage. The PR requires unique credential keys, randomized provider names, exact ownership checks, and status warnings, but endpoint-exclusive credential attachment depends on future OpenShell support and remains part of the same residual-risk finding.
  • PRA-T7 Acceptance clause — Provider mutation is name-based rather than atomic compare-and-swap. — add test evidence or identify existing coverage. `mcp-bridge-provider-mutation.ts` compensates with lifecycle locks, expected provider IDs, credential-key checks, resourceVersion checks, and fail-closed behavior. Native CAS remains upstream future work and is documented as a limitation.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Stable OpenShell MCP attribution remains interpreter/provider-scoped

  • Location: src/lib/actions/sandbox/mcp-bridge-policy-render.ts:54
  • Category: security
  • Problem: The generated MCP policies authorize script-based adapters through generic Node and Python interpreter binaries, and OpenShell 0.0.72 attaches static provider credentials at sandbox scope rather than binding one credential exclusively to one endpoint. The PR documents this as an accepted v0.0.74 release boundary and adds strong compensating controls: exact HTTPS destination, literal path, explicit MCP method profile, request-size limit, DNS pins, unique credential-key checks, provider ownership checks, non-allowlisted-binary denial controls, and artifact credential scans. However, the residual boundary is still active in the shipping stable path.
  • Impact: A compromised or unintended process already running under an allowlisted Node or Python interpreter inside the sandbox can attempt the configured MCP endpoint with the sandbox-scoped OpenShell provider placeholder if it stays within the generated destination, path, method, body-size, and DNS-pin envelope. This is much narrower than broad egress, but it is weaker than package-entrypoint attribution plus endpoint-exclusive provider credentials.
  • Recommended action: Keep the accepted-risk disposition explicit for the v0.0.74/OpenShell 0.0.72 release boundary and avoid weakening the compensating controls. If maintainers no longer accept this residual risk, tighten the policy only after the shipping OpenShell dependency exposes stable script/package-entrypoint attribution and endpoint-exclusive credential attachment.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `binariesForAdapter()` in `src/lib/actions/sandbox/mcp-bridge-policy-render.ts` and the `Stable OpenShell 0.0.72 Limitations` section in `docs/deployment/set-up-mcp-bridge.mdx`; then inspect `src/lib/actions/sandbox/mcp-bridge-policy.test.ts`, `test/e2e/support/mcp-bridge-sandbox.test.ts`, and `test/e2e/live/mcp-bridge.test.ts` for DNS pins, method/path constraints, non-allowlisted curl denial, and zero-upstream rebinding assertions.
  • Missing regression test: Existing coverage documents and constrains the accepted boundary in `src/lib/actions/sandbox/mcp-bridge-policy.test.ts`, `test/e2e/support/mcp-bridge-sandbox.test.ts`, and `test/e2e/live/mcp-bridge.test.ts`. If this risk is fixed instead of accepted, add a live regression named `unrelated Node and Python interpreter processes are denied while the real MCP adapters succeed under stable package-entrypoint attribution`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `binariesForAdapter()` in `src/lib/actions/sandbox/mcp-bridge-policy-render.ts` and the `Stable OpenShell 0.0.72 Limitations` section in `docs/deployment/set-up-mcp-bridge.mdx`; then inspect `src/lib/actions/sandbox/mcp-bridge-policy.test.ts`, `test/e2e/support/mcp-bridge-sandbox.test.ts`, and `test/e2e/live/mcp-bridge.test.ts` for DNS pins, method/path constraints, non-allowlisted curl denial, and zero-upstream rebinding assertions.
  • Evidence: `binariesForAdapter()` returns Node interpreter paths for `mcporter` and Python interpreter paths for Hermes and Deep Agents. `docs/deployment/set-up-mcp-bridge.mdx` states that OpenShell v0.0.72 attributes script-based clients through Node/Python and attaches provider credentials at sandbox scope.

Workflow run details

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.

@github-actions

Copy link
Copy Markdown
Contributor

Comment thread src/lib/actions/sandbox/mcp-bridge.ts Fixed
Comment thread src/lib/actions/sandbox/mcp-bridge.ts Fixed
Comment thread src/lib/actions/sandbox/mcp-bridge.ts Fixed
Comment thread src/lib/actions/sandbox/mcp-bridge.ts Fixed
@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-4: Credential boundary manifest lacks runtime OpenShell version verification (TypeScript); then add or justify PRA-T1.
Open items: 9 required · 9 warnings · 7 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 37 new items found

Action checklist

  • PRA-4 Fix: Credential boundary manifest lacks runtime OpenShell version verification (TypeScript) in src/lib/actions/sandbox/mcp-bridge-validation.ts:90
  • PRA-5 Fix: Credential boundary manifest lacks runtime OpenShell version verification (Python) in agents/hermes/mcp-config-transaction.py:136
  • PRA-6 Fix: Hermes Dockerfile lacks runtime OpenShell version verification in agents/hermes/Dockerfile:31
  • PRA-7 Fix: Missing startup reconciliation for mcp_servers config drift in Hermes in agents/hermes/start.sh:1
  • PRA-8 Fix: preflight.ts monolith growth exceeds threshold (+56 lines, now 2370) in src/lib/onboard/preflight.ts:1
  • PRA-9 Fix: Test monolith growth: rebuild-resume-config.test.ts grew by 119 lines (now 587) in src/lib/actions/sandbox/rebuild-resume-config.test.ts:1
  • PRA-10 Fix: Test monolith growth: sandbox.test.ts grew by 98 lines (now 1013) in src/lib/onboard/machine/handlers/sandbox.test.ts:1
  • PRA-11 Fix: policy-channel.ts monolith growth exceeds threshold (+39 lines, now 1586) in src/lib/actions/sandbox/policy-channel.ts:1
  • PRA-12 Fix: New Python transaction module needs focused security review in agents/hermes/mcp-config-transaction.py:1
  • PRA-1 Resolve or justify: Source-of-truth review needed: scripts/install-openshell.sh:725-727 (gh CLI → curl fallback)
  • PRA-2 Resolve or justify: Source-of-truth review needed: scripts/install-openshell.sh:784-797 (sudo install → user-local fallback)
  • PRA-3 Resolve or justify: Source-of-truth review needed: scripts/install-openshell.sh:523-584 (macOS VM driver auto-signing with sudo fallback)
  • PRA-13 Resolve or justify: IPv6 literal rejection error message could be clearer in src/lib/actions/sandbox/mcp-bridge-url-validation.ts:83
  • PRA-14 Resolve or justify: Python IPv6 literal rejection error message less clear than TypeScript in agents/hermes/mcp-config-transaction.py:297
  • PRA-15 Resolve or justify: Credential env name validation uses deny-list; may miss future OpenShell keys in src/lib/actions/sandbox/mcp-bridge-validation.ts:90
  • PRA-16 Resolve or justify: Branch validation workflow lacks explicit TRUST BOUNDARY documentation in .github/workflows/e2e-branch-validation.yaml:1
  • PRA-17 Resolve or justify: Four install-openshell.sh fallbacks lack source-of-truth review in scripts/install-openshell.sh:1
  • PRA-18 Resolve or justify: MCP URL validation duplicated between TypeScript and Python without CI parity gate in src/lib/security/mcp-url-target.ts:1
  • PRA-T1 Add or justify test follow-up: Add IPv6 literal rejection test coverage to TypeScript and Python test suites
  • PRA-T2 Add or justify test follow-up: Acceptance clause
  • PRA-T3 Add or justify test follow-up: Acceptance clause
  • PRA-T4 Add or justify test follow-up: Acceptance clause
  • PRA-T5 Add or justify test follow-up: Acceptance clause
  • PRA-T6 Add or justify test follow-up: scripts/install-openshell.sh:725-727 (gh CLI → curl fallback)
  • PRA-T7 Add or justify test follow-up: scripts/install-openshell.sh:784-797 (sudo install → user-local fallback)
  • PRA-T8 Add or justify test follow-up: scripts/install-openshell.sh:523-584 (macOS VM driver auto-signing with sudo fallback)
  • PRA-19 In-scope improvement: Add IPv6 literal rejection test coverage to TypeScript and Python test suites in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts:1
  • PRA-20 In-scope improvement: Extract policy-channel.ts into focused modules in src/lib/actions/sandbox/policy-channel.ts:1
  • PRA-21 In-scope improvement: Extract MCP preflight logic from preflight.ts monolith in src/lib/onboard/preflight.ts:1
  • PRA-22 In-scope improvement: Split rebuild-resume-config.test.ts into focused modules in src/lib/actions/sandbox/rebuild-resume-config.test.ts:1
  • PRA-23 In-scope improvement: Split sandbox.test.ts into focused modules in src/lib/onboard/machine/handlers/sandbox.test.ts:1
  • PRA-24 In-scope improvement: Thin wrapper adapter field could be inlined in agents/hermes/manifest.yaml:126
  • PRA-25 In-scope improvement: Single-use config constant could be simplified in .github/workflows/e2e.yaml:635

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-4 Required security src/lib/actions/sandbox/mcp-bridge-validation.ts:90 Add `openshell --version` runtime check in mcp-bridge-validation.ts (or a shared preflight module) that parses the output and compares against manifest['openshellVersion']. Fail closed on mismatch, missing binary, or parse error. Consider caching the version check with TTL to avoid repeated subprocess calls.
PRA-5 Required security agents/hermes/mcp-config-transaction.py:136 Add `subprocess.run(['openshell', '--version'], capture_output=True, text=True)` in `_load_credential_boundary_manifest()` and compare parsed version against manifest['openshellVersion']. Fail closed on mismatch, missing binary, or parse error.
PRA-6 Required security agents/hermes/Dockerfile:31 Add runtime OpenShell version check in Hermes entrypoint (nemoclaw-start) or in hermes-mcp-config-transaction.py that compares `openshell --version` against the manifest version baked into the image. Fail closed on mismatch.
PRA-7 Required correctness agents/hermes/start.sh:1 Add startup reconciliation in start.sh or runtime-config-guard.py that detects config/hash drift against running gateway state for mcp_servers and either auto-reloads or fails closed with clear guidance. Extend hash verification (STRICT_HASH_PATH) to cover mcp_servers config.
PRA-8 Required architecture src/lib/onboard/preflight.ts:1 Extract MCP-related preflight logic to separate module (e.g., preflight-mcp.ts) or split preflight.ts into smaller domain-focused modules. At minimum, offset the growth by extracting existing unrelated logic.
PRA-9 Required architecture src/lib/actions/sandbox/rebuild-resume-config.test.ts:1 Split into rebuild-resume-config-compatible-endpoint.test.ts, rebuild-resume-config-local.test.ts, rebuild-resume-config-credential.test.ts, or move shared helpers to test/helpers/rebuild-resume-config-support.ts.
PRA-10 Required architecture src/lib/onboard/machine/handlers/sandbox.test.ts:1 Split into focused modules: sandbox-handler-channels.test.ts, sandbox-handler-gpu.test.ts, sandbox-handler-agent.test.ts, sandbox-handler-messaging.test.ts. Move shared fixtures to test/helpers/sandbox-handler-support.ts.
PRA-11 Required architecture src/lib/actions/sandbox/policy-channel.ts:1 Extract focused policy channel logic to separate modules (e.g., policy-channel-add.ts, policy-channel-remove.ts, policy-channel-list.ts, policy-channel-messaging.ts) before merge.
PRA-12 Required security agents/hermes/mcp-config-transaction.py:1 Conduct focused security review of mcp-config-transaction.py. Ensure test coverage includes negative cases for YAML deserialization attacks, command injection via server names, path traversal in config paths, and privilege separation bypasses. Verify yaml.safe_load usage, path validation under /sandbox/.hermes, file locking (fcntl), and subprocess call safety.
PRA-13 Resolve/justify security src/lib/actions/sandbox/mcp-bridge-url-validation.ts:83 Keep current TypeScript message as baseline. Ensure Python message matches clarity. Document and test DNS AAAA resolution parity.
PRA-14 Resolve/justify security agents/hermes/mcp-config-transaction.py:297 Update Python error message to match TypeScript: 'IPv6-literal MCP server URLs (bracketed addresses like [::1]) are not supported by the current OpenShell proxy target parser. Use a DNS hostname with public A/AAAA records.' Document and test DNS AAAA resolution parity.
PRA-15 Resolve/justify security src/lib/actions/sandbox/mcp-bridge-validation.ts:90 Implement runtime OpenShell version check (PRA-5/6/7) as primary defense. Additionally, consider allow-list pattern for credential names (e.g., require *_MCP_TOKEN suffix) as defense-in-depth. Document that manifest must be updated in lockstep with OpenShell upgrades.
PRA-16 Resolve/justify workflow .github/workflows/e2e-branch-validation.yaml:1 Add a 'TRUST BOUNDARY' section to the workflow header documenting: (1) validation job permissions are downgraded to contents:read, pull-requests:read; (2) report-pr job has checks:write, pull-requests:write but no checkout of target code; (3) branch is resolved from pr_number via gh CLI, not from caller-controlled input.
PRA-17 Resolve/justify architecture scripts/install-openshell.sh:1 Document source-of-truth review for each fallback per the rubric, or refactor to make invalid states impossible at source (e.g., require gh auth upfront, require sudo for system install, remove dev-channel path).
PRA-18 Resolve/justify architecture src/lib/security/mcp-url-target.ts:1 Consolidate validation into a shared library (e.g., WASM module or JSON schema + codegen) or add a mandatory CI gate that runs both validators against a canonical test matrix and fails on divergence. The existing parity test should be mandatory in CI.
PRA-19 Improvement tests src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts:1 Add IPv6 literal rejection test cases to src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts and test/hermes-mcp-config-transaction.test.ts covering: private IPv6 (fc00::/7, fe80::/10), loopback (::1), mapped IPv4 (::ffff:127.0.0.1), documentation (2001:db8::/32), and multicast (ff00::/8).
PRA-20 Improvement architecture src/lib/actions/sandbox/policy-channel.ts:1 Extract into focused modules: policy-channel-add.ts, policy-channel-remove.ts, policy-channel-list.ts, policy-channel-messaging.ts. Share common types via policy-channel-types.ts.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-4 Required — Credential boundary manifest lacks runtime OpenShell version verification (TypeScript)

  • Location: src/lib/actions/sandbox/mcp-bridge-validation.ts:90
  • Category: security
  • Problem: The TypeScript validation loads the credential boundary manifest (openshell-child-visible-credentials.v0.0.72.json) at import time and validates against its deny-lists (runtimeControlKeys, runtimeControlPrefixes, rawChildValueKeys, rewrittenChildValueKeys), but never executes `openshell --version` at runtime to verify the running OpenShell matches the manifest version baked into the image.
  • Impact: If OpenShell is upgraded/downgraded in the sandbox (e.g., via install-openshell.sh or manual upgrade), the deny-list becomes stale. Reserved credential names from the new OpenShell version would be accepted, or legitimate names from the old version would be rejected, breaking the credential isolation boundary.
  • Required action: Add `openshell --version` runtime check in mcp-bridge-validation.ts (or a shared preflight module) that parses the output and compares against manifest['openshellVersion']. Fail closed on mismatch, missing binary, or parse error. Consider caching the version check with TTL to avoid repeated subprocess calls.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for 'openshell --version' or 'subprocess' or 'spawn' in src/lib/actions/sandbox/mcp-bridge-validation.ts and src/lib/security/mcp-url-target.ts — no runtime version check exists. The manifest is imported at line 12 and deny-lists derived at lines 90-100.
  • Missing regression test: Add test in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts that mocks openshell --version returning a different version (e.g., 0.0.73) and verifies the validation fails closed when a reserved name from the new version is used.
  • Done when: The required change is committed and verification passes: Search for 'openshell --version' or 'subprocess' or 'spawn' in src/lib/actions/sandbox/mcp-bridge-validation.ts and src/lib/security/mcp-url-target.ts — no runtime version check exists. The manifest is imported at line 12 and deny-lists derived at lines 90-100.
  • Evidence: Lines 12-100 import manifest and build deny-lists from manifest.runtimeControlKeys, manifest.runtimeControlPrefixes, manifest.rawChildValueKeys, manifest.rewrittenChildValueKeys. manifest.openshellVersion is '0.0.72' but never verified at runtime.

PRA-5 Required — Credential boundary manifest lacks runtime OpenShell version verification (Python)

  • Location: agents/hermes/mcp-config-transaction.py:136
  • Category: security
  • Problem: The Python transaction module loads the credential boundary manifest at module import time and validates its openshellVersion field, but never executes `openshell --version` at runtime to verify the running OpenShell matches. The manifest is loaded from a file baked into the image, which can diverge from the actual runtime OpenShell version.
  • Impact: If OpenShell is upgraded/downgraded in the Hermes sandbox (e.g., via install-openshell.sh or manual upgrade), the Python transaction's deny-list becomes stale. Reserved credential names from the new OpenShell version would be accepted, or legitimate names from the old version would be rejected.
  • Required action: Add `subprocess.run(['openshell', '--version'], capture_output=True, text=True)` in `_load_credential_boundary_manifest()` and compare parsed version against manifest['openshellVersion']. Fail closed on mismatch, missing binary, or parse error.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for 'openshell --version' or 'subprocess.run' in agents/hermes/mcp-config-transaction.py — no runtime version check exists. The manifest is loaded at line 136 via `_load_credential_boundary_manifest()` called at module level.
  • Missing regression test: Add test in test/hermes-mcp-config-transaction.test.ts that mocks openshell --version returning a different version and verifies the validation fails closed when a reserved name from the new version is used.
  • Done when: The required change is committed and verification passes: Search for 'openshell --version' or 'subprocess.run' in agents/hermes/mcp-config-transaction.py — no runtime version check exists. The manifest is loaded at line 136 via `_load_credential_boundary_manifest()` called at module level.
  • Evidence: _load_credential_boundary_manifest() at line 136 loads manifest from file and checks manifest.get('openshellVersion') == '0.0.72' but never runs openshell --version.

PRA-6 Required — Hermes Dockerfile lacks runtime OpenShell version verification

  • Location: agents/hermes/Dockerfile:31
  • Category: security
  • Problem: Hermes Dockerfile has a build-time import guard for MCP client availability (`import mcp; from tools import mcp_tool`), but no runtime OpenShell version check in the entrypoint (nemoclaw-start) or in hermes-mcp-config-transaction.py that compares `openshell --version` against the manifest version baked into the image.
  • Impact: A Hermes sandbox image built with the correct manifest could be started with a different OpenShell version at runtime, causing the credential boundary deny-list to be out of sync with the actual OpenShell behavior.
  • Required action: Add runtime OpenShell version check in Hermes entrypoint (nemoclaw-start) or in hermes-mcp-config-transaction.py that compares `openshell --version` against the manifest version baked into the image. Fail closed on mismatch.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check agents/hermes/Dockerfile and agents/hermes/start.sh for any `openshell --version` runtime verification — none exists. The build-time check at line 31 only validates MCP client imports.
  • Missing regression test: Add E2E test in test/e2e/live/rebuild-hermes.test.ts that runs Hermes sandbox with a mismatched OpenShell version (e.g., by overriding the OpenShell binary) and verifies startup fails.
  • Done when: The required change is committed and verification passes: Check agents/hermes/Dockerfile and agents/hermes/start.sh for any `openshell --version` runtime verification — none exists. The build-time check at line 31 only validates MCP client imports.
  • Evidence: Dockerfile line 31 has RUN /opt/hermes/.venv/bin/python -c 'import mcp; from tools import mcp_tool; assert getattr(mcp_tool, "_MCP_AVAILABLE", False)...' but no runtime openshell --version check.

PRA-7 Required — Missing startup reconciliation for mcp_servers config drift in Hermes

  • Location: agents/hermes/start.sh:1
  • Category: correctness
  • Problem: No startup reconciliation for mcp_servers config drift in Hermes. The config hash verification (STRICT_HASH_PATH) only covers config.yaml and .env, not the mcp_servers section. If mcp_servers config is modified at runtime (via the Python transaction module), the gateway reload may not pick up changes or may restart with stale config.
  • Impact: MCP servers added/removed via `nemoclaw mcp add|remove` may not be reflected in the running gateway after a restart/rebuild, leading to silent misconfiguration where the registry shows one set of servers but the gateway serves another.
  • Required action: Add startup reconciliation in start.sh or runtime-config-guard.py that detects config/hash drift against running gateway state for mcp_servers and either auto-reloads or fails closed with clear guidance. Extend hash verification (STRICT_HASH_PATH) to cover mcp_servers config.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for 'mcp_servers' in agents/hermes/start.sh and agents/hermes/runtime-config-guard.py — no drift detection for mcp_servers exists. The hash verification only covers config.yaml and .env.
  • Missing regression test: Add test in test/hermes-mcp-config-transaction.test.ts that modifies mcp_servers config at runtime (via the transaction module) and verifies startup detects the drift and either reloads or fails with actionable error.
  • Done when: The required change is committed and verification passes: Search for 'mcp_servers' in agents/hermes/start.sh and agents/hermes/runtime-config-guard.py — no drift detection for mcp_servers exists. The hash verification only covers config.yaml and .env.
  • Evidence: start.sh uses verify_config_integrity which calls verify_config_integrity from sandbox-init.sh — that function only hashes config.yaml and .env. No mcp_servers coverage.

PRA-8 Required — preflight.ts monolith growth exceeds threshold (+56 lines, now 2370)

  • Location: src/lib/onboard/preflight.ts:1
  • Category: architecture
  • Problem: preflight.ts monolith grew by 56 lines (now 2370 lines). The file contains MCP-related preflight logic mixed with port probing, DNS probes, swap management, CDI assessment, and host remediation. This violates single-responsibility and makes security review difficult.
  • Impact: MCP logic is scattered across a 2370-line file, making it hard to audit security-critical preflight checks. Changes to unrelated preflight areas risk breaking MCP validation.
  • Required action: Extract MCP-related preflight logic to separate module (e.g., preflight-mcp.ts) or split preflight.ts into smaller domain-focused modules. At minimum, offset the growth by extracting existing unrelated logic.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/onboard/preflight.ts` — shows 2370 lines. Search for 'mcp' or 'MCP' in the file to find scattered MCP logic.
  • Missing regression test: No specific test needed — architectural refactoring verified by existing preflight tests passing after extraction.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/onboard/preflight.ts` — shows 2370 lines. Search for 'mcp' or 'MCP' in the file to find scattered MCP logic.
  • Evidence: Monolith delta from drift context: +56 lines, severity blocker. File contains checkContainerRuntimeResources, assessHost, probePortAvailability, DNS probes, swap, CDI, MCP preflight all in one file.

PRA-9 Required — Test monolith growth: rebuild-resume-config.test.ts grew by 119 lines (now 587)

  • Location: src/lib/actions/sandbox/rebuild-resume-config.test.ts:1
  • Category: architecture
  • Problem: Test monolith grew by 119 lines (now 587 lines). Contains tests for compatible endpoint, local, credential, and resume config scenarios all in one file.
  • Impact: Large test file is hard to maintain and review. Related tests are mixed with unrelated scenarios.
  • Required action: Split into rebuild-resume-config-compatible-endpoint.test.ts, rebuild-resume-config-local.test.ts, rebuild-resume-config-credential.test.ts, or move shared helpers to test/helpers/rebuild-resume-config-support.ts.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/actions/sandbox/rebuild-resume-config.test.ts` — shows 587 lines. Review describe blocks for logical groupings.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after split.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/actions/sandbox/rebuild-resume-config.test.ts` — shows 587 lines. Review describe blocks for logical groupings.
  • Evidence: Monolith delta from drift context: +119 lines, severity blocker.

PRA-10 Required — Test monolith growth: sandbox.test.ts grew by 98 lines (now 1013)

  • Location: src/lib/onboard/machine/handlers/sandbox.test.ts:1
  • Category: architecture
  • Problem: Test monolith grew by 98 lines (now 1013 lines). Contains tests for sandbox state handling with various channel, GPU, agent, and messaging configurations.
  • Impact: Large test file is hard to maintain. Related tests for different configurations are mixed together.
  • Required action: Split into focused modules: sandbox-handler-channels.test.ts, sandbox-handler-gpu.test.ts, sandbox-handler-agent.test.ts, sandbox-handler-messaging.test.ts. Move shared fixtures to test/helpers/sandbox-handler-support.ts.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/onboard/machine/handlers/sandbox.test.ts` — shows 1013 lines.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after split.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/onboard/machine/handlers/sandbox.test.ts` — shows 1013 lines.
  • Evidence: Monolith delta from drift context: +98 lines, severity blocker.

PRA-11 Required — policy-channel.ts monolith growth exceeds threshold (+39 lines, now 1586)

  • Location: src/lib/actions/sandbox/policy-channel.ts:1
  • Category: architecture
  • Problem: policy-channel.ts monolith grew by 39 lines (now 1586 lines). Handles policy add, remove, list, and messaging channel mutations in a single file.
  • Impact: Policy channel logic is a 1586-line monolith mixing multiple mutation types. Hard to audit security boundaries for each operation.
  • Required action: Extract focused policy channel logic to separate modules (e.g., policy-channel-add.ts, policy-channel-remove.ts, policy-channel-list.ts, policy-channel-messaging.ts) before merge.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/actions/sandbox/policy-channel.ts` — shows 1586 lines. grep for 'export async function' to see all public functions.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after extraction.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/actions/sandbox/policy-channel.ts` — shows 1586 lines. grep for 'export async function' to see all public functions.
  • Evidence: Monolith delta from drift context: +39 lines, severity blocker.

PRA-12 Required — New Python transaction module needs focused security review

  • Location: agents/hermes/mcp-config-transaction.py:1
  • Category: security
  • Problem: New Python transaction module (969 lines) handles Hermes MCP config mutation and gateway reload control. It performs YAML deserialization, URL validation, credential name validation, file locking, atomic writes, and gateway process management. Requires focused security review for: YAML deserialization attacks, command injection via server names, path traversal in config paths, privilege separation bypasses, and TOCTOU races in config mutation.
  • Impact: This module is the sole writer of Hermes mcp_servers config and controls gateway reload. A vulnerability could allow config tampering, privilege escalation, or gateway takeover.
  • Required action: Conduct focused security review of mcp-config-transaction.py. Ensure test coverage includes negative cases for YAML deserialization attacks, command injection via server names, path traversal in config paths, and privilege separation bypasses. Verify yaml.safe_load usage, path validation under /sandbox/.hermes, file locking (fcntl), and subprocess call safety.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Review agents/hermes/mcp-config-transaction.py for: yaml.safe_load usage (line ~400), path validation (CONFIG_PATH constant), file locking (fcntl in guard), subprocess calls (os.kill for SIGUSR1), and credential handling (never logs raw values).
  • Missing regression test: Add negative test cases in test/hermes-mcp-config-transaction.test.ts for: malicious YAML payloads (e.g., !!python/object/apply:os.system), server names with path traversal (../../../etc/passwd), concurrent mutation attempts, and privilege escalation via config manipulation.
  • Done when: The required change is committed and verification passes: Review agents/hermes/mcp-config-transaction.py for: yaml.safe_load usage (line ~400), path validation (CONFIG_PATH constant), file locking (fcntl in guard), subprocess calls (os.kill for SIGUSR1), and credential handling (never logs raw values).
  • Evidence: New file agents/hermes/mcp-config-transaction.py (969 lines) added in this PR. Uses yaml.safe_load, fcntl locking via hermes-runtime-config-guard.py, validates server names with SERVER_NAME_RE, verifies mutable snapshot ownership.
Review findings by urgency: 9 required fixes, 9 items to resolve/justify, 7 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: scripts/install-openshell.sh:725-727 (gh CLI → curl fallback)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test in test/install-openshell-version-check.test.ts that triggers gh auth failure and verifies curl fallback downloads correct asset with SHA256 verification
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Lines 725-727: if ! gh release download ...; then curl -L ...; fi

PRA-2 Resolve/justify — Source-of-truth review needed: scripts/install-openshell.sh:784-797 (sudo install → user-local fallback)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test that triggers sudo failure and verifies user-local install to ~/.local/bin works and is on PATH
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Lines 784-797: if sudo install ...; then ... else install -m 755 ... ~/.local/bin/openshell; fi

PRA-3 Resolve/justify — Source-of-truth review needed: scripts/install-openshell.sh:523-584 (macOS VM driver auto-signing with sudo fallback)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test on macOS that verifies unsigned VM driver triggers auto-signing and executes successfully
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Lines 523-584: codesign --sign - --force ... with sudo fallback for entitlements

PRA-13 Resolve/justify — IPv6 literal rejection error message could be clearer

  • Location: src/lib/actions/sandbox/mcp-bridge-url-validation.ts:83
  • Category: security
  • Problem: IPv6 literal rejection error message: 'IPv6-literal MCP server URLs are not supported by the current OpenShell proxy target parser. Use a DNS hostname with public A/AAAA records.' This is clear but could explicitly mention bracketed address format (e.g., [::1]).
  • Impact: Operators attempting to use IPv6 literals get a rejection but may not understand the bracketed format is the issue, leading to confusion.
  • Recommended action: Keep current TypeScript message as baseline. Ensure Python message matches clarity. Document and test DNS AAAA resolution parity.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check the error message at line 83 in mcp-bridge-url-validation.ts — it rejects IPv6 literals with the message shown.
  • Missing regression test: Add test in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts that verifies the exact error message for IPv6 literal rejection includes 'bracketed addresses like [::1]'.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check the error message at line 83 in mcp-bridge-url-validation.ts — it rejects IPv6 literals with the message shown.
  • Evidence: Line 83 throws McpBridgeError with message 'IPv6-literal MCP server URLs are not supported by the current OpenShell proxy target parser. Use a DNS hostname with public A/AAAA records.'

PRA-14 Resolve/justify — Python IPv6 literal rejection error message less clear than TypeScript

  • Location: agents/hermes/mcp-config-transaction.py:297
  • Category: security
  • Problem: Python IPv6 literal rejection error message: 'IPv6-literal MCP URLs are not supported' — less clear than TypeScript version. Doesn't mention bracketed format or DNS alternative.
  • Impact: Hermes operators using the Python transaction directly get a less actionable error message, potentially causing confusion about why IPv6 literals are rejected.
  • Recommended action: Update Python error message to match TypeScript: 'IPv6-literal MCP server URLs (bracketed addresses like [::1]) are not supported by the current OpenShell proxy target parser. Use a DNS hostname with public A/AAAA records.' Document and test DNS AAAA resolution parity.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check line ~297 in agents/hermes/mcp-config-transaction.py for the IPv6 literal rejection error — it reads 'IPv6-literal MCP URLs are not supported'.
  • Missing regression test: Add test in test/hermes-mcp-config-transaction.test.ts that verifies the exact error message for IPv6 literal rejection matches TypeScript clarity.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check line ~297 in agents/hermes/mcp-config-transaction.py for the IPv6 literal rejection error — it reads 'IPv6-literal MCP URLs are not supported'.
  • Evidence: In _validate_payload, the check `if ':' in hostname:` raises ValueError('IPv6-literal MCP URLs are not supported') at approximately line 297.

PRA-15 Resolve/justify — Credential env name validation uses deny-list; may miss future OpenShell keys

  • Location: src/lib/actions/sandbox/mcp-bridge-validation.ts:90
  • Category: security
  • Problem: Credential env name validation uses deny-list (checks against runtimeControlKeys, runtimeControlPrefixes, rawChildValueKeys, rewrittenChildValueKeys). May miss future OpenShell keys added in newer versions. The deny-list is based on OpenShell v0.0.72 manifest.
  • Impact: If OpenShell 0.0.73+ introduces new runtime control keys or compatibility keys, they won't be in the deny-list and could be used as MCP credential names, potentially altering sandbox behavior.
  • Recommended action: Implement runtime OpenShell version check (PRA-5/6/7) as primary defense. Additionally, consider allow-list pattern for credential names (e.g., require *_MCP_TOKEN suffix) as defense-in-depth. Document that manifest must be updated in lockstep with OpenShell upgrades.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for 'runtimeControlPrefixes' and 'SANDBOX_RUNTIME_CONTROL_ENV_PREFIXES' in mcp-bridge-validation.ts to see the deny-list approach.
  • Missing regression test: Add test in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts that simulates a future OpenShell version with new runtime control keys and verifies the runtime version check catches the mismatch.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for 'runtimeControlPrefixes' and 'SANDBOX_RUNTIME_CONTROL_ENV_PREFIXES' in mcp-bridge-validation.ts to see the deny-list approach.
  • Evidence: Lines 90-100 check name against OPENSHELL_RAW_CHILD_ENV_KEYS, OPENSHELL_REWRITTEN_CHILD_ENV_KEYS, SANDBOX_RUNTIME_CONTROL_ENV_KEYS, and SANDBOX_RUNTIME_CONTROL_ENV_PREFIXES — all derived from the static v0.0.72 manifest.

PRA-16 Resolve/justify — Branch validation workflow lacks explicit TRUST BOUNDARY documentation

  • Location: .github/workflows/e2e-branch-validation.yaml:1
  • Category: workflow
  • Problem: Branch validation workflow lacks explicit TRUST BOUNDARY documentation. The workflow provisions Brev instances, runs install from source, and has write permissions (checks:write, pull-requests:write) for reporting. No documentation of the trust boundary between the validation job (which receives secrets) and the reporting job.
  • Impact: Without documented trust boundaries, reviewers cannot verify that secrets are properly isolated from the reporting path. The validation job has access to BREV_API_KEY and NVIDIA_INFERENCE_API_KEY but the reporting job should not.
  • Recommended action: Add a 'TRUST BOUNDARY' section to the workflow header documenting: (1) validation job permissions are downgraded to contents:read, pull-requests:read; (2) report-pr job has checks:write, pull-requests:write but no checkout of target code; (3) branch is resolved from pr_number via gh CLI, not from caller-controlled input.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for 'TRUST BOUNDARY' in .github/workflows/e2e-branch-validation.yaml — no such section exists.
  • Missing regression test: No automated test — documentation review required.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for 'TRUST BOUNDARY' in .github/workflows/e2e-branch-validation.yaml — no such section exists.
  • Evidence: Workflow has permissions: contents: read, checks: write, pull-requests: write at top level. Validation job overrides to contents: read, pull-requests: read. Report-pr job has contents: read, checks: write, pull-requests: write. But no TRUST BOUNDARY section in header.

PRA-17 Resolve/justify — Four install-openshell.sh fallbacks lack source-of-truth review

  • Location: scripts/install-openshell.sh:1
  • Category: architecture
  • Problem: Four fallbacks in install-openshell.sh lack source-of-truth review per the rubric: (1) gh CLI → curl fallback (lines 725-727), (2) sudo install → user-local fallback (lines 784-797), (3) macOS VM driver auto-signing with sudo fallback (lines 523-584), (4) sha256sum → shasum fallback (select_sha_cmd function, lines 686-693). Each fallback handles an invalid state but doesn't document: what invalid state, where created, why source can't be fixed, regression test, and removal condition.
  • Impact: Each fallback is a localized workaround for an invalid state. Without source-of-truth review, it's unclear if the invalid state can be prevented at the source, whether the fallback introduces security risks, and when the workaround can be removed.
  • Recommended action: Document source-of-truth review for each fallback per the rubric, or refactor to make invalid states impossible at source (e.g., require gh auth upfront, require sudo for system install, remove dev-channel path).
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Review scripts/install-openshell.sh at the four fallback locations: gh CLI download (725-727), sudo install (784-797), macOS VM driver signing (523-584), select_sha_cmd function (686-693).
  • Missing regression test: For each fallback, add a test that triggers the fallback condition and verifies correct behavior, plus a test that proves the source invalid state cannot regress.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Review scripts/install-openshell.sh at the four fallback locations: gh CLI download (725-727), sudo install (784-797), macOS VM driver signing (523-584), select_sha_cmd function (686-693).
  • Evidence: Four distinct fallback patterns identified in install-openshell.sh with no accompanying source-of-truth review comments.

PRA-18 Resolve/justify — MCP URL validation duplicated between TypeScript and Python without CI parity gate

  • Location: src/lib/security/mcp-url-target.ts:1
  • Category: architecture
  • Problem: MCP URL validation logic duplicated between TypeScript (src/lib/security/mcp-url-target.ts, src/lib/actions/sandbox/mcp-bridge-url-validation.ts) and Python (agents/hermes/mcp-config-transaction.py). No mandatory CI gate enforces parity. The existing parity test (test/hermes-mcp-config-transaction.test.ts) should be mandatory in CI.
  • Impact: Divergence between TypeScript and Python validators could allow URLs accepted by one but rejected by the other, creating inconsistent security posture across agents.
  • Recommended action: Consolidate validation into a shared library (e.g., WASM module or JSON schema + codegen) or add a mandatory CI gate that runs both validators against a canonical test matrix and fails on divergence. The existing parity test should be mandatory in CI.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare URL validation logic in src/lib/security/mcp-url-target.ts and agents/hermes/mcp-config-transaction.py — significant duplication exists. The test test/hermes-mcp-config-transaction.test.ts has a 'keeps Hermes and host MCP URL rejection boundaries in parity' test but it's not marked as a required CI gate.
  • Missing regression test: Add a CI job that runs both validators against a shared test matrix and fails if any divergence is detected. Make test/hermes-mcp-config-transaction.test.ts mandatory in CI.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare URL validation logic in src/lib/security/mcp-url-target.ts and agents/hermes/mcp-config-transaction.py — significant duplication exists. The test test/hermes-mcp-config-transaction.test.ts has a 'keeps Hermes and host MCP URL rejection boundaries in parity' test but it's not marked as a required CI gate.
  • Evidence: Both implementations have similar but not identical validation: hostname normalization, IP blocking, host alias rejection, IPv6 literal rejection, reserved hostname checks, path canonicalization.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-19 Improvement — Add IPv6 literal rejection test coverage to TypeScript and Python test suites

  • Location: src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts:1
  • Category: tests
  • Problem: IPv6 literal rejection test coverage exists in TypeScript (mcp-url-target.test.ts) but not in the MCP bridge input validation test suite or in the Python test suite.
  • Impact: No test verifies that the MCP bridge CLI and Hermes transaction module correctly reject IPv6 literals (private, loopback, mapped IPv4, documentation, multicast).
  • Suggested action: Add IPv6 literal rejection test cases to src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts and test/hermes-mcp-config-transaction.test.ts covering: private IPv6 (fc00::/7, fe80::/10), loopback (::1), mapped IPv4 (::ffff:127.0.0.1), documentation (2001:db8::/32), and multicast (ff00::/8).
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Search for 'IPv6' or 'ipv6' in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts and test/hermes-mcp-config-transaction.test.ts — no IPv6 literal tests exist in either.
  • Missing regression test: Add the IPv6 literal rejection test cases described above to both test files.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/mcp-url-target.test.ts has IPv6 tests but mcp-bridge-input-validation.test.ts and hermes-mcp-config-transaction.test.ts do not.

PRA-20 Improvement — Extract policy-channel.ts into focused modules

  • Location: src/lib/actions/sandbox/policy-channel.ts:1
  • Category: architecture
  • Problem: policy-channel.ts is a 1586-line monolith handling policy add, remove, list, and messaging channel mutations.
  • Impact: Large monolith makes security review and maintenance difficult. Changes to one operation risk breaking others.
  • Suggested action: Extract into focused modules: policy-channel-add.ts, policy-channel-remove.ts, policy-channel-list.ts, policy-channel-messaging.ts. Share common types via policy-channel-types.ts.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run `wc -l src/lib/actions/sandbox/policy-channel.ts` and grep for 'export async function' to see all public functions.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after extraction.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: File is 1586 lines with multiple exported async functions for add, remove, list, and messaging operations.

PRA-21 Improvement — Extract MCP preflight logic from preflight.ts monolith

  • Location: src/lib/onboard/preflight.ts:1
  • Category: architecture
  • Problem: preflight.ts is a 2370-line monolith containing MCP preflight logic mixed with port probing, DNS probes, swap management, CDI assessment, and host remediation.
  • Impact: MCP logic is scattered across a massive file, making it hard to audit and maintain. Unrelated changes risk breaking MCP preflight.
  • Suggested action: Extract MCP preflight logic to preflight-mcp.ts. Extract port/DNS probes to preflight-network.ts. Extract swap/CDI to preflight-resources.ts. Keep preflight.ts as a thin orchestrator.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run `wc -l src/lib/onboard/preflight.ts` and grep for 'mcp\|MCP' to find scattered MCP logic.
  • Missing regression test: No specific test needed — refactoring verified by all existing preflight tests passing after extraction.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: File is 2370 lines with grep showing MCP logic interspersed with network, resource, and host assessment code.

PRA-22 Improvement — Split rebuild-resume-config.test.ts into focused modules

  • Location: src/lib/actions/sandbox/rebuild-resume-config.test.ts:1
  • Category: architecture
  • Problem: Test file is 587 lines covering compatible endpoint, local, credential, and resume config scenarios.
  • Impact: Large test file mixes unrelated scenarios, making it hard to find and run specific tests.
  • Suggested action: Split into rebuild-resume-config-compatible-endpoint.test.ts, rebuild-resume-config-local.test.ts, rebuild-resume-config-credential.test.ts, or move shared helpers to test/helpers/rebuild-resume-config-support.ts.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run `wc -l src/lib/actions/sandbox/rebuild-resume-config.test.ts` and review describe blocks for logical groupings.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after split.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: File is 587 lines with 20 named test blocks covering distinct scenarios.

PRA-23 Improvement — Split sandbox.test.ts into focused modules

  • Location: src/lib/onboard/machine/handlers/sandbox.test.ts:1
  • Category: architecture
  • Problem: Test file is 1013 lines covering sandbox state handling with various channel, GPU, agent, and messaging configurations.
  • Impact: Large test file mixes unrelated configuration scenarios, making maintenance difficult.
  • Suggested action: Split into focused modules: sandbox-handler-channels.test.ts, sandbox-handler-gpu.test.ts, sandbox-handler-agent.test.ts, sandbox-handler-messaging.test.ts. Move shared fixtures to test/helpers/sandbox-handler-support.ts.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run `wc -l src/lib/onboard/machine/handlers/sandbox.test.ts` and review describe blocks for logical groupings.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after split.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: File is 1013 lines with tests for multiple agent types, channel configs, GPU modes, and messaging setups.

PRA-24 Improvement — Thin wrapper adapter field could be inlined

  • Location: agents/hermes/manifest.yaml:126
  • Category: correctness
  • Problem: The MCP adapter field 'adapter: hermes-config' is a thin wrapper — the adapter name matches the agent's config mutation approach (Python transaction module) and could be inlined in the adapter registration code rather than declared in the manifest.
  • Impact: Manifest declares an adapter field that must stay in sync with the adapter registration code, creating a configuration surface that can drift.
  • Suggested action: Consider inlining the adapter type in the MCP bridge adapter registration (src/lib/actions/sandbox/mcp-bridge-adapters.ts) rather than reading from manifest. This removes a configuration surface that must stay in sync.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check agents/hermes/manifest.yaml line 126 and src/lib/actions/sandbox/mcp-bridge-adapters.ts for how the adapter field is consumed.
  • Missing regression test: No specific test needed — refactoring verified by existing MCP bridge adapter tests.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: manifest.yaml has 'adapter: hermes-config' under mcp section. mcp-bridge-adapters.ts likely reads this to determine which adapter to use.

PRA-25 Improvement — Single-use config constant could be simplified

  • Location: .github/workflows/e2e.yaml:635
  • Category: correctness
  • Problem: Single-use config constant NEMOCLAW_OPENSHELL_CHANNEL: stable (line 542) and dev (line 640) used only in the two mcp-bridge jobs. Could be inlined as a job-level env var rather than a matrix-level constant.
  • Impact: Unnecessary configuration abstraction for a value used in only two jobs.
  • Suggested action: Inline the NEMOCLAW_OPENSHELL_CHANNEL value directly in each job's env block rather than as a separate constant. This reduces configuration surface and makes the job self-contained.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check .github/workflows/e2e.yaml lines 542 and 640 — NEMOCLAW_OPENSHELL_CHANNEL is only used in the mcp-bridge and mcp-bridge-dev jobs.
  • Missing regression test: No specific test needed — refactoring verified by workflow running correctly.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Matrix defines NEMOCLAW_OPENSHELL_CHANNEL but only mcp-bridge and mcp-bridge-dev jobs reference it.
Simplification opportunities: 10 possible cuts, net -907 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-8 shrink (src/lib/onboard/preflight.ts:1): Extract MCP preflight logic to src/lib/onboard/preflight-mcp.ts
    • Replacement: Import and call preflight-mcp functions from preflight.ts
    • Net: -200 lines
    • Safety boundary: Must not change preflight check behavior or error messages; only reorganize code
  • PRA-9 shrink (src/lib/actions/sandbox/rebuild-resume-config.test.ts:1): Split test file into 3-4 focused files by scenario
    • Replacement: Shared helpers in test/helpers/rebuild-resume-config-support.ts
    • Net: 0 lines
    • Safety boundary: All existing test cases must pass; no behavior change
  • PRA-10 shrink (src/lib/onboard/machine/handlers/sandbox.test.ts:1): Split test file into 4 focused files by configuration type
    • Replacement: Shared fixtures in test/helpers/sandbox-handler-support.ts
    • Net: 0 lines
    • Safety boundary: All existing test cases must pass; no behavior change
  • PRA-11 shrink (src/lib/actions/sandbox/policy-channel.ts:1): Extract policy-channel-add.ts, policy-channel-remove.ts, policy-channel-list.ts, policy-channel-messaging.ts
    • Replacement: Shared types in policy-channel-types.ts
    • Net: -200 lines
    • Safety boundary: Must not change policy mutation semantics or error messages
  • PRA-20 shrink (src/lib/actions/sandbox/policy-channel.ts:1): Split policy-channel.ts into 4 operation-specific files
    • Replacement: Shared types in policy-channel-types.ts; thin facade in policy-channel.ts
    • Net: -200 lines
    • Safety boundary: Must not change policy mutation semantics, error messages, or CLI behavior
  • PRA-21 shrink (src/lib/onboard/preflight.ts:1): Extract MCP preflight to preflight-mcp.ts, network probes to preflight-network.ts, resources to preflight-resources.ts
    • Replacement: preflight.ts becomes thin orchestrator importing domain modules
    • Net: -300 lines
    • Safety boundary: Must not change preflight check behavior, error messages, or return values
  • PRA-22 shrink (src/lib/actions/sandbox/rebuild-resume-config.test.ts:1): Split into 3-4 scenario-specific test files
    • Replacement: Shared helpers in test/helpers/rebuild-resume-config-support.ts
    • Net: 0 lines
    • Safety boundary: All existing test cases must pass; no behavior change
  • PRA-23 shrink (src/lib/onboard/machine/handlers/sandbox.test.ts:1): Split into 4 configuration-type-specific test files
    • Replacement: Shared fixtures in test/helpers/sandbox-handler-support.ts
    • Net: 0 lines
    • Safety boundary: All existing test cases must pass; no behavior change
  • PRA-24 yagni (agents/hermes/manifest.yaml:126): Remove adapter field from manifest.yaml mcp section
    • Replacement: Hardcode 'hermes-config' in mcp-bridge-adapters.ts for Hermes agent
    • Net: -2 lines
    • Safety boundary: Must not affect adapter selection logic for other agents (OpenClaw, DeepAgents)
  • PRA-25 delete (.github/workflows/e2e.yaml:635): Remove NEMOCLAW_OPENSHELL_CHANNEL from matrix and inline 'stable'/'dev' in job env blocks
    • Replacement: Direct env: NEMOCLAW_OPENSHELL_CHANNEL: stable in mcp-bridge job, dev in mcp-bridge-dev job
    • Net: -5 lines
    • Safety boundary: Must not affect any other job; only these two jobs use this variable
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Add IPv6 literal rejection test coverage to TypeScript and Python test suites — Add IPv6 literal rejection test cases to src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts and test/hermes-mcp-config-transaction.test.ts covering: private IPv6 (fc00::/7, fe80::/10), loopback (::1), mapped IPv4 (::ffff:127.0.0.1), documentation (2001:db8::/32), and multicast (ff00::/8).
  • PRA-T2 Acceptance clause — stdio-to-HTTP proxy on host — add test evidence or identify existing coverage. PR description: 'explicitly supersedes the original host-side stdio-to-HTTP proxy'; accepted design uses native OpenShell protocol: mcp
  • PRA-T3 Acceptance clause — Proxy binds 127.0.0.1 only — add test evidence or identify existing coverage. No host proxy; sandbox connects directly via OpenShell egress
  • PRA-T4 Acceptance clause — Runtime OpenShell version verification — add test evidence or identify existing coverage. No runtime openshell --version check in TypeScript validation, Python transaction, or Hermes Dockerfile entrypoint
  • PRA-T5 Acceptance clause — IPv6 literal rejection parity — add test evidence or identify existing coverage. TypeScript rejects with clear message; Python rejects with less clear message; no test coverage in either MCP bridge or Python tests
  • PRA-T6 scripts/install-openshell.sh:725-727 (gh CLI → curl fallback) — Test in test/install-openshell-version-check.test.ts that triggers gh auth failure and verifies curl fallback downloads correct asset with SHA256 verification. Lines 725-727: if ! gh release download ...; then curl -L ...; fi
  • PRA-T7 scripts/install-openshell.sh:784-797 (sudo install → user-local fallback) — Test that triggers sudo failure and verifies user-local install to ~/.local/bin works and is on PATH. Lines 784-797: if sudo install ...; then ... else install -m 755 ... ~/.local/bin/openshell; fi
  • PRA-T8 scripts/install-openshell.sh:523-584 (macOS VM driver auto-signing with sudo fallback) — Test on macOS that verifies unsigned VM driver triggers auto-signing and executes successfully. Lines 523-584: codesign --sign - --force ... with sudo fallback for entitlements
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: scripts/install-openshell.sh:725-727 (gh CLI → curl fallback)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test in test/install-openshell-version-check.test.ts that triggers gh auth failure and verifies curl fallback downloads correct asset with SHA256 verification
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Lines 725-727: if ! gh release download ...; then curl -L ...; fi

PRA-2 Resolve/justify — Source-of-truth review needed: scripts/install-openshell.sh:784-797 (sudo install → user-local fallback)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test that triggers sudo failure and verifies user-local install to ~/.local/bin works and is on PATH
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Lines 784-797: if sudo install ...; then ... else install -m 755 ... ~/.local/bin/openshell; fi

PRA-3 Resolve/justify — Source-of-truth review needed: scripts/install-openshell.sh:523-584 (macOS VM driver auto-signing with sudo fallback)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test on macOS that verifies unsigned VM driver triggers auto-signing and executes successfully
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Lines 523-584: codesign --sign - --force ... with sudo fallback for entitlements

PRA-4 Required — Credential boundary manifest lacks runtime OpenShell version verification (TypeScript)

  • Location: src/lib/actions/sandbox/mcp-bridge-validation.ts:90
  • Category: security
  • Problem: The TypeScript validation loads the credential boundary manifest (openshell-child-visible-credentials.v0.0.72.json) at import time and validates against its deny-lists (runtimeControlKeys, runtimeControlPrefixes, rawChildValueKeys, rewrittenChildValueKeys), but never executes `openshell --version` at runtime to verify the running OpenShell matches the manifest version baked into the image.
  • Impact: If OpenShell is upgraded/downgraded in the sandbox (e.g., via install-openshell.sh or manual upgrade), the deny-list becomes stale. Reserved credential names from the new OpenShell version would be accepted, or legitimate names from the old version would be rejected, breaking the credential isolation boundary.
  • Required action: Add `openshell --version` runtime check in mcp-bridge-validation.ts (or a shared preflight module) that parses the output and compares against manifest['openshellVersion']. Fail closed on mismatch, missing binary, or parse error. Consider caching the version check with TTL to avoid repeated subprocess calls.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for 'openshell --version' or 'subprocess' or 'spawn' in src/lib/actions/sandbox/mcp-bridge-validation.ts and src/lib/security/mcp-url-target.ts — no runtime version check exists. The manifest is imported at line 12 and deny-lists derived at lines 90-100.
  • Missing regression test: Add test in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts that mocks openshell --version returning a different version (e.g., 0.0.73) and verifies the validation fails closed when a reserved name from the new version is used.
  • Done when: The required change is committed and verification passes: Search for 'openshell --version' or 'subprocess' or 'spawn' in src/lib/actions/sandbox/mcp-bridge-validation.ts and src/lib/security/mcp-url-target.ts — no runtime version check exists. The manifest is imported at line 12 and deny-lists derived at lines 90-100.
  • Evidence: Lines 12-100 import manifest and build deny-lists from manifest.runtimeControlKeys, manifest.runtimeControlPrefixes, manifest.rawChildValueKeys, manifest.rewrittenChildValueKeys. manifest.openshellVersion is '0.0.72' but never verified at runtime.

PRA-5 Required — Credential boundary manifest lacks runtime OpenShell version verification (Python)

  • Location: agents/hermes/mcp-config-transaction.py:136
  • Category: security
  • Problem: The Python transaction module loads the credential boundary manifest at module import time and validates its openshellVersion field, but never executes `openshell --version` at runtime to verify the running OpenShell matches. The manifest is loaded from a file baked into the image, which can diverge from the actual runtime OpenShell version.
  • Impact: If OpenShell is upgraded/downgraded in the Hermes sandbox (e.g., via install-openshell.sh or manual upgrade), the Python transaction's deny-list becomes stale. Reserved credential names from the new OpenShell version would be accepted, or legitimate names from the old version would be rejected.
  • Required action: Add `subprocess.run(['openshell', '--version'], capture_output=True, text=True)` in `_load_credential_boundary_manifest()` and compare parsed version against manifest['openshellVersion']. Fail closed on mismatch, missing binary, or parse error.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for 'openshell --version' or 'subprocess.run' in agents/hermes/mcp-config-transaction.py — no runtime version check exists. The manifest is loaded at line 136 via `_load_credential_boundary_manifest()` called at module level.
  • Missing regression test: Add test in test/hermes-mcp-config-transaction.test.ts that mocks openshell --version returning a different version and verifies the validation fails closed when a reserved name from the new version is used.
  • Done when: The required change is committed and verification passes: Search for 'openshell --version' or 'subprocess.run' in agents/hermes/mcp-config-transaction.py — no runtime version check exists. The manifest is loaded at line 136 via `_load_credential_boundary_manifest()` called at module level.
  • Evidence: _load_credential_boundary_manifest() at line 136 loads manifest from file and checks manifest.get('openshellVersion') == '0.0.72' but never runs openshell --version.

PRA-6 Required — Hermes Dockerfile lacks runtime OpenShell version verification

  • Location: agents/hermes/Dockerfile:31
  • Category: security
  • Problem: Hermes Dockerfile has a build-time import guard for MCP client availability (`import mcp; from tools import mcp_tool`), but no runtime OpenShell version check in the entrypoint (nemoclaw-start) or in hermes-mcp-config-transaction.py that compares `openshell --version` against the manifest version baked into the image.
  • Impact: A Hermes sandbox image built with the correct manifest could be started with a different OpenShell version at runtime, causing the credential boundary deny-list to be out of sync with the actual OpenShell behavior.
  • Required action: Add runtime OpenShell version check in Hermes entrypoint (nemoclaw-start) or in hermes-mcp-config-transaction.py that compares `openshell --version` against the manifest version baked into the image. Fail closed on mismatch.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check agents/hermes/Dockerfile and agents/hermes/start.sh for any `openshell --version` runtime verification — none exists. The build-time check at line 31 only validates MCP client imports.
  • Missing regression test: Add E2E test in test/e2e/live/rebuild-hermes.test.ts that runs Hermes sandbox with a mismatched OpenShell version (e.g., by overriding the OpenShell binary) and verifies startup fails.
  • Done when: The required change is committed and verification passes: Check agents/hermes/Dockerfile and agents/hermes/start.sh for any `openshell --version` runtime verification — none exists. The build-time check at line 31 only validates MCP client imports.
  • Evidence: Dockerfile line 31 has RUN /opt/hermes/.venv/bin/python -c 'import mcp; from tools import mcp_tool; assert getattr(mcp_tool, "_MCP_AVAILABLE", False)...' but no runtime openshell --version check.

PRA-7 Required — Missing startup reconciliation for mcp_servers config drift in Hermes

  • Location: agents/hermes/start.sh:1
  • Category: correctness
  • Problem: No startup reconciliation for mcp_servers config drift in Hermes. The config hash verification (STRICT_HASH_PATH) only covers config.yaml and .env, not the mcp_servers section. If mcp_servers config is modified at runtime (via the Python transaction module), the gateway reload may not pick up changes or may restart with stale config.
  • Impact: MCP servers added/removed via `nemoclaw mcp add|remove` may not be reflected in the running gateway after a restart/rebuild, leading to silent misconfiguration where the registry shows one set of servers but the gateway serves another.
  • Required action: Add startup reconciliation in start.sh or runtime-config-guard.py that detects config/hash drift against running gateway state for mcp_servers and either auto-reloads or fails closed with clear guidance. Extend hash verification (STRICT_HASH_PATH) to cover mcp_servers config.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for 'mcp_servers' in agents/hermes/start.sh and agents/hermes/runtime-config-guard.py — no drift detection for mcp_servers exists. The hash verification only covers config.yaml and .env.
  • Missing regression test: Add test in test/hermes-mcp-config-transaction.test.ts that modifies mcp_servers config at runtime (via the transaction module) and verifies startup detects the drift and either reloads or fails with actionable error.
  • Done when: The required change is committed and verification passes: Search for 'mcp_servers' in agents/hermes/start.sh and agents/hermes/runtime-config-guard.py — no drift detection for mcp_servers exists. The hash verification only covers config.yaml and .env.
  • Evidence: start.sh uses verify_config_integrity which calls verify_config_integrity from sandbox-init.sh — that function only hashes config.yaml and .env. No mcp_servers coverage.

PRA-8 Required — preflight.ts monolith growth exceeds threshold (+56 lines, now 2370)

  • Location: src/lib/onboard/preflight.ts:1
  • Category: architecture
  • Problem: preflight.ts monolith grew by 56 lines (now 2370 lines). The file contains MCP-related preflight logic mixed with port probing, DNS probes, swap management, CDI assessment, and host remediation. This violates single-responsibility and makes security review difficult.
  • Impact: MCP logic is scattered across a 2370-line file, making it hard to audit security-critical preflight checks. Changes to unrelated preflight areas risk breaking MCP validation.
  • Required action: Extract MCP-related preflight logic to separate module (e.g., preflight-mcp.ts) or split preflight.ts into smaller domain-focused modules. At minimum, offset the growth by extracting existing unrelated logic.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/onboard/preflight.ts` — shows 2370 lines. Search for 'mcp' or 'MCP' in the file to find scattered MCP logic.
  • Missing regression test: No specific test needed — architectural refactoring verified by existing preflight tests passing after extraction.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/onboard/preflight.ts` — shows 2370 lines. Search for 'mcp' or 'MCP' in the file to find scattered MCP logic.
  • Evidence: Monolith delta from drift context: +56 lines, severity blocker. File contains checkContainerRuntimeResources, assessHost, probePortAvailability, DNS probes, swap, CDI, MCP preflight all in one file.

PRA-9 Required — Test monolith growth: rebuild-resume-config.test.ts grew by 119 lines (now 587)

  • Location: src/lib/actions/sandbox/rebuild-resume-config.test.ts:1
  • Category: architecture
  • Problem: Test monolith grew by 119 lines (now 587 lines). Contains tests for compatible endpoint, local, credential, and resume config scenarios all in one file.
  • Impact: Large test file is hard to maintain and review. Related tests are mixed with unrelated scenarios.
  • Required action: Split into rebuild-resume-config-compatible-endpoint.test.ts, rebuild-resume-config-local.test.ts, rebuild-resume-config-credential.test.ts, or move shared helpers to test/helpers/rebuild-resume-config-support.ts.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/actions/sandbox/rebuild-resume-config.test.ts` — shows 587 lines. Review describe blocks for logical groupings.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after split.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/actions/sandbox/rebuild-resume-config.test.ts` — shows 587 lines. Review describe blocks for logical groupings.
  • Evidence: Monolith delta from drift context: +119 lines, severity blocker.

PRA-10 Required — Test monolith growth: sandbox.test.ts grew by 98 lines (now 1013)

  • Location: src/lib/onboard/machine/handlers/sandbox.test.ts:1
  • Category: architecture
  • Problem: Test monolith grew by 98 lines (now 1013 lines). Contains tests for sandbox state handling with various channel, GPU, agent, and messaging configurations.
  • Impact: Large test file is hard to maintain. Related tests for different configurations are mixed together.
  • Required action: Split into focused modules: sandbox-handler-channels.test.ts, sandbox-handler-gpu.test.ts, sandbox-handler-agent.test.ts, sandbox-handler-messaging.test.ts. Move shared fixtures to test/helpers/sandbox-handler-support.ts.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/onboard/machine/handlers/sandbox.test.ts` — shows 1013 lines.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after split.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/onboard/machine/handlers/sandbox.test.ts` — shows 1013 lines.
  • Evidence: Monolith delta from drift context: +98 lines, severity blocker.

PRA-11 Required — policy-channel.ts monolith growth exceeds threshold (+39 lines, now 1586)

  • Location: src/lib/actions/sandbox/policy-channel.ts:1
  • Category: architecture
  • Problem: policy-channel.ts monolith grew by 39 lines (now 1586 lines). Handles policy add, remove, list, and messaging channel mutations in a single file.
  • Impact: Policy channel logic is a 1586-line monolith mixing multiple mutation types. Hard to audit security boundaries for each operation.
  • Required action: Extract focused policy channel logic to separate modules (e.g., policy-channel-add.ts, policy-channel-remove.ts, policy-channel-list.ts, policy-channel-messaging.ts) before merge.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/actions/sandbox/policy-channel.ts` — shows 1586 lines. grep for 'export async function' to see all public functions.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after extraction.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/actions/sandbox/policy-channel.ts` — shows 1586 lines. grep for 'export async function' to see all public functions.
  • Evidence: Monolith delta from drift context: +39 lines, severity blocker.

PRA-12 Required — New Python transaction module needs focused security review

  • Location: agents/hermes/mcp-config-transaction.py:1
  • Category: security
  • Problem: New Python transaction module (969 lines) handles Hermes MCP config mutation and gateway reload control. It performs YAML deserialization, URL validation, credential name validation, file locking, atomic writes, and gateway process management. Requires focused security review for: YAML deserialization attacks, command injection via server names, path traversal in config paths, privilege separation bypasses, and TOCTOU races in config mutation.
  • Impact: This module is the sole writer of Hermes mcp_servers config and controls gateway reload. A vulnerability could allow config tampering, privilege escalation, or gateway takeover.
  • Required action: Conduct focused security review of mcp-config-transaction.py. Ensure test coverage includes negative cases for YAML deserialization attacks, command injection via server names, path traversal in config paths, and privilege separation bypasses. Verify yaml.safe_load usage, path validation under /sandbox/.hermes, file locking (fcntl), and subprocess call safety.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Review agents/hermes/mcp-config-transaction.py for: yaml.safe_load usage (line ~400), path validation (CONFIG_PATH constant), file locking (fcntl in guard), subprocess calls (os.kill for SIGUSR1), and credential handling (never logs raw values).
  • Missing regression test: Add negative test cases in test/hermes-mcp-config-transaction.test.ts for: malicious YAML payloads (e.g., !!python/object/apply:os.system), server names with path traversal (../../../etc/passwd), concurrent mutation attempts, and privilege escalation via config manipulation.
  • Done when: The required change is committed and verification passes: Review agents/hermes/mcp-config-transaction.py for: yaml.safe_load usage (line ~400), path validation (CONFIG_PATH constant), file locking (fcntl in guard), subprocess calls (os.kill for SIGUSR1), and credential handling (never logs raw values).
  • Evidence: New file agents/hermes/mcp-config-transaction.py (969 lines) added in this PR. Uses yaml.safe_load, fcntl locking via hermes-runtime-config-guard.py, validates server names with SERVER_NAME_RE, verifies mutable snapshot ownership.

PRA-13 Resolve/justify — IPv6 literal rejection error message could be clearer

  • Location: src/lib/actions/sandbox/mcp-bridge-url-validation.ts:83
  • Category: security
  • Problem: IPv6 literal rejection error message: 'IPv6-literal MCP server URLs are not supported by the current OpenShell proxy target parser. Use a DNS hostname with public A/AAAA records.' This is clear but could explicitly mention bracketed address format (e.g., [::1]).
  • Impact: Operators attempting to use IPv6 literals get a rejection but may not understand the bracketed format is the issue, leading to confusion.
  • Recommended action: Keep current TypeScript message as baseline. Ensure Python message matches clarity. Document and test DNS AAAA resolution parity.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check the error message at line 83 in mcp-bridge-url-validation.ts — it rejects IPv6 literals with the message shown.
  • Missing regression test: Add test in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts that verifies the exact error message for IPv6 literal rejection includes 'bracketed addresses like [::1]'.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check the error message at line 83 in mcp-bridge-url-validation.ts — it rejects IPv6 literals with the message shown.
  • Evidence: Line 83 throws McpBridgeError with message 'IPv6-literal MCP server URLs are not supported by the current OpenShell proxy target parser. Use a DNS hostname with public A/AAAA records.'

PRA-14 Resolve/justify — Python IPv6 literal rejection error message less clear than TypeScript

  • Location: agents/hermes/mcp-config-transaction.py:297
  • Category: security
  • Problem: Python IPv6 literal rejection error message: 'IPv6-literal MCP URLs are not supported' — less clear than TypeScript version. Doesn't mention bracketed format or DNS alternative.
  • Impact: Hermes operators using the Python transaction directly get a less actionable error message, potentially causing confusion about why IPv6 literals are rejected.
  • Recommended action: Update Python error message to match TypeScript: 'IPv6-literal MCP server URLs (bracketed addresses like [::1]) are not supported by the current OpenShell proxy target parser. Use a DNS hostname with public A/AAAA records.' Document and test DNS AAAA resolution parity.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check line ~297 in agents/hermes/mcp-config-transaction.py for the IPv6 literal rejection error — it reads 'IPv6-literal MCP URLs are not supported'.
  • Missing regression test: Add test in test/hermes-mcp-config-transaction.test.ts that verifies the exact error message for IPv6 literal rejection matches TypeScript clarity.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check line ~297 in agents/hermes/mcp-config-transaction.py for the IPv6 literal rejection error — it reads 'IPv6-literal MCP URLs are not supported'.
  • Evidence: In _validate_payload, the check `if ':' in hostname:` raises ValueError('IPv6-literal MCP URLs are not supported') at approximately line 297.

PRA-15 Resolve/justify — Credential env name validation uses deny-list; may miss future OpenShell keys

  • Location: src/lib/actions/sandbox/mcp-bridge-validation.ts:90
  • Category: security
  • Problem: Credential env name validation uses deny-list (checks against runtimeControlKeys, runtimeControlPrefixes, rawChildValueKeys, rewrittenChildValueKeys). May miss future OpenShell keys added in newer versions. The deny-list is based on OpenShell v0.0.72 manifest.
  • Impact: If OpenShell 0.0.73+ introduces new runtime control keys or compatibility keys, they won't be in the deny-list and could be used as MCP credential names, potentially altering sandbox behavior.
  • Recommended action: Implement runtime OpenShell version check (PRA-5/6/7) as primary defense. Additionally, consider allow-list pattern for credential names (e.g., require *_MCP_TOKEN suffix) as defense-in-depth. Document that manifest must be updated in lockstep with OpenShell upgrades.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for 'runtimeControlPrefixes' and 'SANDBOX_RUNTIME_CONTROL_ENV_PREFIXES' in mcp-bridge-validation.ts to see the deny-list approach.
  • Missing regression test: Add test in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts that simulates a future OpenShell version with new runtime control keys and verifies the runtime version check catches the mismatch.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for 'runtimeControlPrefixes' and 'SANDBOX_RUNTIME_CONTROL_ENV_PREFIXES' in mcp-bridge-validation.ts to see the deny-list approach.
  • Evidence: Lines 90-100 check name against OPENSHELL_RAW_CHILD_ENV_KEYS, OPENSHELL_REWRITTEN_CHILD_ENV_KEYS, SANDBOX_RUNTIME_CONTROL_ENV_KEYS, and SANDBOX_RUNTIME_CONTROL_ENV_PREFIXES — all derived from the static v0.0.72 manifest.

PRA-16 Resolve/justify — Branch validation workflow lacks explicit TRUST BOUNDARY documentation

  • Location: .github/workflows/e2e-branch-validation.yaml:1
  • Category: workflow
  • Problem: Branch validation workflow lacks explicit TRUST BOUNDARY documentation. The workflow provisions Brev instances, runs install from source, and has write permissions (checks:write, pull-requests:write) for reporting. No documentation of the trust boundary between the validation job (which receives secrets) and the reporting job.
  • Impact: Without documented trust boundaries, reviewers cannot verify that secrets are properly isolated from the reporting path. The validation job has access to BREV_API_KEY and NVIDIA_INFERENCE_API_KEY but the reporting job should not.
  • Recommended action: Add a 'TRUST BOUNDARY' section to the workflow header documenting: (1) validation job permissions are downgraded to contents:read, pull-requests:read; (2) report-pr job has checks:write, pull-requests:write but no checkout of target code; (3) branch is resolved from pr_number via gh CLI, not from caller-controlled input.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for 'TRUST BOUNDARY' in .github/workflows/e2e-branch-validation.yaml — no such section exists.
  • Missing regression test: No automated test — documentation review required.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for 'TRUST BOUNDARY' in .github/workflows/e2e-branch-validation.yaml — no such section exists.
  • Evidence: Workflow has permissions: contents: read, checks: write, pull-requests: write at top level. Validation job overrides to contents: read, pull-requests: read. Report-pr job has contents: read, checks: write, pull-requests: write. But no TRUST BOUNDARY section in header.

PRA-17 Resolve/justify — Four install-openshell.sh fallbacks lack source-of-truth review

  • Location: scripts/install-openshell.sh:1
  • Category: architecture
  • Problem: Four fallbacks in install-openshell.sh lack source-of-truth review per the rubric: (1) gh CLI → curl fallback (lines 725-727), (2) sudo install → user-local fallback (lines 784-797), (3) macOS VM driver auto-signing with sudo fallback (lines 523-584), (4) sha256sum → shasum fallback (select_sha_cmd function, lines 686-693). Each fallback handles an invalid state but doesn't document: what invalid state, where created, why source can't be fixed, regression test, and removal condition.
  • Impact: Each fallback is a localized workaround for an invalid state. Without source-of-truth review, it's unclear if the invalid state can be prevented at the source, whether the fallback introduces security risks, and when the workaround can be removed.
  • Recommended action: Document source-of-truth review for each fallback per the rubric, or refactor to make invalid states impossible at source (e.g., require gh auth upfront, require sudo for system install, remove dev-channel path).
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Review scripts/install-openshell.sh at the four fallback locations: gh CLI download (725-727), sudo install (784-797), macOS VM driver signing (523-584), select_sha_cmd function (686-693).
  • Missing regression test: For each fallback, add a test that triggers the fallback condition and verifies correct behavior, plus a test that proves the source invalid state cannot regress.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Review scripts/install-openshell.sh at the four fallback locations: gh CLI download (725-727), sudo install (784-797), macOS VM driver signing (523-584), select_sha_cmd function (686-693).
  • Evidence: Four distinct fallback patterns identified in install-openshell.sh with no accompanying source-of-truth review comments.

PRA-18 Resolve/justify — MCP URL validation duplicated between TypeScript and Python without CI parity gate

  • Location: src/lib/security/mcp-url-target.ts:1
  • Category: architecture
  • Problem: MCP URL validation logic duplicated between TypeScript (src/lib/security/mcp-url-target.ts, src/lib/actions/sandbox/mcp-bridge-url-validation.ts) and Python (agents/hermes/mcp-config-transaction.py). No mandatory CI gate enforces parity. The existing parity test (test/hermes-mcp-config-transaction.test.ts) should be mandatory in CI.
  • Impact: Divergence between TypeScript and Python validators could allow URLs accepted by one but rejected by the other, creating inconsistent security posture across agents.
  • Recommended action: Consolidate validation into a shared library (e.g., WASM module or JSON schema + codegen) or add a mandatory CI gate that runs both validators against a canonical test matrix and fails on divergence. The existing parity test should be mandatory in CI.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare URL validation logic in src/lib/security/mcp-url-target.ts and agents/hermes/mcp-config-transaction.py — significant duplication exists. The test test/hermes-mcp-config-transaction.test.ts has a 'keeps Hermes and host MCP URL rejection boundaries in parity' test but it's not marked as a required CI gate.
  • Missing regression test: Add a CI job that runs both validators against a shared test matrix and fails if any divergence is detected. Make test/hermes-mcp-config-transaction.test.ts mandatory in CI.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare URL validation logic in src/lib/security/mcp-url-target.ts and agents/hermes/mcp-config-transaction.py — significant duplication exists. The test test/hermes-mcp-config-transaction.test.ts has a 'keeps Hermes and host MCP URL rejection boundaries in parity' test but it's not marked as a required CI gate.
  • Evidence: Both implementations have similar but not identical validation: hostname normalization, IP blocking, host alias rejection, IPv6 literal rejection, reserved hostname checks, path canonicalization.

PRA-19 Improvement — Add IPv6 literal rejection test coverage to TypeScript and Python test suites

  • Location: src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts:1
  • Category: tests
  • Problem: IPv6 literal rejection test coverage exists in TypeScript (mcp-url-target.test.ts) but not in the MCP bridge input validation test suite or in the Python test suite.
  • Impact: No test verifies that the MCP bridge CLI and Hermes transaction module correctly reject IPv6 literals (private, loopback, mapped IPv4, documentation, multicast).
  • Suggested action: Add IPv6 literal rejection test cases to src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts and test/hermes-mcp-config-transaction.test.ts covering: private IPv6 (fc00::/7, fe80::/10), loopback (::1), mapped IPv4 (::ffff:127.0.0.1), documentation (2001:db8::/32), and multicast (ff00::/8).
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Search for 'IPv6' or 'ipv6' in src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts and test/hermes-mcp-config-transaction.test.ts — no IPv6 literal tests exist in either.
  • Missing regression test: Add the IPv6 literal rejection test cases described above to both test files.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/mcp-url-target.test.ts has IPv6 tests but mcp-bridge-input-validation.test.ts and hermes-mcp-config-transaction.test.ts do not.

PRA-20 Improvement — Extract policy-channel.ts into focused modules

  • Location: src/lib/actions/sandbox/policy-channel.ts:1
  • Category: architecture
  • Problem: policy-channel.ts is a 1586-line monolith handling policy add, remove, list, and messaging channel mutations.
  • Impact: Large monolith makes security review and maintenance difficult. Changes to one operation risk breaking others.
  • Suggested action: Extract into focused modules: policy-channel-add.ts, policy-channel-remove.ts, policy-channel-list.ts, policy-channel-messaging.ts. Share common types via policy-channel-types.ts.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run `wc -l src/lib/actions/sandbox/policy-channel.ts` and grep for 'export async function' to see all public functions.
  • Missing regression test: No specific test needed — refactoring verified by all existing tests passing after extraction.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: File is 1586 lines with multiple exported async functions for add, remove, list, and messaging operations.

Workflow run details

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.

Comment thread test/e2e/test-openshell-version-pin.sh Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/onboard/openshell-install.ts (1)

162-174: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Mirror the new MCP-L7 feature gate in onboarding.

Line 162 raises the fallback floor to 0.0.72, but needsUpgrade still only checks version, dev-channel, and Docker-driver binaries. An already-installed 0.0.72 binary that lacks allow_all_known_mcp_methods will pass this path even though scripts/install-openshell.sh now rejects it, so onboarding can continue with an unsupported OpenShell build. Please add the same capability check before you skip reinstall/failure.

🤖 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 `@src/lib/onboard/openshell-install.ts` around lines 162 - 174, The onboarding
upgrade check in openshell-install.ts is missing the new MCP-L7 capability gate,
so an installed OpenShell at the minimum version can still be treated as valid
even if it lacks allow_all_known_mcp_methods. Update the needsUpgrade logic in
the openshell install flow to also verify the same capability check used by
scripts/install-openshell.sh, and treat the install as needing upgrade/failure
when that feature is absent. Use the existing currentVersionOutput/version
checks and the relevant helpers in openshell-install.ts so the new gate is
applied before skipping reinstall.
🧹 Nitpick comments (2)
schemas/policy-preset.schema.json (1)

51-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add focused schema tests for the new MCP and JSON-RPC branches.

The provided schema tests only show compile coverage plus a legacy rest negative case, so these new json-rpc/mcp/deny_rules paths can regress without a precise failure. Please add explicit accept/reject cases for the new protocol values, matcher shapes, and option objects.

Also applies to: 68-72, 86-144

🤖 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 `@schemas/policy-preset.schema.json` around lines 51 - 56, Add focused schema
validation tests for the new policy-preset branches so regressions are caught
directly: update the schema test coverage around the protocol enum and the $defs
used by jsonRpcOptions, mcpOptions, and denyRulesMatcher to include explicit
accept/reject cases for json-rpc and mcp values, their expected matcher shapes,
and the associated option objects. Keep the existing compile/legacy rest
coverage, but add targeted positive and negative assertions that exercise the
new branches in policy-preset.schema.json rather than relying on broad
compile-only checks.
test/fetch-guard-patch-regression.test.ts (1)

135-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the base-image mcporter pin too.

This helper only reads Dockerfile, so the new regression still passes if Dockerfile.base drifts. Since the base image now also bakes mcporter, please compare both files here (or source the version from one place) so the test guards the cross-file contract instead of just the runtime layer.

🤖 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/fetch-guard-patch-regression.test.ts` around lines 135 - 141, The
regression helper only checks the mcporter version in Dockerfile, so it can miss
drift in Dockerfile.base; update readDockerfileMcporterVersion and the related
test assertion to validate the mcporter pin in both files or derive both from a
single shared source. Make sure the fetch-guard regression test covers the
cross-file contract introduced by the base-image mcporter baking, not just the
runtime layer.
🤖 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 `@Dockerfile`:
- Around line 149-153: The mcporter repair path still installs over the baked-in
copy without clearing the stale binary first, unlike the openclaw flow. Update
the mcporter handling in the Dockerfile so it uses the same pre-delete pattern
as openclaw before running npm install, using the mcporter version check block
to ensure stale base-image layers heal instead of failing on
overlayfs/atomic-move errors.

In `@docs/reference/commands-nemohermes.mdx`:
- Around line 1004-1050: The new `nemohermes <name> mcp
add|status|restart|remove` sections read like supported workflows, but Hermes
does not support MCP bridges, so update these docs to clearly mark the commands
as unsupported on Hermes. Keep the command names for reference, but remove the
OpenClaw bridge/proxy/policy/registry operational details and replace them with
a concise note that MCP bridge support is disabled for Hermes users.

In `@schemas/sandbox-policy.schema.json`:
- Around line 76-81: The endpoint schema currently allows json-rpc and mcp
protocols to validate without any L7 policy, so update the conditional
validation in sandbox-policy.schema.json to require the same allow/deny contract
for these new protocols as for rest and websocket. Adjust the relevant endpoint
schema logic around the protocol/access/rules validation, using the json_rpc and
mcp schema branches as the anchor, so json-rpc and mcp endpoints cannot omit
rules or equivalent L7 controls.

In `@src/lib/actions/sandbox/mcp-bridge.ts`:
- Around line 392-405: Make MCP port allocation atomic in
allocateMcpPort()/startProxy by ensuring the chosen port is reserved under the
same lock/critical section that records bridge state, so concurrent mcp add
calls cannot pick the same free port. Move the registry scan and port selection
into the protected reservation flow, or keep a temporary listener bound until
the proxy child has taken ownership, and keep using bridgeState/bridgePidFile
cleanup only after the reservation is secured.
- Around line 451-463: The restart flow in stopProxy()/restartMcpBridge() starts
the replacement proxy too soon after SIGTERM, which can leave 127.0.0.1:<port>
temporarily bound. Update restartMcpBridge() to wait for the old process or port
to fully exit before calling startProxy(), using the PID returned by stopProxy()
or a port-availability poll. Keep the fix localized to the stopProxy and
restartMcpBridge path so the new proxy only binds after the previous one has
released the port.
- Around line 1022-1058: The subcommand dispatcher in mcp-bridge currently
accepts extra positional tokens because `list`, `status`, `restart`, and
`remove` only inspect part of `rest`. Add explicit leftover-argument validation
in the command handling block for these cases, so any unexpected tokens after
parsing flags trigger a `McpBridgeError` usage message instead of being ignored;
use the existing `parseJsonFlag`, `statusMcpBridge`, `restartMcpBridge`, and
`removeMcpBridge` branches to locate the fix.

In `@src/lib/state/registry.ts`:
- Around line 439-443: normalizeSandboxMcpState() is keeping the original
bridges map key even after normalizeMcpBridgeEntry() falls back to entry.server,
which can leave a malformed persisted key in the normalized state. Re-key the
bridges object by the normalized server name (or omit invalid entries entirely)
when rebuilding the map from Object.entries(bridgesValue), so statusMcpBridge()
and bridgeRuntimeDir() validate against the same key that is stored.

In `@src/mcp-proxy.ts`:
- Around line 366-369: The startup logging in mcp-proxy should not print raw
child-process bridge arguments, because config.args may contain sensitive
values. Update the logging around the proxy startup output in src/mcp-proxy.ts
to remove the direct args dump or replace it with a sanitized summary, while
keeping the existing command/env context in a redacted form. Use the mcp-proxy
startup log block near config.command, config.args, and config.env to make the
change.
- Around line 266-320: The HTTP handler in mcp-proxy keeps an inflight MCP
request alive even after the client disconnects, which can leave stale slots
occupied until timeout. Update the request flow in the server callback to
register a req.on("close", ...) handler that cancels or clears the pending
client.call invocation immediately, and make sure it only affects the active
request identified by request.id so healthy traffic is not blocked when
MCP_PROXY_MAX_INFLIGHT is reached.

---

Outside diff comments:
In `@src/lib/onboard/openshell-install.ts`:
- Around line 162-174: The onboarding upgrade check in openshell-install.ts is
missing the new MCP-L7 capability gate, so an installed OpenShell at the minimum
version can still be treated as valid even if it lacks
allow_all_known_mcp_methods. Update the needsUpgrade logic in the openshell
install flow to also verify the same capability check used by
scripts/install-openshell.sh, and treat the install as needing upgrade/failure
when that feature is absent. Use the existing currentVersionOutput/version
checks and the relevant helpers in openshell-install.ts so the new gate is
applied before skipping reinstall.

---

Nitpick comments:
In `@schemas/policy-preset.schema.json`:
- Around line 51-56: Add focused schema validation tests for the new
policy-preset branches so regressions are caught directly: update the schema
test coverage around the protocol enum and the $defs used by jsonRpcOptions,
mcpOptions, and denyRulesMatcher to include explicit accept/reject cases for
json-rpc and mcp values, their expected matcher shapes, and the associated
option objects. Keep the existing compile/legacy rest coverage, but add targeted
positive and negative assertions that exercise the new branches in
policy-preset.schema.json rather than relying on broad compile-only checks.

In `@test/fetch-guard-patch-regression.test.ts`:
- Around line 135-141: The regression helper only checks the mcporter version in
Dockerfile, so it can miss drift in Dockerfile.base; update
readDockerfileMcporterVersion and the related test assertion to validate the
mcporter pin in both files or derive both from a single shared source. Make sure
the fetch-guard regression test covers the cross-file contract introduced by the
base-image mcporter baking, not just the runtime layer.
🪄 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: 4ab8d0a0-fbf6-4f79-b175-ea6d6865841a

📥 Commits

Reviewing files that changed from the base of the PR and between 5b001aa and 6f58c3e.

📒 Files selected for processing (38)
  • .github/workflows/regression-e2e.yaml
  • Dockerfile
  • Dockerfile.base
  • agents/hermes/manifest.yaml
  • agents/langchain-deepagents-code/manifest.yaml
  • agents/openclaw/manifest.yaml
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx
  • nemoclaw-blueprint/blueprint.yaml
  • schemas/policy-preset.schema.json
  • schemas/sandbox-policy.schema.json
  • scripts/brev-launchable-ci-cpu.sh
  • scripts/install-openshell.sh
  • src/commands/sandbox/mcp.ts
  • src/lib/actions/sandbox/mcp-bridge.test.ts
  • src/lib/actions/sandbox/mcp-bridge.ts
  • src/lib/agent/base-image.test.ts
  • src/lib/agent/defs.test.ts
  • src/lib/agent/defs.ts
  • src/lib/agent/hermes-recovery-boundary-fixtures.ts
  • src/lib/agent/onboard.test.ts
  • src/lib/agent/runtime.test.ts
  • src/lib/cli/command-display.ts
  • src/lib/cli/command-registry.test.ts
  • src/lib/cli/command-registry.ts
  • src/lib/cli/public-argv-translation.test.ts
  • src/lib/cli/public-display-defaults.ts
  • src/lib/onboard/openshell-install.ts
  • src/lib/onboard/openshell-version.ts
  • src/lib/state/registry.ts
  • src/mcp-proxy.test.ts
  • src/mcp-proxy.ts
  • test/e2e-scenario/live/openshell-version-pin.test.ts
  • test/e2e/test-openshell-version-pin.sh
  • test/fetch-guard-patch-regression.test.ts
  • test/install-openshell-version-check.test.ts
  • test/install-preflight.test.ts
  • test/registry.test.ts

Comment thread Dockerfile Outdated
Comment thread docs/reference/commands-nemohermes.mdx Outdated
Comment thread schemas/sandbox-policy.schema.json
Comment thread src/lib/actions/sandbox/mcp-bridge.ts Outdated
Comment thread src/lib/actions/sandbox/mcp-bridge.ts Outdated
Comment thread src/lib/actions/sandbox/mcp-bridge.ts Outdated
Comment thread src/lib/state/registry.ts Outdated
Comment thread src/mcp-proxy.ts Outdated
Comment thread src/mcp-proxy.ts Outdated
@ericksoa ericksoa changed the title feat(mcp): add OpenClaw host bridge feat(mcp): add cross-agent host bridge Jun 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 28269095222
Target ref: e0748636314aab2e2b8118cf6fcd404147b0e105
Workflow ref: feat/mcp-bridge-vdr-566
Requested jobs: mcp-bridge-e2e
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-e2e ❌ failure

Failed jobs: mcp-bridge-e2e. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28269095075
Workflow ref: feat/mcp-bridge-vdr-566
Requested scenarios: (default — all supported)
Requested jobs: mcp-bridge-vitest
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-vitest ❌ failure

Failed jobs: mcp-bridge-vitest. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 28269143340
Target ref: 7e9e1f0ea
Workflow ref: feat/mcp-bridge-vdr-566
Requested jobs: mcp-bridge-e2e
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-e2e ❌ failure

Failed jobs: mcp-bridge-e2e. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28269143284
Workflow ref: feat/mcp-bridge-vdr-566
Requested scenarios: (default — all supported)
Requested jobs: mcp-bridge-vitest
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-vitest ❌ failure

Failed jobs: mcp-bridge-vitest. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 28269290981
Target ref: 3924819ff
Workflow ref: feat/mcp-bridge-vdr-566
Requested jobs: mcp-bridge-e2e
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-e2e ❌ failure

Failed jobs: mcp-bridge-e2e. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28269290870
Workflow ref: feat/mcp-bridge-vdr-566
Requested scenarios: (default — all supported)
Requested jobs: mcp-bridge-vitest
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-vitest ❌ failure

Failed jobs: mcp-bridge-vitest. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 28269537374
Target ref: 9656b7a4c
Workflow ref: feat/mcp-bridge-vdr-566
Requested jobs: mcp-bridge-e2e
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-e2e ❌ failure

Failed jobs: mcp-bridge-e2e. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28269537216
Workflow ref: feat/mcp-bridge-vdr-566
Requested scenarios: (default — all supported)
Requested jobs: mcp-bridge-vitest
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-vitest ❌ failure

Failed jobs: mcp-bridge-vitest. Check run artifacts for logs.

Comment thread scripts/install-openshell.sh Fixed
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 28270356785
Target ref: e7ce70a318850cdd96cf282e5e40268f0ed9778d
Workflow ref: feat/mcp-bridge-vdr-566
Requested jobs: mcp-bridge-e2e
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-e2e ❌ failure

Failed jobs: mcp-bridge-e2e. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28270355861
Workflow ref: feat/mcp-bridge-vdr-566
Requested scenarios: (default — all supported)
Requested jobs: mcp-bridge-vitest
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-vitest ❌ failure

Failed jobs: mcp-bridge-vitest. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 28270636453
Target ref: aecd918d31c8e9459b1bd6b95d9095cbbe91187e
Workflow ref: feat/mcp-bridge-vdr-566
Requested jobs: mcp-bridge-e2e
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-e2e ❌ failure

Failed jobs: mcp-bridge-e2e. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28270635514
Workflow ref: feat/mcp-bridge-vdr-566
Requested scenarios: (default — all supported)
Requested jobs: mcp-bridge-vitest
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
mcp-bridge-vitest ❌ failure

Failed jobs: mcp-bridge-vitest. Check run artifacts for logs.

@ericksoa ericksoa changed the title feat(mcp): add cross-agent host bridge feat(mcp): add OpenShell-managed MCP servers Jun 26, 2026
Comment thread src/lib/onboard/openshell-feature-gate.ts Fixed
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ⚠️ Run cancelled — no signal

Run: 28682759427
Workflow ref: feat/mcp-bridge-vdr-566
Requested targets: (default — all supported)
Requested jobs: mcp-bridge,hermes-e2e
Summary: 0 passed, 0 failed, 2 cancelled, 0 skipped

Job Result
hermes-e2e ⚠️ cancelled
mcp-bridge ⚠️ cancelled

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ✅ All selected jobs passed

Run: 28682976259
Workflow ref: feat/mcp-bridge-vdr-566
Requested targets: ubuntu-repo-cloud-langchain-deepagents-code
Requested jobs: (default — all default-enabled free-standing jobs; explicit-only jobs openshell-gateway-auth-contract, mcp-bridge-dev, hermes-gpu-startup, sandbox-rlimits-connect, and jetson-nvmap-gpu are skipped unless selected)
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
live ✅ success

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ❌ Some jobs failed

Run: 28682975290
Workflow ref: feat/mcp-bridge-vdr-566
Requested targets: (default — all supported)
Requested jobs: mcp-bridge,hermes-e2e
Summary: 0 passed, 1 failed, 1 cancelled, 0 skipped

Job Result
hermes-e2e ❌ failure
mcp-bridge ⚠️ cancelled

Failed jobs: hermes-e2e. Check run artifacts for logs.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ✅ All selected jobs passed

Run: 28683194786
Workflow ref: feat/mcp-bridge-vdr-566
Requested targets: ubuntu-repo-cloud-langchain-deepagents-code
Requested jobs: (default — all default-enabled free-standing jobs; explicit-only jobs openshell-gateway-auth-contract, mcp-bridge-dev, hermes-gpu-startup, sandbox-rlimits-connect, and jetson-nvmap-gpu are skipped unless selected)
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
live ✅ success

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ⚠️ Run cancelled — no signal

Run: 28683193701
Workflow ref: feat/mcp-bridge-vdr-566
Requested targets: (default — all supported)
Requested jobs: mcp-bridge,hermes-e2e
Summary: 0 passed, 0 failed, 2 cancelled, 0 skipped

Job Result
hermes-e2e ⚠️ cancelled
mcp-bridge ⚠️ cancelled

ericksoa added a commit that referenced this pull request Jul 3, 2026
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa added a commit that referenced this pull request Jul 3, 2026
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa added a commit that referenced this pull request Jul 3, 2026
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ✅ All selected jobs passed

Run: 28683611376
Workflow ref: feat/mcp-bridge-vdr-566
Requested targets: ubuntu-repo-cloud-langchain-deepagents-code
Requested jobs: (default — all default-enabled free-standing jobs; explicit-only jobs openshell-gateway-auth-contract, mcp-bridge-dev, hermes-gpu-startup, sandbox-rlimits-connect, and jetson-nvmap-gpu are skipped unless selected)
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
live ✅ success

@apurvvkumaria apurvvkumaria merged commit bddbb3e into main Jul 3, 2026
197 checks passed
@apurvvkumaria apurvvkumaria deleted the feat/mcp-bridge-vdr-566 branch July 3, 2026 21:50
@cv cv mentioned this pull request Jul 3, 2026
21 tasks
cv added a commit that referenced this pull request Jul 4, 2026
## Summary

Hardens the managed-MCP work merged in #5876 so DCode rebuilds validate
every reconstructable input before crossing the destructive delete
boundary, preserve exact policy intent, and migrate legacy managed MCP
state fail-closed. Prepared rebuild artifacts and the derived MCP
runtime snapshot remain ephemeral and process-local; neither is
persisted in FSM or checkpoint state, so this does not implement #6224.

## Related Issue

Refs #5876
Refs #6195
Refs #6218

## Changes

- Revalidate DCode route, image, Dockerfile, reasoning, web-search, and
MCP inputs after preparation and before NIM stop or sandbox deletion;
restore MCP state and relock shields on failure.
- Preserve exact custom network policy replay while keeping generated
MCP rules under the MCP adapter's exclusive ownership.
- Add protocol-specific policy schema validation for REST, WebSocket,
JSON-RPC, and MCP matchers, including cross-rule `tools/call` conflict
rejection.
- Pin Deep Agents Code 0.1.30 and load only a strict, canonicalized
managed MCP projection from a process-local integrity-bound snapshot.
Sealed memfd is preferred; when OpenShell seccomp blocks it, an
anonymous `O_TMPFILE` inode is reopened read-only and bound by
descriptor, device, inode, size, kind, and SHA-256, with ambient
discovery disabled.
- Bind the canonical TypeScript secret-pattern source and flags to one
shared behavior corpus executed through the Bash and Python DCode
enforcement boundaries, including the full ECMAScript whitespace set.
- Add capability-v2 gating and legacy-v1 teardown/rollback that
preserves unrelated user configuration and fails closed on malformed,
unsafe, or drifted state.
- Add rebuild, migration, runtime-patch, schema, snapshot, and lifecycle
coverage; update the MCP, policy, security, command, and DCode
documentation.

Verification notes:

- Final DCode-adjacent run: 9 files, 187 tests passed; the focused
descriptor/projection run passed 4 files and 138 tests.
- Final review-follow-up run: 82 focused Bash/Python/TypeScript parity
and descriptor-fallback tests passed, including all 25 ECMAScript
whitespace code points under both `C` and `C.UTF-8` Bash locales.
- Full pre-squash-equivalent run: 1,068 files passed, 2 skipped; 12,149
tests passed, 35 skipped.
- CLI coverage ratchet passed with the repository include/exclude set
expressed as one Vitest glob: lines 65.24%, statements 64.45%, functions
67.06%, branches 57.21%.
- Python compile, Biome, ShellCheck, shfmt, source-shape, test-size,
repository, secret-scan, and diff checks passed. The normal push hook
passed CLI typechecking.
- Main-sync validation after merging #6265 passed: 9 CLI files/82 tests,
6 integration files/174 tests, an additional 3 preparation tests, CLI
typecheck, Biome, and diff checks. Generated-context provenance was
ported into the split preflight fixtures without restoring the obsolete
monolith.
- Exact-head CI for `9a31537785ef2d456901de622721ed215627fdec` passed:
40 checks green, all five required contexts passed, and there were 0
failures, cancellations, or pending checks. The only skips were the
expected docs-only job and two duplicate NVSkills request jobs. This
includes all five CLI shards plus the aggregate, both CodeQL languages,
both sandbox image builds, macOS, WSL, four self-hosted runtime checks,
CodeRabbit, and both review advisors.
- Exact-head live E2E for `9a31537785ef2d456901de622721ed215627fdec`
passed:
[`mcp-bridge`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844701),
[`mcp-bridge-dev`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844719),
and
[`ubuntu-repo-cloud-langchain-deepagents-code`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844639).
Stable and dev each passed OpenClaw, Hermes, and DCode 3/3;
authenticated MCP calls passed initially and after restart, credential
rotation, and rebuild, then removal denied access with no provider,
policy, tunnel, or credential residue. The dedicated DCode lane passed
Landlock 5/5, Python egress 14/14, headless inference 10/10, secret
boundary 8/8, Tavily 6/6, and TUI 4/4; BuildKit accepted the merged
generated-context handoff, and invalid-credential rebuild failure
remained pre-destructive with the original sandbox, marker, and route
recovered. Artifact inspection found one unchanged pre-existing harness
defect: two OpenShell audit-log filtering subassertions can false-pass
because awk treats `close` as reserved; runtime-output, sandbox-log,
env-file immutability, and raw-secret checks passed, and this PR does
not modify that E2E file.
- The base `test-cli` pre-commit invocation remains affected by Vitest
4.1.9 collapsing repeated `--coverage.exclude` arguments to a
zero-file/invalid summary. All other commit and push hooks passed;
targeted tests and the authoritative sharded CI coverage checks provide
the exact-head gate.
- `npm run docs` completed with 0 errors and 2 pre-existing Fern
warnings. Two documentation-writer audits confirmed the final behavior
is accurately documented.

## 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 security
and correctness reviews passed after fixes; destructive-boundary
rollback, capability migration, the process-local integrity-bound
snapshot handoff (sealed memfd preferred, anonymous `O_TMPFILE`
fallback), cross-language secret-pattern parity, policy fidelity, and
the #6224 boundary were checked.
- [ ] 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
- [x] 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)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Enhanced managed MCP bridge support with managed-only configuration
snapshots for safer add/restart/rebuild/teardown.
* Network policy protocol rules now support protocol-specific matching
plus stricter `endpoint.path` validation.
* **Bug Fixes**
* Stronger fail-fast validation for MCP server names/hostnames and
endpoint details (rejected before changes are applied).
* Rebuild flows improved to preserve/replay custom policies and validate
after MCP preparation, with rollback on failure.
* **Documentation**
* Updated setup/quickstart/reference and MCP bridge/rebuild guidance for
managed MCP capability v2 behavior and stricter validation rules.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv added a commit that referenced this pull request Jul 4, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds a shared tool-disclosure mode across Deep Agents Code, Hermes, and
OpenClaw. New sandboxes default to `progressive`; `--tool-disclosure
direct` or `NEMOCLAW_TOOL_DISCLOSURE=direct` restores the prior fully
visible catalog. The mode persists through resume and transactional
rebuilds.

The design follows [LangChain progressive
disclosure](https://support.langchain.com/articles/8488719552-progressive-tool-disclosure-with-deep-agents),
[Hermes Tool
Search](https://hermes-agent.nousresearch.com/docs/user-guide/features/tool-search),
and the pinned [OpenClaw v2026.5.27 Tool Search
contract](https://github.com/openclaw/openclaw/blob/v2026.5.27/docs/tools/tool-search.md).
This is the v0.0.74 bundle.

Exact head `e4214144c11dd47b9d3185f19c1c73ec4e690552` includes `main` at
`6f5ccbcbaa87367f7b521318fbc940467045b601`.

## Related Issue

#5876 is merged. This PR has no remaining stacked dependency and
preserves its managed-MCP ownership contract.

## Changes

- Add the shared `--tool-disclosure progressive|direct` onboarding and
rebuild option, with `NEMOCLAW_TOOL_DISCLOSURE` as the environment
equivalent. CLI input takes precedence and fresh sandboxes default to
`progressive`.
- Persist the selected mode in session and registry state. Resume,
recreation, and MCP-bearing rebuilds preserve or transactionally change
it in either direction.
- Add bounded `search_tools` middleware to Deep Agents Code when at
least one MCP tool loads successfully. Core tools remain visible,
discovery state survives checkpointing, and only model requests are
filtered; the complete executor registry and existing authorization
controls remain intact.
- Enable Hermes native Tool Search with opinionated 5/20 result limits
and direct core tools.
- Enable OpenClaw native structured `mode: "tools"` discovery with 8/20
limits while retaining model-specific compatibility safeguards.
- Patch and validate the exact pinned `deepagents-code==0.1.30` runtime
with fail-closed, idempotent anchors and separate middleware instances
for the main agent and local subagents.
- Add build-time/runtime validators, lifecycle and overflow coverage,
transactional custom-image handling, and user-facing documentation for
progressive/direct behavior.

### Managed MCP boundary

This PR does not change MCP configuration, credential placeholders,
bridge ownership, policy generation, MCP CLI behavior, or registry
schemas from #5876.

Managed OpenClaw MCP remains registered in mcporter's home registry.
Pinned OpenClaw Tool Search reads native bundle and `mcp.servers`
catalogs, and NemoClaw does not currently synchronize those ownership
stores. The OpenClaw runtime validator therefore proves native search,
describe, and call behavior for eligible hidden catalog tools, while the
existing live MCP scenario separately proves mcporter execution,
credential rewriting and rotation, restart/rebuild behavior, policy
enforcement, DNS-rebinding protection, and secret boundaries. A
fixture-only projection between those stores would not represent shipped
behavior.

### Apurv security review resolution

1. **Callable namespace:** Deep Agents Code rejects every duplicate
resolved callable name and every non-managed owner of a reserved core
name before the original factory in progressive and direct modes.
Coverage includes regular/regular, regular/MCP, cross-MCP-server,
direct-mode, and schema/executor mismatch cases.
2. **Build-context seal:** fingerprints require a real directory root
and encode portable mode bits, nanosecond modification times, link
counts, and deterministic hardlink topology. Pre-delete and final
one-shot tests cover hardlink, timestamp, permission, and root-symlink
mutations.
3. **Dockerfile patch boundary:** patching anchors and revalidates the
staging parent, rejects multi-link files, writes a fresh private
same-directory file, and atomically replaces the staged Dockerfile
without truncating an attacker-selectable inode.
4. **Chunk-safe diagnostics:** raw cloudflared child output is not
emitted in failures. Only bounded per-stream carry is retained for
origin discovery, with split-event credential regressions proving
fragments and reconstructed secrets remain absent.

### CodeRabbit and CodeQL resolution

- Every CodeRabbit finding is implemented or explicitly dispositioned
with repository evidence, including URL-token punctuation, the
intentional private hosted model identifier, and the final Darwin
fixture fail-fast nit. GraphQL reports zero unresolved review threads;
exact-head CodeRabbit status is green.
- [CodeQL alert
#1210](https://github.com/NVIDIA/NemoClaw/security/code-scanning/1210)
is dismissed as a false positive. The flagged sink opens an existing
directory read-only with `O_DIRECTORY | O_NOFOLLOW | O_NONBLOCK`; it
does not use `O_CREAT`. The real replacement file remains UUID-named,
`O_CREAT | O_EXCL | O_NOFOLLOW`, mode `0600`, descriptor/path identity
checked, single-link validated, fsynced, and atomically renamed.
Exact-head CodeQL is green with no open branch alerts.

### Validation

- Local focused matrix: 19 files / 327 tests passed; one true Linux-only
`memfd`/`O_TMPFILE` restart case was skipped on macOS.
- Exact pinned Deep Agents Code/LangChain patch and runtime validator
passed: `progressive-disclosure-runtime-ok`.
- Both TypeScript typechecks, build, repository checks,
source-shape/test-size guards, Biome, Ruff/Python compilation, secret
scanning, push checks, and `git diff --check` passed.
- Final-head standard validation is green: [PR
CI](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184882),
[growth
guardrails](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184428),
[WSL](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184879),
[macOS](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184874),
[security](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184917),
[CodeQL](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184138),
[review
advisors](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184863),
and the [E2E
advisor](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184880).
The original exact-head check set and subsequent metadata checks are all
green, with only the two expected skips.
- The exact-head [typed OpenClaw and Deep Agents
targets](https://github.com/NVIDIA/NemoClaw/actions/runs/28700290435)
passed.
- The exact-head [production-image
workflow](https://github.com/NVIDIA/NemoClaw/actions/runs/28700292369)
passed both requested image builds and all four downstream image E2E
checks.
- In the exact-head [advisor-selected live
union](https://github.com/NVIDIA/NemoClaw/actions/runs/28700286465), 18
of 19 concrete jobs passed. The sole Hermes security-posture failure
occurred in its deliberate gateway-recovery probe after restart,
secret-boundary, and disclosure assertions had passed. The isolated
exact-head [security
retry](https://github.com/NVIDIA/NemoClaw/actions/runs/28700509914)
passed both Hermes and OpenClaw legs, so every selected live lane has
passing final-head evidence.
- The GPT review advisor's remaining OpenClaw MCP model-loop warning is
explicitly dispositioned by the managed-MCP ownership boundary above:
the native OpenClaw catalog and mcporter registry are separate shipped
stores, and a test-only projection would claim behavior the production
bridge does not provide.

### Remaining

- Obtain Apurv's exact-head re-review to clear the formal
`CHANGES_REQUESTED` state and complete sensitive-path approval.
- Rebuild affected existing sandboxes after this change ships; newly
created sandboxes default to progressive disclosure.

## 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)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Apurv exact-head
re-review pending after all four requested fixes.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue: no waiver requested;
exact-head security retry is green.

## Verification

- [x] PR description includes the DCO sign-off declaration and every
feature commit appears as `Verified` in GitHub
- [ ] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes — all applicable hooks passed; the
known hanging local unsharded `test-cli` coverage hook was skipped and
exact-head sharded Linux CI is authoritative.
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [ ] 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; current
build has two pre-existing warnings)
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [x] New doc pages include SPDX header and frontmatter (new pages only;
no new doc pages)

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
@ericksoa ericksoa mentioned this pull request Jul 4, 2026
21 tasks
ericksoa added a commit that referenced this pull request Jul 4, 2026
<!-- markdownlint-disable MD041 -->
## Summary
This PR prepares the user-facing documentation for v0.0.74 before the
release plan is frozen.
It expands the release notes across the 56-commit train and closes
durable documentation gaps found during the pre-tag commit scan.

## Changes
- Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed
MCP, progressive tool disclosure, LangChain Deep Agents Code,
onboarding, local inference, messaging, recovery, and contributor
workflows.
- Correct the `destroy` contract for retained per-name volumes,
gateway-unreachable `--force` cleanup, managed MCP ownership, and
same-name recovery.
- Document separate remediation for an unreachable container DNS
resolver versus one that answers with `NXDOMAIN` or `REFUSED`.
- Document the Windows on Arm N1X automatic Ollama safeguard and its
remaining large-model limitations.
- State that messaging conflicts abort rebuild before backup or
deletion, leaving the original sandbox intact.
- Link the agent-runnable value benchmark from the contributor task
index.
- Synchronize generated agent command variants.
- Validate with `npm run docs:sync-agent-variants` and `npm run docs`;
Fern completed with 0 errors and 2 existing warnings.
- Source summary:
- [#6020](#6020) and
[#5876](#5876) ->
`docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy
boundary and managed MCP lifecycle.
- [#6251](#6251) and
[#5989](#5989) ->
`docs/about/release-notes.mdx`: Summarize progressive tool disclosure
and sandbox-first inference controls.
- [#6232](#6232),
[#6082](#6082),
[#6219](#6219),
[#6214](#6214),
[#6215](#6215),
[#6230](#6230), and
[#6260](#6260) ->
`docs/about/release-notes.mdx`: Summarize the experimental LangChain
Deep Agents Code status, secret, version, rebuild, snapshot, and MCP
boundaries.
- [#6166](#6166),
[#6254](#6254),
[#6265](#6265),
[#6164](#6164), and
[#6017](#6017) ->
`docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated
image reuse, bounded readiness, and preflight improvements.
- [#6150](#6150) ->
`docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`:
Separate unreachable-resolver remediation from reachable-but-rejected
DNS responses.
- [#6234](#6234) ->
`docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`, and
`docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B
selection and the remaining explicit-large-model boundary.
- [#6129](#6129),
[#5987](#5987),
[#5955](#5955), and
[#6220](#6220) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Document messaging policy
persistence, status, and the pre-destructive conflict check.
- [#5963](#5963),
[#6050](#6050),
[#6094](#6094),
[#6238](#6238),
[#5988](#5988),
[#6235](#6235),
[#6181](#6181), and
[#5986](#5986) ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and
clarify retained-volume and local-only destroy semantics.
- [#6200](#6200),
[#6248](#6248),
[#6168](#6168),
[#6270](#6270), and
[#5649](#5649) ->
`docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize
contributor setup and verification improvements and expose the advisory
value benchmark.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: documentation-only release
preparation; generated-variant synchronization and the Fern docs build
validate the changed pages and routes.
- [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:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: tests
are not applicable to this documentation-only change; `npm run docs`
validates the source and generated routes.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Expanded setup guidance for Windows on Arm devices with safer default
local model selection.
* Clarified local inference and sandbox messaging behavior, including
conflict checks before rebuilds and safer recovery steps.
* Updated destroy/rebuild/reference docs with more detailed warnings,
failure handling, and volume-retention guidance.
* Improved troubleshooting instructions for Docker DNS issues with
clearer paths for unreachable vs. blocked resolvers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@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>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

NemoClaw `v0.0.74` will ship stable OpenShell `v0.0.72`. This dependency
layer advances the supported OpenShell contract from `0.0.71` to
`0.0.72`, pins every consumed release artifact, preserves
round-trippable policy state, and hardens installer verification so
pull-request code cannot authorize its own pins.

## Related Issue

Refs NVIDIA#5591.
Follow-up to merged NVIDIA#5596.
Dependency layer for NVIDIA#5876 and the accepted MCP design in NVIDIA#566.

## Changes

- Pin stable OpenShell `0.0.72` across the supported version
floor/ceiling, installer, Brev launchable, blueprint, supervisor image,
workflow, and live-E2E contracts. OpenShell development builds remain
compatibility evidence, not the shipping runtime.
- Pin all consumed CLI, gateway, and sandbox archives plus both Brev CLI
references to the official `v0.0.72` checksum manifests.
- Read mutation input from `openshell policy get --base`, strip reserved
`_provider_*` entries before `policy set`, retain `--full` only for
read-only diagnostics, and preserve future mapping sections plus
MCP/JSON-RPC fields during merges.
- Route the CommonJS CLI and ESM plugin through one generated OpenShell
policy boundary and exact-pin `yaml` `2.8.3` in both production package
graphs.
- Normalize that boundary for both compiled CommonJS and source-mode
`tsx` loading. A subprocess package-contract test reproduces the live
source-loader path that exposed the mismatch.
- Run installer verification from base-trusted code. The introducing PR
falls back only to immutable commit
`cb5e9aefab2b16fedc0995149fc3520da0d5e0c7`, verified as tree
`1fdf59efe40b78c407e222fd42043b23a61e199a`, with an enforced expiry at
`2026-12-29T19:35:41Z`.
- Treat PR-head installer files as data only. The trusted parser rejects
symbolic links, a symbolic-link `scripts` parent, non-regular files,
changed inode/device identity, and input over 1 MiB; it opens with
`O_NOFOLLOW` and performs a bounded descriptor read.
- Fail installer verification closed on missing, duplicate, mismatched,
incomplete, or unreachable OpenShell/Brev pin data.
- Publish the OpenShell `0.0.72` compatibility review and align version,
policy, gateway-authentication, and troubleshooting documentation.

### Exact-head evidence

- PR head: `2d06fa01b624b63813fe558ce36b29d47ad31e36`, based exactly on
current `main` `dc96deb24d67eeeb2cb7b2bb42c7c53f000507f3`. The final
signed merge incorporates the release-boundary revert that defers
unrelated dcode-status work, so this dependency PR does not reintroduce
NVIDIA#6202 outside its scope.
- GitHub verifies the new merge commit signature, DCO is green, the
prior maintainer approval remains recorded at [review
4611344448](NVIDIA#6020 (review)),
and GitHub reports the PR graph as `MERGEABLE`.
- Post-restack local validation passes `build:cli`, full and CLI
typechecks, repository checks, generated agent-doc synchronization,
affected Deep Agents image contracts, and `git diff --check`.
- All exact-head ordinary PR checks are terminal green (33 successful,
three skipped/neutral, zero failures), including macOS/WSL E2E, every
CLI shard and aggregate, static/security scans, DCO, and both PR Review
Advisor jobs. GitHub reports `APPROVED` and `MERGEABLE/CLEAN`.
- Exact-head selected OpenShell [E2E run
28632123304](https://github.com/NVIDIA/NemoClaw/actions/runs/28632123304)
is terminal green: version pin, gateway-auth contract, network policy,
gateway upgrade/state restoration, scorecard, and the no-comment
reporter all passed from a temporary no-PR ref at the identical commit.
The temporary ref was deleted after completion.
- Exact-head PR Review [run
28632002111](https://github.com/NVIDIA/NemoClaw/actions/runs/28632002111)
and E2E Advisor [run
28632002140](https://github.com/NVIDIA/NemoClaw/actions/runs/28632002140)
are green. GPT reports no actionable finding; Nemotron's check passed
but both JSON synthesis attempts were unparseable, so that model's
artifact is incomplete rather than clearance. E2E Advisor reports high
confidence and selects the four live lanes linked above.

### Trust-boundary notes

- The immutable bootstrap is intentionally used only while the PR base
lacks the trusted action. Once that action exists on the base, executing
the newer base-trusted verifier is the stronger boundary; the expiring
bootstrap should then be removed rather than run redundantly.
- No untrusted PR process executes alongside the parser. GitHub checks
out inert PR data, then trusted code validates and reads the
already-opened descriptor. The link/type/identity/bounds checks cover
repository-controlled redirection and exhaustion inputs without claiming
protection from a privileged concurrent host writer.
- Stable OpenShell `0.0.72` accepts an unmarked policy root only when it
contains `version` or `network_policies`; metadata-only and malformed
documents fail closed. Versionless `network_policies` is retained for
the supported compatibility contract.

### Advisor disposition

- GPT reported no required findings and one warning about the mutable
default `BASE_IMAGE` tag. That `ARG
BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest` line is
unchanged from current `main`; this dependency PR neither introduces nor
broadens that repository-wide build default.
- Nemotron's bootstrap finding would weaken the intended trust
transition: the immutable bootstrap exists only for the introducing PR.
Once the action is present on the base, the newer base-trusted action
must replace the older bootstrap; both paths are immutable for the
current event and are contract-tested.
- Nemotron's parser race assumes an untrusted concurrent filesystem
writer. PR code is never executed in this job: GitHub checks out inert
data, then trusted code rejects links/special files, checks the opened
descriptor's device/inode, bounds the read, and closes it. A privileged
host writer is outside this PR-input threat model.
- Nemotron's checksum finding is not circular. The trusted checker pins
the SHA-256 of each upstream checksum manifest, verifies that immutable
manifest before reading it, and compares every embedded installer pin
with exactly one manifest entry. At install time each named archive must
exist and match its pinned digest, so a missing asset still fails closed
without downloading all archives during every PR check.
- The generated-boundary auditor executes in the Docker builder stage
exercised by ordinary `build-sandbox-images` CI. The exact source-mode
`.cts` versus generated `.cjs` mismatch found by live proof is now
covered directly by the subprocess package-contract test and the
compiled runner suites.

## 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
- [x] Existing tests cover changed behavior — justification: policy
mutation, package boundary, installer trust, workflow selection, and
runtime upgrade/state-restoration have focused coverage; final selected
E2E is linked above.
- [ ] Tests not applicable — justification: not applicable; this changes
security-sensitive installer, policy, and runtime compatibility
behavior.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification: not applicable; supported
OpenShell versions and policy behavior are user-facing.
- [x] 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: the linked approval
predates the current head; exact-head human review or an explicit
carried-approval decision remains required, and no waiver is requested.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue: no waiver is requested;
exact-head ordinary CI is green, and the selected run's comment-only
reporter caveat is documented above and is not a required PR check.

## 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)
- [x] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Preksha Vyas <prekshiv@nvidia.com>
Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds native OpenShell-managed authenticated HTTPS Streamable HTTP MCP
server lifecycle for OpenClaw, Hermes, and LangChain Deep Agents Code.
It exposes `mcp add|list|status|restart|remove`, integrates crash-safe
rebuild and destroy recovery, and keeps raw credentials out of sandbox
files and NemoClaw registry state. Stable OpenShell `0.0.72` is the
shipping dependency for NemoClaw `v0.0.74`.

## Related Issue

Closes NVIDIA#566.
Refs NVIDIA#6195.
Implements the [accepted native OpenShell design
decision](NVIDIA#566 (comment)).
Supersedes the closed host-proxy implementation in NVIDIA#565.

## Changes

- Support OpenClaw, Hermes, and LangChain Deep Agents Code with `mcp
add|list|status|restart|remove`; normal sandbox rebuild and destroy
participate in lifecycle reconciliation.
- Accept only authenticated canonical HTTPS Streamable HTTP endpoints
with exactly one host-side `--env KEY` credential reference per server.
- Use native OpenShell `protocol: mcp` policy enforcement and
provider-backed credential replacement. NemoClaw starts no MCP proxy,
relay, listener, stdio adapter, or persistent secret-bearing host
process.
- Persist only credential names and opaque ownership metadata. Agent
configuration contains `Bearer openshell:resolve:env:KEY`; raw values
are supplied only to the allowlisted OpenShell provider subprocess.
- Fail closed on private/local/special-use targets, unsupported host
aliases, userinfo, query, fragment, percent characters, non-canonical
paths, ambiguous provider/policy/adapter state, and ownership drift.
- Serialize supported per-sandbox writers with one cross-process
mutation lock and exact-check policy/provider state before and after
mutation.
- Preserve owned MCP intent across rebuild, recover interrupted
add/remove/destroy transactions, and delete shared state only after
exact ownership verification.
- Add stable and explicit dev-channel live lanes, artifact credential
scanning, workflow boundary tests, operator documentation, and focused
security/lifecycle regression coverage.

### Apurv review feedback addressed

- [x] [Use the checked-in Hermes `BASE_IMAGE`
pin](NVIDIA#5876 (comment)):
validation now reads the single immutable `ARG
BASE_IMAGE=...@sha256:...` from the actual final Dockerfile, accepts
only that official digest, and rejects a different digest. The obsolete
`NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST` guard is removed.
- [x] [Preserve falsey non-map YAML
roots](NVIDIA#5876 (comment)):
both Hermes transaction paths normalize only `None` to `{}`. `[]`,
`false`, `0`, and `""` reach the object-root guard and fail without a
write; tests prove byte preservation and zero writes.
- [x] [Block rebuild while MCP destroy is
incomplete](NVIDIA#5876 (comment)):
live rebuild and absent-sandbox onboarding reject both
`destroyPreparedAt` and `destroyPendingAt` before policy, adapter,
provider, gateway, pruning, or recreation work.
- [x] [Reject every percent character and retain legacy
cleanup](NVIDIA#5876 (comment)):
host and Hermes add validation reject `%`, `%GG`, `%2`, and valid
percent escapes before side effects. Forced Hermes cleanup validates the
action and server name, then removes the exact legacy key without
re-rejecting its already-persisted URL.
- [x] [Recheck exact policy content and serialize supported
writers](NVIDIA#5876 (comment)):
the fresh policy read deep-compares the live key with the exact
previously owned content and refuses changed or removed values before
`policy set`; a post-set exact read gates ownership commit. Supported
NemoClaw policy, channel, shields, inference, snapshot, onboarding, and
config writers share the per-sandbox lock.
- [x] [Keep credential revision proof
host-side](NVIDIA#5876 (comment)):
the sandbox-writable revision snapshot is removed. Fresh
OpenShell-mediated execs return only bounded `absent`, `canonical`, or
`vN` observations; the host retains the prior value and requires a
changed, non-absent revision after update.

The review fixes landed in signed commit
`299efb5c1eaeec21f9b91d7d1ce9874b6768d8e8` and remain present on final
exact head `f9283fe624a479796ee1dcba35e5e95fde182ab2`.

### Release boundary and residual-risk disposition

- Stable OpenShell `0.0.72` is the only supported and shipped OpenShell
version for NemoClaw `v0.0.74`; installer, blueprint, workflow, and
credential-boundary data are aligned to that version.
- OpenShell `0.0.72` attributes some script clients to Node/Python
interpreters rather than immutable package entrypoints. Exact
destination, literal path, MCP method profile, request-size bound,
public-IP pins, provider ownership, and runtime checks narrow that
grant. Remove interpreter grants when OpenShell exposes stable
package-entrypoint attribution.
- Static provider credentials are sandbox-scoped rather than
endpoint-exclusive. Every managed server requires a unique credential
name and a least-privilege token; operators must not grant the same
runtime a broader route capable of resolving that placeholder.
- Supported NemoClaw writers are serialized, and external drift visible
before or after mutation is rejected by exact reads. A direct external
`openshell policy set` does not participate in NemoClaw's lock, and
OpenShell `0.0.72` exposes no policy compare-and-swap/version
precondition. If an external set lands in the narrow interval after the
fresh read and loses the final write race, it can be overwritten.
Closing that arbitrary-client race requires an upstream OpenShell CAS
primitive; this PR does not claim atomicity beyond supported NemoClaw
writers.
- OpenShell policy does not bind URL scheme, HTTP `Host`, or query
parameters. NemoClaw therefore accepts only canonical HTTPS URLs and
rejects userinfo, query, fragment, percent characters, credential-shaped
paths, and unsupported aliases.
- Stable OpenShell cannot safely parse bracketed IPv6 literals in HTTPS
CONNECT targets. Direct IPv6 literals are rejected; DNS AAAA
destinations remain supported through resolve/validate/connect and exact
public-address pins.
- The explicit compatibility-only dev lane consumed mutable OpenShell
`0.0.76-dev.3+g6461677c`. OpenClaw and Deep Agents passed, but Hermes
rebuild exited during pre-delete MCP preservation. The same exact
NemoClaw head passed the identical Hermes rebuild with supported stable
`0.0.72`. Because the dev lane is non-default, unverified, outside the
`v0.0.74` support range, and not a required PR check, this PR does not
add speculative compatibility code for the moving artifact;
compatibility should be re-evaluated with OpenShell before NemoClaw
widens its supported range.

### Exact-head verification

- PR head `f9283fe624a479796ee1dcba35e5e95fde182ab2` is signed and
GitHub-verified, based on current `main`
`fdf1d585666c95429c4ca2222288dd8a5ce7830a`, and labeled `v0.0.74`.
- The final restack integrates current-main DCode `0.1.30` runtime
hardening while preserving managed MCP in interactive and headless
paths. The complete sandbox-writable MCP file is validated fail-closed
as HTTPS-only registry-owned shape before launch; stdio commands, extra
headers, raw credentials, mismatched placeholders, unsafe
ownership/mode, and unrelated top-level configuration are rejected.
- Final local validation passed build, typecheck, all 44 config schemas,
repository checks, commit/push hooks, 290/290 focused CLI tests, 192/192
MCP integration tests, 77/77 DCode image-contract tests, and the
affected DCode direct-entrypoint, snapshot, rebuild, proxy, and
legacy-lifecycle suites. The third-party-notice harness passes 14/14
DCode rebuild tests. Two Linux-only test fixtures needed adaptation
after the current-main DCode wrapper added isolated managed-MCP
validation: `bf9100f` stubs every isolated Python invocation in the
empty-prompt fixture, and `f9283fe` uses the established exact validator
stub in the identity fixture before its existing launch stub. Both fixes
are test-only; formatting, repository growth, fixture-transform, commit,
and push guards pass.
- Exact-head [CI / Pull Request run
28683600527](https://github.com/NVIDIA/NemoClaw/actions/runs/28683600527)
and the standard PR check rollup are in progress.
- Exact-head selective E2E runs
[28683610226](https://github.com/NVIDIA/NemoClaw/actions/runs/28683610226)
(`mcp-bridge`, `hermes-e2e`) and
[28683611376](https://github.com/NVIDIA/NemoClaw/actions/runs/28683611376)
(`ubuntu-repo-cloud-langchain-deepagents-code`) are in progress.
- Exact-head [Sandbox Images and E2E run
28683612355](https://github.com/NVIDIA/NemoClaw/actions/runs/28683612355)
is in progress.
- Results will be recorded here after completion; no required-check
waiver is requested.

GitHub reports the graph `MERGEABLE`. The prior `CHANGES_REQUESTED`
state is being re-requested after the six addressed threads are
dispositioned; no review waiver is requested.

## 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
- [x] Existing tests cover changed behavior — justification: focused
coverage exercises adapter, policy, credential, ownership, race,
crash/rebuild/destroy, Hermes transaction/reload, workflow, and
live-runtime boundaries.
- [ ] Tests not applicable — justification: not applicable; this adds
security-sensitive credential and sandbox lifecycle behavior.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification: not applicable; MCP setup and
stable limitations are user-facing.
- [x] 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: Apurv's six findings
are addressed and dispositioned; refreshed human approval is pending,
and no waiver is requested.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue: no required-check waiver
is needed. Required checks are running on the final restacked head; no
waiver is requested. The non-required mutable dev-channel compatibility
result disclosed above remains outside the `v0.0.74` release boundary.

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] 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)
- [x] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary

Hardens the managed-MCP work merged in NVIDIA#5876 so DCode rebuilds validate
every reconstructable input before crossing the destructive delete
boundary, preserve exact policy intent, and migrate legacy managed MCP
state fail-closed. Prepared rebuild artifacts and the derived MCP
runtime snapshot remain ephemeral and process-local; neither is
persisted in FSM or checkpoint state, so this does not implement NVIDIA#6224.

## Related Issue

Refs NVIDIA#5876
Refs NVIDIA#6195
Refs NVIDIA#6218

## Changes

- Revalidate DCode route, image, Dockerfile, reasoning, web-search, and
MCP inputs after preparation and before NIM stop or sandbox deletion;
restore MCP state and relock shields on failure.
- Preserve exact custom network policy replay while keeping generated
MCP rules under the MCP adapter's exclusive ownership.
- Add protocol-specific policy schema validation for REST, WebSocket,
JSON-RPC, and MCP matchers, including cross-rule `tools/call` conflict
rejection.
- Pin Deep Agents Code 0.1.30 and load only a strict, canonicalized
managed MCP projection from a process-local integrity-bound snapshot.
Sealed memfd is preferred; when OpenShell seccomp blocks it, an
anonymous `O_TMPFILE` inode is reopened read-only and bound by
descriptor, device, inode, size, kind, and SHA-256, with ambient
discovery disabled.
- Bind the canonical TypeScript secret-pattern source and flags to one
shared behavior corpus executed through the Bash and Python DCode
enforcement boundaries, including the full ECMAScript whitespace set.
- Add capability-v2 gating and legacy-v1 teardown/rollback that
preserves unrelated user configuration and fails closed on malformed,
unsafe, or drifted state.
- Add rebuild, migration, runtime-patch, schema, snapshot, and lifecycle
coverage; update the MCP, policy, security, command, and DCode
documentation.

Verification notes:

- Final DCode-adjacent run: 9 files, 187 tests passed; the focused
descriptor/projection run passed 4 files and 138 tests.
- Final review-follow-up run: 82 focused Bash/Python/TypeScript parity
and descriptor-fallback tests passed, including all 25 ECMAScript
whitespace code points under both `C` and `C.UTF-8` Bash locales.
- Full pre-squash-equivalent run: 1,068 files passed, 2 skipped; 12,149
tests passed, 35 skipped.
- CLI coverage ratchet passed with the repository include/exclude set
expressed as one Vitest glob: lines 65.24%, statements 64.45%, functions
67.06%, branches 57.21%.
- Python compile, Biome, ShellCheck, shfmt, source-shape, test-size,
repository, secret-scan, and diff checks passed. The normal push hook
passed CLI typechecking.
- Main-sync validation after merging NVIDIA#6265 passed: 9 CLI files/82 tests,
6 integration files/174 tests, an additional 3 preparation tests, CLI
typecheck, Biome, and diff checks. Generated-context provenance was
ported into the split preflight fixtures without restoring the obsolete
monolith.
- Exact-head CI for `9a31537785ef2d456901de622721ed215627fdec` passed:
40 checks green, all five required contexts passed, and there were 0
failures, cancellations, or pending checks. The only skips were the
expected docs-only job and two duplicate NVSkills request jobs. This
includes all five CLI shards plus the aggregate, both CodeQL languages,
both sandbox image builds, macOS, WSL, four self-hosted runtime checks,
CodeRabbit, and both review advisors.
- Exact-head live E2E for `9a31537785ef2d456901de622721ed215627fdec`
passed:
[`mcp-bridge`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844701),
[`mcp-bridge-dev`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844719),
and
[`ubuntu-repo-cloud-langchain-deepagents-code`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844639).
Stable and dev each passed OpenClaw, Hermes, and DCode 3/3;
authenticated MCP calls passed initially and after restart, credential
rotation, and rebuild, then removal denied access with no provider,
policy, tunnel, or credential residue. The dedicated DCode lane passed
Landlock 5/5, Python egress 14/14, headless inference 10/10, secret
boundary 8/8, Tavily 6/6, and TUI 4/4; BuildKit accepted the merged
generated-context handoff, and invalid-credential rebuild failure
remained pre-destructive with the original sandbox, marker, and route
recovered. Artifact inspection found one unchanged pre-existing harness
defect: two OpenShell audit-log filtering subassertions can false-pass
because awk treats `close` as reserved; runtime-output, sandbox-log,
env-file immutability, and raw-secret checks passed, and this PR does
not modify that E2E file.
- The base `test-cli` pre-commit invocation remains affected by Vitest
4.1.9 collapsing repeated `--coverage.exclude` arguments to a
zero-file/invalid summary. All other commit and push hooks passed;
targeted tests and the authoritative sharded CI coverage checks provide
the exact-head gate.
- `npm run docs` completed with 0 errors and 2 pre-existing Fern
warnings. Two documentation-writer audits confirmed the final behavior
is accurately documented.

## 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 security
and correctness reviews passed after fixes; destructive-boundary
rollback, capability migration, the process-local integrity-bound
snapshot handoff (sealed memfd preferred, anonymous `O_TMPFILE`
fallback), cross-language secret-pattern parity, policy fidelity, and
the NVIDIA#6224 boundary were checked.
- [ ] 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
- [x] 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)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Enhanced managed MCP bridge support with managed-only configuration
snapshots for safer add/restart/rebuild/teardown.
* Network policy protocol rules now support protocol-specific matching
plus stricter `endpoint.path` validation.
* **Bug Fixes**
* Stronger fail-fast validation for MCP server names/hostnames and
endpoint details (rejected before changes are applied).
* Rebuild flows improved to preserve/replay custom policies and validate
after MCP preparation, with rollback on failure.
* **Documentation**
* Updated setup/quickstart/reference and MCP bridge/rebuild guidance for
managed MCP capability v2 behavior and stricter validation rules.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds a shared tool-disclosure mode across Deep Agents Code, Hermes, and
OpenClaw. New sandboxes default to `progressive`; `--tool-disclosure
direct` or `NEMOCLAW_TOOL_DISCLOSURE=direct` restores the prior fully
visible catalog. The mode persists through resume and transactional
rebuilds.

The design follows [LangChain progressive
disclosure](https://support.langchain.com/articles/8488719552-progressive-tool-disclosure-with-deep-agents),
[Hermes Tool
Search](https://hermes-agent.nousresearch.com/docs/user-guide/features/tool-search),
and the pinned [OpenClaw v2026.5.27 Tool Search
contract](https://github.com/openclaw/openclaw/blob/v2026.5.27/docs/tools/tool-search.md).
This is the v0.0.74 bundle.

Exact head `e4214144c11dd47b9d3185f19c1c73ec4e690552` includes `main` at
`6f5ccbcbaa87367f7b521318fbc940467045b601`.

## Related Issue

NVIDIA#5876 is merged. This PR has no remaining stacked dependency and
preserves its managed-MCP ownership contract.

## Changes

- Add the shared `--tool-disclosure progressive|direct` onboarding and
rebuild option, with `NEMOCLAW_TOOL_DISCLOSURE` as the environment
equivalent. CLI input takes precedence and fresh sandboxes default to
`progressive`.
- Persist the selected mode in session and registry state. Resume,
recreation, and MCP-bearing rebuilds preserve or transactionally change
it in either direction.
- Add bounded `search_tools` middleware to Deep Agents Code when at
least one MCP tool loads successfully. Core tools remain visible,
discovery state survives checkpointing, and only model requests are
filtered; the complete executor registry and existing authorization
controls remain intact.
- Enable Hermes native Tool Search with opinionated 5/20 result limits
and direct core tools.
- Enable OpenClaw native structured `mode: "tools"` discovery with 8/20
limits while retaining model-specific compatibility safeguards.
- Patch and validate the exact pinned `deepagents-code==0.1.30` runtime
with fail-closed, idempotent anchors and separate middleware instances
for the main agent and local subagents.
- Add build-time/runtime validators, lifecycle and overflow coverage,
transactional custom-image handling, and user-facing documentation for
progressive/direct behavior.

### Managed MCP boundary

This PR does not change MCP configuration, credential placeholders,
bridge ownership, policy generation, MCP CLI behavior, or registry
schemas from NVIDIA#5876.

Managed OpenClaw MCP remains registered in mcporter's home registry.
Pinned OpenClaw Tool Search reads native bundle and `mcp.servers`
catalogs, and NemoClaw does not currently synchronize those ownership
stores. The OpenClaw runtime validator therefore proves native search,
describe, and call behavior for eligible hidden catalog tools, while the
existing live MCP scenario separately proves mcporter execution,
credential rewriting and rotation, restart/rebuild behavior, policy
enforcement, DNS-rebinding protection, and secret boundaries. A
fixture-only projection between those stores would not represent shipped
behavior.

### Apurv security review resolution

1. **Callable namespace:** Deep Agents Code rejects every duplicate
resolved callable name and every non-managed owner of a reserved core
name before the original factory in progressive and direct modes.
Coverage includes regular/regular, regular/MCP, cross-MCP-server,
direct-mode, and schema/executor mismatch cases.
2. **Build-context seal:** fingerprints require a real directory root
and encode portable mode bits, nanosecond modification times, link
counts, and deterministic hardlink topology. Pre-delete and final
one-shot tests cover hardlink, timestamp, permission, and root-symlink
mutations.
3. **Dockerfile patch boundary:** patching anchors and revalidates the
staging parent, rejects multi-link files, writes a fresh private
same-directory file, and atomically replaces the staged Dockerfile
without truncating an attacker-selectable inode.
4. **Chunk-safe diagnostics:** raw cloudflared child output is not
emitted in failures. Only bounded per-stream carry is retained for
origin discovery, with split-event credential regressions proving
fragments and reconstructed secrets remain absent.

### CodeRabbit and CodeQL resolution

- Every CodeRabbit finding is implemented or explicitly dispositioned
with repository evidence, including URL-token punctuation, the
intentional private hosted model identifier, and the final Darwin
fixture fail-fast nit. GraphQL reports zero unresolved review threads;
exact-head CodeRabbit status is green.
- [CodeQL alert
NVIDIA#1210](https://github.com/NVIDIA/NemoClaw/security/code-scanning/1210)
is dismissed as a false positive. The flagged sink opens an existing
directory read-only with `O_DIRECTORY | O_NOFOLLOW | O_NONBLOCK`; it
does not use `O_CREAT`. The real replacement file remains UUID-named,
`O_CREAT | O_EXCL | O_NOFOLLOW`, mode `0600`, descriptor/path identity
checked, single-link validated, fsynced, and atomically renamed.
Exact-head CodeQL is green with no open branch alerts.

### Validation

- Local focused matrix: 19 files / 327 tests passed; one true Linux-only
`memfd`/`O_TMPFILE` restart case was skipped on macOS.
- Exact pinned Deep Agents Code/LangChain patch and runtime validator
passed: `progressive-disclosure-runtime-ok`.
- Both TypeScript typechecks, build, repository checks,
source-shape/test-size guards, Biome, Ruff/Python compilation, secret
scanning, push checks, and `git diff --check` passed.
- Final-head standard validation is green: [PR
CI](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184882),
[growth
guardrails](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184428),
[WSL](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184879),
[macOS](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184874),
[security](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184917),
[CodeQL](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184138),
[review
advisors](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184863),
and the [E2E
advisor](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184880).
The original exact-head check set and subsequent metadata checks are all
green, with only the two expected skips.
- The exact-head [typed OpenClaw and Deep Agents
targets](https://github.com/NVIDIA/NemoClaw/actions/runs/28700290435)
passed.
- The exact-head [production-image
workflow](https://github.com/NVIDIA/NemoClaw/actions/runs/28700292369)
passed both requested image builds and all four downstream image E2E
checks.
- In the exact-head [advisor-selected live
union](https://github.com/NVIDIA/NemoClaw/actions/runs/28700286465), 18
of 19 concrete jobs passed. The sole Hermes security-posture failure
occurred in its deliberate gateway-recovery probe after restart,
secret-boundary, and disclosure assertions had passed. The isolated
exact-head [security
retry](https://github.com/NVIDIA/NemoClaw/actions/runs/28700509914)
passed both Hermes and OpenClaw legs, so every selected live lane has
passing final-head evidence.
- The GPT review advisor's remaining OpenClaw MCP model-loop warning is
explicitly dispositioned by the managed-MCP ownership boundary above:
the native OpenClaw catalog and mcporter registry are separate shipped
stores, and a test-only projection would claim behavior the production
bridge does not provide.

### Remaining

- Obtain Apurv's exact-head re-review to clear the formal
`CHANGES_REQUESTED` state and complete sensitive-path approval.
- Rebuild affected existing sandboxes after this change ships; newly
created sandboxes default to progressive disclosure.

## 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)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Apurv exact-head
re-review pending after all four requested fixes.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue: no waiver requested;
exact-head security retry is green.

## Verification

- [x] PR description includes the DCO sign-off declaration and every
feature commit appears as `Verified` in GitHub
- [ ] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes — all applicable hooks passed; the
known hanging local unsharded `test-cli` coverage hook was skipped and
exact-head sharded Linux CI is authoritative.
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [ ] 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; current
build has two pre-existing warnings)
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [x] New doc pages include SPDX header and frontmatter (new pages only;
no new doc pages)

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.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
<!-- markdownlint-disable MD041 -->
## Summary
This PR prepares the user-facing documentation for v0.0.74 before the
release plan is frozen.
It expands the release notes across the 56-commit train and closes
durable documentation gaps found during the pre-tag commit scan.

## Changes
- Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed
MCP, progressive tool disclosure, LangChain Deep Agents Code,
onboarding, local inference, messaging, recovery, and contributor
workflows.
- Correct the `destroy` contract for retained per-name volumes,
gateway-unreachable `--force` cleanup, managed MCP ownership, and
same-name recovery.
- Document separate remediation for an unreachable container DNS
resolver versus one that answers with `NXDOMAIN` or `REFUSED`.
- Document the Windows on Arm N1X automatic Ollama safeguard and its
remaining large-model limitations.
- State that messaging conflicts abort rebuild before backup or
deletion, leaving the original sandbox intact.
- Link the agent-runnable value benchmark from the contributor task
index.
- Synchronize generated agent command variants.
- Validate with `npm run docs:sync-agent-variants` and `npm run docs`;
Fern completed with 0 errors and 2 existing warnings.
- Source summary:
- [NVIDIA#6020](NVIDIA#6020) and
[NVIDIA#5876](NVIDIA#5876) ->
`docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy
boundary and managed MCP lifecycle.
- [NVIDIA#6251](NVIDIA#6251) and
[NVIDIA#5989](NVIDIA#5989) ->
`docs/about/release-notes.mdx`: Summarize progressive tool disclosure
and sandbox-first inference controls.
- [NVIDIA#6232](NVIDIA#6232),
[NVIDIA#6082](NVIDIA#6082),
[NVIDIA#6219](NVIDIA#6219),
[NVIDIA#6214](NVIDIA#6214),
[NVIDIA#6215](NVIDIA#6215),
[NVIDIA#6230](NVIDIA#6230), and
[NVIDIA#6260](NVIDIA#6260) ->
`docs/about/release-notes.mdx`: Summarize the experimental LangChain
Deep Agents Code status, secret, version, rebuild, snapshot, and MCP
boundaries.
- [NVIDIA#6166](NVIDIA#6166),
[NVIDIA#6254](NVIDIA#6254),
[NVIDIA#6265](NVIDIA#6265),
[NVIDIA#6164](NVIDIA#6164), and
[NVIDIA#6017](NVIDIA#6017) ->
`docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated
image reuse, bounded readiness, and preflight improvements.
- [NVIDIA#6150](NVIDIA#6150) ->
`docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`:
Separate unreachable-resolver remediation from reachable-but-rejected
DNS responses.
- [NVIDIA#6234](NVIDIA#6234) ->
`docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`, and
`docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B
selection and the remaining explicit-large-model boundary.
- [NVIDIA#6129](NVIDIA#6129),
[NVIDIA#5987](NVIDIA#5987),
[NVIDIA#5955](NVIDIA#5955), and
[NVIDIA#6220](NVIDIA#6220) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Document messaging policy
persistence, status, and the pre-destructive conflict check.
- [NVIDIA#5963](NVIDIA#5963),
[NVIDIA#6050](NVIDIA#6050),
[NVIDIA#6094](NVIDIA#6094),
[NVIDIA#6238](NVIDIA#6238),
[NVIDIA#5988](NVIDIA#5988),
[NVIDIA#6235](NVIDIA#6235),
[NVIDIA#6181](NVIDIA#6181), and
[NVIDIA#5986](NVIDIA#5986) ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and
clarify retained-volume and local-only destroy semantics.
- [NVIDIA#6200](NVIDIA#6200),
[NVIDIA#6248](NVIDIA#6248),
[NVIDIA#6168](NVIDIA#6168),
[NVIDIA#6270](NVIDIA#6270), and
[NVIDIA#5649](NVIDIA#5649) ->
`docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize
contributor setup and verification improvements and expose the advisory
value benchmark.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: documentation-only release
preparation; generated-variant synchronization and the Fern docs build
validate the changed pages and routes.
- [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:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: tests
are not applicable to this documentation-only change; `npm run docs`
validates the source and generated routes.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Expanded setup guidance for Windows on Arm devices with safer default
local model selection.
* Clarified local inference and sandbox messaging behavior, including
conflict checks before rebuilds and safer recovery steps.
* Updated destroy/rebuild/reference docs with more detailed warnings,
failure handling, and volume-retention guidance.
* Improved troubleshooting instructions for Docker DNS issues with
clearer paths for unreachable vs. blocked resolvers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: integrations Third-party service integration behavior area: security Security controls, permissions, secrets, or hardening feature PR adds or expands user-visible functionality integration: dcode LangChain Deep Code integration behavior integration: hermes Hermes integration behavior integration: openclaw OpenClaw integration behavior v0.0.74 Release target VDR Linked to VDR finding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: MCP server bridge for host-to-sandbox credential isolation

4 participants