diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 206ca17..e1caffc 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -1287,7 +1287,7 @@ if (continuing) return await completeOwnershipExitToday(item); paintMyWork(lastContextSnapshot); return false; - }); + }, ()=>selectedPullDetail, ()=>confirmedOwnerLogin); } attachReleaseReceipt(); if (!reviewController) reviewController = createReviewController({ fetchJson: fetchReviewJson, storage: localStorage }); @@ -4632,6 +4632,7 @@ qs('#pull-sheet-title').textContent = item.title || 'Assigned pull request'; qs('#pull-sheet-status').textContent = 'Loading pull request…'; qs('#pull-sheet-body').textContent = ''; + qs('#pull-files').textContent = ''; qs('#pull-review').open = false; qs('#pull-review-status').textContent = 'Expand to load changed files and merge readiness.'; @@ -4663,6 +4664,7 @@ pullConversation = pullController.conversation(item, detail.conversation); qs('#pull-sheet-title').textContent = detail.title || 'Assigned pull request'; qs('#pull-sheet-body').innerHTML = renderMarkdown(detail.body || 'No description provided.'); + renderPullConversation(pullConversation.snapshot()); qs('#open-pull-gitea').href = detail.url || item.url || '#'; qs('#pull-sheet-status').textContent = 'Pull request ready · by ' + (detail.author || 'unknown author'); diff --git a/frontend/index.html b/frontend/index.html index 6bd4051..5b4d0db 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -1651,6 +1651,16 @@
Choose a pull request.
+ +

Full conversation

