ci: move the review comment and the image build out of the CI run
The CI run stayed in progress until its last job ended. Two advisory jobs set that time: the review-comment poller (40 minutes) and the Docker image build (45 minutes). Neither job was required to merge. GitHub refuses `gh run rerun` on a run that is in progress. Thus a reviewer who added the `ci-reviewed` label had to wait for the two slow jobs, and label-rerun.yml carried a 2100-second wait loop for this reason. The fast required jobs were ready long before. Each slow job now runs in its own workflow: - docker.yml owns its `pull_request` trigger and does its own change detection. The new `detect` job runs the same composite action with the same condition that ci.yml applied, so a tests-only PR still skips the build. The `workflow_call` trigger is gone. - ci-review-comment.yml starts on `workflow_run` when CI starts. It reads the workflow and the scripts from the default branch, which is the trust boundary that the old job got from its `ref: default_branch` checkout. The poller reads job results through the API, so it can report on a run that it does not belong to. `WATCH_WORKFLOWS` names sibling workflows for the same commit, and `select_watched_runs` keeps the newest run for each name. Thus the comment still shows the Docker results. The list is newline-separated, because a workflow name can contain a comma. The poller always exits 0 now. It reports on the CI run from a different run, so a failed CI job is not a failure of the poller. The CI run has its own gate for that. Also correct a parse error in label-rerun.yml. STATUS came from the already truncated RUN_ID, so its value was the run id and never "completed". Thus the wait branch always ran. ci.yml no longer needs `packages: write`, because the image build has left.
This commit is contained in:
69
.github/workflows/ci-review-comment.yml
vendored
Normal file
69
.github/workflows/ci-review-comment.yml
vendored
Normal file
@@ -0,0 +1,69 @@
|
||||
name: CI review comment
|
||||
|
||||
# Live-updating PR review comment.
|
||||
#
|
||||
# The poller runs for up to 40 minutes.
|
||||
# This run lives in its own workflow.
|
||||
# A run stays in progress until its last job ends, and GitHub refuses
|
||||
# ``gh run rerun`` on a run that is in progress.
|
||||
#
|
||||
# ``workflow_run`` starts this when CI starts. It always reads the workflow
|
||||
# and the scripts from the default branch, never from the PR head.
|
||||
# It makes a write token safe here.
|
||||
#
|
||||
# The poller reads job results through the API. Thus it watches the CI run
|
||||
# and the separate docker run, and it depends on neither.
|
||||
|
||||
on:
|
||||
workflow_run:
|
||||
workflows: [CI]
|
||||
types: [requested]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
actions: read
|
||||
pull-requests: write
|
||||
|
||||
# One poller per CI run. A new push starts a new CI run, and its poller
|
||||
# cancels the poller of the run that GitHub superseded.
|
||||
concurrency:
|
||||
group: ci-review-comment-${{ github.event.workflow_run.head_branch }}
|
||||
cancel-in-progress: true
|
||||
|
||||
jobs:
|
||||
comment:
|
||||
name: CI review comment (live)
|
||||
# Fork PRs get no comment: the poller needs a write token, and the
|
||||
# ``pull_requests`` payload is empty for a fork run.
|
||||
if: >-
|
||||
github.event.workflow_run.event == 'pull_request' &&
|
||||
github.event.workflow_run.head_repository.full_name == github.repository
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 60
|
||||
steps:
|
||||
- name: Checkout trusted default branch
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
ref: ${{ github.event.repository.default_branch }}
|
||||
persist-credentials: false
|
||||
|
||||
- name: Run live comment poller
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
GITHUB_REPOSITORY: ${{ github.repository }}
|
||||
# The CI run to report on — not this run.
|
||||
GITHUB_RUN_ID: ${{ github.event.workflow_run.id }}
|
||||
# Sibling runs for the same commit that the comment also covers,
|
||||
# one workflow name per line (a name can contain a comma).
|
||||
# The poller resolves each name to its runs through the API.
|
||||
WATCH_WORKFLOWS: |
|
||||
Docker Build, Test, and Publish
|
||||
PR_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }}
|
||||
RUN_URL: ${{ github.event.workflow_run.html_url }}
|
||||
# Commit info for the review comment header.
|
||||
COMMIT_SHA: ${{ github.event.workflow_run.head_sha }}
|
||||
COMMIT_MESSAGE: ${{ github.event.workflow_run.head_commit.message }}
|
||||
run: |
|
||||
python3 scripts/ci/live_comment.py \
|
||||
--interval 15 \
|
||||
--timeout 3000
|
||||
61
.github/workflows/ci.yml
vendored
61
.github/workflows/ci.yml
vendored
@@ -24,7 +24,6 @@ permissions:
|
||||
pull-requests: write # needed by lint (PR comment) + supply-chain review_status
|
||||
actions: read # needed by osv-scanner (SARIF upload)
|
||||
security-events: write # needed by osv-scanner (SARIF upload)
|
||||
packages: write # needed by docker build
|
||||
|
||||
concurrency:
|
||||
group: ci-${{ github.ref }}
|
||||
@@ -153,17 +152,6 @@ jobs:
|
||||
if: needs.detect.outputs.docker_meta == 'true'
|
||||
uses: ./.github/workflows/docker-lint.yml
|
||||
|
||||
docker:
|
||||
name: Build&Test Docker image
|
||||
needs: detect
|
||||
# Trusted main pushes run docker.yml directly so its container-publish
|
||||
# environment secrets never cross this reusable-workflow call. PR runs
|
||||
# remain build/test-only and secret-free. Gated on python_prod (not
|
||||
# python): the image copies installed code, never tests/ — tests-only
|
||||
# PRs skip the build.
|
||||
if: needs.detect.outputs.event_name == 'pull_request' && (needs.detect.outputs.python_prod == 'true' || needs.detect.outputs.frontend == 'true' || needs.detect.outputs.docker_meta == 'true')
|
||||
uses: ./.github/workflows/docker.yml
|
||||
|
||||
supply-chain:
|
||||
name: Supply-chain scan
|
||||
needs: detect
|
||||
@@ -189,46 +177,6 @@ jobs:
|
||||
name: OSV scan
|
||||
uses: ./.github/workflows/osv-scanner.yml
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
# Live-updating PR review comment.
|
||||
#
|
||||
# A single ``comment-live`` job polls the GitHub Actions API every 15s
|
||||
# for job statuses in this run, re-assembles the review comment from
|
||||
# whatever results are available, and upserts it via the
|
||||
# ``<!-- hermes-ci-review-bot -->`` marker.
|
||||
#
|
||||
# When the visible job set goes quiet, the poller waits 10 seconds and polls
|
||||
# once more so downstream jobs created by an aggregate gate get included.
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
comment-live:
|
||||
name: CI review comment (live)
|
||||
needs: [detect, review-labels, lockfile-diff, supply-chain, osv-scanner, uv-lockfile, history-check, contributor-check, e2e-desktop]
|
||||
if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork != true
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 40
|
||||
steps:
|
||||
- name: Checkout code
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
ref: ${{ github.event.repository.default_branch }}
|
||||
persist-credentials: false
|
||||
|
||||
- name: Run live comment poller
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
GITHUB_REPOSITORY: ${{ github.repository }}
|
||||
GITHUB_RUN_ID: ${{ github.run_id }}
|
||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
|
||||
# Commit info for the review comment header.
|
||||
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 }}
|
||||
run: |
|
||||
python3 scripts/ci/live_comment.py \
|
||||
--interval 15 \
|
||||
--timeout 2100
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
# Gate: runs after everything. ``if: always()`` ensures it reports a
|
||||
# status even when some deps were skipped. Only actual ``failure``
|
||||
@@ -257,9 +205,10 @@ jobs:
|
||||
- supply-chain
|
||||
- review-labels
|
||||
- osv-scanner
|
||||
# comment-live is a polling job — it doesn't block the gate.
|
||||
# we don't require docker to pass rn because it's so slow lol
|
||||
# - docker
|
||||
# The image build runs in its own workflow (docker.yml) and reports
|
||||
# its own check. It was never required here, because it is too slow
|
||||
# to block a merge. A separate run also stops it from holding this
|
||||
# run open. That is what blocked ``gh run rerun``.
|
||||
if: always()
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 10
|
||||
@@ -302,7 +251,7 @@ jobs:
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
ci-timings:
|
||||
name: CI timing report
|
||||
needs: [all-checks-pass, docker]
|
||||
needs: [all-checks-pass]
|
||||
if: always()
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 10
|
||||
|
||||
53
.github/workflows/docker.yml
vendored
53
.github/workflows/docker.yml
vendored
@@ -1,16 +1,19 @@
|
||||
name: Docker Build, Test, and Publish
|
||||
|
||||
on:
|
||||
# Trusted main pushes run this workflow directly so environment-scoped
|
||||
# Docker Hub secrets are resolved by the top-level workflow, never across
|
||||
# a reusable-workflow boundary.
|
||||
# This workflow owns its own triggers. ci.yml does not call it.
|
||||
# A reusable-workflow call eeps the caller run in progress for that full time.
|
||||
# GitHub refuses ``gh run rerun`` on a run that is still in progress.
|
||||
# Thus one slow advisory job blocked every rerun of the fast required jobs. A separate
|
||||
# run reruns and cancels independently.
|
||||
#
|
||||
# Trusted main pushes resolve the environment-scoped Docker Hub secrets in
|
||||
# this same workflow, never across a workflow boundary.
|
||||
pull_request:
|
||||
push:
|
||||
branches: [main]
|
||||
release:
|
||||
types: [published]
|
||||
# CI calls this only for untrusted PR build/test coverage. Those runs never
|
||||
# reach the protected publish or merge jobs below.
|
||||
workflow_call:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
@@ -27,11 +30,47 @@ env:
|
||||
IMAGE_NAME: nousresearch/hermes-agent
|
||||
|
||||
jobs:
|
||||
# Classify the PR's changed files. ci.yml used to gate the docker call on
|
||||
# its own ``detect`` outputs; now that this workflow triggers itself, it
|
||||
# runs the same composite action. On push and release the classifier fails
|
||||
# open (every lane true), so post-merge validation is never weakened.
|
||||
detect:
|
||||
name: Detect affected areas
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 10
|
||||
outputs:
|
||||
build: ${{ steps.gate.outputs.build }}
|
||||
steps:
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
- name: Detect affected areas
|
||||
id: classify
|
||||
uses: ./.github/actions/detect-changes
|
||||
with:
|
||||
github-token: ${{ github.token }}
|
||||
|
||||
- name: Decide whether to build
|
||||
id: gate
|
||||
env:
|
||||
# python_prod (not python): the image copies installed code, never
|
||||
# tests/, so tests-only PRs skip the build.
|
||||
PYTHON_PROD: ${{ steps.classify.outputs.python_prod }}
|
||||
FRONTEND: ${{ steps.classify.outputs.frontend }}
|
||||
DOCKER_META: ${{ steps.classify.outputs.docker_meta }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
if [ "$PYTHON_PROD" = "true" ] || [ "$FRONTEND" = "true" ] || [ "$DOCKER_META" = "true" ]; then
|
||||
echo "build=true" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
echo "build=false" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
|
||||
# Build and test the image for each architecture. This job runs PR code,
|
||||
# so it must remain secret-free. Publishing happens in the separate,
|
||||
# protected publish job after these tests pass.
|
||||
build:
|
||||
if: github.repository == 'NousResearch/hermes-agent'
|
||||
needs: [detect]
|
||||
if: github.repository == 'NousResearch/hermes-agent' && needs.detect.outputs.build == 'true'
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
|
||||
28
.github/workflows/label-rerun.yml
vendored
28
.github/workflows/label-rerun.yml
vendored
@@ -2,13 +2,16 @@ name: Label rerun
|
||||
|
||||
# When the ``ci-reviewed`` label is added to a PR, rerun all failed jobs in
|
||||
# the latest CI run. This re-evaluates ``review-labels`` (which now sees the
|
||||
# label) and GitHub automatically reruns dependent jobs (``comment-live``,
|
||||
# ``all-checks-pass``) — so the review comment gets updated too.
|
||||
# label) and GitHub reruns the dependent ``all-checks-pass`` gate.
|
||||
#
|
||||
# If the CI run is still in progress when the label is added, we wait for it
|
||||
# to finish before rerunning (``gh run rerun`` only works on completed runs).
|
||||
# The wait can be long (20+ min for a full CI run), but it's better than
|
||||
# silently failing and leaving the reviewer stuck.
|
||||
# ``gh run rerun`` only works on a completed run. Thus this waits when the
|
||||
# run is still in progress. The wait is now short. The two slowest jobs are
|
||||
# the 40-minute comment poller and the 45-minute image build. Each one moved
|
||||
# to its own workflow, so a CI run ends when its required jobs end.
|
||||
#
|
||||
# The review comment updates without help. The poller in
|
||||
# ci-review-comment.yml watches the CI run through the API. It gets the
|
||||
# rerun results.
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
@@ -38,7 +41,7 @@ jobs:
|
||||
set -uo pipefail
|
||||
|
||||
# Find the latest CI run for this PR's head SHA.
|
||||
RUN_ID=$(gh run list \
|
||||
RUN_INFO=$(gh run list \
|
||||
--repo "$REPO" \
|
||||
--commit "$HEAD_SHA" \
|
||||
--workflow ci.yml \
|
||||
@@ -46,14 +49,17 @@ jobs:
|
||||
--json databaseId,status \
|
||||
--jq '.[0] | "\(.databaseId) \(.status)"' 2>/dev/null || true)
|
||||
|
||||
if [ -z "$RUN_ID" ]; then
|
||||
if [ -z "$RUN_INFO" ]; then
|
||||
echo "No CI run found for this PR — nothing to rerun."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Split "RUN_ID STATUS" into two vars.
|
||||
RUN_ID="${RUN_ID%% *}"
|
||||
STATUS="${RUN_ID##* }"
|
||||
# Split "RUN_ID STATUS" into two vars. Read STATUS from RUN_INFO,
|
||||
# not from the truncated RUN_ID. Both values came from the same
|
||||
# var before, which made STATUS the run id. Thus the wait branch
|
||||
# always ran.
|
||||
RUN_ID="${RUN_INFO%% *}"
|
||||
STATUS="${RUN_INFO##* }"
|
||||
|
||||
echo "Latest CI run: $RUN_ID (status: $STATUS)"
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Live-updating CI review comment.
|
||||
|
||||
Polls the GitHub Actions API for job statuses in the current run, assembles
|
||||
Polls the GitHub Actions API for job statuses in the CI run, assembles
|
||||
the review comment from whatever results are available, and upserts it as a
|
||||
PR comment. Repeats every ``--interval`` seconds until all jobs are
|
||||
completed (or ``--timeout`` is reached), so the comment updates in real time
|
||||
@@ -11,6 +11,13 @@ The comment is identified by the ``<!-- hermes-ci-review-bot -->`` marker
|
||||
— the same one ``assemble_review_comment.py`` uses — so it replaces any
|
||||
previous comment from an earlier run.
|
||||
|
||||
This runs from ``.github/workflows/ci-review-comment.yml``, a separate
|
||||
``workflow_run`` workflow. Thus ``GITHUB_RUN_ID`` names the CI run to report
|
||||
on, not the run that contains this script. The poller reports on runs that
|
||||
it does not belong to. This is also how it covers a workflow that CI does
|
||||
not contain: ``WATCH_WORKFLOWS`` names sibling workflows that the same
|
||||
commit triggered (the Docker image build). Their jobs join the comment.
|
||||
|
||||
Architecture:
|
||||
|
||||
- :func:`classify_jobs` (pure, testable) — takes a list of raw API job
|
||||
@@ -18,10 +25,13 @@ Architecture:
|
||||
is a ``{name: result}`` dict (for :func:`assemble_review_comment.assemble`)
|
||||
and ``pending`` is a list of job names still running.
|
||||
|
||||
- :func:`select_watched_runs` (pure, testable) — picks the sibling runs
|
||||
to merge in, newest attempt per workflow.
|
||||
|
||||
- :func:`find_comment_id` / :func:`upsert_comment` — thin API wrappers.
|
||||
|
||||
- :func:`fetch_all_review_statuses` — lists all ``review-status-*``
|
||||
artifacts on the orchestrator run (GitHub attaches reusable-workflow
|
||||
artifacts on the CI run (GitHub attaches reusable-workflow
|
||||
artifacts to the caller run), downloads each, parses the
|
||||
``review_status=`` line from ``review-status.json``, and merges into
|
||||
one array. Recomputed from source every poll cycle, so statuses
|
||||
@@ -172,53 +182,87 @@ def _api_get_paginated(url: str, token: str, list_key: str | None = None) -> lis
|
||||
return results
|
||||
|
||||
|
||||
def collect_run_jobs(token: str, repo: str, run_id: str) -> list[dict]:
|
||||
"""Collect all jobs in the orchestrator run + sub-workflow runs.
|
||||
def select_watched_runs(
|
||||
runs: list[dict], watch_names: list[str], exclude_run_id: str = "",
|
||||
) -> list[dict]:
|
||||
"""Pick the sibling runs whose jobs belong in the comment.
|
||||
|
||||
``runs`` is the API's run list for one commit. ``watch_names`` holds
|
||||
workflow names from ``WATCH_WORKFLOWS``. One commit can have more than
|
||||
one run of the same workflow, after a rerun or a new push. Thus this
|
||||
keeps only the newest run for each workflow name. An older attempt
|
||||
reports results that a rerun replaced.
|
||||
|
||||
``exclude_run_id`` removes the CI run itself when its name is also in
|
||||
``watch_names``.
|
||||
"""
|
||||
newest: dict[str, dict] = {}
|
||||
wanted = {n.strip() for n in watch_names if n.strip()}
|
||||
|
||||
for candidate in runs:
|
||||
name = str(candidate.get("name", ""))
|
||||
if name not in wanted:
|
||||
continue
|
||||
if exclude_run_id and str(candidate.get("id", "")) == str(exclude_run_id):
|
||||
continue
|
||||
current = newest.get(name)
|
||||
if current is None or str(candidate.get("created_at", "")) > str(current.get("created_at", "")):
|
||||
newest[name] = candidate
|
||||
|
||||
return list(newest.values())
|
||||
|
||||
|
||||
def collect_run_jobs(
|
||||
token: str, repo: str, run_id: str, watch_workflows: list[str] | None = None,
|
||||
) -> list[dict]:
|
||||
"""Collect all jobs in the CI run + any watched sibling runs.
|
||||
|
||||
Returns a flat list of job dicts (same shape as the API returns, plus
|
||||
``_workflow_name`` on sub-workflow jobs).
|
||||
``_workflow_name`` on jobs from a watched run).
|
||||
|
||||
Reusable-workflow (``workflow_call``) jobs need no special handling:
|
||||
GitHub flattens them into the caller run's job list, already named
|
||||
``\"Workflow / job\"``. Watched runs are separate top-level runs
|
||||
(the Docker image build), so their jobs are fetched per run and
|
||||
prefixed here.
|
||||
"""
|
||||
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", "")
|
||||
|
||||
# Orchestrator jobs
|
||||
# CI run jobs (includes every reusable-workflow job).
|
||||
all_jobs: list[dict] = []
|
||||
orch_jobs = _api_get_paginated(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs/{run_id}/jobs",
|
||||
token, list_key="jobs",
|
||||
)
|
||||
|
||||
# Sub-workflow runs (workflow_call)
|
||||
sub_runs = _api_get_paginated(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs?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]
|
||||
|
||||
all_jobs: list[dict] = []
|
||||
# Orchestrator jobs: skip workflow-call placeholder steps (they're
|
||||
# sub-workflow triggers, not review signal), but KEEP in_progress /
|
||||
# queued jobs so the poller knows they're still running.
|
||||
# Skip workflow-call placeholder steps (they're sub-workflow triggers,
|
||||
# not review signal), but KEEP in_progress / queued jobs so the poller
|
||||
# knows they're still running.
|
||||
for job in orch_jobs:
|
||||
steps = job.get("steps") or []
|
||||
if any(s.get("name", "").startswith("Run ./.github/workflows/") for s in steps):
|
||||
continue
|
||||
all_jobs.append(job)
|
||||
|
||||
# Sub-workflow jobs (workflow_call).
|
||||
# These runs may not exist yet on the first few polls — that's fine,
|
||||
# classify_jobs() will just show 0 pending for them.
|
||||
for sr in sub_runs:
|
||||
sr_id = sr["id"]
|
||||
sr_name = sr.get("name", "")
|
||||
sr_jobs = _api_get_paginated(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs/{sr_id}/jobs",
|
||||
if not watch_workflows or not head_sha:
|
||||
return all_jobs
|
||||
|
||||
# Watched sibling runs for the same commit. A run can be absent on the
|
||||
# first polls. Then classify_jobs() shows nothing for it.
|
||||
sibling_runs = _api_get_paginated(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs?head_sha={head_sha}&per_page=100",
|
||||
token, list_key="workflow_runs",
|
||||
)
|
||||
for watched in select_watched_runs(sibling_runs, watch_workflows, exclude_run_id=run_id):
|
||||
watched_jobs = _api_get_paginated(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs/{watched['id']}/jobs",
|
||||
token, list_key="jobs",
|
||||
)
|
||||
for j in sr_jobs:
|
||||
j["_workflow_name"] = sr_name
|
||||
all_jobs.append(j)
|
||||
for job in watched_jobs:
|
||||
job["_workflow_name"] = watched.get("name", "")
|
||||
all_jobs.append(job)
|
||||
|
||||
return all_jobs
|
||||
|
||||
@@ -482,12 +526,13 @@ def run(
|
||||
interval: int = 15,
|
||||
timeout: int = 1800,
|
||||
dry_run: bool = False,
|
||||
watch_workflows: list[str] | None = None,
|
||||
) -> int:
|
||||
"""Poll for job statuses and update the PR comment until all done.
|
||||
|
||||
Returns 0 on success; 1 when all jobs completed but a dependency
|
||||
failed (so ``gh run rerun --failed`` can pick it up). Comment posting
|
||||
is best-effort.
|
||||
Always returns 0. The poller reports on the CI run from a different run.
|
||||
Thus a failed CI job is not a failure of this job. The CI run has its
|
||||
own gate, which reports that. Comment posting is best-effort.
|
||||
"""
|
||||
asm = _import_assembler()
|
||||
start = time.time()
|
||||
@@ -504,7 +549,7 @@ def run(
|
||||
break
|
||||
|
||||
try:
|
||||
jobs = collect_run_jobs(token, repo, run_id)
|
||||
jobs = collect_run_jobs(token, repo, run_id, watch_workflows)
|
||||
except Exception as e:
|
||||
print(f" API error collecting jobs: {e}", file=sys.stderr)
|
||||
time.sleep(interval)
|
||||
@@ -588,15 +633,11 @@ def run(
|
||||
continue
|
||||
|
||||
if not pending:
|
||||
# Check if any dependency failed. If so, exit non-zero so the
|
||||
# run shows as failed — this lets ``gh run rerun --failed``
|
||||
# (e.g. from label-rerun.yml) pick up and rerun the failed jobs.
|
||||
failed_deps = [name for name, result in completed.items() if result == "failure"]
|
||||
if failed_deps:
|
||||
print(f" All jobs done, but {len(failed_deps)} failed: {', '.join(failed_deps)}")
|
||||
print(" Exiting with error so the run can be rerun via --failed.")
|
||||
return 1
|
||||
print(" All jobs completed — done.")
|
||||
failed = [name for name, result in completed.items() if result == "failure"]
|
||||
if failed:
|
||||
print(f" All jobs done, {len(failed)} failed: {', '.join(failed)}")
|
||||
else:
|
||||
print(" All jobs completed — done.")
|
||||
break
|
||||
|
||||
quiet_grace_used = False
|
||||
@@ -605,6 +646,38 @@ def run(
|
||||
return 0
|
||||
|
||||
|
||||
def parse_watch_workflows(raw: str) -> list[str]:
|
||||
"""Parse the ``WATCH_WORKFLOWS`` value into workflow names.
|
||||
|
||||
One name per line. Not comma-separated: a workflow name can contain a
|
||||
comma ("Docker Build, Test, and Publish").
|
||||
"""
|
||||
return [name.strip() for name in raw.splitlines() if name.strip()]
|
||||
|
||||
|
||||
def resolve_pr_number(token: str, repo: str, head_sha: str) -> str:
|
||||
"""Find the PR number for a commit when the event payload has none.
|
||||
|
||||
``workflow_run.pull_requests`` is empty for some runs. The poller has no
|
||||
comment to post without a number.
|
||||
"""
|
||||
if not head_sha:
|
||||
return ""
|
||||
owner, repo_name = repo.split("/")
|
||||
try:
|
||||
results = _api_get_paginated(
|
||||
f"{API_BASE}/repos/{owner}/{repo_name}/commits/{head_sha}/pulls",
|
||||
token,
|
||||
)
|
||||
except Exception as e:
|
||||
print(f" API error resolving PR number: {e}", file=sys.stderr)
|
||||
return ""
|
||||
for item in results:
|
||||
if isinstance(item, dict) and item.get("state") == "open":
|
||||
return str(item.get("number", ""))
|
||||
return ""
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--interval", type=int, default=15,
|
||||
@@ -621,6 +694,10 @@ def main() -> int:
|
||||
pr_number = os.environ.get("PR_NUMBER", "")
|
||||
run_url = os.environ.get("RUN_URL", "")
|
||||
|
||||
# Sibling workflows to merge into the comment, one name per line. Their
|
||||
# runs are separate from the CI run, so the poller resolves them by name.
|
||||
watch_workflows = parse_watch_workflows(os.environ.get("WATCH_WORKFLOWS", ""))
|
||||
|
||||
if not args.dry_run:
|
||||
if not token:
|
||||
print("GITHUB_TOKEN is required", file=sys.stderr)
|
||||
@@ -631,14 +708,23 @@ def main() -> int:
|
||||
if not run_id:
|
||||
print("GITHUB_RUN_ID is required", file=sys.stderr)
|
||||
return 1
|
||||
if not pr_number:
|
||||
print("PR_NUMBER is required", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
# Build commit info line from env vars (set by ci.yml).
|
||||
# Build commit info line from env vars (set by ci-review-comment.yml).
|
||||
commit_sha = os.environ.get("COMMIT_SHA", "")
|
||||
commit_msg = os.environ.get("COMMIT_MESSAGE", "")
|
||||
|
||||
if not pr_number and not args.dry_run:
|
||||
pr_number = resolve_pr_number(token, repo, commit_sha)
|
||||
if not pr_number:
|
||||
print("No PR number found — nothing to comment on.", file=sys.stderr)
|
||||
return 0
|
||||
print(f"Resolved PR #{pr_number} from commit {commit_sha[:7]}")
|
||||
|
||||
commit_url = os.environ.get("COMMIT_URL", "")
|
||||
if not commit_url and commit_sha and pr_number:
|
||||
server = os.environ.get("GITHUB_SERVER_URL", "https://github.com")
|
||||
commit_url = f"{server}/{repo}/pull/{pr_number}/commits/{commit_sha}"
|
||||
|
||||
commit_info = ""
|
||||
if commit_sha:
|
||||
short_sha = commit_sha[:7]
|
||||
@@ -664,6 +750,7 @@ def main() -> int:
|
||||
interval=args.interval,
|
||||
timeout=args.timeout,
|
||||
dry_run=args.dry_run,
|
||||
watch_workflows=watch_workflows,
|
||||
)
|
||||
|
||||
|
||||
|
||||
115
tests/ci/test_live_comment.py
Normal file
115
tests/ci/test_live_comment.py
Normal file
@@ -0,0 +1,115 @@
|
||||
"""Tests for scripts/ci/live_comment.py run selection.
|
||||
|
||||
The poller now reports on a run it is not part of, and merges jobs from
|
||||
sibling runs of the same commit (the Docker image build, which left ci.yml
|
||||
to stop holding the CI run open). ``select_watched_runs`` decides which
|
||||
sibling runs count.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "live_comment.py"
|
||||
_spec = importlib.util.spec_from_file_location("live_comment", _PATH)
|
||||
if _spec is None or _spec.loader is None:
|
||||
raise ImportError("Failed to load live_comment.py")
|
||||
_mod = importlib.util.module_from_spec(_spec)
|
||||
sys.modules["live_comment"] = _mod
|
||||
_spec.loader.exec_module(_mod)
|
||||
|
||||
select_watched_runs = _mod.select_watched_runs
|
||||
classify_jobs = _mod.classify_jobs
|
||||
|
||||
DOCKER = "Docker Build, Test, and Publish"
|
||||
|
||||
|
||||
def _run(run_id: int, name: str, created_at: str) -> dict:
|
||||
return {"id": run_id, "name": name, "created_at": created_at}
|
||||
|
||||
|
||||
def test_selects_only_named_workflows():
|
||||
runs = [
|
||||
_run(1, DOCKER, "2026-08-08T10:00:00Z"),
|
||||
_run(2, "Deploy site", "2026-08-08T10:00:00Z"),
|
||||
_run(3, "CI", "2026-08-08T10:00:00Z"),
|
||||
]
|
||||
selected = select_watched_runs(runs, [DOCKER])
|
||||
assert [r["id"] for r in selected] == [1]
|
||||
|
||||
|
||||
def test_keeps_newest_attempt_per_workflow():
|
||||
"""A rerun makes a second run for the same commit; the old one is stale."""
|
||||
runs = [
|
||||
_run(1, DOCKER, "2026-08-08T10:00:00Z"),
|
||||
_run(2, DOCKER, "2026-08-08T11:30:00Z"),
|
||||
]
|
||||
selected = select_watched_runs(runs, [DOCKER])
|
||||
assert [r["id"] for r in selected] == [2]
|
||||
|
||||
|
||||
def test_excludes_the_ci_run_itself():
|
||||
runs = [_run(7, "CI", "2026-08-08T10:00:00Z")]
|
||||
assert select_watched_runs(runs, ["CI"], exclude_run_id="7") == []
|
||||
assert len(select_watched_runs(runs, ["CI"], exclude_run_id="8")) == 1
|
||||
|
||||
|
||||
def test_no_watch_names_selects_nothing():
|
||||
runs = [_run(1, DOCKER, "2026-08-08T10:00:00Z")]
|
||||
assert select_watched_runs(runs, []) == []
|
||||
assert select_watched_runs(runs, [""]) == []
|
||||
|
||||
|
||||
def test_watched_run_jobs_carry_the_workflow_name_into_the_comment():
|
||||
"""A watched run's jobs must stay distinguishable from CI's own jobs."""
|
||||
jobs = [
|
||||
{"name": "build (amd64)", "status": "completed", "conclusion": "failure",
|
||||
"html_url": "https://example/1", "_workflow_name": DOCKER},
|
||||
{"name": "Python tests", "status": "completed", "conclusion": "success",
|
||||
"html_url": "https://example/2"},
|
||||
]
|
||||
completed, pending, job_urls = classify_jobs(jobs)
|
||||
assert completed[f"{DOCKER} / build (amd64)"] == "failure"
|
||||
assert completed["Python tests"] == "success"
|
||||
assert pending == []
|
||||
assert job_urls[f"{DOCKER} / build (amd64)"] == "https://example/1"
|
||||
|
||||
|
||||
def test_parse_watch_workflows_keeps_commas_inside_a_name():
|
||||
"""Workflow names contain commas, so the list is newline-separated."""
|
||||
assert _mod.parse_watch_workflows("Docker Build, Test, and Publish\n") == [
|
||||
"Docker Build, Test, and Publish"
|
||||
]
|
||||
assert _mod.parse_watch_workflows("A\nB\n\n C \n") == ["A", "B", "C"]
|
||||
assert _mod.parse_watch_workflows("") == []
|
||||
|
||||
|
||||
def test_workflow_watch_list_names_a_workflow_that_exists():
|
||||
"""The names the workflow passes must match real workflow ``name:`` values.
|
||||
|
||||
A name that matches nothing makes the poller silently drop that run
|
||||
from the comment, which no unit test on its own would notice.
|
||||
"""
|
||||
yaml = pytest.importorskip("yaml")
|
||||
root = Path(__file__).resolve().parents[2]
|
||||
caller = yaml.safe_load(
|
||||
(root / ".github/workflows/ci-review-comment.yml").read_text(encoding="utf-8")
|
||||
)
|
||||
step = next(
|
||||
s for s in caller["jobs"]["comment"]["steps"]
|
||||
if "WATCH_WORKFLOWS" in (s.get("env") or {})
|
||||
)
|
||||
watched = _mod.parse_watch_workflows(step["env"]["WATCH_WORKFLOWS"])
|
||||
assert watched, "the poller is watching nothing"
|
||||
|
||||
known = set()
|
||||
for path in (root / ".github/workflows").glob("*.yml"):
|
||||
doc = yaml.safe_load(path.read_text(encoding="utf-8"))
|
||||
if isinstance(doc, dict) and isinstance(doc.get("name"), str):
|
||||
known.add(doc["name"])
|
||||
|
||||
assert set(watched) <= known, f"unknown workflow names: {set(watched) - known}"
|
||||
Reference in New Issue
Block a user