Skip to content

Commit 0ee4499

Browse files
fix(tables): say which type a no-op column update restated
A copilot `update_column` payload whose only content was the column's current type used to return success with the live schema, while the v1, v2, and UI routes rejected the same payload with "No updates specified". Delegating to `performUpdateTableColumn` unified them onto the routes' rejection — correct, but the message tells the caller its request was empty when it named a type. The orchestration function now reports the same thing `updateColumnType` reports when it loses this race concurrently: the column is already that type, re-issue without the type change. An empty payload still reads "No updates specified". Drops the copilot's `outcome.table ?? tableForUpdate` fallback with it — the comment described the no-op that can no longer reach that line, and a success always carries a table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGzbVDZpe2dEALbu2BUU8a
1 parent ad29585 commit 0ee4499

3 files changed

Lines changed: 20 additions & 4 deletions

File tree

apps/sim/lib/copilot/tools/server/table/user-table.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1677,15 +1677,13 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
16771677
...(currencyCode !== undefined ? { currencyCode } : {}),
16781678
},
16791679
})
1680-
if (!outcome.success) {
1680+
if (!outcome.success || !outcome.table) {
16811681
return { success: false, message: outcome.error ?? 'Failed to update column' }
16821682
}
16831683
return {
16841684
success: true,
16851685
message: `Updated column "${colName}"`,
1686-
// A payload that only restates the current type is a no-op; still
1687-
// report the live schema rather than an undefined one.
1688-
data: { schema: (outcome.table ?? tableForUpdate).schema },
1686+
data: { schema: outcome.table.schema },
16891687
}
16901688
}
16911689
case 'rename': {

apps/sim/lib/table/orchestration/columns.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,18 @@ describe('performUpdateTableColumn', () => {
164164
const result = await run({})
165165

166166
expect(result).toMatchObject({ success: false, errorCode: 'validation' })
167+
expect(result.error).toBe('No updates specified')
167168
expect(mockRecordAudit).not.toHaveBeenCalled()
168169
})
169170

171+
it("names the type when a payload only restates the column's current type", async () => {
172+
const result = await run({ type: 'select' })
173+
174+
expect(result).toMatchObject({ success: false, errorCode: 'validation' })
175+
expect(result.error).toContain('is already type "select"')
176+
expect(mockUpdateColumnType).not.toHaveBeenCalled()
177+
})
178+
170179
it('classifies a table lock as locked and does not audit', async () => {
171180
mockUpdateColumnConstraints.mockRejectedValue(new TableLockedError('update'))
172181

apps/sim/lib/table/orchestration/columns.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,15 @@ export async function performUpdateTableColumn(
262262
}
263263

264264
if (!updated) {
265+
// A payload whose only content is the type the column already has names a
266+
// change and asks for nothing. Say which, the way `updateColumnType` does
267+
// when it loses the same race, rather than claiming the request was empty.
268+
if (updates.type !== undefined) {
269+
return fail(
270+
`Column "${currentColumn.name}" is already type "${currentColumn.type}"; re-issue the request without a type change.`,
271+
'validation'
272+
)
273+
}
265274
return fail('No updates specified', 'validation')
266275
}
267276

0 commit comments

Comments
 (0)