diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 6108d85..3acf077 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -57,7 +57,7 @@ jobs: pip install -r requirements-e2e.txt python3 -m playwright install --with-deps chromium - name: Exercise packaged mobile work journeys - run: 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 tests/e2e/test_mobile_address_review_feedback_release.py tests/e2e/test_mobile_cancel_pull_review_request_release.py tests/e2e/test_mobile_authored_pull_queue_release.py tests/e2e/test_mobile_close_authored_pull_release.py tests/e2e/test_mobile_search_authored_pull_recovery_release.py tests/e2e/test_mobile_source_branch_cleanup_release.py tests/e2e/test_mobile_release_failure_recovery.py + run: python3 -m pytest tests/e2e -q release-candidate: runs-on: ubuntu-latest diff --git a/frontend/dashboard.css b/frontend/dashboard.css index d398c7c..e1ae6a9 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -1310,6 +1310,14 @@ textarea { resize: vertical; min-height: 120px; } .pull-feedback-pass-heading h4 { margin:2px 0 8px; } .pull-feedback-pass p { white-space:pre-wrap; } .pull-feedback-pass textarea { width:100%; min-height:88px; box-sizing:border-box; } +#make-pull-feedback-fix { width:100%; margin-top:8px; border-color:#4ade80; } +.pull-feedback-file-editor { display:grid; min-width:0; gap:8px; margin-top:10px; padding:10px; border:1px solid #4ade80; border-radius:10px; overflow-x:hidden; } +.pull-feedback-file-editor[hidden] { display:none; } +.pull-feedback-file-editor h5 { margin:0; overflow-wrap:anywhere; } +#pull-feedback-file-content { min-height:36dvh; resize:vertical; font:13px/1.45 ui-monospace, SFMono-Regular, Consolas, monospace; tab-size:2; white-space:pre; overflow:auto; } +#pull-feedback-commit-message { box-sizing:border-box; width:100%; min-width:0; } +.pull-feedback-file-actions { display:grid; grid-template-columns:1fr 1fr; gap:8px; min-width:0; } +@media (max-width:359px) { .pull-feedback-file-actions { grid-template-columns:1fr; } } .pull-feedback-dispositions { display:grid; grid-template-columns:1fr; gap:6px; margin:10px 0; padding:8px; } .pull-feedback-dispositions button[aria-pressed="true"] { border-color:#60a5fa; background:#173b63; } .pull-feedback-navigation { display:grid; grid-template-columns:1fr 1fr; gap:8px; margin-top:8px; } diff --git a/frontend/index.html b/frontend/index.html index 1c6abab..201e5ff 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -1741,6 +1741,18 @@

