Skip to content

fix(snapshot): harden dcode exec probe boundary - #6215

Merged
cv merged 7 commits into
mainfrom
codex/issue-6180-hardening
Jul 3, 2026
Merged

fix(snapshot): harden dcode exec probe boundary#6215
cv merged 7 commits into
mainfrom
codex/issue-6180-hardening

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR fixes the dcode snapshot idle-probe regression caused by OpenShell output framing and hardens the exec-output trust boundary against injected, duplicated, or conflicting marker/state output. It builds on and supersedes Tinson Lai's original implementation in #6190, with Tinson retained as a co-author on the hardening commit.

Related Issue

Fixes #6180

Supersedes #6190.

Changes

  • Preserve stdout and stderr separately while accepting OpenShell-framed child output from either stream.
  • Generate a fresh marker per exec and require exactly one marker and one valid dcode probe state across both streams.
  • Use a non-login shell so profile startup output cannot influence the probe.
  • Fail closed on duplicate markers, duplicate/conflicting states, nonzero status, signals, or exec errors.
  • Add focused parser and snapshot tests for framing, cross-stream output, injection, ambiguity, and failure cases.
  • Credit Tinson Lai for the original fix: the fix(snapshot): strip OpenShell exec framing in dcode idleness probe #6190 commits remain intact and the new hardening commit includes Co-authored-by: Tinson Lai <tinsonl@nvidia.com>.

Validation performed locally:

  • 37 focused snapshot/parser tests passed.
  • 23 process-recovery unit tests passed.
  • 23 process-recovery integration/primitives tests passed.
  • CLI type checking, Biome, diff checks, commit lint, and pre-push checks passed.
  • The broad CLI hook was attempted but encountered 105 unrelated local environment/baseline failures (including Node 26 deprecation stderr and missing packaged JSON5 tooling); none were in the four changed files.

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:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: this restores the existing documented snapshot contract and changes only internal probe parsing and validation.
  • 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: local security-focused review covered marker ambiguity, cross-stream parsing, duplicate/conflicting states, login-shell startup output, and fail-closed outcomes; no blocking findings.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

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: Apurv Kumaria 36614+apurvvkumaria@users.noreply.github.com

Summary by CodeRabbit

  • New Features

    • Added randomized execution markers for sandbox command wrapping and improved boundary validation during output detection.
    • Snapshot runtime checks now use marker-aware probing across captured streams to determine idle vs active more accurately.
  • Bug Fixes

    • Reduced false positives by requiring an exact single marker occurrence and rejecting missing, duplicate, or ambiguous matches.
    • Improved handling of framed stdout/stderr output, including correct extraction even when content spans streams.
  • Tests

    • Expanded sandbox execution output, stream parsing, and snapshot probe coverage with additional edge cases.

laitingsheng and others added 4 commits July 2, 2026 09:27
The dcode snapshot-create guard matched the raw `openshell sandbox exec`
output with an anchored sentinel regex, so an OpenShell build that frames
child stdout as `stdout: <line>` never matched and every idle terminal
runtime sandbox was refused with "cannot verify". Route the probe through
the shared marked-command and stdout-frame-stripping helpers, and decide
on the extracted sentinel instead of the exec wrapper's exit status.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Use a per-exec marker and preserve stdout/stderr separately.

Keep framed OpenShell output compatible while rejecting duplicate markers and states.

Avoid login-shell profile output.

Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <36614+apurvvkumaria@users.noreply.github.com>
@apurvvkumaria apurvvkumaria self-assigned this Jul 2, 2026
@github-code-quality

github-code-quality Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/issue-6180-har... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/issue-6180-har... 944d4b3 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/blueprint/ssrf.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 codex/issue-6180-har... branch is 69%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/issue-6180-har... 944d4b3 +/-
src/lib/actions...dbox/rebuild.ts 82%
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 80%
src/lib/shields/index.ts 75%
src/lib/state/sandbox.ts 73%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/onboard/preflight.ts 69%
src/lib/policy/index.ts 67%
src/lib/actions...licy-channel.ts 59%
src/lib/onboard.ts 20%

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

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Blocked

Merge posture: Do not merge until addressed
Primary next action: Fix PRA-1: Test file exceeds 20-line growth threshold for large-file hotspot; then add or justify PRA-T1.
Open items: 4 required · 2 warnings · 3 suggestions · 7 test follow-ups
Since last review: 0 prior items resolved · 6 still apply · 2 new items found

