feat(tables): per-table mutation locks (schema/insert/update/delete) - #5960
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview API & contracts: Table GET/list responses now include Clients: Admin Lock settings modal, header lock chip, grid/context-menu gating with blocked-action toasts (not hidden controls), CSV import UI respecting replace/schema locks, React Query Reviewed by Cursor Bugbot for commit 43e497d. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryAdds independently configurable schema, insert, update, and delete locks for tables.
Confidence Score: 5/5The PR appears safe to merge because no blocking failures remain within the eligible follow-up scope. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/table/mutation-locks.ts | Defines the four lock assertions, update exemptions, lock errors, and branded mutation proofs. |
| apps/sim/lib/table/rows/ordering.ts | Adds proof requirements and transaction-scoped advisory-lock revalidation to low-level batch writes. |
| apps/sim/lib/table/delete-runner.ts | Revalidates delete locks for each batch and cancels jobs when a lock blocks further deletion. |
| apps/sim/lib/table/update-runner.ts | Revalidates update locks for each batch using the patch’s actual columns and cancels blocked jobs. |
| apps/sim/lib/table/import-runner.ts | Enforces insert, delete, and schema locks at import startup and within corresponding write transactions. |
| apps/sim/lib/table/import-data.ts | Moves import writes behind transaction-scoped lock guards and fresh table-definition reads. |
| apps/sim/lib/table/rows/service.ts | Threads lock assertions and mutation proofs through synchronous row operations. |
| apps/sim/app/api/table/[tableId]/route.ts | Adds admin-gated lock updates, feature-flag checks, and lock state to table responses. |
| packages/db/migrations/0270_table_mutation_locks.sql | Adds non-null schema, insert, update, and delete lock columns with unlocked defaults. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Admin[Workspace admin] --> API[Table lock API]
API --> Locks[(Table lock columns)]
Locks --> Services[Table service assertions]
Services --> Sync[Sync mutations]
Services --> Jobs[Background jobs]
Jobs --> Guard[Transactional batch revalidation]
Guard --> Writes[Row and schema writes]
Locks --> UI[Lock-aware table UI]
Reviews (13): Last reviewed commit: "fix(tables): restore the workspace owner..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
On the remaining summary point — "both worker-start checks mint unversioned proofs before later page writes, allowing a lock-state change in that interval to be bypassed" — the two concrete findings there are fixed in 0344bc9 (the update worker now applies It is not specific to the workers. Every mutation path in this feature loads the table, then writes: a single-row The two ways to actually close it are both known-harmful here. Re-reading the definition inside the write transaction means either The window is bounded by design. Locks gate starting a mutation; workspace This is recorded as an explicit non-goal in the |
|
@cursor review |
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 24ca513. Configure here.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ba4beb6. Configure here.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3a1b8a6. Configure here.
Adds four independent, admin-only locks to a table so a workspace can make it append-only, read-only, or schema-frozen. Enforced at the lib/table service layer rather than the routes, so the API, workflow blocks, and Mothership (which calls the services directly) are all covered by one assert. Violations return 423; changing a lock requires workspace admin and returns 403.
- Add the drizzle meta snapshot the hand-written migration was missing, which made CI regenerate the lock columns as a phantom 0271 - Stop a queued or retried delete/update job that starts after its lock is enabled; a run that has already committed pages still finishes, since pages are never rolled back and aborting would leave an uncancelled half-done state - Map TableLockedError to 423 on the internal single-row DELETE (was a 500) - Gate the lock settings UI behind NEXT_PUBLIC_TABLE_LOCKS so it can't open onto a Save that 403s against the server-side flag - Respect the delete lock on column drops in the grid, matching assertColumnDestructive, instead of failing only on click
- Assert insert (and delete for replace) at the start of the import worker. Replace deleted every row before the first insert assert, so an insert-locked table was wiped and then failed, leaving it empty. - Run the delete/update worker start checks through assertRowDelete / assertRowUpdate instead of reading the lock flags directly, so the worker and its enqueue site apply identical rules — including the workflow-column exemption a bulk update was being cancelled despite. - Make undo/redo verb mapping direction-aware for column ops: dropping or retyping a column needs the delete lock clear too, matching assertColumnDestructive, so those steps are skipped instead of stranded.
…mports - Run / Re-run / Stop no longer inherit the cell-edit gate: they write only workflow-output columns (which the update lock exempts) and Stop is a cancel - Duplicate needs only the insert lock — it inserts a full copied row, so it stays available on an append-only table - updateWorkflowGroup asserts the destructive rule only when a patch actually drops or remaps output columns; rename / autoRun / mapping edits need just the schema lock - Gate the import-async enqueue on insert (and delete for replace) so a locked table 423s instead of claiming the write-job slot and failing in the worker - Disable Import CSV under an insert lock, and wire the expanded cell editor and the previously-unused stable add-row handler to the lock-aware flags
- Pass the blocked-delete handler instead of undefined so ColumnOptionsMenu still mounts on a delete-locked table; withholding it hid the entire menu, including Insert column, which the delete lock must not affect - Restore ColumnHeaderMenu readOnly to permission-only: it swaps the header for a static label, which was also disabling pin, column-select and open-config - Assert the schema lock at the import-async enqueue when createColumns is set - Allow a locks PATCH that only clears locks even when the feature flag is off, and keep the settings entry reachable on a locked table, so flipping the kill switch can't strand a table with locks nobody can remove
- Make the workflow-output carve-out opt-in via `computedWrite`, set only by the cell-write path. It was caller-agnostic, so an ordinary API caller could PATCH a workflow-output column on an update-locked table - Re-read the lock before every page in the delete and update runners. The single worker-start check went stale immediately, so enabling a lock could not stop a job already deleting or overwriting rows. Committed pages still stay committed, exactly as with an explicit cancel. Import keeps its one-shot check: a half-imported table needs the deletes the lock now forbids - Route Insert column to the locked-action modal under a schema lock instead of leaving it live (regular header) or hiding the whole menu (group header)
…artial unlock - Re-assert the lock inside each page-write transaction, under the same advisory lock updateTableLocks takes, so check-then-write is atomic against a lock change. The per-page check alone still left the page-selection await between the assert and the write - Pass computedWrite from the backfill runner: batchUpdateRows is the workflow-output backfill path, so scoping the exemption had made rebuilding outputs 423 on an update-locked table while live cell writes still worked - Gate the feature flag on a lock actually going off->on rather than on an all-false payload. The settings UI always submits all four flags, so with the flag off an admin could not clear one lock while another stayed on - Hoist the lock-kind -> flag map to lib/table/types as TABLE_LOCK_FLAGS
- Re-assert the insert lock inside each import batch's insert transaction, under the same advisory lock updateTableLocks takes. Reversing the earlier "let the file finish" call: an admin can lift the lock to clean up a partial import, so honouring it beats letting the rest of the file land - Gate paste per verb. Overwriting existing rows is an update and extending past the last row is a full-row insert, so a paste-append still works on an append-only table while an overwrite explains itself. Refuses the whole paste rather than applying half of it - Space opens the same row editor as double-click, so it now follows the update lock instead of filling in a form that 423s on save
…ift+Enter - Route the replace-mode wipe, the inferred-schema write and createColumns through the same guardBatch transaction as the batch inserts, so a delete or schema lock committed while the file is downloading or being sampled is seen instead of the job-start snapshot - Shift+Enter takes the manual-add path, so it now opens the lock modal like the Add row button instead of returning silently
…odal - Replace TableLockedModal with a warning toast carrying a "Lock settings" action button for admins. Being told you can't edit shouldn't cost a dismiss click, and the button still routes admins straight to the panel - Dedupe by id so repeated attempts on a locked cell replace one notice instead of stacking a column of them - Move the copy into lock-copy.ts alongside the rest of the lock vocabulary and tighten it for toast length
… access Double-click checked the update lock before any permission check, so a read-only member on a locked table got "Editing rows is locked" — misleading (the lock isn't why they can't edit) and it swallowed the expanded-cell viewer, which is a legitimate read-only affordance.
… with permissions - The sync append branch returns instead of rethrowing, so the outer catch's mapper never saw a TableLockedError and every lock violation became a 500. Map it in that catch, with a regression test (replace mode already rethrows) - Stop mounting the workflow-group column menu for users without edit access: passing the blocked handlers unconditionally made it appear for read-only members and report a lock even on an unlocked table - Enter/F2 now raises the same lock notice as double-click and Space instead of silently doing nothing - guardBatch returns the freshly-read definition so addTableColumnsWithTx asserts live state; a schema lock cleared mid-import no longer fails the createColumns step. The snapshot pre-asserts now only run as the fallback for callers that pass no revalidator - Append-only no longer labels a table whose schema is also locked, which claimed columns were mutable when they weren't - Import dialog withholds Replace on a delete-locked table and create-column on a schema-locked one, instead of offering a configuration that only 423s
… every blocked key - The sync append/replace paths asserted only the request-start snapshot, so a lock committed while the CSV was parsed still let the write through. Both now re-read under the schema advisory lock at the top of their own transaction, taken before acquireRowOrderLock so the order stays advisory -> rows_pos -> definitions - Delete/Backspace, Cmd+D, typeahead and cut raised no notice on an update-locked table; they now explain the lock, and stay silent for users without write access - The multipart import fetch threw a plain Error, dropping the 423 status, so the lock self-heal never ran and the stale detail cache survived. It throws ApiClientError now and both import-into-table hooks call handleTableLockRejection - Force append at submit when the delete lock landed while the dialog was open with Replace already selected
…etes Switching deleteRow/deleteRowsByIds to take a TableDefinition dropped the workspaceId argument that previously scoped the query, and the two branches I added loaded the table without the ownership comparison every other operation in the tool performs — letting a caller delete rows from a table in another workspace. Both now reject a foreign table as not found.
3a1b8a6 to
43e497d
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 43e497d. Configure here.
1 participant
Summary
lib/tableservice layer, not the routes: Mothership calls the services directly, so route-level asserts would miss it entirely. API, workflow blocks, and Mothership all covered by one assertrows/ordering.tsnow require a branded proof, so a new write path can't be added without asserting firsttable-locks/TABLE_LOCKS(off by default). Enforcement of already-set locks always runs; the flag gates setting themDELETE /rows/[rowId]route did a rawdb.deleteand would have returned 200 on a locked tableworkflow.locked— forking requires admin on the source, the same bar as unlocking, so it can't launder a lockType of Change
Testing
Tested manually.
bun run lint:check,type-check,check:migrations,check:api-validation:strict, and the rest of the CI audit suite all pass. 506 table/hook/route tests pass, including new unit tests for each assert and its exemptions (executions-only patches and workflow-group output columns stay writable under an update lock).Not yet exercised against a running app — worth a click-through on an append-only table before merge.
Checklist