feat: review assigned pull diffs before merge (#187)

tests/test_my_work.py

@@ -1220,6 +1220,65 @@ process.stdout.write(JSON.stringify(states.map(createPullSheet.mergeEligibility)
     assert output[3]["allowed"] is False and "conflict" in output[3]["reason"].lower()
 
 
+def test_pull_sheet_persists_head_scoped_file_review_and_gates_merge():
+    script = f"""
+const createPullSheet = require({json.dumps(str(PULL_SHEET))});
+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 item = {{repository:'stackchain/api', number:7}};
+const detail = {{state:'open', draft:false, mergeable:true, merged:false, ci_state:'success', head_sha:'abc', files:[
+  {{filename:'src/api.py'}}, {{filename:'frontend/app.js'}}
+]}};
+const first = createPullSheet({{storage, fetchJson:()=>Promise.resolve()}});
+const before = first.reviewState(item, detail);
+const afterOne = first.toggleReviewed(item, detail, 'src/api.py');
+const restored = createPullSheet({{storage, fetchJson:()=>Promise.resolve()}}).reviewState(item, detail);
+const complete = first.toggleReviewed(item, detail, 'frontend/app.js');
+const changedHead = first.reviewState(item, {{...detail, head_sha:'def'}});
+process.stdout.write(JSON.stringify({{
+  before, afterOne, restored, complete, changedHead,
+  blocked:createPullSheet.mergeEligibility(detail, afterOne),
+  allowed:createPullSheet.mergeEligibility(detail, complete),
+  next:first.nextUnreviewed(detail, afterOne),
+}}));
+"""
+    result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
+    output = json.loads(result.stdout)
+
+    assert output["before"] == {"reviewed": [], "total": 2, "complete": False}
+    assert output["afterOne"]["reviewed"] == ["src/api.py"]
+    assert output["restored"] == output["afterOne"]
+    assert output["complete"]["complete"] is True
+    assert output["changedHead"]["reviewed"] == []
+    assert output["blocked"] == {"allowed": False, "reason": "Review every changed file before merging"}
+    assert output["allowed"] == {"allowed": True, "reason": "Ready to merge"}
+    assert output["next"] == "frontend/app.js"
+
+
+def test_pull_sheet_renders_mobile_diff_fallbacks_and_review_controls():
+    script = f"""
+const createPullSheet = require({json.dumps(str(PULL_SHEET))});
+const escapeHtml = value => String(value).replaceAll('&', '&amp;').replaceAll('<', '&lt;');
+const text = createPullSheet.renderFile({{
+  filename:'src/<unsafe>.py', status:'modified', additions:1, deletions:1,
+  diff_available:true, diff_lines:['@@ -1 +1 @@', '-old', '+new'], diff_truncated:true
+}}, 0, false, escapeHtml);
+const binary = createPullSheet.renderFile({{
+  filename:'static/logo.png', diff_available:false, diff_binary:true, diff_truncated:false
+}}, 1, true, escapeHtml);
+process.stdout.write(JSON.stringify({{text, binary}}));
+"""
+    result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
+    output = json.loads(result.stdout)
+
+    assert 'aria-controls="pull-diff-0"' in output["text"]
+    assert "&lt;unsafe>" in output["text"] and "+new" in output["text"]
+    assert "Preview truncated" in output["text"]
+    assert 'data-pull-review-file="src/&lt;unsafe>.py"' in output["text"]
+    assert "Binary file · preview unavailable" in output["binary"]
+    assert 'aria-pressed="true"' in output["binary"] and "Reviewed" in output["binary"]
+
+
 @pytest.mark.anyio
 async def test_assigned_pulls_open_accessible_mobile_completion_sheet():
     html = await dashboard()
@@ -1228,10 +1287,17 @@ async def test_assigned_pulls_open_accessible_mobile_completion_sheet():
     assert 'class="my-work-card-main pull-trigger"' in html
     assert 'id="pull-sheet-status"' in html
     assert 'id="pull-files"' in html and 'id="pull-comments"' in html
+    assert 'id="pull-review-progress"' in html and 'aria-live="polite"' in html
+    assert 'id="next-unreviewed-pull-file"' in html
     assert 'id="pull-comment"' in html and 'maxlength="10000"' in html
     assert 'id="merge-pull"' in html and 'id="open-pull-gitea"' in html
     assert '<script src="static/pull-sheet.js"></script>' in html
     assert "pullController.load(item)" in html
+    assert "createPullSheet.renderFile" in html
+    assert "pullController.toggleReviewed" in html
+    assert "pullController.nextUnreviewed" in html
+    assert ".pull-diff { overflow-x:auto;" in html
+    assert ".pull-file-toggle, .pull-review-file { min-height:44px;" in html
     assert "window.confirm('Merge ' + selectedPull.key" in html
     assert "expected_head_sha" in html
     assert "item.kind === 'pull'" in html and "pull-trigger" in html

tests/test_pull_api.py

@@ -53,6 +53,49 @@ async def test_assigned_pull_detail_rejects_unassigned_pull(monkeypatch):
     assert response.status_code == 404
 
 
+@pytest.mark.anyio
+async def test_gitea_assigned_pull_detail_includes_bounded_diff_previews():
+    async def handler(request):
+        path = request.url.path
+        if path.endswith("/pulls/7"):
+            return httpx.Response(200, json={
+                "number": 7,
+                "title": "Review this patch",
+                "state": "open",
+                "mergeable": True,
+                "head": {"sha": "abc123"},
+            })
+        if path.endswith("/pulls/7/files"):
+            return httpx.Response(200, json=[
+                {"filename": "src/api.py", "status": "modified", "additions": 1, "deletions": 1},
+                {"filename": "static/logo.png", "status": "modified"},
+            ])
+        if path.endswith("/commits/abc123/status"):
+            return httpx.Response(200, json={"state": "success"})
+        if path.endswith("/issues/7/comments"):
+            return httpx.Response(200, json=[])
+        if path.endswith("/pulls/7.diff"):
+            return httpx.Response(200, text=(
+                "diff --git a/src/api.py b/src/api.py\n"
+                "--- a/src/api.py\n+++ b/src/api.py\n"
+                "@@ -1 +1 @@\n-old\n+new\n"
+                "diff --git a/static/logo.png b/static/logo.png\n"
+                "Binary files a/static/logo.png and b/static/logo.png differ\n"
+            ))
+        raise AssertionError(f"unexpected request: {request.method} {path}")
+
+    gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+    try:
+        detail = await gitea_proxy.pull_completion_detail("stackchain/api", 7)
+    finally:
+        await gitea_proxy.stop_client()
+
+    assert detail["files"][0]["diff_available"] is True
+    assert "+new" in detail["files"][0]["diff_lines"]
+    assert detail["files"][1]["diff_binary"] is True
+    assert detail["files"][1]["diff_available"] is False
+
+
 @pytest.mark.anyio
 async def test_assigned_pull_comment_posts_only_after_assignment_check(monkeypatch):
     calls = []