fix(sandbox): terminate sandbox when proxy accept loop exits unexpectedly by politerealism · Pull Request #2370 · NVIDIA/OpenShell

politerealism · 2026-07-20T16:10:24Z

Summary

Add a oneshot notification channel to ProxyHandle that fires when the proxy task exits for any reason (panic, abort, or loop break)
The sandbox main loop now races this signal alongside the entrypoint process and shutdown signals, terminating with a clear OCSF AppLifecycle error at High severity if the proxy dies
Prevents the sandbox from continuing to report Ready with a dead proxy

Related Issue

Note: This is step 2 of the fix for #2337, building on PR #2369 (step 1: accept loop retry with backoff). Step 1 prevents the proxy from dying on transient errors; step 2 catches any unexpected death as defense-in-depth.

Changes

crates/openshell-supervisor-network/src/proxy.rs

Added exited_rx oneshot receiver field to ProxyHandle
Created exited_tx sender inside the spawned proxy task as _proxy_exit_guard — dropped on task exit for any reason
Added take_exit_receiver() method to extract the receiver for monitoring

crates/openshell-sandbox/src/lib.rs

Extract proxy exit receiver before the main wait section
Wrap it in a boxed future (std::future::pending() when no proxy exists)
Added proxy-death arm to all 5 tokio::select! wait paths:
- Process-enabled + sidecar control
- Process-enabled, no sidecar
- Network-only + sidecar control (Linux)
- Network-only, no sidecar (Linux)
- Non-Linux network-only

Testing

cargo check -p openshell-supervisor-network — compiles clean, no warnings
cargo check -p openshell-sandbox — compiles clean
cargo test -p openshell-supervisor-network -p openshell-sandbox — all tests pass

Checklist

Conventional commit format
Signed-off for DCO compliance
No secrets or credentials included
Scoped to the issue at hand
Follows existing sidecar control channel pattern (lib.rs:692-710)

Note: This PR depends on PR #2369 (step 1: accept loop retry with backoff) and should be merged after it.

copy-pr-bot · 2026-07-20T16:10:28Z

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.

politerealism · 2026-07-20T16:37:12Z

Follow-up filed: #2372 — the SSH server task is spawned fire-and-forget (run.rs:238), so if it exits unexpectedly the sandbox has no way to detect it. The same exit-notification pattern from this PR (oneshot drop-guard + tokio::select! arm) should be applied to the SSH server task to enforce the "Ready implies exec relay operational" invariant.

politerealism · 2026-07-20T16:38:22Z

Rebase note: This PR was branched off #2369. Once #2369 merges, this branch needs a rebase to pick up the final version of the proxy backoff code (libc constants, extracted helpers, fixed 5s cap). No code changes required — just a rebase.

johntmyers

gator-agent

PR Review Status

Validation: This is a concentrated sandbox/network-supervisor correctness fix for the reproducible stale-Ready failure in #2337, and no duplicate implementation was found.
Head SHA: 4e915eddb6d4c5111e95d44fff5c1b3e539f20b6

Thanks @politerealism, I checked your rebase note against PR #2369. Its current head differs from the retry commit stacked here; rebasing after #2369 lands should pick up the portable errno helper and corrected 5-second cap covered by the first two inline findings.

Review findings:

Four actionable warnings are attached inline.
The fail-closed proxy-exit signal is wired into every sandbox wait path and does not weaken the sandbox security boundary.

Docs: Fern docs are not needed because this restores the documented readiness invariant without changing commands, configuration, APIs, or an intentional user workflow.

Next state: gator:in-review

Please rebase after #2369 lands and address the exit-receiver contract and production-path test coverage. E2E will be required after review feedback is resolved.

johntmyers · 2026-07-21T15:43:44Z

gator-agent

Blocked

Head SHA: 6a1fe36ae0357d7bc0d22b91323ff8ae296b54a2

Gator is blocked because GitHub reports this PR as having merge conflicts with main (mergeStateStatus: DIRTY).

Next action: @politerealism, please rebase the branch onto the current main, resolve the conflicts, and push the updated head. Gator will review the new head and decide the required E2E label and /ok to test action after the conflict is resolved.

politerealism · 2026-07-21T18:48:32Z

Waiting to test and recheck this one until 2369 is approved and merges so I can rebase then go through checks.

johntmyers

gator-agent

PR Review Status

Validation: This remains a project-valid, concentrated fix for the stale-Ready proxy failure in #2337.
Head SHA: 00dc53e89455857e75b8dcceb2f91cd788ad228b
Base SHA: 0a3ec7a1126aad908ffb742435db25bea1c19c25
Merge base SHA: 0a3ec7a1126aad908ffb742435db25bea1c19c25
Patch ID: 287b2741b08a8605dfb9f7325b7d7e284227be8a
Gator payload: 2
Review mode: follow_up
Previous reviewed SHA: 4e915eddb6d4c5111e95d44fff5c1b3e539f20b6

