fix(agent): surface json failure provenance - #5683
Merged
Merged
Conversation
Signed-off-by: Aaron Erickson <aerickson@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:
📝 WalkthroughWalkthroughAdds JSON passthrough provenance extraction, routes ChangesOpenClaw Agent JSON Passthrough with Provenance Tracking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
ericksoa
changed the title
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: None Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings 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: 2
🤖 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/openclaw/agent-json-provenance.ts`:
- Around line 176-179: The loop iterating over raw.matchAll(/\{/gu) and calling
findJsonObjectEnd() for each match creates quadratic time complexity because
every brace position triggers a rescan of the string. Replace this nested
scanning approach with a single-pass linear scanner that traverses the raw
string once, maintaining brace-nesting state and only extracting JSON objects
when a complete match is found, eliminating redundant rescans of previously
processed content.
In `@test/openclaw-agent-json.test.ts`:
- Around line 99-103: The assertions at lines 99-103 (and similarly at 126-129
and 148-150) only verify that strings are present in stdout using toContain, but
do not verify their ordering. To strengthen the regression guarantee, modify
these assertions to verify that provenance messages appear before assistant
text. Replace the individual toContain assertions with assertions that check the
ordering of these strings, such as comparing the indexOf values of the
provenance and assistant text to ensure provenance appears first in the output.
🪄 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: e112a7c2-de06-44c6-bef5-85fc95f52461
📒 Files selected for processing (9)
src/lib/actions/sandbox/agent/passthrough-json.test.tssrc/lib/actions/sandbox/agent/passthrough-json.tssrc/lib/actions/sandbox/agent/passthrough.test.tssrc/lib/actions/sandbox/agent/passthrough.tssrc/lib/openclaw/agent-json-provenance.test.tssrc/lib/openclaw/agent-json-provenance.tstest/e2e/lib/openclaw-agent-json.pytest/e2e/lib/openclaw-json.shtest/openclaw-agent-json.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
wscurran
added
area: cli
Contributor
|
✨ |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-5683.docs.buildwithfern.com/nemoclaw |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/reference/commands.mdx`:
- Line 561: The prose in the referenced documentation line needs to be split so
each sentence appears on its own source line, and the literal buffer size must
be inline code. Update the sentence block in the MDX content near the OpenClaw
command behavior description so the `openclaw agent`/`--json` explanation is one
sentence per line, and format the `64 MiB` value with inline code while
preserving the existing meaning.
🪄 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: 64cf35b0-5bb8-48cc-b742-a03b8552006f
📒 Files selected for processing (6)
docs/reference/commands.mdxsrc/commands/sandbox/agent.tssrc/lib/actions/sandbox/agent/passthrough-json.test.tssrc/lib/actions/sandbox/agent/passthrough-json.tssrc/lib/actions/sandbox/agent/passthrough.test.tssrc/lib/actions/sandbox/agent/passthrough.ts
✅ Files skipped from review due to trivial changes (1)
- src/commands/sandbox/agent.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/actions/sandbox/agent/passthrough-json.ts
- src/lib/actions/sandbox/agent/passthrough.ts
- src/lib/actions/sandbox/agent/passthrough.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/openclaw-agent-json.test.ts (1)
20-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
bash -cinstead ofbash -lcfor hermeticity.The
-l(login) flag makes bash source profile files (/etc/profile,~/.bash_profile, etc.) before running the command. Since the helper is explicitly sourced viasource "$OPENCLAW_JSON_HELPER", the login behavior adds no value here and risks non-deterministic, environment-dependent test behavior (and extra startup cost).♻️ Proposed change
- return spawnSync("bash", ["-lc", 'source "$OPENCLAW_JSON_HELPER"; parse_openclaw_agent_text'], { + return spawnSync("bash", ["-c", 'source "$OPENCLAW_JSON_HELPER"; parse_openclaw_agent_text'], {Note: the ast-grep
detect-child-processhint here is a false positive — the command string is static and untrusted data flows in only viainput(stdin), not into the command/args.🤖 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/openclaw-agent-json.test.ts` around lines 20 - 27, The shell helper in runShellHelper is using bash -lc, which adds login-shell startup behavior that can make the test non-hermetic and environment-dependent. Update the spawnSync invocation to use bash -c while still sourcing OPENCLAW_JSON_HELPER and calling parse_openclaw_agent_text, keeping the helper loaded explicitly and preserving stdin input handling.
🤖 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/openclaw-agent-json.test.ts`:
- Around line 20-27: The shell helper in runShellHelper is using bash -lc, which
adds login-shell startup behavior that can make the test non-hermetic and
environment-dependent. Update the spawnSync invocation to use bash -c while
still sourcing OPENCLAW_JSON_HELPER and calling parse_openclaw_agent_text,
keeping the helper loaded explicitly and preserving stdin input handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dc0082c5-2743-4e4a-83b9-3e864c9416c6
📒 Files selected for processing (9)
docs/reference/commands.mdxsrc/lib/actions/sandbox/agent/passthrough-json.test.tssrc/lib/actions/sandbox/agent/passthrough-json.tssrc/lib/actions/sandbox/agent/passthrough.test.tssrc/lib/actions/sandbox/agent/passthrough.tssrc/lib/openclaw/agent-json-provenance.test.tssrc/lib/openclaw/agent-json-provenance.tstest/e2e-scenario/live/sandbox-operations.test.tstest/openclaw-agent-json.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/reference/commands.mdx
- src/lib/actions/sandbox/agent/passthrough-json.ts
- src/lib/openclaw/agent-json-provenance.ts
- src/lib/actions/sandbox/agent/passthrough.ts
- src/lib/actions/sandbox/agent/passthrough.test.ts
…-provenance Signed-off-by: Aaron Erickson <aerickson@nvidia.com> # Conflicts: # test/e2e-scenario/live/sandbox-operations.test.ts # test/e2e/lib/openclaw-json.sh
cv
approved these changes
Jun 25, 2026
…-provenance # Conflicts: # docs/reference/commands.mdx # src/commands/sandbox/agent.ts # src/lib/actions/sandbox/agent/passthrough.test.ts # src/lib/actions/sandbox/agent/passthrough.ts
21 tasks
21 tasks
cv
pushed a commit
that referenced
this pull request
Jun 29, 2026
## Summary Adds the v0.0.69 release notes to the published release-notes page so users can see the shipped sandbox recovery, Deep Agents Code, Hermes, inference, policy, and release-validation changes. The section is based on the v0.0.69 announcement and links each user-facing theme to the deeper docs pages that already cover the behavior. ## Changes - Added a new `v0.0.69` section to `docs/about/release-notes.mdx`. - Linked release-note themes to lifecycle, backup, troubleshooting, Deep Agents Code, commands, workspace, messaging, Hermes, inference, security, monitoring, and network-policy docs. Source summary: - #5455 -> `docs/about/release-notes.mdx`: Summarized persistent workspace and state cleanup during sandbox destroy. - #5738 -> `docs/about/release-notes.mdx`: Summarized nonzero exit status preservation for failed hosted endpoint validation. - #5786 -> `docs/about/release-notes.mdx`: Summarized live sandbox rediscovery when local registry state is missing. - #5881 -> `docs/about/release-notes.mdx`: Summarized the `nemo-deepagents` alias command surface. - #5594 -> `docs/about/release-notes.mdx`: Summarized the Hermes Agent 2026.6.19 update. - #5777 -> `docs/about/release-notes.mdx`: Summarized manifest-derived messaging channel support. - #5825 -> `docs/about/release-notes.mdx`: Summarized DeepSeek V4 Flash managed-vLLM defaults for DGX Station. - #5877 -> `docs/about/release-notes.mdx`: Summarized provider switch metadata preservation. - #5932 -> `docs/about/release-notes.mdx`: Summarized transient inference smoke retry behavior. - #5934 -> `docs/about/release-notes.mdx`: Summarized constrained inference smoke retry boundaries. - #5681 -> `docs/about/release-notes.mdx`: Summarized Shields config-hash sealing during auto-restore. - #5682 -> `docs/about/release-notes.mdx`: Summarized sandbox connect process-limit enforcement. - #5683 -> `docs/about/release-notes.mdx`: Summarized JSON agent failure provenance warnings. - #5711 -> `docs/about/release-notes.mdx`: Summarized sparse-source log breadcrumbs. - #5838 -> `docs/about/release-notes.mdx`: Summarized host-authoritative Shields status. - #5880 -> `docs/about/release-notes.mdx`: Summarized policy round-trip documentation updates. - #5886 -> `docs/about/release-notes.mdx`: Summarized network request approval-flow documentation updates. ## 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 - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: doc-only release-notes prose; no runtime behavior changed. - [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 - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) `npm run docs` passed with 0 errors and the existing Fern light-mode accent contrast warning. `fern check --warnings` reported the same accent-color warning. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for **v0.0.69**, covering improved sandbox lifecycle recovery (state preservation across destroy/recreate/rebuild/recovery/validation failures), clearer Deep Agents Code terminal/CLI behavior, and safer Hermes messaging/provider switching with manifest-driven channels. * Improved inference setup validation guidance, including handling of local/compatible endpoints and redaction of sensitive validation errors. * Refreshed release-gate documentation with clearer approval examples and validation behavior for NVIDIA API keys vs hosted inference keys. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary - add a production `agent --json` passthrough path that preserves JSON stdout and appends provenance warnings to stderr - detect failed tool-result metadata and untrusted child-result framing in OpenClaw JSON/log-prefixed JSON - route the E2E JSON parser through provenance-preserving logic so plausible assistant text cannot hide failed or unverified work Fixes NVIDIA#3933 ## Test plan - `./node_modules/.bin/vitest run src/lib/openclaw/agent-json-provenance.test.ts src/lib/actions/sandbox/agent/passthrough-json.test.ts src/lib/actions/sandbox/agent/passthrough.test.ts test/openclaw-agent-json.test.ts --reporter=verbose` - `npx tsc -p tsconfig.src.json --noEmit` - `npm run build:cli` - `npm run typecheck:cli` - `python3 -m py_compile test/e2e/lib/openclaw-agent-json.py` - `bash -n test/e2e/lib/openclaw-json.sh` - `git diff --check` ## Boundary NemoClaw can surface provenance when OpenClaw JSON includes failed tool-result metadata or untrusted-child framing; if upstream omits that metadata entirely, NemoClaw cannot reconstruct it. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added a `--json` mode for `openclaw agent` that keeps stdout parseable JSON (no TTY) while forwarding stderr and preserving exit behavior. * Detects and appends failed-tool and untrusted-child provenance to stderr when appropriate, with sanitized/limited provenance details. * **Documentation** * Updated command help and reference docs to clarify when the `--json` mode is used and how forwarded args after `--` behave. * **Tests** * Expanded unit and E2E coverage for provenance extraction, sanitization, ordering expectations, and wrapper argument/flag handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Adds the v0.0.69 release notes to the published release-notes page so users can see the shipped sandbox recovery, Deep Agents Code, Hermes, inference, policy, and release-validation changes. The section is based on the v0.0.69 announcement and links each user-facing theme to the deeper docs pages that already cover the behavior. ## Changes - Added a new `v0.0.69` section to `docs/about/release-notes.mdx`. - Linked release-note themes to lifecycle, backup, troubleshooting, Deep Agents Code, commands, workspace, messaging, Hermes, inference, security, monitoring, and network-policy docs. Source summary: - NVIDIA#5455 -> `docs/about/release-notes.mdx`: Summarized persistent workspace and state cleanup during sandbox destroy. - NVIDIA#5738 -> `docs/about/release-notes.mdx`: Summarized nonzero exit status preservation for failed hosted endpoint validation. - NVIDIA#5786 -> `docs/about/release-notes.mdx`: Summarized live sandbox rediscovery when local registry state is missing. - NVIDIA#5881 -> `docs/about/release-notes.mdx`: Summarized the `nemo-deepagents` alias command surface. - NVIDIA#5594 -> `docs/about/release-notes.mdx`: Summarized the Hermes Agent 2026.6.19 update. - NVIDIA#5777 -> `docs/about/release-notes.mdx`: Summarized manifest-derived messaging channel support. - NVIDIA#5825 -> `docs/about/release-notes.mdx`: Summarized DeepSeek V4 Flash managed-vLLM defaults for DGX Station. - NVIDIA#5877 -> `docs/about/release-notes.mdx`: Summarized provider switch metadata preservation. - NVIDIA#5932 -> `docs/about/release-notes.mdx`: Summarized transient inference smoke retry behavior. - NVIDIA#5934 -> `docs/about/release-notes.mdx`: Summarized constrained inference smoke retry boundaries. - NVIDIA#5681 -> `docs/about/release-notes.mdx`: Summarized Shields config-hash sealing during auto-restore. - NVIDIA#5682 -> `docs/about/release-notes.mdx`: Summarized sandbox connect process-limit enforcement. - NVIDIA#5683 -> `docs/about/release-notes.mdx`: Summarized JSON agent failure provenance warnings. - NVIDIA#5711 -> `docs/about/release-notes.mdx`: Summarized sparse-source log breadcrumbs. - NVIDIA#5838 -> `docs/about/release-notes.mdx`: Summarized host-authoritative Shields status. - NVIDIA#5880 -> `docs/about/release-notes.mdx`: Summarized policy round-trip documentation updates. - NVIDIA#5886 -> `docs/about/release-notes.mdx`: Summarized network request approval-flow documentation updates. ## 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 - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: doc-only release-notes prose; no runtime behavior changed. - [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 - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) `npm run docs` passed with 0 errors and the existing Fern light-mode accent contrast warning. `fern check --warnings` reported the same accent-color warning. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for **v0.0.69**, covering improved sandbox lifecycle recovery (state preservation across destroy/recreate/rebuild/recovery/validation failures), clearer Deep Agents Code terminal/CLI behavior, and safer Hermes messaging/provider switching with manifest-driven channels. * Improved inference setup validation guidance, including handling of local/compatible endpoints and redaction of sensitive validation errors. * Refreshed release-gate documentation with clearer approval examples and validation behavior for NVIDIA API keys vs hosted inference keys. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 task
prekshivyas
added a commit
that referenced
this pull request
Aug 12, 2026
## Summary `nemoclaw <sandbox> agent` reported success for a turn that never happened. Both captured transports treated "the child exited 0" as the only success signal, so an exec that returned status `0` with zero bytes on stdout **and** stderr was relayed as a completed turn — no output, no warning, no non-zero exit, and no signal to the caller that the message was never delivered. This PR makes that state fail loud, and fixes two adjacent defects on the same dispatch. It also includes #8857, which rejects a second false-success state: a response whose authoritative run metadata marks the turn incomplete or abandoned. ## Related Issue Refs #8796 Deliberately `Refs`, not `Closes` — see [Scope](#scope). This PR fixes the *reporting* contract, which is NemoClaw-owned and proven. It does not establish why the dispatch was empty. ## Changes **1. Empty-dispatch guard (the reported contract violation).** A delivered OpenClaw turn cannot be byte-empty on both streams: the in-sandbox NemoClaw plugin writes its registration banner to stderr on every invocation (`nemoclaw/src/index.ts:404-419`, already documented in `docs/reference/commands.mdx`). A zero-exit, zero-byte dispatch is therefore reported as a failure with the documented recovery paths instead of a successful turn. - Requires **both** streams empty, so a quiet-but-real turn never misfires. - Excludes non-zero status, signal kills (`status === null`) and transport errors — those already report themselves. - On the JSON path it runs ahead of the stdout write, so machine-readable stdout stays byte-empty and no provenance line is appended for a turn that never ran. **2. Restore the owning-gateway `-g` pin on both transports.** #7113 established the explicit gateway argument as the per-subprocess authority precisely because the process-global active selection can be changed by another CLI at any moment — `gateway-state.ts:540` documents this ("never trust that process-global state ... The explicit gateway argument below is the per-subprocess authority"). #8191 dropped that pin when it moved the non-JSON transport off `execSandbox` onto a raw `spawnSync`; the JSON transport (#5683) predates #7113 and never had it. Restored on both. **3. Stop handing an interactive terminal to a non-interactive dispatch.** #8191 also hard-coded `stdio[0] = "inherit"`, so a live TTY was forwarded into a dispatch whose stdout and stderr are pipes. A TTY is now withheld; a genuine pipe or redirect is still forwarded, so `printf 'ping' | nemoclaw my-assistant agent --agent main` keeps working. **This is fd hygiene, not a delivery fix** — see below. **4. Reject response envelopes that mark the turn incomplete.** A turn can produce a JSON trace and exit `0` even though its authoritative metadata carries `error.kind = incomplete_turn`, `livenessState = abandoned`, or `replayInvalid = true`. The wrapper now preserves stdout byte-for-byte, reports those markers and verify-before-retry guidance on stderr, and exits `1`. It selects only the final matching OpenClaw response envelope — local `{ payloads, meta }` or gateway `{ status, result: { payloads, meta } }` — so earlier JSON progress records, tool results, and tool-call arguments cannot reclassify a completed turn. The classifier and stdio shape live in a new `passthrough-dispatch.ts`; the operator-facing failure text lives beside the existing help copy in `passthrough-help.ts`. ## Scope Symptoms (a) exit 0, (b) no output and (d) `--json` silent are fixed and NemoClaw-owned. The merged #8857 follow-up also fixes the NemoClaw-owned case where an emitted JSON response explicitly marks the turn incomplete. Symptom (c) — the message is never delivered, no session created — is **not** fixed here, and I could not identify its cause. I traced this against the exact OpenShell the reporter ran (tag `v0.0.85`) to test the leading hypothesis, that a live TTY on fd 0 combined with `--no-tty` caused the drop. **That hypothesis is disproven at the source level:** - `crates/openshell-cli/src/run.rs:2942` — a terminal fd 0 yields an *empty* stdin payload without reading; `/dev/null` yields an empty payload after a 0-byte read. Identical value, and no blocking read on a terminal. - `run.rs:2965` — `--no-tty` pins `tty=false` either way. - `run.rs:2968` — the interactive RPC is gated on `tty_override == Some(true)`, so `--no-tty` never reaches it. - `run.rs:2981-2993` — the command vector is sent unconditionally. With `--no-tty`, OpenShell builds a **byte-identical `ExecSandboxRequest`** whether fd 0 is a live terminal or `/dev/null`. So change 3 alters nothing OpenShell can observe. It is still worth doing — a documented non-interactive one-shot should not hand a terminal to a captured dispatch — but it is not why anything would start working, and I have removed the earlier claim that it was. That also disposes of the two upstream TTY/exec fixes landed after `v0.0.85` (`a2cd5f8e`, first in `v0.0.88`; `0d5e5c53`, first in `v0.0.93`): both are on the *interactive* exec path, which this dispatch never takes. **What I did find upstream** is a matching silent-success shape in the OpenShell CLI itself: ```rust // crates/openshell-cli/src/run.rs:2997 let mut exit_code = 0i32; // only overwritten by an Exit event (:3014) ... Ok(exit_code) // :3021 — stream ended, no Exit event -> 0 ``` The server already has the correct anti-default for the analogous case — `exec_loop_result` maps a missing exit status to `Status::unavailable("exec relay closed before the command reported an exit status")` (`crates/openshell-server/src/grpc/sandbox.rs:1507-1516`) — but the CLI does not apply the same rule to its own stream. If that is what the reporter hit, the root cause is upstream and this guard is the correct host-side response until it is fixed. I'm happy to file that OpenShell issue if maintainers agree with the reading. Note also that the reporter ran OpenShell `0.0.85` while current `main` ships `0.0.101` (#8660), so a re-test on current main is worth doing before assuming the delivery symptom is still live. **Residual, out of scope:** `proc.exit()` runs in the same tick as the stdout write on both transports, truncating replies past ~80 KiB when stdout is a pipe. The fix is `process.exitCode` + a normal return, which changes two `: never` signatures. Separate PR. ## Testing - `npx vitest run --project cli src/lib/openclaw/ src/lib/actions/sandbox/agent/ test/openclaw-agent-json.test.ts` — 8 files, 148 tests, all pass - `npx vitest run --project cli src/lib/actions/sandbox/` — 2685/2686 pass. The one failure, `gateway-restart-hermes-drift.test.ts:126`, reproduces on a clean tree without this diff: it shells out to host `python3` and uses `zip(strict=)`, which needs ≥3.10. - `npm run typecheck:cli`, `npm run lint`, `npm run build:cli` — clean - Repo gates: source-architecture budget, test-file-size budget, source-shape test budget, test-title style, layer import boundaries — all pass. The source-architecture budget records the expected fan-in increases for `core/shell-quote.ts` (26 to 27) and `security/redact.ts` (51 to 52); the new module avoids importing `cli/branding`, whose fan-in remains 86. New coverage: the classifier's misfire cases (stdout-only, stderr-only, non-zero, signal-killed, transport error), the stdio shape in both stdin postures, the diagnostic copy, and — on both transports — the empty-dispatch exit, the `-g` argv pin, and the withheld TTY. Completion-marker coverage includes each accepted marker, healthy and non-JSON responses, tool-result and tool-argument false positives, log-prefixed framing, and a marker-bearing JSON progress record followed by a healthy response at both classifier and transport layers. The JSON pin test asserts against the real `buildOpenshellExecArgs`, so it pins the actual argv (`["sandbox","exec","--name","alpha","-g","nemoclaw-8081"]`). Four pre-existing `passthrough-json.test.ts` cases gained injected seams: without them the new defaults would read the developer's real `~/.nemoclaw` registry and the real fd 0. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Agent dispatches that exit successfully without producing output now fail clearly with status `1`. - Added recovery guidance, including direct execution and gateway recovery commands. - Sensitive information is redacted from recovery diagnostics. - Interactive terminal input is no longer forwarded during non-interactive dispatches. - Genuine piped or redirected input continues to be preserved. - Agent commands now target the explicitly resolved owning gateway. - **Documentation** - Updated command reference documentation with silent-dispatch failure behavior, recovery guidance, gateway selection, and input-handling details. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Maintainer Readiness Evidence - Scope increase: this PR has a substantial net line increase, dominated by focused regression tests and command-reference documentation. - Security review: all nine categories pass. Recovery commands retain shell quoting, detected credential values are redacted, and a changed diagnostic command is marked non-replayable. No authorization bypass, dependency, cryptography, or configuration weakening was introduced. - Verification: 148 targeted agent/OpenClaw tests, 24 gateway-health tests, and 30 OpenClaw startup compatibility tests pass; the documentation build and full PR validation pass. The four maintainer commits include DCO sign-off and appear as Verified in GitHub. - CI classification: the earlier installer failure reproduced as passing locally and was transient. The earlier gateway-health failure reproduced locally as an upstream fixture regression and is fixed by including the shared launcher in the extracted fixture. The branch now includes current `main`. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/reference/commands.mdx` documents shell quoting, credential redaction, runnable versus non-replayable recovery guidance, and final-response-envelope selection that ignores earlier JSON progress records. The OpenClaw-only generated variant and published routes were verified. - Agent: Codex Desktop <!-- docs-review-head-sha: 625fec1 --> <!-- docs-review-agents-blob-sha: c4923a3 --> --------- Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: prekshivyas <prekshiv@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@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
4 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
agent --jsonpassthrough path that preserves JSON stdout and appends provenance warnings to stderrFixes #3933
Test plan
./node_modules/.bin/vitest run src/lib/openclaw/agent-json-provenance.test.ts src/lib/actions/sandbox/agent/passthrough-json.test.ts src/lib/actions/sandbox/agent/passthrough.test.ts test/openclaw-agent-json.test.ts --reporter=verbosenpx tsc -p tsconfig.src.json --noEmitnpm run build:clinpm run typecheck:clipython3 -m py_compile test/e2e/lib/openclaw-agent-json.pybash -n test/e2e/lib/openclaw-json.shgit diff --checkBoundary
NemoClaw can surface provenance when OpenClaw JSON includes failed tool-result metadata or untrusted-child framing; if upstream omits that metadata entirely, NemoClaw cannot reconstruct it.
Summary by CodeRabbit
Release Notes
--jsonmode foropenclaw agentthat keeps stdout parseable JSON (no TTY) while forwarding stderr and preserving exit behavior.--jsonmode is used and how forwarded args after--behave.