Skip to content

Commit b7d9553

Browse files
committed
fix(sandbox): finalize terminal delivery on disconnect
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent f4e2628 commit b7d9553

35 files changed

Lines changed: 1246 additions & 660 deletions

File tree

.agents/skills/openshell-cli/SKILL.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,9 @@ Key flags:
260260
- `--editor vscode|cursor`: Open a remote editor after creation and keep the sandbox alive
261261

262262
`--detach` adds no attachment grace period. When the canonical process exits,
263-
its terminal phase is reported immediately. A foreground create instead keeps
264-
ephemeral cleanup deferred until its active SSH connection closes naturally.
263+
its terminal phase is reported immediately. A foreground create declares one
264+
expected main-process SSH attachment; cleanup finalizes after that connection
265+
closes naturally.
265266

266267
Do not combine `--upload` with a trailing main command. Uploads currently finish
267268
after the canonical process starts; create a scratch sandbox and use

.agents/skills/openshell-cli/cli-reference.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,9 @@ previous runtime generation.
226226
| `--memory <QUANTITY>` | Memory limit (for example: `512Mi`, `4Gi`, `8G`) |
227227

228228
`--detach` adds no attachment grace period: the sandbox reports the canonical
229-
process result immediately when it exits. Foreground creation defers ephemeral
230-
cleanup only while its active SSH connection drains and closes naturally.
229+
process result immediately when it exits. Foreground creation declares one
230+
expected main-process SSH attachment; cleanup finalizes after that connection
231+
drains and closes naturally.
231232
| `--driver-config-json <JSON>` | Experimental driver-keyed configuration object |
232233
| `--provider <NAME>` | Provider to attach (repeatable) |
233234
| `--policy <PATH>` | Custom policy YAML; overrides the built-in default and `OPENSHELL_SANDBOX_POLICY` |

architecture/gateway.md

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,15 @@ identity.
2828
The live supervisor session is the readiness authority for its main-process
2929
instance. The supervisor reports its normalized result through the
3030
sandbox-authenticated `ReportMainProcessExit` RPC, and the gateway rejects
31-
results from stale instance IDs. The process supervisor keeps the main SSH
32-
session alive only when a foreground attachment is active at process exit. It
33-
durably reports the result immediately, sends the SSH exit status, and waits for
34-
the peer's SSH channel close before releasing deferred ephemeral cleanup.
35-
Detached commands have no attachment to drain, so they report their result and
36-
exit immediately without a grace period.
31+
results from stale instance IDs. Foreground creation carries a one-shot
32+
attachment intent to the process supervisor. The supervisor durably reports the
33+
result immediately, accepts that declared SSH attachment even when the process
34+
has already exited, sends the retained output and exit status, and waits for the
35+
peer's channel close before finalizing the result for ephemeral cleanup.
36+
Detached commands carry no attachment intent, so they finalize and exit
37+
immediately without a grace period. Finalization is persisted separately from
38+
the exit result; the gateway deletes an ephemeral sandbox only after the
39+
finalized supervisor session disconnects.
3740

3841
## Protocol and Auth
3942

architecture/sandbox.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -482,10 +482,13 @@ engine with a gateway policy revision.
482482
- If the supervisor relay drops, the sandbox can keep running, but connect and
483483
exec operations fail until the supervisor registers again.
484484
- If the canonical main process exits, the supervisor durably reports the
485-
normalized result immediately. When a foreground main attachment is active,
486-
it sends the retained output and SSH exit status, waits for the peer's channel
487-
close, and then releases deferred ephemeral cleanup. With no active
488-
attachment, it exits without a grace period. Exit code 0 records
485+
normalized result immediately. A foreground create declares a one-shot main
486+
attachment, so the supervisor accepts it even after a fast process exits,
487+
sends the retained output and SSH exit status, waits for the peer's channel
488+
close, and then finalizes ephemeral cleanup. With no declared or active
489+
attachment, it finalizes and exits without a grace period. The gateway waits
490+
for that finalized supervisor session to disconnect before deleting an
491+
ephemeral sandbox. Exit code 0 records
489492
`Completed/MainProcessCompleted`; nonzero and signal-normalized exits record
490493
`Error/MainProcessFailed`. Infrastructure failures also use `Error`, with a
491494
distinct condition reason and no fabricated canonical-process result. Runtime

