fix(mcp): harden DCode rebuild handoff#6260
Merged
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughManaged MCP handling for Deep Agents Code now uses a versioned projection file, sealed descriptor-based runtime validation, and updated adapter wiring. Rebuild flows now preserve web-search settings and custom policies across backup, restore, and destroy phases. Network policy schemas now validate REST, WebSocket, JSON-RPC, and MCP rules with protocol-specific matchers. ChangesDeep Agents Code Managed MCP v2
Rebuild Web-Search and Custom Policy Preservation
Protocol-Specific Network Policy Schemas
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe 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.
TypeScript / code-coverage/cliThe 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.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6260.docs.buildwithfern.com/nemoclaw |
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/mcp-bridge-adapter-deepagents.ts (1)
232-240: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSymlink reads need the same O_NOFOLLOW guard as legacy.
The v2 register/rollback/remove paths still do
is_symlink()and then reopen withread_text(), so a path swap between the check and the read can redirect the config read. Use the legacyO_NOFOLLOW/fingerprint pattern for the managed projection too.🤖 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/actions/sandbox/mcp-bridge-adapter-deepagents.ts` around lines 232 - 240, The managed projection read path still relies on is_symlink() followed by read_text(), which leaves a race window before the file is reopened. Update the v2 register/rollback/remove handling in mcp-bridge-adapter-deepagents to use the same O_NOFOLLOW plus fingerprint verification pattern as the legacy path, so the config is opened safely and validated after the open rather than before it. Keep the existing config_path validation flow, but route the actual read through the hardened helper logic used for the legacy projection.
🧹 Nitpick comments (5)
src/lib/actions/sandbox/rebuild-durable-config.ts (1)
28-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
DCODE_AGENT_NAMEinstead of duplicating the literal.
rebuild-durable-config.tsre-declares the same"langchain-deepagents-code"value already exported fromrebuild-dcode-target.ts; share the export to keep the agent name authoritative.🤖 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/actions/sandbox/rebuild-durable-config.ts` around lines 28 - 29, The agent name literal is duplicated in rebuild-durable-config.ts instead of reusing the shared export. Update the code in rebuild-durable-config.ts to import DCODE_AGENT_NAME from rebuild-dcode-target.ts and use that symbol directly so the name stays authoritative in one place.Source: Path instructions
src/lib/actions/sandbox/rebuild-destroy-phase.ts (1)
71-80: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog the swallowed error before falling back to the generic message.
checkAtDeleteEdgealready converts expectedbailoutcomes into{ ok: false, message, code }; it only throws for genuinely unexpected faults. Discarding that error here collapses the only diagnostic for an unexpected failure on the destructive pre-delete path into a fixed string, making a rare rebuild abort hard to triage.🩹 Proposed change to retain diagnostics
try { validation = await validateAfterMcpPreparation(); - } catch { + } catch (error) { + log( + `DCode delete-edge validation threw unexpectedly: ${error instanceof Error ? error.message : String(error)}`, + ); validation = { ok: false, message: "DCode replacement validation failed before sandbox deletion.", }; }🤖 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/actions/sandbox/rebuild-destroy-phase.ts` around lines 71 - 80, The validation fallback in rebuild-destroy-phase should not swallow unexpected errors from validateAfterMcpPreparation. In the try/catch around validateAfterMcpPreparation, capture the thrown error and log it before assigning the generic RebuildDeleteValidationResult fallback, so the destructive pre-delete path retains diagnostics. Use the existing validateAfterMcpPreparation and validation handling in rebuild-destroy-phase to add the error details to the log while still returning the same fallback message.src/lib/actions/sandbox/mcp-bridge-adapter-deepagents.ts (1)
79-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
64duplicatesDEEPAGENTS_MCP_MAX_SERVERS.The rollback script embeds the server cap as a literal
64in the generated Python ("if is_v2 and len(payload['expectedServers']) > 64:") instead of interpolating theDEEPAGENTS_MCP_MAX_SERVERSconstant already defined in this file. If the constant ever changes, the two values will silently drift.♻️ Suggested fix
- "if is_v2 and len(payload['expectedServers']) > 64:", - " print('Managed MCP v2 supports at most 64 servers', file=sys.stderr)", + `if is_v2 and len(payload['expectedServers']) > ${String(DEEPAGENTS_MCP_MAX_SERVERS)}:`, + ` print('Managed MCP v2 supports at most ${String(DEEPAGENTS_MCP_MAX_SERVERS)} servers', file=sys.stderr)`,🤖 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/actions/sandbox/mcp-bridge-adapter-deepagents.ts` around lines 79 - 113, The rollback command in buildDeepAgentsMcpRollbackRegisterCommand hardcodes the Managed MCP server limit as 64, which can drift from DEEPAGENTS_MCP_MAX_SERVERS. Replace the literal in the generated Python check with the shared DEEPAGENTS_MCP_MAX_SERVERS constant from this module, so the limit stays consistent wherever the rollback script is built.src/lib/actions/sandbox/mcp-bridge-destroy.ts (1)
108-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared scrub/rollback helper to avoid destroy/rebuild drift.
The scrub-adapter-with-unowned-check loop and the corresponding rollback registration loop are duplicated almost verbatim between
prepareMcpBridgesForDestroyhere andprepareMcpBridgesForRebuildinmcp-bridge-rebuild.ts. This PR had to land the same hardening in both places; a shared helper (e.g.scrubManagedAdapterOrThrow/rollbackScrubbedAdapters) would remove that duplication risk for future changes.🤖 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/actions/sandbox/mcp-bridge-destroy.ts` around lines 108 - 210, The scrub-and-rollback logic in prepareMcpBridgesForDestroy is duplicated with prepareMcpBridgesForRebuild, so extract the shared adapter scrub and rollback behavior into a common helper. Move the repeated unregisterAgentAdapter/isAgentMcpAdapter/getBridgeAdapter flow and the rollback registerAgentAdapter flow into reusable functions such as scrubManagedAdapterOrThrow and rollbackScrubbedAdapters, then call them from both prepareMcpBridgesForDestroy and prepareMcpBridgesForRebuild to keep the destroy/rebuild paths in sync.agents/langchain-deepagents-code/patch-managed-deepagents-code.py (1)
532-593: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSealed-descriptor validation duplicated between
SERVER_POPEN_PATCHand the helper's_sealed_managed_mcp_snapshot.The
required_sealsbitmask and fstat/seal-check logic inSERVER_POPEN_PATCH(Lines 561-583) reimplement the same checks performed when the descriptor is originally sealed in_sealed_managed_mcp_snapshot(Lines 1156-1194). If the required seal set or size cap changes in one location without the other, the server subprocess boundary could silently accept a descriptor that no longer matches the sealing invariant the rest of the system assumes.Consider exposing a single
validate_sealed_mcp_descriptor(descriptor: int) -> Nonehelper in_nemoclaw_managed.py(or a shared constant forrequired_seals/262_144) and importing it from the injectedSERVER_POPEN_PATCH, so both call sites stay in lockstep.♻️ Illustrative direction (not a drop-in diff, since both blocks are raw string patches)
-required_seals = ( - fcntl.F_SEAL_WRITE - | fcntl.F_SEAL_GROW - | fcntl.F_SEAL_SHRINK - | fcntl.F_SEAL_SEAL -) +from deepagents_code._nemoclaw_managed import REQUIRED_MCP_SEALS as required_seals🤖 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 `@agents/langchain-deepagents-code/patch-managed-deepagents-code.py` around lines 532 - 593, `SERVER_POPEN_PATCH` duplicates the sealed MCP descriptor validation already enforced by `_sealed_managed_mcp_snapshot`, so the two paths can drift. Extract the seal/size/fstat checks into a shared helper such as `validate_sealed_mcp_descriptor(descriptor: int) -> None` in `_nemoclaw_managed.py` (or shared constants for `required_seals` and the size cap), then call that helper from both `_sealed_managed_mcp_snapshot` and the `SERVER_POPEN_PATCH` startup path before `subprocess.Popen`. Keep the `pass_fds` wiring in `SERVER_POPEN_PATCH`, but move the validation logic out so both call sites stay consistent.
🤖 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 `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents.ts`:
- Around line 226-227: The register path in mcp-bridge-adapter-deepagents still
shells out through the bare python3 interpreter, unlike the rollback/remove
flows that use the isolated /opt/venv/bin/python3 -I invocation. Update the
command construction in the registration logic so it uses the same isolated
interpreter and flags as the other paths, keeping the behavior consistent in the
managed MCP projection and avoiding PATH/PYTHONPATH or local-site shadowing.
---
Outside diff comments:
In `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents.ts`:
- Around line 232-240: The managed projection read path still relies on
is_symlink() followed by read_text(), which leaves a race window before the file
is reopened. Update the v2 register/rollback/remove handling in
mcp-bridge-adapter-deepagents to use the same O_NOFOLLOW plus fingerprint
verification pattern as the legacy path, so the config is opened safely and
validated after the open rather than before it. Keep the existing config_path
validation flow, but route the actual read through the hardened helper logic
used for the legacy projection.
---
Nitpick comments:
In `@agents/langchain-deepagents-code/patch-managed-deepagents-code.py`:
- Around line 532-593: `SERVER_POPEN_PATCH` duplicates the sealed MCP descriptor
validation already enforced by `_sealed_managed_mcp_snapshot`, so the two paths
can drift. Extract the seal/size/fstat checks into a shared helper such as
`validate_sealed_mcp_descriptor(descriptor: int) -> None` in
`_nemoclaw_managed.py` (or shared constants for `required_seals` and the size
cap), then call that helper from both `_sealed_managed_mcp_snapshot` and the
`SERVER_POPEN_PATCH` startup path before `subprocess.Popen`. Keep the `pass_fds`
wiring in `SERVER_POPEN_PATCH`, but move the validation logic out so both call
sites stay consistent.
In `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents.ts`:
- Around line 79-113: The rollback command in
buildDeepAgentsMcpRollbackRegisterCommand hardcodes the Managed MCP server limit
as 64, which can drift from DEEPAGENTS_MCP_MAX_SERVERS. Replace the literal in
the generated Python check with the shared DEEPAGENTS_MCP_MAX_SERVERS constant
from this module, so the limit stays consistent wherever the rollback script is
built.
In `@src/lib/actions/sandbox/mcp-bridge-destroy.ts`:
- Around line 108-210: The scrub-and-rollback logic in
prepareMcpBridgesForDestroy is duplicated with prepareMcpBridgesForRebuild, so
extract the shared adapter scrub and rollback behavior into a common helper.
Move the repeated unregisterAgentAdapter/isAgentMcpAdapter/getBridgeAdapter flow
and the rollback registerAgentAdapter flow into reusable functions such as
scrubManagedAdapterOrThrow and rollbackScrubbedAdapters, then call them from
both prepareMcpBridgesForDestroy and prepareMcpBridgesForRebuild to keep the
destroy/rebuild paths in sync.
In `@src/lib/actions/sandbox/rebuild-destroy-phase.ts`:
- Around line 71-80: The validation fallback in rebuild-destroy-phase should not
swallow unexpected errors from validateAfterMcpPreparation. In the try/catch
around validateAfterMcpPreparation, capture the thrown error and log it before
assigning the generic RebuildDeleteValidationResult fallback, so the destructive
pre-delete path retains diagnostics. Use the existing
validateAfterMcpPreparation and validation handling in rebuild-destroy-phase to
add the error details to the log while still returning the same fallback
message.
In `@src/lib/actions/sandbox/rebuild-durable-config.ts`:
- Around line 28-29: The agent name literal is duplicated in
rebuild-durable-config.ts instead of reusing the shared export. Update the code
in rebuild-durable-config.ts to import DCODE_AGENT_NAME from
rebuild-dcode-target.ts and use that symbol directly so the name stays
authoritative in one place.
🪄 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: ab00d379-3a80-4b24-9856-98e861adb76e
📒 Files selected for processing (49)
agents/langchain-deepagents-code/dcode-wrapper.shagents/langchain-deepagents-code/manifest.yamlagents/langchain-deepagents-code/patch-managed-deepagents-code.pydocs/deployment/set-up-mcp-bridge.mdxdocs/get-started/quickstart-langchain-deepagents-code.mdxdocs/network-policy/customize-network-policy.mdxdocs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxdocs/security/best-practices.mdxschemas/policy-preset.schema.jsonschemas/sandbox-policy.schema.jsonsrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents.tssrc/lib/actions/sandbox/mcp-bridge-adapter-inspection.tssrc/lib/actions/sandbox/mcp-bridge-adapter-status.tssrc/lib/actions/sandbox/mcp-bridge-adapters.tssrc/lib/actions/sandbox/mcp-bridge-destroy.tssrc/lib/actions/sandbox/mcp-bridge-input-validation.test.tssrc/lib/actions/sandbox/mcp-bridge-rebuild.tssrc/lib/actions/sandbox/mcp-bridge-remove.tssrc/lib/actions/sandbox/mcp-bridge-restart.tssrc/lib/actions/sandbox/mcp-bridge-url-validation.tssrc/lib/actions/sandbox/rebuild-backup-phase.test.tssrc/lib/actions/sandbox/rebuild-backup-phase.tssrc/lib/actions/sandbox/rebuild-dcode-flow.test.tssrc/lib/actions/sandbox/rebuild-dcode-orchestrator.tssrc/lib/actions/sandbox/rebuild-dcode-preflight.tssrc/lib/actions/sandbox/rebuild-destroy-phase.tssrc/lib/actions/sandbox/rebuild-durable-config.test.tssrc/lib/actions/sandbox/rebuild-durable-config.tssrc/lib/actions/sandbox/rebuild-managed-image-preflight.test.tssrc/lib/actions/sandbox/rebuild-managed-image-preflight.tssrc/lib/actions/sandbox/rebuild-mcp-order.test.tssrc/lib/actions/sandbox/rebuild-mcp-order.tssrc/lib/actions/sandbox/rebuild-pipeline.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.tssrc/lib/actions/sandbox/rebuild-preflight-phase.tssrc/lib/actions/sandbox/rebuild-restore-phase.test.tssrc/lib/actions/sandbox/rebuild-restore-phase.tstest/deepagents-mcp-legacy-lifecycle.test.tstest/deepagents-mcp-runtime-capability.test.tstest/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.shtest/e2e/live/mcp-bridge.test.tstest/helpers/rebuild-flow-harness.tstest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-managed-entrypoints.test.tstest/snapshot.test.tstest/validate-config-schemas.test.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com> # Conflicts: # src/lib/actions/sandbox/rebuild-dcode-orchestrator.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
Review follow-up —
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28693440273
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts (1)
26-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSource-text assertions duplicate the executed race/attack tests below.
This test asserts on literal substrings inside the generated Python source (
os.O_NOFOLLOW,os.fstat(descriptor), etc.) without executing the command, which is implementation lock-in rather than an observable-behavior check. The later tests in this file (duplicate-JSON, symlink, race-condition cases) already exercise these same guarantees behaviorally by running the generated command and asserting on exit status/stderr/file state — consider relying on those and trimming this to only what can't be proven behaviorally (e.g., the isolated-interpreter prefix check).🤖 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/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts` around lines 26 - 47, The test in mcp-bridge-adapter-deepagents-projection.test.ts is over-asserting generated Python source strings instead of behavior, and it duplicates the protection already covered by the executed race/attack tests. Trim the assertions in the affected `it("uses the isolated runtime and one stable-read contract for every v2 mutation")` case to only check what can’t be verified behaviorally, such as the `buildDeepAgentsMcpStatusCommand`/registration command isolated-interpreter prefix, and remove the literal substring checks for internals like `os.O_NOFOLLOW`, `os.fstat(descriptor)`, `assert_managed_source_stable(path, identity)`, `os.link(...)`, and `os.ftruncate(...)`.Source: Path instructions
src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-command.ts (1)
10-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffMove the sandbox process call behind an adapter
runDeepAgentsAdapterCommandstill invokesexecuteSandboxCommandfromsrc/lib/actions/sandbox, whilesrc/lib/adapters/README.mdsays host-boundary calls belong in adapters. If this helper is meant to own execution, relocate it undersrc/lib/adapters/**and keepactionsorchestration-only.🤖 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/actions/sandbox/mcp-bridge-adapter-deepagents-command.ts` around lines 10 - 28, runDeepAgentsAdapterCommand is still making a host-boundary sandbox call directly through executeSandboxCommand, which belongs in an adapter layer. Move the execution responsibility into an adapter under src/lib/adapters/** and keep this helper in actions limited to orchestration/redaction/error handling. Preserve the current behavior by having runDeepAgentsAdapterCommand call the new adapter entry point instead of importing executeSandboxCommand directly, and update any related adapter symbols such as McpBridgeError and redactBridgeSecretsForDisplay usage as needed.Source: Path instructions
src/lib/actions/sandbox/rebuild-dcode-mutation-edge.test.ts (1)
55-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrdering assertions rely on positional destructuring without a length guard.
invocationCallOrderis destructured into four named variables assuming exactly four calls occurred; if the implementation's call count regresses, destructured values becomeundefinedandtoBeLessThancomparisons will fail with a less-informative error rather than clearly reporting a call-count mismatch. This is low-risk sincetoHaveBeenCalledTimes(4)at line 37 already validates the count before the destructuring.🤖 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/actions/sandbox/rebuild-dcode-mutation-edge.test.ts` around lines 55 - 78, The ordering assertions in the rebuild-dcode-mutation-edge test rely on destructuring invocationCallOrder without an explicit guard, so keep the existing call-count check before using firstRouteOrder, preBackupRouteOrder, preMcpRouteOrder, and deleteEdgeRouteOrder. In the test around harness.preflightDcodeRouteSpy and the other spy orders, make sure the sequence assertions only run after verifying the expected number of calls so failures report a clear count mismatch rather than undefined comparison noise.test/langchain-deepagents-code-managed-mcp-hardening.test.ts (1)
44-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftPrefer the managed runtime’s public boundary over private globals.
These assertions call underscored helpers and mutate
_MANAGED_MCP_*state directly, which locks the test to implementation shape. Where possible, set up descriptor/config inputs and assert the observablemanaged_mcp_config_path()/managed_mcp_server_descriptor()outcomes instead.As per path instructions, tests should prefer “observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions.”
Also applies to: 86-99
🤖 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/langchain-deepagents-code-managed-mcp-hardening.test.ts` around lines 44 - 49, The test is asserting behavior through private helpers and globals instead of the managed runtime’s public boundary. Update the affected cases to drive setup through descriptor/config inputs and verify observable outcomes via managed_mcp_config_path() and managed_mcp_server_descriptor() rather than calling _canonicalize_managed_mcp_config(), _read_managed_mcp_config(), or mutating _MANAGED_MCP_* directly. Use the same approach in the other referenced block so the test stays aligned with the public contract of the managed runtime.Source: Path instructions
src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-runtime-guards.test.ts (1)
87-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the command-specific expectations.
Line 93 allows either command to pass on any status
2, which can hide an unrelated failure while still claiming “does not mutate.” Assert the teardown and rollback outcomes separately.Proposed test tightening
- for (const command of [ - buildDeepAgentsMcpRemoveCommand(baseEntry, true, true), - buildDeepAgentsMcpRegisterCommand(baseEntry, true, [baseEntry], true), - ]) { - const result = runDeepAgentsConfigCommand(command, undefined, "legacy", legacyConfig, 0o644); - expect(result.legacyConfigText).toBe(original); - expect(result.status === 2 || result.stdout.includes("REMOVAL=unowned")).toBe(true); - } + const removal = runDeepAgentsConfigCommand( + buildDeepAgentsMcpRemoveCommand(baseEntry, true, true), + undefined, + "legacy", + legacyConfig, + 0o644, + ); + expect(removal.legacyConfigText).toBe(original); + expect(removal.status, removal.stderr).toBe(0); + expect(removal.stdout).toContain("NEMOCLAW_DEEPAGENTS_MCP_REMOVAL=unowned"); + + const rollback = runDeepAgentsConfigCommand( + buildDeepAgentsMcpRegisterCommand(baseEntry, true, [baseEntry], true), + undefined, + "legacy", + legacyConfig, + 0o644, + ); + expect(rollback.legacyConfigText).toBe(original); + expect(rollback.status, rollback.stderr).toBe(2); + expect(rollback.stderr).toContain("unsafe");As per path instructions, “Flag ... conditionals that make a test pass without exercising its claim.”
🤖 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/actions/sandbox/mcp-bridge-adapter-deepagents-runtime-guards.test.ts` around lines 87 - 94, The loop in the deep agents runtime guards test is masking command-specific failures by accepting either status 2 or a rollback marker for both commands. Split the assertions for buildDeepAgentsMcpRemoveCommand and buildDeepAgentsMcpRegisterCommand in mcp-bridge-adapter-deepagents-runtime-guards.test so each command separately verifies its expected teardown or rollback behavior, and keep the legacyConfigText unchanged check for each case without allowing a generic pass condition.Source: Path instructions
src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-legacy.ts (1)
16-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBound the legacy compatibility window with a retirement reference.
The retained v1 path has a removal condition, but no GitHub issue/PR link or observable exit criteria. Add the retirement reference and concrete criteria, or mark this as an explicitly bounded rollback window.
As per path instructions, retaining an old path requires linking the retirement issue or PR and stating observable exit criteria.
🤖 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/actions/sandbox/mcp-bridge-adapter-deepagents-legacy.ts` around lines 16 - 26, The legacy compatibility note in mcp-bridge-adapter-deepagents-legacy needs a concrete retirement anchor and exit criteria. Update the retirement guidance near the removalCondition/sourceBoundary comments to include the GitHub issue or PR that tracks deprecation, or explicitly state this is a bounded rollback window. Make the criteria observable by tying deletion of the compatibility module to specific supported-release or registry-owned v1 entry conditions, and keep the reference close to the existing legacy-path comments so it’s easy to find.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts`:
- Around line 44-46: The ordering assertion in the MCP bridge projection test
can pass even when one or both substrings are missing because it uses indexOf
directly. Update the test around registrationCommand to first assert that both
"len(payload) > MANAGED_MCP_MAX_BYTES" and "os.ftruncate(descriptor, 0)" are
present, then compare their positions so the ordering check in
src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts
actually fails when either marker is absent.
In `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.ts`:
- Around line 109-121: The managed projection rewrite in
mcp-bridge-adapter-deepagents-projection should not truncate the live file in
place, because failures in the write/fsync/verification path can leave
.nemoclaw-mcp.json empty or partial. Update the rewrite flow around the managed
projection logic to keep the opened descriptor only as the identity anchor,
write the new payload to a 0600 temp file, re-check
assert_managed_source_stable(path, identity) before publishing, then atomically
replace the target and verify the published content. Ensure the failure path
preserves state invariants and avoids destructive in-place updates.
In `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-v2-removal.test.ts`:
- Around line 13-22: The test is asserting on the generated command’s source
text instead of the observable behavior of the Deep Agents removal flow. Update
the mcp-bridge-adapter-deepagents-v2-removal test to use
runDeepAgentsConfigCommand with a corrupted v2 projection fixture, and verify
the actual exit status and resulting config/state outcome rather than checking
substrings like Invalid managed MCP v2 projection, raise SystemExit(2), or the
force flag serialization. Use buildDeepAgentsMcpRemoveCommand only as the
command generator under test, but validate through the public execution boundary
like the other tests in this file.
In `@test/langchain-deepagents-code-managed-mcp-hardening.test.ts`:
- Around line 17-31: The temp directory cleanup in runManagedHelper is only
protected after the helper file is written, so failures in the setup path can
leak tempDir. Move the cleanup into a broader try/finally that wraps all setup
work in runManagedHelper, including the readFileSync, helperSource
extraction/assertion, and writeFileSync steps, so tempDir is always removed even
if those operations throw.
---
Nitpick comments:
In `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-command.ts`:
- Around line 10-28: runDeepAgentsAdapterCommand is still making a host-boundary
sandbox call directly through executeSandboxCommand, which belongs in an adapter
layer. Move the execution responsibility into an adapter under
src/lib/adapters/** and keep this helper in actions limited to
orchestration/redaction/error handling. Preserve the current behavior by having
runDeepAgentsAdapterCommand call the new adapter entry point instead of
importing executeSandboxCommand directly, and update any related adapter symbols
such as McpBridgeError and redactBridgeSecretsForDisplay usage as needed.
In `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-legacy.ts`:
- Around line 16-26: The legacy compatibility note in
mcp-bridge-adapter-deepagents-legacy needs a concrete retirement anchor and exit
criteria. Update the retirement guidance near the
removalCondition/sourceBoundary comments to include the GitHub issue or PR that
tracks deprecation, or explicitly state this is a bounded rollback window. Make
the criteria observable by tying deletion of the compatibility module to
specific supported-release or registry-owned v1 entry conditions, and keep the
reference close to the existing legacy-path comments so it’s easy to find.
In `@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts`:
- Around line 26-47: The test in
mcp-bridge-adapter-deepagents-projection.test.ts is over-asserting generated
Python source strings instead of behavior, and it duplicates the protection
already covered by the executed race/attack tests. Trim the assertions in the
affected `it("uses the isolated runtime and one stable-read contract for every
v2 mutation")` case to only check what can’t be verified behaviorally, such as
the `buildDeepAgentsMcpStatusCommand`/registration command isolated-interpreter
prefix, and remove the literal substring checks for internals like
`os.O_NOFOLLOW`, `os.fstat(descriptor)`, `assert_managed_source_stable(path,
identity)`, `os.link(...)`, and `os.ftruncate(...)`.
In
`@src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-runtime-guards.test.ts`:
- Around line 87-94: The loop in the deep agents runtime guards test is masking
command-specific failures by accepting either status 2 or a rollback marker for
both commands. Split the assertions for buildDeepAgentsMcpRemoveCommand and
buildDeepAgentsMcpRegisterCommand in
mcp-bridge-adapter-deepagents-runtime-guards.test so each command separately
verifies its expected teardown or rollback behavior, and keep the
legacyConfigText unchanged check for each case without allowing a generic pass
condition.
In `@src/lib/actions/sandbox/rebuild-dcode-mutation-edge.test.ts`:
- Around line 55-78: The ordering assertions in the rebuild-dcode-mutation-edge
test rely on destructuring invocationCallOrder without an explicit guard, so
keep the existing call-count check before using firstRouteOrder,
preBackupRouteOrder, preMcpRouteOrder, and deleteEdgeRouteOrder. In the test
around harness.preflightDcodeRouteSpy and the other spy orders, make sure the
sequence assertions only run after verifying the expected number of calls so
failures report a clear count mismatch rather than undefined comparison noise.
In `@test/langchain-deepagents-code-managed-mcp-hardening.test.ts`:
- Around line 44-49: The test is asserting behavior through private helpers and
globals instead of the managed runtime’s public boundary. Update the affected
cases to drive setup through descriptor/config inputs and verify observable
outcomes via managed_mcp_config_path() and managed_mcp_server_descriptor()
rather than calling _canonicalize_managed_mcp_config(),
_read_managed_mcp_config(), or mutating _MANAGED_MCP_* directly. Use the same
approach in the other referenced block so the test stays aligned with the public
contract of the managed runtime.
🪄 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: 1fba9007-07ce-4bea-8b79-a4ebc9dfcabc
📒 Files selected for processing (47)
agents/langchain-deepagents-code/patch-managed-deepagents-code.pydocs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxsrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-capability.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-command.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-inspection.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-legacy-teardown.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-legacy.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-rollback.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-runtime-guards.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-teardown.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-v2-removal.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents.tssrc/lib/actions/sandbox/mcp-bridge-adapter-status.tssrc/lib/actions/sandbox/mcp-bridge-adapter-teardown.tssrc/lib/actions/sandbox/mcp-bridge-destroy.tssrc/lib/actions/sandbox/mcp-bridge-rebuild.tssrc/lib/actions/sandbox/mcp-bridge-status.tssrc/lib/actions/sandbox/rebuild-dcode-artifact-drift.test.tssrc/lib/actions/sandbox/rebuild-dcode-flow.test.tssrc/lib/actions/sandbox/rebuild-dcode-mutation-edge.test.tssrc/lib/actions/sandbox/rebuild-dcode-orchestrator.test.tssrc/lib/actions/sandbox/rebuild-dcode-orchestrator.tssrc/lib/actions/sandbox/rebuild-dcode-pre-delete-drift.test.tssrc/lib/actions/sandbox/rebuild-dcode-preflight.test.tssrc/lib/actions/sandbox/rebuild-dcode-recovery.test.tssrc/lib/actions/sandbox/rebuild-destroy-phase.test.tssrc/lib/actions/sandbox/rebuild-destroy-phase.tssrc/lib/actions/sandbox/rebuild-durable-config.tssrc/lib/actions/sandbox/rebuild-managed-image-configuration.test.tssrc/lib/actions/sandbox/rebuild-managed-image-preflight.test.tssrc/lib/actions/sandbox/rebuild-managed-image-preparation.test.tssrc/lib/actions/sandbox/rebuild-managed-image-verification.test.tstest/deepagents-mcp-legacy-lifecycle.test.tstest/helpers/mcp-bridge-adapter-deepagents-fixture.tstest/helpers/rebuild-dcode-flow-support.tstest/helpers/rebuild-flow-harness.tstest/helpers/rebuild-managed-image-preflight-harness.tstest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-managed-entrypoints.test.tstest/langchain-deepagents-code-managed-mcp-hardening.test.ts
💤 Files with no reviewable changes (3)
- src/lib/actions/sandbox/mcp-bridge-adapter-deepagents.test.ts
- src/lib/actions/sandbox/rebuild-managed-image-preflight.test.ts
- src/lib/actions/sandbox/rebuild-dcode-flow.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/actions/sandbox/mcp-bridge-status.ts
- docs/reference/commands.mdx
🚧 Files skipped from review as they are similar to previous changes (9)
- test/langchain-deepagents-code-managed-entrypoints.test.ts
- src/lib/actions/sandbox/rebuild-durable-config.ts
- test/deepagents-mcp-legacy-lifecycle.test.ts
- test/langchain-deepagents-code-direct-module-patch.test.ts
- test/langchain-deepagents-code-image.test.ts
- src/lib/actions/sandbox/rebuild-dcode-orchestrator.ts
- src/lib/actions/sandbox/rebuild-destroy-phase.ts
- test/helpers/rebuild-flow-harness.ts
- agents/langchain-deepagents-code/patch-managed-deepagents-code.py
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
E2E Target Results — ❌ Some jobs failedRun: 28693440345
|
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28693937143
|
Contributor
E2E Target Results — ❌ Some jobs failedRun: 28693440291
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28694837746
|
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28694882305
|
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28694837778
|
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28694837793
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28694882243
|
Contributor
E2E Target Results — ❌ Some jobs failedRun: 28694882228
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
|
Follow-up to the Nemotron Ultra review on the previous head:
The follow-up also addresses every CodeRabbit/CodeQL inline item in |
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28696027039
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
agents/langchain-deepagents-code/dcode-wrapper.sh (1)
144-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider finishing the extraction:
has_non_slack_secret_shapeandis_secret_shaped_valuestill duplicate ~10 prefix checks.This PR extracted the bearer/context checks into shared helpers (
has_bearer_secret_shape,has_context_secret_shape), but the remaining prefix-pattern checks (nvapi-, ghp_, AKIA/ASIA, bot-token, JWT, etc.) are still copy-pasted verbatim between these two functions. Given the PR's explicit goal of closing secret-pattern parity gaps, extracting the shared prefix list into one helper (parameterized by whether Slack's ownxox*/xapp-prefixes should be excluded) would remove the remaining drift risk between the two call sites.♻️ Sketch of consolidation
+has_common_secret_shape() { + local value="$1" + if has_private_key_block_shape "$value"; then return 0; fi + if [[ "$value" =~ (sk-proj-|sk-ant-)[A-Za-z0-9_-]{10,} ]]; then return 0; fi + if [[ "$value" =~ sk-[A-Za-z0-9_-]{20,} ]]; then return 0; fi + if [[ "$value" =~ (nvapi-|nvcf-|ghp_|hf_|glpat-|gsk_|pypi-|tvly-)[A-Za-z0-9_-]{10,} ]]; then return 0; fi + if [[ "$value" =~ github_pat_[A-Za-z0-9_]{30,} ]]; then return 0; fi + if [[ "$value" =~ A(K|S)IA[A-Z0-9]{16} ]]; then return 0; fi + if [[ "$value" =~ bot[0-9]{8,10}:[A-Za-z0-9_-]{35} ]]; then return 0; fi + if [[ "$value" =~ [0-9]{8,10}:[A-Za-z0-9_-]{35} ]]; then return 0; fi + if [[ "$value" =~ [A-Za-z0-9]{24}\.[A-Za-z0-9_-]{6}\.[A-Za-z0-9_-]{27,} ]]; then return 0; fi + return 1 +}Also applies to: 235-280
🤖 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 `@agents/langchain-deepagents-code/dcode-wrapper.sh` around lines 144 - 183, The secret-shape checks are still duplicated between has_non_slack_secret_shape and is_secret_shaped_value, so consolidate the remaining prefix/regex list into a shared helper. Extract the common non-Slack secret patterns (for example nvapi-, ghp_, AKIA/ASIA, bot-token, JWT, lsv2_, etc.) into one function and have both call sites reuse it, keeping the Slack-specific xox*/xapp- exclusions only where needed to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@agents/langchain-deepagents-code/dcode-wrapper.sh`:
- Around line 144-183: The secret-shape checks are still duplicated between
has_non_slack_secret_shape and is_secret_shaped_value, so consolidate the
remaining prefix/regex list into a shared helper. Extract the common non-Slack
secret patterns (for example nvapi-, ghp_, AKIA/ASIA, bot-token, JWT, lsv2_,
etc.) into one function and have both call sites reuse it, keeping the
Slack-specific xox*/xapp- exclusions only where needed to avoid drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b8d06258-f982-43ca-9e28-885dd33691ca
📒 Files selected for processing (9)
agents/langchain-deepagents-code/dcode-wrapper.shagents/langchain-deepagents-code/managed-dcode-runtime.pysrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.tssrc/lib/actions/sandbox/mcp-bridge-adapter-deepagents-v2-removal.test.tstest/fixtures/langchain-deepagents-code/app.pytest/helpers/langchain-deepagents-code-secret-patterns.tstest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-managed-mcp-hardening.test.tstest/langchain-deepagents-code-secret-pattern-parity.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/helpers/langchain-deepagents-code-secret-patterns.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts
- test/fixtures/langchain-deepagents-code/app.py
- agents/langchain-deepagents-code/managed-dcode-runtime.py
Collaborator
Author
|
Final automated-review dispositions on exact head
GPT-5.5 recommends |
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28696027101
|
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28696027070
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28696844639
|
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28696844719
|
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28696844701
|
Collaborator
Author
|
Exact-head follow-up for
Nemotron PRA-6/PRA-7 disposition:
No code change is warranted for either request. One unchanged pre-existing DCode E2E awk defect found during artifact inspection is now disclosed in the PR verification notes; it does not affect the runtime-output, sandbox-log, env-file immutability, or raw-secret proofs from these runs. |
This was referenced Jul 4, 2026
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>
20 tasks
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 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
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
tools/callconflict rejection.O_TMPFILEinode is reopened read-only and bound by descriptor, device, inode, size, kind, and SHA-256, with ambient discovery disabled.Verification notes:
CandC.UTF-8Bash locales.9a31537785ef2d456901de622721ed215627fdecpassed: 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.9a31537785ef2d456901de622721ed215627fdecpassed:mcp-bridge,mcp-bridge-dev, andubuntu-repo-cloud-langchain-deepagents-code. 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 treatscloseas reserved; runtime-output, sandbox-log, env-file immutability, and raw-secret checks passed, and this PR does not modify that E2E file.test-clipre-commit invocation remains affected by Vitest 4.1.9 collapsing repeated--coverage.excludearguments 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 docscompleted with 0 errors and 2 pre-existing Fern warnings. Two documentation-writer audits confirmed the final behavior is accurately documented.Type of Change
Quality Gates
O_TMPFILEfallback), cross-language secret-pattern parity, policy fidelity, and the Epic: stabilize onboarding and runtime lifecycle contracts before public extensibility #6224 boundary were checked.Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
endpoint.pathvalidation.