refactor(sandbox): guard detached against output callbacks, clearer c…

packages/cli/README.md

@@ -961,7 +961,7 @@ bunny sandbox cp my-sandbox:/workplace/out.log ./out.log
 bunny sandbox cp my-sandbox:/workplace/out.log ./logs/
 ```
 
-Uploads preserve the local file's Unix mode (so executables stay executable). Sandbox-to-sandbox copies are not supported.
+Uploads preserve the local file's Unix mode (so executables stay executable). Only single files are supported — directory and sandbox-to-sandbox copies are not.
 
 #### `bunny sandbox ssh`
 

packages/cli/src/commands/sandbox/cp.ts

@@ -83,13 +83,12 @@ export const sandboxCpCommand = defineCommand<CpArgs>({
         "Sandbox-to-sandbox copies are not supported. One side must be a local path.",
       );
     }
-    if (!srcRemote && !destRemote) {
+    const ref = srcRemote ?? destRemote;
+    if (!ref) {
       throw new UserError(
         "One path must reference a sandbox as <sandbox>:<path>.",
       );
     }
-
-    const ref = (srcRemote ?? destRemote) as RemoteRef;
     const record = getSandbox(ref.sandbox);
     if (!record) {
       throw new UserError(
@@ -125,37 +124,39 @@ export const sandboxCpCommand = defineCommand<CpArgs>({
     let from: string;
     let to: string;
     try {
-      if (destRemote) {
-        // Local -> sandbox.
+      if (uploading) {
         const local = source;
-        const file = Bun.file(local);
-        if (!(await file.exists())) {
+        const info = await stat(local).catch(() => null);
+        if (!info) {
           throw new UserError(`Local file not found: ${local}`);
         }
+        if (info.isDirectory()) {
+          throw new UserError(
+            `${local} is a directory. Only single files can be copied.`,
+          );
+        }
         // A trailing slash on the remote path means "into this directory".
-        const remotePath = destRemote.path.endsWith("/")
-          ? `${destRemote.path}${basename(local)}`
-          : destRemote.path;
-        const content = Buffer.from(await file.arrayBuffer());
-        const mode = (await stat(local)).mode & 0o777;
-        await sandbox.writeFiles([{ path: remotePath, content, mode }]);
+        const remotePath = ref.path.endsWith("/")
+          ? `${ref.path}${basename(local)}`
+          : ref.path;
+        const content = Buffer.from(await Bun.file(local).arrayBuffer());
+        await sandbox.writeFiles([
+          { path: remotePath, content, mode: info.mode & 0o777 },
+        ]);
         from = local;
         to = `${ref.sandbox}:${remotePath}`;
       } else {
-        // Sandbox -> local.
-        const content = await sandbox.readFile((srcRemote as RemoteRef).path);
+        const content = await sandbox.readFile(ref.path);
         if (content === null) {
-          throw new UserError(
-            `File not found in sandbox: ${(srcRemote as RemoteRef).path}`,
-          );
+          throw new UserError(`File not found in sandbox: ${ref.path}`);
         }
         // An existing local directory (or trailing slash) means "into it".
         const localPath =
           dest.endsWith("/") || (await isDirectory(dest))
-            ? join(dest, basename((srcRemote as RemoteRef).path))
+            ? join(dest, basename(ref.path))
             : dest;
         await Bun.write(localPath, content);
-        from = `${ref.sandbox}:${(srcRemote as RemoteRef).path}`;
+        from = `${ref.sandbox}:${ref.path}`;
         to = localPath;
       }
     } catch (err) {

packages/sandbox/src/sandbox.test.ts

@@ -59,6 +59,15 @@ describe("Sandbox.runCommand validation", () => {
       }),
     ).rejects.toThrow("not supported with detached");
   });
+
+  test("rejects output callbacks combined with detached", async () => {
+    await expect(
+      sandbox.runCommand({ cmd: "ls", detached: true, onStdout: () => {} }),
+    ).rejects.toThrow("not supported with detached");
+    await expect(
+      sandbox.runCommand({ cmd: "ls", detached: true, onStderr: () => {} }),
+    ).rejects.toThrow("not supported with detached");
+  });
 });
 
 function fakeStream() {

packages/sandbox/src/sandbox.ts

@@ -1,6 +1,6 @@
 import { randomBytes } from "node:crypto";
 import type { createMcClient } from "@bunny.net/openapi-client";
-import { Command, CommandFinished } from "./command.ts";
+import { Command, CommandFinished, type LogChunk } from "./command.ts";
 import { SandboxError } from "./errors.ts";
 import {
   addCdnEndpoint,
@@ -189,9 +189,13 @@ export class Sandbox {
         "timeout must be a positive number of milliseconds.",
       );
     }
-    if (opts.detached && (opts.timeout !== undefined || opts.signal)) {
+    const { onStdout, onStderr } = opts;
+    if (
+      opts.detached &&
+      (opts.timeout !== undefined || opts.signal || onStdout || onStderr)
+    ) {
       throw new SandboxError(
-        "timeout and signal are not supported with detached; use command.kill().",
+        "timeout, signal, onStdout, and onStderr are not supported with detached; use command.kill() and command.logs().",
       );
     }
     const remote = buildRemoteCommand(opts);
@@ -200,10 +204,10 @@ export class Sandbox {
       return new Command(await this.transport.execStream(remote));
     }
     const onData =
-      opts.onStdout || opts.onStderr
-        ? (chunk: { stream: "stdout" | "stderr"; data: string }) => {
-            if (chunk.stream === "stdout") opts.onStdout?.(chunk.data);
-            else opts.onStderr?.(chunk.data);
+      onStdout || onStderr
+        ? ({ stream, data }: LogChunk) => {
+            if (stream === "stdout") onStdout?.(data);
+            else onStderr?.(data);
           }
         : undefined;
     const { stdout, stderr, exitCode } = await this.transport.exec(remote, {
@@ -365,10 +369,8 @@ export class Sandbox {
   }
 
   /**
-   * Release the SSH connection when the sandbox leaves a `using` /
-   * `await using` scope. This mirrors `disconnect()` and deliberately does
-   * NOT delete the sandbox — the backing app keeps running. Call `delete()`
-   * explicitly to tear down an ephemeral sandbox.
+   * `using` / `await using` release the SSH connection but deliberately do
+   * NOT delete the sandbox — call `delete()` explicitly to tear one down.
    */
   [Symbol.dispose](): void {
     this.disconnect();

packages/sandbox/src/transport.ts

@@ -4,6 +4,7 @@ import {
   type ConnectConfig,
   type SFTPWrapper,
 } from "ssh2";
+import type { LogChunk } from "./command.ts";
 import { CommandTimeoutError, SandboxError } from "./errors.ts";
 import type { SandboxFileEntry } from "./types.ts";
 
@@ -27,7 +28,7 @@ export interface ExecLimits {
   /** Abort to kill the command and reject with the signal's reason. */
   signal?: AbortSignal;
   /** Called with each output chunk as it arrives, before the buffered result resolves. */
-  onData?: (chunk: { stream: "stdout" | "stderr"; data: string }) => void;
+  onData?: (chunk: LogChunk) => void;
 }
 
 /** Minimal exec-channel surface, kept narrow so tests can fake it. */
@@ -85,17 +86,18 @@ export function collectExec(
     });
     if (limits.signal?.aborted) return onAbort();
     limits.signal?.addEventListener("abort", onAbort, { once: true });
-    if (limits.timeoutMs !== undefined) {
+    const { timeoutMs } = limits;
+    if (timeoutMs !== undefined) {
       timer = setTimeout(() => {
         cleanup();
         kill();
-        reject(new CommandTimeoutError(limits.timeoutMs ?? 0, stdout, stderr));
-      }, limits.timeoutMs);
+        reject(new CommandTimeoutError(timeoutMs, stdout, stderr));
+      }, timeoutMs);
     }
   });
 }
 
-/** SFTP attrs surface used to build a SandboxFileEntry. */
+/** Minimal SFTP attrs surface, kept narrow so tests can fake it. */
 export interface FileAttrsLike {
   size?: number;
   mode: number;
@@ -104,7 +106,6 @@ export interface FileAttrsLike {
   isFile(): boolean;
 }
 
-/** Map SFTP attrs onto a SandboxFileEntry. */
 export function fileEntryFromAttrs(
   name: string,
   attrs: FileAttrsLike,