From 1d6afdc747d9e05510ab8b3ac4a83eb0ed7c1bbb Mon Sep 17 00:00:00 2001 From: Brian Date: Thu, 8 Oct 2026 00:30:12 -0700 Subject: [PATCH 1/4] test(e2e): enforce replay revisions in fixture and cover offline reconnect The loopback Convex fixture accepted every *Replay mutation without checking expected revisions or predecessor receipts, so a regression of #238 (an offline uncheck rejected after its own check replayed) passed the browser suite. The fixture now mirrors convex/lib/replay.ts: receipts keyed by operation ID with fingerprint checks, target/expected matching, predecessor revisions from receipts, and REPLAY_CONFLICT errors sent as ConvexError data. Adds per-account backend outage and lost-response controls plus a collaborator-edit hook, and browser tests for: - offline check then uncheck across a reload replays both, final unchecked - a committed-but-lost acknowledgment retries with the same operation ID and applies once; a later edit chains through the recovered receipt - another member's concurrent edit surfaces as a recoverable conflict and can be explicitly reapplied Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/fixtures/app.ts | 14 ++++- tests/fixtures/server.mjs | 97 ++++++++++++++++++++++++++----- tests/offline-replay.e2e.ts | 113 ++++++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 16 deletions(-) create mode 100644 tests/offline-replay.e2e.ts diff --git a/tests/fixtures/app.ts b/tests/fixtures/app.ts index 2cc19c3..a9b033f 100644 --- a/tests/fixtures/app.ts +++ b/tests/fixtures/app.ts @@ -22,13 +22,17 @@ interface FixtureState { calls: { path: string; args: Record }[]; errors: string[]; published: boolean; - receipts: { operationId: string }[]; + receipts: { operationId: string; result: unknown; revisions: Record }[]; } export interface Scenario { open(path: string, options?: ScenarioOptions): Promise; state(): Promise; resumeTemplateResponses(): Promise; + /** Simulates a backend outage for this account; optionally commit and then lose replay responses. */ + setBackend(options: { online: boolean; dropReplayResponses?: number }): Promise; + /** Commits another member's edit to an item, bypassing this client's receipts. */ + collaboratorEdit(itemId: string, changes: Record): Promise; httpCalls: { path: string; body: Record }[]; } @@ -49,6 +53,14 @@ export const test = webTest.extend<{ scenario: Scenario }>({ const response = await fetch(new URL('/__e2e/resume-template', base), {method:'POST',headers:{authorization:`Bearer ${token}`}}); if (!response.ok) throw new Error('Could not resume fixture responses'); }, + async setBackend(options) { + const response = await fetch(new URL('/__e2e/network', base), { method: 'POST', headers: { authorization: `Bearer ${token}` }, body: JSON.stringify(options) }); + if (!response.ok) throw new Error('Could not change fixture network'); + }, + async collaboratorEdit(itemId, changes) { + const response = await fetch(new URL('/__e2e/collaborator-edit', base), { method: 'POST', headers: { authorization: `Bearer ${token}` }, body: JSON.stringify({ itemId, changes }) }); + if (!response.ok) throw new Error('Could not apply collaborator edit'); + }, async open(path, options = {}) { if (token) throw new Error('Use app.open/browser.reload after scenario.open; a scenario is seeded once per test.'); const response = await fetch(new URL('/__e2e/seed', base), { diff --git a/tests/fixtures/server.mjs b/tests/fixtures/server.mjs index b6dd22d..df73071 100644 --- a/tests/fixtures/server.mjs +++ b/tests/fixtures/server.mjs @@ -3,7 +3,7 @@ import { createServer } from 'vite'; import { randomUUID } from 'node:crypto'; import { BUILTIN_TEMPLATES } from '../../convex/lib/templateCatalog.ts'; -import { revision } from '../../shared/replay.ts'; +import { canonical, revision, replayTargets } from '../../shared/replay.ts'; const TS_ZERO = 'AAAAAAAAAAA='; const accounts = new Map(); @@ -26,7 +26,7 @@ function seed(options) { createdAt: i + 1, updatedAt: i + 1, order: i, })); const account = { key, token, options, lists, items, published: options.published ?? false, - sequence: 0, receipts: [], calls: [], errors: [], nextId: 0 }; + sequence: 0, receipts: [], calls: [], errors: [], nextId: 0, offline: false, dropReplayResponses: 0 }; accounts.set(token, account); return account; } @@ -39,9 +39,10 @@ function query(account, path, args) { case 'items:getListItems': return account.items.filter(i => i.listId === args.listId); case 'items:getListItemsForReplay': return { items: account.items.filter(i => i.listId === args.listId), - acknowledgments: account.receipts.filter(r => args.operationIds.includes(r.operationId)), + acknowledgments: account.receipts.filter(r => args.operationIds.includes(r.operationId)).map(r => r.ack), sequence: account.sequence, }; + case 'items:getItemForSync': return account.items.find(i => i._id === args.itemId) ?? null; case 'items:getOfflineAccount': return { accountId: user.turnkeySubOrgId, did: user.did }; case 'items:getOfflineAccess': return [...new Set([...args.listIds, ...(args.items ?? []).map(i => i.listId)])].map(listId => { const canRead = account.lists.some(l => l._id === listId); @@ -86,10 +87,54 @@ function query(account, path, args) { } } +// Mirrors convex/lib/replay.ts: receipts are keyed by operation ID, and the +// expected revision (or the acknowledged predecessor's revision) is checked in +// the same step as the write. Client clocks are never consulted. +const REPLAY_TYPES = { + 'items:addItemReplay': 'addItem', 'items:checkItemReplay': 'checkItem', 'items:uncheckItemReplay': 'uncheckItem', + 'items:updateItemReplay': 'updateItem', 'items:removeItemReplay': 'removeItem', 'items:reorderItemsReplay': 'reorderItem', + 'items:batchCheckItemsReplay': 'batchCheckItems', 'items:batchUncheckItemsReplay': 'batchUncheckItems', + 'items:batchDeleteItemsReplay': 'batchDeleteItems', 'lists:createListReplay': 'createList', + 'lists:renameListReplay': 'renameList', 'lists:deleteListReplay': 'deleteList', +}; +class ReplayConflict extends Error { + constructor(message) { super(message); this.errorData = { code: 'REPLAY_CONFLICT', message }; } +} +const findDoc = (account, id) => account.items.find(i => i._id === id) ?? account.lists.find(l => l._id === id); + async function mutation(account, path, args) { - const endpoint = path.replace(/Replay$/, ''); - const previous = args.replay && account.receipts.find(r => r.operationId === args.replay.operationId); - if (previous) return previous; + if (!args.replay) return apply(account, path.replace(/Replay$/, ''), args); + const operation = REPLAY_TYPES[path]; + if (!operation) throw new Error(`Unimplemented fixture replay: ${path}`); + const { authToken: _authToken, replay, ...payload } = args; + const fingerprint = await revision({ operation, payload, expected: replay.expected }); + const previous = account.receipts.find(r => r.operationId === replay.operationId); + if (previous) { + if (previous.fingerprint !== fingerprint) throw new Error('Operation ID reused with different content'); + return previous.ack; + } + const targets = replayTargets(operation, payload); + if (canonical([...targets].sort()) !== canonical(replay.expected.map(e => e.id).sort())) throw new ReplayConflict('Missing expected revision. Review this saved edit before applying it.'); + for (const expected of replay.expected) { + let wanted = expected.revision; + if (expected.predecessor) { + const prior = account.receipts.find(r => r.operationId === expected.predecessor); + if (!prior?.ack.revisions[expected.id]) throw new ReplayConflict('A preceding local edit has not been acknowledged.'); + wanted = prior.ack.revisions[expected.id]; + } + const doc = findDoc(account, expected.id); + if (!doc || await revision(doc) !== wanted) throw new ReplayConflict('This item changed on the server. Your edit is saved for review.'); + } + const result = await apply(account, path.replace(/Replay$/, ''), payload); + const revisions = {}; + for (const id of targets) revisions[id] = await revision(findDoc(account, id) ?? null); + if (typeof result === 'string' && (operation === 'addItem' || operation === 'createList')) revisions[result] = await revision(findDoc(account, result)); + const ack = { operationId: replay.operationId, result, revisions, sequence: ++account.sequence }; + account.receipts.push({ operationId: replay.operationId, fingerprint, ack }); + return ack; +} + +async function apply(account, endpoint, args) { let result = null; const item = account.items.find(i => i._id === args.itemId); const list = account.lists.find(l => l._id === args.listId); @@ -146,12 +191,7 @@ async function mutation(account, path, args) { case 'referrals:getOrCreateReferralCode': result = 'e2e-code'; break; default: throw new Error(`Unimplemented fixture mutation: ${path}`); } - if (!args.replay) return result; - const revisions = {}; - for (const doc of account.items) revisions[doc._id] = await revision(doc); - const ack = { operationId: args.replay.operationId, result, revisions, sequence: ++account.sequence }; - account.receipts.push(ack); - return ack; + return result; } function transition(ws, newVersion = ws.data.version) { @@ -205,7 +245,25 @@ const backend = Bun.serve({ const account = accounts.get(token); if (url.pathname === '/__e2e/state' && account) { return Response.json({ lists: account.lists, items: account.items, calls: account.calls, - errors: account.errors, published: account.published, receipts: account.receipts }); + errors: account.errors, published: account.published, receipts: account.receipts.map(r => r.ack) }); + } + if (url.pathname === '/__e2e/network' && account && request.method === 'POST') { + // A backend outage for this account only: live sockets drop, and new ones + // are closed as soon as they identify the account, until it is restored. + const { online, dropReplayResponses } = await request.json(); + account.offline = !online; + if (dropReplayResponses !== undefined) account.dropReplayResponses = dropReplayResponses; + if (account.offline) for (const socket of sockets) if (socket.data.account === account) socket.close(); + return new Response(null, { status: 204 }); + } + if (url.pathname === '/__e2e/collaborator-edit' && account && request.method === 'POST') { + // Another member's write, committed without any of this client's receipts. + const { itemId, changes } = await request.json(); + const item = account.items.find(i => i._id === itemId); + if (!item) return new Response('Unknown item', { status: 404 }); + Object.assign(item, changes, { updatedAt: Date.now() }); + for (const socket of sockets) if (socket.data.account === account) transition(socket); + return new Response(null, { status: 204 }); } if (url.pathname === '/__e2e/resume-template' && account && request.method === 'POST') { account.options.loseTemplateResponseOnce = false; @@ -227,12 +285,14 @@ const backend = Bun.serve({ if (msg.type === 'Connect' || msg.type === 'Authenticate' || msg.type === 'Event') return; const args = msg.args?.[0] ?? {}; if (args.authToken) ws.data.account = accounts.get(args.authToken); + if (ws.data.account?.offline) { ws.close(); return; } if (msg.type === 'ModifyQuerySet') { for (const mod of msg.modifications) { if (mod.type === 'Remove') ws.data.queries.delete(mod.queryId); else { const queryArgs = mod.args?.[0] ?? {}; if (queryArgs.authToken) ws.data.account = accounts.get(queryArgs.authToken); + if (ws.data.account?.offline) { ws.close(); return; } ws.data.queries.set(mod.queryId, { path: mod.udfPath, args: queryArgs }); } } @@ -251,11 +311,18 @@ const backend = Bun.serve({ ws.send(JSON.stringify({ type: 'MutationResponse', requestId: msg.requestId, success: false, result: 'Fixture: committed response lost', logLines: [] })); return; } + if (args.replay && account.dropReplayResponses > 0) { + // The write and its receipt committed; the connection dies first. + account.dropReplayResponses--; + ws.close(); + return; + } ws.send(JSON.stringify({ type: 'MutationResponse', requestId: msg.requestId, success: true, result, ts: TS_ZERO, logLines: [] })); for (const socket of sockets) if (socket.data.account === account) transition(socket); } catch (error) { - if (!error.message.startsWith('PLAN_LIMIT')) account?.errors.push(error.message); - ws.send(JSON.stringify({ type: 'MutationResponse', requestId: msg.requestId, success: false, result: error.message, logLines: [] })); + if (!error.message.startsWith('PLAN_LIMIT') && !error.errorData) account?.errors.push(error.message); + ws.send(JSON.stringify({ type: 'MutationResponse', requestId: msg.requestId, success: false, result: error.message, + ...(error.errorData ? { errorData: error.errorData } : {}), logLines: [] })); } return; } diff --git a/tests/offline-replay.e2e.ts b/tests/offline-replay.e2e.ts new file mode 100644 index 0000000..1ae4abd --- /dev/null +++ b/tests/offline-replay.e2e.ts @@ -0,0 +1,113 @@ +// Issue #238: queued offline edits replay against a fixture that enforces the +// same receipt and expected-revision rules as convex/lib/replay.ts. +import { expect, type TestFixtures } from 'e2e'; +import type { Browser } from '@e2e-dev/web'; +import { test, openList, type Scenario } from './fixtures/app'; + +const BREAD = 'items:mockitem0'; + +async function goOffline(scenario: Scenario, browser: Browser) { + await scenario.setBackend({ online: false }); + await browser.evaluate(() => window.dispatchEvent(new Event('offline'))); +} + +async function goOnline(scenario: Scenario, browser: Browser, dropReplayResponses?: number) { + await scenario.setBackend({ online: true, dropReplayResponses }); + await browser.evaluate(() => window.dispatchEvent(new Event('online'))); +} + +async function checkBread(screen: TestFixtures['screen']) { + await screen.getByRole('button', { name: 'Check Bread' }).click(); + // Checked items move into the collapsible Done section. + await screen.getByRole('button', { name: /^Done/ }).click({ timeout: 5000 }); + await expect(screen.getByRole('button', { name: 'Uncheck Bread' })).toBeVisible({ timeout: 5000 }); +} + +async function eventually(read: () => Promise, accept: (value: T) => boolean, timeout = 20_000): Promise { + const deadline = Date.now() + timeout; + let value = await read(); + while (!accept(value) && Date.now() < deadline) { + await new Promise(resolve => setTimeout(resolve, 250)); + value = await read(); + } + return value; +} + +test.describe('Offline replay', () => { + test('check then uncheck while offline, across a reload, replays both edits in order', async ({ screen, browser, scenario }) => { + await openList(scenario, screen, { items: [{ name: 'Bread' }] }); + await goOffline(scenario, browser); + + await checkBread(screen); + await screen.getByRole('button', { name: 'Uncheck Bread' }).click(); + await expect(screen.getByRole('button', { name: 'Check Bread' })).toBeVisible({ timeout: 5000 }); + + // Both edits must survive a reload before the backend is reachable again. + // (The signed-in shell itself waits for the backend, so nothing renders yet.) + await browser.reload(); + await expect(screen.getByText('Loading...')).toBeVisible({ timeout: 10_000 }); + expect((await scenario.state()).receipts).toEqual([]); + + await goOnline(scenario, browser); + await eventually(scenario.state, s => s.receipts.length >= 2 || s.calls.some(c => c.path === 'items:uncheckItemReplay')); + await expect(screen.getByRole('button', { name: 'Check Bread' })).toBeVisible({ timeout: 10_000 }); + const state = await eventually(scenario.state, s => s.receipts.length >= 2, 5000); + // The user's final intent (unchecked) reached the server, and neither edit was dropped. + expect(state.items.find(i => i._id === BREAD)?.checked).toBe(false); + expect(state.receipts).toHaveLength(2); + + const replays = state.calls.filter(c => c.path.endsWith('Replay')); + expect(replays.map(c => c.path)).toEqual(['items:checkItemReplay', 'items:uncheckItemReplay']); + const [check, uncheck] = replays.map(c => c.args.replay as { operationId: string; expected: { id: string; predecessor?: string }[] }); + // The uncheck is based on the check's acknowledgment, not on a clock or a + // fresh read, so the check's own write is never mistaken for a conflict. + expect(uncheck.expected).toEqual([expect.objectContaining({ id: BREAD, predecessor: check.operationId })]); + expect(state.receipts.map(r => r.operationId)).toEqual([check.operationId, uncheck.operationId]); + await expect(screen.getByText(/saved edit\(s\) awaiting sync/)).not.toBeVisible({ timeout: 10_000 }); + }); + + test('a lost acknowledgment is retried with the same operation ID and applied once', async ({ screen, browser, scenario }) => { + await openList(scenario, screen, { items: [{ name: 'Bread' }] }); + await goOffline(scenario, browser); + await checkBread(screen); + + // The check commits on reconnect, but its response never reaches the client. + await goOnline(scenario, browser, 1); + let state = await eventually(scenario.state, s => s.calls.filter(c => c.path === 'items:checkItemReplay').length >= 2); + const sent = state.calls.filter(c => c.path === 'items:checkItemReplay').map(c => (c.args.replay as { operationId: string }).operationId); + expect(new Set(sent).size).toBe(1); + expect(state.receipts.map(r => r.operationId)).toEqual([sent[0]]); + + // A later edit chains through the recovered receipt instead of conflicting. + await screen.getByRole('button', { name: 'Uncheck Bread' }).click(); + state = await eventually(scenario.state, s => s.receipts.length >= 2); + expect(state.receipts).toHaveLength(2); + expect(state.items.find(i => i._id === BREAD)?.checked).toBe(false); + await expect(screen.getByText(/saved edit\(s\) awaiting sync/)).not.toBeVisible({ timeout: 10_000 }); + }); + + test("another member's concurrent edit surfaces as a recoverable conflict", async ({ screen, browser, scenario }) => { + await openList(scenario, screen, { items: [{ name: 'Bread' }] }); + await goOffline(scenario, browser); + await checkBread(screen); + + await scenario.collaboratorEdit(BREAD, { name: 'Rye bread' }); + await goOnline(scenario, browser); + + // Never last-write-wins over the collaborator, and never silently dropped. + const recovery = screen.getByText('1 saved edit(s) awaiting sync'); + await expect(recovery).toBeVisible({ timeout: 20_000 }); + let state = await scenario.state(); + expect(state.receipts).toEqual([]); + expect(state.items.find(i => i._id === BREAD)).toEqual(expect.objectContaining({ name: 'Rye bread', checked: false })); + + await recovery.click(); + await expect(screen.getByText('This item changed on the server. Your edit is saved for review.')).toBeVisible(); + await screen.getByRole('button', 'Review conflict').click(); + await screen.getByRole('button', 'Apply saved edit to this version').click(); + + state = await eventually(scenario.state, s => s.receipts.length >= 1); + expect(state.items.find(i => i._id === BREAD)).toEqual(expect.objectContaining({ name: 'Rye bread', checked: true })); + await expect(screen.getByText(/saved edit\(s\) awaiting sync/)).not.toBeVisible({ timeout: 10_000 }); + }); +}); From dcdd67d9a8d22f9cbb7c89b932743f882eb592e2 Mon Sep 17 00:00:00 2001 From: Brian Date: Thu, 8 Oct 2026 00:36:03 -0700 Subject: [PATCH 2/4] fix(offline): stop the client's own queued edits from replaying as conflicts Two remaining paths still made a user's own offline work come back as a REPLAY_CONFLICT (#238): - A details draft opened on an optimistic row pins its predecessor to that row's operation. If the user then made another edit to the same item (e.g. unchecked it in the list), the draft still expected the older revision and conflicted with the user's own edit. Enqueue now chains a pinned base through the newest of this client's own later edits that descend from it. A collaborator edit in between still conflicts. - BatchOperations and ListItem's remove/fallback toggles queued without a snapshot. For an item not yet in the IndexedDB cache (a collaborator's new row tapped before the cache effect commits), the edit was recorded with revision 'unknown' and no list, so it was never projected and always rejected. They now pass the rendered rows the user acted on as the edit base, as the hook-based paths already do. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/offline-edit-base-ui.test.mjs | 55 +++++++++++++++++++++++++++ scripts/offline-edit-base.test.mjs | 54 ++++++++++++++++++++++++++ src/components/BatchOperations.tsx | 11 ++++-- src/components/ListItem.tsx | 6 +-- src/lib/offline.ts | 5 ++- src/pages/ListView.tsx | 1 + 6 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 scripts/offline-edit-base-ui.test.mjs create mode 100644 scripts/offline-edit-base.test.mjs diff --git a/scripts/offline-edit-base-ui.test.mjs b/scripts/offline-edit-base-ui.test.mjs new file mode 100644 index 0000000..f1ddebb --- /dev/null +++ b/scripts/offline-edit-base-ui.test.mjs @@ -0,0 +1,55 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { build } from 'esbuild'; +import { pathToFileURL } from 'node:url'; +import { GlobalRegistrator } from '@happy-dom/global-registrator'; +if (!GlobalRegistrator.isRegistered) GlobalRegistrator.register(); +globalThis.IS_REACT_ACT_ENVIRONMENT = true; +const { render, fireEvent, waitFor, cleanup } = await import('@testing-library/react'); +const { createElement } = await import('react'); +const fixture = globalThis.__editBase = { queued: [] }; +await build({ entryPoints: ['src/components/BatchOperations.tsx', 'src/components/ListItem.tsx'], outdir: 'tmp/offline-edit-base-ui', outbase: '.', bundle: true, splitting: true, platform: 'node', format: 'esm', outExtension: { '.js': '.mjs' }, jsx: 'automatic', external: ['react', 'react/jsx-runtime', 'react-dom'], plugins: [{ name: 'edit-base-fixtures', setup(b) { + b.onResolve({ filter: /\/useOffline$|\/useSettings$|\/SubItems$|\/ItemAttribution$|\/ItemDetailsModal$|\/share$/ }, args => ({ path: args.path, namespace: 'fixture' })); + b.onLoad({ filter: /.*/, namespace: 'fixture' }, ({ path }) => ({ contents: + path.endsWith('/useOffline') ? 'export const useOffline=()=>({queueMutation:async(input,snapshots)=>{globalThis.__editBase.queued.push({input,snapshots});return 1;}});' : + path.endsWith('/useSettings') ? 'export const useSettings=()=>({haptic(){}});' : + path.endsWith('/SubItems') ? 'export const useSubItemProgress=()=>null;' : + path.endsWith('/ItemAttribution') ? 'export const ItemAttribution=()=>null;' : + path.endsWith('/ItemDetailsModal') ? 'export const ItemDetailsModal=()=>null;' : + 'export const shareItem=async()=>{};', + })); +} }] }); +const load = path => import(pathToFileURL(`${process.cwd()}/tmp/offline-edit-base-ui/${path}.mjs`)); +const { BatchOperations } = await load('src/components/BatchOperations'); +const { ListItem } = await load('src/components/ListItem'); +const item = (id, name, extra = {}) => ({ _id: id, _creationTime: 1, listId: 'L1', name, checked: false, createdByDid: 'did:a', createdAt: 1, updatedAt: 1, ...extra }); + +// Without the rendered rows, an uncached target is queued with an 'unknown' +// revision and the server rejects the user's own edit as a conflict. +test('batch operations queue the selected rendered rows as their edit base', async () => { + const rows = [item('I1', 'Milk'), item('I2', 'Bread', { _operationId: 'op-prior', _isOptimistic: true }), item('I3', 'Eggs')]; + for (const [label, type] of [['Check all selected items', 'batchCheckItems'], ['Uncheck all selected items', 'batchUncheckItems']]) { + fixture.queued = []; + const view = render(createElement(BatchOperations, { selectedIds: new Set(['I1', 'I2']), items: rows, onClearSelection() {}, userDid: 'did:a' })); + try { + fireEvent.click(view.getByLabelText(label)); + await waitFor(() => assert.equal(fixture.queued.length, 1)); + assert.equal(fixture.queued[0].input.type, type); + assert.deepEqual(fixture.queued[0].snapshots?.map(i => i._id), ['I1', 'I2']); + } finally { cleanup(); } + } +}); + +test('row remove and fallback toggles queue the rendered row as their edit base', async () => { + const row = item('I1', 'Milk'); + for (const [label, type] of [['Remove Milk', 'removeItem'], ['Check Milk', 'checkItem']]) { + fixture.queued = []; + const view = render(createElement(ListItem, { item: row, userDid: 'did:a' })); + try { + fireEvent.click(view.getByLabelText(label)); + await waitFor(() => assert.equal(fixture.queued.length, 1)); + assert.equal(fixture.queued[0].input.type, type); + assert.deepEqual(fixture.queued[0].snapshots, [row]); + } finally { cleanup(); } + } +}); diff --git a/scripts/offline-edit-base.test.mjs b/scripts/offline-edit-base.test.mjs new file mode 100644 index 0000000..4e03ab7 --- /dev/null +++ b/scripts/offline-edit-base.test.mjs @@ -0,0 +1,54 @@ +import 'fake-indexeddb/auto'; +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { loadReplayModules, replayFixture } from './helpers/replay-fixture.mjs'; +const modules = await loadReplayModules('offline-edit-base-tests'); +const { offline: store, sync: { SyncManager }, optimistic: { projectItems } } = modules; +const queue = (f, type, payload, snapshots = structuredClone(f.rows.items)) => store.queueMutation(f.session.accountId, { type, payload }, snapshots); +const pending = f => store.getQueuedMutations(f.session.accountId); +const all = f => store.getOperations(f.session.accountId); + +test("a draft opened on an optimistic version chains through the client's own later edits instead of conflicting", async () => { + const f = await replayFixture(modules); + await queue(f, 'checkItem', { itemId: 'I1', checkedAt: 1 }); + // The details modal opens on the projected (checked) row, tagged with the check. + const draftSource = projectItems(structuredClone(f.rows.items), await all(f), 'L1')[0]; + // While it is open, the same user unchecks the row in the list. + await queue(f, 'uncheckItem', { itemId: 'I1' }); + await queue(f, 'updateItem', { itemId: 'I1', name: 'Oat milk' }, [draftSource]); + const [check, uncheck, update] = await all(f); + assert.equal(uncheck.expected[0].predecessor, check.operationId); + assert.equal(update.expected[0].predecessor, uncheck.operationId); + await new SyncManager().sync(f.client, f.session); + assert.deepEqual(await pending(f), []); + assert.equal(f.rows.items[0].name, 'Oat milk'); + assert.equal(f.rows.items[0].checked, false); +}); + +test("a draft still conflicts with a collaborator's edit made after the client's own later edits", async () => { + const f = await replayFixture(modules); + await queue(f, 'checkItem', { itemId: 'I1', checkedAt: 1 }); + const draftSource = projectItems(structuredClone(f.rows.items), await all(f), 'L1')[0]; + await queue(f, 'uncheckItem', { itemId: 'I1' }); + await new SyncManager().sync(f.client, f.session); + await f.call('items', 'updateItem', { itemId: 'I1', name: 'Collaborator name' }, f.collaborator); + await queue(f, 'updateItem', { itemId: 'I1', name: 'Oat milk' }, [draftSource]); + await new SyncManager().sync(f.client, f.session); + const [conflict] = await pending(f); + assert.equal(conflict.state, 'conflict'); + assert.equal(conflict.payload.name, 'Oat milk'); + assert.equal(f.rows.items[0].name, 'Collaborator name'); +}); + +test('a rendered row is a valid edit base for an uncached item; without one the edit cannot be verified', async () => { + const f = await replayFixture(modules); + // No cached snapshot exists yet (e.g. a collaborator's new row, tapped before + // the cache effect commits). The rendered row is the version the user acted on. + await queue(f, 'batchCheckItems', { itemIds: ['I1'] }, structuredClone(f.rows.items)); + const [withBase] = await all(f); + assert.notEqual(withBase.expected[0].revision, 'unknown'); + assert.deepEqual(withBase.listIds, ['L1']); + await new SyncManager().sync(f.client, f.session); + assert.deepEqual(await pending(f), []); + assert.equal(f.rows.items[0].checked, true); +}); diff --git a/src/components/BatchOperations.tsx b/src/components/BatchOperations.tsx index d52be26..350c61a 100644 --- a/src/components/BatchOperations.tsx +++ b/src/components/BatchOperations.tsx @@ -6,10 +6,13 @@ import { useState } from "react"; import type { Id } from "../../convex/_generated/dataModel"; import { useSettings } from "../hooks/useSettings"; import { useOffline } from "../hooks/useOffline"; +import type { OptimisticItem } from "../hooks/useOptimisticItems"; import { ConfirmDialog } from "./ConfirmDialog"; interface BatchOperationsProps { selectedIds: Set>; + /** Rendered rows: the versions the user acted on are the replay edit base. */ + items: OptimisticItem[]; onClearSelection: () => void; userDid: string; legacyDid?: string; @@ -17,6 +20,7 @@ interface BatchOperationsProps { export function BatchOperations({ selectedIds, + items, onClearSelection, userDid, legacyDid, @@ -27,6 +31,7 @@ export function BatchOperations({ const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); const count = selectedIds.size; + const selectedRows = () => items.filter(item => selectedIds.has(item._id)); const handleCheckAll = async () => { if (isProcessing) return; @@ -44,7 +49,7 @@ export function BatchOperations({ payload, timestamp: Date.now(), retryCount: 0, - }); + }, selectedRows()); haptic("success"); onClearSelection(); @@ -72,7 +77,7 @@ export function BatchOperations({ payload, timestamp: Date.now(), retryCount: 0, - }); + }, selectedRows()); haptic("success"); onClearSelection(); @@ -100,7 +105,7 @@ export function BatchOperations({ payload, timestamp: Date.now(), retryCount: 0, - }); + }, selectedRows()); haptic("success"); onClearSelection(); diff --git a/src/components/ListItem.tsx b/src/components/ListItem.tsx index d8ba1a5..d1b5866 100644 --- a/src/components/ListItem.tsx +++ b/src/components/ListItem.tsx @@ -107,10 +107,10 @@ export const ListItem = memo(function ListItem({ try { if (item.checked) { if (onUncheck) await onUncheck(item._id, userDid, legacyDid); - else await queueMutation({ type: "uncheckItem", payload: { itemId: item._id, userDid, legacyDid } }); + else await queueMutation({ type: "uncheckItem", payload: { itemId: item._id, userDid, legacyDid } }, [item]); } else { if (onCheck) await onCheck(item._id, userDid, legacyDid); - else await queueMutation({ type: "checkItem", payload: { itemId: item._id, checkedByDid: userDid, legacyDid, checkedAt: Date.now() } }); + else await queueMutation({ type: "checkItem", payload: { itemId: item._id, checkedByDid: userDid, legacyDid, checkedAt: Date.now() } }, [item]); } } catch (err) { console.error("Failed to toggle item:", err); @@ -125,7 +125,7 @@ export const ListItem = memo(function ListItem({ haptic('medium'); setIsUpdating(true); try { - await queueMutation({ type: "removeItem", payload: { itemId: item._id, userDid, legacyDid } }); + await queueMutation({ type: "removeItem", payload: { itemId: item._id, userDid, legacyDid } }, [item]); } catch (err) { console.error("Failed to remove item:", err); haptic('error'); diff --git a/src/lib/offline.ts b/src/lib/offline.ts index 2f8dd32..681fedb 100644 --- a/src/lib/offline.ts +++ b/src/lib/offline.ts @@ -174,7 +174,10 @@ async function enqueueMutation(accountId: string, input: { // whose real ID is now visible but not yet present in any cached snapshot. return revisions.get(id) === 'unknown' || (!observed && (createsTarget || target?.revision === revisions.get(id))); }); - const predecessorId = sourceOperations.get(id) ?? predecessor?.operationId; + let predecessorId = sourceOperations.get(id) ?? predecessor?.operationId; + // This client's own later edits descended from a pinned draft base are not + // competing writes. Chain through the newest, so only collaborators conflict. + for (let next; sourceOperations.has(id) && (next = [...prior].reverse().find(m => !m.denied && m.expected.some(e => e.predecessor === predecessorId && resolveOperationId(e.id, prior) === resolvedId)));) predecessorId = next.operationId; return { id, revision: revisions.get(id)!, ...(predecessorId ? { predecessor: predecessorId } : {}) }; }); const dependencies = [...expected.flatMap(e => e.predecessor ? [e.predecessor] : []), ...operationReferences({ payload, expected }).filter(id => id.startsWith('temp-')).map(id => id.slice(5))]; diff --git a/src/pages/ListView.tsx b/src/pages/ListView.tsx index f006793..e2ba1d2 100644 --- a/src/pages/ListView.tsx +++ b/src/pages/ListView.tsx @@ -1363,6 +1363,7 @@ export function ListView() { {canUserEdit && ( Date: Thu, 8 Oct 2026 00:42:37 -0700 Subject: [PATCH 3/4] fix(offline): chain drafts through own edits queued after observation Review follow-up. A pinned draft base only walked explicit predecessor links. If the user's later edit to the same item was queued after the earlier edit had been acknowledged and observed, that edit carried no predecessor (it was based on the observed server row), and the stale draft still conflicted with it. The walk now also follows an own later edit whose expected revision equals the revision the current link's receipt produced: it was based on exactly that state, so no collaborator write sits between. BatchOperations now matches selected rows with matchesItemId, so a selection that still holds a temp- ID keeps its row (and edit base) after the row acquires its server ID. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/offline-edit-base-ui.test.mjs | 11 +++++++++++ scripts/offline-edit-base.test.mjs | 22 ++++++++++++++++++++++ src/components/BatchOperations.tsx | 4 +++- src/lib/offline.ts | 6 +++++- 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/scripts/offline-edit-base-ui.test.mjs b/scripts/offline-edit-base-ui.test.mjs index f1ddebb..f382432 100644 --- a/scripts/offline-edit-base-ui.test.mjs +++ b/scripts/offline-edit-base-ui.test.mjs @@ -40,6 +40,17 @@ test('batch operations queue the selected rendered rows as their edit base', asy } }); +test('batch snapshots include a selected temporary row that has since acquired its server ID', async () => { + fixture.queued = []; + const row = item('I9', 'New', { _localKey: 'op-create' }); + const view = render(createElement(BatchOperations, { selectedIds: new Set(['temp-op-create']), items: [row], onClearSelection() {}, userDid: 'did:a' })); + try { + fireEvent.click(view.getByLabelText('Check all selected items')); + await waitFor(() => assert.equal(fixture.queued.length, 1)); + assert.deepEqual(fixture.queued[0].snapshots, [row]); + } finally { cleanup(); } +}); + test('row remove and fallback toggles queue the rendered row as their edit base', async () => { const row = item('I1', 'Milk'); for (const [label, type] of [['Remove Milk', 'removeItem'], ['Check Milk', 'checkItem']]) { diff --git a/scripts/offline-edit-base.test.mjs b/scripts/offline-edit-base.test.mjs index 4e03ab7..4ef7bb0 100644 --- a/scripts/offline-edit-base.test.mjs +++ b/scripts/offline-edit-base.test.mjs @@ -25,6 +25,28 @@ test("a draft opened on an optimistic version chains through the client's own la assert.equal(f.rows.items[0].checked, false); }); +test("a draft chains through the client's own later edit made after its base was acknowledged and observed", async () => { + const f = await replayFixture(modules); + await queue(f, 'checkItem', { itemId: 'I1', checkedAt: 1 }); + const draftSource = projectItems(structuredClone(f.rows.items), await all(f), 'L1')[0]; + await new SyncManager().sync(f.client, f.session); + const observe = async () => { + const snap = await f.call('items', 'getListItemsForReplay', { listId: 'L1', operationIds: (await all(f)).map(m => m.operationId) }); + await store.cacheListSnapshot(f.session.accountId, 'L1', snap.items, snap.acknowledgments, snap.sequence); + return snap.items; + }; + // The uncheck is based on the observed server row, so it has no predecessor link. + await queue(f, 'uncheckItem', { itemId: 'I1' }, await observe()); + assert.equal((await pending(f))[0].expected[0].predecessor, undefined); + await new SyncManager().sync(f.client, f.session); + await observe(); + await queue(f, 'updateItem', { itemId: 'I1', name: 'Oat milk' }, [draftSource]); + await new SyncManager().sync(f.client, f.session); + assert.deepEqual(await pending(f), []); + assert.equal(f.rows.items[0].name, 'Oat milk'); + assert.equal(f.rows.items[0].checked, false); +}); + test("a draft still conflicts with a collaborator's edit made after the client's own later edits", async () => { const f = await replayFixture(modules); await queue(f, 'checkItem', { itemId: 'I1', checkedAt: 1 }); diff --git a/src/components/BatchOperations.tsx b/src/components/BatchOperations.tsx index 350c61a..c8165ed 100644 --- a/src/components/BatchOperations.tsx +++ b/src/components/BatchOperations.tsx @@ -7,6 +7,7 @@ import type { Id } from "../../convex/_generated/dataModel"; import { useSettings } from "../hooks/useSettings"; import { useOffline } from "../hooks/useOffline"; import type { OptimisticItem } from "../hooks/useOptimisticItems"; +import { matchesItemId } from "../lib/optimisticItems"; import { ConfirmDialog } from "./ConfirmDialog"; interface BatchOperationsProps { @@ -31,7 +32,8 @@ export function BatchOperations({ const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); const count = selectedIds.size; - const selectedRows = () => items.filter(item => selectedIds.has(item._id)); + // A selection can keep a temp- ID after its row acquires the server ID. + const selectedRows = () => items.filter(item => [...selectedIds].some(id => matchesItemId(item, id))); const handleCheckAll = async () => { if (isProcessing) return; diff --git a/src/lib/offline.ts b/src/lib/offline.ts index 681fedb..5ed7b4d 100644 --- a/src/lib/offline.ts +++ b/src/lib/offline.ts @@ -177,7 +177,11 @@ async function enqueueMutation(accountId: string, input: { let predecessorId = sourceOperations.get(id) ?? predecessor?.operationId; // This client's own later edits descended from a pinned draft base are not // competing writes. Chain through the newest, so only collaborators conflict. - for (let next; sourceOperations.has(id) && (next = [...prior].reverse().find(m => !m.denied && m.expected.some(e => e.predecessor === predecessorId && resolveOperationId(e.id, prior) === resolvedId)));) predecessorId = next.operationId; + // A descendant either names the link as predecessor, or was based on exactly + // the revision the link's receipt produced (queued after it was observed). + const descends = (m: QueuedMutation, link: QueuedMutation) => !m.denied && m.id! > link.id! && m.expected.some(e => resolveOperationId(e.id, prior) === resolvedId && + (e.predecessor === link.operationId || (!e.predecessor && e.revision === link.ack?.revisions[resolvedId]))); + for (let link, next; sourceOperations.has(id) && (link = prior.find(m => m.operationId === predecessorId)) && (next = [...prior].reverse().find(m => descends(m, link!)));) predecessorId = next.operationId; return { id, revision: revisions.get(id)!, ...(predecessorId ? { predecessor: predecessorId } : {}) }; }); const dependencies = [...expected.flatMap(e => e.predecessor ? [e.predecessor] : []), ...operationReferences({ payload, expected }).filter(id => id.startsWith('temp-')).map(id => id.slice(5))]; From b6c1221d105dad3aca71dfb0c316d4e6b9684f0f Mon Sep 17 00:00:00 2001 From: Brian Date: Thu, 8 Oct 2026 01:13:06 -0700 Subject: [PATCH 4/4] fix(offline): address adversarial review of draft chaining and edit bases - Route row remove and batch actions through useOptimisticItems (removeItem, new queueBatch), so every list edit queues against the same server/cache base rows. This replaces passing on-screen rows from ListItem/BatchOperations, which split the edit-base rule across four places and could pin a just-discarded operation and drop a tap. - Do not chain a draft through an own edit already rejected as a conflict: the draft waited behind it forever and joined its discard cascade (on main it applied independently). - Conflict review now lists the later edits that will be reapplied with the reviewed edit (masking legacy-pending payloads), so a dependent draft can't overwrite a collaborator's newer version unseen. - Extract the descendant walk into newestOwnDescendant. - E2E fixture: fix the `path` ReferenceError in apply's default branch, throw resourceUnavailable-shaped errorData from getItemForSync like production, and notify the account's other sockets before dropping a committed replay response. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/offline-edit-base-ui.test.mjs | 53 ++++++++-------------- scripts/offline-edit-base.test.mjs | 42 +++++++++++++++++ scripts/offline-hooks.test.mjs | 21 +++++++++ src/components/BatchOperations.tsx | 34 +++----------- src/components/ListItem.tsx | 9 ++-- src/components/NestedListItem.tsx | 4 ++ src/components/offline/OfflineRecovery.tsx | 6 ++- src/hooks/useOptimisticItems.tsx | 7 ++- src/lib/offline.ts | 25 +++++++--- src/pages/ListView.tsx | 7 ++- tests/fixtures/server.mjs | 15 ++++-- 11 files changed, 145 insertions(+), 78 deletions(-) diff --git a/scripts/offline-edit-base-ui.test.mjs b/scripts/offline-edit-base-ui.test.mjs index f382432..5952ecc 100644 --- a/scripts/offline-edit-base-ui.test.mjs +++ b/scripts/offline-edit-base-ui.test.mjs @@ -7,11 +7,11 @@ if (!GlobalRegistrator.isRegistered) GlobalRegistrator.register(); globalThis.IS_REACT_ACT_ENVIRONMENT = true; const { render, fireEvent, waitFor, cleanup } = await import('@testing-library/react'); const { createElement } = await import('react'); -const fixture = globalThis.__editBase = { queued: [] }; +const fixture = globalThis.__editBase = { direct: [] }; await build({ entryPoints: ['src/components/BatchOperations.tsx', 'src/components/ListItem.tsx'], outdir: 'tmp/offline-edit-base-ui', outbase: '.', bundle: true, splitting: true, platform: 'node', format: 'esm', outExtension: { '.js': '.mjs' }, jsx: 'automatic', external: ['react', 'react/jsx-runtime', 'react-dom'], plugins: [{ name: 'edit-base-fixtures', setup(b) { b.onResolve({ filter: /\/useOffline$|\/useSettings$|\/SubItems$|\/ItemAttribution$|\/ItemDetailsModal$|\/share$/ }, args => ({ path: args.path, namespace: 'fixture' })); b.onLoad({ filter: /.*/, namespace: 'fixture' }, ({ path }) => ({ contents: - path.endsWith('/useOffline') ? 'export const useOffline=()=>({queueMutation:async(input,snapshots)=>{globalThis.__editBase.queued.push({input,snapshots});return 1;}});' : + path.endsWith('/useOffline') ? 'export const useOffline=()=>({queueMutation:async(input,snapshots)=>{globalThis.__editBase.direct.push({input,snapshots});return 1;}});' : path.endsWith('/useSettings') ? 'export const useSettings=()=>({haptic(){}});' : path.endsWith('/SubItems') ? 'export const useSubItemProgress=()=>null;' : path.endsWith('/ItemAttribution') ? 'export const ItemAttribution=()=>null;' : @@ -22,45 +22,32 @@ await build({ entryPoints: ['src/components/BatchOperations.tsx', 'src/component const load = path => import(pathToFileURL(`${process.cwd()}/tmp/offline-edit-base-ui/${path}.mjs`)); const { BatchOperations } = await load('src/components/BatchOperations'); const { ListItem } = await load('src/components/ListItem'); -const item = (id, name, extra = {}) => ({ _id: id, _creationTime: 1, listId: 'L1', name, checked: false, createdByDid: 'did:a', createdAt: 1, updatedAt: 1, ...extra }); +const item = (id, name) => ({ _id: id, _creationTime: 1, listId: 'L1', name, checked: false, createdByDid: 'did:a', createdAt: 1, updatedAt: 1 }); -// Without the rendered rows, an uncached target is queued with an 'unknown' -// revision and the server rejects the user's own edit as a conflict. -test('batch operations queue the selected rendered rows as their edit base', async () => { - const rows = [item('I1', 'Milk'), item('I2', 'Bread', { _operationId: 'op-prior', _isOptimistic: true }), item('I3', 'Eggs')]; +// Without a base snapshot, an uncached target is queued with an 'unknown' +// revision and the server rejects the user's own edit as a conflict. These +// entry points must use useOptimisticItems, which queues against base rows. +test('batch operations queue through the list hook, never directly', async () => { for (const [label, type] of [['Check all selected items', 'batchCheckItems'], ['Uncheck all selected items', 'batchUncheckItems']]) { - fixture.queued = []; - const view = render(createElement(BatchOperations, { selectedIds: new Set(['I1', 'I2']), items: rows, onClearSelection() {}, userDid: 'did:a' })); + const batches = []; + const view = render(createElement(BatchOperations, { selectedIds: new Set(['I1', 'temp-op']), queueBatch: async (...args) => { batches.push(args); }, onClearSelection() {}, userDid: 'did:a' })); try { fireEvent.click(view.getByLabelText(label)); - await waitFor(() => assert.equal(fixture.queued.length, 1)); - assert.equal(fixture.queued[0].input.type, type); - assert.deepEqual(fixture.queued[0].snapshots?.map(i => i._id), ['I1', 'I2']); + await waitFor(() => assert.equal(batches.length, 1)); + assert.equal(batches[0][0], type); + assert.deepEqual(batches[0][1].itemIds, ['I1', 'temp-op']); + assert.deepEqual(fixture.direct, []); } finally { cleanup(); } } }); -test('batch snapshots include a selected temporary row that has since acquired its server ID', async () => { - fixture.queued = []; - const row = item('I9', 'New', { _localKey: 'op-create' }); - const view = render(createElement(BatchOperations, { selectedIds: new Set(['temp-op-create']), items: [row], onClearSelection() {}, userDid: 'did:a' })); +test('row remove queues through the list hook when provided', async () => { + const removed = []; + const view = render(createElement(ListItem, { item: item('I1', 'Milk'), userDid: 'did:a', onRemove: async (...args) => { removed.push(args); } })); try { - fireEvent.click(view.getByLabelText('Check all selected items')); - await waitFor(() => assert.equal(fixture.queued.length, 1)); - assert.deepEqual(fixture.queued[0].snapshots, [row]); + fireEvent.click(view.getByLabelText('Remove Milk')); + await waitFor(() => assert.equal(removed.length, 1)); + assert.deepEqual(removed[0], ['I1', 'did:a', undefined]); + assert.deepEqual(fixture.direct, []); } finally { cleanup(); } }); - -test('row remove and fallback toggles queue the rendered row as their edit base', async () => { - const row = item('I1', 'Milk'); - for (const [label, type] of [['Remove Milk', 'removeItem'], ['Check Milk', 'checkItem']]) { - fixture.queued = []; - const view = render(createElement(ListItem, { item: row, userDid: 'did:a' })); - try { - fireEvent.click(view.getByLabelText(label)); - await waitFor(() => assert.equal(fixture.queued.length, 1)); - assert.equal(fixture.queued[0].input.type, type); - assert.deepEqual(fixture.queued[0].snapshots, [row]); - } finally { cleanup(); } - } -}); diff --git a/scripts/offline-edit-base.test.mjs b/scripts/offline-edit-base.test.mjs index 4ef7bb0..70af45c 100644 --- a/scripts/offline-edit-base.test.mjs +++ b/scripts/offline-edit-base.test.mjs @@ -74,3 +74,45 @@ test('a rendered row is a valid edit base for an uncached item; without one the assert.deepEqual(await pending(f), []); assert.equal(f.rows.items[0].checked, true); }); + +const observe = async f => { + const snap = await f.call('items', 'getListItemsForReplay', { listId: 'L1', operationIds: (await all(f)).map(m => m.operationId) }); + await store.cacheListSnapshot(f.session.accountId, 'L1', snap.items, snap.acknowledgments, snap.sequence); + return snap.items; +}; + +test('a draft does not chain through an own edit already rejected as a conflict', async () => { + const f = await replayFixture(modules); + f.rows.items.push({ ...structuredClone(f.rows.items[0]), _id: 'I2', name: 'Bread', vcProofs: [] }); + await queue(f, 'checkItem', { itemId: 'I1', checkedAt: 1 }); + const draftSource = projectItems(structuredClone(f.rows.items), await all(f), 'L1').find(i => i._id === 'I1'); + await new SyncManager().sync(f.client, f.session); + await queue(f, 'batchUncheckItems', { itemIds: ['I1', 'I2'] }, await observe(f)); + await f.call('items', 'updateItem', { itemId: 'I2', name: 'Rye' }, f.collaborator); + await new SyncManager().sync(f.client, f.session); + const [batch] = await pending(f); + assert.equal(batch.state, 'conflict'); + await queue(f, 'updateItem', { itemId: 'I1', name: 'Oat milk' }, [draftSource]); + // The draft must neither wait behind the rejected batch nor join its discard cascade. + assert.deepEqual(store.discardCascade(await all(f), batch.operationId).map(m => m.type), ['batchUncheckItems']); + await new SyncManager().sync(f.client, f.session); + assert.equal(f.rows.items[0].name, 'Oat milk'); + assert.deepEqual((await pending(f)).map(m => m.operationId), [batch.operationId]); +}); + +test('a draft chained behind a pending own edit is part of that edit when it is reviewed as a conflict', async () => { + const f = await replayFixture(modules); + await queue(f, 'checkItem', { itemId: 'I1', checkedAt: 1 }); + const draftSource = projectItems(structuredClone(f.rows.items), await all(f), 'L1')[0]; + await new SyncManager().sync(f.client, f.session); + await queue(f, 'uncheckItem', { itemId: 'I1' }, await observe(f)); + await f.call('items', 'updateItem', { itemId: 'I1', name: 'Collaborator name' }, f.collaborator); + await queue(f, 'updateItem', { itemId: 'I1', name: 'Oat milk' }, [draftSource]); + await new SyncManager().sync(f.client, f.session); + const [uncheck, draft] = await pending(f); + assert.equal(uncheck.state, 'conflict'); + assert.equal(f.rows.items[0].name, 'Collaborator name'); + // OfflineRecovery lists this cascade beside the reviewed conflict, so the + // draft's rename is never reapplied over the collaborator's without review. + assert.deepEqual(store.discardCascade(await all(f), uncheck.operationId).map(m => m.operationId), [uncheck.operationId, draft.operationId]); +}); diff --git a/scripts/offline-hooks.test.mjs b/scripts/offline-hooks.test.mjs index 18c433c..e6880f5 100644 --- a/scripts/offline-hooks.test.mjs +++ b/scripts/offline-hooks.test.mjs @@ -301,3 +301,24 @@ test('revocation notification clears mounted cache fallbacks and denied optimist assert.equal((await store.getOperations(accountId))[0].payload.name, 'Only my new text'); } finally { view.unmount(); cleanup(); } }); + +test('row remove and batch edits queue against the server base rows, even before the list cache is written', async () => { + const { revision } = await import('../shared/replay.ts'); + fixture.user = { turnkeySubOrgId: 'hook-edit-base', did: 'did:a' }; fixture.token = 'token-edit-base'; + // A collaborator's new row: visible in the live snapshot, never cached on this device. + const remote = { ...item('I7'), name: 'Collaborator row' }; + fixture.snapshots.set('hook-edit-base:L1', { items: [remote], acknowledgments: [], sequence: 0 }); + const view = renderHook(() => useOptimisticItems('L1')); + try { + await waitFor(() => assert.equal(view.result.current.items.length, 1)); + await act(async () => { + await view.result.current.queueBatch('batchCheckItems', { itemIds: ['I7'], checkedByDid: 'did:a' }); + await view.result.current.removeItem('I7', 'did:a'); + }); + const operations = await store.getOperations('hook-edit-base'); + assert.deepEqual(operations.map(m => m.type), ['batchCheckItems', 'removeItem']); + assert.equal(operations[0].expected[0].revision, await revision(remote)); + assert.deepEqual(operations.map(m => m.listIds), [['L1'], ['L1']]); + assert.equal(operations[1].expected[0].predecessor, operations[0].operationId); + } finally { view.unmount(); cleanup(); } +}); diff --git a/src/components/BatchOperations.tsx b/src/components/BatchOperations.tsx index c8165ed..de63bde 100644 --- a/src/components/BatchOperations.tsx +++ b/src/components/BatchOperations.tsx @@ -5,15 +5,13 @@ import { useState } from "react"; import type { Id } from "../../convex/_generated/dataModel"; import { useSettings } from "../hooks/useSettings"; -import { useOffline } from "../hooks/useOffline"; -import type { OptimisticItem } from "../hooks/useOptimisticItems"; -import { matchesItemId } from "../lib/optimisticItems"; +import type { BatchMutationType } from "../hooks/useOptimisticItems"; import { ConfirmDialog } from "./ConfirmDialog"; interface BatchOperationsProps { selectedIds: Set>; - /** Rendered rows: the versions the user acted on are the replay edit base. */ - items: OptimisticItem[]; + /** useOptimisticItems().queueBatch: queues against the list's base rows. */ + queueBatch: (type: BatchMutationType, payload: { itemIds: Id<"items">[]; [key: string]: unknown }) => Promise; onClearSelection: () => void; userDid: string; legacyDid?: string; @@ -21,19 +19,16 @@ interface BatchOperationsProps { export function BatchOperations({ selectedIds, - items, + queueBatch, onClearSelection, userDid, legacyDid, }: BatchOperationsProps) { const { haptic } = useSettings(); - const { queueMutation } = useOffline(); const [isProcessing, setIsProcessing] = useState(false); const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); const count = selectedIds.size; - // A selection can keep a temp- ID after its row acquires the server ID. - const selectedRows = () => items.filter(item => [...selectedIds].some(id => matchesItemId(item, id))); const handleCheckAll = async () => { if (isProcessing) return; @@ -46,12 +41,7 @@ export function BatchOperations({ legacyDid, }; - await queueMutation({ - type: "batchCheckItems", - payload, - timestamp: Date.now(), - retryCount: 0, - }, selectedRows()); + await queueBatch("batchCheckItems", payload); haptic("success"); onClearSelection(); @@ -74,12 +64,7 @@ export function BatchOperations({ legacyDid, }; - await queueMutation({ - type: "batchUncheckItems", - payload, - timestamp: Date.now(), - retryCount: 0, - }, selectedRows()); + await queueBatch("batchUncheckItems", payload); haptic("success"); onClearSelection(); @@ -102,12 +87,7 @@ export function BatchOperations({ legacyDid, }; - await queueMutation({ - type: "batchDeleteItems", - payload, - timestamp: Date.now(), - retryCount: 0, - }, selectedRows()); + await queueBatch("batchDeleteItems", payload); haptic("success"); onClearSelection(); diff --git a/src/components/ListItem.tsx b/src/components/ListItem.tsx index d1b5866..6c23790 100644 --- a/src/components/ListItem.tsx +++ b/src/components/ListItem.tsx @@ -32,6 +32,7 @@ interface ListItemProps { onTouchStart?: (e: React.TouchEvent, itemId: string, element: HTMLElement) => void; onCheck?: (itemId: Id<"items">, checkedByDid: string, legacyDid?: string) => Promise; onUncheck?: (itemId: Id<"items">, userDid: string, legacyDid?: string) => Promise; + onRemove?: (itemId: Id<"items">, userDid: string, legacyDid?: string) => Promise; // Selection mode props isSelectMode?: boolean; isSelected?: boolean; @@ -53,6 +54,7 @@ export const ListItem = memo(function ListItem({ onTouchStart, onCheck, onUncheck, + onRemove, isSelectMode = false, isSelected = false, onToggleSelect, @@ -107,10 +109,10 @@ export const ListItem = memo(function ListItem({ try { if (item.checked) { if (onUncheck) await onUncheck(item._id, userDid, legacyDid); - else await queueMutation({ type: "uncheckItem", payload: { itemId: item._id, userDid, legacyDid } }, [item]); + else await queueMutation({ type: "uncheckItem", payload: { itemId: item._id, userDid, legacyDid } }); } else { if (onCheck) await onCheck(item._id, userDid, legacyDid); - else await queueMutation({ type: "checkItem", payload: { itemId: item._id, checkedByDid: userDid, legacyDid, checkedAt: Date.now() } }, [item]); + else await queueMutation({ type: "checkItem", payload: { itemId: item._id, checkedByDid: userDid, legacyDid, checkedAt: Date.now() } }); } } catch (err) { console.error("Failed to toggle item:", err); @@ -125,7 +127,8 @@ export const ListItem = memo(function ListItem({ haptic('medium'); setIsUpdating(true); try { - await queueMutation({ type: "removeItem", payload: { itemId: item._id, userDid, legacyDid } }, [item]); + if (onRemove) await onRemove(item._id, userDid, legacyDid); + else await queueMutation({ type: "removeItem", payload: { itemId: item._id, userDid, legacyDid } }); } catch (err) { console.error("Failed to remove item:", err); haptic('error'); diff --git a/src/components/NestedListItem.tsx b/src/components/NestedListItem.tsx index 4a94f9f..38257da 100644 --- a/src/components/NestedListItem.tsx +++ b/src/components/NestedListItem.tsx @@ -25,6 +25,7 @@ interface NestedListItemProps { onTouchStart?: (e: React.TouchEvent, itemId: string, element: HTMLElement) => void; onCheck?: (itemId: Id<"items">, checkedByDid: string, legacyDid?: string) => Promise; onUncheck?: (itemId: Id<"items">, userDid: string, legacyDid?: string) => Promise; + onRemove?: (itemId: Id<"items">, userDid: string, legacyDid?: string) => Promise; isSelectMode?: boolean; isSelected?: boolean; onToggleSelect?: () => void; @@ -51,6 +52,7 @@ export function NestedListItem({ onTouchStart, onCheck, onUncheck, + onRemove, isSelectMode = false, isSelected = false, onToggleSelect, @@ -136,6 +138,7 @@ export function NestedListItem({ onTouchStart={onTouchStart} onCheck={onCheck} onUncheck={onUncheck} + onRemove={onRemove} isSelectMode={isSelectMode} isSelected={isSelected} onToggleSelect={onToggleSelect} @@ -171,6 +174,7 @@ export function NestedListItem({ canEdit={canEdit} onCheck={onCheck} onUncheck={onUncheck} + onRemove={onRemove} isSelectMode={isSelectMode} depth={depth + 1} maxDepth={maxDepth} diff --git a/src/components/offline/OfflineRecovery.tsx b/src/components/offline/OfflineRecovery.tsx index 6fb292f..fb20525 100644 --- a/src/components/offline/OfflineRecovery.tsx +++ b/src/components/offline/OfflineRecovery.tsx @@ -44,6 +44,7 @@ export function OfflineRecovery() { const discardRoot = discard?.accountId === accountId && discard.token === token ? operations.find(m => m.operationId === discard.operationId && (m.state === 'conflict' || m.state === 'failed')) : undefined; const discardEdits = discardRoot ? discardCascade(operations, discardRoot.operationId) : []; + const reviewDependents = review ? discardCascade(operations, review.mutation.operationId).filter(m => m.operationId !== review.mutation.operationId) : []; const confirmDiscard = async () => { if (!isCurrent() || !discardRoot || discarding) return; setDiscarding(true); @@ -88,10 +89,13 @@ export function OfflineRecovery() { {review?.accountId === accountId && !operations.find(m => m.operationId === review.mutation.operationId)?.denied &&

