Skip to content

fix(cli): close non-terminal exec stdin by default - #6393

Closed
chengjiew wants to merge 1 commit into
mainfrom
fix/6319_exec-stdin
Closed

fix(cli): close non-terminal exec stdin by default#6393
chengjiew wants to merge 1 commit into
mainfrom
fix/6319_exec-stdin

Conversation

@chengjiew

@chengjiew chengjiew commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Close non-terminal or unavailable stdin by default for nemoclaw <sandbox> exec, preventing live pipes/SSH stdin from hanging one-shot commands.
  • Add explicit --stdin / --no-stdin controls so intentional piped scripts still work.
  • Update command docs and focused CLI/action tests for the new stdin policy.

Related Issue

Fixes #6319

Verification

  • npx vitest run --project cli src/commands/sandbox/exec.test.ts src/lib/actions/sandbox/exec-stdio.test.ts src/lib/actions/sandbox/exec.multiline-guard.test.ts src/lib/actions/sandbox/exec-openclaw-permission-cleanup.test.ts — 4 files / 47 tests passed.
  • npm run build:cli — passed.
  • npm run typecheck:cli — passed.
  • npm run docs:check-agent-variants — passed.
  • git diff --cached --check — passed before commit.
  • Commit hooks passed: format/lint/repository checks, gitleaks, env-var docs gate, test file size budget, commitlint.
  • Push hooks passed: CLI typecheck and package/tag version sync.

Linux Reproduction

Validated on aits@aits-log-worker-6 with Ready sandbox repro-6182-live.

Baseline nemoclaw v0.0.74-13-gf0d25491c:

/dev/null control:
ok
control_rc=0 elapsed=2s

held-open stdin:
live_rc=124 elapsed=25s

Fixed PR head nemoclaw v0.0.74-37-gd73686641:

/dev/null control:
ok
fixed_control_rc=0 elapsed=1s

held-open stdin default:
ok
fixed_live_rc=0 elapsed=1s

--stdin held-open stdin:
fixed_explicit_stdin_rc=124 elapsed=12s

Signed-off-by: Chengjie Wang chengjiew@nvidia.com

Summary by CodeRabbit

  • New Features

    • Added explicit --stdin and --no-stdin options for sandbox/exec commands to control stdin forwarding.
    • Updated command examples and usage text to show piping input and terminal/SSH-friendly workflows.
  • Bug Fixes

    • Changed default stdin behavior so non-terminal input is no longer inherited unexpectedly.
    • Improved command guidance for multi-line and pipe-based execution scenarios.

Fixes #6319

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c7036fad-fff5-4b16-a9b8-7cf244a686bf

📥 Commits

Reviewing files that changed from the base of the PR and between 5a6f489 and f458d01.

📒 Files selected for processing (10)
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx
  • src/commands/sandbox/exec.test.ts
  • src/commands/sandbox/exec.ts
  • src/lib/actions/sandbox/exec-openclaw-permission-cleanup.test.ts
  • src/lib/actions/sandbox/exec-policy-hint-emission.ts
  • src/lib/actions/sandbox/exec-stdio.test.ts
  • src/lib/actions/sandbox/exec-stdio.ts
  • src/lib/actions/sandbox/exec.multiline-guard.test.ts
  • src/lib/actions/sandbox/exec.ts

📝 Walkthrough

Walkthrough

Adds explicit --stdin/--no-stdin control to sandbox exec commands. A new stdio helper module decides whether to inherit stdin based on explicit flags or TTY detection, threaded through execSandbox, runSandboxExecChild, and the spawner. CLI flags, tests, and documentation are updated accordingly.

Changes

Sandbox exec stdin handling