Thanks @politerealism, I checked your rebase note now that #2369 has merged. The merge conflict is resolved, and the 5-second backoff cap finding (gator-inline-3618433659) is fixed. The latest author delta also replaces parts of #2369's finalized classifier, which introduces the new inline blocker below.

Blocking findings:

GATOR-00dc53e8-01: documented transient accept errors now consume the unknown-error budget and terminate the proxy on the tenth occurrence.

Carried findings:

gator-inline-3618433657: still open; numeric errno literals remain instead of target-aware libc constants.
gator-inline-3618433663: still open; a second take_exit_receiver() call still manufactures an immediate false exit signal.
gator-inline-3618433665: still open; tests still bypass the production proxy task and sandbox lifecycle selection.

Non-blocking suggestions: None.

Docs: Fern docs are not needed because this restores an existing readiness invariant without changing commands, configuration, APIs, or intentional user workflows.

Next state: gator:in-review

Please preserve #2369's portable transient-error classifier while layering the proxy-exit notification on its terminal outcome, and address the carried obligations above. E2E remains required after review feedback is resolved.

johntmyers

gator-agent

PR Review Status

Validation: This remains a project-valid, concentrated fix for the stale-Ready proxy failure in #2337.
Head SHA: fe8298791181db1dd5f606e4914de4f9bd406e85
Base SHA: 0a3ec7a1126aad908ffb742435db25bea1c19c25
Merge base SHA: 0a3ec7a1126aad908ffb742435db25bea1c19c25
Patch ID: 8ebd81958c1bf8492797c1af6c62754d1ebd732d
Gator payload: 2
Review mode: follow_up
Previous reviewed SHA: 00dc53e89455857e75b8dcceb2f91cd788ad228b

Thanks @politerealism, I checked your latest commit restoring the complete transient errno classifier. GATOR-00dc53e8-01 is resolved: the supported transient family now bypasses the unknown-error budget, resource-pressure errors retain capped backoff, and the added coverage exercises repeated transient classification. The Option-based receiver change also resolves the double-take contract issue.

Blocking findings:

No blocking findings remain.

Carried findings:

None.

Docs: Fern docs are not needed because this restores an existing readiness invariant without changing commands, configuration, APIs, or intentional user workflows.

Next state: gator:watch-pipeline

This sandbox lifecycle and network-proxy change requires test:e2e; gator will enable that gate and monitor the required checks.

github-actions · 2026-08-03T18:30:33Z

Label test:e2e applied, but pull-request/2370 does not exist yet. A maintainer needs to comment /ok to test fe8298791181db1dd5f606e4914de4f9bd406e85 to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

johntmyers · 2026-08-03T18:30:45Z

/ok to test fe82987

github-actions · 2026-08-03T18:31:31Z

Label test:e2e applied for fe82987. Open the existing run 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.

github-actions · 2026-08-11T12:50:34Z

Label test:e2e applied for 6262068. Open Branch E2E Checks, find the run for commit 6262068, 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 · 2026-08-11T17:22:04Z

/ok to test dc882ca

johntmyers · 2026-08-11T17:54:55Z

gator-agent

Merge Ready

Head SHA: dc882cab65f8a96d5afbd806390ca74c8ccaa632
Gator payload: 3

Gator validation and PR monitoring are complete, and maintainer approval is present.

Validation: This is a project-valid, concentrated sandbox/network-proxy lifecycle correctness fix for #2337.
Review: All durable gator findings are resolved; the current head only adds an empty CI-trigger commit and retains the checkpointed effective patch.
Approval: Maintainer approval is present.
Docs: Fern docs are not needed because this restores an existing readiness invariant without changing commands, configuration, APIs, or intentional user workflows.
Checks: OpenShell Branch Checks and Helm Lint are green.
E2E: test:e2e is applied and OpenShell E2E is green.

Human maintainer merge or close decision is now required.

…edly Add a oneshot notification channel to ProxyHandle that fires when the proxy task exits for any reason (panic, abort, or loop break). The sandbox main loop now races this signal alongside the entrypoint process and shutdown signals, terminating with a clear OCSF error if the proxy dies. This prevents the sandbox from continuing to report Ready with a dead proxy. Follows the existing sidecar control channel pattern. All five wait paths (process+sidecar, process-only, network+sidecar, network-only on Linux, and non-Linux) now monitor proxy liveness. Refs NVIDIA#2337 Signed-off-by: politerealism <burdcat17@gmail.com>