Action checklist

  • PRA-1 Fix: Test file exceeds 20-line growth threshold for large-file hotspot in src/lib/actions/sandbox/snapshot.test.ts:1
  • PRA-2 Fix: Source file exceeds 20-line growth threshold for large-file hotspot in src/lib/actions/sandbox/snapshot.ts:1
  • PRA-3 Fix: Duplicate PR fix(snapshot): strip OpenShell exec framing in dcode idleness probe #6190 with identical changes and linked issue
  • PRA-4 Fix: Silent fail-closed on duplicate marker — no debug logging in src/lib/actions/sandbox/sandbox-exec-output.ts:65
  • PRA-5 Resolve or justify: OpenShell framing compatibility shim lacks tracking issue for removal in src/lib/actions/sandbox/sandbox-exec-output.ts:48
  • PRA-6 Resolve or justify: Integration-style test uses spawnSync in unit test file in src/lib/actions/sandbox/snapshot.test.ts:553
  • 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: Runtime validation
  • PRA-T6 Add or justify test follow-up: Integration-style test uses spawnSync in unit test file
  • PRA-T7 Add or justify test follow-up: Missing edge-case tests for marker validation and base64 encoding
  • PRA-7 In-scope improvement: Compatibility shim comment lacks explicit removal tracking reference in src/lib/actions/sandbox/sandbox-exec-output.ts:48
  • PRA-8 In-scope improvement: Silent fail-closed on duplicate marker could benefit from debug logging in src/lib/actions/sandbox/sandbox-exec-output.ts:65
  • PRA-9 In-scope improvement: Missing edge-case tests for marker validation and base64 encoding in src/lib/actions/sandbox/sandbox-exec-output.test.ts:1

Findings index

ID Severity Category Location Required action
PRA-1 Required architecture src/lib/actions/sandbox/snapshot.test.ts:1 Extract dcode probe test helpers and the 8 new probe scenario tests into a separate module (e.g., snapshot-dcode-probe.test.ts) to bring snapshot.test.ts under ~840 lines.
PRA-2 Required architecture src/lib/actions/sandbox/snapshot.ts:1 Extract the dcode probe logic into a dedicated module like snapshot-dcode-probe.ts. Import and call from snapshot.ts.
PRA-3 Required scope Close PR #6190 as superseded/duplicate with a reference to this PR (#6215). Confirm this PR is the canonical one.
PRA-4 Required security src/lib/actions/sandbox/sandbox-exec-output.ts:65 Add debug-level log when duplicate marker detected, indicating stream (stdout/stderr) and line index. Guard behind a debug flag (e.g., process.env.NEMOCLAW_DEBUG or console.debug) to avoid noise.
PRA-5 Resolve/justify security src/lib/actions/sandbox/sandbox-exec-output.ts:48 Add GitHub issue reference in JSDoc (e.g., 'Tracking: #XXXX') so removal can be prioritized.
PRA-6 Resolve/justify tests src/lib/actions/sandbox/snapshot.test.ts:553 Move this test to a dedicated integration/e2e test file (e.g., test/e2e/snapshot-dcode-probe.integration.test.ts) if the project has such a directory, or annotate with // @vitest-environment node and keep in a separate integration test file.
PRA-7 Improvement correctness src/lib/actions/sandbox/sandbox-exec-output.ts:48 Add a GitHub issue reference or tracking label in the JSDoc comment (e.g., 'Tracking: #XXXX') so removal can be prioritized when OpenShell adds the stable mode.
PRA-8 Improvement correctness src/lib/actions/sandbox/sandbox-exec-output.ts:65 Add a debug-level log when duplicate marker is detected, indicating which stream and line index. Guard behind a debug flag to avoid noise.
PRA-9 Improvement tests src/lib/actions/sandbox/sandbox-exec-output.test.ts:1 Add edge-case tests for assertSandboxExecMarker (empty, whitespace, invalid chars) and buildSandboxExecMarkedCommand base64 path with shell metacharacters.

🚨 Required before merge

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

PRA-1 Required — Test file exceeds 20-line growth threshold for large-file hotspot

  • Location: src/lib/actions/sandbox/snapshot.test.ts:1
  • Category: architecture
  • Problem: snapshot.test.ts grew by 162 lines (818→980) due to 8 new dcode probe scenario tests and helper functions (mockDcodeProbe*, framedDcodeProbeOutput, captureOpenshellStreams, capturedDcodeProbeScript, runProbeScriptWithProcesses). Exceeds the project's 20-line large-file ratchet.
  • Impact: Monolith growth reduces maintainability, increases cognitive load, and violates the project's large-file ratchet. Future changes to this file will compound the problem.
  • Required action: Extract dcode probe test helpers and the 8 new probe scenario tests into a separate module (e.g., snapshot-dcode-probe.test.ts) to bring snapshot.test.ts under ~840 lines.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/actions/sandbox/snapshot.test.ts` — should be under ~840 after extraction. Verify extracted tests still pass with `pnpm test src/lib/actions/sandbox/snapshot-dcode-probe.test.ts`.
  • Missing regression test: No regression test needed; this is a structural code health finding. Extracted tests preserve existing coverage.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/actions/sandbox/snapshot.test.ts` — should be under ~840 after extraction. Verify extracted tests still pass with `pnpm test src/lib/actions/sandbox/snapshot-dcode-probe.test.ts`.
  • Evidence: Diff shows +162 lines in snapshot.test.ts; driftEvidence monolithDeltas confirms severity=blocker with 20-line threshold rationale.

