From 07f5770aa83015a4195feddf2c55a8a4a49b3521 Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 24 Aug 2026 20:29:32 +0000 Subject: [PATCH 1/2] feat: delete merged source branches safely (Closes #1364) --- .gitea/workflows/ci.yml | 2 +- frontend/dashboard.css | 9 +- frontend/release-receipt.js | 83 +++++++++- src/gitea_proxy.py | 93 +++++++++++ src/main.py | 78 +++++++++ ...st_mobile_source_branch_cleanup_release.py | 67 ++++++++ tests/test_ci_workflow.py | 7 + tests/test_pull_api.py | 151 +++++++++++++++++- tests/test_release_receipt.py | 57 +++++++ 9 files changed, 540 insertions(+), 7 deletions(-) create mode 100644 tests/e2e/test_mobile_source_branch_cleanup_release.py diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 26cc37c..99b28cb 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 + 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 release-candidate: runs-on: ubuntu-latest diff --git a/frontend/dashboard.css b/frontend/dashboard.css index da104b7..3637e72 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -35,7 +35,14 @@ header { position: sticky; top: 0; z-index: 20; padding: 12px 16px; display:flex .release-watchlist-item { display:grid; grid-template-columns:minmax(0,1fr) auto; align-items:center; gap:10px; padding:10px; border:1px solid #334155; border-radius:10px; } .release-watchlist-item > div { display:grid; min-width:0; gap:3px; overflow-wrap:anywhere; } .release-watchlist-item > button { width:auto; min-width:88px; } -@media (max-width:359px) { .release-receipt-actions { grid-template-columns:1fr; } } +.release-watchlist-actions { display:grid; grid-template-columns:1fr 1fr; gap:8px; min-width:min(240px,46vw); } +.release-watchlist-actions > button { min-height:44px; width:100%; } +.release-branch-cleanup-status { color:#cbd5e1; } +@media (max-width:359px) { + .release-receipt-actions, .release-watchlist-actions { grid-template-columns:1fr; } + .release-watchlist-item { grid-template-columns:1fr; } + .release-watchlist-actions { min-width:0; width:100%; } +} .issue-filing-receipt-panel { box-sizing:border-box; width:min(560px,100%); max-height:100dvh; overflow:auto; overflow-x:hidden; padding:18px; padding-bottom:calc(18px + env(safe-area-inset-bottom)); border:1px solid #4ade80; border-radius:18px 18px 0 0; background:#0b1526; overflow-wrap:anywhere; } .issue-filing-receipt-panel h2, .issue-filing-receipt-panel h3 { margin:.25rem 0; } .issue-filing-receipt-actions { display:grid; grid-template-columns:1fr 1fr; gap:8px; margin-top:16px; } diff --git a/frontend/release-receipt.js b/frontend/release-receipt.js index 2d40116..d6a2dd8 100644 --- a/frontend/release-receipt.js +++ b/frontend/release-receipt.js @@ -1,4 +1,4 @@ -function createReleaseReceipt({ storage, getLogin, fetchJson, launcher = null, dialog = null, statusNode = null, checksNode = null, releaseNode = null, listNode = null, documentRef = null, windowRef = null, setTimer = setTimeout, clearTimer = clearTimeout, pollMs = 30000 }) { +function createReleaseReceipt({ storage, getLogin, fetchJson, launcher = null, dialog = null, statusNode = null, checksNode = null, releaseNode = null, listNode = null, documentRef = null, windowRef = null, confirmAction = null, setTimer = setTimeout, clearTimer = clearTimeout, pollMs = 30000 }) { const prefix = 'stackchain.release-receipt.v1:'; const limit = 12; let entries = []; @@ -62,6 +62,17 @@ function createReleaseReceipt({ storage, getLogin, fetchJson, launcher = null, d number: Number(item.number), key: item.key || item.repository + '#' + item.number, commit_sha: commitSha, + ...( + mergeResult?.source_repository === item.repository + && mergeResult?.source_branch + && mergeResult?.source_head_sha + ? { + source_branch: String(mergeResult.source_branch), + source_head_sha: String(mergeResult.source_head_sha), + cleanup: { state: 'available', message: 'Merged branch retained.' }, + } + : {} + ), status: null, captured_at: new Date().toISOString(), }; @@ -121,6 +132,14 @@ function createReleaseReceipt({ storage, getLogin, fetchJson, launcher = null, d state.className = 'small'; state.textContent = entry.status?.label || 'Checking the exact merge commit…'; copy.append(title, state); + if (entry.source_branch && entry.source_head_sha) { + const branch = document.createElement('span'); + branch.className = 'small release-branch-cleanup-status'; + branch.textContent = entry.cleanup?.state === 'deleted' + ? 'Branch ' + entry.source_branch + ' · deleted' + : 'Branch ' + entry.source_branch + ' · ' + entry.source_head_sha.slice(0, 8); + copy.append(branch); + } if (entry.status?.release?.url) { const link = document.createElement('a'); link.href = entry.status.release.url; @@ -132,7 +151,25 @@ function createReleaseReceipt({ storage, getLogin, fetchJson, launcher = null, d button.textContent = 'Dismiss'; button.setAttribute('aria-label', 'Dismiss ' + entry.key + ' from release tracking'); button.addEventListener('click', () => dismiss(entry.repository, entry.commit_sha)); - row.append(copy, button); + if (entry.source_branch && entry.cleanup?.state !== 'deleted') { + const actions = document.createElement('div'); + actions.className = 'release-watchlist-actions'; + const cleanup = document.createElement('button'); + cleanup.type = 'button'; + cleanup.textContent = entry.cleanup?.state === 'deleting' ? 'Deleting…' : 'Delete source branch'; + cleanup.disabled = entry.cleanup?.state === 'deleting' || entry.cleanup?.state === 'advanced'; + cleanup.setAttribute( + 'aria-label', + 'Delete merged source branch ' + entry.source_branch + ' at ' + entry.source_head_sha.slice(0, 8), + ); + cleanup.addEventListener('click', () => deleteBranch(entry.repository, entry.commit_sha).catch(error => { + if (statusNode) statusNode.textContent = String(error?.message || error); + })); + actions.append(cleanup, button); + row.append(copy, actions); + } else { + row.append(copy, button); + } return row; }); listNode.replaceChildren(...rows); @@ -168,6 +205,46 @@ function createReleaseReceipt({ storage, getLogin, fetchJson, launcher = null, d finally { refreshing = null; } } + async function deleteBranch(repository, commitSha) { + const entry = entries.find(value => identity(value) === repository + '@' + commitSha); + if (!entry?.source_branch || !entry?.source_head_sha || entry.cleanup?.state === 'deleted') { + throw new Error('Source branch cleanup is unavailable.'); + } + const approve = confirmAction || (windowRef?.confirm ? message => windowRef.confirm(message) : () => false); + if (!approve('Delete ' + entry.source_branch + ' at ' + entry.source_head_sha.slice(0, 8) + '?')) return false; + entry.cleanup = { state: 'deleting', message: 'Deleting source branch…' }; + persist(); + render(); + try { + await fetchJson( + 'api/v1/repos/' + entry.repository.split('/').map(encodeURIComponent).join('/') + + '/pulls/' + encodeURIComponent(entry.number) + '/source-branch', + { + method: 'DELETE', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify({ + source_branch: entry.source_branch, + expected_head_sha: entry.source_head_sha, + }), + }, + ); + entry.cleanup = { state: 'deleted', message: 'Source branch deleted.' }; + persist(); + render(); + return true; + } catch (error) { + entry.cleanup = { + state: error?.status === 409 ? 'advanced' : 'available', + message: error?.status === 409 + ? 'Source branch has newer commits and was retained.' + : 'Branch retained. Retry when connected.', + }; + persist(); + render(); + throw error; + } + } + function dismiss(repository, commitSha) { if (repository && commitSha) entries = entries.filter(entry => identity(entry) !== repository + '@' + commitSha); else entries = []; @@ -195,7 +272,7 @@ function createReleaseReceipt({ storage, getLogin, fetchJson, launcher = null, d schedule(); } - return { capture, restore, refresh, dismiss, bind, fetchJson }; + return { capture, restore, refresh, deleteBranch, dismiss, bind, fetchJson }; } if (typeof module !== 'undefined' && module.exports) module.exports = createReleaseReceipt; diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 0e332af..e7aefb0 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -181,6 +181,14 @@ class PullCreateConflictError(ValueError): """Raised when the selected source branch changed before pull creation.""" +class SourceBranchChangedError(ValueError): + """Raised before deletion when a merged source branch has advanced.""" + + +class SourceBranchCleanupForbiddenError(ValueError): + """Raised before deletion when a merged branch is not safe for this operator.""" + + class IssueDependencyInvalidError(ValueError): """Raised when a requested blocker relationship is not valid.""" @@ -3610,11 +3618,96 @@ async def merge_assigned_pull( merge_commit_sha = payload.get("sha") if isinstance(payload, dict) else None if not isinstance(merge_commit_sha, str) or not merge_commit_sha: raise ValueError("Gitea merge response did not include the merge commit") + head_repo = head.get("repo") if isinstance(head.get("repo"), dict) else {} + source_branch = head.get("ref") + source_repository = head_repo.get("full_name") return { "number": number, "merged": True, "state": "closed", "merge_commit_sha": merge_commit_sha, + **( + { + "source_branch": source_branch, + "source_head_sha": current_sha, + "source_repository": source_repository, + } + if isinstance(source_branch, str) + and source_branch + and isinstance(source_repository, str) + and source_repository + else {} + ), + } + + +async def delete_merged_source_branch( + repository: str, + number: int, + source_branch: str, + expected_head_sha: str, +) -> dict: + """Delete an author's merged same-repository branch only at its exact head.""" + pull, operator, repo = await asyncio.gather( + fetch(f"repos/{repository}/pulls/{number}"), + fetch("user"), + fetch(f"repos/{repository}"), + ) + if not all(isinstance(value, dict) for value in (pull, operator, repo)): + raise SourceBranchCleanupForbiddenError("Branch cleanup identity is unavailable") + author = pull.get("user") if isinstance(pull.get("user"), dict) else {} + head = pull.get("head") if isinstance(pull.get("head"), dict) else {} + head_repo = head.get("repo") if isinstance(head.get("repo"), dict) else {} + if ( + pull.get("merged") is not True + or pull.get("state") != "closed" + or author.get("login") != operator.get("login") + or head.get("ref") != source_branch + or head.get("sha") != expected_head_sha + or head_repo.get("full_name") != repository + or repo.get("default_branch") == source_branch + ): + raise SourceBranchCleanupForbiddenError("Source branch is not eligible for cleanup") + + branch_path = f"/api/v1/repos/{repository}/branches/{quote(source_branch, safe='')}" + branch_response = await _get_client().get(branch_path, headers=_auth()) + if branch_response.status_code == 404: + return { + "number": number, + "deleted": True, + "source_branch": source_branch, + "source_head_sha": expected_head_sha, + } + branch_response.raise_for_status() + branch = branch_response.json() + commit = branch.get("commit") if isinstance(branch, dict) and isinstance(branch.get("commit"), dict) else {} + if not isinstance(branch, dict) or branch.get("protected") is True: + raise SourceBranchCleanupForbiddenError("Protected source branches cannot be deleted") + if commit.get("id") != expected_head_sha: + raise SourceBranchChangedError("Source branch advanced after merge") + + try: + deleted = await _get_client().delete(branch_path, headers=_auth()) + deleted.raise_for_status() + except Exception: + confirmation = await _get_client().get(branch_path, headers=_auth()) + if confirmation.status_code == 404: + return { + "number": number, + "deleted": True, + "source_branch": source_branch, + "source_head_sha": expected_head_sha, + } + raise + confirmation = await _get_client().get(branch_path, headers=_auth()) + if confirmation.status_code != 404: + confirmation.raise_for_status() + raise RuntimeError("Source branch deletion could not be confirmed") + return { + "number": number, + "deleted": True, + "source_branch": source_branch, + "source_head_sha": expected_head_sha, } diff --git a/src/main.py b/src/main.py index bee7b8b..ac987d7 100644 --- a/src/main.py +++ b/src/main.py @@ -1237,6 +1237,15 @@ class PullMergeSubmission(BaseModel): expected_head_sha: str = Field(min_length=1, max_length=128) +class SourceBranchCleanupSubmission(BaseModel): + source_branch: str = Field( + min_length=1, max_length=255, pattern=r"^[A-Za-z0-9_./-]+$" + ) + expected_head_sha: str = Field( + min_length=7, max_length=64, pattern=r"^[A-Fa-f0-9]+$" + ) + + async def _run_idempotent_authored_action( operation: Coroutine[Any, Any, Any], *, @@ -7413,6 +7422,75 @@ async def merge_assigned_pull( ) +@app.delete("/api/v1/repos/{owner}/{repo}/pulls/{number}/source-branch") +async def delete_merged_source_branch( + submission: SourceBranchCleanupSubmission, + request: Request, + owner: str, + repo: str, + number: int = PathParam(gt=0), + step_up_grant: str | None = Header( + default=None, alias="X-Step-Up-Grant", max_length=128 + ), +): + repository = f"{owner}/{repo}" + await _require_step_up( + request, + step_up_grant, + action="delete_source_branch", + target=f"{repository}#{number}@{submission.expected_head_sha}", + ) + journal = _security_event_store() + try: + operation_id = await asyncio.to_thread( + journal.reserve, + "source_branch_deleted", + target=f"{repository}#{number}@{submission.expected_head_sha}", + ) + except SecurityEventStoreError: + return JSONResponse( + {"error": "Security activity is temporarily unavailable. The branch was retained."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + try: + result = await asyncio.wait_for( + gitea_proxy.delete_merged_source_branch( + repository, + number, + submission.source_branch, + submission.expected_head_sha, + ), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + try: + await asyncio.to_thread(journal.finalize, operation_id) + except SecurityEventStoreError: + pass + return result + except gitea_proxy.SourceBranchChangedError: + try: + await asyncio.to_thread(journal.discard, operation_id) + except SecurityEventStoreError: + pass + return JSONResponse( + {"error": "The source branch has newer commits and was retained."}, + status_code=409, + ) + except gitea_proxy.SourceBranchCleanupForbiddenError: + try: + await asyncio.to_thread(journal.discard, operation_id) + except SecurityEventStoreError: + pass + raise HTTPException(status_code=422, detail="Source branch cleanup is unavailable") + except Exception: + return JSONResponse( + {"error": "Branch deletion could not be confirmed. The merge remains complete."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + + @app.get("/api/v1/repos/{owner}/{repo}/release-receipt/{commit_sha}") async def release_receipt(owner: str, repo: str, commit_sha: str): result = await gitea_proxy.release_receipt_status(f"{owner}/{repo}", commit_sha) diff --git a/tests/e2e/test_mobile_source_branch_cleanup_release.py b/tests/e2e/test_mobile_source_branch_cleanup_release.py new file mode 100644 index 0000000..e48b7fa --- /dev/null +++ b/tests/e2e/test_mobile_source_branch_cleanup_release.py @@ -0,0 +1,67 @@ +import os +from pathlib import Path + +import pytest + +if os.getenv("STACKCHAIN_RUN_RELEASE_E2E") != "1": + pytest.skip("packaged branch-cleanup journey runs only in the browser gate", allow_module_level=True) +pytest.importorskip("playwright.sync_api") +from playwright.sync_api import expect, sync_playwright + + +ROOT = Path(__file__).parents[2] +FRONTEND = ROOT / "frontend" + + +@pytest.mark.parametrize("viewport", [ + {"width": 320, "height": 568}, + {"width": 390, "height": 844}, +]) +def test_merged_source_branch_cleanup_is_phone_openable_and_exact(viewport): + with sync_playwright() as playwright: + browser = playwright.chromium.launch(headless=True) + page = browser.new_page(viewport=viewport) + page.set_content((FRONTEND / "index.html").read_text()) + page.add_style_tag(path=FRONTEND / "dashboard.css") + page.add_script_tag(path=FRONTEND / "release-receipt.js") + result = page.evaluate("""async () => { + const calls=[]; + const values=new Map(); + const storage={ + getItem:key=>values.get(key)||null, + setItem:(key,value)=>values.set(key,value), + removeItem:key=>values.delete(key), + }; + const receipt=createReleaseReceipt({ + storage, + getLogin:()=> 'timmy', + listNode:document.querySelector('#release-watchlist'), + confirmAction:()=>true, + fetchJson:async(path, options={})=>{ + calls.push({path, method:options.method, body:options.body}); + return {deleted:true, source_branch:'timmy/feature', source_head_sha:'abc1234'}; + }, + }); + receipt.capture( + {repository:'stackchain/api', number:7, key:'stackchain/api#7'}, + {merge_commit_sha:'merge456', source_branch:'timmy/feature', source_head_sha:'abc1234', source_repository:'stackchain/api'}, + ); + document.querySelector('#release-receipt-sheet').showModal(); + const button=document.querySelector('.release-watchlist-actions button'); + const box=button.getBoundingClientRect(); + const label=button.getAttribute('aria-label'); + button.click(); + await new Promise(resolve=>setTimeout(resolve, 0)); + return { + calls, label, height:box.height, + branch:document.querySelector('.release-branch-cleanup-status').textContent, + overflow:document.documentElement.scrollWidth > document.documentElement.clientWidth, + }; + }""") + expect(page.locator("#release-receipt-sheet")).to_be_visible() + assert result["height"] >= 44 + assert result["label"] == "Delete merged source branch timmy/feature at abc1234" + assert result["calls"][0]["method"] == "DELETE" + assert result["branch"] == "Branch timmy/feature · deleted" + assert result["overflow"] is False + browser.close() diff --git a/tests/test_ci_workflow.py b/tests/test_ci_workflow.py index 13cf220..833aabe 100644 --- a/tests/test_ci_workflow.py +++ b/tests/test_ci_workflow.py @@ -102,6 +102,13 @@ def test_browser_job_gates_authored_pull_queue_journey(): 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:")] diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py index 842f4ae..d0a22ed 100644 --- a/tests/test_pull_api.py +++ b/tests/test_pull_api.py @@ -1892,7 +1892,12 @@ async def test_gitea_merge_returns_the_exact_merge_commit_for_release_tracking() if request.url.path.endswith("/pulls/7"): return httpx.Response(200, json={ "number": 7, "state": "open", "draft": False, "mergeable": True, - "merged": False, "head": {"sha": "abc123"}, + "merged": False, + "head": { + "sha": "abc123", + "ref": "timmy/feature", + "repo": {"full_name": "stackchain/api"}, + }, }) if request.url.path.endswith("/commits/abc123/status"): return httpx.Response(200, json={"state": "success"}) @@ -1909,10 +1914,117 @@ async def test_gitea_merge_returns_the_exact_merge_commit_for_release_tracking() await gitea_proxy.stop_client() assert result == { - "number": 7, "merged": True, "state": "closed", "merge_commit_sha": "merge456" + "number": 7, + "merged": True, + "state": "closed", + "merge_commit_sha": "merge456", + "source_branch": "timmy/feature", + "source_head_sha": "abc123", + "source_repository": "stackchain/api", } +@pytest.mark.anyio +async def test_source_branch_cleanup_endpoint_audits_and_deletes_the_exact_merged_head(monkeypatch): + lifecycle = [] + + class Journal: + def reserve(self, kind, *, target): + lifecycle.append(("reserve", kind, target)) + return "branch-cleanup" + + def finalize(self, operation_id): + lifecycle.append(("finalize", operation_id)) + + def discard(self, operation_id): + lifecycle.append(("discard", operation_id)) + + async def cleanup(repository, number, source_branch, expected_head_sha): + lifecycle.append(("delete", repository, number, source_branch, expected_head_sha)) + return { + "number": number, + "deleted": True, + "source_branch": source_branch, + "source_head_sha": expected_head_sha, + } + + monkeypatch.setattr(main, "_security_event_store", lambda: Journal()) + monkeypatch.setattr(main.gitea_proxy, "delete_merged_source_branch", cleanup, raising=False) + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.request( + "DELETE", + "/api/v1/repos/stackchain/api/pulls/7/source-branch", + json={"source_branch": "timmy/feature", "expected_head_sha": "abc1234"}, + ) + + assert response.status_code == 200 + assert response.json()["deleted"] is True + assert lifecycle == [ + ("reserve", "source_branch_deleted", "stackchain/api#7@abc1234"), + ("delete", "stackchain/api", 7, "timmy/feature", "abc1234"), + ("finalize", "branch-cleanup"), + ] + + +@pytest.mark.anyio +async def test_gitea_deletes_only_the_merged_authors_same_repository_exact_source_branch(): + requests = [] + + async def handler(request): + path = request.url.raw_path.decode() + requests.append((request.method, path)) + if request.url.path.endswith("/user"): + return httpx.Response(200, json={"login": "timmy"}) + if request.url.path.endswith("/pulls/7"): + return httpx.Response(200, json={ + "state": "closed", + "merged": True, + "user": {"login": "timmy"}, + "head": { + "ref": "timmy/feature", + "sha": "abc123", + "repo": {"full_name": "stackchain/api"}, + }, + }) + if request.url.path.endswith("/repos/stackchain/api"): + return httpx.Response(200, json={"default_branch": "main"}) + if path.endswith("/branches/timmy%2Ffeature"): + branch_reads = sum( + method == "GET" and seen_path.endswith("/branches/timmy%2Ffeature") + for method, seen_path in requests + ) + if request.method == "DELETE": + return httpx.Response(204) + if branch_reads == 1: + return httpx.Response(200, json={ + "name": "timmy/feature", + "protected": False, + "commit": {"id": "abc123"}, + }) + return httpx.Response(404) + raise AssertionError(request.url.path) + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + result = await gitea_proxy.delete_merged_source_branch( + "stackchain/api", 7, "timmy/feature", "abc123" + ) + finally: + await gitea_proxy.stop_client() + + assert result == { + "number": 7, + "deleted": True, + "source_branch": "timmy/feature", + "source_head_sha": "abc123", + } + assert requests[-2:] == [ + ("DELETE", "/api/v1/repos/stackchain/api/branches/timmy%2Ffeature"), + ("GET", "/api/v1/repos/stackchain/api/branches/timmy%2Ffeature"), + ] + + @pytest.mark.anyio async def test_release_receipt_matches_only_the_captured_commit_and_reports_checks(monkeypatch): async def receipt(repository, commit_sha): @@ -1979,3 +2091,38 @@ async def test_gitea_release_receipt_ignores_other_commits_and_normalizes_matchi "url": f"{forge}/stackchain/api/releases/download/rc-42/manifest.json", }], } + + +@pytest.mark.anyio +async def test_source_branch_cleanup_resolves_a_lost_delete_response_with_one_absence_check(): + requests = [] + + async def handler(request): + raw_path = request.url.raw_path.decode() + requests.append((request.method, raw_path)) + if request.url.path.endswith("/user"): + return httpx.Response(200, json={"login": "timmy"}) + if request.url.path.endswith("/pulls/7"): + return httpx.Response(200, json={ + "state": "closed", "merged": True, "user": {"login": "timmy"}, + "head": {"ref": "timmy/feature", "sha": "abc123", "repo": {"full_name": "stackchain/api"}}, + }) + if request.url.path.endswith("/repos/stackchain/api"): + return httpx.Response(200, json={"default_branch": "main"}) + if raw_path.endswith("/branches/timmy%2Ffeature"): + if request.method == "DELETE": + raise httpx.ReadTimeout("delete response was lost") + reads = sum(method == "GET" and path.endswith("/branches/timmy%2Ffeature") for method, path in requests) + return httpx.Response(200, json={"protected": False, "commit": {"id": "abc123"}}) if reads == 1 else httpx.Response(404) + raise AssertionError(raw_path) + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + result = await gitea_proxy.delete_merged_source_branch( + "stackchain/api", 7, "timmy/feature", "abc123" + ) + finally: + await gitea_proxy.stop_client() + + assert result["deleted"] is True + assert sum(method == "GET" and path.endswith("/branches/timmy%2Ffeature") for method, path in requests) == 2 diff --git a/tests/test_release_receipt.py b/tests/test_release_receipt.py index f636115..b3a8e8c 100644 --- a/tests/test_release_receipt.py +++ b/tests/test_release_receipt.py @@ -36,6 +36,40 @@ restored && first.refresh().then(status=>{ assert output["keys"] == ["stackchain.release-receipt.v1:timmy"] +def test_release_receipt_deletes_the_exact_captured_source_branch_without_stopping_release_tracking(): + output = run_node(r""" +const values=new Map(), calls=[]; +const storage={getItem:key=>values.get(key)||null,setItem:(key,value)=>values.set(key,value),removeItem:key=>values.delete(key)}; +const receipt=createReleaseReceipt({storage,getLogin:()=> 'timmy',confirmAction:()=>true,fetchJson:async(path,options={})=>{ + calls.push({path,method:options.method||'GET',body:options.body||null}); + if ((options.method||'GET') === 'DELETE') return {number:7,deleted:true,source_branch:'timmy/feature',source_head_sha:'abc1234'}; + return {commit_sha:'merge456',ci_state:'pending',checks:[],release:null}; +}}); +const captured=receipt.capture( + {repository:'stackchain/api',number:7,key:'stackchain/api#7'}, + {merge_commit_sha:'merge456',source_branch:'timmy/feature',source_head_sha:'abc1234',source_repository:'stackchain/api'} +); +(async()=>{ + await receipt.deleteBranch('stackchain/api','merge456'); + await receipt.refresh(); + process.stdout.write(JSON.stringify({captured,calls,restored:receipt.restore()})); +})(); +""") + + assert output["captured"]["source_branch"] == "timmy/feature" + assert output["calls"][0] == { + "path": "api/v1/repos/stackchain/api/pulls/7/source-branch", + "method": "DELETE", + "body": json.dumps({ + "source_branch": "timmy/feature", + "expected_head_sha": "abc1234", + }, separators=(",", ":")), + } + restored = output["restored"][0] + assert restored["cleanup"] == {"state": "deleted", "message": "Source branch deleted."} + assert restored["status"]["label"] == "Checks running" + + def test_receipt_distinguishes_failed_checks_waiting_for_release_and_exact_release(): output = run_node(r""" const values=new Map(); @@ -142,6 +176,25 @@ receipt.capture({repository:'stackchain/web',number:2,key:'stackchain/web#2'}, { assert [item["commit_sha"] for item in output["after"]] == ["pending"] +def test_watchlist_renders_confirmed_touch_cleanup_for_the_exact_source_branch(): + output = run_node(r""" +function node(tag='div') { return {tag,children:[],hidden:false,textContent:'',disabled:false,append(...xs){this.children.push(...xs)},replaceChildren(...xs){this.children=[...xs]},setAttribute(k,v){this[k]=v},addEventListener(k,fn){this[k]=fn}}; } +global.document={createElement:tag=>node(tag)}; +const values=new Map(), calls=[], listNode=node(); +const storage={getItem:key=>values.get(key)||null,setItem:(key,value)=>values.set(key,value),removeItem:key=>values.delete(key)}; +const receipt=createReleaseReceipt({storage,getLogin:()=> 'timmy',listNode,confirmAction:()=>true,fetchJson:async(path,options={})=>{calls.push({path,method:options.method});return {deleted:true};}}); +receipt.capture({repository:'stackchain/api',number:7,key:'stackchain/api#7'}, {merge_commit_sha:'merge456',source_branch:'timmy/feature',source_head_sha:'abc1234',source_repository:'stackchain/api'}); +const row=listNode.children[0], copy=row.children[0], actions=row.children[1], cleanup=actions.children[0]; +(async()=>{ await cleanup.click(); process.stdout.write(JSON.stringify({branch:copy.children[2].textContent,cleanupText:cleanup.textContent,minLabel:cleanup['aria-label'],calls,after:receipt.restore()[0].cleanup})); })(); +""") + + assert output["branch"] == "Branch timmy/feature · abc1234" + assert output["cleanupText"] == "Delete source branch" + assert output["minLabel"] == "Delete merged source branch timmy/feature at abc1234" + assert output["calls"][0]["method"] == "DELETE" + assert output["after"]["state"] == "deleted" + + def test_watchlist_polls_pending_merges_only_while_foregrounded(): output = run_node(r""" const values=new Map(), scheduled=[], calls=[]; @@ -219,3 +272,7 @@ def test_mobile_release_receipt_is_wired_into_the_merge_flow_and_phone_safe(): assert "min-height:44px" in css[css.index(".release-receipt-sheet"):] assert "overflow-x:hidden" in css[css.index(".release-receipt-sheet"):] assert ".release-watchlist-item" in css + assert ".release-watchlist-actions" in css + cleanup_css = css[css.index(".release-watchlist-actions"):] + assert "min-height:44px" in cleanup_css + assert "grid-template-columns:1fr" in cleanup_css From b6684bb7ba73834533dcfed1f024151fe0903bc8 Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 24 Aug 2026 20:44:42 +0000 Subject: [PATCH 2/2] feat: label source branch deletion security events --- frontend/security-center.js | 1 + tests/test_security_center.py | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/frontend/security-center.js b/frontend/security-center.js index ff1ada0..8908301 100644 --- a/frontend/security-center.js +++ b/frontend/security-center.js @@ -231,6 +231,7 @@ passkey_counter_anomaly: 'Passkey counter anomaly', device_revoked: 'Device access revoked', all_sessions_revoked: 'All device access revoked', issue_closed: 'Issue closed', pull_merged: 'Pull request merged', + source_branch_deleted: 'Source branch deleted', comment_deleted: 'Comment deleted', pull_review_approved: 'Pull request approved', pull_review_changes_requested: 'Changes requested', gitea_time_logged: 'Gitea time logged', diff --git a/tests/test_security_center.py b/tests/test_security_center.py index 02e225b..26e1ff9 100644 --- a/tests/test_security_center.py +++ b/tests/test_security_center.py @@ -6,6 +6,12 @@ from pathlib import Path SECURITY_CENTER = Path(__file__).parents[1] / "frontend" / "security-center.js" +def test_source_branch_cleanup_has_a_specific_security_activity_label(): + source = SECURITY_CENTER.read_text() + + assert "source_branch_deleted: 'Source branch deleted'" in source + + def test_open_security_center_loads_all_sections_concurrently_and_is_awaitable(): harness = f""" const attachSecurityCenter=require({json.dumps(str(SECURITY_CENTER))});