From 483e338a5491844c40c71ca7520200f8daad5cae Mon Sep 17 00:00:00 2001 From: xarmian Date: Fri, 8 May 2026 23:14:56 -0400 Subject: [PATCH] feat(collab): drop conservative content-skip when collab active + applier toast (TASK-1262) (#460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Drop TASK-1243's content-skip when collab is active The conservative `item = { ...updated, content: item.content }` preservation in the SSE/sync handlers was protecting against clobbering a user's mid-keystroke edit with a stale content snapshot. Under collab the editor reads from Y.Doc — NOT the content prop — so the Editor.svelte $effect's `if (ydoc) return` gate at line 810 makes adopting `updated.content` harmless to the live editor while keeping `item.content` fresh for downstream consumers (UI summaries, search-index hints, subsequent share-page renders). For non-collab viewers (view-only, raw mode, items where canEdit=false) the content-skip stays — those paths DO render from item.content via the prop $effect, and adopting a stale SSE snapshot mid-keystroke would clobber unsaved chars. Applied to all four adoption sites: - SSE item_updated - SSE item_restored - syncService incremental update - syncService full-refresh fallback ## Applier-success toast The applier handler (wired in TASK-1259's absorption of TASK-1262 scope) silently called setContent. Users would see their editor change under them with no UI hint. Adds a brief toastStore.show('External edit applied', 'info') after the setContent succeeds; preserves the late-apply guard so toasts only fire on actual mutations. ## Acceptance criteria - [x] `pad item update REF --stdin < new.md` while two browser tabs are open: both tabs reflect the change (the applier path fires setContent on the longest-connected tab; ops broadcast to peers; SSE adoption keeps item.content fresh). - [x] `pad item update REF --status done` (field-only): both tabs see the field change via SSE; no editor disruption (the guard `input.Content != nil` skips the applier branch entirely; SSE adoption updates fields atomically). - [x] Designated client disconnects mid-flight: server retries next applier (TASK-1257 logic; pending follow-up TASK-1268 for the all-applier-failed case). - [x] Toast: "External edit applied" surfaced. Parent: PLAN-1248 --- .../[collection]/[slug]/+page.svelte | 56 +++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte b/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte index 5be8a36a..be9f3b16 100644 --- a/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte @@ -226,7 +226,27 @@ const updated = await api.items.get(reqWsSlug, reqItemSlug); // Bail if the user navigated away before this resolved. if (!item || item.id !== reqItemId) return; - item = { ...updated, content: item.content }; + // Drop TASK-1243's conservative content-skip + // when collab is active (TASK-1262). Under + // collab the editor reads from Y.Doc, NOT + // from the `content` prop — Editor.svelte's + // `if (ydoc) return` gate at the prop $effect + // makes adopting updated.content harmless to + // the live editor while keeping item.content + // fresh for downstream consumers (UI summaries, + // search index hints, etc.). + // + // Non-collab still preserves the local content + // to avoid clobbering an unsaved typing burst — + // the saveStatus guard above already skips when + // a debounced save is in flight, but a user + // mid-keystroke with no save yet pending would + // still lose chars without this branch. + if (collabProvider) { + item = updated; + } else { + item = { ...updated, content: item.content }; + } const links = await api.links.list(reqWsSlug, updated.slug).catch(() => []); if (!item || item.id !== reqItemId) return; itemLinks = links; @@ -239,7 +259,11 @@ try { const updated = await api.items.get(reqWsSlug, reqItemSlug); if (!item || item.id !== reqItemId) return; - item = { ...updated, content: item.content }; + if (collabProvider) { + item = updated; + } else { + item = { ...updated, content: item.content }; + } } catch { // Ignore — will catch up on next event } @@ -277,12 +301,15 @@ // Check if our item is in the changed set const updated = result.changes.updated.find(i => i.id === reqItemId); if (updated) { - // Merge server state without disrupting the editor + // Merge server state without disrupting the editor. + // Same collab-aware adoption rule as the SSE + // handler above (TASK-1262). if (!item || item.id !== reqItemId) return; - item = { - ...updated, - content: item.content - }; + if (collabProvider) { + item = updated; + } else { + item = { ...updated, content: item.content }; + } const links = await api.links.list(reqWsSlug, updated.slug).catch(() => []); if (!item || item.id !== reqItemId) return; itemLinks = links; @@ -294,10 +321,11 @@ try { const updated = await api.items.get(reqWsSlug, reqItemSlug); if (!item || item.id !== reqItemId) return; - item = { - ...updated, - content: item.content - }; + if (collabProvider) { + item = updated; + } else { + item = { ...updated, content: item.content }; + } const links = await api.links.list(reqWsSlug, updated.slug).catch(() => []); if (!item || item.id !== reqItemId) return; itemLinks = links; @@ -497,6 +525,12 @@ } try { editorInstance.commands.setContent(markdown); + // Brief notification so users see WHY their editor + // just changed under them. The applier path is + // triggered by external (CLI / MCP / API) writes + // — a silent setContent would otherwise look like + // a glitch. Per TASK-1262 acceptance criteria. + toastStore.show('External edit applied', 'info'); return true; } catch (err) { console.warn('collab: setContent failed', err);