Skip to content

fix(sandbox): target the owning gateway for status and exec queries - #7113

Merged
cv merged 10 commits into
mainfrom
fix/sandbox-gateway-target-resolution
Jul 18, 2026
Merged

fix(sandbox): target the owning gateway for status and exec queries#7113
cv merged 10 commits into
mainfrom
fix/sandbox-gateway-target-resolution

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

On a multi-instance host the active OpenShell gateway can be a sibling of the one a sandbox was onboarded against. nemohermes <sandbox> exec dispatched against whatever gateway was last selected, and a sandbox status query trusted a present result from that active gateway — so after inspecting one sandbox, a second sandbox was reported against the wrong gateway (e.g. Provisioning instead of Ready) and exec ran on the wrong gateway. Sandbox-scoped status and exec now resolve and select the sandbox's own gateway before talking to OpenShell.

Related Issue

Fixes #7105

Changes

  • Add selectSandboxOwningGateway in src/lib/actions/sandbox/gateway-select.ts: for a registry-known sandbox it runs openshell gateway select <owning-gateway>; for an unregistered sandbox it does nothing.
  • Add getKnownSandboxTargetGatewayName in src/lib/actions/sandbox/gateway-target.ts (returns null when the sandbox is not in the registry); getSandboxTargetGatewayName now builds on it.
  • In getReconciledSandboxGatewayState (src/lib/actions/sandbox/gateway-state.ts), stop trusting a present result that came from a different active gateway: when the sandbox's own gateway is registered but another gateway is active (connected_other), select the owning gateway and re-query — symmetric to the existing reconcile-on-missing path.
  • In execSandbox (src/lib/actions/sandbox/exec.ts), select the sandbox's owning gateway before the workdir probe and dispatch. Adds a selectGateway test seam (default: selectSandboxOwningGateway); its ordering is protected by exec-gateway-target.test.ts.
  • Cover the new behaviour in gateway-select.test.ts, gateway-state-owning-gateway.test.ts, and exec-gateway-target.test.ts.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • 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:
  • Tests not applicable — justification:
  • 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

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification:
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • New Features
    • Sandbox operations now pin and use the owning gateway (including sandbox-specific non-default instances) for workdir validation, execution, and related RPCs.
    • Policy denial hint probing/enabling and sandbox audit/log collection can now run gateway-scoped when available.
  • Bug Fixes
    • Rejects direct gateway endpoint overrides and fails fast on override or unsuccessful gateway selection.
    • Gateway-scoped state lookups prevent “present”/status from being reported from the wrong gateway, returning wrong_gateway_active when ownership can’t be selected.
  • Tests
    • Expanded Vitest coverage for gateway selection outcomes, exec ordering, override rejection, reconciliation/recovery, and gateway-scoped hint/log behavior.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Sandbox execution selects the target sandbox’s gateway before probing or running commands. Gateway state reconciliation scopes lookups and recovery to the owning gateway, with endpoint-override, selection-failure, recovery, and unregistered cases covered by tests.

Changes

Sandbox gateway routing

Layer / File(s) Summary
Gateway ownership resolution
src/lib/actions/sandbox/gateway-target.ts, src/lib/actions/sandbox/gateway-select.ts, src/lib/actions/sandbox/gateway-select.test.ts
Known sandbox mappings are resolved, gateway selection returns structured outcomes, and default, non-default, unregistered, and command-failure cases are tested.
Exec gateway dispatch and policy hints
src/lib/actions/sandbox/exec.ts, src/lib/actions/sandbox/exec-policy-hint-*.ts, src/lib/domain/sandbox/logs.ts, src/lib/actions/sandbox/exec-gateway-target.test.ts, src/lib/actions/sandbox/exec-policy-hint-runtime.test.ts
execSandbox selects the gateway before workdir probing and execution, passes gateway-scoped arguments through policy-hint logging, and aborts on endpoint overrides or selection failure.
Gateway-scoped state reconciliation
src/lib/actions/sandbox/gateway-state.ts, src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts, src/lib/actions/sandbox/gateway-state-drift.test.ts, src/lib/actions/sandbox/gateway-state-hints.test.ts
Sandbox and policy state lookups, reconciliation, and recovery use the owning gateway, with endpoint-override, failure, recovery, and unregistered cases covered.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant execSandbox
  participant GatewaySelector
  participant OpenShell
  Client->>execSandbox: execute sandbox command
  execSandbox->>GatewaySelector: select owning gateway
  GatewaySelector->>OpenShell: gateway select
  OpenShell-->>GatewaySelector: selection result
  execSandbox->>OpenShell: probe workdir and run command
  OpenShell-->>Client: command result
