Skip to content

fix(cli): fail sandbox exec when the relay closes without an exit status sandbox exec seeded its exit code to 0 and only overwrote it on an Exit event, so a stream that ended early, was cancelled, or was truncated reported a successful run. The gateway already treats the same condition as a relay failure (Status::unavailable); mirror that on the CLI side so exit 0 always means an observed exit status of 0. Fixes #2732 - #2957

Open
rootkiller6788 wants to merge 1 commit into
NVIDIA:mainfrom
rootkiller6788:fix/sandbox-exec-missing-exit-status

Conversation

@rootkiller6788

Copy link
Copy Markdown

Summary

sandbox exec seeded its exit code to 0 and only ever overwrote it when an
Exit event arrived, so an event stream that closed early, was cancelled, or
was truncated reported a successful (exit 0) run even though the command's
outcome was never observed. This makes "the command ran and returned 0" and
"we never saw what the command did" indistinguishable to callers.

Related Issue

Closes #2732

Changes

  • Track whether an Exit event was observed in both exec paths
    (sandbox_exec_grpc and sandbox_exec_interactive_grpc).
  • When the stream ends without an Exit event, return a diagnostic error
    instead of Ok(0), mirroring the gateway's existing exec_loop_result
    handling (Status::unavailable: "exec relay closed before the command
    reported an exit status").
  • The interactive path reports the error only after the terminal is restored.

Testing

  • cargo check -p openshell-cli
  • Existing CLI unit tests (cargo test -p openshell-cli)

Checklist

  • Change is scoped to the issue at hand
  • Conventional commit + DCO sign-off
  • Verified against the real code path on main

`sandbox exec` seeded its exit code to 0 and only overwrote it on an
`Exit` event, so a stream that ended early, was cancelled, or was truncated
reported a successful run. The gateway already treats the same condition as
a relay failure (`Status::unavailable`); mirror that on the CLI side so exit
0 always means an observed exit status of 0.

Fixes NVIDIA#2732

Signed-off-by: rootkiller6788 <rootkiller6788@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 26, 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.

@johntmyers

Copy link
Copy Markdown
Collaborator

@rootkiller6788 please trim the title down

@johntmyers johntmyers added the gator:in-review Gator is reviewing or awaiting PR review feedback label Aug 26, 2026
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

The initial code review found no blocking findings: both sandbox-exec paths now distinguish a missing Exit event from an observed exit code of zero, and the interactive path restores the terminal before returning the relay error. Thanks @johntmyers — I also checked the title problem you flagged, and the PR title still includes the commit-body paragraphs instead of ending after the concise subject line.

Action required: @rootkiller6788, please trim the PR title as @johntmyers requested. The authorized E2E dispatch can proceed for this head while that metadata fix is pending.

Blocking findings:

  • No blocking code findings remain.

Carried findings:

  • None.

Non-blocking suggestions:

  • None.
Gator metadata
  • Validation: Project-valid because this focused CLI correctness fix implements accepted issue bug: sandbox exec exits 0 when the event stream ends without an exit status #2732.
  • Docs: No Fern update needed because this restores the existing exit-status contract for an abnormal relay termination and adds no command, flag, configuration, or supported workflow.
  • Checks: Required current-head checks have not started successfully yet; test dispatch follows this disposition.
  • E2E: test:e2e required for the sandbox exec relay path and authorized by the operator.
  • Head SHA: 8b645ee518028a40e6388e0936ca52e958e5f179
  • Base SHA: c3993426498d5b602ec080bc1d3cbad5e8460f4a
  • Merge base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
  • Patch ID: 6b0668b85b7fd3b62b0059d7b3fca4806b24be5d
  • Gator payload: 7
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:in-review

@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Aug 26, 2026
@johntmyers

Copy link
Copy Markdown
Collaborator

/ok to test 8b645ee

@github-actions

Copy link
Copy Markdown

Label test:e2e applied for 8b645ee. Open Branch E2E Checks, find the run for commit 8b645ee, and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers johntmyers added gator:blocked Gator is blocked by process or repository gates gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates labels Aug 26, 2026
@johntmyers johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: sandbox exec exits 0 when the event stream ends without an exit status

2 participants