+
+
@@ -383,6 +385,7 @@ textarea { resize: vertical; min-height: 120px; }
let progress = null;
let draft = null;
let reviewFiles = [];
+ let selectedReviewHead = '';
let bulkConfirmationPending = false;
let bulkMarkPending = false;
let reviewHandoffPending = false;
@@ -717,8 +720,11 @@ textarea { resize: vertical; min-height: 120px; }
qs('#review-handoff-link').hidden = true;
qs('#review-handoff-link').href = item.url;
qs('#review-handoff-status').textContent = '';
+ qs('#review-submit-status').textContent = '';
+ qs('#submit-review').disabled = true;
draft = null;
reviewFiles = [];
+ selectedReviewHead = '';
qs('#close-review-sheet').focus();
try {
const detail = await reviewController.load(selectedReview);
@@ -742,6 +748,7 @@ textarea { resize: vertical; min-height: 120px; }
files: detail.files || [],
});
reviewFiles = detail.files || [];
+ selectedReviewHead = detail.head_sha || '';
draft = createReviewController.createDraft({
storage: localStorage,
repository: item.repository,
@@ -771,6 +778,7 @@ textarea { resize: vertical; min-height: 120px; }
escapeHtml(review.state || 'commented') + (review.body ? '
' + escapeHtml(review.body) + '
' : '') + '
No prior reviews.
';
qs('#review-sheet-status').textContent = 'Ready to review · by ' + (detail.author || 'unknown author');
+ qs('#submit-review').disabled = false;
} catch (error) {
if (selectedReview !== item) return;
qs('#review-sheet-status').textContent = error.message + ' Retry here or use Open in Gitea.';
@@ -785,6 +793,7 @@ textarea { resize: vertical; min-height: 120px; }
progress = null;
draft = null;
reviewFiles = [];
+ selectedReviewHead = '';
if (reviewTrigger?.isConnected) reviewTrigger.focus();
}
@@ -978,6 +987,37 @@ textarea { resize: vertical; min-height: 120px; }
});
qs('#review-summary').addEventListener('input', event => draft?.setSummary(event.target.value));
qs('#review-decision').addEventListener('change', event => draft?.setDecision(event.target.value));
+ qs('#submit-review').addEventListener('click', async () => {
+ if (!draft || !selectedReview || !selectedReviewHead) return;
+ const snapshot = draft.snapshot();
+ const labels = { approve: 'Approve', request_changes: 'Request changes' };
+ if (labels[snapshot.decision] && !window.confirm(
+ labels[snapshot.decision] + ' ' + selectedReview.repository + '#' + selectedReview.number + '?'
+ )) return;
+ const button = qs('#submit-review');
+ button.disabled = true;
+ qs('#review-submit-status').textContent = 'Submitting review…';
+ try {
+ const result = await reviewController.submit(selectedReview, {
+ decision: snapshot.decision,
+ body: createReviewController.formatFeedback(snapshot, reviewFiles),
+ expected_head_sha: selectedReviewHead,
+ });
+ draft.clear();
+ progress?.clear();
+ qs('#review-decision').value = 'comment';
+ qs('#review-summary').value = '';
+ document.querySelectorAll('.review-note').forEach(note => { note.value = ''; });
+ if (progress) showReviewProgress(progress.snapshot());
+ qs('#review-submit-status').textContent = 'Review submitted · ' + (result.state || 'complete') + '.';
+ await load();
+ button.focus();
+ } catch (error) {
+ qs('#review-submit-status').textContent = error.message + ' Your draft is safe; retry or open in Gitea.';
+ button.disabled = false;
+ button.focus();
+ }
+ });
qs('#copy-review-feedback').addEventListener('click', async () => {
if (!draft || !selectedReview || reviewHandoffPending) return;
reviewHandoffPending = true;
diff --git a/frontend/review-sheet.js b/frontend/review-sheet.js
index ed56a65..f31c8e4 100644
--- a/frontend/review-sheet.js
+++ b/frontend/review-sheet.js
@@ -1,4 +1,6 @@
function createReviewController({ fetchJson }) {
+ let pendingSubmission = null;
+
function endpoint(item) {
const [owner, repo] = String(item.repository || '').split('/');
if (!owner || !repo || !Number.isInteger(Number(item.number))) {
@@ -12,7 +14,17 @@ function createReviewController({ fetchJson }) {
return fetchJson(endpoint(item), { headers: { Accept: 'application/json' } });
}
- return { load };
+ function submit(item, payload) {
+ if (pendingSubmission) return pendingSubmission;
+ pendingSubmission = fetchJson(endpoint(item), {
+ method: 'POST',
+ headers: { Accept: 'application/json', 'Content-Type': 'application/json' },
+ body: JSON.stringify(payload),
+ }).finally(() => { pendingSubmission = null; });
+ return pendingSubmission;
+ }
+
+ return { load, submit };
}
function diffLineClass(line) {
@@ -90,7 +102,14 @@ function createProgress({ storage, repository, number, headSha, files }) {
return snapshot();
}
- return { snapshot, markReviewed };
+ function clear() {
+ reviewed = [];
+ newHead = false;
+ try { storage.removeItem(key); } catch (error) { /* cleared in memory */ }
+ return snapshot();
+ }
+
+ return { snapshot, markReviewed, clear };
}
function createDraft({ storage, repository, number, headSha, files }) {
@@ -140,7 +159,13 @@ function createDraft({ storage, repository, number, headSha, files }) {
return snapshot();
}
- return { snapshot, setNote, setSummary, setDecision };
+ function clear() {
+ draft = { notes: {}, summary: '', decision: 'comment' };
+ try { storage.removeItem(key); } catch (error) { /* cleared in memory */ }
+ return snapshot();
+ }
+
+ return { snapshot, setNote, setSummary, setDecision, clear };
}
function formatFeedback(draft, files) {
diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py
index 7d1f2e7..aa985c1 100644
--- a/src/gitea_proxy.py
+++ b/src/gitea_proxy.py
@@ -14,6 +14,10 @@ REVIEW_DIFF_MAX_LINES = 400
_client: httpx.AsyncClient | None = None
+class StaleReviewError(ValueError):
+ """Raised before mutation when a pull request head changed during review."""
+
+
def _auth() -> dict[str, str]:
headers: dict[str, str] = {"Accept": "application/json"}
if GITEA_TOKEN:
@@ -416,6 +420,43 @@ async def pull_review_detail(repository: str, number: int) -> dict:
}
+async def submit_pull_review(
+ repository: str,
+ number: int,
+ expected_head_sha: str,
+ decision: str,
+ body: str,
+) -> dict:
+ base = f"repos/{repository}/pulls/{number}"
+ pull = await fetch(base)
+ head = pull.get("head") if isinstance(pull, dict) else None
+ current_sha = head.get("sha") if isinstance(head, dict) else None
+ if current_sha != expected_head_sha:
+ raise StaleReviewError("Pull request changed while it was being reviewed")
+ response = await _get_client().post(
+ f"/api/v1/{base}/reviews",
+ headers=_auth(),
+ json={
+ "body": body,
+ "event": {
+ "comment": "COMMENT",
+ "approve": "APPROVE",
+ "request_changes": "REQUEST_CHANGES",
+ }[decision],
+ "commit_id": current_sha,
+ },
+ )
+ response.raise_for_status()
+ review = response.json()
+ if not isinstance(review, dict):
+ raise ValueError("Gitea review response was not an object")
+ return {
+ "id": review.get("id"),
+ "state": review.get("state") or "COMMENT",
+ "url": _safe_web_url(review.get("html_url")),
+ }
+
+
async def activity_events(user: dict | None = None) -> list[dict]:
if user is None:
user = await current_user()
diff --git a/src/main.py b/src/main.py
index f1ae60c..9629c63 100644
--- a/src/main.py
+++ b/src/main.py
@@ -93,6 +93,19 @@ class NotificationReply(BaseModel):
return value
+class PullReviewSubmission(BaseModel):
+ decision: str
+ body: str = Field(max_length=10_000)
+ expected_head_sha: str
+
+ @field_validator("decision")
+ @classmethod
+ def validate_decision(cls, value: str) -> str:
+ if value not in {"comment", "approve", "request_changes"}:
+ raise ValueError("unsupported review decision")
+ return value
+
+
def _context_payload(user_data, repo_data, issues_data, prs_data) -> dict:
user_model = User(
id=user_data["id"],
@@ -680,3 +693,40 @@ async def review_detail(owner: str, repo: str, number: int):
{"error": "Pull request review details are temporarily unavailable"},
status_code=503,
)
+
+
+@app.post("/api/v1/repos/{owner}/{repo}/pulls/{number}/review", status_code=201)
+async def submit_review(
+ submission: PullReviewSubmission, owner: str, repo: str, number: int
+):
+ repository = f"{owner}/{repo}"
+
+ async def submit_requested_review():
+ if not await is_requested_review(repository, number):
+ raise HTTPException(status_code=404, detail="Review request not found")
+ return await gitea_proxy.submit_pull_review(
+ repository,
+ number,
+ submission.expected_head_sha,
+ submission.decision,
+ submission.body,
+ )
+
+ try:
+ result = await asyncio.wait_for(
+ submit_requested_review(), timeout=REVIEW_DETAIL_TIMEOUT_SECONDS
+ )
+ except HTTPException:
+ raise
+ except gitea_proxy.StaleReviewError:
+ return JSONResponse(
+ {"error": "New commits were pushed. Refresh the review before submitting."},
+ status_code=409,
+ )
+ except Exception:
+ return JSONResponse(
+ {"error": "The review could not be submitted. Your draft is safe; please retry."},
+ status_code=503,
+ headers={"Retry-After": "1"},
+ )
+ return JSONResponse(result, status_code=201)
diff --git a/tests/test_my_work.py b/tests/test_my_work.py
index 3d3f9e2..51ecc22 100644
--- a/tests/test_my_work.py
+++ b/tests/test_my_work.py
@@ -974,7 +974,12 @@ async def test_review_sheet_loads_details_and_preserves_safe_gitea_handoff():
assert "review-files" in html
assert "review-history" in html
assert "open-review-gitea" in html
- assert "reviewController.submit" not in html
+ assert 'id="submit-review"' in html
+ assert 'id="review-submit-status"' in html and 'aria-live="assertive"' in html
+ assert "reviewController.submit(selectedReview" in html
+ assert "window.confirm" in html
+ assert "expected_head_sha: selectedReviewHead" in html
+ assert "await load()" in html
@pytest.mark.anyio
@@ -1003,6 +1008,86 @@ async def test_mobile_review_sheet_captures_and_safely_hands_off_feedback():
assert "handoffWindow.opener = null" in html
+def test_review_controller_submits_once_while_request_is_in_flight():
+ script = f"""
+const createReviewController = require({json.dumps(str(REVIEW_SHEET))});
+let resolveRequest;
+const calls = [];
+const controller = createReviewController({{ fetchJson: (url, options) => {{
+ calls.push({{url, options}});
+ return new Promise(resolve => {{ resolveRequest = resolve; }});
+}} }});
+const item = {{repository:'stackchain/api', number:7}};
+const payload = {{decision:'approve', body:'Looks good.', expected_head_sha:'abc123'}};
+const first = controller.submit(item, payload);
+const second = controller.submit(item, payload);
+resolveRequest({{id:91, state:'APPROVED'}});
+Promise.all([first, second]).then(results => process.stdout.write(JSON.stringify({{
+ calls, results
+}})));
+"""
+ result = subprocess.run(
+ ["node", "-e", script], check=True, capture_output=True, text=True
+ )
+
+ payload = json.loads(result.stdout)
+ assert len(payload["calls"]) == 1
+ assert payload["calls"][0] == {
+ "url": "api/v1/repos/stackchain/api/pulls/7/review",
+ "options": {
+ "method": "POST",
+ "headers": {
+ "Accept": "application/json",
+ "Content-Type": "application/json",
+ },
+ "body": json.dumps(
+ {
+ "decision": "approve",
+ "body": "Looks good.",
+ "expected_head_sha": "abc123",
+ },
+ separators=(",", ":"),
+ ),
+ },
+ }
+ assert payload["results"] == [
+ {"id": 91, "state": "APPROVED"},
+ {"id": 91, "state": "APPROVED"},
+ ]
+
+
+def test_successful_review_can_clear_head_scoped_draft_and_progress():
+ script = f"""
+const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
+const values = new Map();
+const removed = [];
+const storage = {{
+ getItem: key => values.get(key) || null,
+ setItem: (key, value) => values.set(key, value),
+ removeItem: key => {{ removed.push(key); values.delete(key); }},
+}};
+const options = {{storage, repository:'stackchain/api', number:7, headSha:'abc123', files:[{{filename:'a.py'}}]}};
+const draft = reviewSheet.createDraft(options);
+const progress = reviewSheet.createProgress(options);
+draft.setSummary('Looks good.');
+progress.markReviewed('a.py');
+draft.clear();
+progress.clear();
+process.stdout.write(JSON.stringify({{removed, draft:draft.snapshot(), progress:progress.snapshot()}}));
+"""
+ result = subprocess.run(
+ ["node", "-e", script], check=True, capture_output=True, text=True
+ )
+
+ payload = json.loads(result.stdout)
+ assert payload["removed"] == [
+ "stackchain.review-draft.v1:stackchain/api#7@abc123",
+ "stackchain.review-progress.v1:stackchain/api#7@abc123",
+ ]
+ assert payload["draft"] == {"notes": {}, "summary": "", "decision": "comment"}
+ assert payload["progress"]["reviewedCount"] == 0
+
+
@pytest.mark.anyio
async def test_mobile_review_failure_offers_an_in_place_retry_for_the_same_item():
html = await dashboard()
diff --git a/tests/test_review_api.py b/tests/test_review_api.py
index 8fc14d3..fd005f2 100644
--- a/tests/test_review_api.py
+++ b/tests/test_review_api.py
@@ -3,7 +3,7 @@ import asyncio
import httpx
import pytest
-from src import main
+from src import gitea_proxy, main
@pytest.mark.anyio
@@ -76,13 +76,202 @@ async def test_review_detail_has_one_retryable_deadline_and_cancels_pending_work
@pytest.mark.anyio
-async def test_public_review_endpoint_does_not_expose_service_token_mutations():
+async def test_review_submission_posts_validated_decision_for_requested_current_head(monkeypatch):
+ calls = []
+
+ async def requested(repository, number):
+ return (repository, number) == ("stackchain/api", 7)
+
+ async def submit(repository, number, expected_head_sha, decision, body):
+ calls.append((repository, number, expected_head_sha, decision, body))
+ return {"id": 91, "state": "APPROVED", "url": "https://forge.example/reviews/91"}
+
+ monkeypatch.setattr(main, "is_requested_review", requested)
+ monkeypatch.setattr(main.gitea_proxy, "submit_pull_review", submit, raising=False)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
response = await client.post(
"/api/v1/repos/stackchain/api/pulls/7/review",
- json={"action": "approve"},
+ json={
+ "decision": "approve",
+ "body": "Looks good on mobile.",
+ "expected_head_sha": "abc123",
+ },
)
- assert response.status_code == 405
+ assert response.status_code == 201
+ assert response.headers["cache-control"] == "no-store"
+ assert response.json() == {
+ "id": 91,
+ "state": "APPROVED",
+ "url": "https://forge.example/reviews/91",
+ }
+ assert calls == [
+ ("stackchain/api", 7, "abc123", "approve", "Looks good on mobile.")
+ ]
+
+
+@pytest.mark.anyio
+async def test_gitea_review_submission_checks_head_then_maps_decision_upstream():
+ requests = []
+
+ async def handler(request):
+ requests.append(request)
+ if request.method == "GET":
+ return httpx.Response(200, json={"head": {"sha": "abc123"}})
+ return httpx.Response(
+ 200,
+ json={
+ "id": 91,
+ "state": "APPROVED",
+ "html_url": "https://forge.example/reviews/91",
+ },
+ )
+
+ gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+ try:
+ result = await gitea_proxy.submit_pull_review(
+ "stackchain/api", 7, "abc123", "approve", "Looks good."
+ )
+ finally:
+ await gitea_proxy.stop_client()
+
+ assert [(request.method, request.url.path) for request in requests] == [
+ ("GET", "/api/v1/repos/stackchain/api/pulls/7"),
+ ("POST", "/api/v1/repos/stackchain/api/pulls/7/reviews"),
+ ]
+ assert requests[1].content == b'{"body":"Looks good.","event":"APPROVE","commit_id":"abc123"}'
+ assert result == {
+ "id": 91,
+ "state": "APPROVED",
+ "url": "https://forge.example/reviews/91",
+ }
+
+
+@pytest.mark.anyio
+async def test_gitea_review_submission_rejects_changed_head_without_posting():
+ methods = []
+
+ async def handler(request):
+ methods.append(request.method)
+ return httpx.Response(200, json={"head": {"sha": "new456"}})
+
+ gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+ try:
+ with pytest.raises(gitea_proxy.StaleReviewError):
+ await gitea_proxy.submit_pull_review(
+ "stackchain/api", 7, "abc123", "request_changes", "Please revise."
+ )
+ finally:
+ await gitea_proxy.stop_client()
+
+ assert methods == ["GET"]
+
+
+@pytest.mark.anyio
+async def test_review_submission_returns_conflict_when_pull_head_changed(monkeypatch):
+ async def requested(repository, number):
+ return True
+
+ async def submit(*args):
+ raise gitea_proxy.StaleReviewError("changed")
+
+ monkeypatch.setattr(main, "is_requested_review", requested)
+ monkeypatch.setattr(main.gitea_proxy, "submit_pull_review", submit)
+ transport = httpx.ASGITransport(app=main.app)
+ async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+ response = await client.post(
+ "/api/v1/repos/stackchain/api/pulls/7/review",
+ json={
+ "decision": "request_changes",
+ "body": "Please revise.",
+ "expected_head_sha": "abc123",
+ },
+ )
+
+ assert response.status_code == 409
+ assert response.json() == {
+ "error": "New commits were pushed. Refresh the review before submitting."
+ }
+
+
+@pytest.mark.anyio
+async def test_review_submission_rejects_unsupported_decision_before_upstream(monkeypatch):
+ called = False
+
+ async def requested(repository, number):
+ nonlocal called
+ called = True
+ return True
+
+ monkeypatch.setattr(main, "is_requested_review", requested)
+ transport = httpx.ASGITransport(app=main.app)
+ async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+ response = await client.post(
+ "/api/v1/repos/stackchain/api/pulls/7/review",
+ json={
+ "decision": "merge",
+ "body": "Ship it.",
+ "expected_head_sha": "abc123",
+ },
+ )
+
+ assert response.status_code == 422
+ assert called is False
+
+
+@pytest.mark.anyio
+async def test_review_submission_rejects_oversized_feedback_before_upstream(monkeypatch):
+ called = False
+
+ async def requested(repository, number):
+ nonlocal called
+ called = True
+ return True
+
+ monkeypatch.setattr(main, "is_requested_review", requested)
+ transport = httpx.ASGITransport(app=main.app)
+ async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+ response = await client.post(
+ "/api/v1/repos/stackchain/api/pulls/7/review",
+ json={
+ "decision": "comment",
+ "body": "x" * 10_001,
+ "expected_head_sha": "abc123",
+ },
+ )
+
+ assert response.status_code == 422
+ assert called is False
+
+
+@pytest.mark.anyio
+async def test_review_submission_deadline_cancels_request_check(monkeypatch):
+ cancelled = asyncio.Event()
+
+ async def requested(repository, number):
+ try:
+ await asyncio.Event().wait()
+ finally:
+ cancelled.set()
+
+ monkeypatch.setattr(main, "is_requested_review", requested)
+ monkeypatch.setattr(main, "REVIEW_DETAIL_TIMEOUT_SECONDS", 0.01)
+ transport = httpx.ASGITransport(app=main.app)
+ async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+ response = await asyncio.wait_for(
+ client.post(
+ "/api/v1/repos/stackchain/api/pulls/7/review",
+ json={
+ "decision": "comment",
+ "body": "Review note.",
+ "expected_head_sha": "abc123",
+ },
+ ),
+ timeout=0.2,
+ )
+
+ assert response.status_code == 503
+ assert response.headers["retry-after"] == "1"
+ assert cancelled.is_set()