Show actionable CI checks in mobile pull reviews #518

Merged
timmy merged 1 commits from timmy/517-actionable-ci-checks into main 2026-08-10 22:33:52 +00:00
8 changed files with 278 additions and 8 deletions

View File

@ -232,6 +232,18 @@ textarea { resize: vertical; min-height: 120px; }
.review-diff-note, .review-diff-empty { display:block; padding:8px; color:#fcd34d; white-space:normal; } .review-diff-note, .review-diff-empty { display:block; padding:8px; color:#fcd34d; white-space:normal; }
.review-action { min-height:44px; } .review-action { min-height:44px; }
.review-retry { min-height:44px; margin-top:10px; } .review-retry { min-height:44px; margin-top:10px; }
.ci-checks { max-width:100%; margin:10px 0; border:1px solid #2a496e; border-radius:10px; overflow:hidden; }
.ci-checks > summary { min-height:44px; display:flex; align-items:center; justify-content:space-between; gap:8px; padding:0 10px; cursor:pointer; }
.ci-checks > summary .small { min-width:0; overflow-wrap:anywhere; text-align:right; }
.ci-check-refresh { width:100%; min-height:44px; margin-bottom:6px; }
.ci-check-list { display:grid; gap:6px; min-width:0; padding:0 8px 8px; }
.ci-check { min-width:0; display:grid; grid-template-columns:minmax(0,1fr) auto; align-items:center; gap:8px; padding:8px; border:1px solid #203a5c; border-radius:8px; }
.ci-check-failure, .ci-check-error { border-color:#dc2626; background:rgba(127,29,29,.22); }
.ci-check-pending { border-color:#d97706; background:rgba(120,53,15,.2); }
.ci-check-success { border-color:#15803d; }
.ci-check-copy { min-width:0; display:grid; gap:3px; overflow-wrap:anywhere; }
.ci-check-copy strong, .ci-check-copy span { min-width:0; overflow-wrap:anywhere; }
.ci-check-link { min-height:44px; display:flex; align-items:center; justify-content:center; padding:0 10px; border:1px solid #60a5fa; border-radius:8px; white-space:nowrap; }
.update-sheet { position:fixed; inset:0; z-index:55; display:none; justify-content:flex-end; background:rgba(5,12,21,.72); backdrop-filter:blur(4px); } .update-sheet { position:fixed; inset:0; z-index:55; display:none; justify-content:flex-end; background:rgba(5,12,21,.72); backdrop-filter:blur(4px); }
.update-sheet.open { display:flex; } .update-sheet.open { display:flex; }
.update-sheet-panel { width:min(560px,100%); height:100%; overflow:auto; padding:18px; background:#0b1526; border-left:1px solid #2a496e; } .update-sheet-panel { width:min(560px,100%); height:100%; overflow:auto; padding:18px; background:#0b1526; border-left:1px solid #2a496e; }

View File

@ -2293,7 +2293,15 @@
if (issueTrigger?.isConnected) issueTrigger.focus(); if (issueTrigger?.isConnected) issueTrigger.focus();
} }
function renderCheckSection(prefix, detail, offline = false) {
const rendered = createReviewController.renderChecks(detail?.checks, escapeHtml, { offline });
qs('#' + prefix + '-checks-summary').textContent = rendered.summary;
qs('#' + prefix + '-check-list').innerHTML = rendered.html || '<div class="muted">No individual checks were reported.</div>';
qs('#' + prefix + '-checks').open = rendered.expanded;
}
function renderPullReview(detail, focusFilename = null) { function renderPullReview(detail, focusFilename = null) {
renderCheckSection('pull', detail);
pullReviewState = pullController.reviewState(selectedPull, detail); pullReviewState = pullController.reviewState(selectedPull, detail);
qs('#pull-review-progress').textContent = pullReviewState.total ? qs('#pull-review-progress').textContent = pullReviewState.total ?
pullReviewState.reviewed.length + ' of ' + pullReviewState.total + ' files reviewed' : 'No changed files to review'; pullReviewState.reviewed.length + ' of ' + pullReviewState.total + ' files reviewed' : 'No changed files to review';
@ -2340,7 +2348,7 @@
} }
} }
async function loadPullReview() { async function loadPullReview({ refresh = false } = {}) {
if (!selectedPull || !selectedPullDetail?.head_sha) return; if (!selectedPull || !selectedPullDetail?.head_sha) return;
const item = selectedPull; const item = selectedPull;
const readDetail = selectedPullDetail; const readDetail = selectedPullDetail;
@ -2348,7 +2356,7 @@
qs('#pull-review-retry').hidden = true; qs('#pull-review-retry').hidden = true;
qs('#merge-pull').disabled = true; qs('#merge-pull').disabled = true;
try { try {
const review = await pullController.loadReview(item, readDetail.head_sha); const review = await pullController.loadReview(item, readDetail.head_sha, { refresh });
if (selectedPull !== item) return; if (selectedPull !== item) return;
selectedPullDetail = { ...readDetail, ...review }; selectedPullDetail = { ...readDetail, ...review };
qs('#pull-ci-state').textContent = 'CI ' + (review.ci_state || 'unknown'); qs('#pull-ci-state').textContent = 'CI ' + (review.ci_state || 'unknown');
@ -2396,6 +2404,9 @@
qs('#pull-comment').value = pullController.loadDraft(item); qs('#pull-comment').value = pullController.loadDraft(item);
qs('#pull-comment-status').textContent = ''; qs('#pull-comment-status').textContent = '';
qs('#pull-ci-state').textContent = 'CI unknown'; qs('#pull-ci-state').textContent = 'CI unknown';
qs('#pull-checks-summary').textContent = 'Not loaded';
qs('#pull-check-list').textContent = '';
qs('#pull-checks').open = false;
qs('#pull-merge-state').textContent = 'Review data not loaded'; qs('#pull-merge-state').textContent = 'Review data not loaded';
qs('#merge-pull').disabled = true; qs('#merge-pull').disabled = true;
qs('#merge-pull').textContent = workSession.active() ? 'Merge & next' : 'Merge'; qs('#merge-pull').textContent = workSession.active() ? 'Merge & next' : 'Merge';
@ -2946,6 +2957,11 @@
qs('#review-handoff-status').textContent = ''; qs('#review-handoff-status').textContent = '';
qs('#review-submit-status').textContent = ''; qs('#review-submit-status').textContent = '';
qs('#continue-review-to-merge').hidden = true; qs('#continue-review-to-merge').hidden = true;
qs('#review-ci-state').textContent = 'CI unknown';
qs('#review-checks-summary').textContent = cachedDetail ? 'Last known · loading' : 'Loading';
qs('#review-check-list').textContent = '';
qs('#review-checks').open = false;
qs('#refresh-review-checks').disabled = offlineReview;
qs('#submit-review').disabled = true; qs('#submit-review').disabled = true;
qs('#submit-review').textContent = offlineReview && reviewingActiveTodayItem() ? 'Queue review & next' : qs('#submit-review').textContent = offlineReview && reviewingActiveTodayItem() ? 'Queue review & next' :
(offlineReview ? 'Queue review for reconnect' : 'Submit review'); (offlineReview ? 'Queue review for reconnect' : 'Submit review');
@ -2959,6 +2975,7 @@
if (selectedReview !== item) return; if (selectedReview !== item) return;
qs('#review-sheet-body').innerHTML = renderMarkdown(detail.body || 'No description provided.'); qs('#review-sheet-body').innerHTML = renderMarkdown(detail.body || 'No description provided.');
qs('#review-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown'); qs('#review-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown');
renderCheckSection('review', detail, offlineReview);
qs('#review-files').innerHTML = (detail.files || []).length ? detail.files.map((file, index) => qs('#review-files').innerHTML = (detail.files || []).length ? detail.files.map((file, index) =>
createReviewController.renderDiffFile(file, index, escapeHtml) createReviewController.renderDiffFile(file, index, escapeHtml)
).join('') : '<div>No changed files reported.</div>'; ).join('') : '<div>No changed files reported.</div>';
@ -4262,6 +4279,13 @@
if (event.currentTarget.open) loadPullReview(); if (event.currentTarget.open) loadPullReview();
}); });
qs('#pull-review-retry').addEventListener('click', loadPullReview); qs('#pull-review-retry').addEventListener('click', loadPullReview);
qs('#refresh-pull-checks').addEventListener('click', async () => {
const button = qs('#refresh-pull-checks');
button.disabled = true;
qs('#pull-checks-summary').textContent = 'Refreshing…';
try { await loadPullReview({ refresh: true }); }
finally { button.disabled = false; }
});
qs('#next-unreviewed-pull-file').addEventListener('click', focusNextUnreviewedPullFile); qs('#next-unreviewed-pull-file').addEventListener('click', focusNextUnreviewedPullFile);
qs('#load-older-pull-comments').addEventListener('click', async () => { qs('#load-older-pull-comments').addEventListener('click', async () => {
if (!pullConversation) return; if (!pullConversation) return;
@ -4459,6 +4483,25 @@
qs('#retry-review-load').addEventListener('click', () => { qs('#retry-review-load').addEventListener('click', () => {
if (selectedReview) openReviewSheet(selectedReview, reviewTrigger); if (selectedReview) openReviewSheet(selectedReview, reviewTrigger);
}); });
qs('#refresh-review-checks').addEventListener('click', async () => {
if (!selectedReview || offlineReview) return;
const item = selectedReview;
const button = qs('#refresh-review-checks');
button.disabled = true;
qs('#review-checks-summary').textContent = 'Refreshing…';
try {
const detail = await reviewController.load(item);
if (selectedReview !== item) return;
if (selectedReviewHead && detail.head_sha !== selectedReviewHead) {
qs('#review-sheet-status').textContent = 'New commits detected. Reload the review before submitting feedback.';
qs('#submit-review').disabled = true;
}
qs('#review-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown');
renderCheckSection('review', detail);
} catch (error) {
if (selectedReview === item) qs('#review-checks-summary').textContent = 'Refresh failed · retry';
} finally { button.disabled = false; }
});
qs('#next-unreviewed-review').addEventListener('click', () => { qs('#next-unreviewed-review').addEventListener('click', () => {
if (progress) openNextUnreviewed(progress.snapshot()); if (progress) openNextUnreviewed(progress.snapshot());
}); });

View File

@ -578,6 +578,11 @@
<div id="pull-review-status" class="small" aria-live="polite">Expand to load changed files and merge readiness.</div> <div id="pull-review-status" class="small" aria-live="polite">Expand to load changed files and merge readiness.</div>
<button class="pull-retry" id="pull-review-retry" type="button" hidden>Retry review data</button> <button class="pull-retry" id="pull-review-retry" type="button" hidden>Retry review data</button>
<div class="row"><span class="pill" id="pull-ci-state">CI unknown</span><span class="pill" id="pull-merge-state">Review data not loaded</span></div> <div class="row"><span class="pill" id="pull-ci-state">CI unknown</span><span class="pill" id="pull-merge-state">Review data not loaded</span></div>
<details class="ci-checks" id="pull-checks">
<summary><span>Checks</span><span class="small" id="pull-checks-summary">Not loaded</span></summary>
<button class="ci-check-refresh" id="refresh-pull-checks" type="button">Refresh checks</button>
<div class="ci-check-list" id="pull-check-list"></div>
</details>
<div class="pull-review-tools"><span id="pull-review-progress" class="small" aria-live="polite">Review progress unavailable.</span><button id="next-unreviewed-pull-file" type="button" disabled>Next unreviewed</button></div> <div class="pull-review-tools"><span id="pull-review-progress" class="small" aria-live="polite">Review progress unavailable.</span><button id="next-unreviewed-pull-file" type="button" disabled>Next unreviewed</button></div>
<div id="pull-files"></div> <div id="pull-files"></div>
<button id="merge-pull" type="button" disabled>Merge</button> <button id="merge-pull" type="button" disabled>Merge</button>
@ -609,6 +614,11 @@
<button class="review-retry" id="retry-review-load" hidden>Retry loading review</button> <button class="review-retry" id="retry-review-load" hidden>Retry loading review</button>
<div class="review-sheet-body markdown-content" id="review-sheet-body"></div> <div class="review-sheet-body markdown-content" id="review-sheet-body"></div>
<div class="row"><span class="pill" id="review-ci-state">CI unknown</span><button class="share-work-route" type="button">Share</button><a id="open-review-gitea" href="#" target="_blank" rel="noopener noreferrer">Open in Gitea</a></div> <div class="row"><span class="pill" id="review-ci-state">CI unknown</span><button class="share-work-route" type="button">Share</button><a id="open-review-gitea" href="#" target="_blank" rel="noopener noreferrer">Open in Gitea</a></div>
<details class="ci-checks" id="review-checks">
<summary><span>Checks</span><span class="small" id="review-checks-summary">Not loaded</span></summary>
<button class="ci-check-refresh" id="refresh-review-checks" type="button">Refresh checks</button>
<div class="ci-check-list" id="review-check-list"></div>
</details>
<h2>Changed files</h2> <h2>Changed files</h2>
<div class="review-display-tools"> <div class="review-display-tools">
<button id="review-wrap-lines" type="button" aria-pressed="false" aria-controls="review-files">Wrap lines</button> <button id="review-wrap-lines" type="button" aria-pressed="false" aria-controls="review-files">Wrap lines</button>

View File

@ -5,7 +5,11 @@ function mergeEligibility(detail, reviewState) {
if (detail.draft) return { allowed: false, reason: 'Draft pull requests cannot be merged' }; if (detail.draft) return { allowed: false, reason: 'Draft pull requests cannot be merged' };
if (!detail.mergeable) return { allowed: false, reason: 'Resolve conflicts before merging' }; if (!detail.mergeable) return { allowed: false, reason: 'Resolve conflicts before merging' };
if (detail.ci_state !== 'success') { if (detail.ci_state !== 'success') {
return { allowed: false, reason: 'CI must succeed before merging' }; const blockers = (detail.checks || []).filter(check =>
['failure', 'error', 'pending'].includes(check.state)
).map(check => check.name).filter(Boolean).slice(0, 3);
return { allowed: false, reason: blockers.length ?
'CI blocked by ' + blockers.join(', ') : 'CI must succeed before merging' };
} }
if (!detail.head_sha) return { allowed: false, reason: 'Current head is unavailable' }; if (!detail.head_sha) return { allowed: false, reason: 'Current head is unavailable' };
if (reviewState && !reviewState.complete) { if (reviewState && !reviewState.complete) {
@ -64,9 +68,9 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
load(item) { load(item) {
return fetchJson(pathFor(item) + '/detail', { headers: { Accept: 'application/json' } }); return fetchJson(pathFor(item) + '/detail', { headers: { Accept: 'application/json' } });
}, },
loadReview(item, headSha) { loadReview(item, headSha, { refresh = false } = {}) {
const key = item.repository + '#' + item.number + ':' + headSha; const key = item.repository + '#' + item.number + ':' + headSha;
if (reviewCache.has(key)) return Promise.resolve(reviewCache.get(key)); if (!refresh && reviewCache.has(key)) return Promise.resolve(reviewCache.get(key));
if (reviewRequests.has(key)) return reviewRequests.get(key); if (reviewRequests.has(key)) return reviewRequests.get(key);
const request = fetchJson(pathFor(item) + '/review-data', { const request = fetchJson(pathFor(item) + '/review-data', {
headers: { Accept: 'application/json' }, headers: { Accept: 'application/json' },

View File

@ -355,6 +355,34 @@ function prepareMergeContinuation({ storage, item, headSha, reviewed, decision }
return true; return true;
} }
function renderChecks(checks, escapeHtml, { offline = false } = {}) {
const rank = { error: 0, failure: 0, pending: 1, warning: 2, unknown: 2, success: 3 };
const items = (Array.isArray(checks) ? checks : []).filter(check =>
check && typeof check.name === 'string' && check.name
).map((check, index) => ({ ...check, index })).sort((left, right) =>
(rank[left.state] ?? 2) - (rank[right.state] ?? 2) || left.index - right.index
);
const failed = items.filter(check => ['failure', 'error'].includes(check.state)).length;
const pending = items.filter(check => check.state === 'pending').length;
const passed = items.filter(check => check.state === 'success').length;
const other = items.length - failed - pending - passed;
const parts = [
failed ? failed + ' failed' : '', pending ? pending + ' pending' : '',
passed ? passed + ' passed' : '', other ? other + ' other' : '',
].filter(Boolean);
const summary = (offline ? 'Last known · ' : '') + (parts.join(' · ') || 'No checks reported');
const html = items.map(check => {
const url = typeof check.url === 'string' ? check.url : '';
const link = url ? '<a class="ci-check-link" href="' + escapeHtml(url) +
'" target="_blank" rel="noopener noreferrer">Open job</a>' : '';
return '<article class="ci-check ci-check-' + escapeHtml(check.state || 'unknown') + '">' +
'<div class="ci-check-copy"><strong>' + escapeHtml(check.name) + '</strong>' +
'<span class="small">' + escapeHtml(check.state || 'unknown') +
(check.description ? ' · ' + escapeHtml(check.description) : '') + '</span></div>' + link + '</article>';
}).join('');
return { summary, html, expanded: failed > 0 };
}
createReviewController.renderDiffFile = renderDiffFile; createReviewController.renderDiffFile = renderDiffFile;
createReviewController.createWrapPreference = createWrapPreference; createReviewController.createWrapPreference = createWrapPreference;
createReviewController.parseDiffLines = parseDiffLines; createReviewController.parseDiffLines = parseDiffLines;
@ -364,6 +392,7 @@ createReviewController.createDraft = createDraft;
createReviewController.formatFeedback = formatFeedback; createReviewController.formatFeedback = formatFeedback;
createReviewController.copyAndContinue = copyAndContinue; createReviewController.copyAndContinue = copyAndContinue;
createReviewController.prepareMergeContinuation = prepareMergeContinuation; createReviewController.prepareMergeContinuation = prepareMergeContinuation;
createReviewController.renderChecks = renderChecks;
if (typeof module !== 'undefined' && module.exports) { if (typeof module !== 'undefined' && module.exports) {
module.exports = createReviewController; module.exports = createReviewController;

View File

@ -4,7 +4,7 @@ import re
import shlex import shlex
from contextlib import asynccontextmanager from contextlib import asynccontextmanager
from typing import Any from typing import Any
from urllib.parse import urlsplit from urllib.parse import urljoin, urlsplit
import httpx import httpx
@ -600,6 +600,51 @@ def _safe_web_url(value: Any) -> str:
return value if parsed.scheme in {"http", "https"} and parsed.netloc else "" return value if parsed.scheme in {"http", "https"} and parsed.netloc else ""
def _safe_gitea_web_url(value: Any) -> str:
if not isinstance(value, str) or not value.strip():
return ""
resolved = urljoin(f"{GITEA_URL}/", value.strip())
parsed = urlsplit(resolved)
configured = urlsplit(GITEA_URL)
base_path = configured.path.rstrip("/")
if (
parsed.scheme not in {"http", "https"}
or parsed.scheme != configured.scheme
or parsed.netloc != configured.netloc
or (base_path and parsed.path != base_path and not parsed.path.startswith(f"{base_path}/"))
):
return ""
return resolved
def _normalize_commit_checks(status: Any) -> list[dict]:
entries = status.get("statuses") if isinstance(status, dict) else None
if not isinstance(entries, list):
return []
rank = {"error": 0, "failure": 0, "pending": 1, "warning": 2, "success": 3}
checks = []
for index, entry in enumerate(entries):
if not isinstance(entry, dict):
continue
name = entry.get("context")
if not isinstance(name, str) or not name.strip():
continue
state = entry.get("status", entry.get("state", "unknown"))
state = state.lower() if isinstance(state, str) else "unknown"
if state not in rank:
state = "unknown"
description = entry.get("description")
checks.append({
"name": name.strip()[:120],
"state": state,
"description": description.strip()[:240] if isinstance(description, str) else "",
"url": _safe_gitea_web_url(entry.get("target_url")),
"_index": index,
})
checks.sort(key=lambda check: (rank.get(check["state"], 2), check["_index"]))
return [{key: value for key, value in check.items() if key != "_index"} for check in checks[:20]]
def _normalize_notifications(threads: Any) -> list[dict]: def _normalize_notifications(threads: Any) -> list[dict]:
if not isinstance(threads, list): if not isinstance(threads, list):
raise ValueError("Gitea notification response was not a list") raise ValueError("Gitea notification response was not a list")
@ -1741,6 +1786,7 @@ async def pull_completion_review(repository: str, number: int) -> dict:
"mergeable": pull.get("mergeable") is True, "mergeable": pull.get("mergeable") is True,
"merged": pull.get("merged") is True, "merged": pull.get("merged") is True,
"ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown", "ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown",
"checks": _normalize_commit_checks(status),
"files": [ "files": [
{ {
"filename": item.get("filename", ""), "filename": item.get("filename", ""),
@ -1873,6 +1919,7 @@ async def pull_review_detail(repository: str, number: int) -> dict:
"author": user.get("login", ""), "author": user.get("login", ""),
"head_sha": sha, "head_sha": sha,
"ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown", "ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown",
"checks": _normalize_commit_checks(status),
"files": normalized_files, "files": normalized_files,
"reviews": normalized_reviews, "reviews": normalized_reviews,
} }

View File

@ -3965,7 +3965,9 @@ const createPullSheet = require({json.dumps(str(PULL_SHEET))});
const states = [ const states = [
{{state:'open', draft:false, mergeable:true, merged:false, ci_state:'success', head_sha:'abc'}}, {{state:'open', draft:false, mergeable:true, merged:false, ci_state:'success', head_sha:'abc'}},
{{state:'open', draft:true, mergeable:true, merged:false, ci_state:'success', head_sha:'abc'}}, {{state:'open', draft:true, mergeable:true, merged:false, ci_state:'success', head_sha:'abc'}},
{{state:'open', draft:false, mergeable:true, merged:false, ci_state:'failure', head_sha:'abc'}}, {{state:'open', draft:false, mergeable:true, merged:false, ci_state:'failure', head_sha:'abc', checks:[
{{name:'lint', state:'failure'}}, {{name:'release', state:'pending'}}, {{name:'build', state:'success'}}
]}},
{{state:'open', draft:false, mergeable:false, merged:false, ci_state:'success', head_sha:'abc'}}, {{state:'open', draft:false, mergeable:false, merged:false, ci_state:'success', head_sha:'abc'}},
]; ];
process.stdout.write(JSON.stringify(states.map(createPullSheet.mergeEligibility))); process.stdout.write(JSON.stringify(states.map(createPullSheet.mergeEligibility)));
@ -3974,7 +3976,7 @@ process.stdout.write(JSON.stringify(states.map(createPullSheet.mergeEligibility)
output = json.loads(result.stdout) output = json.loads(result.stdout)
assert output[0] == {"allowed": True, "reason": "Ready to merge"} assert output[0] == {"allowed": True, "reason": "Ready to merge"}
assert output[1]["allowed"] is False and "draft" in output[1]["reason"].lower() assert output[1]["allowed"] is False and "draft" in output[1]["reason"].lower()
assert output[2]["allowed"] is False and "CI" in output[2]["reason"] assert output[2] == {"allowed": False, "reason": "CI blocked by lint, release"}
assert output[3]["allowed"] is False and "conflict" in output[3]["reason"].lower() assert output[3]["allowed"] is False and "conflict" in output[3]["reason"].lower()
@ -4053,6 +4055,34 @@ Promise.allSettled([first, concurrent]).then(async failed => {{
] ]
def test_pull_sheet_refreshes_checks_without_clearing_head_scoped_progress():
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)}};
const calls = [];
const sheet = createPullSheet({{storage, fetchJson: url => {{
calls.push(url);
return Promise.resolve({{head_sha:'abc123', ci_state:calls.length === 1 ? 'pending' : 'success',
checks:[{{name:'lint', state:calls.length === 1 ? 'pending' : 'success'}}],
files:[{{filename:'src/api.py'}}]}});
}}}});
const item = {{repository:'stackchain/api', number:7}};
sheet.loadReview(item, 'abc123').then(first => {{
sheet.toggleReviewed(item, first, 'src/api.py');
return sheet.loadReview(item, 'abc123', {{refresh:true}}).then(refreshed => {{
process.stdout.write(JSON.stringify({{calls, refreshed, progress:sheet.reviewState(item, refreshed)}}));
}});
}});
"""
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
output = json.loads(result.stdout)
assert len(output["calls"]) == 2
assert output["refreshed"]["ci_state"] == "success"
assert output["progress"] == {"reviewed": ["src/api.py"], "total": 1, "complete": True}
@pytest.mark.anyio @pytest.mark.anyio
async def test_mobile_pull_sheet_puts_reading_before_collapsed_review_controls(): async def test_mobile_pull_sheet_puts_reading_before_collapsed_review_controls():
html = await dashboard() html = await dashboard()
@ -4493,6 +4523,50 @@ process.stdout.write(JSON.stringify({{ html, expanded: button.attrs['aria-expand
assert output["hidden"] is False assert output["hidden"] is False
def test_review_checks_render_blockers_first_with_safe_touch_links():
script = f"""
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
const escapeHtml = value => String(value)
.replaceAll('&', '&amp;').replaceAll('<', '&lt;').replaceAll('>', '&gt;').replaceAll('"', '&quot;');
const result = reviewSheet.renderChecks([
{{name:'build', state:'success', description:'Passed'}},
{{name:'lint <mobile>', state:'failure', description:'Open & fix', url:'https://forge.example/jobs/4'}},
{{name:'release', state:'pending', description:'Waiting'}},
], escapeHtml, {{offline:false}});
const offline = reviewSheet.renderChecks([
{{name:'lint', state:'success', description:'Passed'}},
], escapeHtml, {{offline:true}});
process.stdout.write(JSON.stringify({{result, offline}}));
"""
result = subprocess.run(
["node", "-e", script], check=True, capture_output=True, text=True
)
output = json.loads(result.stdout)
assert output["result"]["summary"] == "1 failed · 1 pending · 1 passed"
assert output["result"]["expanded"] is True
assert output["result"]["html"].index("lint &lt;mobile&gt;") < output["result"]["html"].index("build")
assert 'href="https://forge.example/jobs/4"' in output["result"]["html"]
assert 'target="_blank" rel="noopener noreferrer"' in output["result"]["html"]
assert "Open job" in output["result"]["html"]
assert output["offline"]["summary"].startswith("Last known · ")
@pytest.mark.anyio
async def test_mobile_pull_review_sheets_expose_actionable_refreshable_checks():
html = await dashboard()
assert html.index('id="pull-checks"') < html.index('id="pull-files"')
assert html.index('id="review-checks"') < html.index('id="review-files"')
assert 'id="refresh-pull-checks"' in html
assert 'id="refresh-review-checks"' in html
assert "createReviewController.renderChecks" in html
assert "{ refresh: true }" in html
assert "qs('#refresh-review-checks').disabled = offlineReview" in html
assert ".ci-check-link" in html and "min-height:44px" in html
assert ".ci-check-copy" in html and "overflow-wrap:anywhere" in html
def test_review_wrap_preference_defaults_to_phone_layout_and_persists_explicit_choice(): def test_review_wrap_preference_defaults_to_phone_layout_and_persists_explicit_choice():
script = f""" script = f"""
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))}); const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});

View File

@ -36,6 +36,57 @@ async def test_review_detail_endpoint_returns_normalized_no_store_payload(monkey
assert response.json()["files"][0]["filename"] == "src/api.py" assert response.json()["files"][0]["filename"] == "src/api.py"
@pytest.mark.anyio
async def test_gitea_review_detail_returns_bounded_actionable_checks(monkeypatch):
monkeypatch.setattr(gitea_proxy, "GITEA_URL", "https://forge.example/git")
async def handler(request):
path = request.url.path
if path.endswith("/pulls/7"):
return httpx.Response(200, json={
"title": "Review API", "head": {"sha": "abc123"},
"user": {"login": "alex"},
})
if path.endswith("/pulls/7/files") or path.endswith("/pulls/7/reviews"):
return httpx.Response(200, json=[])
if path.endswith("/commits/abc123/status"):
return httpx.Response(200, json={
"state": "failure",
"statuses": [
{
"context": "lint/<unsafe>", "status": "failure",
"description": "Formatting failed " + "x" * 300,
"target_url": "https://forge.example/git/stackchain/api/actions/runs/9/jobs/4",
},
{
"context": "external", "status": "pending",
"description": "Waiting", "target_url": "https://evil.example/job/1",
},
{"context": "build-release", "status": "success", "description": "Passed"},
] + [{"context": f"extra-{index}", "status": "success"} for index in range(30)],
})
if path.endswith("/pulls/7.diff"):
return httpx.Response(200, text="")
raise AssertionError(f"unexpected request: {request.method} {path}")
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
try:
detail = await gitea_proxy.pull_review_detail("stackchain/api", 7)
finally:
await gitea_proxy.stop_client()
assert len(detail["checks"]) == 20
assert detail["checks"][:3] == [
{
"name": "lint/<unsafe>", "state": "failure",
"description": "Formatting failed " + "x" * 222,
"url": "https://forge.example/git/stackchain/api/actions/runs/9/jobs/4",
},
{"name": "external", "state": "pending", "description": "Waiting", "url": ""},
{"name": "build-release", "state": "success", "description": "Passed", "url": ""},
]
@pytest.mark.anyio @pytest.mark.anyio
async def test_review_detail_rejects_pulls_not_requested_from_service_user(monkeypatch): async def test_review_detail_rejects_pulls_not_requested_from_service_user(monkeypatch):
async def requested(repository, number): async def requested(repository, number):