ci: poll review statuses from artifacts dynamically
The live comment poller previously got its review status payloads from two sources: (1) REVIEW_STATUSES env var, frozen at comment-live job start from needs.*.outputs.review_status, and (2) a single ci-timings artifact fetched at the end. This meant status details (error messages, action_required items, etc.) only appeared in the comment after all jobs finished, even though job pass/fail was visible in real-time. Now every status-producing workflow_call uploads a small review-status artifact (review-status-<name>) as soon as it completes. The poller enumerates all review-status-* artifacts across the orchestrator run and all sub-workflow runs every cycle, downloads each, and merges them into the comment. Statuses appear as soon as each job finishes, not just at the end. Changes: - live_comment.py: replace _fetch_artifact_statuses (single artifact via gh CLI) with fetch_all_review_statuses (enumerate all review-status-* artifacts via API across all runs, download + parse each). Remove review_statuses_json parameter and --review-statuses- file CLI arg. Remove subprocess import (no longer shells out to gh). - ci.yml: remove REVIEW_STATUSES env var, inline Python merger, and --review-statuses-file arg from the comment-live step. Rename ci-timings-review-status artifact to review-status-ci-timings. - 8 workflow_call files: add a write review-status.json + upload artifact step after each review_status output is produced. - test_live_comment.py: add tests for _parse_status_file (with/without prefix, empty, invalid, nonexistent, non-list) and _merge_statuses.
This commit is contained in:
ethernet
parent
18481742ee
commit
7144eb4900
46
.github/workflows/ci.yml
vendored
46
.github/workflows/ci.yml
vendored
@ -196,47 +196,10 @@ jobs:
|
||||
COMMIT_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
COMMIT_MESSAGE: ${{ github.event.pull_request.head.commit.message }}
|
||||
COMMIT_URL: ${{ github.server_url }}/${{ github.repository }}/pull/${{ github.event.pull_request.number }}/commits/${{ github.event.pull_request.head.sha }}
|
||||
# Structured review statuses from workflow_call jobs.
|
||||
# Each job outputs a JSON array of {source, results: [...]} objects
|
||||
# that the assembler renders directly — no hardcoded job-name
|
||||
# matching. We merge all available outputs into one array.
|
||||
REVIEW_STATUSES: ${{ toJSON(needs.*.outputs.review_status) }}
|
||||
run: |
|
||||
set -uo pipefail
|
||||
|
||||
# REVIEW_STATUSES is a JSON array of strings (some may be empty
|
||||
# when a job was skipped). Parse each string and merge into one
|
||||
# flat array for the assembler.
|
||||
python3 - <<'PYEOF'
|
||||
import json, os, sys
|
||||
|
||||
raw = os.environ.get("REVIEW_STATUSES", "")
|
||||
merged = []
|
||||
if raw:
|
||||
try:
|
||||
arr = json.loads(raw)
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
arr = []
|
||||
for item in arr:
|
||||
if not item:
|
||||
continue
|
||||
try:
|
||||
statuses = json.loads(item)
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
continue
|
||||
if isinstance(statuses, list):
|
||||
merged.extend(statuses)
|
||||
|
||||
# Write merged array to a temp file the poller reads.
|
||||
with open("/tmp/review_statuses.json", "w") as f:
|
||||
json.dump(merged, f)
|
||||
print(f"Merged {len(merged)} review status entries")
|
||||
PYEOF
|
||||
|
||||
python3 scripts/ci/live_comment.py \
|
||||
--interval 15 \
|
||||
--timeout 2100 \
|
||||
--review-statuses-file /tmp/review_statuses.json
|
||||
--timeout 2100
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
# Gate: runs after everything. ``if: always()`` ensures it reports a
|
||||
@ -304,8 +267,9 @@ jobs:
|
||||
# report with a gantt chart + per-step breakdown. The report is uploaded
|
||||
# as an artifact and a markdown summary is written to $GITHUB_STEP_SUMMARY.
|
||||
#
|
||||
# The live comment poller can read the standalone review-status artifact
|
||||
# after the HTML report is uploaded, so its link points straight at that report.
|
||||
# The live comment poller dynamically fetches all review-status-* artifacts
|
||||
# across the orchestrator and sub-workflow runs every cycle, so its link
|
||||
# points straight at that report.
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
ci-timings:
|
||||
name: CI timing report
|
||||
@ -364,7 +328,7 @@ jobs:
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
with:
|
||||
name: ci-timings-review-status
|
||||
name: review-status-ci-timings
|
||||
path: review-status.json
|
||||
retention-days: 14
|
||||
|
||||
|
||||
15
.github/workflows/contributor-check.yml
vendored
15
.github/workflows/contributor-check.yml
vendored
@ -33,6 +33,7 @@ jobs:
|
||||
if [ -z "$NEW_EMAILS" ]; then
|
||||
echo "No new commits to check."
|
||||
echo "review_status=[]" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=[]" > review-status.json
|
||||
exit 0
|
||||
fi
|
||||
|
||||
@ -84,8 +85,22 @@ jobs:
|
||||
--arg how_to_fix "$HOW_TO_FIX" \
|
||||
'[{"source":"contributor attribution","results":[{"kind":"action_required","title":"Unmapped contributor email(s)","summary":"New contributor email(s) are not in AUTHOR_MAP.","detail":$detail,"how_to_fix":$how_to_fix}]}]')
|
||||
echo "review_status=$REVIEW_STATUS" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=$REVIEW_STATUS" > review-status.json
|
||||
|
||||
exit 1
|
||||
else
|
||||
echo "✅ All contributor emails are mapped."
|
||||
echo "review_status=[]" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=[]" > review-status.json
|
||||
fi
|
||||
|
||||
- name: Upload review status artifact
|
||||
if: always() && steps.check-emails.outcome != 'skipped'
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
with:
|
||||
name: review-status-contributor-check
|
||||
path: review-status.json
|
||||
retention-days: 1
|
||||
overwrite: true
|
||||
if-no-files-found: ignore
|
||||
|
||||
12
.github/workflows/e2e-desktop.yml
vendored
12
.github/workflows/e2e-desktop.yml
vendored
@ -170,6 +170,18 @@ jobs:
|
||||
cat /tmp/e2e-review-status.json
|
||||
echo '__E2E_REVIEW_STATUS__'
|
||||
} >> "$GITHUB_OUTPUT"
|
||||
cp /tmp/e2e-review-status.json review-status.json
|
||||
|
||||
- name: Upload review status artifact
|
||||
if: always() && steps.review-status.outcome != 'skipped'
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
with:
|
||||
name: review-status-e2e-desktop
|
||||
path: apps/desktop/review-status.json
|
||||
retention-days: 1
|
||||
overwrite: true
|
||||
if-no-files-found: ignore
|
||||
|
||||
# The trusted workflow_run publisher consumes only this flat, bounded
|
||||
# artifact. It turns selected images into GitHub attachment URLs; it
|
||||
|
||||
13
.github/workflows/history-check.yml
vendored
13
.github/workflows/history-check.yml
vendored
@ -44,6 +44,7 @@ jobs:
|
||||
if ! BASE=$(git merge-base origin/main HEAD 2>/dev/null) || [ -z "$BASE" ]; then
|
||||
STATUS='[{"source":"unrelated histories","results":[{"kind":"action_required","title":"Unrelated histories","summary":"This PR has no common ancestor with main.","detail":"","how_to_fix":"Rebase your changes onto current main:\n```\ngit fetch origin main\ngit checkout -b fix-branch origin/main\n# re-apply your changes (cherry-pick, copy files, etc.)\ngit push -f origin fix-branch\n```\n"}]}]'
|
||||
echo "review_status=${STATUS}" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=${STATUS}" > review-status.json
|
||||
echo ""
|
||||
echo "::error::This PR has no common ancestor with main."
|
||||
echo ""
|
||||
@ -66,3 +67,15 @@ jobs:
|
||||
fi
|
||||
echo "::notice::Common ancestor with main: $BASE"
|
||||
echo "review_status=[]" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=[]" > review-status.json
|
||||
|
||||
- name: Upload review status artifact
|
||||
if: always() && steps.merge-base-check.outcome != 'skipped'
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
with:
|
||||
name: review-status-history-check
|
||||
path: review-status.json
|
||||
retention-days: 1
|
||||
overwrite: true
|
||||
if-no-files-found: ignore
|
||||
|
||||
14
.github/workflows/lockfile-diff.yml
vendored
14
.github/workflows/lockfile-diff.yml
vendored
@ -79,12 +79,24 @@ jobs:
|
||||
|
||||
if [ "$CHANGED" = "true" ]; then
|
||||
CONTENT=$(cat /tmp/lockfile-diff.md | python3 -c "import sys,json; print(json.dumps(sys.stdin.read()))")
|
||||
STATUS="[{\"source\":\"lockfile-diff\",\"results\":[{\"kind\":\"action_required\",\"title\":\"package-lock.json\",\"summary\":\"Locked npm dependency versions changed.\",\"detail\":${CONTENT},\"how_to_fix\":\"Add the \`ci-reviewed\` label after verifying the version changes are expected.\"}]}"
|
||||
STATUS="[{\"source\":\"lockfile-diff\",\"results\":[{\"kind\":\"action_required\",\"title\":\"package-lock.json\",\"summary\":\"Locked npm dependency versions changed.\",\"detail\":${CONTENT},\"how_to_fix\":\"Add the \`ci-reviewed\` label after verifying the version changes are expected.\"}]}]"
|
||||
else
|
||||
STATUS="[]"
|
||||
fi
|
||||
|
||||
echo "review_status=${STATUS}" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=${STATUS}" > review-status.json
|
||||
|
||||
- name: Upload review status artifact
|
||||
if: always() && steps.emit-status.outcome != 'skipped'
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
with:
|
||||
name: review-status-lockfile-diff
|
||||
path: review-status.json
|
||||
retention-days: 1
|
||||
overwrite: true
|
||||
if-no-files-found: ignore
|
||||
|
||||
- name: Upload diff artifact
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
|
||||
12
.github/workflows/osv-scanner.yml
vendored
12
.github/workflows/osv-scanner.yml
vendored
@ -125,3 +125,15 @@ jobs:
|
||||
fi
|
||||
|
||||
echo "review_status=${STATUS}" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=${STATUS}" > review-status.json
|
||||
|
||||
- name: Upload review status artifact
|
||||
if: always() && steps.emit.outcome != 'skipped'
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
with:
|
||||
name: review-status-osv-scanner
|
||||
path: review-status.json
|
||||
retention-days: 1
|
||||
overwrite: true
|
||||
if-no-files-found: ignore
|
||||
|
||||
14
.github/workflows/review-labels.yml
vendored
14
.github/workflows/review-labels.yml
vendored
@ -98,9 +98,23 @@ jobs:
|
||||
if [ "$SUPPLY_CHAIN" = "true" ]; then args+=(--supply-chain); fi
|
||||
if [ "$LABEL_PRESENT" = "true" ]; then args+=(--label-present); fi
|
||||
|
||||
# Write to both $GITHUB_OUTPUT and review-status.json for the
|
||||
# live comment poller to pick up as an artifact.
|
||||
python3 scripts/ci/emit_review_status.py "${args[@]}" \
|
||||
--repo-url "$REPO_URL" --base-sha "$BASE_SHA" --head-sha "$HEAD_SHA" \
|
||||
--output "$GITHUB_OUTPUT"
|
||||
grep '^review_status=' "$GITHUB_OUTPUT" > review-status.json
|
||||
|
||||
- name: Upload review status artifact
|
||||
if: always() && steps.build-status.outcome != 'skipped'
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
with:
|
||||
name: review-status-review-labels
|
||||
path: review-status.json
|
||||
retention-days: 1
|
||||
overwrite: true
|
||||
if-no-files-found: ignore
|
||||
|
||||
- name: Fail on missing label
|
||||
if: steps.label-check.outputs.ci_reviewed != 'true'
|
||||
|
||||
15
.github/workflows/supply-chain-audit.yml
vendored
15
.github/workflows/supply-chain-audit.yml
vendored
@ -290,4 +290,19 @@ jobs:
|
||||
with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f:
|
||||
f.write(f"review_status={json.dumps(merged)}\n")
|
||||
f.write("critical_findings=" + os.environ.get("CRITICAL_FINDINGS", "false") + "\n")
|
||||
|
||||
# Write review-status.json for the live comment poller artifact.
|
||||
with open("review-status.json", "w", encoding="utf-8") as f:
|
||||
f.write(f"review_status={json.dumps(merged)}\n")
|
||||
PYEOF
|
||||
|
||||
- name: Upload review status artifact
|
||||
if: always() && steps.merge.outcome != 'skipped'
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
with:
|
||||
name: review-status-supply-chain
|
||||
path: review-status.json
|
||||
retention-days: 1
|
||||
overwrite: true
|
||||
if-no-files-found: ignore
|
||||
|
||||
13
.github/workflows/uv-lockfile-check.yml
vendored
13
.github/workflows/uv-lockfile-check.yml
vendored
@ -126,7 +126,20 @@ jobs:
|
||||
echo "::error title=uv.lock out of sync::Run \`uv lock\` locally and commit the result. If on a PR, sync with main first."
|
||||
review_status='[{"source":"uv.lock check","results":[{"kind":"action_required","title":"uv.lock out of sync","summary":"uv.lock is out of sync with pyproject.toml.","how_to_fix":"Run `uv lock` locally and commit the result. If on a PR, sync with main first:\n```\ngit fetch origin main\ngit rebase origin/main\nuv lock\ngit add uv.lock\ngit commit -m \"chore: refresh uv.lock\"\n```\n"}]}]'
|
||||
echo "review_status=${review_status}" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=${review_status}" > review-status.json
|
||||
exit 1
|
||||
fi
|
||||
review_status='[]'
|
||||
echo "review_status=${review_status}" >> "$GITHUB_OUTPUT"
|
||||
echo "review_status=${review_status}" > review-status.json
|
||||
|
||||
- name: Upload review status artifact
|
||||
if: always() && steps.verify.outcome != 'skipped'
|
||||
continue-on-error: true
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
with:
|
||||
name: review-status-uv-lockfile
|
||||
path: review-status.json
|
||||
retention-days: 1
|
||||
overwrite: true
|
||||
if-no-files-found: ignore
|
||||
|
||||
@ -20,13 +20,17 @@ Architecture:
|
||||
|
||||
- :func:`find_comment_id` / :func:`upsert_comment` — thin API wrappers.
|
||||
|
||||
- :func:`_fetch_timings_statuses` — downloads the ci-timings artifact
|
||||
(if available) and parses the ``review_status=`` line from it, merging
|
||||
the status objects into the review statuses array.
|
||||
- :func:`fetch_all_review_statuses` — enumerates all
|
||||
``review-status-*`` artifacts across the orchestrator run and all
|
||||
sub-workflow runs, downloads each, parses the ``review_status=``
|
||||
line from ``review-status.json``, and merges into one array.
|
||||
Recomputed from source every poll cycle, so statuses appear as
|
||||
soon as each job uploads its artifact.
|
||||
|
||||
- :func:`run` — the polling loop. Calls the API, classifies, assembles,
|
||||
upserts, sleeps, repeats. Before its final exit, it gives downstream jobs
|
||||
a short grace period to appear.
|
||||
- :func:`run` — the polling loop. Calls the API, classifies,
|
||||
fetches artifacts, assembles, upserts, sleeps, repeats. Before
|
||||
its final exit, it gives downstream jobs a short grace period
|
||||
to appear.
|
||||
|
||||
The orchestrator job names (detect, all-checks-pass, comment-live, etc.)
|
||||
are excluded from the comment — they're infrastructure, not review signal.
|
||||
@ -37,11 +41,12 @@ from __future__ import annotations
|
||||
import argparse
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
import shutil
|
||||
import sys
|
||||
import time
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
import zipfile
|
||||
from pathlib import Path
|
||||
|
||||
API_BASE = "https://api.github.com"
|
||||
@ -265,39 +270,95 @@ def upsert_comment(
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Artifact fetching (ci-timings review_status)
|
||||
# Artifact fetching (dynamic review-status artifacts)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# Prefix for all review-status artifacts uploaded by status-producing jobs.
|
||||
# Each job uploads a ``review-status-<name>`` artifact containing a
|
||||
# ``review-status.json`` file in GITHUB_OUTPUT format:
|
||||
# review_status=<json array of {source, results: [...]} objects>
|
||||
_REVIEW_STATUS_ARTIFACT_PREFIX = "review-status-"
|
||||
|
||||
def _fetch_artifact_statuses(
|
||||
token: str, repo: str, run_id: str, artifact_name: str,
|
||||
) -> list[dict]:
|
||||
"""Download a workflow artifact and extract review_status entries.
|
||||
|
||||
The ci-timings job writes a ``review-status.json`` file containing
|
||||
``review_status=<json>`` (GITHUB_OUTPUT format) into its artifact.
|
||||
This function downloads the artifact, parses the line, and returns
|
||||
the parsed status array. Returns ``[]`` if the artifact doesn't exist
|
||||
yet or can't be parsed.
|
||||
def _list_run_ids(token: str, repo: str, run_id: str) -> list[str]:
|
||||
"""Return the orchestrator run ID + all sub-workflow run IDs.
|
||||
|
||||
Sub-workflow runs (workflow_call) may not exist yet on the first few
|
||||
polls — that's fine, they just won't be in the list.
|
||||
"""
|
||||
owner, repo_name = repo.split("/")
|
||||
run_info = _api_request(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs/{run_id}", token
|
||||
)
|
||||
created_at = run_info.get("created_at", "")
|
||||
head_sha = run_info.get("head_sha", "")
|
||||
|
||||
run_ids = [run_id]
|
||||
|
||||
sub_runs = _api_get_paginated(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs"
|
||||
f"?head_sha={head_sha}&event=workflow_call&per_page=100",
|
||||
token, list_key="workflow_runs",
|
||||
)
|
||||
sub_runs = [r for r in sub_runs if r.get("created_at", "") >= created_at]
|
||||
run_ids.extend(str(r["id"]) for r in sub_runs)
|
||||
|
||||
return run_ids
|
||||
|
||||
|
||||
def _list_artifacts(token: str, repo: str, run_id: str) -> list[dict]:
|
||||
"""List artifacts for a given run (paginated)."""
|
||||
owner, repo_name = repo.split("/")
|
||||
return _api_get_paginated(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs/{run_id}/artifacts",
|
||||
token, list_key="artifacts",
|
||||
)
|
||||
|
||||
|
||||
def _download_artifact(
|
||||
token: str, repo: str, artifact: dict, dest_dir: Path,
|
||||
) -> Path | None:
|
||||
"""Download a single artifact zip via the API and extract it.
|
||||
|
||||
Returns the path to ``review-status.json`` inside the extracted dir,
|
||||
or ``None`` if the download or extraction failed.
|
||||
"""
|
||||
owner, repo_name = repo.split("/")
|
||||
archive_download_url = artifact.get("archive_download_url", "")
|
||||
if not archive_download_url:
|
||||
return None
|
||||
|
||||
# The archive_download_url is an API URL that redirects to a S3 URL.
|
||||
# Build the request with our auth headers so the redirect works.
|
||||
req = urllib.request.Request(archive_download_url, headers={
|
||||
"Authorization": f"Bearer {token}",
|
||||
"Accept": "application/vnd.github+json",
|
||||
"X-GitHub-Api-Version": "2022-11-28",
|
||||
"User-Agent": "ci-live-comment",
|
||||
})
|
||||
zip_path = dest_dir / f"{artifact['name']}.zip"
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["gh", "run", "download", run_id, "--repo", repo,
|
||||
"--name", artifact_name, "--dir", "/tmp/artifact-dl"],
|
||||
capture_output=True, timeout=30,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
return []
|
||||
with urllib.request.urlopen(req) as resp:
|
||||
zip_path.write_bytes(resp.read())
|
||||
except Exception:
|
||||
return []
|
||||
return None
|
||||
|
||||
status_file = Path("/tmp/artifact-dl/review-status.json")
|
||||
if not status_file.exists():
|
||||
return []
|
||||
extract_dir = dest_dir / artifact["name"]
|
||||
extract_dir.mkdir(parents=True, exist_ok=True)
|
||||
try:
|
||||
with zipfile.ZipFile(zip_path) as zf:
|
||||
zf.extractall(extract_dir)
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
status_file = extract_dir / "review-status.json"
|
||||
return status_file if status_file.exists() else None
|
||||
|
||||
|
||||
def _parse_status_file(status_file: Path) -> list[dict]:
|
||||
"""Parse a review-status.json file in GITHUB_OUTPUT format."""
|
||||
try:
|
||||
content = status_file.read_text(encoding="utf-8").strip()
|
||||
# GITHUB_OUTPUT format: review_status=<json>
|
||||
if content.startswith("review_status="):
|
||||
content = content[len("review_status="):]
|
||||
statuses = json.loads(content)
|
||||
@ -305,10 +366,59 @@ def _fetch_artifact_statuses(
|
||||
return statuses
|
||||
except (json.JSONDecodeError, OSError):
|
||||
pass
|
||||
|
||||
return []
|
||||
|
||||
|
||||
def fetch_all_review_statuses(
|
||||
token: str, repo: str, run_id: str,
|
||||
) -> list[dict]:
|
||||
"""Fetch and merge all review-status artifacts across all runs.
|
||||
|
||||
Enumerates artifacts with the ``review-status-`` prefix from the
|
||||
orchestrator run and all sub-workflow runs (workflow_call). Downloads
|
||||
each, parses the ``review-status.json`` inside, and merges into a
|
||||
single flat array.
|
||||
|
||||
Returns the merged list of ``{source, results: [...]}`` objects.
|
||||
Artifacts that don't exist yet or fail to parse are silently skipped.
|
||||
"""
|
||||
all_statuses: list[dict] = []
|
||||
temp_base = Path("/tmp/review-status-artifacts")
|
||||
|
||||
try:
|
||||
run_ids = _list_run_ids(token, repo, run_id)
|
||||
except Exception:
|
||||
return all_statuses
|
||||
|
||||
for rid in run_ids:
|
||||
try:
|
||||
artifacts = _list_artifacts(token, repo, rid)
|
||||
except Exception:
|
||||
continue
|
||||
|
||||
rs_artifacts = [
|
||||
a for a in artifacts
|
||||
if a.get("name", "").startswith(_REVIEW_STATUS_ARTIFACT_PREFIX)
|
||||
]
|
||||
if not rs_artifacts:
|
||||
continue
|
||||
|
||||
# Clean temp dir for this run's artifacts.
|
||||
run_dl_dir = temp_base / rid
|
||||
if run_dl_dir.exists():
|
||||
shutil.rmtree(run_dl_dir)
|
||||
run_dl_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
for artifact in rs_artifacts:
|
||||
status_file = _download_artifact(token, repo, artifact, run_dl_dir)
|
||||
if status_file is None:
|
||||
continue
|
||||
statuses = _parse_status_file(status_file)
|
||||
all_statuses.extend(statuses)
|
||||
|
||||
return all_statuses
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Comment assembly
|
||||
# ---------------------------------------------------------------------------
|
||||
@ -344,12 +454,9 @@ def build_comment_body(
|
||||
)
|
||||
|
||||
|
||||
def _merge_statuses(
|
||||
base_statuses: list[dict], extra_statuses: list[dict]
|
||||
) -> str:
|
||||
"""Merge two status arrays into one JSON string."""
|
||||
merged = list(base_statuses) + list(extra_statuses)
|
||||
return json.dumps(merged) if merged else ""
|
||||
def _merge_statuses(statuses: list[dict]) -> str:
|
||||
"""Merge a list of status arrays into one JSON string."""
|
||||
return json.dumps(statuses) if statuses else ""
|
||||
|
||||
|
||||
def _commit_info_for_state(commit_info: str, pending: list[str]) -> str:
|
||||
@ -370,7 +477,6 @@ def run(
|
||||
run_id: str,
|
||||
pr_number: str,
|
||||
run_url: str,
|
||||
review_statuses_json: str = "",
|
||||
commit_info: str = "",
|
||||
interval: int = 15,
|
||||
timeout: int = 1800,
|
||||
@ -386,13 +492,7 @@ def run(
|
||||
quiet_grace_used = False
|
||||
prev_completed: dict[str, str] = {}
|
||||
prev_pending: list[str] = []
|
||||
|
||||
# Parse the base statuses once (from review-labels, lockfile-diff, etc.)
|
||||
try:
|
||||
base_statuses = json.loads(review_statuses_json) if review_statuses_json else []
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
base_statuses = []
|
||||
print(f" Loaded {len(base_statuses)} base review status entries")
|
||||
prev_artifact_count = 0
|
||||
|
||||
while True:
|
||||
elapsed = time.time() - start
|
||||
@ -426,14 +526,14 @@ def run(
|
||||
if gone_pending:
|
||||
print(f" → {len(gone_pending)} job(s) disappeared from pending: {', '.join(gone_pending)}")
|
||||
|
||||
# Try to fetch ci-timings artifact statuses (may not exist yet).
|
||||
artifact_statuses = _fetch_artifact_statuses(
|
||||
token, repo, run_id, "ci-timings-review-status",
|
||||
)
|
||||
if artifact_statuses:
|
||||
print(f" Found ci-timings artifact with {len(artifact_statuses)} status entries")
|
||||
# Dynamically fetch all review-status artifacts from every run.
|
||||
artifact_statuses = fetch_all_review_statuses(token, repo, run_id)
|
||||
if len(artifact_statuses) != prev_artifact_count:
|
||||
print(f" Found {len(artifact_statuses)} review status entries from artifacts "
|
||||
f"(was {prev_artifact_count} last poll)")
|
||||
prev_artifact_count = len(artifact_statuses)
|
||||
|
||||
merged_json = _merge_statuses(base_statuses, artifact_statuses)
|
||||
merged_json = _merge_statuses(artifact_statuses)
|
||||
current_commit_info = _commit_info_for_state(commit_info, pending)
|
||||
|
||||
body = build_comment_body(
|
||||
@ -450,7 +550,7 @@ def run(
|
||||
change_reasons.append(f"{len(new_pending)} new pending job(s)")
|
||||
if gone_pending:
|
||||
change_reasons.append(f"{len(gone_pending)} job(s) left pending")
|
||||
if artifact_statuses:
|
||||
if len(artifact_statuses) != prev_artifact_count:
|
||||
change_reasons.append("artifact statuses updated")
|
||||
if not change_reasons:
|
||||
change_reasons.append("initial post")
|
||||
@ -507,8 +607,6 @@ def main() -> int:
|
||||
help="Seconds between polls (default: 15).")
|
||||
parser.add_argument("--timeout", type=int, default=1800,
|
||||
help="Max seconds to poll before giving up (default: 1800).")
|
||||
parser.add_argument("--review-statuses-file", type=Path, default=None,
|
||||
help="Path to a JSON file with merged review statuses from workflow_call jobs.")
|
||||
parser.add_argument("--dry-run", action="store_true",
|
||||
help="Print comment body instead of posting to PR.")
|
||||
args = parser.parse_args()
|
||||
@ -533,14 +631,6 @@ def main() -> int:
|
||||
print("PR_NUMBER is required", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
# Read merged review statuses from file (prepared by the ci.yml step).
|
||||
review_statuses_json = ""
|
||||
if args.review_statuses_file:
|
||||
try:
|
||||
review_statuses_json = args.review_statuses_file.read_text(encoding="utf-8")
|
||||
except OSError as e:
|
||||
print(f"Warning: could not read review statuses file: {e}", file=sys.stderr)
|
||||
|
||||
# Build commit info line from env vars (set by ci.yml).
|
||||
commit_sha = os.environ.get("COMMIT_SHA", "")
|
||||
commit_msg = os.environ.get("COMMIT_MESSAGE", "")
|
||||
@ -566,7 +656,6 @@ def main() -> int:
|
||||
run_id=run_id,
|
||||
pr_number=pr_number,
|
||||
run_url=run_url,
|
||||
review_statuses_json=review_statuses_json,
|
||||
commit_info=commit_info,
|
||||
interval=args.interval,
|
||||
timeout=args.timeout,
|
||||
|
||||
@ -1,14 +1,17 @@
|
||||
"""Tests for scripts/ci/live_comment.py — classify_jobs().
|
||||
"""Tests for scripts/ci/live_comment.py — classify_jobs() + artifact helpers.
|
||||
|
||||
The poller's core logic is a pure function: take raw GitHub API job dicts
|
||||
and split them into (completed, pending). The API wrapper + polling loop
|
||||
are tested via E2E in CI, not here.
|
||||
and split them into (completed, pending). Artifact parsing helpers are also
|
||||
pure and tested here. The API wrapper + polling loop are tested via E2E
|
||||
in CI, not here.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import json
|
||||
import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "live_comment.py"
|
||||
@ -172,3 +175,85 @@ def test_commit_info_uses_past_tense_after_jobs_complete():
|
||||
assert _mod._commit_info_for_state(info, []) == (
|
||||
"<sub>ran on [abc1234](https://commit-url) — fix: thing</sub>"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Artifact parsing helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_parse_status_file_with_prefix():
|
||||
"""GITHUB_OUTPUT format: review_status=<json>"""
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f:
|
||||
f.write('review_status=[{"source":"test","results":[]}]')
|
||||
f.flush()
|
||||
statuses = _mod._parse_status_file(Path(f.name))
|
||||
assert len(statuses) == 1
|
||||
assert statuses[0]["source"] == "test"
|
||||
|
||||
|
||||
def test_parse_status_file_without_prefix():
|
||||
"""Raw JSON (no review_status= prefix) is also accepted."""
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f:
|
||||
f.write('[{"source":"raw","results":[]}]')
|
||||
f.flush()
|
||||
statuses = _mod._parse_status_file(Path(f.name))
|
||||
assert len(statuses) == 1
|
||||
assert statuses[0]["source"] == "raw"
|
||||
|
||||
|
||||
def test_parse_status_file_empty_array():
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f:
|
||||
f.write('review_status=[]')
|
||||
f.flush()
|
||||
statuses = _mod._parse_status_file(Path(f.name))
|
||||
assert statuses == []
|
||||
|
||||
|
||||
def test_parse_status_file_invalid_json():
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f:
|
||||
f.write('review_status=not json')
|
||||
f.flush()
|
||||
statuses = _mod._parse_status_file(Path(f.name))
|
||||
assert statuses == []
|
||||
|
||||
|
||||
def test_parse_status_file_nonexistent():
|
||||
assert _mod._parse_status_file(Path("/nonexistent/file.json")) == []
|
||||
|
||||
|
||||
def test_parse_status_file_not_a_list():
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f:
|
||||
f.write('review_status={"not":"a list"}')
|
||||
f.flush()
|
||||
statuses = _mod._parse_status_file(Path(f.name))
|
||||
assert statuses == []
|
||||
|
||||
|
||||
def test_merge_statuses_empty():
|
||||
assert _mod._merge_statuses([]) == ""
|
||||
|
||||
|
||||
def test_merge_statuses_single():
|
||||
statuses = [{"source": "a", "results": []}]
|
||||
result = _mod._merge_statuses(statuses)
|
||||
assert json.loads(result) == statuses
|
||||
|
||||
|
||||
def test_merge_statuses_multiple():
|
||||
statuses = [
|
||||
{"source": "a", "results": []},
|
||||
{"source": "b", "results": [{"kind": "warning"}]},
|
||||
]
|
||||
result = _mod._merge_statuses(statuses)
|
||||
assert json.loads(result) == statuses
|
||||
|
||||
|
||||
def test_review_status_artifact_prefix():
|
||||
"""The prefix is used to filter artifacts from the API."""
|
||||
assert _mod._REVIEW_STATUS_ARTIFACT_PREFIX == "review-status-"
|
||||
# Artifacts with this prefix should be picked up
|
||||
assert "review-status-ci-timings".startswith(_mod._REVIEW_STATUS_ARTIFACT_PREFIX)
|
||||
assert "review-status-review-labels".startswith(_mod._REVIEW_STATUS_ARTIFACT_PREFIX)
|
||||
# Artifacts without it should not
|
||||
assert not "ci-timings-report".startswith(_mod._REVIEW_STATUS_ARTIFACT_PREFIX)
|
||||
assert not "playwright-report".startswith(_mod._REVIEW_STATUS_ARTIFACT_PREFIX)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user