Skip to content

Commit ad0911f

Browse files
committed
fix: sandbox cancelation
1 parent 947b1a8 commit ad0911f

7 files changed

Lines changed: 246 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# CLAUDE.md
2+
3+
Behavioral guidelines to reduce common LLM coding mistakes. Merge with project-specific instructions as needed.
4+
5+
**Tradeoff:** These guidelines bias toward caution over speed. For trivial tasks, use judgment.
6+
7+
## 1. Think Before Coding
8+
9+
**Don't assume. Don't hide confusion. Surface tradeoffs.**
10+
11+
Before implementing:
12+
13+
- State your assumptions explicitly. If uncertain, ask.
14+
- If multiple interpretations exist, present them - don't pick silently.
15+
- If a simpler approach exists, say so. Push back when warranted.
16+
- If something is unclear, stop. Name what's confusing. Ask.
17+
18+
## 2. Simplicity First
19+
20+
**Minimum code that solves the problem. Nothing speculative.**
21+
22+
- No features beyond what was asked.
23+
- No abstractions for single-use code.
24+
- No "flexibility" or "configurability" that wasn't requested.
25+
- No error handling for impossible scenarios.
26+
- If you write 200 lines and it could be 50, rewrite it.
27+
28+
Ask yourself: "Would a senior engineer say this is overcomplicated?" If yes, simplify.
29+
30+
## 3. Surgical Changes
31+
32+
**Touch only what you must. Clean up only your own mess.**
33+
34+
When editing existing code:
35+
36+
- Don't "improve" adjacent code, comments, or formatting.
37+
- Don't refactor things that aren't broken.
38+
- Match existing style, even if you'd do it differently.
39+
- If you notice unrelated dead code, mention it - don't delete it.
40+
41+
When your changes create orphans:
42+
43+
- Remove imports/variables/functions that YOUR changes made unused.
44+
- Don't remove pre-existing dead code unless asked.
45+
46+
The test: Every changed line should trace directly to the user's request.
47+
48+
## 4. Goal-Driven Execution
49+
50+
**Define success criteria. Loop until verified.**
51+
52+
Transform tasks into verifiable goals:
53+
54+
- "Add validation" → "Write tests for invalid inputs, then make them pass"
55+
- "Fix the bug" → "Write a test that reproduces it, then make it pass"
56+
- "Refactor X" → "Ensure tests pass before and after"
57+
58+
For multi-step tasks, state a brief plan:
59+
60+
```
61+
1. [Step] → verify: [check]
62+
2. [Step] → verify: [check]
63+
3. [Step] → verify: [check]
64+
```
65+
66+
Strong success criteria let you loop independently. Weak criteria ("make it work") require constant clarification.
67+
68+
---
69+
70+
**These guidelines are working if:** fewer unnecessary changes in diffs, fewer rewrites due to overcomplication, and clarifying questions come before implementation rather than after mistakes.

src/lib/cancel-run.test.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
22
import type { RunRegistryAdapter } from "../adapters/run-registry/types.js";
33

44
const mockGetRun = vi.fn();
5+
const mockStopTicketSandboxes = vi.fn();
56
vi.mock("workflow/api", () => ({
67
getRun: (...args: any[]) => mockGetRun(...args),
78
}));
9+
vi.mock("../sandbox/stop-ticket-sandboxes.js", () => ({
10+
stopTicketSandboxes: (...args: any[]) => mockStopTicketSandboxes(...args),
11+
}));
812