PRA-2 Required — Source file exceeds 20-line growth threshold for large-file hotspot

  • Location: src/lib/actions/sandbox/snapshot.ts:1
  • Category: architecture
  • Problem: snapshot.ts grew by 21 lines (859→880) due to new imports and updated probe logic using sandbox-exec-output module. The dcode probe logic (DCODE_BUSY_PROBE_SCRIPT, parseDcodeProbeState, shouldCheckDcodeActivity, isSnapshotCreationAllowedByDcodeActivity) is a cohesive security boundary that should be isolated.
  • Impact: Monolith growth reduces maintainability and violates the project's large-file ratchet. The dcode probe logic is a cohesive security boundary that should be isolated for testability and clarity.
  • Required action: Extract the dcode probe logic into a dedicated module like snapshot-dcode-probe.ts. Import and call from snapshot.ts.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/actions/sandbox/snapshot.ts` — should be under ~860 after extraction. Verify snapshot create/restore still works for dcode and non-dcode sandboxes.
  • Missing regression test: No regression test needed; this is a structural code health finding. Existing tests cover the extracted logic.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/actions/sandbox/snapshot.ts` — should be under ~860 after extraction. Verify snapshot create/restore still works for dcode and non-dcode sandboxes.
  • Evidence: Diff shows +21 lines in snapshot.ts; driftEvidence monolithDeltas confirms severity=blocker with 20-line threshold rationale.

PRA-3 Required — Duplicate PR #6190 with identical changes and linked issue

PRA-4 Required — Silent fail-closed on duplicate marker — no debug logging

  • Location: src/lib/actions/sandbox/sandbox-exec-output.ts:65
  • Category: security
  • Problem: extractSandboxExecCommandStdoutFromStreams returns null with no logging when duplicate marker detected across stdout/stderr. Production error 'Cannot verify whether sandbox is actively running a dcode task' doesn't indicate root cause was duplicate marker vs missing marker vs framed mismatch.
  • Impact: Operators cannot distinguish attack/injection (duplicate marker) from benign framing mismatch or missing marker, delaying incident response.
  • Required action: Add debug-level log when duplicate marker detected, indicating stream (stdout/stderr) and line index. Guard behind a debug flag (e.g., process.env.NEMOCLAW_DEBUG or console.debug) to avoid noise.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for console.debug or structured logging in extractSandboxExecCommandStdoutFromStreams around the duplicate marker check (lines 65-70).
  • Missing regression test: No automated test needed; manual verification via debug log output when duplicate marker is injected.
  • Done when: The required change is committed and verification passes: Search for console.debug or structured logging in extractSandboxExecCommandStdoutFromStreams around the duplicate marker check (lines 65-70).
  • Evidence: Security Category 5 (Error Handling) — WARNING. Code at line 65: `if (markerLocation !== null) return null;` with no logging.
Review findings by urgency: 4 required fixes, 2 items to resolve/justify, 3 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-5 Resolve/justify — OpenShell framing compatibility shim lacks tracking issue for removal

  • Location: src/lib/actions/sandbox/sandbox-exec-output.ts:48
  • Category: security
  • Problem: Compatibility shim for OpenShell's stdout: / [stdout] framed output has no linked issue tracking when OpenShell will provide stable machine-readable exec output mode.
  • Impact: Technical debt may persist indefinitely; shim handles external framing variants that could change without notice.
  • Recommended action: Add GitHub issue reference in JSDoc (e.g., 'Tracking: #XXXX') so removal can be prioritized.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read JSDoc comment at line 48 in sandbox-exec-output.ts. Verify tracking issue reference added.
  • Missing regression test: N/A — documentation improvement
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read JSDoc comment at line 48 in sandbox-exec-output.ts. Verify tracking issue reference added.
  • Evidence: Security Category 9 (Holistic) — WARNING. JSDoc at line 48: 'Remove this compatibility shim once OpenShell exposes a stable machine-readable exec output mode' with no tracking reference.

PRA-6 Resolve/justify — Integration-style test uses spawnSync in unit test file

  • Location: src/lib/actions/sandbox/snapshot.test.ts:553
  • Category: tests
  • Problem: The test 'detects managed dcode process argv without matching the probe shell' uses spawnSync to execute the probe script with a mocked ps binary in a unit test file. This crosses the unit/integration boundary and makes the test slower and more fragile.
  • Impact: Unit test file becomes slow and flaky; integration concerns mixed with unit tests reduce test suite reliability.
  • Recommended action: Move this test to a dedicated integration/e2e test file (e.g., test/e2e/snapshot-dcode-probe.integration.test.ts) if the project has such a directory, or annotate with // @vitest-environment node and keep in a separate integration test file.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check if test/e2e/ or similar directory exists. Verify the spawnSync-based test is relocated.
  • Missing regression test: Relocated test preserves coverage; no new test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check if test/e2e/ or similar directory exists. Verify the spawnSync-based test is relocated.
  • Evidence: simplificationSignals in validationContext flags line 553 as integration-style test in unit test file.

