From 27fc74ced6ea8995f581de70ec79a1cdcfbccaa2 Mon Sep 17 00:00:00 2001 From: timmy Date: Sun, 9 Aug 2026 06:37:30 +0000 Subject: [PATCH] fix: keep Later plans conflict-safe across devices (#377) --- README.md | 7 +- frontend/dashboard.js | 7 +- frontend/later-sync.js | 17 +++- frontend/service-worker.js | 2 +- src/later_store.py | 105 ++++++++++++---------- src/main.py | 2 + tests/test_later_store.py | 70 +++++++++++++++ tests/test_later_sync.py | 36 +++++++- tests/test_markdown_renderer.py | 2 +- tests/test_mobile_composer_integration.py | 2 +- tests/test_service_worker.py | 12 +-- tests/test_today_sync.py | 2 +- 12 files changed, 197 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 8669ea9..a2b6a26 100644 --- a/README.md +++ b/README.md @@ -224,8 +224,11 @@ or changing any Gitea issue or pull request. They automatically return to their existing priority position at the wake time, and **Bring back now** restores them early. Later wake times are scoped to the confirmed Gitea login and synchronize across signed-in tabs and devices. Offline changes apply immediately, survive reload, -and replay after reconnect; `STACKCHAIN_LATER_DB` can override the default durable -store at `.stackchain-state/later.sqlite3`. +and replay after reconnect. Each queued edit carries the account revision it was based +on; if another device has since changed the same item, Stackchain keeps the newer +account plan and reports the conflict instead of silently overwriting it. Unrelated +items continue syncing in the same batch. `STACKCHAIN_LATER_DB` can override the +default durable store at `.stackchain-state/later.sqlite3`. After one successful online load, the installed dashboard precaches a versioned, subpath-scoped application shell. During a network outage or a dashboard HTTP diff --git a/frontend/dashboard.js b/frontend/dashboard.js index de8d9e8..d82d6b7 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -186,9 +186,10 @@ }, onStatus: (state, detail = {}) => { qs('#later-sync-status').textContent = state === 'saved' ? 'Later saved to account.' : - (state === 'retrying' ? `Later saved on this device · retrying in ${Math.ceil(detail.delayMs / 1000)}s.` : - (state === 'pending' ? 'Later saved on this device · sync pending.' : - 'Later sync unavailable · changes stay on this device.')); + (state === 'conflict' ? `Another device changed this Later item${detail.count > 1 ? 's' : ''} · account plan kept.` : + (state === 'retrying' ? `Later saved on this device · retrying in ${Math.ceil(detail.delayMs / 1000)}s.` : + (state === 'pending' ? 'Later saved on this device · sync pending.' : + 'Later sync unavailable · changes stay on this device.'))); }, }); laterSync.startLifecycle({ window, document }); diff --git a/frontend/later-sync.js b/frontend/later-sync.js index c8c4f4a..5fe9156 100644 --- a/frontend/later-sync.js +++ b/frontend/later-sync.js @@ -115,7 +115,11 @@ function createLaterSync({ storage, getLogin, fetchJson, onRemoteRecords, onStat knownOperationKeys.delete(record.recordKey); }); return records.filter(record => latestByItem.get(record.operation.item_id) === record) - .map(record => record.operation).filter(operation => + .map(record => ({ + ...record.operation, + base_revision: Number.isInteger(record.operation.base_revision) + ? record.operation.base_revision : 0, + })).filter(operation => operation && typeof operation.operation_id === 'string' && ['defer', 'restore'].includes(operation.action) && typeof operation.item_id === 'string' && (operation.action === 'restore' || typeof operation.wake_at === 'string')); @@ -148,7 +152,10 @@ function createLaterSync({ storage, getLogin, fetchJson, onRemoteRecords, onStat (action === 'defer' && typeof wakeAt !== 'string')) return false; pending().filter(operation => operation.item_id === itemId) .forEach(operation => removeOperation(operation.operation_id)); - const operation = { operation_id: operationId(), action, item_id: itemId, wake_at: wakeAt }; + const operation = { + operation_id: operationId(), action, item_id: itemId, wake_at: wakeAt, + base_revision: Math.max(0, savedRevision()), + }; const storageKey = key(); if (!storageKey || !storage) return false; const recordKey = storageKey + '.operation.' + encodeURIComponent(operation.operation_id); @@ -186,6 +193,7 @@ function createLaterSync({ storage, getLogin, fetchJson, onRemoteRecords, onStat try { let operations = pending(); let plan; + let conflictCount = 0; if (!operations.length) plan = await fetchJson('api/v1/later'); while (operations.length) { if (key() !== ownerKey) return false; @@ -202,6 +210,8 @@ function createLaterSync({ storage, getLogin, fetchJson, onRemoteRecords, onStat ...(plan.duplicate_operation_ids || []), ...(plan.rejected_operations || []).map(item => item.operation_id), ] : batch.map(item => item.operation_id); + conflictCount += (plan.rejected_operations || []) + .filter(item => item.reason === 'stale_intent').length; for (const operationId of received) { if (pending().some(candidate => candidate.operation_id === operationId) && !removeOperation(operationId)) { @@ -211,7 +221,8 @@ function createLaterSync({ storage, getLogin, fetchJson, onRemoteRecords, onStat operations = pending(); } adopt(plan); - onStatus?.(pending().length ? 'pending' : 'saved'); + onStatus?.(pending().length ? 'pending' : + (conflictCount ? 'conflict' : 'saved'), conflictCount ? { count: conflictCount } : {}); retryAttempt = 0; cancelRetry(); return true; diff --git a/frontend/service-worker.js b/frontend/service-worker.js index 76d1653..4ecfba6 100644 --- a/frontend/service-worker.js +++ b/frontend/service-worker.js @@ -1,6 +1,6 @@ const BASE = new URL('./', self.location.href).pathname; importScripts(BASE + 'static/background-issue-sync.js'); -const CACHE = 'stackchain-dashboard-shell-v54'; +const CACHE = 'stackchain-dashboard-shell-v55'; const OUTAGE_STATUSES = new Set([500, 502, 503, 504]); const NAVIGATION_TIMEOUT_MS = self.__STACKCHAIN_NAVIGATION_TIMEOUT_MS || 4000; const SHELL = [ diff --git a/src/later_store.py b/src/later_store.py index 5580b92..6a6ca80 100644 --- a/src/later_store.py +++ b/src/later_store.py @@ -33,6 +33,16 @@ class LaterStore: ) """ ) + connection.execute( + """ + CREATE TABLE IF NOT EXISTS later_item_revisions ( + login TEXT NOT NULL, + item_id TEXT NOT NULL, + revision INTEGER NOT NULL, + PRIMARY KEY (login, item_id) + ) + """ + ) return connection @staticmethod @@ -74,54 +84,16 @@ class LaterStore: item_id: str, *, wake_at: str | None = None, + base_revision: int | None = None, ) -> dict: - login = self._normalize_login(login) - if not operation_id or not item_id: - raise ValueError("operation_id and item_id are required") - if action not in {"defer", "restore"}: - raise ValueError("unsupported Later action") - if action == "defer": - wake_at = self._validate_wake_at(wake_at) - - with self._connect() as connection: - connection.execute("BEGIN IMMEDIATE") - row = connection.execute( - "SELECT revision, records FROM later_plans WHERE login = ?", (login,) - ).fetchone() - snapshot = self._snapshot(row) - duplicate = connection.execute( - "SELECT 1 FROM later_operations WHERE login = ? AND operation_id = ?", - (login, operation_id), - ).fetchone() - if duplicate: - return snapshot - - records = dict(snapshot["records"]) - before = records.get(item_id) - if action == "defer": - records[item_id] = wake_at - changed = before != wake_at - else: - changed = item_id in records - records.pop(item_id, None) - - revision = snapshot["revision"] + (1 if changed else 0) - serialized = json.dumps(records, separators=(",", ":"), sort_keys=True) - if row is None: - connection.execute( - "INSERT INTO later_plans(login, revision, records) VALUES (?, ?, ?)", - (login, revision, serialized), - ) - elif changed: - connection.execute( - "UPDATE later_plans SET revision = ?, records = ? WHERE login = ?", - (revision, serialized, login), - ) - connection.execute( - "INSERT INTO later_operations(login, operation_id) VALUES (?, ?)", - (login, operation_id), - ) - return {"revision": revision, "records": records} + result = self.apply_batch(login, [{ + "operation_id": operation_id, + "action": action, + "item_id": item_id, + "wake_at": wake_at, + "base_revision": base_revision, + }]) + return {"revision": result["revision"], "records": result["records"]} def apply_batch(self, login: str, operations: list[dict]) -> dict: """Apply ordered deferrals with one SQLite write transaction.""" @@ -136,6 +108,20 @@ class LaterStore: revision = snapshot["revision"] accepted: list[str] = [] duplicates: list[str] = [] + rejected: list[dict[str, str]] = [] + + # Existing databases predate per-item revisions. Conservatively mark + # active deferrals as changed at the latest known plan revision. + for item_id in records: + connection.execute( + "INSERT OR IGNORE INTO later_item_revisions(login, item_id, revision) VALUES (?, ?, ?)", + (login, item_id, revision), + ) + item_revisions = dict(connection.execute( + "SELECT item_id, revision FROM later_item_revisions WHERE login = ?", + (login,), + ).fetchall()) + batch_start_item_revisions = dict(item_revisions) for operation in operations: operation_id = operation.get("operation_id", "") @@ -156,6 +142,22 @@ class LaterStore: duplicates.append(operation_id) continue + base_revision = operation.get("base_revision") + if base_revision is None: + base_revision = snapshot["revision"] + if not isinstance(base_revision, int) or isinstance(base_revision, bool) or base_revision < 0: + raise ValueError("base_revision must be a non-negative integer") + if batch_start_item_revisions.get(item_id, 0) > base_revision: + connection.execute( + "INSERT INTO later_operations(login, operation_id) VALUES (?, ?)", + (login, operation_id), + ) + rejected.append({ + "operation_id": operation_id, + "reason": "stale_intent", + }) + continue + before = records.get(item_id) if action == "defer": records[item_id] = wake_at @@ -164,6 +166,13 @@ class LaterStore: changed = item_id in records records.pop(item_id, None) revision += 1 if changed else 0 + if changed: + item_revisions[item_id] = revision + connection.execute( + "INSERT INTO later_item_revisions(login, item_id, revision) VALUES (?, ?, ?) " + "ON CONFLICT(login, item_id) DO UPDATE SET revision = excluded.revision", + (login, item_id, revision), + ) connection.execute( "INSERT INTO later_operations(login, operation_id) VALUES (?, ?)", (login, operation_id), @@ -186,5 +195,5 @@ class LaterStore: "records": records, "accepted_operation_ids": accepted, "duplicate_operation_ids": duplicates, - "rejected_operations": [], + "rejected_operations": rejected, } diff --git a/src/main.py b/src/main.py index d1450bc..b73ddc8 100644 --- a/src/main.py +++ b/src/main.py @@ -222,6 +222,7 @@ class LaterOperation(BaseModel): action: Literal["defer", "restore"] item_id: str = Field(min_length=1, max_length=500) wake_at: str | None = Field(default=None, max_length=100) + base_revision: int | None = Field(default=None, ge=0) class TodayOperationBatch(BaseModel): @@ -969,6 +970,7 @@ async def update_later_plan(payload: LaterOperation | LaterOperationBatch): payload.action, payload.item_id, wake_at=payload.wake_at, + base_revision=payload.base_revision, ) except ValueError as error: raise HTTPException(status_code=422, detail=str(error)) diff --git a/tests/test_later_store.py b/tests/test_later_store.py index 6d88352..533a72f 100644 --- a/tests/test_later_store.py +++ b/tests/test_later_store.py @@ -86,6 +86,57 @@ def test_batch_applies_in_one_ordered_idempotent_unit(tmp_path): assert replay["duplicate_operation_ids"] == ["first", "second"] +def test_batch_rejects_stale_same_item_intent_without_blocking_other_items(tmp_path): + store = LaterStore(tmp_path / "later.sqlite3") + first = store.apply_batch("timmy", [{ + "operation_id": "newer-device", + "action": "defer", + "item_id": "issue:r:1:", + "wake_at": "2026-08-12T09:00:00.000Z", + "base_revision": 0, + }]) + + replay = store.apply_batch("timmy", [ + { + "operation_id": "stale-device", + "action": "defer", + "item_id": "issue:r:1:", + "wake_at": "2026-08-10T09:00:00.000Z", + "base_revision": 0, + }, + { + "operation_id": "unrelated-item", + "action": "defer", + "item_id": "issue:r:2:", + "wake_at": "2026-08-11T09:00:00.000Z", + "base_revision": 0, + }, + ]) + + assert first["revision"] == 1 + assert replay == { + "revision": 2, + "records": { + "issue:r:1:": "2026-08-12T09:00:00.000Z", + "issue:r:2:": "2026-08-11T09:00:00.000Z", + }, + "accepted_operation_ids": ["unrelated-item"], + "duplicate_operation_ids": [], + "rejected_operations": [ + {"operation_id": "stale-device", "reason": "stale_intent"} + ], + } + duplicate = store.apply_batch("timmy", [{ + "operation_id": "stale-device", + "action": "restore", + "item_id": "issue:r:2:", + "base_revision": 2, + }]) + assert duplicate["duplicate_operation_ids"] == ["stale-device"] + assert duplicate["revision"] == 2 + assert duplicate["records"] == replay["records"] + + @pytest.mark.anyio async def test_authenticated_later_api_uses_confirmed_account_and_csrf(monkeypatch, tmp_path): monkeypatch.setenv("STACKCHAIN_DASHBOARD_AUTH_MODE", "operator") @@ -129,6 +180,21 @@ async def test_authenticated_later_api_uses_confirmed_account_and_csrf(monkeypat "X-CSRF-Token": client.cookies["stackchain_csrf"], }, ) + stale = await client.patch( + "/api/v1/later", + json={ + "operations": [{ + "operation_id": "offline-stale", + "action": "restore", + "item_id": "issue:stackchain/dashboard:363:", + "base_revision": 0, + }], + }, + headers={ + "Origin": "https://test", + "X-CSRF-Token": client.cookies["stackchain_csrf"], + }, + ) fetched = await client.get("/api/v1/later") assert forbidden.status_code == 403 @@ -143,6 +209,10 @@ async def test_authenticated_later_api_uses_confirmed_account_and_csrf(monkeypat "duplicate_operation_ids": [], "rejected_operations": [], } + assert stale.json()["rejected_operations"] == [ + {"operation_id": "offline-stale", "reason": "stale_intent"} + ] + assert stale.json()["revision"] == 2 assert fetched.json() == {"revision": 2, "records": { "issue:stackchain/dashboard:363:": "2026-08-10T09:00:00.000Z", "issue:stackchain/dashboard:365:": "2026-08-11T09:00:00.000Z", diff --git a/tests/test_later_sync.py b/tests/test_later_sync.py index 67d0eae..6908325 100644 --- a/tests/test_later_sync.py +++ b/tests/test_later_sync.py @@ -90,12 +90,44 @@ sync.enqueue('defer','issue:r:2:','2026-08-10T09:00:00.000Z'); "action": "defer", "item_id": "issue:r:2:", "wake_at": "2026-08-10T09:00:00.000Z", + "base_revision": 0, }]} assert result["records"]["issue:r:2:"] == "2026-08-10T09:00:00.000Z" assert result["status"] == "saved" assert result["pending"] == [] +def test_stale_receipt_adopts_server_truth_and_reports_another_device_conflict(): + script = f""" +const createLaterSync=require({json.dumps(str(LATER_SYNC))}); +const values=new Map();const requests=[];const statuses=[];const adopted=[]; +const storage={{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v),removeItem:k=>values.delete(k)}}; +const sync=createLaterSync({{ + storage,getLogin:()=> 'timmy',createOperationId:()=> 'offline-old', + fetchJson:async(_url,options={{}})=>{{ + if(!options.method)return {{revision:3,records:{{'issue:r:1:':'2026-08-12T09:00:00.000Z'}}}}; + const body=JSON.parse(options.body);requests.push(body); + return {{revision:4,records:{{'issue:r:1:':'2026-08-13T09:00:00.000Z'}},accepted_operation_ids:[],duplicate_operation_ids:[],rejected_operations:[{{operation_id:'offline-old',reason:'stale_intent'}}]}}; + }},onRemoteRecords:records=>adopted.push(records),onStatus:(state,detail={{}})=>statuses.push([state,detail]), +}}); +(async()=>{{await sync.flush();sync.enqueue('defer','issue:r:1:','2026-08-10T09:00:00.000Z');await sync.flush(); +process.stdout.write(JSON.stringify({{requests,statuses,adopted,pending:sync.pending()}}));}})(); +""" + result = run_node(script) + assert result["requests"] == [{"operations": [{ + "operation_id": "offline-old", + "action": "defer", + "item_id": "issue:r:1:", + "wake_at": "2026-08-10T09:00:00.000Z", + "base_revision": 3, + }]}] + assert result["statuses"][-1] == ["conflict", {"count": 1}] + assert result["adopted"][-1] == { + "issue:r:1:": "2026-08-13T09:00:00.000Z" + } + assert result["pending"] == [] + + def test_pending_later_edits_are_sent_as_one_batch_without_a_preflight_get(): script = f""" const createLaterSync=require({json.dumps(str(LATER_SYNC))}); @@ -136,6 +168,7 @@ sync.enqueue('restore','issue:r:2:'); "action": "restore", "item_id": "issue:r:2:", "wake_at": None, + "base_revision": 0, } ], "status": "retrying", @@ -283,10 +316,11 @@ async def test_dashboard_syncs_every_later_change_and_exposes_account_status(): assert "laterSync.migrate(laterWork.read());" in html assert "laterSync.flush();" in html assert "Later saved to account." in html + assert "Another device changed this Later item" in html def test_later_sync_ships_atomically_in_the_offline_shell(): source = (Path(__file__).parents[1] / "frontend" / "service-worker.js").read_text() - assert "stackchain-dashboard-shell-v54" in source + assert "stackchain-dashboard-shell-v55" in source assert "BASE + 'static/later-sync.js'" in source diff --git a/tests/test_markdown_renderer.py b/tests/test_markdown_renderer.py index ab9fd69..304d6ba 100644 --- a/tests/test_markdown_renderer.py +++ b/tests/test_markdown_renderer.py @@ -137,4 +137,4 @@ def test_markdown_work_bodies_are_mobile_safe_block_containers(): assert ".markdown-content { min-width:0; max-width:100%; overflow-wrap:anywhere;" in css assert ".markdown-content pre { max-width:100%; overflow-x:auto;" in css assert ".markdown-content a { min-height:44px;" in css - assert "stackchain-dashboard-shell-v54" in worker + assert "stackchain-dashboard-shell-v55" in worker diff --git a/tests/test_mobile_composer_integration.py b/tests/test_mobile_composer_integration.py index a3179c1..c089eb2 100644 --- a/tests/test_mobile_composer_integration.py +++ b/tests/test_mobile_composer_integration.py @@ -35,4 +35,4 @@ def test_offline_shell_contains_every_local_dashboard_runtime_asset(): shell_assets = set(re.findall(r"BASE \+ '([^']+)'", worker.split("async function sessionCsrf", 1)[0])) assert local_assets <= shell_assets, f"Offline shell is missing: {sorted(local_assets - shell_assets)}" - assert "stackchain-dashboard-shell-v54" in worker + assert "stackchain-dashboard-shell-v55" in worker diff --git a/tests/test_service_worker.py b/tests/test_service_worker.py index e138b43..0562a8a 100644 --- a/tests/test_service_worker.py +++ b/tests/test_service_worker.py @@ -108,7 +108,7 @@ async function dispatchNotificationClick(route) {{ def test_duplicate_aware_capture_ships_in_a_new_offline_shell(): source = WORKER.read_text() - assert "stackchain-dashboard-shell-v54" in source + assert "stackchain-dashboard-shell-v55" in source assert "BASE + 'static/create-issue-sheet.js'" in source assert "BASE + 'static/dashboard.js'" in source @@ -116,14 +116,14 @@ def test_duplicate_aware_capture_ships_in_a_new_offline_shell(): def test_exact_later_picker_ships_atomically_in_a_new_offline_shell(): source = WORKER.read_text() - assert "stackchain-dashboard-shell-v54" in source + assert "stackchain-dashboard-shell-v55" in source assert "BASE + 'static/later-picker.js'" in source def test_navigation_deadline_ships_in_a_new_shell_cache(): source = WORKER.read_text() - assert "stackchain-dashboard-shell-v54" in source + assert "stackchain-dashboard-shell-v55" in source assert "BASE + 'static/dashboard.css'" in source assert "BASE + 'static/dashboard.js'" in source assert "BASE + 'static/install-app.js'" in source @@ -132,21 +132,21 @@ def test_navigation_deadline_ships_in_a_new_shell_cache(): def test_today_convergence_ships_in_a_new_shell_cache(): source = WORKER.read_text() - assert "stackchain-dashboard-shell-v54" in source + assert "stackchain-dashboard-shell-v55" in source assert "BASE + 'static/today-sync.js'" in source def test_mobile_search_viewport_ships_in_a_new_offline_shell(): source = WORKER.read_text() - assert "stackchain-dashboard-shell-v54" in source + assert "stackchain-dashboard-shell-v55" in source assert "BASE + 'static/mobile-search-viewport.js'" in source def test_update_ownership_flow_ships_atomically_in_a_new_offline_shell(): source = WORKER.read_text() - assert "stackchain-dashboard-shell-v54" in source + assert "stackchain-dashboard-shell-v55" in source assert "BASE + 'static/update-ownership.js'" in source diff --git a/tests/test_today_sync.py b/tests/test_today_sync.py index 9e9963d..9b17cb6 100644 --- a/tests/test_today_sync.py +++ b/tests/test_today_sync.py @@ -86,7 +86,7 @@ sync.enqueue('add', 'issue:r:1:'); def test_inflight_today_drain_ships_in_a_new_offline_shell(): source = (Path(__file__).parents[1] / "frontend" / "service-worker.js").read_text() - assert "stackchain-dashboard-shell-v54" in source + assert "stackchain-dashboard-shell-v55" in source assert "BASE + 'static/today-sync.js'" in source