913
function makeRegistry(overrides: Partial<RunRegistryAdapter> = {}): RunRegistryAdapter {
1014
return {
@@ -21,7 +25,10 @@ function makeRegistry(overrides: Partial<RunRegistryAdapter> = {}): RunRegistryA
2125
}
2226

2327
describe("cancelRun", () => {
24-
beforeEach(() => vi.clearAllMocks());
28+
beforeEach(() => {
29+
vi.clearAllMocks();
30+
mockStopTicketSandboxes.mockResolvedValue(0);
31+
});
2532

2633
it("cancels the run and unregisters", async () => {
2734
const mockCancel = vi.fn().mockResolvedValue(undefined);
@@ -34,6 +41,7 @@ describe("cancelRun", () => {
3441
expect(result).toBe(true);
3542
expect(mockGetRun).toHaveBeenCalledWith("run_abc");
3643
expect(mockCancel).toHaveBeenCalled();
44+
expect(mockStopTicketSandboxes).toHaveBeenCalledWith("PROJ-1");
3745
expect(registry.unregister).toHaveBeenCalledWith("PROJ-1");
3846
});
3947

@@ -47,6 +55,7 @@ describe("cancelRun", () => {
4755
const result = await cancelRun("PROJ-1", "run_abc", registry);
4856

4957
expect(result).toBe(false);
58+
expect(mockStopTicketSandboxes).toHaveBeenCalledWith("PROJ-1");
5059
expect(registry.unregister).toHaveBeenCalledWith("PROJ-1");
5160
});
5261

@@ -63,5 +72,6 @@ describe("cancelRun", () => {
6372

6473
expect(result).toBe(false);
6574
expect(unregister).toHaveBeenCalledTimes(2);
75+
expect(mockStopTicketSandboxes).toHaveBeenCalledTimes(2);
6676
});
6777
});

src/lib/cancel-run.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { getRun } from "workflow/api";
22
import { logger } from "./logger.js";
33
import type { RunRegistryAdapter } from "../adapters/run-registry/types.js";
4+
import { stopTicketSandboxes } from "../sandbox/stop-ticket-sandboxes.js";
45

56
/**
67
* Cancel a workflow run and unregister it from the registry.
@@ -24,6 +25,7 @@ export async function cancelRun(
2425
);
2526
}
2627

28+
await stopTicketSandboxes(ticketKey).catch(() => {});
2729
await runRegistry.unregister(ticketKey).catch(() => {});
2830
return cancelled;
2931
}

src/lib/dispatch.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@ vi.mock("../workflows/agent.js", () => ({
2121
}));
2222

2323
const mockSandboxList = vi.fn();
24+
const mockStopTicketSandboxes = vi.fn();
2425
vi.mock("@vercel/sandbox", () => ({
2526
Sandbox: {
2627
list: (...args: any[]) => mockSandboxList(...args),
2728
},
2829
}));
30+
vi.mock("../sandbox/stop-ticket-sandboxes.js", () => ({
31+
stopTicketSandboxes: (...args: any[]) => mockStopTicketSandboxes(...args),
32+
}));
2933

3034
function makeTicket(overrides: Partial<TicketContent> = {}): TicketContent {
3135
return {
@@ -103,6 +107,7 @@ describe("dispatchTicket", () => {
103107
json: { sandboxes: [] },
104108
});
105109
mockStart.mockResolvedValue({ runId: "run_123" });
110+
mockStopTicketSandboxes.mockResolvedValue(0);
106111
});
107112

108113
it("dispatches agentWorkflow for a ticket in configured project + AI column", async () => {
@@ -205,6 +210,7 @@ describe("dispatchTicket", () => {
205210
expect(mockStart).toHaveBeenCalled();
206211
expect(mockGetRun).toHaveBeenCalledWith("run_123");
207212
expect(mockCancel).toHaveBeenCalled();
213+
expect(mockStopTicketSandboxes).toHaveBeenCalledWith("PROJ-42");
208214
expect(adapters.runRegistry.register).not.toHaveBeenCalled();
209215
});
210216

@@ -272,6 +278,7 @@ describe("failed-ticket safeguard full loop", () => {
272278
vi.clearAllMocks();
273279
mockSandboxList.mockResolvedValue({ json: { sandboxes: [] } });
274280
mockStart.mockResolvedValue({ runId: "run_123" });
281+
mockStopTicketSandboxes.mockResolvedValue(0);
275282
});
276283

277284
it("mark → skip → clear → redispatch", async () => {

src/lib/dispatch.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { env } from "../../env.js";
33
import { agentWorkflow } from "../workflows/agent.js";
44
import { logger } from "./logger.js";
55
import type { Adapters } from "./adapters.js";
6+
import { stopTicketSandboxes } from "../sandbox/stop-ticket-sandboxes.js";
67

78
const CLAIMING_PREFIX = "claiming:";
89

@@ -140,6 +141,7 @@ async function abortWorkflow(runId: string, ticketKey: string): Promise<void> {
140141
const run = getRun(runId);
141142
await run.cancel();
142143
} catch {}
144+
await stopTicketSandboxes(ticketKey).catch(() => {});
143145
}
144146

145147
function extractProjectKey(ticketIdentifier: string): string | null {
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
const mockList = vi.fn();
4+
const mockGet = vi.fn();
5+
6+
vi.mock("@vercel/sandbox", () => ({
7+
Sandbox: {
8+
list: (...args: any[]) => mockList(...args),
9+
get: (...args: any[]) => mockGet(...args),
10+
},
11+
}));
12+
13+
vi.mock("./credentials.js", () => ({
14+
getSandboxCredentials: vi.fn(() => ({})),
15+
}));
16+
17+
function makeSandbox(branch: string, status: "running" | "stopped" = "running") {
18+
return {
19+
status,
20+
runCommand: vi.fn().mockResolvedValue({
21+
exitCode: 0,
22+
stdout: async () => branch,
23+
}),
24+
stop: vi.fn().mockResolvedValue(undefined),
25+
};
26+
}
27+
28+
describe("stopTicketSandboxes", () => {
29+
beforeEach(() => {
30+
vi.clearAllMocks();
31+
});
32+
33+
it("stops running sandboxes on the ticket branch", async () => {
34+
const matching = makeSandbox("blazebot/proj-42");
35+
const other = makeSandbox("blazebot/proj-99");
36+
37+
mockList.mockResolvedValue({
38+
json: {
39+
sandboxes: [
40+
{ id: "sbx-1", status: "running" },
41+
{ id: "sbx-2", status: "running" },
42+
],
43+
},
44+
});
45+
46+
mockGet.mockImplementation(async ({ sandboxId }: { sandboxId: string }) => {
47+
if (sandboxId === "sbx-1") return matching;
48+
if (sandboxId === "sbx-2") return other;
49+
throw new Error(`unexpected sandbox id: ${sandboxId}`);
50+
});
51+
52+
const { stopTicketSandboxes } = await import("./stop-ticket-sandboxes.js");
53+
const stopped = await stopTicketSandboxes("PROJ-42");
54+
55+
expect(stopped).toBe(1);
56+
expect(matching.stop).toHaveBeenCalledTimes(1);
57+
expect(other.stop).not.toHaveBeenCalled();
58+
});
59+
60+
it("returns 0 when sandbox listing fails", async () => {
61+
mockList.mockRejectedValue(new Error("sandbox api down"));
62+
63+
const { stopTicketSandboxes } = await import("./stop-ticket-sandboxes.js");
64+
const stopped = await stopTicketSandboxes("PROJ-42");
65+
66+
expect(stopped).toBe(0);
67+
expect(mockGet).not.toHaveBeenCalled();
68+
});
69+
});
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { logger } from "../lib/logger.js";
2+
import { getSandboxCredentials } from "./credentials.js";
3+
4+
const BRANCH_PREFIX = "blazebot/";
5+
6+
/**
7+
* Best-effort cleanup for leaked sandboxes after ticket cancellation.
8+
* Finds running sandboxes whose checked-out branch matches the ticket branch
9+
* and requests stop on each match.
10+
*/
11+
export async function stopTicketSandboxes(ticketKey: string): Promise<number> {
12+
const normalizedTicket = ticketKey.trim().toLowerCase();
13+
if (!normalizedTicket) return 0;
14+
15+
const expectedBranch = `${BRANCH_PREFIX}${normalizedTicket}`;
16+
17+
try {
18+
const { Sandbox } = await import("@vercel/sandbox");
19+
const credentials = getSandboxCredentials();
20+
const { json } = await Sandbox.list({ ...credentials, limit: 100 });
21+
const running = json.sandboxes.filter((sandbox) => sandbox.status === "running");
22+
23+
let stopped = 0;
24+
for (const entry of running) {
25+
try {
26+
const sandbox = await Sandbox.get({
27+
...credentials,
28+
sandboxId: entry.id,
29+
});
30+
if (sandbox.status !== "running") continue;
31+
32+
const branch = await getSandboxBranch(sandbox);
33+
if (branch !== expectedBranch) continue;
34+
35+
await sandbox.stop();
36+
stopped++;
37+
} catch (err) {
38+
logger.warn(
39+
{
40+
ticketKey,
41+
sandboxId: entry.id,
42+
error: (err as Error).message,
43+
},
44+
"cancel_run_sandbox_stop_failed",
45+
);
46+
}
47+
}
48+
49+
if (stopped > 0) {
50+
logger.info(
51+
{ ticketKey, expectedBranch, stopped },
52+
"cancel_run_stopped_ticket_sandboxes",
53+
);
54+
}
55+
return stopped;
56+
} catch (err) {
57+
logger.warn(
58+
{ ticketKey, expectedBranch, error: (err as Error).message },
59+
"cancel_run_sandbox_discovery_failed",
60+
);
61+
return 0;
62+
}
63+
}
64+
65+
async function getSandboxBranch(sandbox: {
66+
runCommand: (
67+
params: {
68+
cmd: string;
69+
args: string[];
70+
cwd: string;
71+
},
72+
) => Promise<{ exitCode: number; stdout: () => Promise<string> }>;
73+
}): Promise<string | null> {
74+
try {
75+
const result = await sandbox.runCommand({
76+
cmd: "git",
77+
args: ["rev-parse", "--abbrev-ref", "HEAD"],
78+
cwd: "/vercel/sandbox",
79+
});
80+
if (result.exitCode !== 0) return null;
81+
return (await result.stdout()).trim();
82+
} catch {
83+
return null;
84+
}
85+
}

0 commit comments

Comments
 (0)