💡 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-7 Improvement — Compatibility shim comment lacks explicit removal tracking reference

  • Location: src/lib/actions/sandbox/sandbox-exec-output.ts:48
  • Category: correctness
  • Problem: Compatibility shim comment lacks explicit removal tracking reference.
  • Impact: Technical debt may persist without visibility; removal cannot be prioritized.
  • Suggested action: Add a GitHub issue reference or tracking label in the JSDoc comment (e.g., 'Tracking: #XXXX') so removal can be prioritized when OpenShell adds the stable mode.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read JSDoc comment at line 48 in sandbox-exec-output.ts. Verify tracking issue reference added.
  • Missing regression test: N/A — documentation improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Previous review PRA-5; JSDoc at line 48 has no tracking reference.

PRA-8 Improvement — Silent fail-closed on duplicate marker could benefit from debug logging

  • Location: src/lib/actions/sandbox/sandbox-exec-output.ts:65
  • Category: correctness
  • Problem: Silent fail-closed on duplicate marker could benefit from debug logging for operational visibility.
  • Impact: Operators cannot distinguish attack/injection from benign framing mismatch without debug logs.
  • Suggested action: Add a debug-level log when duplicate marker is detected, indicating which stream and line index. Guard behind a debug flag to avoid noise.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Search for console.debug or structured logging in extractSandboxExecCommandStdoutFromStreams around duplicate marker check.
  • Missing regression test: No automated test needed; manual verification via debug log output when duplicate marker is injected.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Previous review PRA-6; code at line 65 returns null silently.