diff --git a/frontend/pull-sheet.js b/frontend/pull-sheet.js index 8d44ca3..3b58d60 100644 --- a/frontend/pull-sheet.js +++ b/frontend/pull-sheet.js @@ -99,6 +99,9 @@ function resetOwnershipControls(doc, item, checkpointed) { qs('#release-pull').disabled = false; qs('#release-pull').textContent = checkpointed(item) ? 'Release & next' : 'Release assignment'; qs('#pull-handoff-status').textContent = 'Load teammates to transfer ownership.'; + qs('#edit-pull-content').hidden = true; + qs('#pull-edit-form').hidden = true; + qs('#pull-edit-status').textContent = ''; } function resetReviewRequestControls(doc, detail) { @@ -118,7 +121,10 @@ function bindReviewRequestControls(doc, controller, getSelected) { const load = qs('#load-pull-reviewers'); if (load.dataset.reviewRequestBound === 'true') return; load.dataset.reviewRequestBound = 'true'; - controller.setReviewDetail = detail => resetReviewRequestControls(doc, detail); + controller.setReviewDetail = detail => { + resetReviewRequestControls(doc, detail); + controller.edit?.setDetail(detail, Boolean(detail?.saved_at)); + }; resetReviewRequestControls(doc, null); load.addEventListener('click', async () => { const selected = getSelected(); @@ -163,8 +169,81 @@ function bindReviewRequestControls(doc, controller, getSelected) { }); } -function bindOwnershipControls(doc, controller, getSelected, finish) { +function bindContextEditor(doc, controller, getSelected, getDetail, getLogin) { + const qs = selector => doc.querySelector(selector); + const form = qs('#pull-edit-form'); + const controls = () => form.querySelectorAll('input,textarea,button'); + const draft = () => ({ + title:qs('#pull-edit-title').value.trim(), body:qs('#pull-edit-body').value, + expectedHeadSha:getDetail()?.head_sha || '', + }); + qs('#edit-pull-content').addEventListener('click', () => { + const item = getSelected(), detail = getDetail(); + if (!item || !detail) return; + const saved = controller.loadEditDraft(item) || { + title:detail.title || '', body:detail.body || '', expectedHeadSha:detail.head_sha || '', + }; + qs('#pull-edit-title').value = saved.title; + qs('#pull-edit-body').value = saved.body; + form.hidden = false; + qs('#pull-edit-status').textContent = ''; + qs('#pull-edit-title').focus(); + }); + qs('#cancel-pull-edit').addEventListener('click', () => { + form.hidden = true; + qs('#edit-pull-content').focus(); + }); + ['#pull-edit-title', '#pull-edit-body'].forEach(selector => qs(selector).addEventListener('input', () => { + const item = getSelected(); + if (item && getDetail()) controller.saveEditDraft(item, draft()); + })); + form.addEventListener('submit', async event => { + event.preventDefault(); + const item = getSelected(), update = draft(); + if (!item || !getDetail()) return; + if (!update.title) { + qs('#pull-edit-status').textContent = 'Title is required.'; + qs('#pull-edit-title').focus(); + return; + } + controls().forEach(control => { control.disabled = true; }); + qs('#pull-edit-status').textContent = 'Saving review context…'; + try { + const result = await controller.updateContent(item, update); + if (getSelected() !== item) return; + const detail = getDetail(); + Object.assign(detail, result); + item.title = result.title; + qs('#pull-sheet-title').textContent = result.title; + qs('#pull-sheet-body').innerHTML = renderMarkdown(result.body || 'No description provided.'); + controller.setReviewDetail(detail); + qs('#pull-sheet-status').textContent = 'Review context updated · ready to request review'; + form.hidden = true; + qs('#pull-edit-status').textContent = ''; + qs('#edit-pull-content').focus(); + } catch (error) { + qs('#pull-edit-status').textContent = error.message + ' Your change is still here; retry.'; + qs('#pull-edit-title').focus(); + } finally { + controls().forEach(control => { control.disabled = false; }); + } + }); + return { + reset() { + qs('#edit-pull-content').hidden = true; + form.hidden = true; + qs('#pull-edit-status').textContent = ''; + }, + setDetail(detail, offline = false) { + const editable = !offline && detail?.state === 'open' && detail.author === getLogin(); + qs('#edit-pull-content').hidden = !editable; + }, + }; +} + +function bindOwnershipControls(doc, controller, getSelected, finish, getDetail, getLogin) { bindReviewRequestControls(doc, controller, getSelected); + controller.edit = bindContextEditor(doc, controller, getSelected, getDetail, getLogin); const qs = selector => doc.querySelector(selector); const load = qs('#load-pull-handoff'); if (load.dataset.ownershipBound === 'true') return; @@ -237,12 +316,14 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT let candidateRequest = null; let reviewCandidateRequest = null; let reviewRequestMutation = null; + let editRequest = null; const reviewRequests = new Map(); const reviewCache = new Map(); const pathFor = item => 'api/v1/repos/' + String(item.repository || '').split('/') .map(encodeURIComponent).join('/') + '/pulls/' + encodeURIComponent(item.number); const draftKey = item => 'stackchain.pull-comment.v1:' + item.repository + '#' + item.number; const operationKey = item => draftKey(item) + ':operation'; + const editDraftKey = item => 'stackchain.pull-edit.v1:' + item.repository + '#' + item.number; const reviewKey = (item, detail) => 'stackchain.pull-review.v1:' + item.repository + '#' + item.number + ':' + detail.head_sha; const fileNames = detail => (detail?.files || []).map(file => file.filename).filter(Boolean); @@ -305,6 +386,39 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT }).finally(() => { reviewRequestMutation = null; }); return reviewRequestMutation; }, + loadEditDraft(item) { + try { + const value = JSON.parse(storage?.getItem(editDraftKey(item)) || 'null'); + return value && typeof value.title === 'string' && typeof value.body === 'string' && + typeof value.expectedHeadSha === 'string' ? value : null; + } catch (_error) { return null; } + }, + saveEditDraft(item, draft) { + try { storage?.setItem(editDraftKey(item), JSON.stringify(draft)); } + catch (_error) { /* The edit fields remain the fallback. */ } + }, + updateContent(item, draft) { + if (editRequest) return editRequest; + this.saveEditDraft(item, draft); + editRequest = fetchJson(pathFor(item) + '/content', { + method: 'PATCH', + headers: { Accept: 'application/json', 'Content-Type': 'application/json' }, + body: JSON.stringify({ + title: draft.title, + body: draft.body, + expected_head_sha: draft.expectedHeadSha, + }), + }).then(result => { + if (result?.number !== item.number || result?.title !== draft.title || + result?.body !== draft.body || result?.head_sha !== draft.expectedHeadSha) { + throw new Error('Pull request update was not confirmed.'); + } + try { storage?.removeItem(editDraftKey(item)); } + catch (_error) { /* Confirmed upstream content is authoritative. */ } + return result; + }).finally(() => { editRequest = null; }); + return editRequest; + }, handoff(item, recipient) { if (ownershipRequest) return ownershipRequest; ownershipRequest = fetchJson(pathFor(item) + '/handoff', { @@ -404,6 +518,7 @@ createPullSheet.renderHandoffCandidates = renderHandoffCandidates; createPullSheet.ownershipExitMessage = ownershipExitMessage; createPullSheet.resetOwnershipControls = resetOwnershipControls; createPullSheet.bindOwnershipControls = bindOwnershipControls; +createPullSheet.bindContextEditor = bindContextEditor; createPullSheet.resetReviewRequestControls = resetReviewRequestControls; createPullSheet.bindReviewRequestControls = bindReviewRequestControls; if (typeof module !== 'undefined' && module.exports) module.exports = createPullSheet; diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 98fa3aa..e6f9158 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -2703,6 +2703,52 @@ async def is_assigned_pull(repository: str, number: int) -> bool: ) +async def update_authored_assigned_pull( + repository: str, + number: int, + title: str, + body: str, + expected_head_sha: str, +) -> dict: + path = f"repos/{repository}/pulls/{number}" + login, pull = await _current_login_and_target(path) + author = pull.get("user") if isinstance(pull.get("user"), dict) else {} + head = pull.get("head") if isinstance(pull.get("head"), dict) else {} + if ( + pull.get("state") != "open" + or pull.get("merged") is True + or author.get("login") != login + or not _login_in_users(login, pull.get("assignees")) + ): + raise IssueNotAvailableError("pull request not found") + if head.get("sha") != expected_head_sha: + raise IssueEditConflictError("pull request head changed upstream") + response = await _get_client().patch( + f"/api/v1/{path}", + headers=_auth(), + json={"title": title, "body": body}, + ) + response.raise_for_status() + confirmed = response.json() + confirmed_head = confirmed.get("head") if isinstance(confirmed.get("head"), dict) else {} + if ( + confirmed.get("number") != number + or confirmed.get("title") != title + or confirmed.get("body", "") != body + or confirmed_head.get("sha") != expected_head_sha + ): + raise ValueError("Gitea did not confirm the pull request update") + return { + "repository": repository, + "number": number, + "title": title, + "body": body, + "head_sha": expected_head_sha, + "state": confirmed.get("state", "open"), + "url": _safe_gitea_web_url(confirmed.get("html_url")), + } + + async def pull_completion_detail(repository: str, number: int) -> dict: base = f"repos/{repository}/pulls/{number}" pull = await fetch(base) diff --git a/src/main.py b/src/main.py index 5459955..05ab136 100644 --- a/src/main.py +++ b/src/main.py @@ -1117,6 +1117,22 @@ class PullReviewRequest(BaseModel): ) +class PullContentUpdate(BaseModel): + title: str = Field(min_length=1, max_length=255) + body: str = Field(default="", max_length=10_000) + expected_head_sha: str = Field( + min_length=7, max_length=64, pattern=r"^[A-Fa-f0-9]+$" + ) + + @field_validator("title") + @classmethod + def strip_title(cls, value: str) -> str: + value = value.strip() + if not value: + raise ValueError("title cannot be blank") + return value + + class IssueReassignment(IssueHandoff): expected_assignees: list[str] = Field(min_length=1, max_length=10) @@ -6558,6 +6574,41 @@ async def assigned_pull_detail(owner: str, repo: str, number: int = PathParam(gt ) +@app.patch("/api/v1/repos/{owner}/{repo}/pulls/{number}/content") +async def update_authored_assigned_pull_content( + update: PullContentUpdate, + owner: str, + repo: str, + number: int = PathParam(gt=0), +): + repository = f"{owner}/{repo}" + try: + result = await asyncio.wait_for( + gitea_proxy.update_authored_assigned_pull( + repository, + number, + update.title, + update.body, + update.expected_head_sha, + ), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.IssueEditConflictError: + return JSONResponse( + {"error": "This pull request changed in Gitea. Your draft is safe; reload the latest pull request before saving."}, + status_code=409, + ) + except gitea_proxy.IssueNotAvailableError: + raise HTTPException(status_code=404, detail="Editable pull request not found") + except Exception: + return JSONResponse( + {"error": "The pull request could not be updated. Your draft is safe; please 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/test_my_work.py b/tests/test_my_work.py index 154a980..ce4fab7 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -7424,6 +7424,80 @@ Promise.all([first, duplicate]).then(async comments => {{ assert len(output["comments"]) == 2 and len(output["merges"]) == 2 +def test_pull_sheet_preserves_context_draft_until_confirmed_update(): + 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 calls = []; +let fail = true; +const controller = createPullSheet({{ + storage, + fetchJson: async (url, options={{}}) => {{ + const body = JSON.parse(options.body); + calls.push({{url, method:options.method, body}}); + if (fail) {{ fail = false; throw new Error('network down'); }} + return {{number:7,title:body.title,body:body.body,head_sha:body.expected_head_sha,state:'open'}}; + }}, +}}); +const item = {{repository:'stackchain/api',number:7}}; +const draft = {{title:'Clear review brief',body:'Review mobile handoff.',expectedHeadSha:'abc1234'}}; +(async () => {{ + await controller.updateContent(item, draft).catch(() => null); + const kept = controller.loadEditDraft(item); + const result = await controller.updateContent(item, draft); + process.stdout.write(JSON.stringify({{kept,result,after:controller.loadEditDraft(item),calls}})); +}})().catch(error => {{ console.error(error); process.exit(1); }}); +""" + result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + output = json.loads(result.stdout) + assert output["kept"] == { + "title": "Clear review brief", + "body": "Review mobile handoff.", + "expectedHeadSha": "abc1234", + } + assert output["after"] is None + assert output["result"]["head_sha"] == "abc1234" + assert output["calls"] == [ + { + "url": "api/v1/repos/stackchain/api/pulls/7/content", + "method": "PATCH", + "body": { + "title": "Clear review brief", + "body": "Review mobile handoff.", + "expected_head_sha": "abc1234", + }, + }, + { + "url": "api/v1/repos/stackchain/api/pulls/7/content", + "method": "PATCH", + "body": { + "title": "Clear review brief", + "body": "Review mobile handoff.", + "expected_head_sha": "abc1234", + }, + }, + ] + + +@pytest.mark.anyio +async def test_pull_overview_exposes_author_only_context_editor_with_retry_safe_status(): + html = (Path(__file__).parents[1] / "frontend" / "index.html").read_text() + source = await dashboard() + pull_source = PULL_SHEET.read_text() + + assert 'id="edit-pull-content"' in html + assert 'id="pull-edit-form"' in html + assert 'id="pull-edit-title"' in html + assert 'id="pull-edit-body"' in html + assert 'id="save-pull-content"' in html + assert "detail.author === getLogin()" in pull_source + assert "controller.loadEditDraft(item)" in pull_source + assert "controller.updateContent(item" in pull_source + assert "Your change is still here; retry." in pull_source + assert "controller.setReviewDetail(detail)" in pull_source + + def test_pull_sheet_removes_one_pull_from_snapshot_without_touching_other_work(): script = f""" const createPullSheet = require({json.dumps(str(PULL_SHEET))}); @@ -7463,6 +7537,9 @@ const elements = new Map([ ['#load-pull-handoff', {{disabled:true}}], ['#release-pull', {{disabled:true,textContent:''}}], ['#pull-handoff-status', {{textContent:''}}], + ['#edit-pull-content', {{hidden:false}}], + ['#pull-edit-form', {{hidden:false}}], + ['#pull-edit-status', {{textContent:'stale'}}], ]); const select = {{textContent:'stale', appendChild:node => children.push(node)}}; const doc = {{ diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py index 3c108a2..5bcd70b 100644 --- a/tests/test_pull_api.py +++ b/tests/test_pull_api.py @@ -1,4 +1,5 @@ import asyncio +import json import httpx import pytest @@ -7,6 +8,90 @@ from src import gitea_proxy, main from src.security_event_store import SecurityEventStoreError +@pytest.mark.anyio +async def test_pull_author_can_update_assigned_open_pull_context(monkeypatch): + calls = [] + + async def update(repository, number, title, body, expected_head_sha): + calls.append((repository, number, title, body, expected_head_sha)) + return { + "repository": repository, + "number": number, + "title": title, + "body": body, + "head_sha": expected_head_sha, + "state": "open", + } + + monkeypatch.setattr( + main.gitea_proxy, "update_authored_assigned_pull", update, raising=False + ) + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.patch( + "/api/v1/repos/stackchain/api/pulls/7/content", + json={ + "title": "Clarify mobile handoff", + "body": "Explain the reviewer path.", + "expected_head_sha": "abc1234", + }, + ) + + assert response.status_code == 200 + assert response.json()["title"] == "Clarify mobile handoff" + assert calls == [ + ( + "stackchain/api", + 7, + "Clarify mobile handoff", + "Explain the reviewer path.", + "abc1234", + ) + ] + + +@pytest.mark.anyio +async def test_gitea_pull_context_update_requires_author_assignment_and_matching_head(): + requests = [] + + async def handler(request): + requests.append((request.method, request.url.path, request.content)) + 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" and request.method == "GET": + return httpx.Response(200, json={ + "number": 7, "title": "Old context", "body": "Old body", "state": "open", + "merged": False, "user": {"login": "alex"}, "assignees": [{"login": "alex"}], + "head": {"sha": "abc1234"}, + "html_url": "https://forge.example/stackchain/api/pulls/7", + }) + if request.url.path == "/api/v1/repos/stackchain/api/pulls/7" and request.method == "PATCH": + assert json.loads(request.content) == { + "title": "Clear review brief", "body": "Review the mobile handoff." + } + return httpx.Response(200, json={ + "number": 7, "title": "Clear review brief", "body": "Review the mobile handoff.", + "state": "open", "head": {"sha": "abc1234"}, + "html_url": "https://forge.example/stackchain/api/pulls/7", + }) + raise AssertionError(f"unexpected request: {request.method} {request.url.path}") + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + result = await gitea_proxy.update_authored_assigned_pull( + "stackchain/api", 7, "Clear review brief", "Review the mobile handoff.", "abc1234" + ) + finally: + await gitea_proxy.stop_client() + + assert result["title"] == "Clear review brief" + assert [request[:2] for request in requests] == [ + ("GET", "/api/v1/user"), + ("GET", "/api/v1/repos/stackchain/api/pulls/7"), + ("PATCH", "/api/v1/repos/stackchain/api/pulls/7"), + ] + + @pytest.mark.anyio async def test_assigned_pull_detail_reports_completion_state(monkeypatch): async def assigned(repository, number):