Skip to content

Commit ee7c647

Browse files
sidneyswiftclaude
sidneyswift
and
claude
authored
Test (#168)
* fix: handle duplicate room creation race condition (#165) * fix: handle duplicate room creation race condition Catch 23505 unique constraint error when frontend creates room before backend's selectRoom sees it. Skip notification if room already exists. * fix: keep Promise.all, just wrap in try/catch Simpler fix - keep parallel execution, only catch the 23505 error. * fix: handle duplicate room in createNewRoom.ts This is the actual fix - createNewRoom is called during setupConversation at request START, which is where the 500 error occurs. * fix: revert try/catch bandaid - no longer needed Frontend no longer creates rooms, so no race condition. Backend is single source of truth for room creation. * fix: use upsert instead of insert for room creation - Changed insertRoom to use upsert with ignoreDuplicates: true - Reverted try/catch in createNewRoom.ts and handleChatCompletion.ts - Cleaner solution: duplicates are silently ignored at the DB level * fix: use upsert instead of insert for rooms One-line fix: change insert to upsert in insertRoom.ts. Removes try/catch workarounds since upsert handles duplicates. * refactor: rename insertRoom to upsertRoom to match method Renames the function and file from insertRoom to upsertRoom to align with the actual Supabase method being called (.upsert()). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: add unit tests for upsertRoom function Adds comprehensive unit tests covering: - Basic upsert operation - Null artist_id handling - Null topic handling - Error handling on upsert failure - Upsert behavior (update on conflict) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> * perf: optimize bundle and parallelize sequential awaits (#161) * perf: optimize bundle and parallelize sequential awaits - Add optimizePackageImports for date-fns, @ai-sdk packages - Parallelize MCP tools and Composio tools fetching with Promise.all - Parallelize Trigger.dev schedule deletion and DB deletion in deleteTask These optimizations improve API response times by parallelizing independent operations. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: add unit tests for parallel execution optimizations - Add comprehensive tests for deleteTask function including: - Basic functionality (fetch, delete from DB, delete from Trigger.dev) - Error handling (task not found, propagated errors) - Handling tasks without trigger_schedule_id - Parallel execution verification - Add parallel execution tests for setupToolsForRequest: - Verify MCP and Composio tools are fetched concurrently - Verify both operations are called when authToken is provided Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 9404e3f commit ee7c647

File tree

5 files changed

+257
-12
lines changed

5 files changed

+257
-12
lines changed

‎lib/chat/__tests__/setupToolsForRequest.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,4 +215,67 @@ describe("setupToolsForRequest", () => {
215215
expect(result).toHaveProperty("tool2");
216216
});
217217
});
218+
219+
describe("parallel execution", () => {
220+
it("fetches MCP tools and Composio tools in parallel", async () => {
221+
const executionOrder: string[] = [];
222+
223+
// Track when each operation starts and completes
224+
mockGetMcpTools.mockImplementation(async () => {
225+
executionOrder.push("getMcpTools:start");
226+
await new Promise((resolve) => setTimeout(resolve, 10));
227+
executionOrder.push("getMcpTools:end");
228+
return { mcpTool: { description: "MCP Tool", parameters: {} } };
229+
});
230+
231+
mockGetComposioTools.mockImplementation(async () => {
232+
executionOrder.push("getComposioTools:start");
233+
await new Promise((resolve) => setTimeout(resolve, 10));
234+
executionOrder.push("getComposioTools:end");
235+
return { composioTool: { description: "Composio Tool", parameters: {} } };
236+
});
237+
238+
const body: ChatRequestBody = {
239+
accountId: "account-123",
240+
orgId: null,
241+
authToken: "test-token-123",
242+
messages: [{ id: "1", role: "user", content: "Hello" }],
243+
};
244+
245+
await setupToolsForRequest(body);
246+
247+
// Both should start before either ends (parallel execution)
248+
const mcpStartIndex = executionOrder.indexOf("getMcpTools:start");
249+
const composioStartIndex = executionOrder.indexOf("getComposioTools:start");
250+
const mcpEndIndex = executionOrder.indexOf("getMcpTools:end");
251+
const composioEndIndex = executionOrder.indexOf("getComposioTools:end");
252+
253+
// Both operations should have started
254+
expect(mcpStartIndex).toBeGreaterThanOrEqual(0);
255+
expect(composioStartIndex).toBeGreaterThanOrEqual(0);
256+
257+
// Both starts should come before both ends
258+
expect(mcpStartIndex).toBeLessThan(mcpEndIndex);
259+
expect(composioStartIndex).toBeLessThan(composioEndIndex);
260+
261+
// At least one start should come before the other's end (proves parallelism)
262+
const bothStartedBeforeAnyEnds =
263+
Math.max(mcpStartIndex, composioStartIndex) < Math.min(mcpEndIndex, composioEndIndex);
264+
expect(bothStartedBeforeAnyEnds).toBe(true);
265+
});
266+
267+
it("both operations are called when authToken is provided", async () => {
268+
const body: ChatRequestBody = {
269+
accountId: "account-123",
270+
orgId: null,
271+
authToken: "test-token-123",
272+
messages: [{ id: "1", role: "user", content: "Hello" }],
273+
};
274+
275+
await setupToolsForRequest(body);
276+
277+
expect(mockGetMcpTools).toHaveBeenCalledTimes(1);
278+
expect(mockGetComposioTools).toHaveBeenCalledTimes(1);
279+
});
280+
});
218281
});

