fix(agent): fail loud when an agent dispatch delivers nothing - #8846
Conversation
`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. A delivered OpenClaw turn cannot look like that. The in-sandbox NemoClaw plugin writes its registration banner to stderr on every invocation, so a healthy turn always produces bytes on one of the two streams. Treat a zero-exit, zero-byte dispatch as a failure and print the documented recovery paths instead of laundering it into a success. The guard requires both streams to be empty so a quiet-but-real turn never misfires, and it runs ahead of the JSON stdout write so machine-readable stdout stays byte-empty. Two adjacent defects on the same dispatch, both introduced when #8191 moved the non-JSON transport off `execSandbox` onto a raw `spawnSync`: - The owning-gateway `-g` pin was dropped. #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. Restore it on both transports; the JSON path never had it. - fd 0 was hard-coded to `inherit`, handing an interactive terminal to a documented non-interactive one-shot whose stdout and stderr are pipes. Withhold a TTY; a genuine pipe or redirect is still forwarded, so `printf ping | nemoclaw my-assistant agent --agent main` keeps working. The underlying reason the DGX Spark dispatch returned empty is not provable from this repo and is downstream of NemoClaw, so this does not close the report. Refs #8796 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAgent passthrough now treats zero-output successful dispatches as failures. It adds owning-gateway selection, TTY-aware stdin handling, and recovery diagnostics for JSON and non-JSON paths. It also updates the command reference and architecture budget. ChangesAgent dispatch delivery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Passthrough
participant OpenShell
participant HelpWriter
Caller->>Passthrough: start agent dispatch
Passthrough->>OpenShell: execute with gateway and selected stdio
OpenShell-->>Passthrough: return status, stdout, and stderr
Passthrough->>HelpWriter: report silent dispatch failure
HelpWriter-->>Caller: emit recovery guidance
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit d86efba in the TypeScript / code-coverage/cliThe overall coverage in commit d86efba in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-8846.docs.buildwithfern.com/nemoclaw |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/actions/sandbox/agent/passthrough.test.ts (1)
65-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the spawned argv instead of the helper call.
This test asserts an internal
buildOpenshellExecArgscall. Assert the argv passed tospawnSyncMockinstead. That verifies the OpenShell boundary that must receive-g.Proposed fix
-import { buildOpenshellExecArgs } from "../exec"; ... - expect(buildOpenshellExecArgs).toHaveBeenCalledWith( - "my-sb", - expect.anything(), - { tty: false }, - "nemoclaw-8081", - ); + expect(vi.mocked(spawnSyncMock).mock.calls[0]?.[1].slice(0, 6)).toEqual([ + "sandbox", + "exec", + "--name", + "my-sb", + "-g", + "nemoclaw-8081", + ]);As per path instructions: “Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions.”
Also applies to: 928-945
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/agent/passthrough.test.ts` at line 65, Update the passthrough test to stop asserting the internal buildOpenshellExecArgs helper call; instead, assert the argv supplied to spawnSyncMock, including the required -g argument, so the test verifies the OpenShell process boundary. Apply the same change to the related assertions around the referenced passthrough test cases.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/commands.mdx`:
- Line 1214: Update the inline host CLI example in the shared command reference
to use the variant-aware $$nemoclaw token instead of the literal nemoclaw
command, while preserving the existing arguments and pipe behavior.
In `@src/lib/actions/sandbox/agent/passthrough-help.ts`:
- Around line 26-29: Update the recovery-path output in passthrough-help.ts so
the documented direct-execution command is actionable, using `openclaw agent
--help` or preserving the original selector and arguments. Update
passthrough-help.test.ts to assert the complete corrected command.
---
Nitpick comments:
In `@src/lib/actions/sandbox/agent/passthrough.test.ts`:
- Line 65: Update the passthrough test to stop asserting the internal
buildOpenshellExecArgs helper call; instead, assert the argv supplied to
spawnSyncMock, including the required -g argument, so the test verifies the
OpenShell process boundary. Apply the same change to the related assertions
around the referenced passthrough test cases.
🪄 Autofix
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: 0021e6a1-c228-4395-92ef-76e0232c48dc
📒 Files selected for processing (9)
docs/reference/commands.mdxsrc/lib/actions/sandbox/agent/passthrough-dispatch.test.tssrc/lib/actions/sandbox/agent/passthrough-dispatch.tssrc/lib/actions/sandbox/agent/passthrough-help.test.tssrc/lib/actions/sandbox/agent/passthrough-help.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
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
6 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
3 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 4 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: None Manual-only E2E: 1 optional E2E recommendation
1 warning · 0 suggestionsWarningsWarnings do not block.
|
prekshivyas
left a comment
prekshivyas
left a comment
There was a problem hiding this comment.
Please resolve the two current exact-head correctness threads before approval. In particular, the printed recovery command exec -- openclaw agent has neither a target selector nor the original turn arguments, so it exits instead of providing the promised direct-execution recovery path; make the guidance executable and cover the complete command in the test. The shared command-reference example also needs the variant-aware $$nemoclaw token.
The printed recovery path was `nemoclaw <sb> exec -- openclaw agent`, which carries neither a target selector nor the original turn arguments. Run as printed it hits the selector guard and exits 2 with "No target session selected", so the promised direct-execution recovery path did not actually execute anything. Reproduce the dispatched turn instead: the recovery command now carries the forwarded argv verbatim, so it runs the same turn inside the sandbox. The sandbox name and the forwarded arguments are user-controlled command text, so both are shell-quoted, matching the presentation boundary the shields relock warning already documents in this directory. The command-reference example also now uses the variant-aware `$$nemoclaw` token so it renders correctly for each CLI variant. `shell-quote.ts` gains its 27th consumer, so its architecture budget goes 26 -> 27. Rendering user-controlled argv into a suggested shell command requires quoting, and duplicating the shared quoting primitive to stay under the ratchet would be strictly worse; the sibling relock warning imports it for exactly this purpose. Addresses review feedback on #8846. Refs #8796 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
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 `@src/lib/actions/sandbox/agent/passthrough-help.ts`:
- Around line 26-27: Redact sensitive values before constructing the directRun
recovery command in the passthrough-help flow. Update the command handling near
directRun to mask credential-bearing options such as --session-key and message
or session arguments such as -m, then shell-quote the redacted values before
logging; preserve executable recovery behavior without exposing forwarded agent
text or secrets.
🪄 Autofix
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: 15534469-459d-4a58-ad19-b800f5aad077
📒 Files selected for processing (6)
ci/source-architecture-budget.jsondocs/reference/commands.mdxsrc/lib/actions/sandbox/agent/passthrough-help.test.tssrc/lib/actions/sandbox/agent/passthrough-help.tssrc/lib/actions/sandbox/agent/passthrough-json.tssrc/lib/actions/sandbox/agent/passthrough.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/reference/commands.mdx
- src/lib/actions/sandbox/agent/passthrough-help.test.ts
- src/lib/actions/sandbox/agent/passthrough-json.ts
- src/lib/actions/sandbox/agent/passthrough.ts
The recovery command echoes the forwarded turn argv, which can carry message text and session identifiers. Stderr is captured by CI logs and by automation that may never log the invocation itself, so printing the argv verbatim can amplify a credential into a log it would not otherwise reach. Redact the rendered command with the shared redactor. Redaction is pattern-based, so ordinary prompt text passes through unchanged and the command stays runnable, which keeps the executable-recovery behaviour this PR added; a command that does get masked is one nobody should replay verbatim anyway. `security/redact.ts` gains its 52nd consumer, so its architecture budget goes 51 -> 52, alongside the shell-quote bump to 27 in the previous commit. Both are shared primitives this one diagnostic must apply to user-controlled text: it has to quote for correctness and redact for safety, and duplicating either primitive to stay under a ratchet would be worse than raising it. Addresses review feedback on #8846. Refs #8796 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
All open threads on this PR are addressed. Summary of what landed and where. 1. Recovery command was not executable (your review, and the CodeRabbit thread on
The sandbox name and the forwarded argv are user-controlled command text, so both are shell-quoted, matching the presentation boundary 2. Shared command-reference example needed the variant-aware token (your review, and the CodeRabbit thread on 3. Redact sensitive forwarded arguments before logging (CodeRabbit, 🟠 Major, raised against This one conflicted with making the command executable, so rather than trade one requirement against the other I ran the rendered command through the shared Architecture budgets. This PR raises two: Verification: 120 tests green across One CI note, not caused by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/actions/sandbox/agent/passthrough-help.test.ts (1)
100-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that redaction preserves the recovery command.
The assertion only proves that the token is absent. It also passes if the helper drops the recovery command or the
-mvalue. Assert that the quoted command and a non-secret replacement remain. Do not assert internalredactFullcalls.As per path instructions, “Review tests for behavioral confidence rather than implementation lock-in.”
Proposed test improvement
- expect(lines.join("")).not.toContain("sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH"); + const output = lines.join(""); + expect(output).not.toContain("sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH"); + expect(output).toContain("'--agent' 'main' '-m' '");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/agent/passthrough-help.test.ts` around lines 100 - 113, Strengthen the test around writeSilentAgentDispatchFailure by asserting that the emitted stderr still contains the quoted recovery command with the -m argument, while also containing a non-secret redacted replacement instead of the credential. Keep the assertions focused on observable output and do not verify internal redactFull calls.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/actions/sandbox/agent/passthrough-help.ts`:
- Around line 35-39: Update the passthrough-help rendering around directRun and
the Line 48 message to use directRunWasRedacted: retain the “exact turn” wording
only when the redacted command is unchanged, and otherwise state that sensitive
values were redacted and the command must not be replayed.
---
Nitpick comments:
In `@src/lib/actions/sandbox/agent/passthrough-help.test.ts`:
- Around line 100-113: Strengthen the test around
writeSilentAgentDispatchFailure by asserting that the emitted stderr still
contains the quoted recovery command with the -m argument, while also containing
a non-secret redacted replacement instead of the credential. Keep the assertions
focused on observable output and do not verify internal redactFull calls.
🪄 Autofix
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: 0501ced8-15c3-4cae-9313-509fcfc1f56a
📒 Files selected for processing (3)
ci/source-architecture-budget.jsonsrc/lib/actions/sandbox/agent/passthrough-help.test.tssrc/lib/actions/sandbox/agent/passthrough-help.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- ci/source-architecture-budget.json
The redaction case asserted only that the credential was absent, which also passes if the recovery command is dropped or the `-m` value is lost entirely. Assert the exact surviving command instead, so one assertion covers both halves: the credential is replaced by the redaction marker and the quoted command, its selector, and its `-m` flag all survive. Addresses review feedback on #8846. Refs #8796 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
Fixed in I went slightly further than the suggested prefix and pinned the exact surviving command, so a single assertion covers both halves: const output = lines.join("");
expect(output).not.toContain("sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH");
expect(output).toContain(
"nemoclaw 'my-assistant' exec -- 'openclaw' 'agent' '--agent' 'main' '-m' 'use <REDACTED>'",
);That proves the credential was replaced by the redaction marker and that the quoted command, its selector, and its The sibling case asserting |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
The latest PR commit preserves the complete target selector and forwarded arguments in the recovery command. The command reference also uses the variant-aware token. Dismissing this resolved review before a full current-commit review.
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> # Conflicts: # test/openclaw-2026-7-startup-compat.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
wscurran
added
area: cli
## Summary `nemoclaw <sandbox> agent --json` reports success for a turn its own payload marks incomplete. Reported on #8796: a run with **seven successful tool calls** and a real on-disk side effect surfaced "Agent couldn't generate a response", carried `error.kind = incomplete_turn`, `livenessState = abandoned` and `replayInvalid = true` — and still exited `0` while the envelope's top-level status said `status: ok` / `summary: completed`. Automation reading that status is told the turn succeeded when it did not. > **Stacked on #8846.** Base branch is `fix/8796-agent-gateway-pin`, not `main` — both PRs touch `passthrough-json.ts`, so stacking keeps this diff minimal and conflict-free. Merge #8846 first; this retargets to `main` cleanly afterwards. ## Related Issue Refs #8796 ## Why neither existing guard catches it - **Empty-dispatch guard** (#8846) requires both streams byte-empty. Here stdout carries a full JSON envelope, so it correctly does not fire. - **Provenance walk** only recognises *failed tools*: `toolFailureLine` requires `isToolLike(record) && hasFailureStatus(record)` (`agent-json-provenance.ts:155`). Every tool in this turn succeeded, so no line is produced — and the turn-level error record is not tool-like, so it is skipped entirely. NemoClaw was structurally blind to **turn-level** failure; it only ever understood **tool-level** failure. ## Changes **`openClawAgentIncompleteTurnSignal(raw)`** reads only declared run-metadata records at `<root>.meta` or `<root>.result.meta`. It checks `error.kind`, `livenessState`, and `replayInvalid`, normalizes string marker values, and requires `replayInvalid === true`. It ignores marker-shaped fields in tool results, tool-call arguments, and other descendants, preventing completed turns from being reclassified and retried after side effects. It reuses `parseOpenClawJsonDocs`, including its log-prefixed framing support. **Reporting** follows the reporter's recommendation exactly: - **Non-success process status** — exit `1` when a marker is present and the child exited `0`. - **Partial tool trace preserved** — stdout is still written first and byte-unchanged, ahead of the check, so the JSON trace and its provenance are already on the wire. - **Verify-before-retry guidance retained** — stderr carries the verdict, the markers behind it, and an explicit warning that tool calls in a partial trace may already have applied side effects, so a blind retry can repeat them. - **An upstream non-zero exit status is passed through unchanged** rather than relabelled. ## Testing - `npx vitest run --project cli src/lib/openclaw/ src/lib/actions/sandbox/agent/ test/openclaw-agent-json.test.ts` — 8 files, 146 tests, all pass - `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. No budget file modified. New coverage — detector: healthy turn returns `null`, each marker detected individually, all markers reported deduped, `replayInvalid: "false"` (a string) correctly ignored, log-prefixed framing, and non-JSON stdout. Transport: the incomplete turn exits `1` with stdout preserved byte-for-byte and all three markers plus the retry guidance on stderr; a completed turn stays at exit `0`; an upstream `7` is passed through unchanged. The negative cases confirm that marker-shaped fields in successful tool results and tool-call arguments do not reclassify a completed turn. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/reference/commands.mdx` - Agent: Codex Desktop <!-- docs-review-head-sha: 9fe568f --> <!-- docs-review-agents-blob-sha: c4923a3 --> 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) --------- Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Readiness update after the documentation review and current-base refresh:
Fresh repository checks are running. The earlier static-check failure was an HTTP 503 while downloading hadolint, before repository checks ran; this new push starts a fresh check set. Human review is still required. |
Signed-off-by: prekshivyas <prekshiv@nvidia.com>
Signed-off-by: prekshivyas <prekshiv@nvidia.com>
prekshivyas
left a comment
prekshivyas
left a comment
There was a problem hiding this comment.
Approved after fresh review of the current diff.
5 participants
Summary
nemoclaw <sandbox> agentreported 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 status0with 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, notCloses— see 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 indocs/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.status === null) and transport errors — those already report themselves.2. Restore the owning-gateway
-gpin 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:540documents 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 offexecSandboxonto a rawspawnSync; 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, soprintf 'ping' | nemoclaw my-assistant agent --agent mainkeeps 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
0even though its authoritative metadata carrieserror.kind = incomplete_turn,livenessState = abandoned, orreplayInvalid = true. The wrapper now preserves stdout byte-for-byte, reports those markers and verify-before-retry guidance on stderr, and exits1. 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 inpassthrough-help.ts.Scope
Symptoms (a) exit 0, (b) no output and (d)
--jsonsilent 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-ttycaused 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/nullyields an empty payload after a 0-byte read. Identical value, and no blocking read on a terminal.run.rs:2965—--no-ttypinstty=falseeither way.run.rs:2968— the interactive RPC is gated ontty_override == Some(true), so--no-ttynever reaches it.run.rs:2981-2993— the command vector is sent unconditionally.With
--no-tty, OpenShell builds a byte-identicalExecSandboxRequestwhether 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 inv0.0.88;0d5e5c53, first inv0.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:
The server already has the correct anti-default for the analogous case —
exec_loop_resultmaps a missing exit status toStatus::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.85while currentmainships0.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 isprocess.exitCode+ a normal return, which changes two: neversignatures. 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 passnpx 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 hostpython3and useszip(strict=), which needs ≥3.10.npm run typecheck:cli,npm run lint,npm run build:cli— cleancore/shell-quote.ts(26 to 27) andsecurity/redact.ts(51 to 52); the new module avoids importingcli/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
-gargv 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 realbuildOpenshellExecArgs, so it pins the actual argv (["sandbox","exec","--name","alpha","-g","nemoclaw-8081"]).Four pre-existing
passthrough-json.test.tscases gained injected seams: without them the new defaults would read the developer's real~/.nemoclawregistry 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
Summary by CodeRabbit
Bug Fixes
1.Documentation
Maintainer Readiness Evidence
main.Documentation Writer Review
docs-updateddocs/reference/commands.mdxdocuments 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.