Loading

Possibly related PRs

Suggested labels: area: security

Suggested reviewers: cv, apurvvkumaria, ericksoa

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: targeting the owning gateway for sandbox status and exec queries.
Linked Issues check ✅ Passed The PR resolves sandbox status and exec calls against the registered owning gateway, matching the linked issue’s expected behavior.
Out of Scope Changes check ✅ Passed The gateway-scoped policy hint and log changes are consistent with the PR’s gateway-targeting objective and are not clearly unrelated.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sandbox-gateway-target-resolution

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

@github-code-quality

github-code-quality Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the fix/sandbox-gateway-... branch remains at 80%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 0339d88 fix/sandbox-gateway-... 3f1e648 +/-
src/lib/actions...ateway-state.ts 77% 65% -12%
src/lib/sandbox...vileged-exec.ts 87% 75% -12%
src/lib/actions...flow-helpers.ts 84% 83% -1%
src/lib/domain/sandbox/logs.ts 98% 98% 0%
src/lib/actions...sandbox/exec.ts 83% 85% +2%
src/lib/adapter...shell/client.ts 88% 90% +2%
src/lib/adapter...hell/runtime.ts 53% 58% +5%
src/lib/domain/.../connect-env.ts 89% 97% +8%
src/lib/actions...teway-target.ts 67% 100% +33%
src/lib/actions...teway-select.ts 0% 100% +100%

Updated July 18, 2026 09:27 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/actions/sandbox/exec-gateway-target.test.ts`:
- Around line 23-61: Update the test “selects the sandbox's owning gateway
before dispatching the exec” to provide a workdir and inject a workdir probe
that records its execution order. Assert the ordering is selectGateway("beta") →
probe → run, while preserving the existing gateway-selection and execution
assertions.

In `@src/lib/actions/sandbox/gateway-select.ts`:
- Around line 16-19: Gateway selection must fail closed instead of allowing
execution to continue on the previous gateway. Update gateway-select.ts#L16-L19
to return or propagate an explicit failed-selection outcome, update
exec.ts#L372-L372 to stop before probing or dispatching when selection fails,
and update gateway-state.ts#L450-L453 to preserve/report the original state
without marking recovery. Add failed/nonzero selection coverage in
src/lib/actions/sandbox/gateway-select.test.ts#L14-L25 and verify no
recoveredGateway is returned in
src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts#L16-L42.
🪄 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: 7c9be0b5-584a-4ac3-b5a8-e5292131b3ad

📥 Commits

Reviewing files that changed from the base of the PR and between b37e142 and 723b3ec.

📒 Files selected for processing (7)
  • src/lib/actions/sandbox/exec-gateway-target.test.ts
  • src/lib/actions/sandbox/exec.ts
  • src/lib/actions/sandbox/gateway-select.test.ts
  • src/lib/actions/sandbox/gateway-select.ts
  • src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts
  • src/lib/actions/sandbox/gateway-state.ts
  • src/lib/actions/sandbox/gateway-target.ts

Comment thread src/lib/actions/sandbox/exec-gateway-target.test.ts
Comment thread src/lib/actions/sandbox/gateway-select.ts Outdated
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / medium confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 1 blocker · 0 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported 1 more blocker, the same number of warnings, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: inference-routing, network-policy, onboard-repair, onboard-resume

2 optional E2E recommendations
  • sandbox-operations
  • gateway-guard-recovery

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@wscurran wscurran added area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression labels Jul 17, 2026
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/exec-gateway-target.test.ts`:
- Line 104: Update the tests around the execSandbox calls in the successful
dispatch and selection-failure cases to assert the returned observable exit
values, __exit_0__ and __exit_1__ respectively, instead of attaching catch
handlers that swallow all failures. Preserve the existing public-boundary
assertions and ensure unrelated errors cause the tests to fail.
🪄 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: 42965520-0881-4a84-8126-b0c0a41206c3

📥 Commits

Reviewing files that changed from the base of the PR and between 723b3ec and a0091be.

📒 Files selected for processing (6)
  • src/lib/actions/sandbox/exec-gateway-target.test.ts
  • src/lib/actions/sandbox/exec.ts
  • src/lib/actions/sandbox/gateway-select.test.ts
  • src/lib/actions/sandbox/gateway-select.ts
  • src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts
  • src/lib/actions/sandbox/gateway-state.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts
  • src/lib/actions/sandbox/gateway-state.ts
  • src/lib/actions/sandbox/exec.ts