PRA-9 Improvement — Missing edge-case tests for marker validation and base64 encoding

  • Location: src/lib/actions/sandbox/sandbox-exec-output.test.ts:1
  • Category: tests
  • Problem: New test file sandbox-exec-output.test.ts exists but could add tests for: marker validation edge cases (empty string, whitespace-only), base64 encoding path with special chars, and marker collision probability (though astronomically low with 16 bytes).
  • Impact: Edge cases in marker validation and base64 encoding could allow malformed input to bypass checks if not covered.
  • Suggested action: Add edge-case tests for assertSandboxExecMarker (empty, whitespace, invalid chars) and buildSandboxExecMarkedCommand base64 path with shell metacharacters.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run test suite and verify new edge-case tests pass.
  • Missing regression test: Add tests for assertSandboxExecMarker invalid inputs and base64 encoding round-trip with special characters.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: New test file has 13 test cases but no coverage for assertSandboxExecMarker invalid inputs or base64 round-trip with metacharacters.
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 — Runtime: snapshot create succeeds on real idle dcode sandbox with OpenShell framing. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/actions/sandbox/sandbox-exec-output.ts, src/lib/actions/sandbox/snapshot.ts.
  • PRA-T2 Runtime validation — Runtime: snapshot create rejects active dcode task on real sandbox. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/actions/sandbox/sandbox-exec-output.ts, src/lib/actions/sandbox/snapshot.ts.
  • PRA-T3 Runtime validation — Integration: probe script process detection with edge cases (truncated args, zombie processes, nested shells). Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/actions/sandbox/sandbox-exec-output.ts, src/lib/actions/sandbox/snapshot.ts.
  • PRA-T4 Runtime validation — Unit: assertSandboxExecMarker rejects empty string, whitespace-only, Unicode lookalikes. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/actions/sandbox/sandbox-exec-output.ts, src/lib/actions/sandbox/snapshot.ts.
  • PRA-T5 Runtime validation — Unit: base64 round-trip preserves shell metacharacters: $`"';&|<>(). Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/actions/sandbox/sandbox-exec-output.ts, src/lib/actions/sandbox/snapshot.ts.
  • PRA-T6 Integration-style test uses spawnSync in unit test file — Move this test to a dedicated integration/e2e test file (e.g., test/e2e/snapshot-dcode-probe.integration.test.ts) if the project has such a directory, or annotate with // @vitest-environment node and keep in a separate integration test file.
  • PRA-T7 Missing edge-case tests for marker validation and base64 encoding — Add edge-case tests for assertSandboxExecMarker (empty, whitespace, invalid chars) and buildSandboxExecMarkedCommand base64 path with shell metacharacters.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Required — Test file exceeds 20-line growth threshold for large-file hotspot

  • Location: src/lib/actions/sandbox/snapshot.test.ts:1
  • Category: architecture
  • Problem: snapshot.test.ts grew by 162 lines (818→980) due to 8 new dcode probe scenario tests and helper functions (mockDcodeProbe*, framedDcodeProbeOutput, captureOpenshellStreams, capturedDcodeProbeScript, runProbeScriptWithProcesses). Exceeds the project's 20-line large-file ratchet.
  • Impact: Monolith growth reduces maintainability, increases cognitive load, and violates the project's large-file ratchet. Future changes to this file will compound the problem.
  • Required action: Extract dcode probe test helpers and the 8 new probe scenario tests into a separate module (e.g., snapshot-dcode-probe.test.ts) to bring snapshot.test.ts under ~840 lines.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/actions/sandbox/snapshot.test.ts` — should be under ~840 after extraction. Verify extracted tests still pass with `pnpm test src/lib/actions/sandbox/snapshot-dcode-probe.test.ts`.
  • Missing regression test: No regression test needed; this is a structural code health finding. Extracted tests preserve existing coverage.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/actions/sandbox/snapshot.test.ts` — should be under ~840 after extraction. Verify extracted tests still pass with `pnpm test src/lib/actions/sandbox/snapshot-dcode-probe.test.ts`.
  • Evidence: Diff shows +162 lines in snapshot.test.ts; driftEvidence monolithDeltas confirms severity=blocker with 20-line threshold rationale.

PRA-2 Required — Source file exceeds 20-line growth threshold for large-file hotspot

  • Location: src/lib/actions/sandbox/snapshot.ts:1
  • Category: architecture
  • Problem: snapshot.ts grew by 21 lines (859→880) due to new imports and updated probe logic using sandbox-exec-output module. The dcode probe logic (DCODE_BUSY_PROBE_SCRIPT, parseDcodeProbeState, shouldCheckDcodeActivity, isSnapshotCreationAllowedByDcodeActivity) is a cohesive security boundary that should be isolated.
  • Impact: Monolith growth reduces maintainability and violates the project's large-file ratchet. The dcode probe logic is a cohesive security boundary that should be isolated for testability and clarity.
  • Required action: Extract the dcode probe logic into a dedicated module like snapshot-dcode-probe.ts. Import and call from snapshot.ts.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run `wc -l src/lib/actions/sandbox/snapshot.ts` — should be under ~860 after extraction. Verify snapshot create/restore still works for dcode and non-dcode sandboxes.
  • Missing regression test: No regression test needed; this is a structural code health finding. Existing tests cover the extracted logic.
  • Done when: The required change is committed and verification passes: Run `wc -l src/lib/actions/sandbox/snapshot.ts` — should be under ~860 after extraction. Verify snapshot create/restore still works for dcode and non-dcode sandboxes.
  • Evidence: Diff shows +21 lines in snapshot.ts; driftEvidence monolithDeltas confirms severity=blocker with 20-line threshold rationale.

PRA-3 Required — Duplicate PR #6190 with identical changes and linked issue

PRA-4 Required — Silent fail-closed on duplicate marker — no debug logging

  • Location: src/lib/actions/sandbox/sandbox-exec-output.ts:65
  • Category: security
  • Problem: extractSandboxExecCommandStdoutFromStreams returns null with no logging when duplicate marker detected across stdout/stderr. Production error 'Cannot verify whether sandbox is actively running a dcode task' doesn't indicate root cause was duplicate marker vs missing marker vs framed mismatch.
  • Impact: Operators cannot distinguish attack/injection (duplicate marker) from benign framing mismatch or missing marker, delaying incident response.
  • Required action: Add debug-level log when duplicate marker detected, indicating stream (stdout/stderr) and line index. Guard behind a debug flag (e.g., process.env.NEMOCLAW_DEBUG or console.debug) to avoid noise.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for console.debug or structured logging in extractSandboxExecCommandStdoutFromStreams around the duplicate marker check (lines 65-70).
  • Missing regression test: No automated test needed; manual verification via debug log output when duplicate marker is injected.
  • Done when: The required change is committed and verification passes: Search for console.debug or structured logging in extractSandboxExecCommandStdoutFromStreams around the duplicate marker check (lines 65-70).
  • Evidence: Security Category 5 (Error Handling) — WARNING. Code at line 65: `if (markerLocation !== null) return null;` with no logging.

PRA-5 Resolve/justify — OpenShell framing compatibility shim lacks tracking issue for removal

  • Location: src/lib/actions/sandbox/sandbox-exec-output.ts:48
  • Category: security
  • Problem: Compatibility shim for OpenShell's stdout: / [stdout] framed output has no linked issue tracking when OpenShell will provide stable machine-readable exec output mode.
  • Impact: Technical debt may persist indefinitely; shim handles external framing variants that could change without notice.
  • Recommended action: Add GitHub issue reference in JSDoc (e.g., 'Tracking: #XXXX') so removal can be prioritized.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read JSDoc comment at line 48 in sandbox-exec-output.ts. Verify tracking issue reference added.
  • Missing regression test: N/A — documentation improvement
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read JSDoc comment at line 48 in sandbox-exec-output.ts. Verify tracking issue reference added.
  • Evidence: Security Category 9 (Holistic) — WARNING. JSDoc at line 48: 'Remove this compatibility shim once OpenShell exposes a stable machine-readable exec output mode' with no tracking reference.

PRA-6 Resolve/justify — Integration-style test uses spawnSync in unit test file

  • Location: src/lib/actions/sandbox/snapshot.test.ts:553
  • Category: tests
  • Problem: The test 'detects managed dcode process argv without matching the probe shell' uses spawnSync to execute the probe script with a mocked ps binary in a unit test file. This crosses the unit/integration boundary and makes the test slower and more fragile.
  • Impact: Unit test file becomes slow and flaky; integration concerns mixed with unit tests reduce test suite reliability.
  • Recommended action: Move this test to a dedicated integration/e2e test file (e.g., test/e2e/snapshot-dcode-probe.integration.test.ts) if the project has such a directory, or annotate with // @vitest-environment node and keep in a separate integration test file.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check if test/e2e/ or similar directory exists. Verify the spawnSync-based test is relocated.
  • Missing regression test: Relocated test preserves coverage; no new test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check if test/e2e/ or similar directory exists. Verify the spawnSync-based test is relocated.
  • Evidence: simplificationSignals in validationContext flags line 553 as integration-style test in unit test file.

PRA-7 Improvement — Compatibility shim comment lacks explicit removal tracking reference

  • Location: src/lib/actions/sandbox/sandbox-exec-output.ts:48
  • Category: correctness
  • Problem: Compatibility shim comment lacks explicit removal tracking reference.
  • Impact: Technical debt may persist without visibility; removal cannot be prioritized.
  • Suggested action: Add a GitHub issue reference or tracking label in the JSDoc comment (e.g., 'Tracking: #XXXX') so removal can be prioritized when OpenShell adds the stable mode.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read JSDoc comment at line 48 in sandbox-exec-output.ts. Verify tracking issue reference added.
  • Missing regression test: N/A — documentation improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Previous review PRA-5; JSDoc at line 48 has no tracking reference.

PRA-8 Improvement — Silent fail-closed on duplicate marker could benefit from debug logging

  • Location: src/lib/actions/sandbox/sandbox-exec-output.ts:65
  • Category: correctness
  • Problem: Silent fail-closed on duplicate marker could benefit from debug logging for operational visibility.
  • Impact: Operators cannot distinguish attack/injection from benign framing mismatch without debug logs.
  • Suggested action: Add a debug-level log when duplicate marker is detected, indicating which stream and line index. Guard behind a debug flag to avoid noise.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Search for console.debug or structured logging in extractSandboxExecCommandStdoutFromStreams around duplicate marker check.
  • Missing regression test: No automated test needed; manual verification via debug log output when duplicate marker is injected.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Previous review PRA-6; code at line 65 returns null silently.

PRA-9 Improvement — Missing edge-case tests for marker validation and base64 encoding

  • Location: src/lib/actions/sandbox/sandbox-exec-output.test.ts:1
  • Category: tests
  • Problem: New test file sandbox-exec-output.test.ts exists but could add tests for: marker validation edge cases (empty string, whitespace-only), base64 encoding path with special chars, and marker collision probability (though astronomically low with 16 bytes).
  • Impact: Edge cases in marker validation and base64 encoding could allow malformed input to bypass checks if not covered.
  • Suggested action: Add edge-case tests for assertSandboxExecMarker (empty, whitespace, invalid chars) and buildSandboxExecMarkedCommand base64 path with shell metacharacters.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run test suite and verify new edge-case tests pass.
  • Missing regression test: Add tests for assertSandboxExecMarker invalid inputs and base64 encoding round-trip with special characters.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: New test file has 13 test cases but no coverage for assertSandboxExecMarker invalid inputs or base64 round-trip with metacharacters.

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

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: snapshot-commands, sandbox-operations, hermes-sandbox-secret-boundary
Optional E2E: state-backup-restore

Dispatch hint: snapshot-commands,sandbox-operations,hermes-sandbox-secret-boundary

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • snapshot-commands (medium): Directly covers the modified snapshot CLI lifecycle with a real install, live OpenShell sandbox exec, snapshot create/list/restore, backup artifact checks, and credential-leak assertions.
  • sandbox-operations (medium): Covers adjacent sandbox lifecycle and process/gateway recovery paths that use the shared sandbox exec output marker/parser boundary changed by this PR.
  • hermes-sandbox-secret-boundary (medium): Validates the real Hermes sandbox secret-boundary path that depends on the special sandbox exec marked-command handling touched in this PR.

Optional E2E

  • state-backup-restore (medium): Provides additional confidence for backup/restore state handling adjacent to snapshot creation, but the direct snapshot command path is the merge-blocking coverage.

New E2E recommendations

  • dcode snapshot quiescence guard (high): Existing live snapshot-commands coverage validates snapshot create/list/restore but does not appear to start a real dcode/deepagents-code workload and prove snapshot create fails closed while the task is active, including framed OpenShell stdout/stderr behavior.
    • Suggested test: Add a live E2E target that provisions a dcode-capable sandbox, starts a long-running dcode/deepagents-code task, verifies nemoclaw <sandbox> snapshot create refuses while active, then verifies snapshot create succeeds after the task becomes idle.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: snapshot-commands,sandbox-operations,hermes-sandbox-secret-boundary

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: snapshot-commands
Optional E2E targets: ubuntu-repo-cloud-langchain-deepagents-code

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=snapshot-commands

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • snapshot-commands: Changes modify the snapshot command path and its OpenShell sandbox exec stdout/stderr boundary parsing. The snapshot-commands job drives install.sh plus real snapshot create/list/restore against Docker/OpenShell.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=snapshot-commands

Optional E2E targets

  • ubuntu-repo-cloud-langchain-deepagents-code: Adjacent coverage for the Deep Agents Code sandbox environment touched by the dcode activity guard changes, though the primary snapshot CLI lifecycle is covered by snapshot-commands.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Relevant changed files

  • src/lib/actions/sandbox/sandbox-exec-output.ts
  • src/lib/actions/sandbox/snapshot.ts

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 6 test follow-ups
Since last review: 1 prior item resolved · 0 still apply · 0 new items found

Action checklist

  • 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: Acceptance clause
  • PRA-T5 Add or justify test follow-up: Acceptance clause
  • PRA-T6 Add or justify test follow-up: Acceptance clause
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 — Runtime validation: an idle LangChain Deep Agents Code sandbox on an OpenShell version that frames `sandbox exec` child output creates a named snapshot instead of reporting cannot-verify.. Unit coverage is strong and behavior-specific, but the fixed bug depends on real OpenShell `sandbox exec` stream framing and the LangChain Deep Agents Code runtime/process environment.
  • PRA-T2 Runtime validation — Runtime validation: an active LangChain Deep Agents Code process still causes `snapshot create` to refuse before invoking the backup path.. Unit coverage is strong and behavior-specific, but the fixed bug depends on real OpenShell `sandbox exec` stream framing and the LangChain Deep Agents Code runtime/process environment.
  • PRA-T3 Runtime validation — Runtime validation: a real framed `openshell sandbox exec` dcode probe returns marker and `NEMOCLAW_DCODE_PROBE=idle` in the stream shape accepted by `extractSandboxExecCommandStdoutFromStreams`.. Unit coverage is strong and behavior-specific, but the fixed bug depends on real OpenShell `sandbox exec` stream framing and the LangChain Deep Agents Code runtime/process environment.
  • PRA-T4 Acceptance clause — This blocks P0 DevTest 6159206 ([DeepAgents] snapshot create preserves dcode app state and user skills across rebuild). — add test evidence or identify existing coverage. The diff unblocks the snapshot-create preflight path and issue text says skill persistence itself works via `state_dirs`; there is no full rebuild/user-skill runtime validation in this PR.
  • PRA-T5 Acceptance clause — 2. Connect and create a custom skill, then exit: ```bash nemoclaw dcode-snap connect # Inside sandbox: dcode skills create report-writer dcode skills list # confirms report-writer is present exit ``` — add test evidence or identify existing coverage. This PR does not change connect or skill persistence. It targets the snapshot preflight guard that blocked snapshot creation after the user exited.
  • PRA-T6 Acceptance clause — - This blocks P0 DevTest 6159206 ([DeepAgents] snapshot create preserves dcode app state and user skills across rebuild). — add test evidence or identify existing coverage. The changed code addresses the snapshot-create preflight blocker. Full DevTest rebuild/skill persistence validation is a runtime follow-up rather than directly proven by these unit tests.

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.

