From 7b27f85b39f8e38b95fd500b93d2bfecff99da5e Mon Sep 17 00:00:00 2001 From: timmy Date: Sat, 15 Aug 2026 20:00:14 +0000 Subject: [PATCH] feat: resolve mobile checklist conflicts (Closes #913) --- frontend/authored-outbox.js | 51 ++++++++++++++++++++++++++-- frontend/checklist-conflict.js | 58 ++++++++++++++++++++++++++++++++ frontend/dashboard.js | 41 ++++++++++++++++++++++ frontend/drafts.js | 5 ++- frontend/issue-sheet.js | 2 +- src/frontend_bundle.py | 2 +- tests/test_authored_outbox.py | 37 ++++++++++++++++++++ tests/test_checklist_conflict.py | 49 +++++++++++++++++++++++++++ tests/test_drafts.py | 22 ++++++++++++ tests/test_my_work.py | 20 ++++++++++- 10 files changed, 281 insertions(+), 6 deletions(-) create mode 100644 frontend/checklist-conflict.js create mode 100644 tests/test_checklist_conflict.py diff --git a/frontend/authored-outbox.js b/frontend/authored-outbox.js index ada9a9f..24dbc73 100644 --- a/frontend/authored-outbox.js +++ b/frontend/authored-outbox.js @@ -1,4 +1,4 @@ -function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync, getOwnerLogin = () => '', createOperationId, now = () => Date.now(), maxItems = 50 }) { +function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync, getOwnerLogin = () => '', createOperationId, mergeChecklistConflict, now = () => Date.now(), maxItems = 50 }) { const storageKey = 'stackchain.authored-outbox.v1'; const makeId = createOperationId || (() => globalThis.crypto?.randomUUID?.() || String(Date.now()) + '-' + Math.random().toString(16).slice(2) @@ -113,6 +113,7 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync, } : {}), ...(message.kind === 'issue-content' ? { title: String(message.title || ''), + baseBody: String(message.baseBody ?? message.body ?? ''), expectedUpdatedAt: String(message.expectedUpdatedAt || ''), } : {}), }; @@ -363,6 +364,51 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync, return retryItem(id, currentLogin); } + async function resolveIssueContentConflict(id, latest, currentLogin) { + const previousItems = read(); + const item = previousItems.find(candidate => candidate.id === id); + currentLogin = String(currentLogin || '').trim(); + if (!item || item.kind !== 'issue-content' || item.status !== 'attention') { + throw new Error('This checklist conflict is no longer available.'); + } + if (!currentLogin || item.ownerLogin !== currentLogin) { + throw new Error('Confirm the account that queued this checklist update.'); + } + if (typeof mergeChecklistConflict !== 'function') { + throw new Error('Checklist conflict review is unavailable.'); + } + const merged = mergeChecklistConflict({ + baseBody: String(item.baseBody ?? item.body ?? ''), + localBody: String(item.body || ''), + remoteBody: String(latest?.body || ''), + }); + if (merged.conflicts?.length) return merged; + if (!String(latest?.updated_at || '')) throw new Error('Latest issue revision is unavailable.'); + const rebased = { + ...item, + operationId: String(makeId()).slice(0, 128), + title: String(latest?.title || ''), + baseBody: String(latest?.body || ''), + body: merged.body, + expectedUpdatedAt: String(latest.updated_at), + status: 'queued', + }; + delete rebased.error; + delete rebased.deliveryState; + const nextItems = previousItems.map(candidate => candidate.id === id ? rebased : candidate); + write(nextItems, false); + if (backgroundSync?.reconcile) { + try { await backgroundSync.reconcile(nextItems, 'authored'); } + catch (error) { + write(previousItems, false); + throw error; + } + try { await backgroundSync.requestSync?.(); } + catch (_error) { /* Foreground retry remains available. */ } + } + return { ...merged, item: rebased }; + } + function reconcileBackground(records) { const statuses = new Map((records || []).map(item => [item.id, item])); const items = read().flatMap(item => { @@ -388,7 +434,8 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync, return items; } - return { enqueue, enqueueDurably, update, discard, flush, retry, reconcileBackground, list: () => read().map(item => ({ ...item })) }; + return { enqueue, enqueueDurably, update, discard, flush, retry, resolveIssueContentConflict, + reconcileBackground, list: () => read().map(item => ({ ...item })) }; } if (typeof module !== 'undefined' && module.exports) module.exports = createAuthoredOutbox; diff --git a/frontend/checklist-conflict.js b/frontend/checklist-conflict.js new file mode 100644 index 0000000..3e9f423 --- /dev/null +++ b/frontend/checklist-conflict.js @@ -0,0 +1,58 @@ +function mergeChecklistConflict({ baseBody, localBody, remoteBody }) { + const taskPattern = /^(\s*[-*+]\s+\[)([ xX])(\]\s+)(.*)$/; + + function tasks(body) { + const entries = []; + String(body || '').split('\n').forEach((line, lineIndex) => { + const match = line.match(taskPattern); + if (!match) return; + const label = match[4].trim(); + const key = label.replace(/\s+/g, ' ').toLocaleLowerCase(); + if (!key) return; + entries.push({ key, label, checked: match[2].toLowerCase() === 'x', lineIndex, match }); + }); + return entries; + } + + function grouped(entries) { + const result = new Map(); + entries.forEach(entry => result.set(entry.key, [...(result.get(entry.key) || []), entry])); + return result; + } + + const base = grouped(tasks(baseBody)); + const local = grouped(tasks(localBody)); + const remoteEntries = tasks(remoteBody); + const remote = grouped(remoteEntries); + const changes = []; + const conflicts = []; + + for (const [key, baseMatches] of base) { + const localMatches = local.get(key) || []; + if (baseMatches.length !== 1 || localMatches.length !== 1) continue; + if (baseMatches[0].checked === localMatches[0].checked) continue; + const remoteMatches = remote.get(key) || []; + if (remoteMatches.length !== 1) { + conflicts.push({ + label: baseMatches[0].label, + reason: remoteMatches.length ? 'ambiguous' : 'missing', + }); + continue; + } + changes.push({ label: remoteMatches[0].label, checked: localMatches[0].checked }); + } + + if (conflicts.length) return { body: null, changes, conflicts }; + const desired = new Map(changes.map(change => [ + change.label.replace(/\s+/g, ' ').toLocaleLowerCase(), change.checked, + ])); + const lines = String(remoteBody || '').split('\n'); + remoteEntries.forEach(entry => { + if (!desired.has(entry.key)) return; + const marker = desired.get(entry.key) ? 'x' : ' '; + lines[entry.lineIndex] = entry.match[1] + marker + entry.match[3] + entry.match[4]; + }); + return { body: lines.join('\n'), changes, conflicts: [] }; +} + +if (typeof module !== 'undefined') module.exports = mergeChecklistConflict; diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 7fd0ec6..ac3ba51 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -673,6 +673,7 @@ storage: localStorage, fetchJson: fetchReviewJson, coordinator: outboxCoordinator, backgroundSync: backgroundIssueSync, getOwnerLogin: () => confirmedOwnerLogin, + mergeChecklistConflict: mergeChecklistConflict, }); const queueOfflineIssueBlocker = createOfflineIssueBlocker({ enqueueDurably: message => authoredOutbox.enqueueDurably(message), @@ -2745,6 +2746,7 @@ const isUnfiled = item.kind === 'unfiled-issue'; const reviewOutbox = item.outbox_kind === 'pull-review'; const closureOutbox = item.outbox_kind === 'issue-close'; + const checklistConflict = item.checklist_conflict === true; const sendLabel = item.delivery_state === 'uncertain' ? 'Verified not posted — retry' : 'Send now'; const outboxActions = item.quarantined ? '' + @@ -2764,6 +2766,9 @@ '' + '' + '' : + item.kind === 'authored-outbox' && checklistConflict ? + '' + + '' : item.kind === 'authored-outbox' && closureOutbox ? '' + '' + @@ -2873,6 +2878,42 @@ else applyOutboxResult(await issueOutbox.retry(item.outbox_id, activeFlushLogin)); }); }); + list.querySelectorAll('.draft-review-checklist').forEach(button => { + button.addEventListener('click', async () => { + const item = lastDrafts[Number(button.dataset.draftIndex)]; + const queued = authoredOutbox.list().find(candidate => candidate.id === item?.outbox_id); + if (!item?.checklist_conflict || !queued || !activeFlushLogin) return; + button.disabled = true; + qs('#my-work-action-status').textContent = 'Loading the latest checklist for review…'; + try { + const latest = await issueController.load({ + repository:item.repository, number:item.number, + }); + const preview = mergeChecklistConflict({ + baseBody:queued.baseBody, localBody:queued.body, remoteBody:latest.body, + }); + if (preview.conflicts.length) { + qs('#my-work-action-status').textContent = 'Checklist changes for ' + + preview.conflicts.map(conflict => conflict.label).join(', ') + + ' could not be matched safely. Open the issue to compare renamed or deleted tasks.'; + return; + } + if (!window.confirm('Apply ' + preview.changes.length + ' checklist change' + + (preview.changes.length === 1 ? '' : 's') + ' to the latest issue? Remote prose and other tasks will stay unchanged.')) return; + const result = await authoredOutbox.resolveIssueContentConflict(item.outbox_id, latest, activeFlushLogin); + if (result.conflicts.length) return; + const delivery = await authoredOutbox.retry(item.outbox_id, activeFlushLogin); + applyAuthoredOutboxResult(delivery); + qs('#my-work-action-status').textContent = delivery.confirmed?.length ? + 'Checklist changes applied to the latest issue.' : + 'The issue changed again. Review the checklist conflict against the new version.'; + } catch (error) { + qs('#my-work-action-status').textContent = String(error?.message || 'Checklist review could not be loaded. Retry when online.'); + } finally { + button.disabled = false; + } + }); + }); list.querySelectorAll('.draft-authorize').forEach(button => { button.addEventListener('click', async () => { const item = lastDrafts[Number(button.dataset.draftIndex)]; diff --git a/frontend/drafts.js b/frontend/drafts.js index 37d0e4e..4decd5c 100644 --- a/frontend/drafts.js +++ b/frontend/drafts.js @@ -155,6 +155,8 @@ function createDraftInbox({ storage, getCurrentLogin = () => '', now = () => Dat .filter(Boolean).join('\n\n'); const ownerLogin = String(item.ownerLogin || '').trim(); const quarantined = !ownerLogin || !currentLogin || ownerLogin !== currentLogin; + const checklistConflict = !quarantined && item.kind === 'issue-content' && + item.status === 'attention' && typeof item.baseBody === 'string'; return { id: 'stackchain.authored-outbox.v1:' + item.id, outbox_id: item.id, @@ -162,7 +164,7 @@ function createDraftInbox({ storage, getCurrentLogin = () => '', now = () => Dat kind: 'authored-outbox', status: item.status === 'attention' ? 'attention' : (item.status === 'sending' ? 'sending' : (item.status === 'authorization' || isClosure ? 'authorization' : 'queued')), - label: item.deliveryState === 'uncertain' ? 'Verify delivery' : + label: checklistConflict ? 'Checklist conflict' : item.deliveryState === 'uncertain' ? 'Verify delivery' : (item.status === 'attention' ? (isClosure ? 'Issue closure needs attention' : 'Needs attention') : (isReview ? (item.status === 'authorization' ? 'Review awaiting authorization' : 'Queued review') : (isClosure ? 'Awaiting authorization' : 'Queued message'))), @@ -172,6 +174,7 @@ function createDraftInbox({ storage, getCurrentLogin = () => '', now = () => Dat title: target, preview: textPreview([item.error, item.body].filter(Boolean).join(' — ')), copy_text: copyText, + checklist_conflict: checklistConflict, ...(isReview ? { head_sha: item.expectedHeadSha } : {}), quarantined, ownership: quarantined ? 'Queued by ' + (ownerLogin || 'an unknown account') + diff --git a/frontend/issue-sheet.js b/frontend/issue-sheet.js index 2365187..662990f 100644 --- a/frontend/issue-sheet.js +++ b/frontend/issue-sheet.js @@ -158,7 +158,7 @@ function createIssueSheet({ fetchJson, storage, renderMarkdown = globalThis.rend } const body = toggleTask(detail.body, taskIndex, checked); await enqueueDurably({ kind:'issue-content', repository:item.repository, number:item.number, - title:detail.title, body, expectedUpdatedAt:detail.updated_at }); + title:detail.title, baseBody:detail.body, body, expectedUpdatedAt:detail.updated_at }); return { queued:true, detail:{ ...detail, body, checklist_pending:true } }; }, pendingTask(item, detail, items, ownerLogin) { diff --git a/src/frontend_bundle.py b/src/frontend_bundle.py index ad79b1a..bf1ff0f 100644 --- a/src/frontend_bundle.py +++ b/src/frontend_bundle.py @@ -36,7 +36,7 @@ FEATURE_SOURCES = { "static/draft-filing-session.js", "static/draft-capacity-dialog.js", "static/work-selection.js", "static/today-work.js", "static/pick-work.js", "static/batch-find-work.js", "static/search-batch-plan.js", "static/issue-evidence-review.js", "static/issue-evidence-editor.js", - "static/issue-attachment.js", "static/authored-outbox.js", "static/issue-sheet.js", "static/issue-filing-review.js", "static/issue-filing-receipt.js", + "static/issue-attachment.js", "static/checklist-conflict.js", "static/authored-outbox.js", "static/issue-sheet.js", "static/issue-filing-review.js", "static/issue-filing-receipt.js", ), } CACHE_DECLARATION = re.compile( diff --git a/tests/test_authored_outbox.py b/tests/test_authored_outbox.py index 766740f..85e5344 100644 --- a/tests/test_authored_outbox.py +++ b/tests/test_authored_outbox.py @@ -176,6 +176,43 @@ const outbox = createAuthoredOutbox({{ assert output["items"][0]["body"] == "- [x] Build\n- [ ] Test" +def test_issue_content_conflict_resolution_rebases_task_delta_and_mirrors_before_retry(): + script = f""" +const createAuthoredOutbox = require({json.dumps(str(OUTBOX))}); +const mergeChecklistConflict = require({json.dumps(str(Path(__file__).parents[1] / 'frontend' / 'checklist-conflict.js'))}); +const initial = {{version:2,items:[{{ + id:'check-1',operationId:'old-op',kind:'issue-content',ownerLogin:'timmy',status:'attention', + repository:'o/r',number:9,title:'Ship',baseBody:'Intro\\n- [ ] Build\\n- [ ] Test', + body:'Intro\\n- [x] Build\\n- [ ] Test',expectedUpdatedAt:'old-revision',error:'Issue changed', +}}]}}; +const values = new Map([['stackchain.authored-outbox.v1', JSON.stringify(initial)]]); +const mirrors = []; let syncs = 0; +const storage = {{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}}; +const outbox = createAuthoredOutbox({{ + storage, getOwnerLogin:()=> 'timmy', createOperationId:()=> 'rebased-op', mergeChecklistConflict, + backgroundSync:{{reconcile:async items=>mirrors.push(items.map(item=>({{...item}}))),requestSync:async()=>{{syncs += 1;}}}}, +}}); +(async()=>{{ + const result = await outbox.resolveIssueContentConflict('check-1', {{ + title:'Ship safely', body:'Remote intro\\n- [ ] Test\\n- [ ] Build', updated_at:'latest-revision', + }}, 'timmy'); + process.stdout.write(JSON.stringify({{result,item:outbox.list()[0],mirrors,syncs}})); +}})(); +""" + output = run_node(script) + + assert output["result"]["changes"] == [{"label": "Build", "checked": True}] + assert output["item"]["status"] == "queued" + assert output["item"]["operationId"] == "rebased-op" + assert output["item"]["title"] == "Ship safely" + assert output["item"]["body"] == "Remote intro\n- [ ] Test\n- [x] Build" + assert output["item"]["baseBody"] == "Remote intro\n- [ ] Test\n- [ ] Build" + assert output["item"]["expectedUpdatedAt"] == "latest-revision" + assert "error" not in output["item"] + assert output["mirrors"][-1][0] == output["item"] + assert output["syncs"] == 1 + + def test_authored_outbox_persists_and_delivers_desired_blocker_state(): script = f""" const createAuthoredOutbox = require({json.dumps(str(OUTBOX))}); diff --git a/tests/test_checklist_conflict.py b/tests/test_checklist_conflict.py new file mode 100644 index 0000000..cdd7666 --- /dev/null +++ b/tests/test_checklist_conflict.py @@ -0,0 +1,49 @@ +import json +import subprocess +from pathlib import Path + + +MERGER = Path(__file__).parents[1] / "frontend" / "checklist-conflict.js" + + +def run_node(script: str): + result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + return json.loads(result.stdout) + + +def test_merge_applies_local_task_delta_to_latest_body_without_reverting_remote_edits(): + script = f""" +const mergeChecklistConflict = require({json.dumps(str(MERGER))}); +const result = mergeChecklistConflict({{ + baseBody:'Intro\\n- [ ] Build\\n- [ ] Test', + localBody:'Intro\\n- [x] Build\\n- [ ] Test', + remoteBody:'Updated intro\\n- [ ] Test\\n- [ ] Build\\n- [x] Document', +}}); +process.stdout.write(JSON.stringify(result)); +""" + output = run_node(script) + + assert output == { + "body": "Updated intro\n- [ ] Test\n- [x] Build\n- [x] Document", + "changes": [{"label": "Build", "checked": True}], + "conflicts": [], + } + + +def test_merge_reports_missing_or_ambiguous_changed_tasks_without_writing_a_body(): + script = f""" +const mergeChecklistConflict = require({json.dumps(str(MERGER))}); +const missing = mergeChecklistConflict({{ + baseBody:'- [ ] Ship', localBody:'- [x] Ship', remoteBody:'- [ ] Release', +}}); +const duplicate = mergeChecklistConflict({{ + baseBody:'- [ ] Ship', localBody:'- [x] Ship', remoteBody:'- [ ] Ship\\n- [ ] Ship', +}}); +process.stdout.write(JSON.stringify({{missing,duplicate}})); +""" + output = run_node(script) + + assert output["missing"]["body"] is None + assert output["missing"]["conflicts"] == [{"label": "Ship", "reason": "missing"}] + assert output["duplicate"]["body"] is None + assert output["duplicate"]["conflicts"] == [{"label": "Ship", "reason": "ambiguous"}] diff --git a/tests/test_drafts.py b/tests/test_drafts.py index 6c301a7..75c36f1 100644 --- a/tests/test_drafts.py +++ b/tests/test_drafts.py @@ -121,6 +121,28 @@ process.stdout.write(JSON.stringify(drafts)); assert output[1]["label"] == "Queued issue" +def test_draft_inbox_identifies_account_bound_checklist_conflicts_for_review(): + script = f""" +const createDraftInbox = require({json.dumps(str(DRAFTS))}); +const record = {{version:2,items:[{{ + id:'check-1',kind:'issue-content',ownerLogin:'timmy',status:'attention',repository:'o/r',number:7, + title:'Ship',baseBody:'- [ ] Build',body:'- [x] Build',expectedUpdatedAt:'old',error:'Issue changed',queuedAt:12, +}}]}}; +const values = new Map([['stackchain.authored-outbox.v1', JSON.stringify(record)]]); +const storage = {{get length(){{return values.size}},key:i=>Array.from(values.keys())[i]||null,getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}}; +const owned = createDraftInbox({{storage,getCurrentLogin:()=>'timmy'}}).list()[0]; +const other = createDraftInbox({{storage,getCurrentLogin:()=>'alexander'}}).list()[0]; +process.stdout.write(JSON.stringify({{owned,other}})); +""" + output = run_node(script) + + assert output["owned"]["label"] == "Checklist conflict" + assert output["owned"]["checklist_conflict"] is True + assert output["owned"]["quarantined"] is False + assert output["other"]["checklist_conflict"] is False + assert output["other"]["quarantined"] is True + + def test_draft_inbox_exposes_queued_review_with_review_route_and_actionable_state(): script = f""" const createDraftInbox = require({json.dumps(str(DRAFTS))}); diff --git a/tests/test_my_work.py b/tests/test_my_work.py index 6ff20d7..355efed 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -33,6 +33,7 @@ LATER_PICKER = Path(__file__).parents[1] / "frontend" / "later-picker.js" LATER_AND_START = Path(__file__).parents[1] / "frontend" / "later-and-start.js" REVIEW_SHEET = Path(__file__).parents[1] / "frontend" / "review-sheet.js" ISSUE_SHEET = Path(__file__).parents[1] / "frontend" / "issue-sheet.js" +FRONTEND_BUNDLE = Path(__file__).parents[1] / "src" / "frontend_bundle.py" CREATE_ISSUE_SHEET = Path(__file__).parents[1] / "frontend" / "create-issue-sheet.js" PULL_SHEET = Path(__file__).parents[1] / "frontend" / "pull-sheet.js" CONVERSATION = Path(__file__).parents[1] / "frontend" / "conversation.js" @@ -2224,6 +2225,22 @@ async def test_mobile_issue_detail_toggles_checklist_with_touch_safe_recovery(): assert '.checklist-pending .task-list-toggle { opacity:.65;' in html +@pytest.mark.anyio +async def test_mobile_drafts_reviews_and_retries_an_unambiguous_checklist_conflict(): + html = await dashboard() + + assert "mergeChecklistConflict: mergeChecklistConflict" in html + assert "const checklistConflict = item.checklist_conflict === true" in html + assert 'class="draft-review-checklist"' in html + assert ">Review changes" in html + assert "const latest = await issueController.load({" in html + assert "authoredOutbox.resolveIssueContentConflict(item.outbox_id, latest, activeFlushLogin)" in html + assert "window.confirm('Apply ' + preview.changes.length + ' checklist change'" in html + assert "await authoredOutbox.retry(item.outbox_id, activeFlushLogin)" in html + assert "could not be matched safely" in html + assert "checklist-conflict.js" in FRONTEND_BUNDLE.read_text() + + def test_completed_issue_checklist_offers_close_and_next_during_today(): script = f""" const createIssueSheet = require({json.dumps(str(ISSUE_SHEET))}); @@ -2326,7 +2343,8 @@ pending.then(result => process.stdout.write(JSON.stringify({{before,queued,resul assert output["before"] is False assert output["queued"] == [{ "kind": "issue-content", "repository": "stackchain/api", "number": 17, - "title": "Release", "body": "- [x] Build\n- [X] Ship", + "title": "Release", "baseBody": "- [ ] Build\n- [X] Ship", + "body": "- [x] Build\n- [X] Ship", "expectedUpdatedAt": "2026-08-15T10:00:00Z", }] assert output["result"] == { -- 2.43.0