Commit 5fea5f7
refactor(tables): make lib/table/orchestration the single implementation (#6134)
* refactor(orchestration): move the shared error contract out of lib/workflows
OrchestrationErrorCode and statusForOrchestrationError are the contract every
lib/[resource]/orchestration module returns against, but they lived inside the
workflows module, so resource-neutral code (lib/folders) already had to import
from a workflow path. Moved to lib/core/orchestration/types.
Adds a 'locked' class mapping to 423. Both tables and workflows have a lock
that forbids a mutation, and each caller was translating that to a status
itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(tables): make lib/table/orchestration the single implementation
Column update was implemented four times — the UI route, v1, v2, and the
copilot table tool — each calling the same column services but owning its own
guards, error mapping, and audit. The copies had drifted, and the drift was the
bug: v2 was missing both guards, only the copilot copy minted stable option
ids, and only v1/v2 audited.
performUpdateTableColumn, performDeleteTable, and performDeleteTableRow now own
that logic; all ten call sites reduce to auth, parse, call, render. The guards
are asserted once in lib/table/orchestration rather than four times against
four routes.
Behavior this consolidates, previously true on only some paths:
- The typeChanging guard. updateColumnType early-returns on an unchanged type
and drops any options sent with it, so restating the current type alongside
new options silently discarded them. v2 had no guard at all and, since its
contract shares v1's body schema, accepted options and ignored them.
- The select-unique guard. Each write is its own locked transaction, so a
rename or type change paired with a constraint write that is going to fail
commits first and then throws, half-applying the schema change.
- Stable select-option ids. Cells reference the option id, so an edit that
re-sends an option by name has to reuse it or every cell holding it is
orphaned. Only the copilot path did this; normalizeSelectOptionsInput moves to
lib/table/select-options and now covers every caller. It preserves a supplied
id, so it is a no-op for the fully-formed options the HTTP contracts accept.
- required forwarded into the type and options writes, so a conversion
validates against the constraint the same request is setting.
- An audit on every successful update. The UI route and the copilot tool
emitted none.
- Single-row delete through the row service. v2 did a raw db.delete, skipping
assertRowDelete and deleteOrderedRow, so a delete-locked table returned 200
and the row-count bookkeeping never ran.
- The delete actor handed to deleteTable, which audits only when a row was
actually archived. v1 and v2 omitted it and audited themselves outside that
check, emitting TABLE_DELETED for a no-op delete of an archived table.
Failure classes come back as OrchestrationErrorCode; v2 renders them through a
new v2ErrorForOrchestration, mirroring statusForOrchestrationError on the v1
and UI surfaces, so a given failure maps to the same status everywhere.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(tables): bind the column-update tests to the orchestration function
The base's route tests assert which column service each payload reaches — the
behavior that now lives in performUpdateTableColumn. They mocked the `@/lib/table`
barrel; the orchestration module imports the service directly, so they mock that
too and keep asserting the same thing through the extracted implementation.
The orchestration tests move onto the base's semantics: writes address the
stable column id, a rename rides inside the write it accompanies rather than
running first, and the currency guards replace the non-select options guard the
service now owns.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(copilot): drop the column-type import the delegation made dead
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(tables): move the audit log out of the table service
`lib/table/service.ts` wrote its own audit rows, so whether an operation was
audited depended on which function a caller reached for rather than on a user
having performed it. That is what let v1 and v2 audit a no-op delete, and what
made `deleteTable`'s optional `actingUserId` double as an audit opt-out flag.
Worse, most sites fell back to `actingUserId ?? createdBy`, so an unattributed
call was logged against the table's *creator*. The copilot `mv` path passed no
actor at all: renaming someone else's table recorded them as the renamer.
Audit now lives in the orchestration functions — performDeleteTable,
performRenameTable, performMoveTableToFolder, performUpdateTableLocks — and
the services just write. Internal callers (folder cascade, import rollback)
keep calling the service and are silent by construction rather than by
remembering to omit an argument.
Two services now return what the audit needs: `deleteTable` reports whether it
actually archived a row, so a repeat delete logs nothing; `updateTableLocks`
returns the before/after locks, since only the locked write can observe the
transition its description names.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(tables): restore audit provenance and conflict status in orchestration
Moving the audits into the orchestration functions dropped three things the
routes had been carrying, and added one the orchestration now owns twice.
- The v1 and v2 column-update routes passed `request` to `recordAudit`, so
their audit rows recorded the caller's IP and user-agent. The orchestration
function had no way to receive it. Every table orchestration function now
takes an optional `OrchestrationRequestContext` and every HTTP route
forwards it; the copilot and VFS callers, which have no request, omit it.
- `classifyTableMutation` matched `TableConflictError` on "already exists"
appearing in the message and reported it as `validation`, turning the UI
route's 409 on a duplicate table rename into a 400. It now matches the type,
the way `performRestoreTable` already did.
- `captureServerEvent` ran on every delete while the audit was gated on a row
actually being archived, so a repeat delete of an archived table still
reported `table_deleted`. Both now hang off the same evidence.
- The copilot delete path kept its own `captureServerEvent` from when the
service did not emit one, double-counting every copilot table delete.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YGzbVDZpe2dEALbu2BUU8a
* 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
* refactor(tables): classify failures by type instead of by message text
The table module decided HTTP statuses by searching error messages for
phrases. `VALIDATION_MESSAGE_FRAGMENTS` and `ROW_WRITE_ERROR_PATTERNS` held 32
substrings between them, and fifteen more lists were inlined in routes — 83
matchers over 17 files, each its own copy of the guesswork and already drifted
apart. It made message wording load-bearing: `TableRowLimitError`'s own doc
comment noted that its text had to contain "row limit" for a route to answer
400, and adding "already exists" to a rename message silently demoted a 409 to
a 400 (the bug fixed one commit ago, by adding another special case).
Services now throw `OrchestrationError`, which carries the transport-neutral
`OrchestrationErrorCode` the layers above already speak. Classification is one
`instanceof` in `orchestrationErrorResponse` (UI + v1) and
`v2CaughtOrchestrationError` (v2). Every pattern list is gone. Wording is free
to change; an unclassified error still becomes a generic 500, which is what an
unexpected fault should be.
`asOrchestrationError` walks the `cause` chain rather than testing the caught
value directly: drizzle wraps a throw raised inside a transaction callback in a
`DrizzleQueryError` whose own message is the failed SQL, so a bare `instanceof`
would drop every failure raised inside `withLockedTable`. That is the same
reason `rootErrorMessage` had to dig for a root cause before.
Three throws stay bare `Error` deliberately — `Table ID mismatch`, `Workspace
ID mismatch`, and `Failed to build upsert conflict predicate` are internal
invariants no consumer classified, and they keep falling through to a 500.
`Insufficient capacity` was in the pattern list with no producer anywhere in
the codebase.
Status changes, all deliberate:
- `'forbidden'` joins the code union so the table-row-limit ceiling keeps its
403; without it this refactor would have flattened it to 400.
- import-async's table-limit rejection: 400 -> 403, matching the two other
create routes it had drifted from.
- Renaming a table to an invalid name: 500 -> 400. `validateTableName`
messages don't contain "Invalid", so no matcher ever caught them.
- Restoring a table that isn't archived, or into an archived workspace:
500 -> 400.
- A duplicate *column* name stays `validation`/400 rather than becoming a 409
like a duplicate table name. Both v1 and the orchestration have always
answered 400 for it; changing a published status is not this refactor's job.
The twelve tests that changed were asserting the substring mechanism itself,
constructing plain `Error`s with magic strings. They now assert the real
contract, plus new cases pinning that identical wording carrying no
classification stays internal and keeps its message off the wire.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YGzbVDZpe2dEALbu2BUU8a
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>1 parent fbc7b17 commit 5fea5f7
62 files changed
Lines changed: 2170 additions & 1330 deletions
File tree
- apps/sim
- app/api
- table
- [tableId]
- columns
- run
- import
- rows/[rowId]
- import-async
- import-csv
- tools/deployments
- deploy
- promote
- v1
- tables
- [tableId]
- columns
- rows
- [rowId]
- upsert
- workflows/[id]
- deploy
- rollback
- v2
- lib
- tables
- [tableId]
- columns
- rows
- [rowId]
- upsert
- workflows/[id]
- deployments/[version]
- deploy
- lib
- copilot/tools
- handlers
- server/table
- core/orchestration
- folders
- table
- columns
- orchestration
- rows
- workflows/orchestration
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
45 | 52 | | |
46 | 53 | | |
47 | 54 | | |
| |||
50 | 57 | | |
51 | 58 | | |
52 | 59 | | |
| 60 | + | |
53 | 61 | | |
54 | 62 | | |
55 | 63 | | |
| |||
159 | 167 | | |
160 | 168 | | |
161 | 169 | | |
162 | | - | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
163 | 174 | | |
164 | 175 | | |
165 | 176 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
| 14 | + | |
| 15 | + | |
25 | 16 | | |
26 | 17 | | |
27 | 18 | | |
| |||
120 | 111 | | |
121 | 112 | | |
122 | 113 | | |
123 | | - | |
124 | | - | |
125 | | - | |
126 | | - | |
127 | | - | |
128 | | - | |
129 | | - | |
130 | | - | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
147 | | - | |
148 | | - | |
149 | | - | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | | - | |
162 | | - | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
167 | | - | |
168 | | - | |
169 | | - | |
170 | | - | |
171 | | - | |
172 | | - | |
173 | | - | |
174 | | - | |
175 | | - | |
176 | | - | |
177 | | - | |
178 | | - | |
179 | | - | |
180 | | - | |
181 | | - | |
182 | | - | |
183 | | - | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
190 | | - | |
191 | | - | |
192 | | - | |
193 | | - | |
194 | | - | |
195 | | - | |
196 | | - | |
197 | | - | |
198 | | - | |
199 | | - | |
200 | | - | |
201 | | - | |
202 | | - | |
203 | | - | |
204 | | - | |
205 | | - | |
206 | | - | |
207 | | - | |
208 | | - | |
209 | | - | |
210 | | - | |
211 | | - | |
212 | | - | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
213 | 123 | | |
214 | | - | |
215 | | - | |
| 124 | + | |
| 125 | + | |
216 | 126 | | |
217 | 127 | | |
218 | 128 | | |
219 | | - | |
220 | | - | |
221 | | - | |
222 | | - | |
223 | | - | |
224 | | - | |
225 | | - | |
226 | | - | |
227 | | - | |
228 | | - | |
229 | | - | |
230 | | - | |
231 | | - | |
232 | | - | |
233 | | - | |
234 | | - | |
235 | | - | |
236 | | - | |
237 | | - | |
238 | | - | |
239 | | - | |
240 | | - | |
241 | | - | |
242 | | - | |
243 | | - | |
244 | | - | |
245 | | - | |
246 | | - | |
247 | | - | |
248 | | - | |
249 | | - | |
250 | | - | |
251 | | - | |
252 | | - | |
253 | | - | |
254 | | - | |
255 | | - | |
256 | | - | |
257 | | - | |
258 | | - | |
259 | | - | |
260 | | - | |
261 | | - | |
262 | | - | |
263 | | - | |
264 | | - | |
265 | | - | |
266 | | - | |
267 | | - | |
268 | | - | |
269 | | - | |
270 | | - | |
271 | | - | |
272 | | - | |
273 | | - | |
274 | | - | |
275 | | - | |
276 | | - | |
277 | | - | |
278 | | - | |
279 | | - | |
280 | | - | |
281 | | - | |
282 | | - | |
283 | | - | |
284 | | - | |
285 | | - | |
286 | | - | |
287 | | - | |
288 | | - | |
289 | | - | |
290 | | - | |
291 | | - | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
297 | | - | |
298 | | - | |
299 | 129 | | |
300 | 130 | | |
301 | 131 | | |
302 | | - | |
| 132 | + | |
303 | 133 | | |
304 | 134 | | |
305 | 135 | | |
306 | | - | |
307 | | - | |
308 | 136 | | |
309 | 137 | | |
310 | 138 | | |
311 | 139 | | |
312 | | - | |
313 | | - | |
314 | | - | |
315 | | - | |
316 | | - | |
317 | | - | |
318 | | - | |
319 | | - | |
320 | | - | |
321 | | - | |
322 | | - | |
323 | | - | |
324 | | - | |
325 | | - | |
326 | | - | |
327 | | - | |
328 | | - | |
329 | | - | |
330 | | - | |
331 | | - | |
332 | 140 | | |
333 | 141 | | |
334 | 142 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
12 | 17 | | |
13 | 18 | | |
14 | 19 | | |
| |||
66 | 71 | | |
67 | 72 | | |
68 | 73 | | |
69 | | - | |
70 | | - | |
71 | | - | |
| 74 | + | |
| 75 | + | |
72 | 76 | | |
73 | 77 | | |
74 | 78 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
| 82 | + | |
82 | 83 | | |
83 | 84 | | |
84 | 85 | | |
| |||
372 | 373 | | |
373 | 374 | | |
374 | 375 | | |
375 | | - | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
376 | 380 | | |
377 | 381 | | |
378 | 382 | | |
| |||
516 | 520 | | |
517 | 521 | | |
518 | 522 | | |
519 | | - | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
520 | 526 | | |
521 | 527 | | |
522 | 528 | | |
| |||
529 | 535 | | |
530 | 536 | | |
531 | 537 | | |
532 | | - | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
533 | 541 | | |
534 | 542 | | |
535 | 543 | | |
| |||
0 commit comments