Normalize captured mock streams uniformly.

Keep the test helper linear without changing response behavior.

Signed-off-by: Apurv Kumaria <36614+apurvvkumaria@users.noreply.github.com>
Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f272f35a-a601-443d-9235-17d276ef29a5

📥 Commits

Reviewing files that changed from the base of the PR and between df5df5b and 944d4b3.

📒 Files selected for processing (2)
  • src/lib/actions/sandbox/sandbox-exec-output.test.ts
  • src/lib/actions/sandbox/sandbox-exec-output.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/actions/sandbox/sandbox-exec-output.test.ts
  • src/lib/actions/sandbox/sandbox-exec-output.ts

📝 Walkthrough

Walkthrough

This PR adds randomized sandbox exec markers, extracts sandbox exec output from combined stdout/stderr streams with fail-closed marker validation, and updates snapshot dcode probing to use the new marker-based flow.

Changes

Marker-based sandbox exec output hardening

Layer / File(s) Summary
Marker generation and command building
src/lib/actions/sandbox/sandbox-exec-output.ts, src/lib/actions/sandbox/sandbox-exec-output.test.ts
Adds createSandboxExecMarker() backed by randomBytes, lets marked commands accept an optional marker, and tests marker formatting plus command construction.
Dual-stream stdout extraction
src/lib/actions/sandbox/sandbox-exec-output.ts, src/lib/actions/sandbox/sandbox-exec-output.test.ts
Adds stream-aware stdout extraction across stdout and stderr, keeps the wrapper overload, and tests raw/framed output, duplicate markers, and split-marker cases.
Dcode probe wiring
src/lib/actions/sandbox/snapshot.ts, src/lib/actions/sandbox/snapshot.test.ts
Uses marker-based exec commands for the dcode probe, extracts probe stdout from streams, requires a single parsed probe state, and updates snapshot tests for framed output and shell invocation behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested labels: bug-fix, security

