From 83666b4d2ae18218d21eebfab9a151faf8ce174d Mon Sep 17 00:00:00 2001 From: Recoup Agent Date: Mon, 9 Mar 2026 15:54:57 +0000 Subject: [PATCH 1/7] feat(coding-agent): persist snapshot on merge via upsertAccountSnapshot When the 'Merge All PRs' action button is clicked, the handler now calls upsertAccountSnapshot with the latest snapshotId from thread state and the coding-agent account ID, so new sandboxes start from the post-merge state. - Import upsertAccountSnapshot directly (no HTTP self-call) - Use CODING_AGENT_ACCOUNT_ID constant matching tasks codebase - Gracefully log errors without blocking merge result posting - Added tests: snapshot persistence, skip when no snapshotId, error handling --- .../__tests__/onMergeAction.test.ts | 74 ++++++++++++++++++- lib/coding-agent/handlers/onMergeAction.ts | 25 ++++++- 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/lib/coding-agent/__tests__/onMergeAction.test.ts b/lib/coding-agent/__tests__/onMergeAction.test.ts index f6007f79..a108f58a 100644 --- a/lib/coding-agent/__tests__/onMergeAction.test.ts +++ b/lib/coding-agent/__tests__/onMergeAction.test.ts @@ -7,11 +7,17 @@ vi.mock("../prState", () => ({ deleteCodingAgentPRState: (...args: unknown[]) => mockDeletePRState(...args), })); +const mockUpsertAccountSnapshot = vi.fn(); +vi.mock("@/lib/supabase/account_snapshots/upsertAccountSnapshot", () => ({ + upsertAccountSnapshot: (...args: unknown[]) => mockUpsertAccountSnapshot(...args), +})); + const { registerOnMergeAction } = await import("../handlers/onMergeAction"); beforeEach(() => { vi.clearAllMocks(); process.env.GITHUB_TOKEN = "ghp_test"; + mockUpsertAccountSnapshot.mockResolvedValue({ data: {}, error: null }); }); /** @@ -30,7 +36,7 @@ describe("registerOnMergeAction", () => { expect(bot.onAction).toHaveBeenCalledWith("merge_all_prs", expect.any(Function)); }); - it("squash-merges PRs, cleans up shared state, and posts results", async () => { + it("squash-merges PRs, cleans up shared state, persists snapshot, and posts results", async () => { vi.mocked(fetch).mockResolvedValue({ ok: true } as Response); const bot = createMockBot(); @@ -42,6 +48,7 @@ describe("registerOnMergeAction", () => { status: "pr_created", prompt: "fix bug", branch: "agent/fix-bug", + snapshotId: "snap_abc123", prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], }), post: vi.fn(), @@ -56,7 +63,71 @@ describe("registerOnMergeAction", () => { ); expect(mockThread.setState).toHaveBeenCalledWith({ status: "merged" }); expect(mockDeletePRState).toHaveBeenCalledWith("recoupable/api", "agent/fix-bug"); + expect(mockUpsertAccountSnapshot).toHaveBeenCalledWith({ + account_id: "coding-agent", + snapshot_id: "snap_abc123", + expires_at: expect.any(String), + }); + expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("merged")); + }); + + it("skips snapshot persistence when snapshotId is not in state", async () => { + vi.mocked(fetch).mockResolvedValue({ ok: true } as Response); + + const bot = createMockBot(); + registerOnMergeAction(bot); + const handler = bot.onAction.mock.calls[0][1]; + + const mockThread = { + state: Promise.resolve({ + status: "pr_created", + prompt: "fix bug", + branch: "agent/fix-bug", + prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], + }), + post: vi.fn(), + setState: vi.fn(), + }; + + await handler({ thread: mockThread }); + + expect(mockUpsertAccountSnapshot).not.toHaveBeenCalled(); + expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("merged")); + }); + + it("logs error but does not throw when snapshot persistence fails", async () => { + vi.mocked(fetch).mockResolvedValue({ ok: true } as Response); + mockUpsertAccountSnapshot.mockResolvedValue({ + data: null, + error: { message: "db error", code: "500" }, + }); + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const bot = createMockBot(); + registerOnMergeAction(bot); + const handler = bot.onAction.mock.calls[0][1]; + + const mockThread = { + state: Promise.resolve({ + status: "pr_created", + prompt: "fix bug", + branch: "agent/fix-bug", + snapshotId: "snap_abc123", + prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], + }), + post: vi.fn(), + setState: vi.fn(), + }; + + await handler({ thread: mockThread }); + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("failed to persist snapshot"), + expect.anything(), + ); + // Should still post merge results even if snapshot fails expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("merged")); + consoleSpy.mockRestore(); }); it("posts no PRs message when state has no PRs", async () => { @@ -74,5 +145,6 @@ describe("registerOnMergeAction", () => { expect(mockThread.post).toHaveBeenCalledWith("No PRs to merge."); expect(fetch).not.toHaveBeenCalled(); + expect(mockUpsertAccountSnapshot).not.toHaveBeenCalled(); }); }); diff --git a/lib/coding-agent/handlers/onMergeAction.ts b/lib/coding-agent/handlers/onMergeAction.ts index 4427926e..3974ad75 100644 --- a/lib/coding-agent/handlers/onMergeAction.ts +++ b/lib/coding-agent/handlers/onMergeAction.ts @@ -1,10 +1,15 @@ import type { CodingAgentBot } from "../bot"; import { deleteCodingAgentPRState } from "../prState"; import type { CodingAgentThreadState } from "../types"; +import { upsertAccountSnapshot } from "@/lib/supabase/account_snapshots/upsertAccountSnapshot"; + +const CODING_AGENT_ACCOUNT_ID = "coding-agent"; /** * Registers the "Merge All PRs" button action handler on the bot. - * Squash-merges each PR via the GitHub API. + * Squash-merges each PR via the GitHub API, then persists the latest + * snapshot via PATCH /api/sandboxes so the coding-agent account stays + * up-to-date. * * @param bot */ @@ -55,6 +60,24 @@ export function registerOnMergeAction(bot: CodingAgentBot) { if (state.branch && state.prs?.[0]?.repo) { await deleteCodingAgentPRState(state.prs[0].repo, state.branch); } + + // Persist the latest snapshot for the coding-agent account so new + // sandboxes start from the post-merge state. + if (state.snapshotId) { + const snapshotResult = await upsertAccountSnapshot({ + account_id: CODING_AGENT_ACCOUNT_ID, + snapshot_id: state.snapshotId, + expires_at: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), + }); + + if (snapshotResult.error) { + console.error( + `[coding-agent] failed to persist snapshot for ${CODING_AGENT_ACCOUNT_ID}:`, + snapshotResult.error, + ); + } + } + await thread.post(`Merge results:\n${results.map(r => `- ${r}`).join("\n")}`); }); } From 84e516d851fa2dd414857136fd73ac5004f9d4a8 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 9 Mar 2026 11:15:13 -0500 Subject: [PATCH 2/7] refactor: use RECOUP_ORG_ID from shared const for snapshot persistence Replace local CODING_AGENT_ACCOUNT_ID constant with the shared RECOUP_ORG_ID from lib/const.ts so snapshots are stored under the correct Recoup org account. Co-Authored-By: Claude Opus 4.6 --- lib/coding-agent/__tests__/onMergeAction.test.ts | 2 +- lib/coding-agent/handlers/onMergeAction.ts | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/coding-agent/__tests__/onMergeAction.test.ts b/lib/coding-agent/__tests__/onMergeAction.test.ts index a108f58a..ebb4dd4c 100644 --- a/lib/coding-agent/__tests__/onMergeAction.test.ts +++ b/lib/coding-agent/__tests__/onMergeAction.test.ts @@ -64,7 +64,7 @@ describe("registerOnMergeAction", () => { expect(mockThread.setState).toHaveBeenCalledWith({ status: "merged" }); expect(mockDeletePRState).toHaveBeenCalledWith("recoupable/api", "agent/fix-bug"); expect(mockUpsertAccountSnapshot).toHaveBeenCalledWith({ - account_id: "coding-agent", + account_id: "04e3aba9-c130-4fb8-8b92-34e95d43e66b", snapshot_id: "snap_abc123", expires_at: expect.any(String), }); diff --git a/lib/coding-agent/handlers/onMergeAction.ts b/lib/coding-agent/handlers/onMergeAction.ts index 3974ad75..fda7ff59 100644 --- a/lib/coding-agent/handlers/onMergeAction.ts +++ b/lib/coding-agent/handlers/onMergeAction.ts @@ -2,8 +2,7 @@ import type { CodingAgentBot } from "../bot"; import { deleteCodingAgentPRState } from "../prState"; import type { CodingAgentThreadState } from "../types"; import { upsertAccountSnapshot } from "@/lib/supabase/account_snapshots/upsertAccountSnapshot"; - -const CODING_AGENT_ACCOUNT_ID = "coding-agent"; +import { RECOUP_ORG_ID } from "@/lib/const"; /** * Registers the "Merge All PRs" button action handler on the bot. @@ -65,14 +64,14 @@ export function registerOnMergeAction(bot: CodingAgentBot) { // sandboxes start from the post-merge state. if (state.snapshotId) { const snapshotResult = await upsertAccountSnapshot({ - account_id: CODING_AGENT_ACCOUNT_ID, + account_id: RECOUP_ORG_ID, snapshot_id: state.snapshotId, expires_at: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), }); if (snapshotResult.error) { console.error( - `[coding-agent] failed to persist snapshot for ${CODING_AGENT_ACCOUNT_ID}:`, + `[coding-agent] failed to persist snapshot for ${RECOUP_ORG_ID}:`, snapshotResult.error, ); } From 61a85714d20ec2855e2e2bf46186f897bb718dfb Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 9 Mar 2026 11:29:28 -0500 Subject: [PATCH 3/7] fix: only persist snapshot when all PR merges succeed Skip snapshot upsert when any GitHub merge fails, and set thread state to "merge_failed" instead of "merged". Adds test for the merge failure case. Co-Authored-By: Claude Opus 4.6 --- .../__tests__/onMergeAction.test.ts | 32 +++++++++++++++++++ lib/coding-agent/handlers/onMergeAction.ts | 10 +++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/coding-agent/__tests__/onMergeAction.test.ts b/lib/coding-agent/__tests__/onMergeAction.test.ts index ebb4dd4c..aaeab845 100644 --- a/lib/coding-agent/__tests__/onMergeAction.test.ts +++ b/lib/coding-agent/__tests__/onMergeAction.test.ts @@ -130,6 +130,38 @@ describe("registerOnMergeAction", () => { consoleSpy.mockRestore(); }); + it("does not persist snapshot when a merge fails", async () => { + vi.mocked(fetch).mockResolvedValue({ + ok: false, + status: 409, + text: () => Promise.resolve(JSON.stringify({ message: "merge conflict" })), + } as unknown as Response); + + const bot = createMockBot(); + registerOnMergeAction(bot); + const handler = bot.onAction.mock.calls[0][1]; + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const mockThread = { + state: Promise.resolve({ + status: "pr_created", + prompt: "fix bug", + branch: "agent/fix-bug", + snapshotId: "snap_abc123", + prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], + }), + post: vi.fn(), + setState: vi.fn(), + }; + + await handler({ thread: mockThread }); + + expect(mockUpsertAccountSnapshot).not.toHaveBeenCalled(); + expect(mockThread.setState).toHaveBeenCalledWith({ status: "merge_failed" }); + expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("failed")); + consoleSpy.mockRestore(); + }); + it("posts no PRs message when state has no PRs", async () => { const bot = createMockBot(); registerOnMergeAction(bot); diff --git a/lib/coding-agent/handlers/onMergeAction.ts b/lib/coding-agent/handlers/onMergeAction.ts index fda7ff59..8f6f98fc 100644 --- a/lib/coding-agent/handlers/onMergeAction.ts +++ b/lib/coding-agent/handlers/onMergeAction.ts @@ -55,14 +55,16 @@ export function registerOnMergeAction(bot: CodingAgentBot) { } } - await thread.setState({ status: "merged" }); + const allMerged = results.every(r => r.endsWith("merged")); + + await thread.setState({ status: allMerged ? "merged" : "merge_failed" }); if (state.branch && state.prs?.[0]?.repo) { await deleteCodingAgentPRState(state.prs[0].repo, state.branch); } - // Persist the latest snapshot for the coding-agent account so new - // sandboxes start from the post-merge state. - if (state.snapshotId) { + // Persist the latest snapshot only when every PR merged successfully + // so new sandboxes start from the post-merge state. + if (allMerged && state.snapshotId) { const snapshotResult = await upsertAccountSnapshot({ account_id: RECOUP_ORG_ID, snapshot_id: state.snapshotId, From e5f9a1ddeda388a231577f83ef8d77363d2c77ab Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 9 Mar 2026 13:40:35 -0500 Subject: [PATCH 4/7] fix: revert status to pr_created on merge failure so replies still work When a merge fails, keep status as pr_created so handleFeedback continues accepting thread replies. Also skip PR state cleanup on failure since the user may retry. Co-Authored-By: Claude Opus 4.6 --- lib/coding-agent/__tests__/onMergeAction.test.ts | 3 ++- lib/coding-agent/handlers/onMergeAction.ts | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/coding-agent/__tests__/onMergeAction.test.ts b/lib/coding-agent/__tests__/onMergeAction.test.ts index aaeab845..52a78606 100644 --- a/lib/coding-agent/__tests__/onMergeAction.test.ts +++ b/lib/coding-agent/__tests__/onMergeAction.test.ts @@ -157,7 +157,8 @@ describe("registerOnMergeAction", () => { await handler({ thread: mockThread }); expect(mockUpsertAccountSnapshot).not.toHaveBeenCalled(); - expect(mockThread.setState).toHaveBeenCalledWith({ status: "merge_failed" }); + expect(mockDeletePRState).not.toHaveBeenCalled(); + expect(mockThread.setState).toHaveBeenCalledWith({ status: "pr_created" }); expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("failed")); consoleSpy.mockRestore(); }); diff --git a/lib/coding-agent/handlers/onMergeAction.ts b/lib/coding-agent/handlers/onMergeAction.ts index 8f6f98fc..5543cb28 100644 --- a/lib/coding-agent/handlers/onMergeAction.ts +++ b/lib/coding-agent/handlers/onMergeAction.ts @@ -57,8 +57,9 @@ export function registerOnMergeAction(bot: CodingAgentBot) { const allMerged = results.every(r => r.endsWith("merged")); - await thread.setState({ status: allMerged ? "merged" : "merge_failed" }); - if (state.branch && state.prs?.[0]?.repo) { + // On failure, revert to pr_created so handleFeedback still accepts replies + await thread.setState({ status: allMerged ? "merged" : "pr_created" }); + if (allMerged && state.branch && state.prs?.[0]?.repo) { await deleteCodingAgentPRState(state.prs[0].repo, state.branch); } From b7c594e9e723e87f9ff27a8fe736aaaae0bc1e46 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 9 Mar 2026 13:51:22 -0500 Subject: [PATCH 5/7] refactor: extract handleMergeSuccess for SRP Move PR state cleanup and snapshot persistence into a dedicated handleMergeSuccess function with its own tests. Simplify onMergeAction to delegate on success. Co-Authored-By: Claude Opus 4.6 --- .../__tests__/handleMergeSuccess.test.ts | 77 ++++++++++++++++ .../__tests__/onMergeAction.test.ts | 88 +++---------------- lib/coding-agent/handleMergeSuccess.ts | 29 ++++++ lib/coding-agent/handlers/onMergeAction.ts | 25 +----- 4 files changed, 119 insertions(+), 100 deletions(-) create mode 100644 lib/coding-agent/__tests__/handleMergeSuccess.test.ts create mode 100644 lib/coding-agent/handleMergeSuccess.ts diff --git a/lib/coding-agent/__tests__/handleMergeSuccess.test.ts b/lib/coding-agent/__tests__/handleMergeSuccess.test.ts new file mode 100644 index 00000000..41c33804 --- /dev/null +++ b/lib/coding-agent/__tests__/handleMergeSuccess.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const mockDeletePRState = vi.fn(); +vi.mock("../prState", () => ({ + deleteCodingAgentPRState: (...args: unknown[]) => mockDeletePRState(...args), +})); + +const mockUpsertAccountSnapshot = vi.fn(); +vi.mock("@/lib/supabase/account_snapshots/upsertAccountSnapshot", () => ({ + upsertAccountSnapshot: (...args: unknown[]) => mockUpsertAccountSnapshot(...args), +})); + +const { handleMergeSuccess } = await import("../handleMergeSuccess"); + +beforeEach(() => { + vi.clearAllMocks(); + mockUpsertAccountSnapshot.mockResolvedValue({ data: {}, error: null }); +}); + +describe("handleMergeSuccess", () => { + it("deletes PR state and persists snapshot", async () => { + await handleMergeSuccess({ + status: "pr_created", + branch: "agent/fix-bug", + snapshotId: "snap_abc123", + prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], + }); + + expect(mockDeletePRState).toHaveBeenCalledWith("recoupable/api", "agent/fix-bug"); + expect(mockUpsertAccountSnapshot).toHaveBeenCalledWith({ + account_id: "04e3aba9-c130-4fb8-8b92-34e95d43e66b", + snapshot_id: "snap_abc123", + expires_at: expect.any(String), + }); + }); + + it("skips snapshot persistence when snapshotId is not in state", async () => { + await handleMergeSuccess({ + status: "pr_created", + branch: "agent/fix-bug", + prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], + }); + + expect(mockDeletePRState).toHaveBeenCalledWith("recoupable/api", "agent/fix-bug"); + expect(mockUpsertAccountSnapshot).not.toHaveBeenCalled(); + }); + + it("logs error but does not throw when snapshot persistence fails", async () => { + mockUpsertAccountSnapshot.mockResolvedValue({ + data: null, + error: { message: "db error", code: "500" }, + }); + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + await handleMergeSuccess({ + status: "pr_created", + branch: "agent/fix-bug", + snapshotId: "snap_abc123", + prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], + }); + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("failed to persist snapshot"), + expect.anything(), + ); + consoleSpy.mockRestore(); + }); + + it("skips PR state cleanup when branch is missing", async () => { + await handleMergeSuccess({ + status: "pr_created", + prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], + }); + + expect(mockDeletePRState).not.toHaveBeenCalled(); + }); +}); diff --git a/lib/coding-agent/__tests__/onMergeAction.test.ts b/lib/coding-agent/__tests__/onMergeAction.test.ts index 52a78606..a67d31e6 100644 --- a/lib/coding-agent/__tests__/onMergeAction.test.ts +++ b/lib/coding-agent/__tests__/onMergeAction.test.ts @@ -2,14 +2,9 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; global.fetch = vi.fn(); -const mockDeletePRState = vi.fn(); -vi.mock("../prState", () => ({ - deleteCodingAgentPRState: (...args: unknown[]) => mockDeletePRState(...args), -})); - -const mockUpsertAccountSnapshot = vi.fn(); -vi.mock("@/lib/supabase/account_snapshots/upsertAccountSnapshot", () => ({ - upsertAccountSnapshot: (...args: unknown[]) => mockUpsertAccountSnapshot(...args), +const mockHandleMergeSuccess = vi.fn(); +vi.mock("../handleMergeSuccess", () => ({ + handleMergeSuccess: (...args: unknown[]) => mockHandleMergeSuccess(...args), })); const { registerOnMergeAction } = await import("../handlers/onMergeAction"); @@ -17,7 +12,7 @@ const { registerOnMergeAction } = await import("../handlers/onMergeAction"); beforeEach(() => { vi.clearAllMocks(); process.env.GITHUB_TOKEN = "ghp_test"; - mockUpsertAccountSnapshot.mockResolvedValue({ data: {}, error: null }); + mockHandleMergeSuccess.mockResolvedValue(undefined); }); /** @@ -36,7 +31,7 @@ describe("registerOnMergeAction", () => { expect(bot.onAction).toHaveBeenCalledWith("merge_all_prs", expect.any(Function)); }); - it("squash-merges PRs, cleans up shared state, persists snapshot, and posts results", async () => { + it("squash-merges PRs, calls handleMergeSuccess, and posts results", async () => { vi.mocked(fetch).mockResolvedValue({ ok: true } as Response); const bot = createMockBot(); @@ -62,75 +57,13 @@ describe("registerOnMergeAction", () => { expect.objectContaining({ method: "PUT" }), ); expect(mockThread.setState).toHaveBeenCalledWith({ status: "merged" }); - expect(mockDeletePRState).toHaveBeenCalledWith("recoupable/api", "agent/fix-bug"); - expect(mockUpsertAccountSnapshot).toHaveBeenCalledWith({ - account_id: "04e3aba9-c130-4fb8-8b92-34e95d43e66b", - snapshot_id: "snap_abc123", - expires_at: expect.any(String), - }); - expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("merged")); - }); - - it("skips snapshot persistence when snapshotId is not in state", async () => { - vi.mocked(fetch).mockResolvedValue({ ok: true } as Response); - - const bot = createMockBot(); - registerOnMergeAction(bot); - const handler = bot.onAction.mock.calls[0][1]; - - const mockThread = { - state: Promise.resolve({ - status: "pr_created", - prompt: "fix bug", - branch: "agent/fix-bug", - prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], - }), - post: vi.fn(), - setState: vi.fn(), - }; - - await handler({ thread: mockThread }); - - expect(mockUpsertAccountSnapshot).not.toHaveBeenCalled(); - expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("merged")); - }); - - it("logs error but does not throw when snapshot persistence fails", async () => { - vi.mocked(fetch).mockResolvedValue({ ok: true } as Response); - mockUpsertAccountSnapshot.mockResolvedValue({ - data: null, - error: { message: "db error", code: "500" }, - }); - const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - - const bot = createMockBot(); - registerOnMergeAction(bot); - const handler = bot.onAction.mock.calls[0][1]; - - const mockThread = { - state: Promise.resolve({ - status: "pr_created", - prompt: "fix bug", - branch: "agent/fix-bug", - snapshotId: "snap_abc123", - prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], - }), - post: vi.fn(), - setState: vi.fn(), - }; - - await handler({ thread: mockThread }); - - expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining("failed to persist snapshot"), - expect.anything(), + expect(mockHandleMergeSuccess).toHaveBeenCalledWith( + expect.objectContaining({ branch: "agent/fix-bug", snapshotId: "snap_abc123" }), ); - // Should still post merge results even if snapshot fails expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("merged")); - consoleSpy.mockRestore(); }); - it("does not persist snapshot when a merge fails", async () => { + it("does not call handleMergeSuccess when a merge fails", async () => { vi.mocked(fetch).mockResolvedValue({ ok: false, status: 409, @@ -156,8 +89,7 @@ describe("registerOnMergeAction", () => { await handler({ thread: mockThread }); - expect(mockUpsertAccountSnapshot).not.toHaveBeenCalled(); - expect(mockDeletePRState).not.toHaveBeenCalled(); + expect(mockHandleMergeSuccess).not.toHaveBeenCalled(); expect(mockThread.setState).toHaveBeenCalledWith({ status: "pr_created" }); expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("failed")); consoleSpy.mockRestore(); @@ -178,6 +110,6 @@ describe("registerOnMergeAction", () => { expect(mockThread.post).toHaveBeenCalledWith("No PRs to merge."); expect(fetch).not.toHaveBeenCalled(); - expect(mockUpsertAccountSnapshot).not.toHaveBeenCalled(); + expect(mockHandleMergeSuccess).not.toHaveBeenCalled(); }); }); diff --git a/lib/coding-agent/handleMergeSuccess.ts b/lib/coding-agent/handleMergeSuccess.ts new file mode 100644 index 00000000..44ff0fc6 --- /dev/null +++ b/lib/coding-agent/handleMergeSuccess.ts @@ -0,0 +1,29 @@ +import { deleteCodingAgentPRState } from "./prState"; +import { upsertAccountSnapshot } from "@/lib/supabase/account_snapshots/upsertAccountSnapshot"; +import { RECOUP_ORG_ID } from "@/lib/const"; +import type { CodingAgentThreadState } from "./types"; + +/** + * Handles post-merge cleanup after all PRs merged successfully. + * Deletes the shared PR state key and persists the latest snapshot. + */ +export async function handleMergeSuccess(state: CodingAgentThreadState): Promise { + if (state.branch && state.prs?.[0]?.repo) { + await deleteCodingAgentPRState(state.prs[0].repo, state.branch); + } + + if (state.snapshotId) { + const snapshotResult = await upsertAccountSnapshot({ + account_id: RECOUP_ORG_ID, + snapshot_id: state.snapshotId, + expires_at: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), + }); + + if (snapshotResult.error) { + console.error( + `[coding-agent] failed to persist snapshot for ${RECOUP_ORG_ID}:`, + snapshotResult.error, + ); + } + } +} diff --git a/lib/coding-agent/handlers/onMergeAction.ts b/lib/coding-agent/handlers/onMergeAction.ts index 5543cb28..dc83cd72 100644 --- a/lib/coding-agent/handlers/onMergeAction.ts +++ b/lib/coding-agent/handlers/onMergeAction.ts @@ -1,8 +1,6 @@ import type { CodingAgentBot } from "../bot"; -import { deleteCodingAgentPRState } from "../prState"; import type { CodingAgentThreadState } from "../types"; -import { upsertAccountSnapshot } from "@/lib/supabase/account_snapshots/upsertAccountSnapshot"; -import { RECOUP_ORG_ID } from "@/lib/const"; +import { handleMergeSuccess } from "../handleMergeSuccess"; /** * Registers the "Merge All PRs" button action handler on the bot. @@ -59,25 +57,8 @@ export function registerOnMergeAction(bot: CodingAgentBot) { // On failure, revert to pr_created so handleFeedback still accepts replies await thread.setState({ status: allMerged ? "merged" : "pr_created" }); - if (allMerged && state.branch && state.prs?.[0]?.repo) { - await deleteCodingAgentPRState(state.prs[0].repo, state.branch); - } - - // Persist the latest snapshot only when every PR merged successfully - // so new sandboxes start from the post-merge state. - if (allMerged && state.snapshotId) { - const snapshotResult = await upsertAccountSnapshot({ - account_id: RECOUP_ORG_ID, - snapshot_id: state.snapshotId, - expires_at: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), - }); - - if (snapshotResult.error) { - console.error( - `[coding-agent] failed to persist snapshot for ${RECOUP_ORG_ID}:`, - snapshotResult.error, - ); - } + if (allMerged) { + await handleMergeSuccess(state); } await thread.post(`Merge results:\n${results.map(r => `- ${r}`).join("\n")}`); From 4071cb649376bfa7a908c42926409f904b74101c Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 9 Mar 2026 14:20:00 -0500 Subject: [PATCH 6/7] fix: improve handleMergeSuccess error handling and multi-repo cleanup - Wrap post-merge cleanup in try/catch so merge results always post - Delete PR state for all repos, not just the first - Extract SNAPSHOT_EXPIRY_MS constant - Fix doc comment to match implementation Co-Authored-By: Claude Opus 4.6 --- .../__tests__/handleMergeSuccess.test.ts | 36 ++++++++++++++++++ lib/coding-agent/handleMergeSuccess.ts | 38 +++++++++++-------- lib/coding-agent/handlers/onMergeAction.ts | 5 +-- 3 files changed, 61 insertions(+), 18 deletions(-) diff --git a/lib/coding-agent/__tests__/handleMergeSuccess.test.ts b/lib/coding-agent/__tests__/handleMergeSuccess.test.ts index 41c33804..a5f99df9 100644 --- a/lib/coding-agent/__tests__/handleMergeSuccess.test.ts +++ b/lib/coding-agent/__tests__/handleMergeSuccess.test.ts @@ -34,6 +34,22 @@ describe("handleMergeSuccess", () => { }); }); + it("deletes PR state for all repos when PRs span multiple repos", async () => { + await handleMergeSuccess({ + status: "pr_created", + branch: "agent/fix-bug", + prs: [ + { repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }, + { repo: "recoupable/chat", number: 10, url: "url", baseBranch: "test" }, + { repo: "recoupable/api", number: 43, url: "url", baseBranch: "test" }, + ], + }); + + expect(mockDeletePRState).toHaveBeenCalledTimes(2); + expect(mockDeletePRState).toHaveBeenCalledWith("recoupable/api", "agent/fix-bug"); + expect(mockDeletePRState).toHaveBeenCalledWith("recoupable/chat", "agent/fix-bug"); + }); + it("skips snapshot persistence when snapshotId is not in state", async () => { await handleMergeSuccess({ status: "pr_created", @@ -66,6 +82,26 @@ describe("handleMergeSuccess", () => { consoleSpy.mockRestore(); }); + it("does not throw when deleteCodingAgentPRState throws", async () => { + mockDeletePRState.mockRejectedValue(new Error("Redis connection failed")); + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + await expect( + handleMergeSuccess({ + status: "pr_created", + branch: "agent/fix-bug", + snapshotId: "snap_abc123", + prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }], + }), + ).resolves.toBeUndefined(); + + expect(consoleSpy).toHaveBeenCalledWith( + "[coding-agent] post-merge cleanup failed:", + expect.any(Error), + ); + consoleSpy.mockRestore(); + }); + it("skips PR state cleanup when branch is missing", async () => { await handleMergeSuccess({ status: "pr_created", diff --git a/lib/coding-agent/handleMergeSuccess.ts b/lib/coding-agent/handleMergeSuccess.ts index 44ff0fc6..c9e16070 100644 --- a/lib/coding-agent/handleMergeSuccess.ts +++ b/lib/coding-agent/handleMergeSuccess.ts @@ -3,27 +3,35 @@ import { upsertAccountSnapshot } from "@/lib/supabase/account_snapshots/upsertAc import { RECOUP_ORG_ID } from "@/lib/const"; import type { CodingAgentThreadState } from "./types"; +const SNAPSHOT_EXPIRY_MS = 365 * 24 * 60 * 60 * 1000; // 1 year + /** * Handles post-merge cleanup after all PRs merged successfully. - * Deletes the shared PR state key and persists the latest snapshot. + * Deletes the shared PR state keys for all repos and persists the latest + * snapshot via upsertAccountSnapshot. */ export async function handleMergeSuccess(state: CodingAgentThreadState): Promise { - if (state.branch && state.prs?.[0]?.repo) { - await deleteCodingAgentPRState(state.prs[0].repo, state.branch); - } + try { + if (state.branch && state.prs?.length) { + const repos = [...new Set(state.prs.map(pr => pr.repo))]; + await Promise.all(repos.map(repo => deleteCodingAgentPRState(repo, state.branch!))); + } - if (state.snapshotId) { - const snapshotResult = await upsertAccountSnapshot({ - account_id: RECOUP_ORG_ID, - snapshot_id: state.snapshotId, - expires_at: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), - }); + if (state.snapshotId) { + const snapshotResult = await upsertAccountSnapshot({ + account_id: RECOUP_ORG_ID, + snapshot_id: state.snapshotId, + expires_at: new Date(Date.now() + SNAPSHOT_EXPIRY_MS).toISOString(), + }); - if (snapshotResult.error) { - console.error( - `[coding-agent] failed to persist snapshot for ${RECOUP_ORG_ID}:`, - snapshotResult.error, - ); + if (snapshotResult.error) { + console.error( + `[coding-agent] failed to persist snapshot for ${RECOUP_ORG_ID}:`, + snapshotResult.error, + ); + } } + } catch (error) { + console.error("[coding-agent] post-merge cleanup failed:", error); } } diff --git a/lib/coding-agent/handlers/onMergeAction.ts b/lib/coding-agent/handlers/onMergeAction.ts index dc83cd72..3283d2c9 100644 --- a/lib/coding-agent/handlers/onMergeAction.ts +++ b/lib/coding-agent/handlers/onMergeAction.ts @@ -4,9 +4,8 @@ import { handleMergeSuccess } from "../handleMergeSuccess"; /** * Registers the "Merge All PRs" button action handler on the bot. - * Squash-merges each PR via the GitHub API, then persists the latest - * snapshot via PATCH /api/sandboxes so the coding-agent account stays - * up-to-date. + * Squash-merges each PR via the GitHub API, then delegates to + * handleMergeSuccess to clean up PR state and persist the latest snapshot. * * @param bot */ From 59d448ce1229fa0ce559bc530e5aea6931abdba9 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 9 Mar 2026 14:22:19 -0500 Subject: [PATCH 7/7] refactor: move SNAPSHOT_EXPIRY_MS to shared const and set to 7 days Extract snapshot expiration duration to lib/const.ts and update both handleMergeSuccess and updateSnapshotPatchHandler to use it. Changed from 1 year to 7 days. Co-Authored-By: Claude Opus 4.6 --- lib/coding-agent/handleMergeSuccess.ts | 4 +--- lib/const.ts | 3 +++ lib/sandbox/updateSnapshotPatchHandler.ts | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/coding-agent/handleMergeSuccess.ts b/lib/coding-agent/handleMergeSuccess.ts index c9e16070..f026f48d 100644 --- a/lib/coding-agent/handleMergeSuccess.ts +++ b/lib/coding-agent/handleMergeSuccess.ts @@ -1,10 +1,8 @@ import { deleteCodingAgentPRState } from "./prState"; import { upsertAccountSnapshot } from "@/lib/supabase/account_snapshots/upsertAccountSnapshot"; -import { RECOUP_ORG_ID } from "@/lib/const"; +import { RECOUP_ORG_ID, SNAPSHOT_EXPIRY_MS } from "@/lib/const"; import type { CodingAgentThreadState } from "./types"; -const SNAPSHOT_EXPIRY_MS = 365 * 24 * 60 * 60 * 1000; // 1 year - /** * Handles post-merge cleanup after all PRs merged successfully. * Deletes the shared PR state keys for all repos and persists the latest diff --git a/lib/const.ts b/lib/const.ts index 545dbbeb..f495d63c 100644 --- a/lib/const.ts +++ b/lib/const.ts @@ -35,6 +35,9 @@ export const RECOUP_API_KEY = process.env.RECOUP_API_KEY || ""; export const FLAMINGO_GENERATE_URL = "https://sidney-78147--music-flamingo-musicflamingo-generate.modal.run"; +/** Snapshot expiration duration (7 days) */ +export const SNAPSHOT_EXPIRY_MS = 7 * 24 * 60 * 60 * 1000; + // EVALS export const EVAL_ACCOUNT_ID = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; export const EVAL_ACCESS_TOKEN = process.env.EVAL_ACCESS_TOKEN || ""; diff --git a/lib/sandbox/updateSnapshotPatchHandler.ts b/lib/sandbox/updateSnapshotPatchHandler.ts index ecf79afd..4d5da44d 100644 --- a/lib/sandbox/updateSnapshotPatchHandler.ts +++ b/lib/sandbox/updateSnapshotPatchHandler.ts @@ -3,6 +3,7 @@ import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateSnapshotPatchBody } from "@/lib/sandbox/validateSnapshotPatchBody"; import { upsertAccountSnapshot } from "@/lib/supabase/account_snapshots/upsertAccountSnapshot"; +import { SNAPSHOT_EXPIRY_MS } from "@/lib/const"; import { selectAccountSnapshots } from "@/lib/supabase/account_snapshots/selectAccountSnapshots"; /** @@ -30,7 +31,7 @@ export async function updateSnapshotPatchHandler(request: NextRequest): Promise< account_id: validated.accountId, ...(validated.snapshotId && { snapshot_id: validated.snapshotId, - expires_at: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), + expires_at: new Date(Date.now() + SNAPSHOT_EXPIRY_MS).toISOString(), }), ...(validated.githubRepo && { github_repo: validated.githubRepo }), });