fix: unstrand sessions after a sandbox connecting timeout

packages/control-plane/src/sandbox/client.ts

@@ -240,6 +240,18 @@ export interface SnapshotSandboxResponse {
   error?: string;
 }
 
+export interface TerminateSandboxRequest {
+  providerObjectId: string;
+  sessionId: string;
+  reason: string;
+  signal?: AbortSignal;
+}
+
+export interface TerminateSandboxResponse {
+  success: boolean;
+  error?: string;
+}
+
 export interface SnapshotBuildSandboxRequest {
   buildId: string;
   providerSessionId: string;
@@ -316,6 +328,7 @@ export class ModalApiError extends Error {
 export class ModalClient {
   private createSandboxUrl: string;
   private snapshotSandboxUrl: string;
+  private terminateSandboxUrl: string;
   private snapshotBuildSandboxUrl: string;
   private restoreSandboxUrl: string;
   private createImageBuildSandboxUrl: string;
@@ -362,6 +375,7 @@ export class ModalClient {
     const baseUrl = getModalBaseUrl(workspace, environmentWebSuffix);
     this.createSandboxUrl = `${baseUrl}-api-create-sandbox.modal.run`;
     this.snapshotSandboxUrl = `${baseUrl}-api-snapshot-sandbox.modal.run`;
+    this.terminateSandboxUrl = `${baseUrl}-api-terminate-sandbox.modal.run`;
     this.snapshotBuildSandboxUrl = `${baseUrl}-api-snapshot-build-sandbox.modal.run`;
     this.restoreSandboxUrl = `${baseUrl}-api-restore-sandbox.modal.run`;
     this.createImageBuildSandboxUrl = `${baseUrl}-api-create-build-sandbox.modal.run`;
@@ -535,7 +549,55 @@ export class ModalClient {
   }
 
   /**
-   * Trigger a filesystem snapshot for a sandbox object.
+   * Terminate a sandbox by its Modal object id.
+   */
+  async terminateSandbox(
+    request: TerminateSandboxRequest,
+    correlation?: CorrelationContext
+  ): Promise<TerminateSandboxResponse> {
+    const startTime = Date.now();
+    const endpoint = "terminateSandbox";
+    let httpStatus: number | undefined;
+    let outcome: "success" | "error" = "error";
+
+    try {
+      const result = await this.postJson(
+        this.terminateSandboxUrl,
+        endpoint,
+        MODAL_CLEANUP_REQUEST_DEADLINE_MS,
+        {
+          sandbox_id: request.providerObjectId,
+          session_id: request.sessionId,
+          reason: request.reason,
+        },
+        imageBuildOperationModalResponseSchema,
+        correlation,
+        request.signal,
+        (status) => (httpStatus = status)
+      );
+      if (!result.success) {
+        return { success: false, error: result.error || "Unknown terminate error" };
+      }
+
+      outcome = "success";
+      return { success: true };
+    } finally {
+      log.info("modal.request", {
+        event: "modal.request",
+        endpoint,
+        session_id: request.sessionId,
+        sandbox_id: request.providerObjectId,
+        trace_id: correlation?.trace_id,
+        request_id: correlation?.request_id,
+        http_status: httpStatus,
+        duration_ms: Date.now() - startTime,
+        outcome,
+      });
+    }
+  }
+
+  /**
+   * Take a filesystem snapshot of a running sandbox.
    */
   async snapshotSandbox(
     request: SnapshotSandboxRequest,

packages/control-plane/src/sandbox/lifecycle/manager.test.ts

@@ -1027,6 +1027,37 @@ describe("SandboxLifecycleManager", () => {
       expect(sandbox.last_spawn_error).toContain("temporarily disabled");
     });
 
+    it("schedules a retry alarm when the circuit breaker is open", async () => {
+      const now = Date.now();
+      const sandbox = createMockSandbox({
+        status: "pending",
+        spawn_failure_count: 3,
+        last_spawn_failure: now - 60000,
+      });
+      const storage = createMockStorage(createMockSession(), sandbox);
+      const alarmScheduler = createMockAlarmScheduler();
+      const manager = new SandboxLifecycleManager(
+        createMockProvider(),
+        storage,
+        createMockBroadcaster(),
+        createMockWebSocketManager(false),
+        alarmScheduler,
+        createMockIdGenerator(),
+        createTestConfig()
+      );
+
+      await manager.spawnSandbox();
+
+      // Bot-triggered prompts have no next message to retry on: without a
+      // scheduled retry the open breaker strands them for good.
+      expect(alarmScheduler.alarms.length).toBeGreaterThan(0);
+      const last = alarmScheduler.alarms[alarmScheduler.alarms.length - 1];
+      expect(last).toBeGreaterThan(now);
+      // The retry lands when the breaker window has passed: 3 failures at
+      // 60s ago leaves the full 5-minute window still to run.
+      expect(last).toBeGreaterThanOrEqual(now + 4 * 60 * 1000);
+    });
+
     it("still broadcasts the reason when persisting it throws", async () => {
       const now = Date.now();
       const sandbox = createMockSandbox({
@@ -2205,6 +2236,59 @@ describe("SandboxLifecycleManager", () => {
       expect(provider.takeSnapshot).not.toHaveBeenCalled();
     });
 
+    it("counts a connecting timeout toward the circuit breaker", async () => {
+      const now = Date.now();
+      const sandbox = createMockSandbox({
+        status: "connecting" as SandboxStatus,
+        created_at: now - 130_000,
+        last_heartbeat: null,
+      });
+      const storage = createMockStorage(createMockSession(), sandbox);
+
+      const manager = new SandboxLifecycleManager(
+        createMockProvider(),
+        storage,
+        createMockBroadcaster(),
+        createMockWebSocketManager(),
+        createMockAlarmScheduler(),
+        createMockIdGenerator(),
+        createTestConfig()
+      );
+
+      await manager.handleAlarm();
+
+      // The queued prompt is re-driven after this failure, so a repository
+      // whose boot always exceeds the timeout must eventually stop respawning.
+      expect(storage.calls).toContain("incrementCircuitBreakerFailure");
+    });
+
+    it("does not reset the circuit breaker until the sandbox connects", async () => {
+      const now = Date.now();
+      const sandbox = createMockSandbox({
+        status: "pending" as SandboxStatus,
+        created_at: now - 60_000,
+      });
+      const storage = createMockStorage(createMockSession(), sandbox);
+
+      const manager = new SandboxLifecycleManager(
+        createMockProvider(),
+        storage,
+        createMockBroadcaster(),
+        createMockWebSocketManager(false),
+        createMockAlarmScheduler(),
+        createMockIdGenerator(),
+        createTestConfig()
+      );
+
+      await manager.spawnSandbox();
+      // Spawn initiation is not success: the sandbox has not connected yet,
+      // and connecting timeouts counted before it must stay counted.
+      expect(storage.calls).not.toContain("resetCircuitBreaker");
+
+      manager.onSandboxConnected();
+      expect(storage.calls).toContain("resetCircuitBreaker");
+    });
+
     it("does not timeout connecting sandbox within timeout window", async () => {
       const now = Date.now();
       const sandbox = createMockSandbox({

packages/control-plane/src/sandbox/lifecycle/manager.ts

@@ -352,6 +352,9 @@ export class SandboxLifecycleManager implements SandboxLifecycle {
         failure_count: circuitBreakerState.failureCount,
         wait_time_ms: cbDecision.waitTimeMs || 0,
       });
+      // Bot-triggered prompts have no "next message" to retry on; without a
+      // scheduled retry the open breaker strands them for good.
+      await this.alarmScheduler.schedule(now + (cbDecision.waitTimeMs || 0));
       this.reportSandboxError(
         `Sandbox spawning temporarily disabled after ${circuitBreakerState.failureCount} failures. Try again in ${Math.ceil((cbDecision.waitTimeMs || 0) / 1000)} seconds.`
       );
@@ -577,9 +580,6 @@ export class SandboxLifecycleManager implements SandboxLifecycle {
 
       await this.finishProviderStartup();
 
-      // Reset circuit breaker on successful spawn initiation
-      this.storage.resetCircuitBreaker();
-
       this.log.info("Sandbox spawn completed", {
         event: "sandbox.spawn",
         outcome: "success",
@@ -988,7 +988,6 @@ export class SandboxLifecycleManager implements SandboxLifecycle {
 
       await this.storeAndBroadcastTunnelUrls(result.tunnelUrls);
       await this.finishProviderStartup();
-      this.storage.resetCircuitBreaker();
     } catch (error) {
       const errorMessage = error instanceof Error ? error.message : "Failed to resume sandbox";
       this.storage.updateSandboxStatus("failed");
@@ -1240,6 +1239,10 @@ export class SandboxLifecycleManager implements SandboxLifecycle {
       });
       this.storage.updateSandboxStatus("failed");
       this.clearSandboxAccessState();
+      // Count toward the circuit breaker: the queued prompt is re-driven
+      // after this failure, and a repository whose boot always exceeds the
+      // timeout would otherwise respawn in a loop.
+      this.storage.incrementCircuitBreakerFailure(now);
       if (this.canStopProviderSandbox()) {
         try {
           await this.stopProviderSandbox("connecting_timeout");
@@ -1251,7 +1254,7 @@ export class SandboxLifecycleManager implements SandboxLifecycle {
       }
       this.broadcaster.broadcast({ type: "sandbox_status", status: "failed" });
       this.reportSandboxError(
-        "Sandbox failed to connect within the allowed time. It will be retried on your next message."
+        "Sandbox failed to connect within the allowed time. Retrying with a fresh sandbox."
       );
       return "sandbox_failed";
     }
@@ -1606,12 +1609,16 @@ export class SandboxLifecycleManager implements SandboxLifecycle {
 
   /**
    * Notify the manager that a sandbox has connected.
-   * Resets the in-memory spawning flag and clears any stale spawn error.
+   * Resets the in-memory spawning flag, clears any stale spawn error, and
+   * closes the spawn circuit breaker: connection is the first point where a
+   * spawn actually succeeded, so failures counted before it (connecting
+   * timeouts) stay counted until a sandbox genuinely comes up.
    *
    * Called by SessionDO when sandbox WebSocket connects successfully.
    */
   onSandboxConnected(): void {
     this.isSpawningSandbox = false;
     this.storage.setLastSpawnError(null, null);
+    this.storage.resetCircuitBreaker();
   }
 }

packages/control-plane/src/sandbox/providers/modal-provider.test.ts

@@ -18,6 +18,8 @@ import type {
   SnapshotSandboxRequest,
   SnapshotBuildSandboxRequest,
   SnapshotSandboxResponse,
+  TerminateSandboxRequest,
+  TerminateSandboxResponse,
   CreateImageBuildSandboxRequest,
   CreateImageBuildSandboxResponse,
   StartImageBuildSandboxRequest,
@@ -33,6 +35,7 @@ function createMockModalClient(
     createSandbox: (req: CreateSandboxRequest) => Promise<CreateSandboxResponse>;
     restoreSandbox: (req: RestoreSandboxRequest) => Promise<RestoreSandboxResponse>;
     snapshotSandbox: (req: SnapshotSandboxRequest) => Promise<SnapshotSandboxResponse>;
+    terminateSandbox: (req: TerminateSandboxRequest) => Promise<TerminateSandboxResponse>;
     snapshotBuildSandbox: (req: SnapshotBuildSandboxRequest) => Promise<SnapshotSandboxResponse>;
     createImageBuildSandbox: (
       req: CreateImageBuildSandboxRequest
@@ -63,6 +66,7 @@ function createMockModalClient(
         imageId: "image-123",
       })
     ),
+    terminateSandbox: vi.fn(async (): Promise<TerminateSandboxResponse> => ({ success: true })),
     snapshotBuildSandbox: vi.fn(
       async (): Promise<SnapshotSandboxResponse> => ({
         success: true,
@@ -108,6 +112,7 @@ describe("ModalSandboxProvider", () => {
       expect(provider.name).toBe("modal");
       expect(provider.capabilities.supportsSnapshots).toBe(true);
       expect(provider.capabilities.supportsRestore).toBe(true);
+      expect(provider.capabilities.supportsExplicitStop).toBe(true);
     });
   });
 
@@ -505,6 +510,62 @@ describe("ModalSandboxProvider", () => {
     });
   });
 
+  describe("stopSandbox", () => {
+    it("terminates the provider sandbox by object id", async () => {
+      const client = createMockModalClient();
+      const provider = new ModalSandboxProvider(client);
+
+      const result = await provider.stopSandbox({
+        providerObjectId: "mo-1",
+        sessionId: "session-1",
+        reason: "connecting_timeout",
+      });
+
+      expect(result).toEqual({ success: true });
+      expect(client.terminateSandbox).toHaveBeenCalledWith(
+        {
+          providerObjectId: "mo-1",
+          sessionId: "session-1",
+          reason: "connecting_timeout",
+          signal: undefined,
+        },
+        undefined
+      );
+    });
+
+    it("returns the failure message when terminate reports one", async () => {
+      const client = createMockModalClient({
+        terminateSandbox: vi.fn(async () => ({ success: false, error: "sandbox lookup failed" })),
+      });
+      const provider = new ModalSandboxProvider(client);
+
+      const result = await provider.stopSandbox({
+        providerObjectId: "mo-1",
+        sessionId: "session-1",
+        reason: "connecting_timeout",
+      });
+
+      expect(result).toEqual({ success: false, error: "sandbox lookup failed" });
+    });
+
+    it("classifies HTTP 503 from terminate as transient", async () => {
+      const client = createMockModalClient({
+        terminateSandbox: vi.fn(async () => {
+          throw new ModalApiError("stop failed", 503);
+        }),
+      });
+      const provider = new ModalSandboxProvider(client);
+
+      await expect(
+        provider.stopSandbox({
+          providerObjectId: "mo-1",
+          sessionId: "session-1",
+          reason: "connecting_timeout",
+        })
+      ).rejects.toMatchObject({ errorType: "transient" });
+    });
+  });
+
   describe("image builds", () => {
     it("binds a created image-build sandbox before starting it", async () => {
       const client = createMockModalClient();