+ +
Disposition diff --git a/frontend/pull-sheet.js b/frontend/pull-sheet.js index 78c3042..7bf104b 100644 --- a/frontend/pull-sheet.js +++ b/frontend/pull-sheet.js @@ -432,6 +432,8 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin) qs('#finish-pull-feedback').disabled = active.state.posted === true || items.some(item => !active.state.responses[String(item.id)]?.disposition); qs('#request-feedback-review').hidden = active.state.posted !== true; + qs('#make-pull-feedback-fix').hidden = active.detail?.capabilities?.authored !== true || + active.detail?.state !== 'open' || active.detail?.merged === true || !comment.path; const found = focusPullFile(doc, comment.path); qs('#pull-feedback-context').textContent = found ? 'Matching change opened below.' : 'Matching change is unavailable in this preview.'; @@ -475,10 +477,77 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin) active = { item, detail, reviewer, login, state:controller.loadFeedbackPass(item, detail, reviewer, login) }; panel.hidden = false; + qs('#pull-feedback-file-editor').hidden = true; + qs('#cancel-pull-feedback-fix').hidden = true; + qs('#commit-pull-feedback-fix').hidden = true; + active.file = null; qs('#pull-feedback-status').textContent = active.state.posted ? 'Response already posted. You can request an updated review.' : 'Progress saves on this device.'; render(); }); + qs('#make-pull-feedback-fix').addEventListener('click', async () => { + const comment = current(); + if (!active || !comment?.path || active.detail?.capabilities?.authored !== true) return; + const button = qs('#make-pull-feedback-fix'); + button.disabled = true; + qs('#pull-feedback-status').textContent = 'Loading the current pull file…'; + try { + const file = await controller.loadFeedbackFile(active.item, comment.path, active.detail.head_sha); + if (!active || file.head_sha !== active.detail.head_sha || file.path !== comment.path) return; + active.file = file; + qs('#pull-feedback-file-path').textContent = file.path; + qs('#pull-feedback-file-content').value = file.content; + qs('#pull-feedback-commit-message').value = 'fix: address review feedback'; + qs('#pull-feedback-file-editor').hidden = false; + qs('#cancel-pull-feedback-fix').hidden = false; + qs('#commit-pull-feedback-fix').hidden = false; + qs('#pull-feedback-status').textContent = 'Edit the bounded file, then commit it to this pull branch.'; + qs('#pull-feedback-file-content').focus(); + } catch (error) { + qs('#pull-feedback-status').textContent = error.message + ' Retry without losing your feedback progress.'; + } finally { + button.disabled = false; + } + }); + qs('#cancel-pull-feedback-fix').addEventListener('click', () => { + qs('#pull-feedback-file-editor').hidden = true; + qs('#cancel-pull-feedback-fix').hidden = true; + qs('#commit-pull-feedback-fix').hidden = true; + active.file = null; + qs('#make-pull-feedback-fix').focus(); + }); + qs('#commit-pull-feedback-fix').addEventListener('click', async () => { + if (!active?.file) return; + const content = qs('#pull-feedback-file-content').value; + const message = qs('#pull-feedback-commit-message').value.trim(); + if (!message || content === active.file.content) { + qs('#pull-feedback-status').textContent = 'Change the file and enter a commit message before committing.'; + return; + } + if (!globalThis.confirm('Commit this change to ' + active.item.key + '?')) return; + const button = qs('#commit-pull-feedback-fix'); + button.disabled = true; + qs('#pull-feedback-status').textContent = 'Committing and verifying the new pull head…'; + try { + const result = await controller.commitFeedbackFix(active.item, { + path:active.file.path, content, message, + expectedHeadSha:active.file.head_sha, expectedBlobSha:active.file.blob_sha, + }); + active.detail.head_sha = result.head_sha; + active.state.headSha = result.head_sha; + active.file = null; + save(); + qs('#pull-feedback-file-editor').hidden = true; + qs('#cancel-pull-feedback-fix').hidden = true; + qs('#commit-pull-feedback-fix').hidden = true; + qs('#pull-feedback-status').textContent = 'Code change committed. Mark the comment Addressed when satisfied.'; + render(); + } catch (error) { + qs('#pull-feedback-status').textContent = error.message + ' Draft kept; reload if the branch changed, or retry.'; + button.disabled = false; + button.focus(); + } + }); panel.querySelector('.pull-feedback-dispositions').addEventListener('click', event => { const disposition = event.target.dataset?.feedbackDisposition; const comment = current(); @@ -701,6 +770,7 @@ function createPullSheet({ fetchJson, storage, onState, createConversationPager let lifecycleMutation = null; let editRequest = null; let feedbackRequest = null; + let feedbackFixRequest = null; const reviewRequests = new Map(); const reviewCache = new Map(); const feedbackRequests = new Map(); @@ -764,6 +834,30 @@ function createPullSheet({ fetchJson, storage, onState, createConversationPager feedbackRequests.set(key, request); return request; }, + loadFeedbackFile(item, path, expectedHeadSha) { + return fetchJson(pathFor(item) + '/feedback-file?path=' + encodeURIComponent(path) + + '&expected_head_sha=' + encodeURIComponent(expectedHeadSha), { + headers:{ Accept:'application/json' }, + }); + }, + commitFeedbackFix(item, draft) { + if (feedbackFixRequest) return feedbackFixRequest; + feedbackFixRequest = fetchJson(pathFor(item) + '/feedback-file', { + method:'PATCH', + headers:{ Accept:'application/json', 'Content-Type':'application/json' }, + body:JSON.stringify({ + path:draft.path, content:draft.content, message:draft.message, + expected_head_sha:draft.expectedHeadSha, + expected_blob_sha:draft.expectedBlobSha, + }), + }).then(result => { + if (!result?.head_sha || result.head_sha === draft.expectedHeadSha || result.path !== draft.path) { + throw new Error('The feedback fix was not confirmed.'); + } + return result; + }).finally(() => { feedbackFixRequest = null; }); + return feedbackFixRequest; + }, loadChecks(item) { if (checkRequest) return checkRequest; checkRequest = fetchJson(pathFor(item) + '/checks', { diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 872de6b..d14f300 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -2221,6 +2221,124 @@ async def reopen_authored_pull( ) +async def authored_pull_feedback_file( + repository: str, number: int, path: str, expected_head_sha: str +) -> dict: + """Load one bounded UTF-8 file from an authored same-repository pull head.""" + if ( + not path or path.startswith("/") or "\\" in path + or any(part in {"", ".", ".."} for part in path.split("/")) + ): + raise IssueNotAvailableError("File path is not eligible") + login, pull = await _current_login_and_target( + f"repos/{repository}/pulls/{number}" + ) + author = pull.get("user") if isinstance(pull.get("user"), dict) else {} + head = pull.get("head") if isinstance(pull.get("head"), dict) else {} + base = pull.get("base") if isinstance(pull.get("base"), dict) else {} + head_repo = head.get("repo") if isinstance(head.get("repo"), dict) else {} + base_repo = base.get("repo") if isinstance(base.get("repo"), dict) else {} + if ( + pull.get("state") != "open" or pull.get("merged") is True + or author.get("login", "").casefold() != login.casefold() + or head.get("sha") != expected_head_sha + or head_repo.get("full_name") != repository + or base_repo.get("full_name") != repository + ): + raise IssueNotAvailableError("Pull request state changed") + response = await _get_client().get( + f"/api/v1/repos/{repository}/contents/{quote(path, safe='/')}", + headers=_auth(), params={"ref": expected_head_sha}, + ) + response.raise_for_status() + payload = response.json() + if not isinstance(payload, dict): + raise IssueNotAvailableError("File is not editable text") + encoded = payload.get("content") + if ( + payload.get("type") != "file" or payload.get("encoding") != "base64" + or not isinstance(encoded, str) or not isinstance(payload.get("sha"), str) + ): + raise IssueNotAvailableError("File is not editable text") + try: + raw = base64.b64decode(encoded, validate=True) + content = raw.decode("utf-8") + except (ValueError, UnicodeDecodeError): + raise IssueNotAvailableError("File is not editable text") from None + if len(raw) > 128 * 1024 or "\x00" in content: + raise IssueNotAvailableError("File is not editable text") + return { + "repository": repository, "number": number, "path": path, + "head_sha": expected_head_sha, "blob_sha": payload["sha"], + "content": content, + } + + +async def commit_authored_pull_feedback_fix( + repository: str, + number: int, + path: str, + content: str, + message: str, + expected_head_sha: str, + expected_blob_sha: str, +) -> dict: + """Commit and verify one race-guarded text-file fix on an authored pull.""" + encoded = content.encode("utf-8") + message = message.strip() + if len(encoded) > 128 * 1024 or "\x00" in content or not message or len(message) > 120: + raise IssueNotAvailableError("Feedback fix is outside the editable bounds") + login, pull = await _current_login_and_target( + f"repos/{repository}/pulls/{number}" + ) + author = pull.get("user") if isinstance(pull.get("user"), dict) else {} + head = pull.get("head") if isinstance(pull.get("head"), dict) else {} + base = pull.get("base") if isinstance(pull.get("base"), dict) else {} + head_repo = head.get("repo") if isinstance(head.get("repo"), dict) else {} + base_repo = base.get("repo") if isinstance(base.get("repo"), dict) else {} + branch = head.get("ref") + if ( + pull.get("state") != "open" or pull.get("merged") is True + or author.get("login", "").casefold() != login.casefold() + or head.get("sha") != expected_head_sha + or head_repo.get("full_name") != repository + or base_repo.get("full_name") != repository + or not isinstance(branch, str) or not branch + ): + raise IssueNotAvailableError("Pull request state changed") + current = await authored_pull_feedback_file( + repository, number, path, expected_head_sha + ) + if current["blob_sha"] != expected_blob_sha or current["content"] == content: + raise IssueNotAvailableError("File changed or has no new content") + response = await _get_client().put( + f"/api/v1/repos/{repository}/contents/{quote(path, safe='/')}", + headers=_auth(), + json={ + "branch": branch, "sha": expected_blob_sha, "message": message, + "content": base64.b64encode(encoded).decode(), + }, + ) + if response.status_code in {409, 422}: + raise IssueNotAvailableError("Pull request or file changed") + response.raise_for_status() + confirmed_pull = await fetch(f"repos/{repository}/pulls/{number}") + confirmed_head = confirmed_pull.get("head") if isinstance(confirmed_pull, dict) and isinstance(confirmed_pull.get("head"), dict) else {} + new_head_sha = confirmed_head.get("sha") + if not isinstance(new_head_sha, str) or not new_head_sha or new_head_sha == expected_head_sha: + raise ValueError("Gitea did not confirm a new pull request head") + confirmed = await authored_pull_feedback_file( + repository, number, path, new_head_sha + ) + if confirmed["content"] != content: + raise ValueError("Gitea did not confirm the committed file content") + return { + "repository": repository, "number": number, "path": path, + "previous_head_sha": expected_head_sha, "head_sha": new_head_sha, + "blob_sha": confirmed["blob_sha"], "message": message, + } + + async def update_authored_pull_branch( repository: str, number: int, expected_head_sha: str ) -> dict: diff --git a/src/main.py b/src/main.py index 7afcc59..0a8f300 100644 --- a/src/main.py +++ b/src/main.py @@ -1152,6 +1152,16 @@ class PullReadyRequest(BaseModel): ) +class PullFeedbackFileUpdate(BaseModel): + path: str = Field(min_length=1, max_length=1024) + content: str = Field(max_length=131_072) + message: str = Field(min_length=1, max_length=120) + expected_head_sha: str = Field( + min_length=7, max_length=64, pattern=r"^[A-Fa-f0-9]+$" + ) + expected_blob_sha: str = Field(min_length=1, max_length=128) + + class PullContentUpdate(BaseModel): title: str = Field(min_length=1, max_length=255) body: str = Field(default="", max_length=10_000) @@ -6954,6 +6964,59 @@ async def update_authored_pull_branch( return JSONResponse(result) +@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/feedback-file") +async def authored_pull_feedback_file( + owner: str, + repo: str, + number: int = PathParam(gt=0), + path: str = Query(min_length=1, max_length=1024), + expected_head_sha: str = Query(min_length=7, max_length=64, pattern=r"^[A-Fa-f0-9]+$"), +): + try: + result = await asyncio.wait_for( + gitea_proxy.authored_pull_feedback_file( + f"{owner}/{repo}", number, path, expected_head_sha + ), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.IssueNotAvailableError: + return JSONResponse({"error": "The pull request or file changed. Reload before editing."}, status_code=409) + except Exception: + return JSONResponse( + {"error": "The review file could not be loaded. Please retry."}, + status_code=503, headers={"Retry-After": "1"}, + ) + return JSONResponse(result) + + +@app.patch("/api/v1/repos/{owner}/{repo}/pulls/{number}/feedback-file") +async def commit_authored_pull_feedback_fix( + update: PullFeedbackFileUpdate, + owner: str, + repo: str, + number: int = PathParam(gt=0), +): + try: + result = await asyncio.wait_for( + gitea_proxy.commit_authored_pull_feedback_fix( + f"{owner}/{repo}", number, update.path, update.content, + update.message, update.expected_head_sha, update.expected_blob_sha, + ), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.IssueNotAvailableError: + return JSONResponse( + {"error": "The pull request or file changed. Your draft was not committed."}, + status_code=409, + ) + except Exception: + return JSONResponse( + {"error": "The feedback fix could not be confirmed. Keep the draft and retry."}, + status_code=503, headers={"Retry-After": "1"}, + ) + return JSONResponse(result) + + @app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review-data") async def assigned_pull_review_data( owner: str, repo: str, number: int = PathParam(gt=0) diff --git a/tests/e2e/test_mobile_apply_pull_feedback_fix_release.py b/tests/e2e/test_mobile_apply_pull_feedback_fix_release.py new file mode 100644 index 0000000..393547e --- /dev/null +++ b/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 diff --git a/tests/test_ci_workflow.py b/tests/test_ci_workflow.py index 833aabe..09e286c 100644 --- a/tests/test_ci_workflow.py +++ b/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 diff --git a/tests/test_detail_watch.py b/tests/test_detail_watch.py index 64e1f59..422504b 100644 --- a/tests/test_detail_watch.py +++ b/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(): diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py index 6a0b8fb..c4b809d 100644 --- a/tests/test_pull_api.py +++ b/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 = [] diff --git a/tests/test_today_summary.py b/tests/test_today_summary.py index f0106ea..ab82682 100644 --- a/tests/test_today_summary.py +++ b/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(): diff --git a/tests/test_today_wrap_up.py b/tests/test_today_wrap_up.py index 6cf7fc4..fd6a330 100644 --- a/tests/test_today_wrap_up.py +++ b/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():