Suggested reviewers: ericksoa, cjagwani

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: hardening the snapshot dcode exec probe boundary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 codex/issue-6180-hardening

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

cv added 2 commits July 2, 2026 23:52
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ✅ All requested jobs passed

Run: 28644636962
Workflow ref: codex/issue-6180-hardening
Requested targets: (default — all supported)
Requested jobs: snapshot-commands,sandbox-operations,hermes-sandbox-secret-boundary
Summary: 3 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-sandbox-secret-boundary ✅ success
sandbox-operations ✅ success
snapshot-commands ✅ success

@cv

cv commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Maintainer gate note for exact head 944d4b3:

  • Deterministic merge gate: allPass=true; all 45 checks green, merge state CLEAN, DCO present, all 7 commits Verified, CodeRabbit clear, and canonical PR Advisor merge_as_is.
  • Required live E2E run https://github.com/NVIDIA/NemoClaw/actions/runs/28644636962 passed snapshot-commands, sandbox-operations, and hermes-sandbox-secret-boundary on this exact head. Jetson was neither recommended nor dispatched.
  • Nemotron structural suggestions are overridden: the claimed 20-line ratchet is not repository policy; the enforced 1,500-line test budget and exact-head codebase growth guard both pass. Extracting modules would broaden this narrow security fix.
  • Duplicate-marker parsing intentionally fails closed and the caller emits the cannot-verify error. This codebase has no generic NEMOCLAW_DEBUG convention, so adding ad hoc console output to the shared pure parser is not justified here. Tracking-comment and test-relocation suggestions are likewise nonblocking architecture work.
  • Documentation review found no user-facing behavior change requiring docs edits.

