Skip to content

feat: replace snapshotId with name-based sandbox persistence #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
recoup-coding-agent wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: replace snapshotId with name-based sandbox persistence #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions src/recoup/updateAccountSnapshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,21 @@ import { NEW_API_BASE_URL, RECOUP_API_KEY } from "../consts";
* Updates sandbox config for an account via PATCH /api/sandboxes.
*
* @param accountId - The account ID to update
* @param snapshotId - Optional snapshot ID to set
* @param _snapshotId - Deprecated, unused. Kept for call-site compat during migration.
* @param githubRepo - Optional GitHub repo URL to persist
* @returns The updated snapshot data or undefined on error
* @returns The updated data or undefined on error
*/
export async function updateAccountSnapshot(
accountId: string,
snapshotId?: string,
_snapshotId?: string,
githubRepo?: string
): Promise<{ success: boolean; snapshotId: string } | undefined> {
): Promise<{ success: boolean } | undefined> {
Comment on lines 12 to +16
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This no longer clears persisted snapshot_id state.

src/recoup/getAccountSandboxes.ts:5-9 still models snapshot_id, and src/sandboxes/getOrCreateSandbox.ts:12-13 says POST /api/sandboxes still performs restoration internally. Deprecating _snapshotId without sending an explicit clear value means old snapshot links can survive this rollout, which makes the name-based migration incomplete and can keep the backend restoring stale sandbox state.

Also applies to: 22-25

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/recoup/updateAccountSnapshot.ts` around lines 12 - 16, The
updateAccountSnapshot function currently drops the optional _snapshotId and so
no longer clears persisted snapshot_id in the backend; modify
updateAccountSnapshot (and any call sites that pass _snapshotId) to explicitly
send a clear value (e.g., null or empty-string per API contract) when the intent
is to remove the saved snapshot_id so the backend restores do not keep stale
state; ensure the request body includes snapshot_id: null (or "") rather than
omitting the field, and coordinate with src/recoup/getAccountSandboxes (which
still models snapshot_id) and src/sandboxes/getOrCreateSandbox (which expects
POST /api/sandboxes to restore) to preserve the intended migration semantics.

const url = `${NEW_API_BASE_URL}/api/sandboxes`;

logger.log("Updating account snapshot", { accountId, snapshotId, githubRepo, url });
logger.log("Updating account sandbox metadata", { accountId, githubRepo, url });

try {
const body: Record<string, string> = { account_id: accountId };
if (snapshotId) {
body.snapshotId = snapshotId;
}
if (githubRepo) {
body.github_repo = githubRepo;
}
Expand All @@ -38,7 +35,7 @@ export async function updateAccountSnapshot(

if (!response.ok) {
const errorText = await response.text();
logger.error("Failed to update account snapshot", {
logger.error("Failed to update account sandbox metadata", {
status: response.status,
statusText: response.statusText,
error: errorText,
Expand All @@ -48,16 +45,14 @@ export async function updateAccountSnapshot(

const data = await response.json();

logger.log("Account snapshot updated successfully", {
logger.log("Account sandbox metadata updated successfully", {
accountId,
snapshotId: data.snapshotId,
});

return data;
} catch (error) {
logger.error("Error updating account snapshot", {
logger.error("Error updating account sandbox metadata", {
accountId,
snapshotId,
error: error instanceof Error ? error.message : String(error),
});
return undefined;
Expand Down
6 changes: 3 additions & 3 deletions src/sandboxes/__tests__/getOrCreateSandbox.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ vi.mock("@trigger.dev/sdk/v3", () => ({
vi.mock("@vercel/sandbox", () => ({
Sandbox: {
get: vi.fn().mockResolvedValue({
sandboxId: "sbx_123",
name: "sbx_123",
status: "running",
}),
},
Expand Down Expand Up @@ -53,7 +53,7 @@ describe("getOrCreateSandbox", () => {
await getOrCreateSandbox("acc_1");

expect(Sandbox.get).toHaveBeenCalledWith(
expect.objectContaining({ sandboxId: "sbx_123" }),
expect.objectContaining({ name: "sbx_123" }),
);
});

Expand All @@ -62,7 +62,7 @@ describe("getOrCreateSandbox", () => {

expect(result.sandboxId).toBe("sbx_123");
expect(result.sandbox).toEqual(
expect.objectContaining({ sandboxId: "sbx_123" }),
expect.objectContaining({ name: "sbx_123" }),
);
});

Expand Down
1 change: 0 additions & 1 deletion src/sandboxes/__tests__/notifyCodingAgentCallback.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ describe("notifyCodingAgentCallback", () => {
threadId: "slack:C123:123",
status: "pr_created",
branch: "agent/fix-123",
snapshotId: "snap_abc",
prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }],
});

Expand Down
29 changes: 16 additions & 13 deletions src/sandboxes/__tests__/snapshotAndPersist.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ vi.mock("@trigger.dev/sdk/v3", () => ({
}));

vi.mock("../../recoup/updateAccountSnapshot", () => ({
updateAccountSnapshot: vi.fn().mockResolvedValue({ success: true, snapshotId: "snap_123" }),
updateAccountSnapshot: vi.fn().mockResolvedValue({ success: true }),
}));

const { snapshotAndPersist } = await import("../snapshotAndPersist");

const SEVEN_DAYS_MS = 7 * 24 * 60 * 60 * 1000;

function createMockSandbox() {
return {
snapshot: vi.fn().mockResolvedValue({
Expand All @@ -26,26 +24,31 @@ beforeEach(() => {
});

describe("snapshotAndPersist", () => {
it("passes 7-day expiration to sandbox.snapshot()", async () => {
it("persists github repo without snapshotId", async () => {
const sandbox = createMockSandbox();
await snapshotAndPersist(sandbox, "acc_456");
expect(sandbox.snapshot).toHaveBeenCalledWith({ expiration: SEVEN_DAYS_MS });
const { updateAccountSnapshot } = await import("../../recoup/updateAccountSnapshot");
await snapshotAndPersist(sandbox, "acc_456", "https://github.com/org/repo");
expect(updateAccountSnapshot).toHaveBeenCalledWith(
"acc_456",
undefined,
"https://github.com/org/repo",
);
});

it("returns the snapshot result", async () => {
it("does not take a sandbox snapshot", async () => {
const sandbox = createMockSandbox();
const result = await snapshotAndPersist(sandbox, "acc_456");
expect(result.snapshotId).toBe("snap_123");
await snapshotAndPersist(sandbox, "acc_456");
expect(sandbox.snapshot).not.toHaveBeenCalled();
});

it("persists snapshot with github repo when provided", async () => {
it("persists without snapshotId when no github repo", async () => {
const sandbox = createMockSandbox();
const { updateAccountSnapshot } = await import("../../recoup/updateAccountSnapshot");
await snapshotAndPersist(sandbox, "acc_456", "https://github.com/org/repo");
await snapshotAndPersist(sandbox, "acc_456");
expect(updateAccountSnapshot).toHaveBeenCalledWith(
"acc_456",
"snap_123",
"https://github.com/org/repo",
undefined,
undefined,
);
});
});
3 changes: 1 addition & 2 deletions src/sandboxes/ensureGithubRepo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ export async function ensureGithubRepo(

// Persist the repo URL via PATCH /api/sandboxes
logger.log("Persisting GitHub repo URL", { accountId, repoUrl });
const snapshotId = sandboxesInfo?.snapshotId ?? undefined;
await updateAccountSnapshot(accountId, snapshotId, repoUrl);
await updateAccountSnapshot(accountId, undefined, repoUrl);

githubRepo = repoUrl;
}
Expand Down
2 changes: 1 addition & 1 deletion src/sandboxes/getOrCreateSandbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function getOrCreateSandbox(
}

const sandbox = await Sandbox.get({
sandboxId: created.sandboxId,
name: created.sandboxId,
token,
teamId,
projectId,
Expand Down
1 change: 0 additions & 1 deletion src/sandboxes/notifyCodingAgentCallback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ interface CallbackPayload {
threadId: string;
status: "pr_created" | "no_changes" | "failed" | "updated";
branch?: string;
snapshotId?: string;
prs?: { repo: string; number: number; url: string; baseBranch: string }[];
message?: string;
stdout?: string;
Expand Down
30 changes: 10 additions & 20 deletions src/sandboxes/snapshotAndPersist.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,24 @@ import { logger } from "@trigger.dev/sdk/v3";
import { updateAccountSnapshot } from "../recoup/updateAccountSnapshot";

/**
* Takes a sandbox snapshot and persists the snapshotId (and optional
* github_repo) via the API.
* Persists sandbox metadata (github_repo) via the API.
*
* @param sandbox - The Vercel Sandbox instance
* With the Vercel Sandbox names feature, sandbox state persists automatically
* by name — explicit snapshots are no longer required for sandbox resumption.
*
* @param _sandbox - The Vercel Sandbox instance (unused with name-based persistence)
* @param accountId - The account ID to update
* @param githubRepo - Optional GitHub repo URL to persist alongside the snapshot
* @returns The snapshot result with snapshotId and expiresAt
* @param githubRepo - Optional GitHub repo URL to persist
*/
export async function snapshotAndPersist(
sandbox: Sandbox,
_sandbox: Sandbox,
accountId: string,
githubRepo?: string
) {
const SEVEN_DAYS_MS = 7 * 24 * 60 * 60 * 1000;

logger.log("Taking sandbox snapshot");
const snapshotResult = await sandbox.snapshot({ expiration: SEVEN_DAYS_MS });

logger.log("Snapshot created", {
snapshotId: snapshotResult.snapshotId,
expiresAt: snapshotResult.expiresAt,
});

logger.log("Persisting snapshot ID", {
logger.log("Persisting sandbox metadata (names replace snapshots)", {
accountId,
snapshotId: snapshotResult.snapshotId,
githubRepo,
});
await updateAccountSnapshot(accountId, snapshotResult.snapshotId, githubRepo);

return snapshotResult;
await updateAccountSnapshot(accountId, undefined, githubRepo);
}
1 change: 0 additions & 1 deletion src/schemas/codingAgentSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export type CodingAgentPayload = z.infer<typeof codingAgentPayloadSchema>;

export interface CodingAgentResult {
branch: string;
snapshotId: string;
prs: { repo: string; number: number; url: string; baseBranch: string }[];
stdout: string;
stderr: string;
Expand Down
8 changes: 0 additions & 8 deletions src/schemas/sandboxSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,10 @@ export type RunSandboxCommandPayload = z.infer<
typeof runSandboxCommandPayloadSchema
>;

export const snapshotSchema = z.object({
id: z.string(),
expiresAt: z.string(),
});

export type Snapshot = z.infer<typeof snapshotSchema>;

export const sandboxResultSchema = z.object({
stdout: z.string(),
stderr: z.string(),
exitCode: z.number(),
snapshot: snapshotSchema,
});

export type SandboxResult = z.infer<typeof sandboxResultSchema>;
1 change: 0 additions & 1 deletion src/schemas/updatePRSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { z } from "zod";

export const updatePRPayloadSchema = z.object({
feedback: z.string().min(1, "feedback is required"),
snapshotId: z.string().min(1, "snapshotId is required"),
branch: z.string().min(1, "branch is required"),
repo: z.string().min(1, "repo is required"),
callbackThreadId: z.string().min(1, "callbackThreadId is required"),
Expand Down
4 changes: 1 addition & 3 deletions src/tasks/__tests__/codingAgentTask.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ vi.mock("@trigger.dev/sdk/v3", () => ({
}));

const mockSandboxStop = vi.fn();
const mockSandboxSnapshot = vi.fn().mockResolvedValue({ snapshotId: "snap_123" });
const mockGetOrCreateSandbox = vi.fn();

vi.mock("../../sandboxes/getOrCreateSandbox", () => ({
Expand Down Expand Up @@ -60,9 +59,8 @@ beforeEach(() => {
mockGetOrCreateSandbox.mockResolvedValue({
sandboxId: "sbx-123",
sandbox: {
sandboxId: "sbx-123",
name: "sbx-123",
stop: mockSandboxStop,
snapshot: mockSandboxSnapshot,
},
});
});
Expand Down
8 changes: 2 additions & 6 deletions src/tasks/__tests__/setupSandboxTask.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ vi.mock("../../sandboxes/provisionSandbox", () => ({
}));

vi.mock("../../sandboxes/snapshotAndPersist", () => ({
snapshotAndPersist: vi.fn().mockResolvedValue({
snapshotId: "snap_abc",
expiresAt: new Date("2026-03-12"),
}),
snapshotAndPersist: vi.fn().mockResolvedValue(undefined),
}));

const { setupSandboxTask } = await import("../setupSandboxTask");
Expand Down Expand Up @@ -79,12 +76,11 @@ describe("setupSandboxTask", () => {
);
});

it("returns snapshotId and githubRepo on success", async () => {
it("returns githubRepo on success", async () => {
const result = await run({ accountId: "acc_1" });

expect(result).toEqual({
githubRepo: "https://github.com/org/repo",
snapshotId: "snap_abc",
});
});

Expand Down
32 changes: 8 additions & 24 deletions src/tasks/__tests__/updatePRTask.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@ vi.mock("@trigger.dev/sdk/v3", () => ({
},
}));

const mockSandboxCreate = vi.fn();
const mockSandboxGet = vi.fn();
const mockSandboxStop = vi.fn();
const mockSandboxSnapshot = vi.fn().mockResolvedValue({ snapshotId: "snap_new" });

vi.mock("@vercel/sandbox", () => ({
Sandbox: {
create: (...args: unknown[]) => mockSandboxCreate(...args),
get: (...args: unknown[]) => mockSandboxGet(...args),
},
}));

Expand Down Expand Up @@ -57,28 +56,26 @@ await import("../updatePRTask");

beforeEach(() => {
vi.clearAllMocks();
mockSandboxCreate.mockResolvedValue({
sandboxId: "sbx-456",
mockSandboxGet.mockResolvedValue({
name: CODING_AGENT_ACCOUNT_ID,
stop: mockSandboxStop,
snapshot: mockSandboxSnapshot,
});
});

describe("updatePRTask", () => {
const basePayload = {
feedback: "Make the button blue instead",
snapshotId: "snap_old",
branch: "agent/fix-bug-123",
repo: "recoupable/api",
callbackThreadId: "slack:C123:123.456",
};

it("resumes sandbox from snapshot", async () => {
it("resumes sandbox by name", async () => {
await mockRun(basePayload);

expect(mockSandboxCreate).toHaveBeenCalledWith(
expect(mockSandboxGet).toHaveBeenCalledWith(
expect.objectContaining({
source: { type: "snapshot", snapshotId: "snap_old" },
name: CODING_AGENT_ACCOUNT_ID,
}),
);
});
Expand Down Expand Up @@ -112,7 +109,7 @@ describe("updatePRTask", () => {
);
});

it("notifies callback with updated status and new snapshot", async () => {
it("notifies callback with updated status", async () => {
const { notifyCodingAgentCallback } = await import("../../sandboxes/notifyCodingAgentCallback");

await mockRun(basePayload);
Expand All @@ -121,7 +118,6 @@ describe("updatePRTask", () => {
expect.objectContaining({
threadId: "slack:C123:123.456",
status: "updated",
snapshotId: "snap_new",
stdout: "done",
stderr: "",
}),
Expand All @@ -141,18 +137,6 @@ describe("updatePRTask", () => {
expect(configureGitAuth).toHaveBeenCalledOnce();
});

it("creates sandbox with correct timeout param", async () => {
await mockRun(basePayload);

expect(mockSandboxCreate).toHaveBeenCalledWith(
expect.objectContaining({
timeout: 30 * 60 * 1000,
}),
);
const callArgs = mockSandboxCreate.mock.calls[0][0];
expect(callArgs).not.toHaveProperty("timeoutMs");
});

it("passes sandbox env to both agent calls", async () => {
const { runClaudeCodeAgent } = await import("../../sandboxes/runClaudeCodeAgent");
const { getSandboxEnv } = await import("../../sandboxes/getSandboxEnv");
Expand Down
Loading
Loading