diff --git a/internal/server/handlers_item_versions.go b/internal/server/handlers_item_versions.go index 5b5695969..d202393b2 100644 --- a/internal/server/handlers_item_versions.go +++ b/internal/server/handlers_item_versions.go @@ -280,6 +280,8 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request } var updated *models.Item + // BUG-3230 U2: the unflushed rows an overwrite restore deleted. + var prunedPendingEdits int if s.collab != nil { // Atomic capture + write + version + op-log prune in one tx: the precheck // hook reads the pre-prune MAX(op-log) AND wipes the op-log inside @@ -347,16 +349,20 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request // baseline capture, so the Postgres reconcile above never sees // a captured baseline for a restore that wrote nothing. Frozen // conns cannot append, so the count cannot move under us. - if !body.OverwritePendingEdits { - n, perr := s.store.CountPendingContentRowsTx(tx, item.ID) - if perr != nil { - return perr - } - if n > 0 { - pendingRefusal = &store.ContentPendingFlushError{ItemID: item.ID, PendingRows: n} - return pendingRefusal - } + // + // BUG-3230 U2: counted on the overwrite path too, because that + // is the restore that DELETES them, and the response names how + // many (warnings.pruned_pending_edits), as a content update's + // does. Counted here, under the same lock, so it is exact. + n, perr := s.store.CountPendingContentRowsTx(tx, item.ID) + if perr != nil { + return perr } + if n > 0 && !body.OverwritePendingEdits { + pendingRefusal = &store.ContentPendingFlushError{ItemID: item.ID, PendingRows: n} + return pendingRefusal + } + prunedPendingEdits = n // Capture the pre-restore seq under the per-item + workspace seq // lock, before any mutation (BUG-2276 P2 — see the baselineSeq note // above). `existing` is the row as read at the top of the update tx. @@ -444,6 +450,14 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request }) } + // BUG-3230 U2: additive and omitempty, like a content update's. On the + // Postgres lost-ack path the reconcile re-reads the row, and the count the + // precheck captured still applies: it was taken inside the transaction that + // landed. + if prunedPendingEdits > 0 && updated.Warnings == nil { + updated.Warnings = &models.ItemWriteWarnings{PrunedPendingEdits: prunedPendingEdits} + } + writeJSON(w, http.StatusOK, updated) } diff --git a/internal/server/handlers_item_versions_bug3031_test.go b/internal/server/handlers_item_versions_bug3031_test.go index c9e3bc73b..e2a48d15d 100644 --- a/internal/server/handlers_item_versions_bug3031_test.go +++ b/internal/server/handlers_item_versions_bug3031_test.go @@ -1,6 +1,7 @@ package server import ( + "encoding/json" "net/http" "net/http/httptest" "strings" @@ -142,6 +143,15 @@ func TestRestoreOverrideDiscardsPendingEdits(t *testing.T) { if got := f.versionCount(t); got != versions+1 { t.Fatalf("override must mint the undo point: versions %d → %d", versions, got) } + // BUG-3230 U2: the response names how many unflushed rows it deleted, as a + // content update's does. + var resp models.Item + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatal(err) + } + if resp.Warnings == nil || resp.Warnings.PrunedPendingEdits != 1 { + t.Fatalf("override restore must report pruned_pending_edits = 1; warnings = %+v", resp.Warnings) + } } // No pending edits: a bare POST restores exactly as before, with no body at all. @@ -152,6 +162,14 @@ func TestRestoreWithoutPendingEditsIsUnchanged(t *testing.T) { t.Fatalf("clean restore: want 200, got %d %s", rr.Code, rr.Body.String()) } assertItemContent(t, f.srv, f.itemID, "v1") + // BUG-3230 U2: nothing was deleted, so no warnings key on the wire at all. + var raw map[string]any + if err := json.Unmarshal(rr.Body.Bytes(), &raw); err != nil { + t.Fatal(err) + } + if _, present := raw["warnings"]; present { + t.Errorf("a restore that deleted nothing must carry no warnings; got %v", raw["warnings"]) + } } // Condition (1) of the ruling: the web initiator drains its own editor through diff --git a/web/e2e/bug-3230-pruned-edits-notice.spec.ts b/web/e2e/bug-3230-pruned-edits-notice.spec.ts new file mode 100644 index 000000000..aec98ad79 --- /dev/null +++ b/web/e2e/bug-3230-pruned-edits-notice.spec.ts @@ -0,0 +1,136 @@ +import { test, expect } from './fixtures'; +import { browserLogin, EDITOR_SELECTOR, SYNCED_BADGE_SELECTOR, SYNC_TIMEOUT } from './lib/collab-helpers'; +import { authJson } from './lib/attachment-viewer'; +import type { Browser, APIRequestContext } from '@playwright/test'; +import type { SuiteFixture } from './fixtures'; + +/** + * BUG-3230 U2: a web write the user sent with "Overwrite them" can DELETE another + * tab's unsaved edits from the op-log (the direct path, when no tab is open). + * The server counts them in `warnings.pruned_pending_edits`; the web discarded + * that count, so the save looked no different from one that deleted nothing. + * + * The state is built for real: tab A types with its collab-snapshot flush + * blocked, the edit is confirmed pending, and A is closed. So there is no + * writer, and the next overwrite takes the direct path and prunes. + */ + +async function createItem(fixture: SuiteFixture, request: APIRequestContext, coll: string, title: string, fields: Record) { + const resp = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/collections/${coll}/items`, { + headers: authJson(fixture), + data: { title, content: 'Original body.', fields: JSON.stringify(fields) }, + }); + if (!resp.ok()) throw new Error(`create failed (${resp.status()}): ${await resp.text()}`); + return (await resp.json()) as { slug: string; ref: string; id: string }; +} + +/** Leave an unsaved edit on the item from a tab that is then gone. */ +async function strandEdit(browser: Browser, fixture: SuiteFixture, request: APIRequestContext, ref: string, id: string) { + const a = await browser.newPage(); + await browserLogin(a); + await a.route(/source=collab-snapshot/, (r) => r.abort()); + // The docs route opens any collection's item in the pane (as BUG-3050's spec does). + await a.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${ref}`); + const editor = a.locator(EDITOR_SELECTOR); + await expect(editor).toBeVisible({ timeout: SYNC_TIMEOUT }); + await expect(a.locator(SYNCED_BADGE_SELECTOR)).toBeVisible({ timeout: SYNC_TIMEOUT }); + await editor.click(); + await a.keyboard.press('End'); + await a.keyboard.type(' stranded'); + await expect + .poll(async () => { + const r = await request.get(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${id}`, { headers: authJson(fixture) }); + return ((await r.json()) as { content_state?: string }).content_state ?? ''; + }, { timeout: 10_000, message: 'PRECONDITION: the stranded edit must be pending' }) + .toBe('applied_pending_flush'); + await a.close({ runBeforeUnload: false }); +} + +test.describe('an overwrite that deletes another tab\'s edits says so (BUG-3230 U2)', () => { + test.setTimeout(120_000); + test.beforeEach(({}, testInfo) => { + test.skip(testInfo.project.name !== 'desktop-chromium', 'one project is enough for a write-path concern'); + }); + + test('conventions inline edit', async ({ page, browser, fixture, request }) => { + const title = `Pruned CONV ${Date.now()}`; + const conv = await createItem(fixture, request, 'conventions', title, { status: 'active', trigger: 'on-implement' }); + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/conventions`); + const row = page.locator('.convention-row', { has: page.locator('.row-title', { hasText: title }) }); + await row.locator('.row-main').click(); + await row.getByRole('button', { name: 'Edit' }).click(); + + await strandEdit(browser, fixture, request, conv.ref, conv.id); + + await row.locator('.edit-textarea').fill('Replacement convention body.'); + await row.getByRole('button', { name: 'Save' }).click(); + const dialog = page.getByRole('dialog', { name: 'Unsaved edits in an open tab' }); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + await dialog.getByRole('button', { name: 'Overwrite them' }).click(); + await expect(page.getByText(/Convention updated\. \d+ unsaved changes? from another tab (was|were) discarded\./)).toBeVisible({ timeout: 10_000 }); + }); + + test('playbook editor', async ({ page, browser, fixture, request }) => { + const title = `Pruned PB ${Date.now()}`; + const pb = await createItem(fixture, request, 'playbooks', title, { status: 'draft' }); + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/playbooks/${pb.slug}`); + await expect(page.locator('.title-input')).toHaveValue(title); + + await strandEdit(browser, fixture, request, pb.ref, pb.id); + + await page.locator('textarea').first().fill('Replacement playbook body.'); + await page.getByRole('button', { name: /^Save/ }).click(); + const dialog = page.getByRole('dialog', { name: 'Unsaved edits in an open tab' }); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + await dialog.getByRole('button', { name: 'Overwrite them' }).click(); + await expect(page.getByText(/Playbook saved\. \d+ unsaved changes? from another tab (was|were) discarded\./)).toBeVisible({ timeout: 10_000 }); + }); + + test('item pane raw-markdown save', async ({ page, browser, fixture, request }) => { + const doc = await createItem(fixture, request, 'docs', `Pruned RAW ${Date.now()}`, {}); + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${doc.ref}`); + await expect(page.locator(EDITOR_SELECTOR)).toBeVisible({ timeout: SYNC_TIMEOUT }); + await page.getByTitle('Raw markdown editor').click(); + await expect(page.locator('.raw-textarea')).toBeVisible(); + + await strandEdit(browser, fixture, request, doc.ref, doc.id); + + await page.locator('.raw-textarea').click(); + await page.keyboard.press('End'); + await page.keyboard.type(' raw-typed'); + const dialog = page.getByRole('dialog', { name: 'Unsaved edits in an open tab' }); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + await dialog.getByRole('button', { name: 'Overwrite them' }).click(); + await expect(page.getByText(/^\d+ unsaved changes? from another tab (was|were) discarded\.$/)).toBeVisible({ timeout: 10_000 }); + }); + + test('version restore confirmed over the pending edits', async ({ page, browser, fixture, request }) => { + const doc = await createItem(fixture, request, 'docs', `Pruned RESTORE ${Date.now()}`, {}); + // A second body mints a version to restore to. + const v = await request.patch(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${doc.slug}`, { + headers: authJson(fixture), + data: { content: 'Second body.' }, + }); + expect(v.ok(), await v.text()).toBeTruthy(); + + await strandEdit(browser, fixture, request, doc.ref, doc.id); + + // This page's own pre-restore drain (BUG-2271) would store the stranded + // edit, since its editor replays it, and leave nothing to discard. Block it, + // so the restore meets the pending edits and asks. + await page.route(/source=collab-snapshot/, (r) => r.abort()); + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs/${doc.slug}`); + await expect(page.locator(EDITOR_SELECTOR)).toBeVisible({ timeout: SYNC_TIMEOUT }); + await page.getByRole('tab', { name: 'Versions' }).click(); + const card = page.locator('#item-timeline .version-card').first(); + await card.locator('.card-header').click(); + await card.getByRole('button', { name: 'Restore this version' }).click(); + await card.getByRole('button', { name: 'Confirm Restore' }).click(); + await card.getByRole('button', { name: 'Discard edits and restore' }).click(); + await expect(page.getByText(/^\d+ unsaved changes? from another tab (was|were) discarded\.$/)).toBeVisible({ timeout: 10_000 }); + }); +}); diff --git a/web/src/lib/components/items/ItemDetail.svelte b/web/src/lib/components/items/ItemDetail.svelte index d36c0aae1..aeaebc6b4 100644 --- a/web/src/lib/components/items/ItemDetail.svelte +++ b/web/src/lib/components/items/ItemDetail.svelte @@ -5,7 +5,7 @@ import { api, PadApiError, isUpdateConflictError, type ImportURLResponse } from '$lib/api/client'; // Its own statement, so units that only use `api` keep their reviewed hash. import { isSupersededWriteError } from '$lib/api/client'; - import { isContentPendingFlush } from '$lib/items/contentWrite'; + import { isContentPendingFlush, prunedEditsNotice } from '$lib/items/contentWrite'; import { pendingEditsDialog } from '$lib/stores/pendingEditsDialog.svelte'; import { keepRefusedRawDraft, @@ -4220,6 +4220,9 @@ if (switchedAway(reqItem, gen)) return; // Kept: the text was not stored, so the pane stays dirty. if (sent === null) return; + // BUG-3230 U2: an overwrite that deleted another tab's edits says so. + const discarded = prunedEditsNotice(sent); + if (discarded) toastStore.show(discarded, 'info'); // Don't overwrite item -- resetting editorContent would // clobber anything typed since the debounce started. editorStore.setLastSaveTime(Date.now()); @@ -4579,6 +4582,9 @@ ...(overwrite ? { overwrite_pending_edits: true } : {}), }).then((updated) => { if (!item || item.id !== reqItemId || genAtSave !== loadGeneration) return; + // BUG-3230 U2: an overwrite that deleted another tab's edits says so. + const discarded = prunedEditsNotice(updated); + if (discarded) toastStore.show(discarded, 'info'); editorStore.setLastSaveTime(Date.now()); localLastSaveTime = Date.now(); // Raw saves change items.content via a path the @@ -4715,6 +4721,9 @@ if (!item || item.id !== reqItemId || gen !== loadGeneration) return; clearRefusedRawDraft(userId, reqItemId, d.markdown); refusedDraft = null; + // BUG-3230 U2: an overwrite that deleted another tab's edits says so. + const discarded = prunedEditsNotice(updated); + if (discarded) toastStore.show(discarded, 'info'); // The raw editor shows its seed over item.content; drop it so // the restored body is what it shows. if (rawMode) rawSeedMarkdown = null; @@ -4836,6 +4845,9 @@ // over. return false; } + // BUG-3230 U2: an overwrite that deleted another tab's edits says so. + const discarded = prunedEditsNotice(updated); + if (discarded) toastStore.show(discarded, 'info'); editorStore.setLastSaveTime(Date.now()); localLastSaveTime = Date.now(); // Raw saves change items.content via a path the @@ -5226,6 +5238,11 @@ // server-side action whose result must land, not be discarded. if (!item || item.id !== updatedItem.id) return; item = withInflightTags(updatedItem); + // BUG-3230 U2: a restore the user confirmed over another session's + // unsaved edits says how many it discarded, as a body save does. Here, + // not in the card: this pane owns the item-scoped toasts. + const discarded = prunedEditsNotice(updatedItem); + if (discarded) toastStore.show(discarded, 'info'); } // BUG-2271: flush the LIVE collab editor's markdown into items.content BEFORE diff --git a/web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts b/web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts index 924f7c0cf..c4b5332cf 100644 --- a/web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts +++ b/web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts @@ -88,7 +88,7 @@ const ASYNC_FUNCTIONS: Record = { jumpToSection: { reviewed: '17407aeb431b', why: 'switches this instance\'s tab and scrolls to an anchor', may: ['document.getElementById', 'document.getElementById(anchorId).scrollIntoView'] }, ensureGraphComp: { reviewed: 'c1565cfb8a18', why: 'lazy-loads a component module into this instance', may: ['ItemGraphComp', 'graphLoadError'] }, handleCopyRef: { reviewed: 'a5e0a0394fcc', why: 'switchedAway before the copied flag' }, - loadData: { reviewed: '71428f6992bb', why: 'IS the load: myGen against loadGeneration after every await; the BUG-3198 re-read of a changed item runs after the install, keyed on the installed item id, and fences itself on itemGen' }, + loadData: { reviewed: '3ac8a3aef331', why: 'IS the load: myGen against loadGeneration after every await; the BUG-3198 re-read of a changed item runs after the install, keyed on the installed item id, and fences itself on itemGen' }, startEditTitle: { reviewed: '17f04352d420', why: 'focuses and sizes the input it opened synchronously', may: ['el', 'titleInputEl.focus', 'titleInputEl.setSelectionRange'] }, // Re-reviewed for BUG-2836: saveTitle's own lines are unchanged (the diff // touches only a $derived beside titleDraft and the textarea's aria @@ -137,7 +137,7 @@ const ASYNC_FUNCTIONS: Record = { updateAssignedUser: { reviewed: '5e4128e45ae7', why: 'gen against loadGeneration on both arms' + SETTLE_NOTE, may: ['saves.settle'] }, updateAgentRole: { reviewed: '9b0a6af92cc2', why: 'gen against loadGeneration on both arms' + SETTLE_NOTE, may: ['saves.settle'] }, flushRawIfPending: { - reviewed: 'f5db8a60d450', + reviewed: '9030c207f9eb', why: 'genAtFlush against loadGeneration after each PATCH; the re-entrancy waiter returns state; the finally clears this drain\'s own in-flight flag' + SETTLE_NOTE, may: ['saves.settle', 'rawFlushInFlight'], bareAwaits: ['await new Promise((r) => setTimeout(r, 50));'], @@ -173,7 +173,7 @@ const ASYNC_FUNCTIONS: Record = { }, copyRefusedDraft: { reviewed: '839b51b59798', why: 'the copied text was captured before the await; the toast after it describes that copy, whichever item is shown', may: ['toastStore.show'] }, restoreRefusedDraft: { - reviewed: '5360a5f10e64', + reviewed: 'd42be5def01e', why: 'item id and gen against loadGeneration after every await; the finally clears this pane\'s own busy flag', may: ['refusedDraftBusy'], }, @@ -193,7 +193,7 @@ const NESTED: SignedRow[] = [ may: ['saves.settle'], }, // BUG-3230 U0: the legacy content save's refusal handler. - { body: /askToOverwritePendingEdits\(reqItem\.id, gen\)/, why: 'fallback refusal: switchedAway before the question, and again after it before the resend', reviewed: '627782b4acf6' }, + { body: /askToOverwritePendingEdits\(reqItem\.id, gen\)/, why: 'fallback refusal: switchedAway before the question, and again after it before the resend', reviewed: '272399d98b0e' }, ]; /** Async functions in the markup. */ @@ -217,7 +217,7 @@ const CONTINUATIONS: SignedRow[] = [ may: ['renameOverride'], }, { call: /^setTimeout\($/, body: /copied = false/, why: 'copy-flag reset: switchedAway', reviewed: '8a8d4754017f' }, - { call: /api\.items\.get\(wsSlug, itemSlug\)\.catch\($/, body: /./, why: 'loadData item fetch: sets a flag local to that load and re-throws', reviewed: 'faaed0d39b9c' }, + { call: /api\.items\.get\(wsSlug, itemSlug\)\.catch\($/, body: /./, why: 'loadData item fetch: sets a flag local to that load and re-throws', reviewed: 'faaa27d23182' }, { call: /^setTimeout\($/, body: /staleConnecting = true/, @@ -233,7 +233,7 @@ const CONTINUATIONS: SignedRow[] = [ body: /teardownFlushed/, in: 'onBeforeUnload', code: '() => { teardownFlushed = false; }', - why: 're-arms the BUG-3005 teardown latch, itself identity-checked', reviewed: 'b158fdd26c1b', + why: 're-arms the BUG-3005 teardown latch, itself identity-checked', reviewed: '07a11fd90d0b', may: ['teardownFlushed'], }, { call: /^queueMicrotask\($/, body: /./, why: 'collab lazy seed: refuses a retired or re-identified context first', reviewed: '197a4a45d219' }, @@ -250,22 +250,22 @@ const CONTINUATIONS: SignedRow[] = [ call: /^setTimeout\($/, body: /send\(false\)\.catch\(/, in: 'handleContentUpdate', - why: 'content debounce: loadData clears this timer before its first await, so the callback never runs across a load', reviewed: '86786c2982c9', + why: 'content debounce: loadData clears this timer before its first await, so the callback never runs across a load', reviewed: '094f908dfba9', startSafe: true, pin: (src, unit) => clearsBeforeFirstAwait(src, 'loadData', 'contentDebounceTimer') ?? assignedTo(src, unit, 'contentDebounceTimer'), }, // BUG-3230 U0: the fallback save is refused rather than replacing another // tab's unstored edits; the refusal asks, then resends once on overwrite. - { call: /^send\(false\)\.catch\($/, body: /./, why: 'content save refusal: switchedAway before the question, and again after it before the resend', reviewed: '627782b4acf6' }, - { call: /return send\(true\); \}\)\.then\($/, body: /^\(sent\) =>/, why: 'content save: switchedAway', reviewed: '29d7003dbe91' }, - { call: /showSaved\(saveTok\); \}\)\.catch\($/, body: /./, why: 'content save failure: switchedAway', reviewed: '17c6256a6238' }, + { call: /^send\(false\)\.catch\($/, body: /./, why: 'content save refusal: switchedAway before the question, and again after it before the resend', reviewed: '272399d98b0e' }, + { call: /return send\(true\); \}\)\.then\($/, body: /^\(sent\) =>/, why: 'content save: switchedAway', reviewed: '99cbdc519986' }, + { call: /showSaved\(saveTok\); \}\)\.catch\($/, body: /./, why: 'content save failure: switchedAway', reviewed: '56f6fd2ab1f8' }, { call: /\.finally\($/, body: /^\(\) => saves\.settle\(saveTok\)$/, in: 'setTimeout(…)', code: '() => saves.settle(saveTok)', why: SETTLE_WHY, - reviewed: '4d4064c71040', + reviewed: '670d366fd798', may: ['saves.settle'], }, // BUG-3230 U0: the unload save keeps its text in this browser before it goes @@ -279,7 +279,7 @@ const CONTINUATIONS: SignedRow[] = [ in: '{save}', code: "() => { clearRefusedRawDraft(draftUser, reqItemId, markdown); if (item && item.id === reqItemId && genAtSave === loadGeneration && rawContentSaver.pending === markdown) { rawContentSaver.clearPending(); editorStore.setDirty(false); localDirty = false; } }", why: 'raw keepalive save: removes the kept text by the captured user and item, then genAtSave against loadGeneration', - reviewed: '559cf1b3f8c7', + reviewed: '435849657502', may: ['clearRefusedRawDraft'], }, { @@ -288,20 +288,20 @@ const CONTINUATIONS: SignedRow[] = [ in: '{save}', code: "(e) => { if (isSupersededWriteError(e)) clearRefusedRawDraft(draftUser, reqItemId, markdown); else if (isContentPendingFlush(e)) { toastStore.show( 'Your markdown edits were not saved: another tab has edits to this item that are not stored yet. They are kept in this browser, and opening the item offers them back.', 'error', ); } }", why: 'raw keepalive failure: a superseded answer removes the kept text by the captured user and item; a refusal is reported whichever item is shown', - reviewed: '747d1ab73da2', + reviewed: 'cd76a2dbd33c', may: ['isSupersededWriteError', 'clearRefusedRawDraft', 'isContentPendingFlush', 'toastStore.show'], }, - { call: /overwrite_pending_edits: true \} : \{\}\), \}\)\.then\($/, body: /^\(updated\) =>/, why: 'raw foreground save: genAtSave against loadGeneration', reviewed: '9f7a01a91c2e' }, + { call: /overwrite_pending_edits: true \} : \{\}\), \}\)\.then\($/, body: /^\(updated\) =>/, why: 'raw foreground save: genAtSave against loadGeneration', reviewed: 'c968ffa55c7f' }, // BUG-3230 U0: the answer to a refused raw save arms one resend. - { call: /askToOverwritePendingEdits\(reqItemId, genAtSave\)\.then\($/, body: /./, why: 'raw refusal answer: genAtSave against loadGeneration before arming the resend', reviewed: '9a7706ba53a8' }, - { call: /content: item\.content, content_state: item\.content_state \}\); \} \}\)\.catch\($/, body: /./, why: 'raw foreground failure: genAtSave against loadGeneration', reviewed: '33b322a475d6' }, + { call: /askToOverwritePendingEdits\(reqItemId, genAtSave\)\.then\($/, body: /./, why: 'raw refusal answer: genAtSave against loadGeneration before arming the resend', reviewed: '9f2fa38e1fb0' }, + { call: /content: item\.content, content_state: item\.content_state \}\); \} \}\)\.catch\($/, body: /./, why: 'raw foreground failure: genAtSave against loadGeneration', reviewed: 'd1a5314e65f5' }, { call: /\.finally\($/, body: /^\(\) => saves\.settle\(saveTok\)$/, in: '{save}', code: '() => saves.settle(saveTok)', why: SETTLE_WHY, - reviewed: 'b1255a2a6ff6', + reviewed: '7f0b66871d51', may: ['saves.settle'], }, ]; @@ -326,7 +326,7 @@ const HELPERS: Record = { identityHeld: '1c5505d51f73', navigateToCollectionRoot: '64dce75693d3', refreshPrintMeta: '2312cc481ca5', - runTeardownFlush: 'e803dd0b8cba', + runTeardownFlush: '28bce9a45cc2', // BUG-3124 unit B: the cursor-advance settle. Synchronous; arms the flusher's // single timer only for the current, identity-held context. showSaved: '456dd972dcf5', diff --git a/web/src/lib/items/contentWrite.test.ts b/web/src/lib/items/contentWrite.test.ts index 478cac39c..c40c2f176 100644 --- a/web/src/lib/items/contentWrite.test.ts +++ b/web/src/lib/items/contentWrite.test.ts @@ -2,7 +2,7 @@ // always with the row's token. import { describe, expect, it } from 'vitest'; import { PadApiError } from '$lib/api/client'; -import { contentWriteFor, isContentPendingFlush } from './contentWrite'; +import { contentWriteFor, isContentPendingFlush, prunedEditsNotice } from './contentWrite'; const row = { content: 'stored body', seq: 7, updated_at: '2026-09-26T05:00:00Z' }; @@ -32,3 +32,17 @@ describe('isContentPendingFlush', () => { expect(isContentPendingFlush(new Error('content_pending_flush'))).toBe(false); }); }); + +// BUG-3230 U2: the sentence an overwrite owes when it deleted another tab's edits. +describe('prunedEditsNotice', () => { + it('is null when nothing was deleted', () => { + expect(prunedEditsNotice(null)).toBeNull(); + expect(prunedEditsNotice({})).toBeNull(); + expect(prunedEditsNotice({ warnings: {} })).toBeNull(); + expect(prunedEditsNotice({ warnings: { pruned_pending_edits: 0 } })).toBeNull(); + }); + it('names the count, singular and plural', () => { + expect(prunedEditsNotice({ warnings: { pruned_pending_edits: 1 } })).toBe('1 unsaved change from another tab was discarded.'); + expect(prunedEditsNotice({ warnings: { pruned_pending_edits: 3 } })).toBe('3 unsaved changes from another tab were discarded.'); + }); +}); diff --git a/web/src/lib/items/contentWrite.ts b/web/src/lib/items/contentWrite.ts index 7afdf7060..84f285c8c 100644 --- a/web/src/lib/items/contentWrite.ts +++ b/web/src/lib/items/contentWrite.ts @@ -33,6 +33,20 @@ export function contentWriteFor( : { content: body, expected_updated_at: token.value }; } +/** + * The sentence a write owes its user when it DELETED another tab's unsaved + * edits (BUG-3230 U2): `warnings.pruned_pending_edits` counts the op-log rows a + * direct content write removed. The web sends such a write only after the user + * chose to overwrite, so this confirms what their choice did rather than + * reporting a surprise. Rows are editor update frames, not keystrokes, so the + * count is given as "changes". Null when nothing was deleted. + */ +export function prunedEditsNotice(resp: Pick | null | undefined): string | null { + const n = resp?.warnings?.pruned_pending_edits ?? 0; + if (!(n > 0)) return null; + return `${n} unsaved ${n === 1 ? 'change' : 'changes'} from another tab ${n === 1 ? 'was' : 'were'} discarded.`; +} + /** The server refused a token-guarded content write because a tab holds unflushed edits. */ export function isContentPendingFlush(err: unknown): boolean { return err instanceof PadApiError && err.code === 'content_pending_flush'; diff --git a/web/src/routes/[username]/[workspace]/conventions/+page.svelte b/web/src/routes/[username]/[workspace]/conventions/+page.svelte index 5b68896f9..fb56a1115 100644 --- a/web/src/routes/[username]/[workspace]/conventions/+page.svelte +++ b/web/src/routes/[username]/[workspace]/conventions/+page.svelte @@ -6,7 +6,7 @@ import { parseFields, parseSchema, itemUrlId, formatItemRef } from '$lib/types'; import { toastStore } from '$lib/stores/toast.svelte'; import { titleLimitError } from '$lib/items/titleLimit'; - import { contentWriteFor, isContentPendingFlush } from '$lib/items/contentWrite'; + import { contentWriteFor, isContentPendingFlush, prunedEditsNotice } from '$lib/items/contentWrite'; import { pendingEditsDialog } from '$lib/stores/pendingEditsDialog.svelte'; import { createScrollRestoration } from '$lib/scroll/restore.svelte'; import { exportAndDownloadArtifact, importArtifactFile } from '$lib/utils/artifacts'; @@ -345,7 +345,10 @@ if (idx !== -1) conventions[idx] = updated; conventions = [...conventions]; editingSlug = null; - toastStore.show('Convention updated', 'success'); + // BUG-3230 U2: an overwrite that deleted another tab's edits says so. + const discarded = prunedEditsNotice(updated); + if (discarded) toastStore.show(`Convention updated. ${discarded}`, 'info'); + else toastStore.show('Convention updated', 'success'); } catch (err) { toastStore.show((err as Error)?.message || 'Failed to update convention', 'error'); } finally { diff --git a/web/src/routes/[username]/[workspace]/playbooks/[slug]/+page.svelte b/web/src/routes/[username]/[workspace]/playbooks/[slug]/+page.svelte index d7f23166e..0aac45a04 100644 --- a/web/src/routes/[username]/[workspace]/playbooks/[slug]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/playbooks/[slug]/+page.svelte @@ -7,7 +7,7 @@ import { toastStore } from '$lib/stores/toast.svelte'; import { authStore } from '$lib/stores/auth.svelte'; import { titleEditError } from '$lib/items/titleLimit'; - import { contentWriteFor, isContentPendingFlush } from '$lib/items/contentWrite'; + import { contentWriteFor, isContentPendingFlush, prunedEditsNotice } from '$lib/items/contentWrite'; import { pendingEditsDialog } from '$lib/stores/pendingEditsDialog.svelte'; import { createScrollRestoration } from '$lib/scroll/restore.svelte'; import { exportAndDownloadArtifact } from '$lib/utils/artifacts'; @@ -258,6 +258,8 @@ ...contentWriteFor(bodyContent, item), ...(Object.keys(fieldsPatch).length ? { fields_patch: fieldsPatch } : {}) }; + // BUG-3230 U2: an overwrite that deleted another tab's edits says so. + let discarded: string | null = null; try { await api.items.update(wsSlug, item.slug, payload); } catch (err) { @@ -269,10 +271,11 @@ toastStore.show("Not saved: the open tab's edits were kept. Your changes are still here.", 'info'); return; } - await api.items.update(wsSlug, item.slug, { ...payload, overwrite_pending_edits: true }); + discarded = prunedEditsNotice(await api.items.update(wsSlug, item.slug, { ...payload, overwrite_pending_edits: true })); } if (!isSameIdentity()) return; - toastStore.show('Playbook saved', 'success'); + if (discarded) toastStore.show(`Playbook saved. ${discarded}`, 'info'); + else toastStore.show('Playbook saved', 'success'); goto(`/${username}/${wsSlug}/playbooks`); } catch (err) { if (!isSameIdentity()) return; diff --git a/web/src/routes/[username]/[workspace]/playbooks/[slug]/playbookIdentityGate.test.ts b/web/src/routes/[username]/[workspace]/playbooks/[slug]/playbookIdentityGate.test.ts index 93c53b8fe..652852198 100644 --- a/web/src/routes/[username]/[workspace]/playbooks/[slug]/playbookIdentityGate.test.ts +++ b/web/src/routes/[username]/[workspace]/playbooks/[slug]/playbookIdentityGate.test.ts @@ -17,7 +17,7 @@ identityGateSuite({ loadItem: { reviewed: '1d65e4a33dc3', why: 'workspace, ref AND the entry identity fence after the fetch, on both arms and in the finally' }, loadPlaybooks: { reviewed: '6825e2ad3d9b', why: 'workspace AND the entry identity fence after the fetch, on both arms' }, loadCollection: { reviewed: '5e49f16c7659', why: 'workspace AND the entry identity fence after the fetch, on both arms' }, - save: { reviewed: 'cf7b25c986f0', why: 'user-initiated save; the entry identity fence before the dialog, after its answer (so the overwrite re-send never goes under another identity), before the success report and navigation, on the failure report, and on the finally that clears saving' }, + save: { reviewed: '8a2edff186a7', why: 'user-initiated save; the entry identity fence before the dialog, after its answer (so the overwrite re-send never goes under another identity), before the success report and navigation, on the failure report, and on the finally that clears saving' }, handleExport: { reviewed: 'bd5b6bebfde2', why: 'user-initiated export; the entry identity fence before either toast and on the finally that clears exporting' }, }, nested: [],