Comment thread src/lib/actions/sandbox/exec-gateway-target.test.ts Outdated

@apurvvkumaria apurvvkumaria left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes on exact head d108d4f7b0fcee10e098c337e7eb74d27a7168a5. The security review found one command-routing blocker and one status-correctness blocker:

  1. P1 — bind the actual OpenShell subprocess to the owning gateway. gateway select mutates shared OpenShell selection state, but the later workdir probe and exec are not bound to that gateway. Another CLI can select a sibling between those operations. In addition, inherited OPENSHELL_GATEWAY can redirect the subprocess and OPENSHELL_GATEWAY_ENDPOINT can bypass the recorded gateway entirely. With same-named sandboxes, this can dispatch the user command to the sibling. Please scope every probe/exec invocation to the recorded owner (for example, the supported sandbox exec -g <gateway> path or an equivalent subprocess-local pin), reject endpoint overrides, and cover ambient overrides plus selection drift.

  2. P2 — target the owner before the first status lookup. getReconciledSandboxGatewayState first queries the ambient gateway and only reselects after particular present/missing results. A sibling timeout or unknown error returns without ever querying the healthy owner. Please select/pin the known owner before the initial lookup and add timeout/error-from-sibling coverage.

The current tests prove same-process call order but do not exercise either routing boundary. The remaining security-review categories were clean: no secrets, dependency, crypto, logging, or configuration regressions were introduced.

Bind status, policy, workdir probes, recovery, and exec subprocesses to the sandbox's recorded OpenShell gateway. Reject endpoint overrides and fail closed when owner selection cannot be established.

Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria
apurvvkumaria dismissed their stale review July 17, 2026 23:09

Addressed on exact head 1ace377: all exec/workdir/status/policy/recovery subprocesses are pinned to the recorded owner, endpoint overrides fail closed, and sibling drift/timeout/unknown plus selection-failure regressions are covered. Independent security re-review passed; 202 focused tests, CLI build/typecheck, Biome, and repository shape/title/project/size gates passed. The signed DCO commit is GitHub Verified and credits the original author.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/gateway-state.ts`:
- Around line 479-499: Update the sandbox gateway lookup flow around
getKnownSandboxTargetGatewayName, selectSandboxOwningGateway, and getState to
use a single ownership snapshot: derive the authoritative gateway once, pass
that same gateway to selection and status lookup, or replace the lookup target
with the selected gateway result. Ensure differing ownership snapshots cannot
cause selection and lookup to target different gateways, and add a regression
test covering that scenario.
🪄 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: 4237173c-30eb-4dc7-80bb-fd5408d654fb

📥 Commits

Reviewing files that changed from the base of the PR and between d108d4f and 1ace377.

📒 Files selected for processing (5)
  • src/lib/actions/sandbox/exec-gateway-target.test.ts
  • src/lib/actions/sandbox/exec.ts
  • src/lib/actions/sandbox/gateway-state-drift.test.ts
  • src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts
  • src/lib/actions/sandbox/gateway-state.ts

Comment thread src/lib/actions/sandbox/gateway-state.ts Outdated
apurvvkumaria and others added 6 commits July 17, 2026 16:17
Keep post-exec audit enablement and policy log reads bound to the same recorded gateway as the user command, even when ambient OpenShell selection drifts.

Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Use the gateway returned by owner selection as the authoritative target for status lookup, reconciliation, and recovery so concurrent registry updates cannot split routing across gateways.

Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved on the refreshed exact head after the deterministic gate passed: 52 current checks green, mergeable/current base, no unresolved major/critical findings, DCO present, and all 10 commits verified. The nine-category security review is clean: every status/workdir/exec/policy/audit/log path pins the selected owning gateway, rejects endpoint overrides, and fails closed on selection or ownership ambiguity. Focused CLI and affected integration suites passed 45 and 55 tests.

@cv
cv merged commit 770a755 into main Jul 18, 2026
53 checks passed
@cv
cv deleted the fix/sandbox-gateway-target-resolution branch July 18, 2026 15:56
@ericksoa ericksoa mentioned this pull request Jul 18, 2026
21 tasks
ericksoa added a commit that referenced this pull request Jul 18, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical `docs/changelog/2026-07-18.mdx` release-prep entry
with the exact `## v0.0.88` heading.
The entry summarizes every user-visible change on `main` since v0.0.87
and links each release theme to the focused user documentation.

## Changes

