feat(dcode): add managed non-interactive JSON envelope - #7797
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
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 non-interactive ChangesManaged DCode JSON output
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant DcodeCLI
participant run_non_interactive
participant JSONRunner
participant Stdout
Client->>DcodeCLI: invoke dcode -n task --json
DcodeCLI->>run_non_interactive: pass output_format and timeout
run_non_interactive->>JSONRunner: collect response and classify status
JSONRunner->>Stdout: write one bounded JSON envelope
DcodeCLI-->>Client: return envelope and exit code
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-7797.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 7383493 in the TypeScript / code-coverage/cliThe overall coverage in commit 7383493 in the Show a code coverage summary of the most impacted files.
Updated |
Security review: PASSReviewed PR head The managed DCode JSON-envelope change is safe to merge from a security perspective, subject to normal CI. The review found no vulnerability or security warning. The implementation reduces stdout ambiguity, bounds serialized output, suppresses unframed progress, omits partial assistant content on failures, and avoids exposing exception details. FindingsNo findings. Detailed analysis
Files reviewed
|
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/helpers/langchain-deepagents-code-patch-fixture.ts (1)
489-497: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName the
fixture-json-tasksentinel where it is shared.The magic message string couples this fixture to
test/dcode-non-interactive-json.test.tsline 57 with no link between them; a rename on either side silently degrades the CLI-forwarding test into akwargs-returning path rather than failing loudly. Exporting the sentinel from the helper and referencing it in the test would keep the contract explicit.🤖 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/helpers/langchain-deepagents-code-patch-fixture.ts` around lines 489 - 497, Export a shared sentinel constant from the fixture helper and update the `fixture-json-task` check in `_run_non_interactive_impl` to use it. Import and reference that same constant in `test/dcode-non-interactive-json.test.ts` so the CLI-forwarding contract remains explicit and renames cannot silently diverge.test/langchain-deepagents-code-image.test.ts (1)
825-832: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThese per-key substring assertions lock in the validator's source text, not its behavior.
test/langchain-deepagents-code-headless-runtime.test.tsalready proves the classifier accepts only a well-formed envelope, so the eight quoted key literals here add brittleness without coverage: reordering theset(envelope) != {...}literal or switching to single quotes in the embedded Python breaks this test while behavior is unchanged. Asserting that the image ships the--jsoninvocation (Line 766) plus one envelope-shape marker would be enough.As per path instructions, "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."
🤖 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-image.test.ts` around lines 825 - 832, Replace the eight per-key substring assertions in the image validation test with a behavioral assertion at the public boundary: retain the check that the image includes the --json invocation and add one representative envelope-shape marker. Remove assertions tied to the validator’s exact source formatting while preserving coverage that the expected envelope structure is present.Source: Path instructions
test/dcode-non-interactive-json.test.ts (1)
96-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSurface driver stderr when the spawned process fails.
Every test gates on
expect(result.status).toBe(0)while the real diagnosis (Python traceback or a failed in-driverassert) sits inresult.stderr. Asserting on a combined value keeps failures self-explanatory instead of showing only0 !== 1.♻️ Example
- expect(result.status).toBe(0); + expect({ status: result.status, stderr: result.stderr }).toMatchObject({ status: 0 });🤖 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/dcode-non-interactive-json.test.ts` around lines 96 - 97, Update the result.status assertion in the test flow before parseEnvelope(result.stdout) to surface result.stderr when the spawned process fails, while preserving the successful status check and envelope parsing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/dcode-non-interactive-json.test.ts`:
- Around line 96-97: Update the result.status assertion in the test flow before
parseEnvelope(result.stdout) to surface result.stderr when the spawned process
fails, while preserving the successful status check and envelope parsing
behavior.
In `@test/helpers/langchain-deepagents-code-patch-fixture.ts`:
- Around line 489-497: Export a shared sentinel constant from the fixture helper
and update the `fixture-json-task` check in `_run_non_interactive_impl` to use
it. Import and reference that same constant in
`test/dcode-non-interactive-json.test.ts` so the CLI-forwarding contract remains
explicit and renames cannot silently diverge.
In `@test/langchain-deepagents-code-image.test.ts`:
- Around line 825-832: Replace the eight per-key substring assertions in the
image validation test with a behavioral assertion at the public boundary: retain
the check that the image includes the --json invocation and add one
representative envelope-shape marker. Remove assertions tied to the validator’s
exact source formatting while preserving coverage that the expected envelope
structure is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9f933171-f3ca-48ae-babe-9108838cbba5
📒 Files selected for processing (8)
agents/langchain-deepagents-code/patch-managed-deepagents-code.pydocs/get-started/quickstart-langchain-deepagents-code.mdxdocs/reference/commands.mdxtest/dcode-non-interactive-json.test.tstest/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.shtest/helpers/langchain-deepagents-code-patch-fixture.tstest/langchain-deepagents-code-headless-runtime.test.tstest/langchain-deepagents-code-image.test.ts
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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)
test/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.sh (1)
598-598: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDelimit the injected exit marker from JSON output.
If
dcode --jsonemits a valid envelope without a final newline, Line 598 produces}DCODE_EXIT:0; Lines 370-373 cannot strip that marker, so this compliant envelope is rejected. Print a leading newline before the marker and add a no-final-newline regression case.Proposed fix
-headless_output="$(sandbox_login_exec "cd /sandbox && timeout ${HEADLESS_TIMEOUT} dcode -n 'Reply with exactly one word: PONG' --json; echo \"DCODE_EXIT:\$?\"" || true)" +headless_output="$(sandbox_login_exec "cd /sandbox && timeout ${HEADLESS_TIMEOUT} dcode -n 'Reply with exactly one word: PONG' --json; dcode_status=\$?; printf '\nDCODE_EXIT:%s\n' \"\$dcode_status\"" || true)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.sh` at line 598, Update the headless_output capture command to print a leading newline before the DCODE_EXIT marker, ensuring the marker is separated from JSON even when dcode emits no trailing newline. Add a regression case covering a valid JSON envelope without a final newline and verify the existing marker-stripping logic accepts it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@test/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.sh`:
- Around line 414-418: Strengthen envelope validation after JSON parsing so each
required field has the exact declared type and content: reject booleans where
numeric values are expected, require integer values for schema_version,
duration_ms, exit_code, and response_bytes as appropriate, and require the
health response string to equal PONG without surrounding whitespace. Update the
validation logic around the existing envelope checks and preserve rejection of
non-dict envelopes and other malformed values.
---
Outside diff comments:
In
`@test/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.sh`:
- Line 598: Update the headless_output capture command to print a leading
newline before the DCODE_EXIT marker, ensuring the marker is separated from JSON
even when dcode emits no trailing newline. Add a regression case covering a
valid JSON envelope without a final newline and verify the existing
marker-stripping logic accepts it.
🪄 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: eecf8cfa-35bb-459a-9816-52a73a2aa149
📒 Files selected for processing (2)
test/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.shtest/langchain-deepagents-code-headless-runtime.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical pre-tag release entry for NemoClaw v0.0.98. The dated entry records the user-visible changes merged after v0.0.97 and links each release theme to its published documentation. ## Changes - Add `docs/changelog/2026-07-29.mdx` with the exact `## v0.0.98` release heading. - Summarize Hermes 0.19, Deep Agents Code automation and skill safety, readiness diagnostics, lifecycle recovery, uninstall behavior, messaging conflicts, dependency hardening, and bounded diagnostics. - Use the parser-safe MDX SPDX comment and root-absolute routes for published OpenClaw, Hermes, and Deep Agents documentation. ### Source summary - [#7849](#7849) -> `docs/changelog/2026-07-29.mdx`: Record the Hermes 0.19 runtime migration repairs for cron state, dashboard seeding, and MCP naming. - [#7662](#7662) -> `docs/changelog/2026-07-29.mdx`: Record bounded gateway and Docker subprocess diagnostics. - [#7850](#7850) -> `docs/changelog/2026-07-29.mdx`: Record verified no-clobber Deep Agents Code skill installation. - [#7848](#7848) -> `docs/changelog/2026-07-29.mdx`: Record post-reboot delivery-chain recovery for visible OpenClaw sandboxes. - [#7831](#7831) -> `docs/changelog/2026-07-29.mdx`: Record OpenShell gateway-state preservation during uninstall. - [#7827](#7827) -> `docs/changelog/2026-07-29.mdx`: Record the removal of upstream test sources from published Hermes images. - [#7775](#7775) -> `docs/changelog/2026-07-29.mdx`: Record the blocking diagnostic for unsupported `DOCKER_HOST` values. - [#7833](#7833) -> `docs/changelog/2026-07-29.mdx`: Record reviewed Python dependency baselines for Hermes and Deep Agents Code images. - [#7771](#7771) -> `docs/changelog/2026-07-29.mdx`: Record the managed Hermes Agent 0.19.0 upgrade. - [#7811](#7811) -> `docs/changelog/2026-07-29.mdx`: Record fail-closed messaging channel conflict handling. - [#7797](#7797) -> `docs/changelog/2026-07-29.mdx`: Record the managed non-interactive Deep Agents Code JSON envelope. - [#7782](#7782) -> `docs/changelog/2026-07-29.mdx`: Record the storage-remediation readiness capability. - [#7784](#7784) -> `docs/changelog/2026-07-29.mdx`: Record the 120-second OpenShell readiness budget for sandbox recreation. - [#7810](#7810) -> `docs/changelog/2026-07-29.mdx`: Record rejection of stale Deep Agents Code security inventories. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the native changelog contract, including the version heading, MDX SPDX comment, and published routes. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-29.mdx` was reviewed against `docs/CONTRIBUTING.md` and `WRITING.md` for release meaning, terminology, structure, voice, sentence form, MDX structure, published routes, and code-sample presentation. The changelog contract passed 6 tests. The docs build completed with 0 errors and 2 existing Fern warnings. - Agent: Codex CLI <!-- docs-review-head-sha: e3221d1 --> <!-- docs-review-agents-blob-sha: c052d60 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable. `scripts/prepare-dgx-station-host.sh` is unchanged. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` passed 6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not run for this documentation-only change. - [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) — The build completed with 0 errors and 2 existing Fern warnings. - [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) — Native changelog entries use the required parser-safe MDX SPDX comment and do not use frontmatter. --- Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added managed Hermes upgrades with verified releases, version reporting, and preserved configuration contracts. - Improved Deep Agents Code JSON output and skill installation behavior. - Added clearer Docker host and system readiness reporting. - Improved post-reboot delivery recovery and sandbox readiness timing. - **Bug Fixes** - Preserved gateway state when uninstalling with `--keep-openshell`. - Prevented conflicting messaging credentials from blocking onboarding and rebuilds. - Improved gateway diagnostics, dependency security, runtime filesystem protection, and evidence handling. - **Documentation** - Published the v0.0.98 release notes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Make managed
dcode -n <prompt> --jsonemit one bounded, schema-versioned JSON envelope instead of mixing assistant and progress text on stdout. The change preserves text mode, classifies terminal outcomes, and gives host-agnostic automation a fail-closed result boundary.Related Issue
Fixes #7773
Changes
--jsonselection into the pinned Deep Agents Code non-interactive runtime and produce one schema version 1 envelope for success, agent failure, process failure, timeout, turn limit, cancellation, and output-limit outcomes.test/dcode-non-interactive-json.test.tsprotects framing, status, timeout, cancellation, size-limit, and unchanged text-mode behavior.PONGenvelope for login-shell and direct-exec paths, rejecting any prefixed, suffixed, or multiple JSON documents.Type of Change
Quality Gates
6d36977fd5a1eb003336a8d52752ec7f85fe3472; no findings. The constant-memory process-level stdout drain strengthens fail-closed framing without retaining untrusted output and introduces no credential, authorization, dependency, cryptography, or privilege changes. Earlier detailed review: feat(dcode): add managed non-interactive JSON envelope #7797 (comment)Documentation Writer Review
no-docs-neededdocs/get-started/quickstart-langchain-deepagents-code.mdxanddocs/reference/commands.mdxremain accurate; 33 focused tests and 152 broader managed-runtime tests passed, with 1 existing skip.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm exec -- vitest run test/dcode-non-interactive-json.test.ts test/langchain-deepagents-code-headless-runtime.test.ts test/langchain-deepagents-code-image.test.ts), 33 passed; broader managed-runtime suite, 152 passed with 1 existing skip.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: localnpm testdid not complete after two attempts because unrelated macOS/environment-sensitive suites timed out or required unavailable host services; GitHub CI remains authoritativenpm run docsbuilds without warnings (doc changes only)Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit
New Features
--jsonoutput mode for non-interactive Deep Agents Code runs that emits exactly one schema-versioned JSON envelope, suppresses unintended stdout/progress, and enforces a 1 MiB bounded output limit with structured metadata.Documentation
Tests