Clean up sandbox command execution flags

Author: cursoragent

pkg/cmd/sandbox/common.go

@@ -38,22 +38,6 @@ func sandboxRef(id string) *sandboxv1.SandboxRef {
 	}
 }
 
-// commandRef wraps a command id in the depot.sandbox.v1 selector oneof.
-// Used by AttachCommand / KillCommand callers.
-func commandRef(id string) *sandboxv1.SandboxCommandExecutionRef {
-	return &sandboxv1.SandboxCommandExecutionRef{
-		Selector: &sandboxv1.SandboxCommandExecutionRef_Id{Id: id},
-	}
-}
-
-// snapshotRef wraps a snapshot id in the depot.sandbox.v1 selector oneof.
-// Used by GetSnapshot / DeleteSnapshot.
-func snapshotRef(id string) *sandboxv1.SnapshotRef {
-	return &sandboxv1.SnapshotRef{
-		Selector: &sandboxv1.SnapshotRef_Id{Id: id},
-	}
-}
-
 // consumeCommandEventStream drains a depot.sandbox.v1 CommandEvent stream
 // into stdout/stderr and returns the final exit code from Finished. The
 // stream shape mirrors RunCommand / RunCommandPipe / AttachCommand /
@@ -63,19 +47,9 @@ func snapshotRef(id string) *sandboxv1.SnapshotRef {
 // see the gap; the stream continues afterward. Error frames are surfaced the
 // same way and do not abort the loop (the server is signalling partial
 // degradation, not a fatal end — Connect transport errors are the fatal path).
-// detachedMode flags a RunCommand stream that the server will close after
-// Started — no Finished event will arrive, and that's not an error.
-type detachedMode bool
-
-const (
-	streamUntilFinished detachedMode = false
-	streamUntilStarted  detachedMode = true
-)
-
 func consumeCommandEventStream(
 	stream *connect.ServerStreamForClient[sandboxv1.SandboxCommandExecutionEvent],
 	stdout, stderr io.Writer,
-	mode detachedMode,
 ) (exitCode int32, err error) {
 	defer func() { _ = stream.Close() }()
 	for stream.Receive() {
@@ -109,13 +83,5 @@ func consumeCommandEventStream(
 	if err := stream.Err(); err != nil && !errors.Is(err, io.EOF) {
 		return 0, fmt.Errorf("command stream: %w", err)
 	}
-	// In detached mode the server hangs up after Started — no Finished is
-	// expected and "exit 0, no error" is the right success signal. For
-	// non-detached commands, end-of-stream without Finished means the server
-	// disconnected mid-run; surface that as an error so callers don't conflate
-	// a clean disconnect with a real exit-0 completion.
-	if mode == streamUntilStarted {
-		return 0, nil
-	}
 	return 0, fmt.Errorf("command stream closed without Finished event")
 }

pkg/cmd/sandbox/common_test.go

@@ -22,31 +22,3 @@ func TestSandboxRef_PinnedShape(t *testing.T) {
 		t.Errorf("id = %q, want cs-abc123", sel.Id)
 	}
 }
-
-func TestCommandRef_PinnedShape(t *testing.T) {
-	r := commandRef("cmd-xyz789")
-	if r == nil {
-		t.Fatal("commandRef returned nil")
-	}
-	sel, ok := r.Selector.(*sandboxv1.SandboxCommandExecutionRef_Id)
-	if !ok {
-		t.Fatalf("expected CommandRef_Id selector, got %T", r.Selector)
-	}
-	if sel.Id != "cmd-xyz789" {
-		t.Errorf("id = %q, want cmd-xyz789", sel.Id)
-	}
-}
-
-func TestSnapshotRef_PinnedShape(t *testing.T) {
-	r := snapshotRef("snap-456")
-	if r == nil {
-		t.Fatal("snapshotRef returned nil")
-	}
-	sel, ok := r.Selector.(*sandboxv1.SnapshotRef_Id)
-	if !ok {
-		t.Fatalf("expected SnapshotRef_Id selector, got %T", r.Selector)
-	}
-	if sel.Id != "snap-456" {
-		t.Errorf("id = %q, want snap-456", sel.Id)
-	}
-}

pkg/cmd/sandbox/exec.go

@@ -54,7 +54,6 @@ parsing stops there.`,
 			cwd, _ := cmd.Flags().GetString("cwd")
 			envSlice, _ := cmd.Flags().GetStringArray("env")
 			sudo, _ := cmd.Flags().GetBool("sudo")
-			detached, _ := cmd.Flags().GetBool("detached")
 
 			envMap, err := parseEnvSlice(envSlice)
 			if err != nil {
@@ -73,9 +72,6 @@ parsing stops there.`,
 			if sudo {
 				req.Sudo = &sudo
 			}
-			if detached {
-				req.Detached = &detached
-			}
 
 			// The --timeout flag is deprecated. Warn if it is still set, but
 			// do not fail, since the wire protocol no longer carries a timeout.
@@ -88,11 +84,7 @@ parsing stops there.`,
 				return fmt.Errorf("run command: %w", err)
 			}
 
-			mode := streamUntilFinished
-			if detached {
-				mode = streamUntilStarted
-			}
-			exit, err := consumeCommandEventStream(stream, os.Stdout, os.Stderr, mode)
+			exit, err := consumeCommandEventStream(stream, os.Stdout, os.Stderr)
 			if err != nil {
 				return err
 			}
@@ -106,7 +98,6 @@ parsing stops there.`,
 	cmd.Flags().String("cwd", "", "Working directory inside the sandbox")
 	cmd.Flags().StringArray("env", nil, "Environment variables to set (KEY=VALUE), repeatable")
 	cmd.Flags().Bool("sudo", false, "Run as root")
-	cmd.Flags().Bool("detached", false, "Return after Started; reattach later via AttachCommand")
 	// Deprecated: hidden and ignored.
 	cmd.Flags().Int("timeout", 0, "Deprecated: timeouts are not part of the v0 wire (will be removed)")
 	_ = cmd.Flags().MarkHidden("timeout")

pkg/cmd/sandbox/hooks.go

@@ -147,7 +147,7 @@ func runHook(ctx context.Context, client sandboxv1connect.SandboxServiceClient,
 	// command with setsid and exits 0, so the stream reaches Finished right
 	// away. Waiting until finished is correct for both detached and
 	// foreground hooks.
-	exit, err := consumeCommandEventStream(stream, stdout, stderr, streamUntilFinished)
+	exit, err := consumeCommandEventStream(stream, stdout, stderr)
 	if err != nil {
 		return err
 	}