Skip to content

Commit ab9e2ae

Browse files
committed
fix(custom-tools): skip title conflicts when persisting inline tools from a workflow save
1 parent 3aad201 commit ab9e2ae

3 files changed

Lines changed: 168 additions & 1 deletion

File tree

‎apps/sim/lib/workflows/custom-tools/operations.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ const logger = createLogger('CustomToolsOperations')
2222

2323
export type CustomToolSortBy = 'title' | 'createdAt' | 'updatedAt'
2424

25+
/**
26+
* What `upsertCustomTools` does with a tool it would insert under a title the
27+
* workspace already holds: `'error'` throws and rolls back the whole batch,
28+
* `'skip'` leaves the existing row untouched and persists the rest.
29+
*/
30+
type CustomToolTitleConflictPolicy = 'error' | 'skip'
31+
2532
/**
2633
* Internal function to create/update custom tools
2734
* Can be called from API routes or internal services
@@ -36,8 +43,15 @@ export async function upsertCustomTools(params: {
3643
workspaceId: string
3744
userId: string
3845
requestId?: string
46+
onTitleConflict?: CustomToolTitleConflictPolicy
3947
}) {
40-
const { tools, workspaceId, userId, requestId = generateRequestId() } = params
48+
const {
49+
tools,
50+
workspaceId,
51+
userId,
52+
requestId = generateRequestId(),
53+
onTitleConflict = 'error',
54+
} = params
4155

4256
/**
4357
* Ahead of the transaction so a batch is rejected whole rather than landing
@@ -106,6 +120,10 @@ export async function upsertCustomTools(params: {
106120
.limit(1)
107121

108122
if (duplicateTitle.length > 0) {
123+
if (onTitleConflict === 'skip') {
124+
logger.info(`[${requestId}] Skipped custom tool "${tool.title}": title already exists`)
125+
continue
126+
}
109127
throw new Error(`A tool with the title "${tool.title}" already exists in this workspace`)
110128
}
111129

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/** Workflow-save persistence of inline custom tools, against real PostgreSQL. */
2+
import { db } from '@sim/db'
3+
import { customTools, user, workspace } from '@sim/db/schema'
4+
import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures'
5+
import { generateId } from '@sim/utils/id'
6+
import { asc, eq } from 'drizzle-orm'
7+
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'
8+
import { upsertCustomTools } from '@/lib/workflows/custom-tools/operations'
9+
import { extractAndPersistCustomTools } from '@/lib/workflows/persistence/custom-tools-persistence'
10+
11+
const owner = `custom-tools-owner-${generateId()}`
12+
const workspaceId = generateId()
13+
14+
function declaration(name: string) {
15+
return {
16+
type: 'function' as const,
17+
function: {
18+
name,
19+
description: `Runs ${name}`,
20+
parameters: { type: 'object', properties: { id: { type: 'string' } }, required: ['id'] },
21+
},
22+
}
23+
}
24+
25+
/** An agent block carrying inline custom tools with no `toolId`, as an imported graph does. */
26+
function workflowStateWithInlineTools(tools: Array<{ title: string; code: string }>) {
27+
return {
28+
blocks: {
29+
agent: {
30+
type: 'agent',
31+
subBlocks: {
32+
tools: {
33+
value: tools.map((tool) => ({
34+
type: 'custom-tool',
35+
title: tool.title,
36+
schema: declaration(tool.title.toLowerCase().replace(/\W+/g, '_')),
37+
code: tool.code,
38+
})),
39+
},
40+
},
41+
},
42+
},
43+
}
44+
}
45+
46+
async function workspaceTools() {
47+
return db
48+
.select({ title: customTools.title, code: customTools.code })
49+
.from(customTools)
50+
.where(eq(customTools.workspaceId, workspaceId))
51+
.orderBy(asc(customTools.title))
52+
}
53+
54+
beforeAll(async () => {
55+
const now = new Date()
56+
await db.insert(user).values({
57+
id: owner,
58+
name: 'Custom Tools',
59+
email: `${owner}@custom-tools.test`,
60+
emailVerified: true,
61+
createdAt: now,
62+
updatedAt: now,
63+
})
64+
await insertWorkspaceFixture(db, [
65+
{ id: workspaceId, name: 'Custom Tools', ownerId: owner, billedAccountUserId: owner },
66+
])
67+
})
68+
69+
beforeEach(async () => {
70+
await db.delete(customTools).where(eq(customTools.workspaceId, workspaceId))
71+
})
72+
73+
afterAll(async () => {
74+
await db.delete(customTools).where(eq(customTools.workspaceId, workspaceId))
75+
await deleteWorkspaceFixture(db, eq(workspace.id, workspaceId))
76+
await db.delete(user).where(eq(user.id, owner))
77+
})
78+
79+
describe('extractAndPersistCustomTools', () => {
80+
it('re-saves a graph whose inline tool has no id and still persists the new tools beside it', async () => {
81+
const first = await extractAndPersistCustomTools(
82+
workflowStateWithInlineTools([{ title: 'Lookup order', code: 'return 1' }]),
83+
workspaceId,
84+
owner
85+
)
86+
expect(first.errors).toEqual([])
87+
88+
const second = await extractAndPersistCustomTools(
89+
workflowStateWithInlineTools([
90+
{ title: 'Lookup order', code: 'return 1' },
91+
{ title: 'Refund order', code: 'return 2' },
92+
]),
93+
workspaceId,
94+
owner
95+
)
96+
97+
expect(second.errors).toEqual([])
98+
expect(await workspaceTools()).toEqual([
99+
{ title: 'Lookup order', code: 'return 1' },
100+
{ title: 'Refund order', code: 'return 2' },
101+
])
102+
})
103+
104+
it('leaves an existing workspace tool with the same title untouched', async () => {
105+
await upsertCustomTools({
106+
tools: [{ title: 'Lookup order', schema: declaration('lookup'), code: 'return "library"' }],
107+
workspaceId,
108+
userId: owner,
109+
})
110+
111+
const result = await extractAndPersistCustomTools(
112+
workflowStateWithInlineTools([{ title: 'Lookup order', code: 'return "inline"' }]),
113+
workspaceId,
114+
owner
115+
)
116+
117+
expect(result.errors).toEqual([])
118+
expect(await workspaceTools()).toEqual([{ title: 'Lookup order', code: 'return "library"' }])
119+
})
120+
})
121+
122+
describe('upsertCustomTools', () => {
123+
it('rejects a create under a taken title and rolls back the rest of the batch', async () => {
124+
await upsertCustomTools({
125+
tools: [{ title: 'Lookup order', schema: declaration('lookup'), code: 'return 1' }],
126+
workspaceId,
127+
userId: owner,
128+
})
129+
130+
await expect(
131+
upsertCustomTools({
132+
tools: [
133+
{ title: 'Refund order', schema: declaration('refund'), code: 'return 2' },
134+
{ title: 'Lookup order', schema: declaration('lookup'), code: 'return 3' },
135+
],
136+
workspaceId,
137+
userId: owner,
138+
})
139+
).rejects.toThrow('A tool with the title "Lookup order" already exists in this workspace')
140+
expect(await workspaceTools()).toEqual([{ title: 'Lookup order', code: 'return 1' }])
141+
})
142+
})

‎apps/sim/lib/workflows/persistence/custom-tools-persistence.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ export function extractCustomToolsFromWorkflowState(workflowState: any): CustomT
116116
/**
117117
* Persist custom tools to the database using the upsert function
118118
* Creates new tools or updates existing ones
119+
*
120+
* @remarks
121+
* An inline tool carries no `toolId` until the block is re-saved with one, so
122+
* every save of the same graph re-offers it under a title its first save
123+
* already inserted. A title conflict therefore skips that tool, leaving the
124+
* existing row untouched, instead of rolling back the rest of the batch.
119125
*/
120126
export async function persistCustomToolsToDatabase(
121127
customToolsList: CustomTool[],
@@ -167,6 +173,7 @@ export async function persistCustomToolsToDatabase(
167173
})),
168174
workspaceId,
169175
userId,
176+
onTitleConflict: 'skip',
170177
})
171178

172179
saved = validTools.length

0 commit comments

Comments
 (0)