Current server version

{JSON.stringify(review.items.map(i => ({ name: i.name, checked: i.checked, description: i.description })), null, 2)}

Your saved edit

{JSON.stringify(review.mutation.payload, null, 2)}
+ {/* Later edits that build on this one are reapplied with it. Show them, + so none overwrites the newer server version without being reviewed. */} + {reviewDependents.length > 0 && <>

Later edits applied with it

{reviewDependents.map(m => 
{m.type}: {m.legacyRecoveryPending ? 'Older edit retained for recovery' : JSON.stringify(m.payload, null, 2)}
)}} + }}>{reviewDependents.length ? `Apply ${reviewDependents.length + 1} saved edits to this version` : 'Apply saved edit to this version'}
} {legacy &&

Older offline edits are retained on this device. They have no verified account binding and will not replay automatically.

Sign in to the original account to export and manually review its edits. Unattributed entries stay retained and hidden.

} {message &&

{message}

} diff --git a/src/hooks/useOptimisticItems.tsx b/src/hooks/useOptimisticItems.tsx index 51e838e..586759a 100644 --- a/src/hooks/useOptimisticItems.tsx +++ b/src/hooks/useOptimisticItems.tsx @@ -9,10 +9,12 @@ import { projectItems } from '../lib/optimisticItems'; const EMPTY_ITEMS: OfflineItem[] = []; const EMPTY_ACKNOWLEDGMENTS: ReplayAck[] = []; const EMPTY_OPERATION_IDS: string[] = []; +export type BatchMutationType = 'batchCheckItems' | 'batchUncheckItems' | 'batchDeleteItems'; export interface OptimisticItem extends Doc<'items'> { assigneeDids?: string[]; _isOptimistic?: boolean; _syncError?: string; _localKey?: string; _operationId?: string } /** Durable queue entries are the optimistic state, including after reload. - * Acknowledgments identify creates by operation ID; names and clocks never do. */ + * Acknowledgments identify creates by operation ID; names and clocks never do. + * Every list edit queues against the same server/cache base rows. */ export function useOptimisticItems(listId: Id<'lists'>) { const { isOnline, accountId, operations, queueMutation, aliases, compaction } = useOffline(); const [scanOffset, setScanOffset] = useState(0); @@ -95,7 +97,8 @@ export function useOptimisticItems(listId: Id<'lists'>) { const reorderItems = useCallback((itemIds: Id<'items'>[], userDid: string, legacyDid?: string) => enqueue('reorderItem', { listId, itemIds, userDid, legacyDid }), [enqueue, listId]); const updateItem = useCallback((args: { itemId: Id<'items'>; userDid: string; legacyDid?: string; [key: string]: unknown }) => enqueue('updateItem', args), [enqueue]); const removeItem = useCallback((itemId: Id<'items'>, userDid: string, legacyDid?: string) => enqueue('removeItem', { itemId, userDid, legacyDid }), [enqueue]); - return { items, addItem, checkItem, uncheckItem, reorderItems, updateItem, removeItem, + const queueBatch = useCallback((type: BatchMutationType, payload: { itemIds: Id<'items'>[]; [key: string]: unknown }) => enqueue(type, payload), [enqueue]); + return { items, addItem, checkItem, uncheckItem, reorderItems, updateItem, removeItem, queueBatch, isLoading: server === undefined && cached.scope !== scope && !last.current, usingCache: !isOnline && server === undefined && cached.scope === scope }; } diff --git a/src/lib/offline.ts b/src/lib/offline.ts index 5ed7b4d..a6f5ed9 100644 --- a/src/lib/offline.ts +++ b/src/lib/offline.ts @@ -175,13 +175,7 @@ async function enqueueMutation(accountId: string, input: { return revisions.get(id) === 'unknown' || (!observed && (createsTarget || target?.revision === revisions.get(id))); }); let predecessorId = sourceOperations.get(id) ?? predecessor?.operationId; - // This client's own later edits descended from a pinned draft base are not - // competing writes. Chain through the newest, so only collaborators conflict. - // A descendant either names the link as predecessor, or was based on exactly - // the revision the link's receipt produced (queued after it was observed). - const descends = (m: QueuedMutation, link: QueuedMutation) => !m.denied && m.id! > link.id! && m.expected.some(e => resolveOperationId(e.id, prior) === resolvedId && - (e.predecessor === link.operationId || (!e.predecessor && e.revision === link.ack?.revisions[resolvedId]))); - for (let link, next; sourceOperations.has(id) && (link = prior.find(m => m.operationId === predecessorId)) && (next = [...prior].reverse().find(m => descends(m, link!)));) predecessorId = next.operationId; + if (sourceOperations.has(id)) predecessorId = newestOwnDescendant(prior, predecessorId!, resolvedId); return { id, revision: revisions.get(id)!, ...(predecessorId ? { predecessor: predecessorId } : {}) }; }); const dependencies = [...expected.flatMap(e => e.predecessor ? [e.predecessor] : []), ...operationReferences({ payload, expected }).filter(id => id.startsWith('temp-')).map(id => id.slice(5))]; @@ -198,6 +192,23 @@ async function enqueueMutation(accountId: string, input: { changed(); return id; } +/** This client's own later edits descended from a pinned draft base are not + * competing writes: chain through the newest, so only collaborators conflict. + * A descendant names the link as predecessor, or was based on exactly the + * revision the link's receipt produced (queued after it was observed). Edits + * already rejected as conflicts are not followed: the draft would wait behind + * them and fall into their discard cascade. */ +function newestOwnDescendant(operations: QueuedMutation[], operationId: string, itemId: string): string { + const newestFirst = [...operations].reverse(); + for (let link = operations.find(m => m.operationId === operationId); link;) { + const base = link; + const next = newestFirst.find(m => !m.denied && m.state !== 'conflict' && m.id! > base.id! && m.expected.some(e => resolveOperationId(e.id, operations) === itemId && + (e.predecessor === base.operationId || (!e.predecessor && e.revision === base.ack?.revisions[itemId])))); + if (!next) return base.operationId; + link = next; + } + return operationId; +} export async function saveOperation(accountId: string, mutation: QueuedMutation) { if (mutation.accountId !== accountId) throw new Error('Offline account mismatch'); const tx = (await getOfflineDB(accountId)).transaction('mutations', 'readwrite'); diff --git a/src/pages/ListView.tsx b/src/pages/ListView.tsx index e2ba1d2..103672c 100644 --- a/src/pages/ListView.tsx +++ b/src/pages/ListView.tsx @@ -111,6 +111,7 @@ export function ListView() { uncheckItem, reorderItems, removeItem, + queueBatch, updateItem, isLoading: itemsLoading, usingCache, @@ -1035,6 +1036,7 @@ export function ListView() { onTouchStart={canUserEdit ? groceryTouchDrag.handleTouchStart : undefined} onCheck={checkItemWithStreak} onUncheck={uncheckItem} + onRemove={removeItem} isSelectMode={canUserEdit && isSelectMode} isSelected={selectedIds.has(item._id)} onToggleSelect={() => toggleSelection(item._id)} @@ -1140,6 +1142,7 @@ export function ListView() { isFocused={focusedIndex === globalIndex} onCheck={checkItemWithStreak} onUncheck={uncheckItem} + onRemove={removeItem} isSelectMode={canUserEdit && isSelectMode} isSelected={selectedIds.has(item._id)} onToggleSelect={() => toggleSelection(item._id)} @@ -1182,6 +1185,7 @@ export function ListView() { onTouchStart={canUserEdit ? touchDrag.handleTouchStart : undefined} onCheck={checkItemWithStreak} onUncheck={uncheckItem} + onRemove={removeItem} isSelectMode={canUserEdit && isSelectMode} isSelected={selectedIds.has(item._id)} onToggleSelect={() => toggleSelection(item._id)} @@ -1239,6 +1243,7 @@ export function ListView() { onTouchStart={canUserEdit ? touchDrag.handleTouchStart : undefined} onCheck={checkItemWithStreak} onUncheck={uncheckItem} + onRemove={removeItem} isSelectMode={canUserEdit && isSelectMode} isSelected={selectedIds.has(item._id)} onToggleSelect={() => toggleSelection(item._id)} @@ -1363,7 +1368,7 @@ export function ListView() { {canUserEdit && ( args.operationIds.includes(r.operationId)).map(r => r.ack), sequence: account.sequence, }; - case 'items:getItemForSync': return account.items.find(i => i._id === args.itemId) ?? null; + case 'items:getItemForSync': { + // Production throws resourceUnavailable() for missing and unreadable items alike. + const item = account.items.find(i => i._id === args.itemId); + if (!item) throw Object.assign(new Error('Resource unavailable'), { errorData: { kind: 'auth', code: 'FORBIDDEN', message: 'Resource unavailable' } }); + return item; + } case 'items:getOfflineAccount': return { accountId: user.turnkeySubOrgId, did: user.did }; case 'items:getOfflineAccess': return [...new Set([...args.listIds, ...(args.items ?? []).map(i => i.listId)])].map(listId => { const canRead = account.lists.some(l => l._id === listId); @@ -189,7 +194,7 @@ async function apply(account, endpoint, args) { case 'publication:publishList': account.published = true; break; case 'publication:unpublishList': account.published = false; break; case 'referrals:getOrCreateReferralCode': result = 'e2e-code'; break; - default: throw new Error(`Unimplemented fixture mutation: ${path}`); + default: throw new Error(`Unimplemented fixture mutation: ${endpoint}`); } return result; } @@ -200,8 +205,9 @@ function transition(ws, newVersion = ws.data.version) { try { return { type: 'QueryUpdated', queryId, value: query(account, path, args), logLines: [] }; } catch (error) { - account.errors.push(error.message); - return { type: 'QueryFailed', queryId, errorMessage: error.message, logLines: [] }; + if (!error.errorData) account.errors.push(error.message); + return { type: 'QueryFailed', queryId, errorMessage: error.message, logLines: [], + ...(error.errorData ? { errorData: error.errorData } : {}) }; } }); ws.send(JSON.stringify({ type: 'Transition', @@ -314,6 +320,7 @@ const backend = Bun.serve({ if (args.replay && account.dropReplayResponses > 0) { // The write and its receipt committed; the connection dies first. account.dropReplayResponses--; + for (const socket of sockets) if (socket !== ws && socket.data.account === account) transition(socket); ws.close(); return; }