feat: commit review feedback fixes from mobile (Closes #1376)

tests/e2e/test_mobile_apply_pull_feedback_fix_release.py

@@ -0,0 +1,85 @@
+from pathlib import Path
+
+import pytest
+
+
+ROOT = Path(__file__).parents[2]
+
+
+@pytest.mark.parametrize("viewport", [(320, 568), (390, 844)])
+def test_mobile_author_commits_one_review_feedback_fix(viewport):
+    playwright = pytest.importorskip("playwright.sync_api")
+    html = (ROOT / "frontend" / "index.html").read_text()
+    detail = {
+        "state": "open", "merged": False, "head_sha": "abc1234",
+        "capabilities": {"authored": True},
+        "reviewers": [{
+            "review_id": 42, "login": "sam", "status": "changes_requested",
+            "head_sha": "abc1234", "blocking": True, "feedback_state": "loaded",
+            "comments": [{"id": 99, "path": "src/api.py", "body": "Handle empty state.", "line": 12}],
+        }],
+        "files": [{"filename": "src/api.py", "status": "modified", "diff_available": True,
+                   "diff_lines": ["@@ -12 +12 @@", "+return empty"]}],
+    }
+
+    with playwright.sync_playwright() as runtime:
+        try:
+            browser = runtime.chromium.launch(headless=True)
+        except Exception as error:
+            pytest.skip(f"Chromium is not installed: {error}")
+        page = browser.new_page(viewport={"width": viewport[0], "height": viewport[1]})
+        page.set_content(html, wait_until="domcontentloaded")
+        page.add_style_tag(path=ROOT / "frontend" / "dashboard.css")
+        page.add_script_tag(path=ROOT / "frontend" / "pull-sheet.js")
+        page.evaluate(
+            """detail => {
+                globalThis.confirm = () => true;
+                globalThis.fixCalls = [];
+                globalThis.fixItem = {repository:'stackchain/api', number:7, key:'stackchain/api#7'};
+                globalThis.fixDetail = detail;
+                globalThis.fixController = createPullSheet({fetchJson: async (path, options = {}) => {
+                    fixCalls.push({path, options});
+                    if ((options.method || 'GET') === 'GET') return {
+                        path:'src/api.py', head_sha:'abc1234', blob_sha:'blob123', content:'return empty\\n'
+                    };
+                    return {path:'src/api.py', previous_head_sha:'abc1234', head_sha:'def5678', blob_sha:'blob456'};
+                }});
+                document.querySelector('#pull-sheet').classList.add('open');
+                document.querySelector('#pull-review').open = true;
+                document.querySelector('#pull-files').innerHTML = detail.files.map((file, index) =>
+                    createPullSheet.renderFile(file, index, false, value => String(value))
+                ).join('');
+                createPullSheet.bindFeedbackControls(document, fixController,
+                    () => fixItem, () => fixDetail, () => 'alex');
+                createPullSheet.review(detail, null, document);
+            }""",
+            detail,
+        )
+
+        page.locator(".pull-review-feedback summary").click()
+        page.locator('[data-address-review-feedback="42"]').click()
+        page.locator("#make-pull-feedback-fix").click()
+        page.locator("#pull-feedback-file-content").fill("return handled\n")
+        page.locator("#pull-feedback-commit-message").fill("fix: handle empty state")
+        page.locator("#commit-pull-feedback-fix").click()
+        page.wait_for_function("fixCalls.length === 2")
+        result = page.evaluate("""() => ({
+            calls:fixCalls, head:fixDetail.head_sha,
+            editorHidden:document.querySelector('#pull-feedback-file-editor').hidden,
+            status:document.querySelector('#pull-feedback-status').textContent,
+            scrollWidth:document.documentElement.scrollWidth,
+            clientWidth:document.documentElement.clientWidth,
+            heights:Array.from(document.querySelectorAll('#pull-feedback-pass button'))
+                .filter(button => button.getClientRects().length > 0).map(button => button.getBoundingClientRect().height),
+        })""")
+        browser.close()
+
+    assert result["scrollWidth"] <= result["clientWidth"]
+    assert min(result["heights"]) >= 44
+    assert result["head"] == "def5678"
+    assert result["editorHidden"] is True
+    assert "committed" in result["status"].lower()
+    assert "path=src%2Fapi.py" in result["calls"][0]["path"]
+    body = result["calls"][1]["options"]["body"]
+    assert '"expected_blob_sha":"blob123"' in body
+    assert '"content":"return handled\\n"' in body

tests/test_ci_workflow.py

@@ -48,7 +48,7 @@ def test_release_promotion_waits_for_tests_and_bundle():
     assert release.index("sha256sum -c") < release.index("curl --fail-with-body")
 
 
-def test_release_promotion_waits_for_packaged_mobile_journeys():
+def test_release_promotion_waits_for_every_packaged_mobile_journey():
     text = WORKFLOW.read_text()
     browser = text[text.index("  browser-journey:") : text.index("  release-candidate:")]
     release = text[text.index("  release-candidate:") :]
@@ -58,66 +58,6 @@ def test_release_promotion_waits_for_packaged_mobile_journeys():
     assert "pip install -r requirements-e2e.txt" in browser
     assert "python3 -m playwright install --with-deps chromium" in browser
     assert 'STACKCHAIN_RUN_RELEASE_E2E: "1"' in browser
-    assert (
-        "python3 -m pytest tests/e2e/test_mobile_offline_issue_release.py "
-        "tests/e2e/test_mobile_search_preview_navigation.py "
-        "tests/e2e/test_mobile_search_week_plan.py "
-        "tests/e2e/test_mobile_find_work_release.py "
-        "tests/e2e/test_mobile_home_bootstrap_release.py "
-        "tests/e2e/test_mobile_sign_out_release.py "
-        "tests/e2e/test_mobile_today_handoff_release.py "
-        "tests/e2e/test_mobile_today_wrap_up_release.py "
-        "tests/e2e/test_mobile_today_summary_release.py "
-        "tests/e2e/test_mobile_tomorrow_conflict_release.py "
-        "tests/e2e/test_mobile_week_ahead_release.py "
-        "tests/e2e/test_mobile_today_week_reschedule_release.py "
-        "tests/e2e/test_mobile_wrap_up_handoff_release.py "
-        "tests/e2e/test_mobile_following_release.py "
-        "tests/e2e/test_mobile_detail_watch_release.py "
-        "tests/e2e/test_mobile_pull_reviewer_status_release.py "
-        "tests/e2e/test_mobile_pull_reviewer_feedback_release.py -q"
-    ) in browser
+    assert "python3 -m pytest tests/e2e -q" in browser
+    assert "test_mobile_" not in browser
     assert "needs: [lint, build-release, browser-journey]" in release
-
-
-def test_browser_job_runs_packaged_today_week_reschedule_journey():
-    text = WORKFLOW.read_text()
-    browser = text[text.index("  browser-journey:") : text.index("  release-candidate:")]
-
-    assert "tests/e2e/test_mobile_today_week_reschedule_release.py" in browser
-
-
-def test_browser_job_gates_latest_pull_review_mobile_journeys():
-    text = WORKFLOW.read_text()
-    browser = text[text.index("  browser-journey:") : text.index("  release-candidate:")]
-
-    assert "tests/e2e/test_mobile_address_review_feedback_release.py" in browser
-    assert "tests/e2e/test_mobile_cancel_pull_review_request_release.py" in browser
-
-
-def test_browser_job_gates_authored_pull_queue_journey():
-    text = WORKFLOW.read_text()
-    browser = text[text.index("  browser-journey:") : text.index("  release-candidate:")]
-
-    assert "tests/e2e/test_mobile_authored_pull_queue_release.py" in browser
-
-
-def test_browser_job_gates_mobile_source_branch_cleanup_journey():
-    text = WORKFLOW.read_text()
-    browser = text[text.index("  browser-journey:") : text.index("  release-candidate:")]
-
-    assert "tests/e2e/test_mobile_source_branch_cleanup_release.py" in browser
-
-
-def test_browser_job_gates_authored_pull_close_recovery_journey():
-    text = WORKFLOW.read_text()
-    browser = text[text.index("  browser-journey:") : text.index("  release-candidate:")]
-
-    assert "tests/e2e/test_mobile_close_authored_pull_release.py" in browser
-
-
-def test_browser_job_gates_search_authored_pull_recovery_journey():
-    text = WORKFLOW.read_text()
-    browser = text[text.index("  browser-journey:") : text.index("  release-candidate:")]
-
-    assert "tests/e2e/test_mobile_search_authored_pull_recovery_release.py" in browser

tests/test_detail_watch.py

@@ -130,7 +130,8 @@ def test_detail_watch_is_wired_into_both_mobile_work_details_and_bundle():
     assert 'pullDetailWatch.open(item)' in dashboard
     assert '#watch-issue-detail' in css
     assert '#watch-pull-detail' in css
-    assert 'tests/e2e/test_mobile_detail_watch_release.py' in workflow
+    assert "python3 -m pytest tests/e2e -q" in workflow
+    assert (ROOT / "tests/e2e/test_mobile_detail_watch_release.py").is_file()
 
 
 def test_detail_watch_ignores_a_late_subscription_response_for_the_previous_item():

tests/test_pull_api.py

@@ -18,6 +18,135 @@ def _content_payload(text: str, sha: str) -> dict:
     }
 
 