…it_receiver Verify the oneshot drop-guard pattern fires on normal task exit and abort, and document the take_exit_receiver contract including the double-call hazard. Signed-off-by: politerealism <burdcat17@gmail.com>

…unrecoverable failures Extract accept-loop error handling into classify_accept_error() with AcceptAction enum so the decision logic is independently testable. Terminal errors (EBADF/EINVAL/ENOTSOCK) now exit the loop immediately, allowing the supervisor to detect proxy death. Unknown errors exit after 10 consecutive failures. FD exhaustion retries with exponential backoff capped at 5s (fix unreachable cap from min(6) to min(7)). Adds 9 unit tests covering classification, counter interactions, backoff progression, and bounded exit behavior. Signed-off-by: politerealism <burdcat17@gmail.com>

Replace raw errno numbers with libc:: constants behind #[cfg(unix)] gates in classify_accept_error and its tests. Refactor ProxyHandle::exited_rx to Option<Receiver<()>> so take_exit_receiver uses .take() instead of allocating a dummy sender. Add a ProxyHandle-level test that verifies the full drop → abort → receiver fires path. Signed-off-by: politerealism <burdcat17@gmail.com>

The refactored classifier only recognized EMFILE/ENFILE as retryable, routing all other documented transient accept errors (ENOBUFS, ENOMEM, ECONNABORTED, ECONNRESET, EINTR, ENETDOWN, etc.) through the unknown- error budget which terminates the proxy after 10 consecutive failures. Restore the complete transient errno table from main so these errors retry indefinitely with appropriate backoff. Signed-off-by: politerealism <burdcat17@gmail.com>

Replace redundant closure |p| p.take_exit_receiver() with the method reference ProxyHandle::take_exit_receiver, and change all five select! arms from _ = &mut proxy_exited to () = &mut proxy_exited to satisfy clippy::ignored_unit_patterns. Also run cargo fmt on the new proxy test in proxy.rs. Signed-off-by: politerealism <burdcat17@gmail.com>

Signed-off-by: politerealism <burdcat17@gmail.com>

politerealism requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners July 20, 2026 16:10

This was referenced Jul 20, 2026

fix(proxy): retry with backoff on transient accept errors instead of exiting #2369

Merged

bug: SSH server accept loop exits permanently on transient errors (same class as #2337) #2372

Open

johntmyers self-assigned this Jul 21, 2026

johntmyers reviewed Jul 21, 2026

View reviewed changes

Comment thread crates/openshell-supervisor-network/src/proxy.rs Outdated

Comment thread crates/openshell-supervisor-network/src/proxy.rs Outdated

Comment thread crates/openshell-supervisor-network/src/proxy.rs Outdated

Comment thread crates/openshell-supervisor-network/src/proxy.rs

johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Jul 21, 2026

politerealism force-pushed the fix/proxy-exit-detection branch from 6a1fe36 to 00dc53e Compare August 3, 2026 17:50

johntmyers reviewed Aug 3, 2026

View reviewed changes

Comment thread crates/openshell-supervisor-network/src/proxy.rs

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

politerealism requested a review from johntmyers August 3, 2026 18:08

johntmyers reviewed Aug 3, 2026

View reviewed changes

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

johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status test:e2e Requires end-to-end coverage and removed gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage labels Aug 3, 2026

johntmyers added gator:blocked Gator is blocked by process or repository gates gator:watch-pipeline Gator is monitoring PR CI/CD status and removed gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates labels Aug 11, 2026

politerealism mentioned this pull request Aug 11, 2026

fix(ssh): add EMFILE backoff and exit notification to SSH accept loop #2705

Open

7 tasks

politerealism added 8 commits August 19, 2026 11:55

ci: trigger required check re-run

611aebb

Signed-off-by: politerealism <burdcat17@gmail.com>

fix(sandbox): remove unnecessary std::pin qualification

df06637

Signed-off-by: politerealism <burdcat17@gmail.com>

Uh oh!

Conversation

politerealism commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Jul 20, 2026

Uh oh!

politerealism commented Jul 20, 2026

Uh oh!

politerealism commented Jul 20, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johntmyers commented Jul 21, 2026

Blocked

Uh oh!

politerealism commented Jul 21, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

johntmyers commented Aug 3, 2026

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

github-actions Bot commented Aug 11, 2026

Uh oh!

johntmyers commented Aug 11, 2026

Uh oh!

johntmyers commented Aug 11, 2026

Merge Ready

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

politerealism commented Jul 20, 2026 •

edited

Loading