PR #6215 is the canonical fix for #6180. PR #6190 will be closed as superseded after this merge succeeds.

@cv
cv merged commit c7a4728 into main Jul 3, 2026
122 checks passed
@cv
cv deleted the codex/issue-6180-hardening branch July 3, 2026 07:22
@cv cv added the v0.0.74 label Jul 3, 2026
@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
<!-- markdownlint-disable MD041 -->
## Summary
This PR fixes the dcode snapshot idle-probe regression caused by
OpenShell output framing and hardens the exec-output trust boundary
against injected, duplicated, or conflicting marker/state output. It
builds on and supersedes Tinson Lai's original implementation in NVIDIA#6190,
with Tinson retained as a co-author on the hardening commit.

## Related Issue
Fixes NVIDIA#6180

Supersedes NVIDIA#6190.

## Changes
- Preserve stdout and stderr separately while accepting OpenShell-framed
child output from either stream.
- Generate a fresh marker per exec and require exactly one marker and
one valid dcode probe state across both streams.
- Use a non-login shell so profile startup output cannot influence the
probe.
- Fail closed on duplicate markers, duplicate/conflicting states,
nonzero status, signals, or exec errors.
- Add focused parser and snapshot tests for framing, cross-stream
output, injection, ambiguity, and failure cases.
- Credit Tinson Lai for the original fix: the NVIDIA#6190 commits remain
intact and the new hardening commit includes `Co-authored-by: Tinson Lai
<tinsonl@nvidia.com>`.

Validation performed locally:
- 37 focused snapshot/parser tests passed.
- 23 process-recovery unit tests passed.
- 23 process-recovery integration/primitives tests passed.
- CLI type checking, Biome, diff checks, commit lint, and pre-push
checks passed.
- The broad CLI hook was attempted but encountered 105 unrelated local
environment/baseline failures (including Node 26 deprecation stderr and
missing packaged JSON5 tooling); none were in the four changed files.

## Type of Change

- [x] 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
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: this restores the existing
documented snapshot contract and changes only internal probe parsing and
validation.
- [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: local security-focused
review covered marker ambiguity, cross-stream parsing,
duplicate/conflicting states, login-shell startup output, and
fail-closed outcomes; no blocking findings.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [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)
- [ ] 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: Apurv Kumaria
<36614+apurvvkumaria@users.noreply.github.com>


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

* **New Features**
* Added randomized execution markers for sandbox command wrapping and
improved boundary validation during output detection.
* Snapshot runtime checks now use marker-aware probing across captured
streams to determine idle vs active more accurately.

* **Bug Fixes**
* Reduced false positives by requiring an exact single marker occurrence
and rejecting missing, duplicate, or ambiguous matches.
* Improved handling of framed stdout/stderr output, including correct
extraction even when content spans streams.

* **Tests**
* Expanded sandbox execution output, stream parsing, and snapshot probe
coverage with additional edge cases.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <36614+apurvvkumaria@users.noreply.github.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Co-authored-by: Apurv Kumaria <36614+apurvvkumaria@users.noreply.github.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>
@wscurran wscurran added area: local-models Local model providers, downloads, launch, or connectivity bug-fix PR fixes a bug or regression integration: dcode LangChain Deep Code integration behavior labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: local-models Local model providers, downloads, launch, or connectivity bug-fix PR fixes a bug or regression integration: dcode LangChain Deep Code integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ubuntu 24.04][Sandbox] nemoclaw snapshot create refuses for idle LangChain Deep Agents Code sandbox — Cannot verify dcode task state

4 participants