Skip to content

Commit 4c02fa1

Browse files
waleedlatif1waleedlatif1
authored
fix(custom-tools): skip title conflicts when persisting inline tools from a workflow save (#9198)
* fix(custom-tools): skip title conflicts when persisting inline tools from a workflow save * fix(custom-tools): count skipped titles and skip title conflicts atomically
1 parent c1c0843 commit 4c02fa1

4 files changed

Lines changed: 299 additions & 80 deletions

File tree

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

Lines changed: 132 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,13 @@ import {
1717
} from '@/lib/api/list-query'
1818
import { generateRequestId } from '@/lib/core/utils/request'
1919
import { assertValidCustomToolDeclaration } from '@/lib/custom-tools/schema'
20+
import type { DbTransaction } from '@/lib/db/types'
2021

2122
const logger = createLogger('CustomToolsOperations')
2223

2324
export type CustomToolSortBy = 'title' | 'createdAt' | 'updatedAt'
2425

25-
/**
26-
* Internal function to create/update custom tools
27-
* Can be called from API routes or internal services
28-
*/
29-
export async function upsertCustomTools(params: {
26+
interface CustomToolWriteParams {
3027
tools: Array<{
3128
id?: string
3229
title: string
@@ -36,101 +33,162 @@ export async function upsertCustomTools(params: {
3633
workspaceId: string
3734
userId: string
3835
requestId?: string
39-
}) {
40-
const { tools, workspaceId, userId, requestId = generateRequestId() } = params
36+
}
4137

42-
/**
43-
* Ahead of the transaction so a batch is rejected whole rather than landing
44-
* the tools that preceded the unstorable one.
45-
*/
38+
/**
39+
* What a batch write does with a tool it would insert under a title the
40+
* workspace already holds: `'error'` throws and rolls back the whole batch,
41+
* `'skip'` leaves the existing row untouched and writes the rest.
42+
*/
43+
type CustomToolTitleConflictPolicy = 'error' | 'skip'
44+
45+
/**
46+
* Ahead of the transaction so a batch is rejected whole rather than landing
47+
* the tools that preceded the unstorable one.
48+
*/
49+
function assertStorableBatch(tools: CustomToolWriteParams['tools']) {
4650
for (const tool of tools) {
4751
assertValidCustomToolDeclaration(tool.schema)
4852
}
53+
}
4954

50-
return await db.transaction(async (tx) => {
51-
for (const tool of tools) {
52-
const nowTime = new Date()
55+
/** Writes the batch inside `tx`, returning the titles skipped under the `'skip'` policy. */
56+
async function writeCustomTools(
57+
tx: DbTransaction,
58+
params: CustomToolWriteParams,
59+
onTitleConflict: CustomToolTitleConflictPolicy
60+
): Promise<string[]> {
61+
const { tools, workspaceId, userId, requestId = generateRequestId() } = params
62+
const skippedTitles: string[] = []
63+
64+
for (const tool of tools) {
65+
const nowTime = new Date()
5366

54-
if (tool.id) {
55-
const existingWorkspaceTool = await tx
56-
.select()
57-
.from(customTools)
67+
if (tool.id) {
68+
const existingWorkspaceTool = await tx
69+
.select()
70+
.from(customTools)
71+
.where(and(eq(customTools.id, tool.id), eq(customTools.workspaceId, workspaceId)))
72+
.limit(1)
73+
74+
if (existingWorkspaceTool.length > 0) {
75+
await tx
76+
.update(customTools)
77+
.set({
78+
title: tool.title,
79+
schema: tool.schema,
80+
code: tool.code,
81+
updatedAt: nowTime,
82+
})
5883
.where(and(eq(customTools.id, tool.id), eq(customTools.workspaceId, workspaceId)))
59-
.limit(1)
60-
61-
if (existingWorkspaceTool.length > 0) {
62-
await tx
63-
.update(customTools)
64-
.set({
65-
title: tool.title,
66-
schema: tool.schema,
67-
code: tool.code,
68-
updatedAt: nowTime,
69-
})
70-
.where(and(eq(customTools.id, tool.id), eq(customTools.workspaceId, workspaceId)))
71-
continue
72-
}
73-
74-
const existingLegacyTool = await tx
75-
.select()
76-
.from(customTools)
77-
.where(
78-
and(
79-
eq(customTools.id, tool.id),
80-
isNull(customTools.workspaceId),
81-
eq(customTools.userId, userId)
82-
)
83-
)
84-
.limit(1)
85-
86-
if (existingLegacyTool.length > 0) {
87-
await tx
88-
.update(customTools)
89-
.set({
90-
title: tool.title,
91-
schema: tool.schema,
92-
code: tool.code,
93-
updatedAt: nowTime,
94-
})
95-
.where(eq(customTools.id, tool.id))
96-
97-
logger.info(`[${requestId}] Updated legacy tool ${tool.id}`)
98-
continue
99-
}
84+
continue
10085
}
10186

102-
const duplicateTitle = await tx
87+
const existingLegacyTool = await tx
10388
.select()
10489
.from(customTools)
105-
.where(and(eq(customTools.workspaceId, workspaceId), eq(customTools.title, tool.title)))
90+
.where(
91+
and(
92+
eq(customTools.id, tool.id),
93+
isNull(customTools.workspaceId),
94+
eq(customTools.userId, userId)
95+
)
96+
)
10697
.limit(1)
10798

108-
if (duplicateTitle.length > 0) {
109-
throw new Error(`A tool with the title "${tool.title}" already exists in this workspace`)
99+
if (existingLegacyTool.length > 0) {
100+
await tx
101+
.update(customTools)
102+
.set({
103+
title: tool.title,
104+
schema: tool.schema,
105+
code: tool.code,
106+
updatedAt: nowTime,
107+
})
108+
.where(eq(customTools.id, tool.id))
109+
110+
logger.info(`[${requestId}] Updated legacy tool ${tool.id}`)
111+
continue
112+
}
113+
}
114+
115+
const row = {
116+
id: generateShortId(),
117+
workspaceId,
118+
userId,
119+
title: tool.title,
120+
schema: tool.schema,
121+
code: tool.code,
122+
createdAt: nowTime,
123+
updatedAt: nowTime,
124+
}
125+
126+
if (onTitleConflict === 'skip') {
127+
/** A conflict-target insert, so a title a concurrent save just committed is skipped too. */
128+
const inserted = await tx
129+
.insert(customTools)
130+
.values(row)
131+
.onConflictDoNothing({ target: [customTools.workspaceId, customTools.title] })
132+
.returning({ id: customTools.id })
133+
if (inserted.length === 0) {
134+
logger.info(`[${requestId}] Skipped custom tool "${tool.title}": title already exists`)
135+
skippedTitles.push(tool.title)
110136
}
137+
continue
138+
}
111139

112-
await tx.insert(customTools).values({
113-
id: generateShortId(),
114-
workspaceId,
115-
userId,
116-
title: tool.title,
117-
schema: tool.schema,
118-
code: tool.code,
119-
createdAt: nowTime,
120-
updatedAt: nowTime,
121-
})
140+
const duplicateTitle = await tx
141+
.select()
142+
.from(customTools)
143+
.where(and(eq(customTools.workspaceId, workspaceId), eq(customTools.title, tool.title)))
144+
.limit(1)
145+
146+
if (duplicateTitle.length > 0) {
147+
throw new Error(`A tool with the title "${tool.title}" already exists in this workspace`)
122148
}
123149

150+
await tx.insert(customTools).values(row)
151+
}
152+
153+
return skippedTitles
154+
}
155+
156+
/**
157+
* Internal function to create/update custom tools
158+
* Can be called from API routes or internal services
159+
*/
160+
export async function upsertCustomTools(params: CustomToolWriteParams) {
161+
assertStorableBatch(params.tools)
162+
163+
return await db.transaction(async (tx) => {
164+
await writeCustomTools(tx, params, 'error')
165+
124166
const resultTools = await tx
125167
.select()
126168
.from(customTools)
127-
.where(eq(customTools.workspaceId, workspaceId))
169+
.where(eq(customTools.workspaceId, params.workspaceId))
128170
.orderBy(desc(customTools.createdAt))
129171

130172
return resultTools
131173
})
132174
}
133175

176+
/**
177+
* `upsertCustomTools` for writes that re-offer tools they may already have
178+
* stored (workflow saves and imports): a tool whose title the workspace already
179+
* holds is skipped, leaving that row untouched, instead of failing the batch.
180+
*
181+
* @returns The titles that were skipped.
182+
*/
183+
export async function upsertCustomToolsSkippingTakenTitles(
184+
params: CustomToolWriteParams
185+
): Promise<{ skippedTitles: string[] }> {
186+
assertStorableBatch(params.tools)
187+
188+
const skippedTitles = await db.transaction((tx) => writeCustomTools(tx, params, 'skip'))
189+
return { skippedTitles }
190+
}
191+
134192
export async function listCustomTools(params: { userId: string; workspaceId?: string }) {
135193
const { userId, workspaceId } = params
136194
return workspaceId

0 commit comments

Comments
 (0)