+@pytest.mark.anyio
+async def test_gitea_loads_bounded_text_file_from_authored_pull_head():
+    pull = {
+        "number": 7, "state": "open", "merged": False,
+        "user": {"login": "alex"},
+        "head": {
+            "sha": "abc1234", "ref": "alex/review-fix",
+            "repo": {"full_name": "stackchain/api"},
+        },
+        "base": {"repo": {"full_name": "stackchain/api"}},
+    }
+
+    async def handler(request):
+        if request.url.path == "/api/v1/user":
+            return httpx.Response(200, json={"login": "alex"})
+        if request.url.path == "/api/v1/repos/stackchain/api/pulls/7":
+            return httpx.Response(200, json=pull)
+        if request.url.path == "/api/v1/repos/stackchain/api/contents/src/api.py":
+            assert request.url.params["ref"] == "abc1234"
+            return httpx.Response(200, json=_content_payload("return empty\n", "blob123"))
+        raise AssertionError(f"unexpected request: {request.method} {request.url}")
+
+    gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+    try:
+        result = await gitea_proxy.authored_pull_feedback_file(
+            "stackchain/api", 7, "src/api.py", "abc1234"
+        )
+    finally:
+        await gitea_proxy.stop_client()
+
+    assert result == {
+        "repository": "stackchain/api", "number": 7, "path": "src/api.py",
+        "head_sha": "abc1234", "blob_sha": "blob123", "content": "return empty\n",
+    }
+
+
+@pytest.mark.anyio
+async def test_gitea_commits_feedback_fix_and_verifies_advanced_pull_head():
+    requests = []
+    pull = {
+        "number": 7, "state": "open", "merged": False,
+        "user": {"login": "alex"},
+        "head": {
+            "sha": "abc1234", "ref": "alex/review-fix",
+            "repo": {"full_name": "stackchain/api"},
+        },
+        "base": {"repo": {"full_name": "stackchain/api"}},
+    }
+
+    async def handler(request):
+        requests.append((request.method, request.url.path))
+        if request.url.path == "/api/v1/user":
+            return httpx.Response(200, json={"login": "alex"})
+        if request.url.path == "/api/v1/repos/stackchain/api/pulls/7":
+            return httpx.Response(200, json=pull)
+        if request.url.path == "/api/v1/repos/stackchain/api/contents/src/api.py" and request.method == "GET":
+            ref = request.url.params["ref"]
+            return httpx.Response(200, json=_content_payload(
+                "return handled\n" if ref == "def5678" else "return empty\n",
+                "blob456" if ref == "def5678" else "blob123",
+            ))
+        if request.url.path == "/api/v1/repos/stackchain/api/contents/src/api.py" and request.method == "PUT":
+            body = json.loads(request.content)
+            assert body == {
+                "branch": "alex/review-fix", "sha": "blob123",
+                "message": "fix: handle empty state",
+                "content": base64.b64encode(b"return handled\n").decode(),
+            }
+            pull["head"]["sha"] = "def5678"
+            return httpx.Response(200, json={"commit": {"sha": "def5678"}})
+        raise AssertionError(f"unexpected request: {request.method} {request.url}")
+
+    gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+    try:
+        result = await gitea_proxy.commit_authored_pull_feedback_fix(
+            "stackchain/api", 7, "src/api.py", "return handled\n",
+            "fix: handle empty state", "abc1234", "blob123",
+        )
+    finally:
+        await gitea_proxy.stop_client()
+
+    assert result["previous_head_sha"] == "abc1234"
+    assert result["head_sha"] == "def5678"
+    assert result["path"] == "src/api.py"
+    assert requests[-2:] == [
+        ("GET", "/api/v1/repos/stackchain/api/pulls/7"),
+        ("GET", "/api/v1/repos/stackchain/api/contents/src/api.py"),
+    ]
+
+
+@pytest.mark.anyio
+async def test_feedback_file_api_loads_and_commits_an_authored_pull_fix(monkeypatch):
+    calls = []
+
+    async def load(repository, number, path, head):
+        calls.append(("load", repository, number, path, head))
+        return {"path": path, "head_sha": head, "blob_sha": "blob123", "content": "before\n"}
+
+    async def commit(repository, number, path, content, message, head, blob):
+        calls.append(("commit", repository, number, path, content, message, head, blob))
+        return {"path": path, "previous_head_sha": head, "head_sha": "def5678", "blob_sha": "blob456"}
+
+    monkeypatch.setattr(main.gitea_proxy, "authored_pull_feedback_file", load, raising=False)
+    monkeypatch.setattr(main.gitea_proxy, "commit_authored_pull_feedback_fix", commit, raising=False)
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        loaded = await client.get(
+            "/api/v1/repos/stackchain/api/pulls/7/feedback-file",
+            params={"path": "src/api.py", "expected_head_sha": "abc1234"},
+        )
+        committed = await client.patch(
+            "/api/v1/repos/stackchain/api/pulls/7/feedback-file",
+            json={
+                "path": "src/api.py", "content": "after\n",
+                "message": "fix: address review", "expected_head_sha": "abc1234",
+                "expected_blob_sha": "blob123",
+            },
+        )
+
+    assert loaded.status_code == 200
+    assert loaded.json()["blob_sha"] == "blob123"
+    assert committed.status_code == 200
+    assert committed.json()["head_sha"] == "def5678"
+    assert calls == [
+        ("load", "stackchain/api", 7, "src/api.py", "abc1234"),
+        ("commit", "stackchain/api", 7, "src/api.py", "after\n", "fix: address review", "abc1234", "blob123"),
+    ]
+
+
 @pytest.mark.anyio
 async def test_gitea_updates_authored_pull_branch_and_confirms_new_head():
     requests = []

