feat: queue offline pull request reviews (#419)

tests/test_authored_outbox.py

@@ -77,6 +77,55 @@ outbox.flush('timmy').then(result => process.stdout.write(JSON.stringify({{persi
     assert output["remaining"] == []
 
 
+def test_authored_outbox_persists_and_delivers_complete_pull_review_payload():
+    script = f"""
+const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});
+const values = new Map(); const calls = [];
+const storage = {{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v),removeItem:k=>values.delete(k)}};
+const outbox = createAuthoredOutbox({{
+  storage, getOwnerLogin:()=> 'timmy',
+  fetchJson: async (url, options) => {{ calls.push({{url, options}}); return {{id:44}}; }},
+}});
+let queued;
+try {{
+  queued = outbox.enqueue({{
+    kind:'pull-review', repository:'stackchain/web', number:8, operationId:'review-op',
+    body:'Looks good', decision:'approve', expectedHeadSha:'abc123',
+    comments:[{{path:'app.js',body:'Nice',new_position:4}}],
+    draftKey:'stackchain.review-draft.v1:stackchain/web#8@abc123',
+    progressKey:'stackchain.review-progress.v1:stackchain/web#8@abc123',
+    draftFingerprint:'draft', progressFingerprint:'progress',
+  }});
+}} catch (error) {{
+  process.stdout.write(JSON.stringify({{error:error.message}}));
+}}
+if (queued) {{
+  storage.setItem(queued.draftKey, 'draft'); storage.setItem(queued.progressKey, 'progress');
+  outbox.flush('timmy').then(result => process.stdout.write(JSON.stringify({{
+    queued, calls:calls.map(call=>({{url:call.url,key:call.options.headers['Idempotency-Key'],body:JSON.parse(call.options.body)}})),
+    result, remaining:outbox.list(), draft:storage.getItem(queued.draftKey), progress:storage.getItem(queued.progressKey),
+  }})));
+}}
+"""
+    output = run_node(script)
+
+    assert "error" not in output, output.get("error")
+
+    assert output["queued"]["kind"] == "pull-review"
+    assert output["queued"]["comments"] == [{"path": "app.js", "body": "Nice", "new_position": 4}]
+    assert output["calls"] == [{
+        "url": "api/v1/repos/stackchain/web/pulls/8/review",
+        "key": "review-op",
+        "body": {
+            "body": "Looks good", "decision": "approve", "expected_head_sha": "abc123",
+            "comments": [{"path": "app.js", "body": "Nice", "new_position": 4}],
+        },
+    }]
+    assert output["remaining"] == []
+    assert output["draft"] is None
+    assert output["progress"] is None
+
+
 def test_authored_outbox_classifies_failures_and_continues_past_attention_items():
     script = f"""
 const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});
@@ -183,6 +232,52 @@ process.stdout.write(JSON.stringify({{first,second,items:outbox.list()}}));
     assert len(output["items"]) == 1
 
 
+def test_authored_outbox_deduplicates_pull_review_for_same_head_after_reload():
+    script = f"""
+const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});
+const values=new Map();let sequence=0;
+const storage={{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}};
+const make=()=>createAuthoredOutbox({{storage,getOwnerLogin:()=>'timmy',createOperationId:()=> 'op-' + (++sequence)}});
+const first=make().enqueue({{kind:'pull-review',repository:'o/r',number:7,expectedHeadSha:'abc',decision:'approve',body:'Ready'}});
+const second=make().enqueue({{kind:'pull-review',repository:'o/r',number:7,expectedHeadSha:'abc',decision:'approve',body:'Ready'}});
+process.stdout.write(JSON.stringify({{first,second,items:make().list()}}));
+"""
+    output = run_node(script)
+
+    assert output["first"] == output["second"]
+    assert len(output["items"]) == 1
+
+
+def test_authored_outbox_rejects_changed_feedback_while_same_head_is_queued():
+    script = f"""
+const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});
+const values=new Map();const storage={{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}};
+const outbox=createAuthoredOutbox({{storage,getOwnerLogin:()=>'timmy'}});
+outbox.enqueue({{kind:'pull-review',repository:'o/r',number:7,expectedHeadSha:'abc',decision:'approve',body:'First'}});
+let error='';try{{outbox.enqueue({{kind:'pull-review',repository:'o/r',number:7,expectedHeadSha:'abc',decision:'request_changes',body:'Changed'}});}}catch(e){{error=e.message;}}
+process.stdout.write(JSON.stringify({{error,items:outbox.list()}}));
+"""
+    output = run_node(script)
+
+    assert output["error"] == "A review for this saved head is already queued. Open Drafts to inspect or discard it first."
+    assert len(output["items"]) == 1
+
+
+def test_confirmed_review_keeps_draft_edits_made_after_it_was_queued():
+    script = f"""
+const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});
+const values=new Map();const storage={{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v),removeItem:k=>values.delete(k)}};
+const draftKey='draft-key';const progressKey='progress-key';storage.setItem(draftKey,'old-draft');storage.setItem(progressKey,'old-progress');
+const outbox=createAuthoredOutbox({{storage,getOwnerLogin:()=>'timmy',fetchJson:async()=>({{id:1}})}});
+outbox.enqueue({{kind:'pull-review',repository:'o/r',number:7,expectedHeadSha:'abc',body:'Queued',draftKey,progressKey,draftFingerprint:'old-draft',progressFingerprint:'old-progress'}});
+storage.setItem(draftKey,'newer-draft');storage.setItem(progressKey,'newer-progress');
+outbox.flush('timmy').then(()=>process.stdout.write(JSON.stringify({{draft:storage.getItem(draftKey),progress:storage.getItem(progressKey)}})));
+"""
+    output = run_node(script)
+
+    assert output == {"draft": "newer-draft", "progress": "newer-progress"}
+
+
 def test_authored_outbox_mirrors_to_background_sync_and_reconciles_worker_results():
     script = f"""
 const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});