‎lib/chat/setupToolsForRequest.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ import { getComposioTools } from "@/lib/composio/toolRouter";
1616
export async function setupToolsForRequest(body: ChatRequestBody): Promise<ToolSet> {
1717
const { accountId, roomId, excludeTools, authToken } = body;
1818

19-
// Only fetch MCP tools if we have an auth token
20-
const mcpTools = authToken ? await getMcpTools(authToken) : {};
21-
22-
// Get Composio Tool Router tools (COMPOSIO_MANAGE_CONNECTIONS, etc.)
23-
const composioTools = await getComposioTools(accountId, roomId);
19+
// Fetch MCP tools and Composio tools in parallel - they're independent
20+
const [mcpTools, composioTools] = await Promise.all([
21+
authToken ? getMcpTools(authToken) : Promise.resolve({}),
22+
getComposioTools(accountId, roomId),
23+
]);
2424

2525
// Merge all tools
2626
const allTools: ToolSet = {

‎lib/tasks/__tests__/deleteTask.test.ts‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
3+
// Mock external dependencies
4+
vi.mock("@/lib/supabase/scheduled_actions/selectScheduledActions", () => ({
5+
selectScheduledActions: vi.fn(),
6+
}));
7+
8+
vi.mock("@/lib/supabase/scheduled_actions/deleteScheduledAction", () => ({
9+
deleteScheduledAction: vi.fn(),
10+
}));
11+
12+
vi.mock("@/lib/trigger/deleteSchedule", () => ({
13+
deleteSchedule: vi.fn(),
14+
}));
15+
16+
// Import after mocks
17+
import { deleteTask } from "../deleteTask";
18+
import { selectScheduledActions } from "@/lib/supabase/scheduled_actions/selectScheduledActions";
19+
import { deleteScheduledAction } from "@/lib/supabase/scheduled_actions/deleteScheduledAction";
20+
import { deleteSchedule } from "@/lib/trigger/deleteSchedule";
21+
22+
const mockSelectScheduledActions = vi.mocked(selectScheduledActions);
23+
const mockDeleteScheduledAction = vi.mocked(deleteScheduledAction);
24+
const mockDeleteSchedule = vi.mocked(deleteSchedule);
25+
26+
describe("deleteTask", () => {
27+
const mockTaskId = "task-123";
28+
const mockScheduleId = "schedule-456";
29+
30+
const mockScheduledAction = {
31+
id: mockTaskId,
32+
account_id: "account-123",
33+
trigger_schedule_id: mockScheduleId,
34+
enabled: true,
35+
action_type: "email",
36+
action_params: {},
37+
cron_expression: "0 9 * * *",
38+
created_at: "2024-01-01T00:00:00Z",
39+
updated_at: "2024-01-01T00:00:00Z",
40+
artist_account_id: null,
41+
last_run_at: null,
42+
next_run_at: null,
43+
};
44+
45+
beforeEach(() => {
46+
vi.clearAllMocks();
47+
48+
// Default mocks
49+
mockSelectScheduledActions.mockResolvedValue([mockScheduledAction]);
50+
mockDeleteScheduledAction.mockResolvedValue();
51+
mockDeleteSchedule.mockResolvedValue();
52+
});
53+
54+
describe("basic functionality", () => {
55+
it("fetches the scheduled action by id", async () => {
56+
await deleteTask({ id: mockTaskId });
57+
58+
expect(mockSelectScheduledActions).toHaveBeenCalledWith({ id: mockTaskId });
59+
});
60+
61+
it("deletes the scheduled action from the database", async () => {
62+
await deleteTask({ id: mockTaskId });
63+
64+
expect(mockDeleteScheduledAction).toHaveBeenCalledWith(mockTaskId);
65+
});
66+
67+
it("deletes the Trigger.dev schedule when trigger_schedule_id exists", async () => {
68+
await deleteTask({ id: mockTaskId });
69+
70+
expect(mockDeleteSchedule).toHaveBeenCalledWith(mockScheduleId);
71+
});
72+
});
73+
74+
describe("error handling", () => {
75+
it("throws error when task is not found", async () => {
76+
mockSelectScheduledActions.mockResolvedValue([]);
77+
78+
await expect(deleteTask({ id: "non-existent" })).rejects.toThrow("Task not found");
79+
});
80+
81+
it("propagates error from selectScheduledActions", async () => {
82+
mockSelectScheduledActions.mockRejectedValue(new Error("Database error"));
83+
84+
await expect(deleteTask({ id: mockTaskId })).rejects.toThrow("Database error");
85+
});
86+
87+
it("propagates error from deleteSchedule", async () => {
88+
mockDeleteSchedule.mockRejectedValue(new Error("Trigger.dev error"));
89+
90+
await expect(deleteTask({ id: mockTaskId })).rejects.toThrow("Trigger.dev error");
91+
});
92+
93+
it("propagates error from deleteScheduledAction", async () => {
94+
mockDeleteScheduledAction.mockRejectedValue(new Error("Delete error"));
95+
96+
await expect(deleteTask({ id: mockTaskId })).rejects.toThrow("Delete error");
97+
});
98+
});
99+
100+
describe("handling tasks without trigger_schedule_id", () => {
101+
it("skips deleteSchedule when trigger_schedule_id is null", async () => {
102+
mockSelectScheduledActions.mockResolvedValue([
103+
{ ...mockScheduledAction, trigger_schedule_id: null },
104+
]);
105+
106+
await deleteTask({ id: mockTaskId });
107+
108+
expect(mockDeleteSchedule).not.toHaveBeenCalled();
109+
expect(mockDeleteScheduledAction).toHaveBeenCalledWith(mockTaskId);
110+
});
111+
112+
it("skips deleteSchedule when trigger_schedule_id is undefined", async () => {
113+
const taskWithoutScheduleId = { ...mockScheduledAction };
114+
// @ts-expect-error - Testing undefined case
115+
delete taskWithoutScheduleId.trigger_schedule_id;
116+
mockSelectScheduledActions.mockResolvedValue([taskWithoutScheduleId]);
117+
118+
await deleteTask({ id: mockTaskId });
119+
120+
expect(mockDeleteSchedule).not.toHaveBeenCalled();
121+
expect(mockDeleteScheduledAction).toHaveBeenCalledWith(mockTaskId);
122+
});
123+
});
124+
125+
describe("parallel execution", () => {
126+
it("executes deleteSchedule and deleteScheduledAction in parallel", async () => {
127+
const executionOrder: string[] = [];
128+
129+
// Track when each operation starts and completes
130+
mockDeleteSchedule.mockImplementation(async () => {
131+
executionOrder.push("deleteSchedule:start");
132+
await new Promise((resolve) => setTimeout(resolve, 10));
133+
executionOrder.push("deleteSchedule:end");
134+
});
135+
136+
mockDeleteScheduledAction.mockImplementation(async () => {
137+
executionOrder.push("deleteScheduledAction:start");
138+
await new Promise((resolve) => setTimeout(resolve, 10));
139+
executionOrder.push("deleteScheduledAction:end");
140+
});
141+
142+
await deleteTask({ id: mockTaskId });
143+
144+
// Both should start before either ends (parallel execution)
145+
const deleteScheduleStartIndex = executionOrder.indexOf("deleteSchedule:start");
146+
const deleteScheduledActionStartIndex = executionOrder.indexOf("deleteScheduledAction:start");
147+
const deleteScheduleEndIndex = executionOrder.indexOf("deleteSchedule:end");
148+
const deleteScheduledActionEndIndex = executionOrder.indexOf("deleteScheduledAction:end");
149+
150+
// Both operations should have started
151+
expect(deleteScheduleStartIndex).toBeGreaterThanOrEqual(0);
152+
expect(deleteScheduledActionStartIndex).toBeGreaterThanOrEqual(0);
153+
154+
// Both starts should come before both ends (parallel behavior)
155+
expect(deleteScheduleStartIndex).toBeLessThan(deleteScheduleEndIndex);
156+
expect(deleteScheduledActionStartIndex).toBeLessThan(deleteScheduledActionEndIndex);
157+
158+
// At least one start should come before the other's end (proves parallelism)
159+
const bothStartedBeforeAnyEnds =
160+
Math.max(deleteScheduleStartIndex, deleteScheduledActionStartIndex) <
161+
Math.min(deleteScheduleEndIndex, deleteScheduledActionEndIndex);
162+
expect(bothStartedBeforeAnyEnds).toBe(true);
163+
});
164+
165+
it("both operations are called even if they would fail", async () => {
166+
// This test verifies that both operations are initiated via Promise.all
167+
// by checking both mocks are called, not their order
168+
await deleteTask({ id: mockTaskId });
169+
170+
expect(mockDeleteSchedule).toHaveBeenCalledTimes(1);
171+
expect(mockDeleteScheduledAction).toHaveBeenCalledTimes(1);
172+
});
173+
});
174+
});

‎lib/tasks/deleteTask.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ export async function deleteTask(input: { id: string }): Promise<void> {
2020
throw new Error("Task not found");
2121
}
2222

23-
// Delete from Trigger.dev if schedule exists
24-
if (scheduledAction.trigger_schedule_id) {
25-
await deleteSchedule(scheduledAction.trigger_schedule_id);
26-
}
27-
28-
// Delete from database
29-
await deleteScheduledAction(id);
23+
// Delete from Trigger.dev and database in parallel - they're independent
24+
await Promise.all([
25+
scheduledAction.trigger_schedule_id
26+
? deleteSchedule(scheduledAction.trigger_schedule_id)
27+
: Promise.resolve(),
28+
deleteScheduledAction(id),
29+
]);
3030
}

‎next.config.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ const nextConfig: NextConfig = {
44
env: {
55
RESOURCE_WALLET_ADDRESS: process.env.RESOURCE_WALLET_ADDRESS,
66
},
7+
experimental: {
8+
optimizePackageImports: [
9+
'date-fns',
10+
'@ai-sdk/anthropic',
11+
'@ai-sdk/openai',
12+
'@ai-sdk/google',
13+
],
14+
},
715
};
816

917
export default nextConfig;

0 commit comments

Comments
 (0)