diff --git a/scripts/offline-edit-base-ui.test.mjs b/scripts/offline-edit-base-ui.test.mjs new file mode 100644 index 0000000..5952ecc --- /dev/null +++ b/scripts/offline-edit-base-ui.test.mjs @@ -0,0 +1,53 @@ +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 = { 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.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;' : + 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) => ({ _id: id, _creationTime: 1, listId: 'L1', name, checked: false, createdByDid: 'did:a', createdAt: 1, updatedAt: 1 }); + +// 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']]) { + 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(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('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('Remove Milk')); + await waitFor(() => assert.equal(removed.length, 1)); + assert.deepEqual(removed[0], ['I1', 'did:a', undefined]); + assert.deepEqual(fixture.direct, []); + } finally { cleanup(); } +}); diff --git a/scripts/offline-edit-base.test.mjs b/scripts/offline-edit-base.test.mjs new file mode 100644 index 0000000..70af45c --- /dev/null +++ b/scripts/offline-edit-base.test.mjs @@ -0,0 +1,118 @@ +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 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 }); + 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); +}); + +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 d52be26..de63bde 100644 --- a/src/components/BatchOperations.tsx +++ b/src/components/BatchOperations.tsx @@ -5,11 +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 { BatchMutationType } from "../hooks/useOptimisticItems"; import { ConfirmDialog } from "./ConfirmDialog"; interface BatchOperationsProps { selectedIds: Set>; + /** 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; @@ -17,12 +19,12 @@ interface BatchOperationsProps { export function BatchOperations({ selectedIds, + queueBatch, onClearSelection, userDid, legacyDid, }: BatchOperationsProps) { const { haptic } = useSettings(); - const { queueMutation } = useOffline(); const [isProcessing, setIsProcessing] = useState(false); const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); @@ -39,12 +41,7 @@ export function BatchOperations({ legacyDid, }; - await queueMutation({ - type: "batchCheckItems", - payload, - timestamp: Date.now(), - retryCount: 0, - }); + await queueBatch("batchCheckItems", payload); haptic("success"); onClearSelection(); @@ -67,12 +64,7 @@ export function BatchOperations({ legacyDid, }; - await queueMutation({ - type: "batchUncheckItems", - payload, - timestamp: Date.now(), - retryCount: 0, - }); + await queueBatch("batchUncheckItems", payload); haptic("success"); onClearSelection(); @@ -95,12 +87,7 @@ export function BatchOperations({ legacyDid, }; - await queueMutation({ - type: "batchDeleteItems", - payload, - timestamp: Date.now(), - retryCount: 0, - }); + await queueBatch("batchDeleteItems", payload); haptic("success"); onClearSelection(); diff --git a/src/components/ListItem.tsx b/src/components/ListItem.tsx index d8ba1a5..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, @@ -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 } }); + 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 2f8dd32..a6f5ed9 100644 --- a/src/lib/offline.ts +++ b/src/lib/offline.ts @@ -174,7 +174,8 @@ 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; + 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))]; @@ -191,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 f006793..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,6 +1368,7 @@ export function ListView() { {canUserEdit && ( }[]; 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..c49e3f9 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,15 @@ 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': { + // 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); @@ -86,10 +92,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); @@ -144,14 +194,9 @@ async function mutation(account, path, 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}`); } - 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) { @@ -160,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', @@ -205,7 +251,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 +291,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 +317,19 @@ 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--; + for (const socket of sockets) if (socket !== ws && socket.data.account === account) transition(socket); + 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 }); + }); +});