Layer / File(s) Summary
Stdin inheritance decision helper
src/lib/actions/sandbox/exec-stdio.ts, src/lib/actions/sandbox/exec-stdio.test.ts
New module exports shouldInheritSandboxExecStdin and buildSandboxExecStdio, deciding stdio inheritance from an explicit flag or TTY detection, with unit tests.
execSandbox options and spawner wiring
src/lib/actions/sandbox/exec.ts, src/lib/actions/sandbox/exec.multiline-guard.test.ts, src/lib/actions/sandbox/exec-openclaw-permission-cleanup.test.ts, src/lib/actions/sandbox/exec-policy-hint-emission.ts
SandboxExecOptions gains stdin, SandboxExecSpawner/runSandboxExecChild accept an options parameter, defaultSandboxExecSpawner uses buildSandboxExecStdio, usage/error text and comments updated, with matching test updates.
CLI --stdin/--no-stdin flag
src/commands/sandbox/exec.ts, src/commands/sandbox/exec.test.ts
SandboxExecCommand adds a stdin boolean flag with allowNo, updates usage/examples, forwards flags.stdin into execSandbox options, with expanded tests.
Documentation updates
docs/reference/commands-nemohermes.mdx, docs/reference/commands.mdx
Documents default stdin inheritance behavior and --stdin/--no-stdin flags with usage examples across both exec command references.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SandboxExecCommand
  participant execSandbox
  participant runSandboxExecChild
  participant defaultSandboxExecSpawner
  participant buildSandboxExecStdio

  SandboxExecCommand->>execSandbox: run(options incl. stdin flag)
  execSandbox->>runSandboxExecChild: options
  runSandboxExecChild->>defaultSandboxExecSpawner: binary, args, options
  defaultSandboxExecSpawner->>buildSandboxExecStdio: options
  buildSandboxExecStdio-->>defaultSandboxExecSpawner: "inherit" or ["ignore","inherit","inherit"]
  defaultSandboxExecSpawner-->>runSandboxExecChild: spawned child process
Loading

Suggested labels: bug-fix

Suggested reviewers: ericksoa, cjagwani

✨ 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 fix/6319_exec-stdin

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

@github-code-quality

github-code-quality Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/6319_exec-stdin 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 fix/6319_exec-stdin f458d01 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.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 fix/6319_exec-stdin branch is 75%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/6319_exec-stdin f458d01 +/-
src/lib/shields...nsition-lock.ts 85%
src/lib/onboard/preflight.ts 83%
src/lib/actions...all/run-plan.ts 81%
src/lib/state/o...oard-session.ts 80%
src/lib/actions...licy-channel.ts 79%
src/lib/state/sandbox.ts 75%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/shields/index.ts 68%
src/lib/policy/index.ts 66%
src/lib/onboard.ts 28%

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

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@chengjiew

Copy link
Copy Markdown
Contributor Author

Superseded by #6394, which carries the same fix on a GitHub-verified signed commit.