tests/test_today_summary.py

@@ -248,9 +248,9 @@ def test_dashboard_connects_recap_and_wrap_up_to_a_mobile_summary_review_sheet()
     assert ".today-summary-actions button { min-height:44px;" in css
     assert ".today-summary-panel" in css and "overflow-x:hidden" in css
     assert ".today-summary-destination" in css and "overflow-wrap:anywhere" in css
-    assert "tests/e2e/test_mobile_today_summary_release.py" in (
-        ROOT / ".gitea" / "workflows" / "ci.yml"
-    ).read_text()
+    workflow = (ROOT / ".gitea" / "workflows" / "ci.yml").read_text()
+    assert "python3 -m pytest tests/e2e -q" in workflow
+    assert (ROOT / "tests/e2e/test_mobile_today_summary_release.py").is_file()
 
 
 def test_summary_uses_the_human_label_from_recap_feedback_rows():

tests/test_today_wrap_up.py

@@ -157,7 +157,9 @@ def test_dashboard_packages_a_mobile_wrap_up_dialog_and_opens_it_after_recap_sav
     assert 'openWrapUp(handoff.actual_minutes, workedItems)' in recap
     assert '.today-wrap-up-actions button { min-height:44px;' in css
     assert '.today-wrap-up-panel' in css and 'overflow-x:hidden' in css
-    assert 'tests/e2e/test_mobile_today_wrap_up_release.py' in (ROOT / '.gitea' / 'workflows' / 'ci.yml').read_text()
+    workflow = (ROOT / '.gitea' / 'workflows' / 'ci.yml').read_text()
+    assert 'python3 -m pytest tests/e2e -q' in workflow
+    assert (ROOT / 'tests/e2e/test_mobile_today_wrap_up_release.py').is_file()
 
 
 def test_wrap_up_repeat_confirmation_does_not_repeat_tomorrow_or_today_operations():