crates/openshell-cli/src/run.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,11 @@ pub async fn sandbox_create(
563563
command.to_vec()
564564
};
565565
let persist = sandbox_should_persist(keep, forward.as_ref());
566+
let create_detaches = detach
567+
|| (persist
568+
&& command.is_empty()
569+
&& (!std::io::stdin().is_terminal() || !std::io::stdout().is_terminal()));
570+
let await_main_process_attachment = output == "table" && editor.is_none() && !create_detaches;
566571
let annotations = if persist {
567572
HashMap::new()
568573
} else {
@@ -586,6 +591,7 @@ pub async fn sandbox_create(
586591
labels,
587592
annotations,
588593
workspace: workspace.to_string(),
594+
await_main_process_attachment,
589595
};
590596

591597
let response = match client.create_sandbox(request).await {

crates/openshell-cli/tests/ensure_providers_integration.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,13 @@ impl OpenShell for TestOpenShell {
8888
Err(Status::unimplemented("not used by this test server"))
8989
}
9090

91+
async fn finalize_main_process_exit(
92+
&self,
93+
_request: tonic::Request<openshell_core::proto::FinalizeMainProcessExitRequest>,
94+
) -> Result<Response<openshell_core::proto::FinalizeMainProcessExitResponse>, Status> {
95+
Err(Status::unimplemented("not used by this test server"))
96+
}
97+
9198
async fn get_current_user(
9299
&self,
93100
_request: tonic::Request<openshell_core::proto::GetCurrentUserRequest>,

crates/openshell-cli/tests/mtls_integration.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ impl OpenShell for TestOpenShell {
4141
Err(Status::unimplemented("not used by this test server"))
4242
}
4343

44+
async fn finalize_main_process_exit(
45+
&self,
46+
_request: tonic::Request<openshell_core::proto::FinalizeMainProcessExitRequest>,
47+
) -> Result<Response<openshell_core::proto::FinalizeMainProcessExitResponse>, Status> {
48+
Err(Status::unimplemented("not used by this test server"))
49+
}
50+
4451
async fn get_current_user(
4552
&self,
4653
_request: tonic::Request<openshell_core::proto::GetCurrentUserRequest>,

crates/openshell-cli/tests/provider_commands_integration.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,13 @@ impl OpenShell for TestOpenShell {
106106
Err(Status::unimplemented("not used by this test server"))
107107
}
108108

109+
async fn finalize_main_process_exit(
110+
&self,
111+
_request: tonic::Request<openshell_core::proto::FinalizeMainProcessExitRequest>,
112+
) -> Result<Response<openshell_core::proto::FinalizeMainProcessExitResponse>, Status> {
113+
Err(Status::unimplemented("not used by this test server"))
114+
}
115+
109116
async fn get_current_user(
110117
&self,
111118
_request: tonic::Request<openshell_core::proto::GetCurrentUserRequest>,

crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ impl OpenShell for TestOpenShell {
6969
Err(Status::unimplemented("not used by this test server"))
7070
}
7171

72+
async fn finalize_main_process_exit(
73+
&self,
74+
_request: tonic::Request<openshell_core::proto::FinalizeMainProcessExitRequest>,
75+
) -> Result<Response<openshell_core::proto::FinalizeMainProcessExitResponse>, Status> {
76+
Err(Status::unimplemented("not used by this test server"))
77+
}
78+
7279
async fn get_current_user(
7380
&self,
7481
_request: tonic::Request<openshell_core::proto::GetCurrentUserRequest>,
@@ -1349,6 +1356,35 @@ async fn sandbox_create_persists_exact_trailing_argv_as_main_process() {
13491356
.expect("sandbox spec should be persisted at create time");
13501357
assert_eq!(spec.command, command);
13511358
assert!(!spec.tty);
1359+
assert!(requests[0].await_main_process_attachment);
1360+
}
1361+
1362+
#[tokio::test]
1363+
async fn detached_command_does_not_declare_main_process_attachment() {
1364+
let server = run_server().await;
1365+
let fake_ssh_dir = tempfile::tempdir().unwrap();
1366+
let xdg_dir = tempfile::tempdir().unwrap();
1367+
let _env = test_env(&fake_ssh_dir, &xdg_dir);
1368+
let tls = test_tls(&server);
1369+
install_fake_ssh(&fake_ssh_dir);
1370+
1371+
run::sandbox_create(
1372+
&server.endpoint,
1373+
"openshell",
1374+
run::SandboxCreateConfig {
1375+
name: Some("detached-main"),
1376+
command: &["echo".into(), "OK".into()],
1377+
detach: true,
1378+
..test_config()
1379+
},
1380+
"default",
1381+
&tls,
1382+
)
1383+
.await
1384+
.expect("detached sandbox create should succeed");
1385+
1386+
let requests = create_requests(&server).await;
1387+
assert!(!requests[0].await_main_process_attachment);
13521388
}
13531389

13541390
#[tokio::test]

crates/openshell-cli/tests/sandbox_name_fallback_integration.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ impl OpenShell for TestOpenShell {
5656
Err(Status::unimplemented("not used by this test server"))
5757
}
5858

59+
async fn finalize_main_process_exit(
60+
&self,
61+
_request: tonic::Request<openshell_core::proto::FinalizeMainProcessExitRequest>,
62+
) -> Result<Response<openshell_core::proto::FinalizeMainProcessExitResponse>, Status> {
63+
Err(Status::unimplemented("not used by this test server"))
64+
}
65+
5966
async fn get_current_user(
6067
&self,
6168
_request: tonic::Request<openshell_core::proto::GetCurrentUserRequest>,

0 commit comments

Comments
 (0)