- Add one parser-safe dated changelog entry for v0.0.88 covering DGX
Station preparation, inference health, multi-gateway sandbox operations
and recovery, onboarding policy defaults, and rebuild credential reuse.
- Reconcile the changelog against the merged v0.0.88-labeled PRs and the
complete `v0.0.87..origin/main` commit range.
- Source mapping:
- [#7152](#7152) ->
`docs/changelog/2026-07-18.mdx`: Document RDMA-aware OpenIB service
remediation during DGX Station preparation.
- [#7155](#7155) ->
`docs/changelog/2026-07-18.mdx`: Document stopped-container preservation
and fail-closed restart-policy boundaries.
- [#7158](#7158) ->
`docs/changelog/2026-07-18.mdx`: Document bounded packaged CDI refresh
for the exact AI Developer Tools Station profile.
- [#7074](#7074) ->
`docs/changelog/2026-07-18.mdx`: Document authenticated upstream model
probes and precise route-reachability claims.
- [#7007](#7007) ->
`docs/changelog/2026-07-18.mdx`: Document the explicit serving-process
health gap in `status` and `doctor`.
- [#7113](#7113) ->
`docs/changelog/2026-07-18.mdx`: Document owning-gateway selection for
sandbox-scoped status and exec operations.
- [#7092](#7092) ->
`docs/changelog/2026-07-18.mdx`: Document idempotent recovery for
target-owned active port forwards.
- [#7133](#7133) ->
`docs/changelog/2026-07-18.mdx`: Document web-search-aware policy preset
defaults during onboarding.
- [#7129](#7129) ->
`docs/changelog/2026-07-18.mdx`: Document gateway-registered web-search
credential reuse during rebuild preflight.

## 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
<!-- Check one tests line and one docs line. Check other lines when
applicable. Add every requested justification or approval reference. -->
- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates the dated changelog contract,
exact release heading, and parser-safe MDX structure.
- [ ] 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:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [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 check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `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:
- [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) —
completed successfully 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)
— not applicable because native changelog entries use the required
parser-safe MDX SPDX comment without frontmatter.

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
  * Added improved DGX Station preparation workflows.
* Enhanced sandbox status and diagnostic reporting for inference health.
  * Improved state selection and recovery across multiple gateways.
  * Added safer onboarding defaults for web search policies.
* Improved rebuild preflight handling for credential reuse and
fail-closed behavior.

* **Documentation**
  * Added release notes for version 0.0.88.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa added a commit that referenced this pull request Jul 20, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Fixes the `status --json` regression introduced by #7113, where
OpenShell's ANSI gateway-selection confirmation bypassed the CLI's
stdout guard and made Deep Agents target discovery fail to parse the
response. Human-readable status output remains unchanged while JSON
stdout is again machine-parseable.

## Changes
<!-- Bullet list of key changes. -->

- Capture `openshell gateway select` stdout instead of letting the child
process write directly to file descriptor 1.
- Replay captured output through an injected stdio adapter that writes
to stderr, keeping the human confirmation visible without contaminating
structured stdout or coupling the action to process streams.
- Add unit coverage for the subprocess stdio contract and integration
coverage for the exact ANSI gateway-selection message.
- Remove the temporary #7233 consumer-side banner sanitizer so the Deep
Agents fresh re-onboard check again parses the raw JSON document and
detects any producer regression.

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: this restores the existing
documented structured `status --json` contract; stdout routing is an
internal implementation detail.
- [x] 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: pending independent
review; no waiver claimed.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — gateway selection unit tests (6
passed); sandbox status JSON integration tests (18 passed); `npm run
typecheck:cli`, `bash -n`, and `shellcheck` passed.
- [x] Exact-head GitHub CI and protected E2E evidence passed — [ordinary
CI](https://github.com/NVIDIA/NemoClaw/actions/runs/29748711264),
[controller](https://github.com/NVIDIA/NemoClaw/actions/runs/29749472530),
and [child
suite](https://github.com/NVIDIA/NemoClaw/actions/runs/29749508328).
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this narrow
stdout-routing regression; focused unit/integration tests, CLI
typecheck, and normal hooks passed.
- [ ] Quality Gates section completed with required justifications or
waivers — pending independent sensitive-path review.
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Gateway selection output is now displayed correctly without
contaminating structured JSON status responses.
* `sandbox status --json` now consistently returns machine-readable
output, even when gateway status messages are generated.

* **Tests**
* Expanded coverage for gateway selection output handling and command
execution behavior.
  * Updated end-to-end validation to parse status JSON reliably.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DGX Station][Sandbox] nemohermes queries the wrong sandbox after the active OpenShell gateway changes

6 participants