tests/test_background_issue_sync.py

@@ -203,6 +203,39 @@ const fetchJson = async (url, options = {{}}) => {{
     assert output["result"]["confirmed"] == [{"id": 42}]
 
 
+def test_closed_app_sync_delivers_pull_review_with_stable_identity_and_full_payload():
+    authored = {
+        "id": "review-op", "operationId": "review-op", "ownerLogin": "timmy", "status": "queued",
+        "kind": "pull-review", "repository": "stackchain/web", "number": 8,
+        "body": "Looks good", "decision": "approve", "expectedHeadSha": "abc123",
+        "comments": [{"path": "app.js", "body": "Nice", "new_position": 4}],
+    }
+    script = f"""
+const createBackgroundIssueSync = require({json.dumps(str(SYNC))});
+let queued = {json.dumps(authored)}; const calls=[];
+const store = {{
+  claimNext:async owner=>queued?.ownerLogin===owner?(queued=null,{json.dumps(authored)}):null,
+  complete:async()=>{{}},release:async()=>{{}},fail:async()=>{{}},countBlocked:async()=>0,
+}};
+const fetchJson=async(url,options={{}})=>{{calls.push({{url,options}});return url==='api/v1/background-identity'?{{login:'timmy'}}:{{id:45}};}};
+createBackgroundIssueSync({{store,fetchJson}}).flush().then(result=>process.stdout.write(JSON.stringify({{calls,result}})));
+"""
+    output = run_node(script)
+
+    mutation = output["calls"][1]
+    assert mutation["url"] == "api/v1/repos/stackchain/web/pulls/8/review"
+    assert mutation["options"]["headers"]["Idempotency-Key"] == "review-op"
+    assert json.loads(mutation["options"]["body"]) == {
+        "body": "Looks good", "decision": "approve", "expected_head_sha": "abc123",
+        "comments": [{"path": "app.js", "body": "Nice", "new_position": 4}],
+    }
+    assert output["result"]["confirmed"] == [{"id": 45}]
+    assert output["result"]["receipts"] == [{
+        "id": "review-op", "status": "confirmed", "kind": "message",
+        "route": "#/my-work/review/stackchain/web/8",
+    }]
+
+
 def test_reconciling_one_outbox_lane_preserves_the_other_lane():
     script = f"""
 const createBackgroundIssueSync = require({json.dumps(str(SYNC))});

tests/test_drafts.py

@@ -121,6 +121,27 @@ process.stdout.write(JSON.stringify(drafts));
     assert output[1]["label"] == "Queued issue"
 
 
+def test_draft_inbox_exposes_queued_review_with_review_route_and_actionable_state():
+    script = f"""
+const createDraftInbox = require({json.dumps(str(DRAFTS))});
+const values=new Map([['stackchain.authored-outbox.v1',JSON.stringify({{version:2,items:[{{
+  id:'review-1',kind:'pull-review',repository:'stackchain/web',number:8,body:'Looks good',
+  decision:'approve',expectedHeadSha:'abc',comments:[{{path:'app.js',body:'Nice',new_position:4}}],ownerLogin:'timmy',status:'queued',queuedAt:200
+}}]}})]]);
+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 item=createDraftInbox({{storage,getCurrentLogin:()=>'timmy'}}).list()[0];
+process.stdout.write(JSON.stringify(item));
+"""
+    output = run_node(script)
+
+    assert output["label"] == "Queued review"
+    assert output["outbox_kind"] == "pull-review"
+    assert output["title"] == "stackchain/web#8"
+    assert output["route"] == {"kind": "review", "repository": "stackchain/web", "number": 8}
+    assert output["copy_text"] == "Looks good\n\nInline feedback\napp.js · new line 4: Nice"
+    assert output["quarantined"] is False
+
+
 def test_draft_inbox_exposes_created_issue_waiting_for_create_and_start_continuation():
     script = f"""
 const createDraftInbox = require({json.dumps(str(DRAFTS))});

tests/test_my_work.py

@@ -4205,6 +4205,17 @@ async def test_review_requests_open_an_accessible_mobile_detail_sheet():
     assert '.review-diff' in html and 'overflow-x:auto' in html
 
 
+@pytest.mark.anyio
+async def test_review_attention_drafts_require_revision_instead_of_resending_stale_payload():
+    html = await dashboard()
+
+    assert "const reviewOutbox = item.outbox_kind === 'pull-review';" in html
+    assert "reviewOutbox && item.status === 'attention'" in html
+    assert ">Open current review</button>" in html
+    assert ">Copy feedback</button>" in html
+    assert "item.kind === 'authored-outbox' && !reviewOutbox" in html
+
+
 @pytest.mark.anyio
 async def test_review_sheet_loads_details_and_preserves_safe_gitea_handoff():
     html = await dashboard()

tests/test_offline_work.py

@@ -264,7 +264,7 @@ async def test_saved_unread_update_opens_offline_with_queued_reply_and_read_cont
 
 
 @pytest.mark.anyio
-async def test_saved_requested_review_opens_offline_for_sha_scoped_drafting_only():
+async def test_saved_requested_review_queues_complete_sha_scoped_feedback_offline():
     html = await dashboard()
 
     assert "item.is_review ? reviewController.load(item)" in html
@@ -272,6 +272,14 @@ async def test_saved_requested_review_opens_offline_for_sha_scoped_drafting_only
     assert "async function openReviewSheet(item, trigger, cachedDetail = null)" in html
     assert "cachedDetail || await reviewController.load(selectedReview)" in html
     assert "Offline review · saved " in html
-    assert "Reconnect to validate & submit" in html
-    assert "qs('#submit-review').disabled = offlineReview;" in html
+    assert "Queue review for reconnect" in html
+    assert "kind: 'pull-review'" in html
+    assert "await authoredOutbox.enqueueDurably" in html
+    assert "expectedHeadSha: selectedReviewHead" in html
+    assert "comments: snapshot.comments" in html
+    assert "draftKey: draft.storageKey" in html
+    assert "progressKey: progress?.storageKey" in html
+    assert "draftFingerprint: localStorage.getItem(draft.storageKey) || ''" in html
+    assert "progressFingerprint: localStorage.getItem(progress?.storageKey) || ''" in html
+    assert "Review queued · it will submit after reconnect." in html
     assert "if (selectedReview && offlineReview) openReviewSheet(selectedReview, reviewTrigger);" in html