feat(realtime): shared room spine + live Files/Tables collaboration + Yjs document editing - #5991
Open
waleedlatif1 wants to merge 83 commits into
Open
feat(realtime): shared room spine + live Files/Tables collaboration + Yjs document editing#5991waleedlatif1 wants to merge 83 commits into
waleedlatif1 wants to merge 83 commits into
Conversation
Introduces the foundation for a unified realtime "room" model spanning the
Socket.IO presence server (apps/realtime), the durable SSE event log, and the
ephemeral pub/sub fanout — all of which today reinvent their own room identity,
naming, and authorization.
- @sim/realtime-protocol/rooms: RoomRef { type, id }, ROOM_TYPES, and a
roomName/parseRoomName codec. WORKFLOW deliberately maps to the bare id so the
~40 existing io.to(workflowId) callsites and presence state keys are unchanged;
every other room type is namespaced so id spaces cannot collide.
- @sim/platform-authz/rooms: authorizeRoom(userId, room, action) generalizing the
exemplary authorizeWorkflowByWorkspacePermission — one resource->workspace
resolver per room type, then the shared resolveEffectiveWorkspacePermission +
permissionSatisfies gate.
Pure foundation, no behavior change: nothing consumes these yet. Prune graph
stays at 14/25 (platform-authz already depended transitively on realtime-protocol
via apps/realtime).
…5930) * refactor(realtime): generalize presence server to multi-room (RoomRef) Generalizes the Socket.IO presence layer from single-workflow-room-per-socket to a domain-neutral, multi-room-per-socket model keyed by RoomRef, so a second domain (workspace files, next PR) can reuse the same membership + presence engine. Behavior-preserving for workflow collaboration. IRoomManager is now domain-neutral (addUserToRoom/removeUserFromRoom/ getRoomForSocket/getRoomUsers/updateUserActivity/... all take a RoomRef). The workflow lifecycle broadcasts (deletion/revert/update/deploy) move out of the manager into WorkflowRoomService, composed over the generic manager. Backward-compat by design (no workflow migration, no regression): - Workflow Socket.IO room name stays the bare workflowId (roomName() maps workflow -> bare id), so the ~40 io.to(workflowId) callsites are untouched. - Workflow Redis presence keys stay workflow:{id}:users/:meta (the type prefix IS "workflow"). Multi-room correctness (from adversarial audit): - socket:{id}:workflow single-value key -> socket:{id}:rooms HASH (type->id). - The SHARED socket:{id}:session key is deleted only when the socket leaves its LAST room (refcount via HLEN) — a leave from one room no longer breaks the other room's handlers. - disconnect enumerates the socket's stored rooms and rebroadcasts presence per room, instead of picking an arbitrary socket.rooms entry. - presence broadcasts use a per-room-type event name (workflow keeps the bare presence-update; others are namespaced). Workflow handlers wrap manager calls with a shared workflowRoom(id) helper; UserPresence.workflowId -> room (the client never reads that field). Tests: existing 112 realtime tests pass unchanged (behavior gate) + 7 new multi-room tests (refcounted session, presence isolation, multi-room disconnect, per-type event names). tsc clean, boundaries + prune (14/25) green. * fix(realtime): harden multi-room disconnect + id-guard room removal Two fixes from an adversarial regression audit of the multi-room refactor: - Disconnect now handles `disconnecting` (where `socket.rooms` is still populated and authoritative) and falls back to the live Socket.IO room set for any room the manager's stored state no longer tracked. This restores reliable presence cleanup + departure broadcast even if the Redis `socket:{id}:rooms` key was evicted or TTL-expired — the one behavioral gap vs the pre-refactor disconnect. - REMOVE_ROOM_SCRIPT now only drops the socket's room mapping (and runs the last-room session cleanup) when the stored id matches the room being removed, matching the memory manager's existing id guard. Prevents a mismatched-room call from wiping a different room's mapping or the shared session. +1 test (id-guarded no-op removal). 120 realtime tests pass, tsc clean. * fix(realtime): only rebroadcast disconnect-fallback rooms whose removal succeeded Greptile 4/5 follow-up: the disconnecting-time fallback ignored removeUserFromRoom's boolean and rebroadcast presence even when the removal reported false. Now it only treats a room as removed (and rebroadcasts) when the manager confirms it — symmetric with removeSocketFromAllRooms, which already only returns rooms it actually removed. * fix(realtime): exclude the disconnecting socket from its farewell broadcast Greptile follow-up (transient-Redis-failure edge): if removeUserFromRoom fails on disconnect, the socket's presence entry can outlive it (room hashes have no TTL) and reappear as a ghost. Disconnect now broadcasts a correction to EVERY room the socket was in (union of the manager's removed rooms and the live Socket.IO membership) and passes the disconnecting socket id as excludeSocketId, so it is never shown as a collaborator regardless of whether the Redis delete succeeded. Any orphaned entry is still reclaimed by the next join's stale-presence sweep. broadcastPresenceUpdate gains an optional excludeSocketId; normal broadcasts are unchanged. +1 test. * fix(realtime): make presence broadcasts liveness-aware (root-cause ghost fix) Presence broadcasts now reconcile the stored list against the live Socket.IO membership (io.in(room).fetchSockets()) before emitting, via a shared filterVisiblePresence helper. This closes the residual behind the earlier disconnect fixes: an entry orphaned by a failed removal (room hashes have no TTL) could reappear in a LATER join's presence snapshot until the 75-min stale sweep. Now such an entry is never emitted, because a non-live socket is filtered out of every broadcast. Combined with excludeSocketId (which handles the disconnecting socket, still momentarily live). Fail-safe: on a fetchSockets throw or an empty result while entries remain, emit the unfiltered list rather than hide live collaborators. Also drops a dead guard in the disconnect union loop (rooms already removed are skipped by the wasInRooms check) and the now-unused isSameRoom import. +1 ghost-guard test. 122 realtime tests pass.
* fix(realtime): address post-merge review-comment findings A re-audit of every inline review comment on the merged stack surfaced real issues that the thread-resolutions and prior audits missed. Fixes: Presence server (#5930 comments): - connection.ts: snapshot `socket.rooms` SYNCHRONOUSLY before the first await. Socket.IO clears the room set once the synchronous part of a `disconnecting` handler returns, so reading it after `await removeSocketFromAllRooms` saw an empty set — the eviction fallback was dead. (Cursor: "Disconnect fallback misses live rooms".) - workflow-room-service: restore the original managers' final unconditional room-state wipe via a new `deleteRoom(room)` manager method, so a deleted workflow leaves no lingering presence/meta even if a per-socket removal failed or a socket joined mid-teardown. (Cursor: "Deletion skips final room wipe".) Files (#5932 comments): - workspace-file-manager.uploadWorkspaceFile now fans out the live-tree signal (all direct-upload paths: multipart fallback, copilot create, /api/files/upload, v1 files — the presigned path already notified). (Cursor: "Creates miss live tree fan-out".) - use-workspace-files-room: clear the pending retry timer on join success; and a module-scoped intended-room guard defers the unmount `leave` so a rapid remount re-claims the room and skips a stale leave — fixing presence flap + a leave-after-join race. (Cursor: "Retry timer survives join success" + "Remount churns files presence".) - workspace-files handler: roll back a partial join (leave room + remove presence) in the catch, mirroring the workflow join. (Cursor: "Join failure skips membership rollback".) +2 tests (deleteRoom). 127 realtime tests pass, both apps tsc clean, api-validation + boundaries green. * fix(files): scope workspace-files leave to a workspace (deferred-leave safety) Self-review of the deferred-leave guard found a real bug: leave-workspace-files was not workspace-scoped, so after a workspace switch (A->B) the deferred leave from A would evict the socket from its new room B. The leave now carries the workspaceId and the server no-ops if the socket's current files room differs. Also excludes the leaving socket from the leave broadcast (consistent with disconnect). * fix(realtime): close files-room presence leak + validate join payload Architecture-audit findings: - S1 (real Redis leak): the files room inherited the shared manager but not the workflow join's liveness sweep, so an UNGRACEFUL disconnect (pod crash — no `disconnecting` event) left its presence entry in the no-TTL room hash forever. Added a shared `sweepStalePresence(manager, room)` (fetchSockets liveness + remove not-live-AND-stale entries, matching the workflow 75min threshold) and run it on files join; also filter the join ack through `filterVisiblePresence` so a joiner never briefly sees an un-swept ghost. - S2: validate the client-supplied `workspaceId` on files join before it reaches the DB query (matches the /api/workspace-files-changed guard; fails closed). - N2: corrected the notify doc — it is awaited (guaranteed dispatch before a Node route returns) and hard-bounded to NOTIFY_TIMEOUT_MS, not "never block". +1 test (sweepStalePresence keeps live/fresh, reclaims not-live-stale). 128 realtime tests pass, both apps tsc clean, biome clean. * fix(realtime): workflow-deletion always notifies + cleans by socket.io membership Review-round findings on #5937: - Always emit `workflow-deleted` (was guarded by users.length>0), so a socket still in the Socket.IO room after a Redis presence eviction is told the workflow is gone before socketsLeave kicks it — the editor no longer keeps showing a deleted workflow. (Cursor: "Silent kick skips deletion event".) - Clean per-socket state for the UNION of live Socket.IO members and presence-tracked sockets, so an evicted/late-joined socket's room mapping + session are dropped too — not just presence-snapshot sockets. (Greptile: "Room deletion leaves reverse state".) - deleteRoom now logs AND rethrows on Redis failure (like addUserToRoom) so a failed wipe isn't reported as a clean deletion; the request surfaces it. (Greptile: "Room deletion failures are suppressed".) The two "deferred leave drops new membership" P1s were already fixed by the workspace-scoped leave in a prior commit (leave carries { workspaceId }; server no-ops on mismatch). 128 tests pass, tsc + biome clean. * refactor(files): drop module-scoped deferred-leave; rely on workspace-scoped leave Removes the one non-idiomatic construct (a module-level mutable `intendedFilesWorkspaceId` + queueMicrotask). It only guarded a same-workspace CONCURRENT remount, which doesn't occur in production (folder nav is shallow/no remount; list<->detail is sequential) — a dev-StrictMode-only case. The real cross-workspace race is already handled by the workspace-scoped leave: if B's join runs first (auto-leaving A), A's leave no-ops because the socket's current files room is B. Simpler, idiomatic, prod-correct.
…4/N] (#5941) Server-side Yjs relay for collaborative document editing (live carets + text selection) in the Files rich-markdown editor. Faithful y-websocket-style relay over the existing authenticated Socket.IO connection + shared room abstraction; in-memory Y.Doc + Awareness per file; awareness ownership binding, userId-keyed client-id uniqueness, seeder election with deadline re-election, concurrent-JOIN generation guard. 25 relay tests. Reviewed to Greptile 5/5 + Cursor pass across multiple rounds, plus an independent 4-lens audit (correctness/security/conventions/simplicity) and /simplify + /cleanup passes.
#5946) Client Yjs provider (FileDocProvider over the authenticated socket) + TipTap Collaboration/CollaborationCaret wiring for live carets + text-selection in the Files rich-markdown editor. Collaboration is a Files-page-only surface (explicit `collaborative` opt-in), disjoint from agent-streaming. Read-only + autosave-gated until synced+seeded. Merges into the realtime-rooms integration branch.
…ation propagation (#5957) * feat(tables): live cell-selection presence — protocol + server + client hook The realtime spine for Google-Sheets-style table presence (mode A, socket): - @sim/realtime-protocol/table-presence: centralized wire protocol (events + TableCellSelection {anchor, focus, editing} + payloads) so server emits and client subscriptions can't drift. - ROOM_TYPES.TABLE + resolveTableWorkspace registered in ROOM_WORKSPACE_RESOLVERS (tableId -> workspace via userTableDefinitions, honoring archivedAt); roomName / presenceEventName / disconnect cleanup / authorizeRoom all derive automatically. - apps/realtime/src/handlers/tables.ts: join/leave (mirrors workspace-files) + a table-cell-selection relay (mirrors the workflow selection channel), broadcasting via roomName(room) since table rooms are namespaced. UserPresence gains a cell field threaded through the memory + Redis managers (Lua ARGV[7], null clears). - Extracted the duplicated resolveAvatarUrl into handlers/avatar.ts. - use-table-room.ts client hook: joins over the shared socket, tracks the roster (avatars) + patches per-socket cell deltas, exposes a throttled emitCellSelection. Grid UI (avatars + selection overlay) lands next; concurrent cell-value edits (last-write-wins via the durable log) are the follow-up PR. * feat(tables): render live cell-selection presence in the grid Wires the table presence room into the grid UI: - Page (table.tsx): useTableRoom (gated off in embedded/mothership mode) — renders <PresenceAvatars> in the header and passes remoteSelections + emitCellSelection down to the grid. - Grid emits its local selection: an effect resolves the index-based anchor/focus to stable (rowId, columnId) via refs and broadcasts it (with an editing flag for the active cell) through the throttled emitter. - RemoteSelectionOverlay: draws each remote viewer's selection in their color (getUserColor), a darker fill while editing, and name-on-hover — measured from live cell rects in the content wrapper's space (scrolls with the grid), hidden when rows are virtualized off-window, pointer-events-none so it never blocks cell clicks (hover via pointer hit-test). * test(tables): cover the table presence handler Mirrors workspace-files.test.ts: join auth/unavailable/denied/success, plus the cell-selection relay (asserts it persists via updateUserActivity and broadcasts on the namespaced roomName, not the bare id) and leave. * feat(tables): propagate manual cell edits live (last-write-wins) A manual row edit now appends a lightweight 'edit' event to the durable table stream; collaborators refetch the row (via the existing debounced rows-invalidate the job events use) so the winning value shows live. The event carries no value — peers refetch in their own wire format, so there's no auth-specific value translation on the wire, and last-write-wins falls out of the DB's committed order (the Google-Sheets model). Edits that also trigger a dispatch already emit dispatch/cell events; the debounce coalesces the two. * refactor(tables): apply /simplify findings - Drop the dead 'add unknown peer' upsert branch in use-table-room (Socket.IO ordering guarantees a peer is in the roster before their selection delta). - TableCellSelectionBroadcast = TablePresenceUser & { cell } (was a copy-paste). - Make TableGrid's presence props required + drop the unused empty-default/guard (only table.tsx mounts it, always passing both). - Drop the unused rowId from the 'edit' event (the handler invalidates all rows). - Overlay: subscribe scroll/resize/pointer listeners once per scroll element and cache the wrapper origin, so incoming deltas re-measure without re-subscribing and the pointer hit-test never forces a per-move layout read. - Server: cache the immutable socket session so a selection delta no longer reads it from Redis every time. * refactor(tables): apply /cleanup findings - Fix the remote-selection name label contrast: text-white is unreadable on the light-pastel user colors (same bug the Files caret fixed) → fixed dark #1a1a1a. - Re-measure via useLayoutEffect so a moving peer selection updates before paint (no one-frame position lag). - Drop 'mothership' from a comment (constitution copy rule). Six cleanup passes ran (effect, memo/callback, state, react-query, emcn, comment); the rest confirmed clean — all state/memos/callbacks/effects are load-bearing, presence correctly lives in useState (socket-pushed), and the edit→rows-invalidate granularity is right. * feat(tables): propagate every table mutation live (edit + schema signals) Comprehensive live collaboration for all user table mutations, via two value-less durable signals + named helpers (signalTableRowsChanged / signalTableSchemaChanged): - edit (rows refetch): single + batch row create, cell/row update, batch update, delete by id/filter, and upsert. - schema (definition + rows refetch): column add/update/delete, workflow-group add/update/delete, table rename, and CSV import (which can add columns). - Client handles 'schema' by invalidating the table detail (exact) + rows. Execution paths (column run, cancel-runs) and async jobs (delete/import-async, job-cancel) already propagate via cell/dispatch/job events — verified applyJob refetches on terminal. No reorder routes exist. Table archive (route DELETE) is a deliberate follow-up: it needs a table-deleted redirect event, not a refetch signal (which would 404). * refactor(tables): apply comprehensive /cleanup audit findings Holistic + react-query + comment audits over the whole PR: - Security/crash fix: a remote peer's rowId flowed unescaped into the overlay's querySelector — a hostile id ('x"]') threw SyntaxError inside a useLayoutEffect, crashing every other viewer's page. CSS.escape it, and validate + whitelist the untrusted cell payload server-side (shape + 200-char id bound) before it is stored/rebroadcast. - Simplify the CELL_SELECTION relay: the delta attached userId/userName/avatarUrl that the client discarded (identity comes from the roster). Drop them + the getUserSession lookup/cache entirely — the delta is now { socketId, cell }. - React Query: schema handler also invalidates lists() (parity with the local column-mutation set); document that the mutating client self-refetches by design. - Comment tightenings; biome fixed a stale import order in workspace-files.ts. * fix(tables): broadcast single-cell selections (focus falls back to anchor) Cursor High: a normal cell click leaves selectionFocus null (the grid treats it as a one-cell selection via focus ?? anchor), but the presence emit required BOTH anchor and focus to resolve — so the most common selection never broadcast and clicking even cleared a prior remote outline. Mirror the grid's focus ?? anchor semantics. * fix(tables): reviewer + regression + per-LOC audit findings Cursor review round (5 findings) + regression audit + per-LOC audit: - Presence roster snapshot now KEEPS the cell we already hold for a known socket, so a join/leave broadcast can't revert a fresher CELL_SELECTION delta. - Reset the selection throttle on table switch (was unmount-only), so a pending selection for table A can't flush into table B's room after a switch. - Metadata writes (column widths, display) use a new lightweight 'metadata' signal that refetches only the definition — a resize no longer forces peers to refetch rows. - Overlay re-measures on row add/remove/reorder via a tbody childList MutationObserver (a live refetch moves cells without a scroll/resize). - Document the actor self-refetch create caveat (scrolled multi-page insert) accurately. - isCellRef narrows to a partial instead of casting to the full type then re-checking; drop a redundant mount measure() (the layout effect covers it); text-[11px]→text-xs. * fix(tables): drop ineffective metadata propagation + re-measure overlay on column resize Cursor round on b8f28b0: - Remove the 'metadata' signal entirely. The grid seeds columnWidths/pinnedColumns from metadata ONCE (metadataSeededRef) and deliberately never re-applies them (to avoid clobbering a local in-progress resize), so refetching the definition on a peer never surfaced their width/pin change — an ineffective path. Width/pin live-sync needs reconciliation that doesn't clobber a local resize; that's a deliberate follow-up, not a no-op refetch. Structural changes still propagate via 'schema'. - Overlay now also observes the content layer with the ResizeObserver, so a column resize (which grows the content, not the scroll container) re-measures remote outlines. - Presence-merge comment now states both sides of the trade-off. * fix(tables): re-broadcast local selection on (re)join Cursor Medium: a selection made before the room join completes (or held across a reconnect) was dropped server-side and never re-sent, so peers didn't see it until the local user moved it again. Track the current selection in a ref (set on every emit, cleared on table switch) and re-emit it from handleJoinSuccess once the room is joined. * fix(tables): re-broadcast selection when a peer's row change shifts it End-to-end lifecycle audit (Low-Med): the selection emit resolved the stable (rowId, columnId) only on selection/editing change, not when a live edit/schema refetch inserted/deleted/reordered rows. The index-based local selection then sat on a different logical row than the rowId peers held, so your outline showed on the old row until you moved. Re-run the emit on rows/displayColumns change and dedup an unchanged result (also drops the redundant null-on-open emit) so the broadcast stays consistent with the local highlight. * fix(tables): schema invalidates run-state/enrichment + guard stale join Cursor round on cdc8796 (2 Medium): - schema handler used detail exact:true, so it skipped the activeDispatches + enrichmentDetails sibling queries the local invalidateTableSchema refreshes via a prefix match. After a peer deletes/restructures a workflow group, peers could keep a stale running badge or enrichment panel. Now invalidates both siblings too (rows stay on the debounce). - Guard against a stale join stealing the room: a fast table A->B switch could let A's async authorize finish after B, leave B, and strand the socket in A. Added a per-socket monotonic join generation checked after authorize (mirrors the file-doc relay's guard) + a test. * feat(tables): live column width/pin/order sync Collaborators now see each other's column resizes, pins, and reorders live — the last piece of Google-Sheets-style layout parity. - New lightweight `metadata` durable event kind (distinct from `schema`): only the table definition carries UI metadata, so peers refetch the definition alone — no rows/run-state refetch. The metadata PUT route now signals it. - The grid reconciles server metadata against its in-progress gesture: the column being actively resized keeps its live local width, and an in-flight column drag blocks a reorder apply — so a peer's change never reverts the local action. Each field is reference-guarded (React Query structural sharing keeps unchanged sub-objects stable), so an unrelated peer change doesn't re-apply the others. * fix(tables): escalate to schema signal when a reorder scrubs group deps Independent audit of the metadata-sync commit found a stale-run-state hole: a columnOrder PUT that moves a column left of a workflow group's leftmost column makes updateTableMetadata scrub that group's dependencies and write a new schema — a real structural change. But the route only fired the lightweight 'metadata' signal (detail-only refetch), so peers' and the actor's activeDispatches / enrichmentDetails queries stayed stale (a lingering running badge / enrichment panel) — exactly what the 'schema' handler exists to prevent. updateTableMetadata now reports whether it scrubbed the schema; the route emits signalTableSchemaChanged in that case and the light signalTableMetadataChanged otherwise. Width/pin/plain-reorder stay on the cheap detail-only path.
#5965) Per-session file-doc presence (avatars count other sessions like the canvas), StrictMode-safe stable Y.Doc (fixes blank-doc on join), flush caret cap + restored hover hit-slop, and three join-lifecycle race fixes unifying file-doc + workspace-files on one intent-tracked monotonic generation model. All findings root-caused with regression tests.
#5971) * improvement(files): smarter bullet delete/indent, fix empty-nested-bullet heading corruption Backspace at the start of a list item now outdents a nested item or clears a top-level item to a paragraph in place instead of deleting the row and jumping the caret to the previous block; Enter on an empty nested item outdents. Empty non-trailing top-level items still collapse cleanly since they cannot round-trip as a lifted paragraph. Also strips nested empty list-item marker lines on serialize: a nested empty bullet re-parsed as a Setext heading underline, silently turning its parent line into an H2 and dropping the bullet. Top-level empty items are preserved. * feat(files): sync an untitled file's name with its leading heading While a file is still named untitled(.md), typing a leading heading auto-renames the file after it (debounced), and renaming the file first seeds a leading H1 from the new name. One-shot: coupling stops once the file has a real name, and the heading seed always prepends so existing content is never clobbered. * fix(files): count inline atoms in list-item emptiness, keep multi-block items on Backspace Addresses review findings on the list Backspace logic: - Emptiness now uses the caret block's content.size (counts inline images/mentions), not textContent, so a bullet holding only a non-text atom is no longer treated as empty and deleted. - An empty first block whose item has sibling blocks removes only that block instead of lifting the whole item out of the list. * fix(files): preserve the untitled to named heading seed across a rename during editor load The parent captures the file name at mount (before content/session finish loading) and passes it as the transition baseline, so a rename that lands in the loading window is still seen as an untitled to named transition and the leading heading seed is not skipped. * fix(files): drop the name-to-heading seed, keep title sync one-way Removes the effect that inserted a leading H1 when an untitled file was renamed. On the collaborative Files page every open client observed the untitled-to-named transition and inserted into the shared doc, producing duplicate headings; it could also re-insert a heading a user had just deleted while a rename was in flight. Seeding document content from an async rename transition is the wrong model on a shared editor. The primary direction — typing a leading heading renames a still-untitled file — is unaffected (it never mutates the doc). * fix(files): keep empty lines between paragraphs on reload The chunked markdown parser (parseMarkdownToDoc) parses each block stripped of the blank lines between them, so it dropped the empty paragraphs @tiptap/markdown builds from runs of blank lines — a saved visual blank line silently vanished on the next load (the settle/reopen re-seed goes through the chunker). The whole-document parser preserves them, but whether a gap yields an empty paragraph is a global, block-type- dependent decision (kept between two paragraphs, dropped after a heading), so it can't be reconstructed block-locally. Route documents with empty-paragraph blank-line spacing to the whole-document parser for exact fidelity — the same tradeoff NON_CHUNKABLE makes; ordinary single-blank-line separation still takes the fast chunked path. Adds a suite asserting chunked output matches the whole-document parser for leading/trailing/between gaps and around lists/headings. * fix(files): only auto-name an untitled file when the user can edit The debounced untitled→filename hook ran on every onUpdate — including the mount-time seed and for view-only viewers — without checking edit permission, so a read-only user could schedule a rename they have no permission to make (a spurious, server-rejected write). Gate the derive-title on editor.isEditable (canEdit + settled + collab-ready, the same signal the autosave path uses), at both schedule and fire time. * fix(files): normalize line endings before the empty-paragraph guard; Enter/Backspace symmetry - markdown-parse: EMPTY_PARAGRAPH_SPACING/NON_CHUNKABLE tested the raw body, but a classic \r-only file (blank lines are \r) would miss the \n-anchored guard and still be chunked, dropping empties. Normalize line endings once up front so the routing guards, the chunker, and the parser all see the same \n. +CRLF/CR test cases. - keymap: Enter on an empty first block of a multi-block item now removes only that block (removeEmptyWrappedBlock) instead of exiting the list, mirroring the Backspace hasSiblingBlocks case — the trailing check no longer swallows multi-block items. +test. * fix(files): editor audit follow-ups (trailing-blank read-only, collab rename, over-strip) A 4-agent independent audit (UX vs inkeep + SOTA, cleanliness, adversarial correctness) surfaced these: - HIGH regression: files ending in a blank line opened READ-ONLY. The empty-paragraph routing preserved a TRAILING empty paragraph, but postProcess collapses trailing newlines → serialize/parse non-idempotent → isRoundTripSafe flipped the file read-only. A trailing empty paragraph can't be serialized stably, so parseMarkdownToDoc now strips trailing empty paragraphs and the guard no longer routes on trailing blanks. Interior/leading empties are unaffected. +regression tests. - Medium: the debounced untitled→filename rename fired on remote Yjs edits too, so every peer renamed and could rename from a not-yet-synced heading. Gate on isChangeOrigin (local edits only; false for non-collab surfaces). - Medium: stripEmptyListItemLines over-stripped a nested empty item that follows a same-indent sibling (a real placeholder the parser keeps). Narrowed to the actual Setext hazard — an empty item DIRECTLY under a shallower parent line — matching the function's own docstring intent. Probe-verified. +test. - Low: corrected untitled-title.ts docstring that described a reverse name→heading coupling removed during review. * fix(files): a remote edit must not cancel the local rename debounce The isChangeOrigin gate cleared the debounce timer BEFORE bailing on a remote update, so a peer's edit arriving within the 600ms window cancelled the local user's pending rename. Bail on isChangeOrigin first, before touching the timer; only local edits clear/reschedule it. * docs(files): correct EMPTY_PARAGRAPH_SPACING rationale after trailing-strip The stacked trailing-empty-paragraph strip made the older comment overstate a correctness necessity it no longer owns, mislabel trailing runs of 2+ blanks, and advertise dead CRLF handling. Reword to match what the code actually does. --------- Co-authored-by: Waleed Latif <walif6@gmail.com>
Catch the realtime-rooms integration branch up to staging (36 commits). Ten files conflicted where staging's work overlapped the multi-room refactor; resolved as follows: Realtime managers (workflow.ts, memory-manager.ts, redis-manager.ts, types.ts): kept the generalized multi-room API. Staging's #5917 (evict revoked collaborators) collided with the single-room→multi-room refactor; its access-revalidation.ts is ported onto the generalized API (getRoomForSocket/removeUserFromRoom(RoomRef, socketId)/broadcastPresence Update(RoomRef)), and its tests updated to match. A disconnected-socket guard in cleanupEvictedSocket preserves the pre-generalization no-op-on-already-gone behavior now that removeUserFromRoom returns a boolean. Table events (events.ts + use-table-event-stream.ts): kept BOTH taxonomies — the live-collab edit/schema/metadata kinds and staging's lock 'definition' kind coexist (emitted by disjoint code paths). Table routes (route.ts, columns/route.ts, rows/[rowId]/route.ts) merged staging's lock/rename logic with the collab signal emits; deleteRow adopts staging's new signature. table-grid.tsx keeps both the presence props and the lock props.
Fix a blocker surfaced by a full cleanup/simplify audit of the branch: the access-revalidation sweep (staging's workflow-only #5917) treated every entry in socket.rooms as a workflow id, but the generalized multi-room model puts namespaced files/tables/file-doc rooms on the same io. It would resolve those as bogus workflows, get null, and evict files/tables collaborators every ~30s. collectScanTargets now decodes each room name with parseRoomName and sweeps only workflow rooms; added a regression test and fixed the now-false TSDoc. Other audit fixes (all behavior-preserving): - workflow.ts reuses resolveAvatarUrl (drops db/user/eq imports duplicated from avatar.ts) - PresenceAvatars: mr-1 was baked into the shared component, silently adding a margin to the workflow sidebar stack; moved to an optional layout className, re-applied on the tables/file-doc header surfaces only - table DELETE routes only signal collaborators when rows were actually removed (matches PUT) - events.ts definition kind: drop the never-emitted reason:'schema', fix its doc - event-log: rename buildMemory -> buildEntry (it builds the entry on the Redis success path too, not just the memory fallback) - remove dead resolveWorkspaceIdForRoom export; parallelize per-socket removals in handleWorkflowDeletion; gate the table columnIndexById map on remote selections; move file-doc module TSDoc off the FileDocOwner interface; fix a stale @returns
…lab gaps Validated each issue with subagents before implementing the cleanest fix. - tables LEAVE in-flight-join race (B8): the table handler tracked no current-table intent, so an unscoped/same-table leave during an in-flight authorize left the socket stranded in the room (present in the roster, broadcasting a ghost until disconnect). Mirror workspace-files: a closure-local currentTableId + a leave that advances joinGeneration to cancel the racing join. + 3 regression tests. - v1 + copilot live-collab signal gap (D1): tables edited via the v1 public API or Sim/copilot emitted no edit/schema signal, so open collaborators didn't live-update. Add the signals at those call sites (add-only, matching the existing route seam) — never in the service, so execution writes can't double-emit. Sync-only for copilot bulk ops, guarded on affected/deleted count; async job branches stay covered by their kind:'job' events; create/delete/get untouched. - table join read consolidation (B5): sweepStalePresence returns its roster so the same-tab dedup reuses it instead of a second getRoomUsers. - shared authorize slice (B6): extract only the guard-safe authorize->allowed branch into resolveRoomJoinAuth, shared by the three room handlers (the full preamble stays inline — file-doc's generation capture sits mid-ladder and must not move). - resize-revert flicker (E3): a peer's value-less metadata event forces a refetch that could momentarily revert a just-finished local resize; a pendingWidthWriteRef keeps local widths leading until the width PUT settles. - embedded-mode stray emit (E5): gate emitCellSelection on a bound table id so the embedded surface stops broadcasting cell selections the server drops.
…nce sweep Follow-ups from a comprehensive review of the branch: - copilot batch_update_rows and import_file's inline append branch wrote rows but emitted no live-collab signal, so collaborators didn't see those edits live (the append's sibling replace branch already signalled). Add the guarded signal to both, matching the internal route. - sweepStalePresence now reads the roster before the fetchSockets liveness probe and returns it on a probe failure, so same-tab dedup still runs during a transient fetchSockets outage instead of being skipped. - reword an internal comment off the retired "mothership" term.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Disconnect and access control move to safer multi-room behavior: cleanup runs on New realtime surfaces include workspace-files join/leave (Socket.IO only, no Redis presence), table presence with validated cell-selection relay and serialized joins, and collaborative file documents via in-memory Yjs sync/awareness relay with seeding, client-id anti-spoofing, and server-authenticated presence rosters. Shared Dependencies add Reviewed by Cursor Bugbot for commit f9f43c3. Configure here. |
Contributor
Greptile SummaryFollow-up on table presence JOIN/LEAVE race threads: serialization + generation guards and always-rollback on join failure address the prior interleaving bugs.
Confidence Score: 5/5The PR appears safe to merge with respect to the previously reported table join/leave race issues; those failures are no longer present in the current handlers. Prior table JOIN/LEAVE interleaving and stranded Socket.IO membership paths are closed by per-socket op serialization, generation cancellation, and always-on join rollback, with matching regression tests; no remaining blocking failure from those threads.
|
| Filename | Overview |
|---|---|
| apps/realtime/src/handlers/tables.ts | Serialized JOIN/LEAVE with generation cancel and always-rollback close the prior table join race threads; no incomplete fix remains in those paths. |
| apps/realtime/src/handlers/tables.test.ts | Covers switch skip, leave-during-auth cancel, non-cancelling cross-table leave, and stranded-membership rollback. |
Reviews (15): Last reviewed commit: "fix(files): drop late sync frames once f..." | Re-trigger Greptile
…n; drop no-op eviction cleanup Review round on #5991: - Table join re-checked the generation only once after authorize, then awaited leave/sweep/avatar before joining + registering presence. A table switch or leave in that window stranded the socket in the wrong room, and the failure catch could tear down a newer successful join. Resolve the avatar up-front, re-check generation immediately before the membership commit (matching the file-doc join), and skip the rollback/error for a superseded join. + a post-authorize-window regression test. - access-revalidation cleanup treated removeUserFromRoom's no-op false as a transport failure and re-enqueued a still-connected socket forever. Only retry when the socket is still mapped to the room (a healthy null mapping means the entry is already gone). Repurposed the expired-mapping test to lock it.
Collaborator
Author
Collaborator
Author
|
@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 0383d79. Configure here.
Round 2 on #5991: a superseded join's leave-prior could still run — during its getRoomForSocket await a newer join commits to its room, so currentRoom is that newer room and the superseded join would leave/remove/broadcast it before the final guard aborts. Re-check the generation immediately after the lookup await, before the leave mutation. Extended the post-authorize-window test to assert the superseded join never tears down the newer join's room.
Resolve files.tsx import conflict: keep useWorkspaceFilesRoom (realtime Files presence), drop the stale MoveOptionNode import from files/move-options (deleted by staging's folder cutover #6045; now provided by components/folders).
…room impl (#6053) * feat(realtime): live workspace tables list, sharing one invalidation-room impl Bring the tables list to parity with the files list: a create/rename/move/delete/ restore now propagates to every viewer live instead of waiting out the 30s staleTime. Following the files pattern, but factoring the two into one shared implementation rather than copy-pasting. - add ROOM_TYPES.WORKSPACE_TABLES + its authz resolver (workspace-id-addressed, reuses the workspace resolver like workspace-files) - extract setupWorkspaceInvalidationRoom (server) and useWorkspaceInvalidationRoom (client) — the presence-free, workspace-scoped live-list room; files and tables now both bind to it, so they can never drift. Event/room names derive from the room type. Replaces the standalone workspace-files handler + hook - notifyWorkspaceTablesChanged fanout fired from the table service (createTable, renameTable, moveTableToFolder, deleteTable, restoreTable) so it covers both the HTTP routes AND copilot, which call the service directly - relay /api/workspace-tables-changed endpoint; wire the hook into the tables page - consolidate the handler test into one suite run against both room types * feat(realtime): live tables list also covers table-folder mutations Fold in the follow-up: a table folder create/rename/move/delete/restore now propagates to the tables list live too, so the browser is fully consistent. - generic notifyFolderResourceChanged(resourceType, workspaceId) dispatches the workspace live-list signal by resource type (a map, not a special-case if), so file/knowledge_base/workflow are no-ops today and gain liveness by adding a map entry when they adopt an invalidation room - fired from the shared folder lifecycle (createFolder/updateFolder/deleteFolder/ restoreFolder), covering routes AND copilot - the tables room hook now invalidates the table folders query too, not just the tables list, since the page renders both * fix(realtime): skip per-table live-list notify during a folder cascade A folder delete/restore already fires one folder-level notifyFolderResourceChanged for the whole subtree, but the cascade also calls deleteTable/restoreTable per table — each awaiting its own notifyWorkspaceTablesChanged. A folder with many tables would run N+1 sequential relay calls (each bounded by NOTIFY_TIMEOUT_MS), blocking the mutation. Add a skipNotify option the cascade passes so only the one folder-level notify fires.
Resolve the api-validation baseline conflict to the merged tree's true route count (994, audit-reported): realtime-rooms 993 + one app/api route added by staging. Staging's other new commits (outlook calendar, db date binds, desktop settings, tables sort labels) auto-merged.
…t fixes (#6059) * fix(collab-doc): make server-side seed conversion work under Next 16 / Turbopack Opening a file left both collaborators read-only and stalled ~12s: the server-side seed (markdown -> Yjs, run through the headless editor engine) was failing, so the doc never seeded and the editor never left its readiness gate. Two root causes, both latent until a real build/runtime (typecheck + unit tests don't exercise either), surfaced by the Next 16 upgrade: 1. Build boundary: the server seed route imported the shared editor schema (`createMarkdownContentExtensions`), which pulled in the React node-view components (`useEffect`) -> 'client component in a Server Component'. Split each node's React-free schema into its own `*-schema.ts` (code-block, image, raw-markdown-snippet); the client editor still injects the React node views via the existing `nodeViews` param, unchanged. 2. Runtime DOM: the converter installs a jsdom `window` on `globalThis`, but Turbopack's server bundle gives bundled `@tiptap/core` a `window` that does NOT read `globalThis`, so `elementFromString` threw 'no window object available'. Externalize the `@tiptap/*` packages the converter uses (native Node require, so their `window` reads the real global) and fix the converter's DOM guard to gate on `window` (what TipTap checks) with no sticky flag. Verified: seed route returns 200 with the Yjs update; 514 collab-doc + editor tests pass; schema byte-identical after the split. * feat(collab-doc): persist the Yjs binary and load it on cold-start (Hocuspocus pattern) Adopt the industry-standard Hocuspocus store/load-document pattern so a cold room open loads the file's last-persisted Yjs binary directly instead of re-converting markdown -> Yjs on every open. Rebuilding the CRDT from markdown on each connect is the exact anti-pattern Tiptap/Yjs warn against (fresh client ids -> duplicated content); it also forced the fragile server-side headless-editor conversion on every open. Now conversion runs only on a genuine first open or an external markdown edit. - new table workspace_file_collab_state(file_id PK->workspace_files cascade, doc_state bytea, source_hash, updated_at): the Yjs binary + a hash of the markdown it was derived from (bounded <=~1MB by the 256KB round-trip gate). Mirrors Hocuspocus's extension-database (binary in a DB column). Migration 0275. - persist upserts the binary (tagged with the exact markdown just written) - cold-start seed returns the cached binary when its source_hash matches the file's current markdown; otherwise converts (and the next persist refreshes the cache) - also externalize yjs / y-protocols / lib0 alongside @tiptap: bundling loaded a second yjs copy, so @tiptap/y-tiptap's 'instanceof Y.XmlElement' failed on app-created nodes ('Unexpected case') during Yjs -> markdown Verified end-to-end: seed -> persist -> seed returns the exact persisted binary (a cache hit, no re-conversion). 18 collab-doc + 51 realtime file-doc tests pass. * fix(collab-doc): best-effort cache read + drop dead barrel - seed: a cache-read failure (transient DB error, not-yet-migrated cache table) no longer aborts a cold room open — the durable markdown is already in hand, so fall through to conversion. Symmetric with persist's best-effort cache write. Addresses the Cursor Bugbot finding on the read/write asymmetry. - remove the collab-doc index.ts barrel: nothing imported it (every consumer uses direct ./seed / ./merge / ./converter imports), so it was dead re-export surface. De-export COLLAB_DOC_FIELD accordingly — it is used only inside converter.ts.
# Conflicts: # apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx # packages/db/migrations/meta/0275_snapshot.json # packages/db/migrations/meta/_journal.json # scripts/check-api-validation-contracts.ts
…not just edit_content (#6070) * fix(collab-doc): stream every external file write into open editors, not just edit_content A copilot/mothership edit to an open markdown file did not appear live in another user's editor: the live-doc merge bridge (mergeEditIntoLiveFileDoc) was wired into the edit_content tool ONLY. Every other server-side write — the file tool (/api/tools/file/manage), function_execute (/api/function/execute via writeWorkspaceFileByPath), create_file overwrite, and the PUT /content route — went straight to updateWorkspaceFileContent and skipped the merge, so the durable file changed but the open editor never updated. Confirmed from live logs (the mothership 'Prepend sentence' ran read + file + function_execute — zero apply-edit calls) and Redis (the prepended text was absent from the doc stream). Centralize the merge at the one chokepoint every external writer shares: - updateWorkspaceFileContent gains an opt-out "syncLiveDoc" (default on) and, after the durable write, merges markdown writes into any open collaborative doc (best-effort; no-op when nobody has it open). Any current OR future writer is covered automatically. - persist.ts opts out (syncLiveDoc:false) — it IS the doc→markdown projection, so merging it back would be a persist→merge→persist self-loop. - create_file opts its empty shell out (real content arrives via a later write) so an open editor never flickers to empty on overwrite; threaded through writeWorkspaceFileByPath. - edit_content drops its now-redundant explicit merge call (the chokepoint handles it). - binary writers (image/video/audio/ffmpeg/download) are naturally excluded — the merge is gated to markdown, the only format the collaborative editor renders. Also bump the api-validation route baseline 994→996 to match the true route count already on this branch (pre-existing ratchet drift from an earlier merge; NOT added by this PR). * fix(collab-doc): defer setEditable out of the render phase (flushSync warning) The collab editability-reapply effect called editor.setEditable synchronously. In collab mode isEditable flips from readiness (synced + seeded), which is driven by a Yjs config.observe firing synchronously inside Y.applyUpdate — so the effect can run while React is mid-render. TipTap's React binding commits setEditable's transaction with flushSync, which throws "flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering." Defer the setEditable to a microtask (runs right after the current commit, before paint), guarding against a destroyed editor or a stale value before it fires. Only the collab path (this effect) hit the warning; the streaming/settle effect's setEditable calls run on the non-collab path where isEditable isn't driven by a mid-render Yjs observer.
…ff the render phase (flushSync) (#6073) * fix(rich-markdown-editor): defer non-collab settle/stream mutations off the render phase (flushSync) The non-collaborative streaming/settle effect called editor.setContent / setEditable / setTextSelection / focus directly in the effect body. setContent mounts the custom node views synchronously through the @tiptap/react flushSync path (tiptap#3764), so when this effect runs while React is mid-render it throws "flushSync was called from inside a lifecycle method." This is the second flushSync source (the collab editability effect was the first, fixed separately); it fires on the agent-streaming-into-a-non-collab-editor surface. Defer the effect-body view mutations to a microtask via a small runOffRender helper (runs right after the current commit, before paint; no-ops if the editor was torn down). The settle block is deferred as ONE microtask so setContent -> collapse selection -> setEditable -> focus keep their order. The streaming rAF tick is left untouched — it already runs off-render, so it keeps writing content directly. queueMicrotask is TipTap's own documented remedy for this warning. 497 rich-markdown-editor tests (incl. stream-settle-selection) pass; tsc + lint + api-validation + boundary + prune green. Needs a live check: stream an agent into a non-collab markdown file and confirm it still renders smoothly. * chore(rich-markdown-editor): trim verbose flushSync-defer comments * fix(rich-markdown-editor): drop superseded settle/stream microtasks via a run token runOffRender previously only guarded editor.isDestroyed, so if React ran the next reconcile pass (a newer stream or settle) before a queued microtask flushed, the stale microtask could still apply setContent/setEditable/setTextSelection over the newer state. Tag each effect run with an incrementing token; a deferred mutation applies only when its run is still the latest (and the editor is alive). A run token fits this effect's several early-return exits better than a per-exit cleanup flag. Addresses Greptile/Cursor review. * fix(rich-markdown-editor): never drop the settle selection-collapse under a superseded run The run token drops a superseded settle's microtask, but the settle had already flipped its state flags synchronously — so a pre-empting steady-sync run took the non-settle path and never collapsed the selection, leaving a post-stream select-all painting the leaf-in-selection decoration. Track the collapse as a debt (pendingCollapseRef): whichever deferred run ultimately applies — settle or the steady-sync path — clears it, so the collapse runs exactly once on the latest content. Addresses the Cursor review finding.
…nel (#6081) The table cell-selection presence room was joined only on the dedicated /tables/[id] page (useTableRoom was passed an empty id in embedded mode). Join it in embedded too, so the mothership chat resource panel shows collaborators' live cell selections and broadcasts the local one. tableId is already resolved from props in embedded (the data event stream already uses it un-gated), and authz runs on join, so this is safe. Avatars are unaffected — they render only in the !embedded Resource.Header, so the panel gets carets without avatars.
…obbers an out-of-band edit (#6085) * feat(collab-doc): optimistic-concurrency guard so persist never clobbers an out-of-band edit The relay projected the live Yjs doc back to durable markdown unconditionally (last-write-wins), so a persist already in flight when an external write landed could overwrite it. Add RFC 7232 If-Match optimistic concurrency end to end, reconciling through the CRDT (never rejecting user work): - updateWorkspaceFileContent gains an expectedUpdatedAt guard: the write commits only if the file is still at that version (checked against the SELECT ... FOR UPDATE-locked row, so it is atomic with the write), else it throws the new ContentVersionConflictError without clobbering. - persistFileDoc takes expectedVersion and returns a discriminated result (persisted | missing | conflict). On conflict it returns the current durable content + version instead of writing. - The relay tracks the durable version its live doc is synced to — set on seed, advanced when a durable write is merged in (apply-edit carries the version), and on each successful persist. It is held cluster-wide in Redis (filedoc:syncver:{name}) so whichever task persists reads the same version, with the per-room value as the single-pod fallback. - flushPersist sends that version as If-Match. On a conflict it merges the current durable content into the live doc (so the out-of-band edit AND the live edits converge) and retries (bounded), so even a last-leave flush racing an external write persists the reconciled result rather than losing the session's edits. Threads the version through the seed + persist contracts and the apply-edit payload. No schema change (reuses workspace_files.updatedAt as the version token). Tests: app-side CAS (match writes, mismatch throws + cleans up the orphan upload), relay conflict handled gracefully without clobber/loop; 236 realtime + 76 sim collab/uploads tests, tsc x2, lint, api-validation, boundaries, prune all green. * chore(collab-doc): heartbeat-refresh the synced-version key TTL alongside its stream Keep filedoc:syncver:{name} alive as long as the room's stream (it was only re-set on seed/merge/persist), so an open-but-idle doc's persist If-Match token can't expire and force a needless reconcile. * fix(collab-doc): stop persist-conflict retries when there is no live doc to reconcile On an If-Match conflict with no live doc to reconcile into (last collaborator gone, no shared stream), applyMarkdownToLiveFileDoc returns no-live-room; re-projecting the same pre-teardown snapshot would only re-conflict, so break the retry loop immediately and leave the out-of-band (durable) content authoritative — the intended conflict policy. Addresses Greptile review. * fix(collab-doc): close three optimistic-concurrency edge cases from review - Single-pod persist retry projected the pre-reconcile snapshot (captureState always returned the initial localState), while the synced version had been advanced by the reconcile — so the If-Match could pass and clobber the reconciled edit. captureState now re-reads the live doc on each attempt (falling back to the pre-teardown snapshot only once the room is gone). - The synced version was recorded from this task's own seed FETCH before knowing whether this task's seed actually won; a peer winning with a different version could leave a newer token than the stream content. Record it only inside the didSeed branch (the task whose seed won); peer-seeded tasks read the winner's cluster value. - Persist wrote UNCONDITIONALLY when no version was available (relay version momentarily missing), which could clobber non-empty durable content. It now returns conflict for a non-empty file with no version (reconcile/retry once the version is re-established); an empty file's first write stays unconditional. * fix(collab-doc): defer (not reconcile) on missing version, and use the freshest version token - Missing-version persist now returns 'deferred' instead of 'conflict'. A missing version token (a Redis blip on a peer-seeded task) is NOT a genuine out-of-band change, so triggering a reconcile would wipe live edits (incoming-wins) even though nothing changed durably. Deferred means: don't write, don't reconcile — leave the edits in the stream and let a later persist write them once the version is re-established. - currentVersion now takes the MAX of the cluster (Redis) and local room versions rather than always preferring Redis, so a lagged/failed fire-and-forget Redis set can't shadow a newer local value and cause spurious If-Match conflicts. Versions are monotonic epoch-ms, so the larger is the later sync. * fix(collab-doc): make persist If-Match teardown-race-immune and recover missing version on final flush Close two last-leave concurrency holes Cursor flagged: - Thread the reconciled version LOCALLY through the persist retry loop. After a conflict+reconcile the correct next If-Match is exactly result.version, so carry it in a local var instead of re-deriving from room.syncedVersion/Redis. On a last-leave flush destroyRoomIfIdle removes the room from the map before the async flush finishes, so mergeMarkdownIntoRoom's recordVersion can no longer update room.syncedVersion — threading makes each retry's precondition correct by construction, immune to that dropped mutation and to a best-effort Redis re-read. - Cache the resolved version back into room.syncedVersion in currentVersion() so a peer-seeded/tail-only task (which never sets it locally) or a later transient Redis read failure still resolves it from the last value seen (monotonic max, never regresses). - On a FINAL flush, briefly retry resolving the If-Match when the version read momentarily fails, rather than deferring and stranding the session's edits in the TTL'd stream — the version is cluster-wide and heartbeat-refreshed. * fix(collab-doc): stamp cluster sync version the moment the seed wins, before the liveness guard The winning seeder set the If-Match token (room + Redis filedoc:syncver) only after the liveness/seeded guard that follows seedIfEmpty. But the tailer can integrate the just-appended seed DURING the seedIfEmpty await, so isDocSeeded(room.doc) is already true when the guard runs and it returns early — leaving the stream holding seed content with no cluster version. Later persists then send no If-Match, the app returns `deferred`, and session edits stay only in the TTL'd stream (the exact stranding this PR prevents elsewhere). Move the version stamp to immediately after seedIfEmpty wins, before the guard. Recording it only once our seed won (not from the fetch) is preserved, so it still can't shadow a peer's winning seed. * fix(collab-doc): make the synced-version token monotonic at every write site The If-Match token is written fire-and-forget from the seed stamp, merges, and persists, both locally and to Redis. An out-of-order write (e.g. a seed's lagged setSyncedVersion landing after a later merge's) could regress it below the version the live doc already incorporates, causing spurious If-Match conflicts — and on a last-leave flush with no live room to reconcile into, a spurious conflict leaves durable authoritative and drops the session's edits. - setSyncedVersion now writes via SET_VERSION_IF_NEWER_SCRIPT (Redis-side compare-and-set): it overwrites only when the new value is greater, refreshing the TTL either way. - recordVersion / the persisted branch / the seed stamp all take Math.max instead of assigning room.syncedVersion directly. Versions are monotonic epoch-ms, so "newer" is a plain numeric compare, exact within a Lua double. * fix(collab-doc): close three last-leave persist edge cases from review - Stale snapshot after reconcile (High): the multi-task captureState fell back to the pre-await localState snapshot even after a reconcile advanced ifMatch, so a failed stream re-read could persist the pre-reconcile state against the new version and clobber the out-of-band edit the reconcile just incorporated. NULL localState after a reconcile so a failed read aborts instead. - Lock miss aborts reconcile (Medium): a merge-lock acquisition failure returned 'no-live-room', indistinguishable from an absent stream, so flushPersist treated transient contention as terminal. Return a distinct 'merge-unavailable' and handle it as retry-later (edits stay in the stream), never as "nothing to reconcile into". - Peer syncver never recovers (Medium): the winner's setSyncedVersion was fire-and-forget with swallowed errors — the only way a peer-seeded task learns the durable version — so a dropped write left that peer deferring forever. Make it retry (bounded) like appendUpdate/seedIfEmpty; the monotonic script keeps a racing retry a no-op. * fix(collab-doc): scope the persist If-Match to a content version so metadata bumps can't clobber edits The optimistic-concurrency validator was `updatedAt`, which rename/move/delete/restore also bump with no content change. A racing live-doc persist then saw a stale token, got `conflict`, reconciled the pre-edit durable body via updateYFragment (incoming-wins on overlap), and wiped the user's in-flight edits. Scope the validator to content (RFC 7232 semantics — validate the representation, not the row): - New `workspace_files.content_updated_at` (NOT NULL, `now()` fast-default — no table rewrite). Advances ONLY on content writes (upload / overwrite / create); metadata writes never touch it. - The FOR UPDATE CAS, the merge-notify version, and the seed version all use `content_updated_at`. A rename now leaves it unchanged, so the persist If-Match still matches -> no spurious conflict, no reconcile, no lost edits. Genuine out-of-band content writes still conflict and reconcile. - Consolidated the collab schema into one migration (the collab-state table + the new column) per request, rather than a separate follow-up migration. Relay/store/contracts unchanged (still a numeric monotonic version). * chore(collab-doc): condense the densest persist comments (no behavior change) Cleanup pass: tighten the three longest comment blocks added while hardening the persist path (currentVersion cache, ifMatch threading, final-flush version retry) without dropping any invariant. No dead code found (biome lint clean; all new symbols referenced). * fix(collab-doc): persist must return the content version, not updatedAt Follow-up to the content-scoped If-Match: persistFileDoc still returned `updatedAt` as the version in both the persisted and conflict results, while the CAS/seed/merge all guard on `content_updated_at`. A content write sets both to the same instant, so it was coincidentally correct — until they diverge: if a metadata write bumps `updatedAt` past `content_updated_at`, the conflict path returned the larger `updatedAt`, so the relay's re-persist sent an If-Match the CAS (which checks `content_updated_at`) could never match → perpetual conflict → dropped reconciled edits. Return `contentUpdatedAt` in both paths so the relay's token always matches what it's checked against. * fix(collab-doc): defer persist whenever the version is missing; guard the content-version test - Empty-file CAS race (Medium): the unconditional-write carve-out for size===0 read `record.size` outside the write transaction, so a concurrent first content write could land after the check and be clobbered. With content_updated_at NOT NULL every existing file always has a real version, so a missing expectedVersion is always transient — always defer, never write unconditionally. Removes the TOCTOU hole. - Content-version test (Low): the merge-chokepoint test kept updatedAt == contentUpdatedAt, so it passed even if wired to the wrong field. Mock distinct values and assert contentUpdatedAt, so a regression to updatedAt now fails the test. * fix(collab-doc): don't reconcile a conflict the live doc already reflects (would wipe newer edits) flushPersist reconciled the durable body into the live doc on every conflict. But when the conflict comes from a racing self-persist (or an apply-edit the chokepoint already merged), the durable body is a STALE SUBSET of the live stream, and the incoming-wins updateYFragment merge moves the doc backward — wiping newer in-flight edits, which the retry then persists. Before reconciling, re-check the freshest synced version. If it already covers the conflict version, the live doc has already incorporated that content (or is ahead), so skip the reconcile and just retry with the freshest version as If-Match — the re-projection captures the current live stream, preserving every edit. Only a genuine out-of-band change the live doc hasn't incorporated (freshest < conflict version) is reconciled in. freshest never exceeds the durable version, so this can't loop. * fix(collab-doc): make content_updated_at monotonic per file; skip-reconcile can't loop The If-Match token was stamped with app-local new Date() on each content write, so cross-instance clock skew could stamp a later write with an EARLIER content_updated_at — breaking the version ordering the whole optimistic-concurrency scheme (and the skip-reconcile branch's freshest>=version assumption) depends on. Under skew the relay's monotonic syncedVersion could exceed the durable version, sticking the If-Match: persist conflicts forever, exhausts retries, drops the session's edits. - Stamp content_updated_at strictly after the current committed value (we hold the row's FOR UPDATE lock): new Date(max(now, currentFile.contentUpdatedAt + 1ms)). Monotonic per file regardless of clocks; also removes same-millisecond collisions. updatedAt stays plain wall-clock (display/sort). - Skip-reconcile branch retries with result.version (the durable value the CAS will match), never freshest (which could exceed it and loop). Belt-and-suspenders now that the version is monotonic. * refactor(collab-doc): drop the destructive in-persist reconcile; adopt-version-and-retry on conflict The in-persist reconcile projected the durable body back over the live doc via updateYFragment ("make the doc match"). That is destructive: when the live stream is already ahead — the common case, because the write chokepoint (mergeEditIntoLiveFileDoc) already merged the out-of-band change into the stream — it moved the doc backward and wiped newer in-flight edits. This produced a run of races (stale snapshot, wipe-newer-edits, version-lag skip miss) that a full-document reconcile fundamentally can't avoid, since deciding when it's safe relies on a laggy cross-task version token. Remove it. On conflict, adopt the durable version as the new If-Match and retry: captureState re-reads the current stream (which holds the out-of-band change AND the live edits), so the re-projection persists the converged result. The durable change reaches the live doc via the chokepoint, never here. Trade-off: the only unmerged out-of-band write is one whose chokepoint merge itself failed (rare, logged), which we accept over the frequent reconcile-wipes-edits race. - flushPersist: conflict -> ifMatch = result.version, retry (bounded). No applyMarkdownToLiveFileDoc. - conflict response drops `markdown` (contract + relay type + persist) — no body needed, saves a blob fetch. applyMarkdownToLiveFileDoc stays (still used by the apply-edit route / the chokepoint). * fix(collab-doc): don't let a last-leave conflict retry clobber via the stale local snapshot Regression from dropping the reconcile: on conflict the retry adopts result.version and re-reads captureState. But after single-pod last-leave teardown the room is already destroyed, so captureState falls back to the pre-teardown localState (which lacks the out-of-band change); the retry then CAS-passes and overwrites the committed external write — undoing the external-wins last-leave policy. Null localState on the first conflict, so the retry can only use freshly-read authoritative state (stream / live doc). When none is available (single-pod room gone, or a transient stream-read failure) captureState returns null and the retry stops, leaving durable content authoritative. Covers both the single-pod and multi-task-stream-unavailable variants of the stale-snapshot clobber. * fix(collab-doc): stop (don't re-persist) on a persist conflict — closes the commit-window clobber The conflict retry adopted the durable version and immediately re-persisted the current stream, assuming the stream already held the out-of-band change. But an external write commits durable BEFORE its chokepoint merge (mergeEditIntoLiveFileDoc) reaches the stream, so a persist landing in that window CAS-passed with a stream that still lacked the external content and clobbered the committed write — not just the rare merge-failed path, but a race on every external write, worst at last-leave flushes. Make persist a single attempt: on conflict, STOP and leave durable authoritative. The chokepoint merges the change into the stream and — only once it is actually there — advances the synced version via its own recordVersion; a later flush (debounced or final) then projects the converged stream with a matching token. The session's edits stay in the stream meanwhile. The conflict handler deliberately does NOT advance the synced version, or the next flush would clobber with a still-behind stream. Removes the retry loop and PERSIST_CONFLICT_RETRIES.
…ancel, and column run (#6094) * improvement(tables): fire the live-rows signal on async delete, run cancel, and column run These three table operations mutate row data but emitted no `rows` change signal, so open editors' grids stayed stale until a manual refresh (enrichment *results* already stream live via `cell` events; these are the bulk paths that don't emit per-cell events): - Async row delete (`runTableDelete`): signal as rows drop out (throttled with the existing progress event) and once more on completion — the `job` progress event only drives the delete meter, not the rows query. Covers the delete-async route and the copilot bulk-delete, since both share the runner. - Cancel runs (`cancel-runs` route): cancelling clears each affected row's exec state; the `dispatch: cancelled` events drop the run overlay but the client then renders authoritative DB state, so refetch. Only when something was actually cancelled. - Run column (`columns/run` route): starting a run bulk-clears the target group's cells to pending; refetch so the cleared cells show. Only when a dispatch was actually created. Guarded so no signal fires on a no-op/failure. Adds a delete-runner test asserting the completion signal. * fix(tables): guarantee the live-rows signal on every mutating path (review) - Delete runner (Greptile P1): a batch could commit and the job then cancel/supersede before the next throttled progress signal or `markJobReady`, bypassing both signals and leaving deleted rows on screen. Track `deletedAny` and fire the grid refetch in a `finally`, so it runs on EVERY exit — completion, cancel/supersede, mid-batch lock, or a rethrown error after a partial delete. - cancel-runs / columns/run routes (Cursor): the `cancelled > 0` / `if (dispatchId)` guards don't always reflect DB row changes — cancel tombstones exec state even when 0 dispatches were active, and a run bulk-clears cells then can return a null dispatchId. Signal unconditionally; a stale-but-harmless refetch beats a missed one. - Tests: assert the delete signal fires on the mid-run-cancel-after-delete path and NOT when nothing was deleted. * fix(tables): mark deletedAny before the page delete so a mid-page lock still refreshes the grid `deletePageByIds` commits in internal batches, so a delete lock landing mid-page can persist earlier batches and THEN throw TableLockedError — the catch returns without a count, so setting `deletedAny` from the return value missed it and the finally skipped the grid refetch. Set `deletedAny = true` before the call (any attempt may commit rows); an attempt that commits nothing only over-refetches (harmless). Adds a test asserting the signal fires when a page throws a mid-page lock.
* fix(files): make embedded resource file view collaborative The /chat resource panel rendered saved files through FileViewer without the collaborative opt-in, so a file open on the Files page and the same file open in the embedded panel never joined the same file-doc room — no live carets and no live content sync between the two surfaces. Pass collaborative on the EmbeddedFile FileViewer. Collaboration still self-gates on canEdit + non-streaming + workspace doc, so the agent token-stream preview (the dedicated streaming-file path, canEdit=false) is untouched. * fix(files): refcount file-doc room membership per shared socket Two collaborative surfaces in one tab (the Files editor and the embedded chat resource panel) share one Socket.IO connection, so both providers for the same file JOIN the same room over that socket. The server's LEAVE does socket.leave(name) with no membership refcount, so the first provider's destroy() would strand the second still-mounted one — no more live content or presence. Count live providers per file per socket (keyed by the stable Socket object, so it survives reconnects) and emit LEAVE only when the last provider for a file tears down. The single-provider path is unchanged (0->1->0).
# Conflicts: # apps/sim/app/api/table/[tableId]/cancel-runs/route.ts # apps/sim/app/api/table/[tableId]/columns/run/route.ts # apps/sim/app/api/table/[tableId]/rows/route.ts # apps/sim/app/api/v1/tables/[tableId]/rows/route.ts # scripts/check-api-validation-contracts.ts
…ve (#6100) Table views (named filter/sort/layout presets) are table-wide shared state — every reader sees every view — but view create/update/delete had no realtime signal, so a collaborator only saw another user's view changes on their own staleTime/focus refetch. Add a 'views' table event kind + signalTableViewsChanged, emitted from the views service (createTableView/updateTableView/deleteTableView, on real success only), and a client handler that invalidates the views query alone (no rows/definition refetch — a view is presentation state on the loaded table). Mirrors how row/schema/metadata changes already propagate.
#6101) - events.test.ts: signalTableViewsChanged appends a single 'views' event carrying the tableId (through the real memory buffer). - views/service.test.ts: create/update/delete emit signalTableViewsChanged on real success, and DON'T on a no-op (a PATCH/DELETE targeting a missing view changes nothing, so it must not signal). Mirrors delete-runner's signal-path coverage; drives the DB via the shared dbChainMock. - Add tableViews to the comprehensive @sim/db/schema test mock so the service tests can queue the in-transaction existence row.
The staging merge unioned realtime-rooms's own `as unknown as` cast (lib/collab-doc/converter.ts) with staging's zod-recursive-type cast (lib/api/contracts/tables.ts), so the non-test double-cast count is 9 — both casts pre-existed and were individually accepted on their branches. Also tighten rawJsonReads 6->5 to the true current count. Fixes the strict API contract boundary audit on realtime-rooms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integration branch for the realtime rooms effort (#5929–#5971) plus this session's catch-up, cleanup, and fixes.
RoomRef+roomName/authorizeRoom, backward-compatible (workflow room name stays the bare id)This session
stagingin (resolved the fix(realtime): evict revoked collaborators from live workflow rooms #5917 access-revalidation vs multi-room collision; ported it onto the generalized API)/cleanup+ reuse/altitude review)Type of Change
Testing
check:api-validation:strict, monorepo boundaries, realtime prune graphChecklist