@chengjiew chengjiew closed this Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 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 · 3 test follow-ups

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
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: with a Ready sandbox and a live held-open non-TTY stdin, `nemoclaw <sandbox> exec -- echo ok` returns promptly and prints `ok` without requiring `< /dev/null`.. Unit coverage is focused and appropriate for parser and stdio-selection logic, but the affected behavior is a sandbox/runtime process-I/O path where one end-to-end check with a live non-EOF stdin would provide higher confidence. I did not run tests or external E2E jobs during this read-only review.
  • PRA-T2 Runtime validation — Runtime validation: with the same live held-open stdin, `nemoclaw <sandbox> exec --stdin -- cat` forwards stdin intentionally and exhibits the expected blocking/consuming behavior until EOF or timeout.. Unit coverage is focused and appropriate for parser and stdio-selection logic, but the affected behavior is a sandbox/runtime process-I/O path where one end-to-end check with a live non-EOF stdin would provide higher confidence. I did not run tests or external E2E jobs during this read-only review.
  • PRA-T3 Runtime validation — Optional unit coverage: mock the fd-0 TTY probe at the production spawn boundary and assert the default runner uses `["ignore", "inherit", "inherit"]` when no `--stdin` flag is supplied and fd 0 is non-TTY.. Unit coverage is focused and appropriate for parser and stdio-selection logic, but the affected behavior is a sandbox/runtime process-I/O path where one end-to-end check with a live non-EOF stdin would provide higher confidence. I did not run tests or external E2E jobs during this read-only review.

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 7, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — 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 · 4 test follow-ups
Since last review: 0 prior items 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: Runtime validation
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: nemoclaw <sandbox> exec -- echo ok with live SSH stdin (no redirect) returns promptly. Unit/integration coverage complete for decision logic, spawn wiring, CLI parsing, and cleanup integration. Runtime/sandbox validation needed for end-to-end behavior with live sandboxes (expected for infrastructure paths).
  • PRA-T2 Runtime validation — Runtime validation: printf 'hello' | nemoclaw <sandbox> exec --stdin -- cat forwards stdin. Unit/integration coverage complete for decision logic, spawn wiring, CLI parsing, and cleanup integration. Runtime/sandbox validation needed for end-to-end behavior with live sandboxes (expected for infrastructure paths).
  • PRA-T3 Runtime validation — Runtime validation: nemoclaw <sandbox> exec --no-stdin -- pwd closes stdin explicitly. Unit/integration coverage complete for decision logic, spawn wiring, CLI parsing, and cleanup integration. Runtime/sandbox validation needed for end-to-end behavior with live sandboxes (expected for infrastructure paths).
  • PRA-T4 Runtime validation — Runtime validation: TTY exec (interactive) still inherits stdin for interactive commands. Unit/integration coverage complete for decision logic, spawn wiring, CLI parsing, and cleanup integration. Runtime/sandbox validation needed for end-to-end behavior with live sandboxes (expected for infrastructure paths).

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 7, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: sandbox-operations
Optional E2E: None

Dispatch hint: sandbox-operations

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • sandbox-operations (medium): Required because the PR changes the live sandbox exec path and child-process stdio behavior used for OpenShell sandbox commands. The sandbox-operations E2E is the closest existing live coverage for real sandbox operations, OpenShell command execution, PTY/process behavior, logs, status, recovery, and hosted-inference-backed assistant flows.

Optional E2E

  • None.

New E2E recommendations

  • sandbox exec stdin contract (high): Existing sandbox-operations coverage explicitly notes that piped stdin is not exercised. This PR changes the default stdin inheritance/closure contract and adds --stdin/--no-stdin, so a live test should verify printf ... | nemoclaw <sandbox> exec --stdin -- cat reaches the sandbox, default non-TTY stdin does not hang, and --no-stdin closes stdin while stdout/stderr remain inherited.
    • Suggested test: Add focused live coverage to test/e2e/live/sandbox-operations.test.ts or a dedicated wired e2e.yaml job for NemoClaw sandbox exec stdin behavior.

Dispatch hint

  • Workflow: e2e.yaml
  • jobs input: sandbox-operations

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: issue-4462-scope-upgrade-approval
Optional E2E targets: sessions-agents-cli

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=issue-4462-scope-upgrade-approval

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • issue-4462-scope-upgrade-approval: The PR changes the host-side sandbox exec command and spawn stdio behavior. This free-standing E2E job directly exercises real nemoclaw <name> exec -- openclaw ... transport through a live OpenClaw sandbox.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=issue-4462-scope-upgrade-approval

Optional E2E targets

  • sessions-agents-cli: Adjacent coverage for repeated live nemoclaw <name> exec -- openclaw ... agent/session operations after the stdin default changed.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=sessions-agents-cli

Relevant changed files

  • src/commands/sandbox/exec.ts
  • src/lib/actions/sandbox/exec-stdio.ts
  • src/lib/actions/sandbox/exec.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WSL2][CLI&UX] nemoclaw exec blocks indefinitely when stdin is a live (non-EOF) pipe — breaks pipelines/CI; stdin from /dev/null works

1 participant