mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-22 16:25:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

ci: live-updating PR review comment with structured job statuses

Browse source 
Replace the static comment-pending + comment-results two-job pattern
with a live-updating comment system that polls the GitHub Actions API
every 15s, re-assembles the review comment from whatever results are
available, and upserts it via the <!-- hermes-ci-review-bot --> marker.
The comment updates in real time as each job finishes — no waiting for
the full pipeline.

Every CI job that wants to appear in the review comment emits a
review_status output — a JSON array of objects, each with a source
and a results array:

    [
      {
        "source": "review-label-gate",
        "results": [
          {"kind": "action_required", "title": "...", "summary": "...",
           "how_to_fix": "..."},
          {"kind": "info", "title": "...", "summary": "..."}
        ]
      },
      {
        "source": "ci timing",
        "results": [
          {"kind": "warning", "title": "CI timings", "summary": "...",
           "detail": "...", "link": "..."}
        ]
      }
    ]

One job can emit multiple results of different kinds. The source field
is used to exclude the corresponding job from the synthesized error
list (case-insensitive, hyphen-normalized matching against GitHub
Actions job display names).

| job                        | source                   | kind (on failure)         | section              |
|----------------------------|--------------------------|---------------------------|----------------------|
| review-labels              | review label gate        | action_required / info    | Action required      |
| lockfile-diff              | lockfile-diff            | action_required           | Action required      |
| ci-timings                 | ci timing                | warning / info            | Warnings             |
| supply-chain scan          | supply chain             | error / (none)            | Job failures         |
| supply-chain dep-bounds    | supply chain             | action_required / (none)  | Action required      |
| osv-scanner                | osv scan                 | warning / (none)          | Warnings             |
| uv-lockfile-check          | uv.lock check            | action_required / (none)  | Action required      |
| history-check              | unrelated histories      | action_required           | Action required      |
| contributor-check          | contributor attribution  | action_required           | Action required      |

Jobs that find nothing emit [] (empty array) — no noise info items.

A single comment-live job polls the GitHub Actions API every 15s,
classifies jobs into (completed, pending), assembles the comment, and
upserts it. Merges review_status outputs from all needs jobs via
toJSON(needs.*.outputs.review_status), and downloads the ci-timings
artifact when it becomes available. Shows commit SHA + message below
the header.

The assembler has ZERO job-specific knowledge. It just:
1. collect_from_statuses() — flattens all nested status objects into ReviewItems
2. collect_failed_jobs() — synthesizes errors for failed jobs with no declared status
3. _attach_job_urls() — fills in per-job log links for ALL items
4. render_comment() — groups by severity, renders with group headers

Each item shows links inline next to the title: View report (job-emitted
URL) and View job (auto-attached logs link). Each info item is its own
collapsible <details> block.

    # ૮ >ﻌ< ა ci review

    running on abc1234 — commit message first line

    ## ❌ Job failures
    ### {title} · [View job](url)
    {summary}

    ## ⚠️ Action required
    ### {title} · [View job](url)
    {summary}
    **How to fix:**
    {how_to_fix}

    ## ⚠️ Warnings
    ### {title} · [View report](url) · [View job](url)
    {summary}
    {detail}

    <details><summary>{title}</summary>
    {content}
    </details>

    Still running 3 jobs: ci-timings, docker

- test_assemble_review_comment.py (48 tests): collect_from_statuses,
  collect_failed_jobs with exclude_sources, _attach_job_urls,
  render_comment (group headers, inline links, commit info, per-item
  details, pending footer), assemble integration
- test_live_comment.py (16 tests): classify_jobs pure function
- test_timings_report.py (10 tests): generate_review_status nested format
- test_lockfile_diff.py (6 tests)
- test_classify_changes.py (32 tests, pre-existing)
This commit is contained in:
ethernet 2026-07-16 18:17:49 -04:00
parent d7b36070ef
commit b9f82ed39f
19 changed files with 2696 additions and 323 deletions
Download patch file Download diff file Expand all files Collapse all files

116
.github/workflows/ci.yml vendored
View file

@ -17,7 +17,7 @@ on:
permissions:
contents: read
pull-requests: write # needed by lint (PR comment) + supply-chain (PR comment)
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
@ -73,11 +73,10 @@ jobs:
lint:
name: Python lints
needs: detect
if: needs.detect.outputs.python == 'true' || needs.detect.outputs.ci_review == 'true'
if: needs.detect.outputs.python == 'true'
uses: ./.github/workflows/lint.yml
with:
event_name: ${{ needs.detect.outputs.event_name }}
ci_review: ${{ needs.detect.outputs.ci_review == 'true' }}
secrets: inherit
js-tests:
@ -144,12 +143,20 @@ jobs:
supply-chain:
name: Supply-chain scan
needs: detect
if: needs.detect.outputs.event_name == 'pull_request' && (needs.detect.outputs.scan == 'true' || needs.detect.outputs.deps == 'true' || needs.detect.outputs.mcp_catalog == 'true')
if: needs.detect.outputs.event_name == 'pull_request' && (needs.detect.outputs.scan == 'true' || needs.detect.outputs.deps == 'true')
uses: ./.github/workflows/supply-chain-audit.yml
with:
event_name: ${{ needs.detect.outputs.event_name }}
scan: ${{ needs.detect.outputs.scan == 'true' }}
deps: ${{ needs.detect.outputs.deps == 'true' }}
review-labels:
name: Review label gate
needs: detect
if: needs.detect.outputs.event_name == 'pull_request' && (needs.detect.outputs.ci_review == 'true' || needs.detect.outputs.mcp_catalog == 'true')
uses: ./.github/workflows/review-labels.yml
with:
ci_review: ${{ needs.detect.outputs.ci_review == 'true' }}
mcp_catalog: ${{ needs.detect.outputs.mcp_catalog == 'true' }}
secrets: inherit
@ -158,12 +165,90 @@ jobs:
uses: ./.github/workflows/osv-scanner.yml
secrets: inherit
# ─────────────────────────────────────────────────────────────────────
# 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.
#
# The poller exits when all non-infra jobs are completed (or on
# timeout). ci-timings' review_status is picked up automatically when
# its artifact becomes available — the poller downloads and merges it.
# ─────────────────────────────────────────────────────────────────────
comment-live:
name: CI review comment (live)
needs: [detect, review-labels, lockfile-diff, supply-chain, osv-scanner, uv-lockfile, history-check, contributor-check]
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
- 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 }}
# 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
# ─────────────────────────────────────────────────────────────────────
# Gate: runs after everything. ``if: always()`` ensures it reports a
# status even when some deps were skipped. Only actual ``failure``
# results cause it to fail; ``skipped`` is treated as success.
#
# Branch protection should require ONLY this check.
#
# Outputs ``needs-json`` — a compact ``{job_name: result}`` dict — so
# the live comment poller can list failed jobs in the PR comment.
# ─────────────────────────────────────────────────────────────────────
all-checks-pass:
name: All required checks pass
@ -179,20 +264,30 @@ jobs:
- lockfile-diff
- docker-lint
- 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
if: always()
runs-on: ubuntu-latest
timeout-minutes: 10
outputs:
needs-json: ${{ steps.evaluate.outputs.needs-json }}
steps:
- name: Evaluate job results
id: evaluate
env:
NEEDS: ${{ toJSON(needs) }}
run: |
echo "$NEEDS" | python3 -c "
import json, sys
needs = json.load(sys.stdin)
# Emit compact {job_name: result} for the comment assembler.
compact = {name: info['result'] for name, info in needs.items()}
print(f'needs-json={json.dumps(compact)}')
with open('$GITHUB_OUTPUT', 'a') as f:
f.write(f'needs-json={json.dumps(compact)}\n')
failed = [name for name, info in needs.items() if info['result'] == 'failure']
for name, info in sorted(needs.items()):
result = info['result']
@ -209,6 +304,9 @@ jobs:
# cache them on main (as a baseline), and on PRs generate an HTML diff
# 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 picks up ci-timings' completion automatically —
# it reads review-status.json from the artifact when the job finishes.
# ─────────────────────────────────────────────────────────────────────
ci-timings:
name: CI timing report
@ -242,18 +340,20 @@ jobs:
--baseline ci-timings-baseline.json \
--output ci-timings-report.html \
--json-out ci-timings.json \
--summary-out ci-timings-summary.md
--summary-out ci-timings-summary.md \
--review-status-out review-status.json
- name: Upload HTML report
- name: Upload HTML report + review status
# Advisory report — artifact-service blips must not fail the job.
continue-on-error: true
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
id: ci-timings-artifact
with:
name: ci-timings-report
path: ci-timings-report.html
path: |
ci-timings-report.html
review-status.json
retention-days: 14
archive: false
- name: Output summary
env:

18
.github/workflows/contributor-check.yml vendored
View file

@ -2,6 +2,10 @@ name: Contributor Attribution Check
on:
workflow_call:
outputs:
review_status:
description: "JSON array of review status objects"
value: ${{ jobs.check-attribution.outputs.review_status }}
permissions:
contents: read
@ -10,12 +14,15 @@ jobs:
check-attribution:
runs-on: ubuntu-latest
timeout-minutes: 10
outputs:
review_status: ${{ steps.check-emails.outputs.review_status }}
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 0 # Full history needed for git log
- name: Check for unmapped contributor emails
id: check-emails
run: |
# Get the merge base between this PR and main
MERGE_BASE=$(git merge-base origin/main HEAD)
@ -25,6 +32,7 @@ jobs:
if [ -z "$NEW_EMAILS" ]; then
echo "No new commits to check."
echo "review_status=[]" >> "$GITHUB_OUTPUT"
exit 0
fi
@ -67,6 +75,16 @@ jobs:
echo ""
echo "To find the GitHub username for an email:"
echo " gh api 'search/users?q=EMAIL+in:email' --jq '.items[0].login'"
# Emit review_status for unmapped emails
DETAIL=$(echo -e "$MISSING" | sed '/^$/d; s/^ //')
HOW_TO_FIX=$'Add mappings to scripts/release.py AUTHOR_MAP:\n```\n"<email>": "<github-username>",\n```\nTo find the GitHub username for an email:\n```\ngh api \'search/users?q=EMAIL+in:email\' --jq \'.items[0].login\'\n```\n'
REVIEW_STATUS=$(jq -nc \
--arg detail "$DETAIL" \
--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"
exit 1
else
echo "✅ All contributor emails are mapped."

12
.github/workflows/history-check.yml vendored
View file

@ -15,6 +15,10 @@ name: History Check
on:
workflow_call:
outputs:
review_status:
description: "JSON array of review_status objects for the synthesizer."
value: ${{ jobs.check-common-ancestor.outputs.review_status }}
permissions:
contents: read
@ -23,18 +27,23 @@ jobs:
check-common-ancestor:
runs-on: ubuntu-latest
timeout-minutes: 10
outputs:
review_status: ${{ steps.merge-base-check.outputs.review_status }}
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 0 # full history both sides for merge-base
- name: Reject PRs with no common ancestor on main
- id: merge-base-check
name: Reject PRs with no common ancestor on main
run: |
# `git merge-base` exits non-zero AND prints nothing when the two
# commits share no ancestor. We check both conditions explicitly
# so the failure message is clear regardless of which signal fires
# first.
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 ""
echo "::error::This PR has no common ancestor with main."
echo ""
@ -56,3 +65,4 @@ jobs:
exit 1
fi
echo "::notice::Common ancestor with main: $BASE"
echo "review_status=[]" >> "$GITHUB_OUTPUT"

81
.github/workflows/label-rerun.yml vendored Normal file
View file

@ -0,0 +1,81 @@
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.
#
# 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.
on:
pull_request:
types: [labeled]
permissions:
actions: write
pull-requests: read
concurrency:
group: label-rerun-${{ github.event.pull_request.number }}
cancel-in-progress: true
jobs:
rerun-review-labels:
name: Rerun review-labels job
if: github.event.label.name == 'ci-reviewed'
runs-on: ubuntu-latest
timeout-minutes: 40
steps:
- name: Wait for CI run to finish, then rerun failed jobs
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
run: |
set -uo pipefail
# Find the latest CI run for this PR's head SHA.
RUN_ID=$(gh run list \
--repo "$REPO" \
--commit "$HEAD_SHA" \
--workflow ci.yml \
--limit 1 \
--json databaseId,status \
--jq '.[0] | "\(.databaseId) \(.status)"' 2>/dev/null || true)
if [ -z "$RUN_ID" ]; 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##* }"
echo "Latest CI run: $RUN_ID (status: $STATUS)"
# If the run is still in progress, wait for it to finish.
# gh run rerun only works on completed runs — if we try while it's
# running, GitHub rejects with "cannot be rerun; This workflow is
# already running".
if [ "$STATUS" != "completed" ]; then
echo "Run is $STATUS — waiting for completion (this may take a while)..."
# gh run watch --exit-status exits non-zero if the run fails,
# which is expected (the label gate fails). Don't let that kill
# the workflow — we WANT to rerun failed jobs.
timeout 2100 gh run watch "$RUN_ID" --repo "$REPO" --interval 15 || true
# Verify it's actually completed now.
STATUS=$(gh run view "$RUN_ID" --repo "$REPO" --json status --jq '.status' 2>/dev/null || echo "unknown")
if [ "$STATUS" != "completed" ]; then
echo "Run is still $STATUS after wait — giving up."
exit 0
fi
fi
echo "Run completed. Rerunning all failed jobs..."
gh run rerun "$RUN_ID" --repo "$REPO" --failed || true
echo "Done. GitHub will rerun review-labels and all dependent jobs."

126
.github/workflows/lint.yml vendored
View file

@ -2,11 +2,14 @@ name: Lint (ruff + ty)
# Two things here:
# 1. Advisory diff — ruff + ty diagnostics as a diff vs the target branch.
# Posts a Markdown summary and a PR comment. Exit zero always.
# Writes a Markdown summary to the run page. Exit zero always.
# 2. Blocking ``ruff check .`` — enforces the explicit rules in
# ``[tool.ruff.lint.select]`` (currently PLW1514). Failure blocks merge.
# Separate job so the advisory diff still runs and posts even when
# enforcement fails.
# Separate job so the advisory diff still runs even when enforcement
# fails.
#
# CI-sensitive file review was previously here as a ``ci-review`` job but
# has moved to ``review-labels.yml`` so it can be rerun independently.
on:
workflow_call:
@ -15,14 +18,9 @@ on:
description: The event name from the calling orchestrator (pull_request or push).
type: string
required: true
ci_review:
description: Whether CI-sensitive files (eslint config, workflows, actions) changed and require a review label.
type: boolean
default: false
permissions:
contents: read
pull-requests: write # needed to post/update PR comments
concurrency:
group: lint-${{ github.ref }}
@ -162,115 +160,3 @@ jobs:
- name: Run footgun checker
run: python scripts/check-windows-footguns.py --all
ci-review:
# Require explicit maintainer review when CI-sensitive files change:
# eslint config, workflow YAMLs, or composite actions. These files
# influence what code the js-autofix job executes and pushes to
# main, so a malicious PR could inject arbitrary code via a custom eslint
# rule's `fix` function. The label gate ensures a human reviews before
# merge. Mirrors the mcp-catalog-reviewed pattern in supply-chain-audit.yml.
name: CI-sensitive file review
if: inputs.event_name == 'pull_request' && inputs.ci_review
runs-on: ubuntu-latest
timeout-minutes: 2
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- name: Require ci-reviewed label
id: label-check
env:
# Read-only label lookup. Use the built-in GITHUB_TOKEN (present and
# read-only on forks) so the gate works on fork PRs; fall back to it
# when AUTOFIX_BOT_PAT is empty. `|| true` degrades an API blip to
# "label absent" rather than hard-failing the step.
GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT || github.token }}
run: |
set -euo pipefail
PR="${{ github.event.pull_request.number }}"
LABELS=$(gh pr view "$PR" --json labels --jq '.labels[].name' || true)
if echo "$LABELS" | grep -Fxq 'ci-reviewed'; then
echo "reviewed=true" >> "$GITHUB_OUTPUT"
echo "ci-reviewed label present."
exit 0
fi
echo "reviewed=false" >> "$GITHUB_OUTPUT"
# On failure: find the bot's previous comment and edit it, or create
# a new one if none exists. Using an HTML comment marker so we can
# locate it reliably across runs without parsing the body text.
# Skipped on fork PRs — GITHUB_TOKEN is read-only there, so the API
# call would fail. The label gate still holds via the step below.
- name: Post or update review warning
if: steps.label-check.outputs.reviewed != 'true' && github.event.pull_request.head.repo.fork != true
env:
GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT || github.token }}
run: |
set -euo pipefail
PR="${{ github.event.pull_request.number }}"
MARKER="<!-- ci-review-bot -->"
BODY="${MARKER}
## ⚠️ CI-sensitive file review required
This PR changes CI-sensitive files (eslint config, workflow YAMLs,
or composite actions). These files influence what code the
js-autofix job executes and pushes to main.
A maintainer should verify:
- no new eslint rules with custom \`fix\` functions that write outside linted paths,
- no workflow changes that widen permissions or remove guards,
- no composite action changes that alter what gets executed.
After review, add the \`ci-reviewed\` label and re-run this check."
# Find an existing comment with our marker.
COMMENT_ID=$(gh api \
"repos/${{ github.repository }}/issues/${PR}/comments" \
--paginate --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" \
| head -1 || true)
if [ -n "$COMMENT_ID" ]; then
gh api --method PATCH \
"repos/${{ github.repository }}/issues/comments/${COMMENT_ID}" \
-f body="$BODY"
else
gh pr comment "$PR" --body "$BODY"
fi
# Fail the job when the label is missing — always runs (including
# fork PRs) so the security gate holds even when the comment step
# was skipped above.
- name: Fail on missing label
if: steps.label-check.outputs.reviewed != 'true'
run: |
echo "::error::CI-sensitive changes require the ci-reviewed label."
exit 1
# On success: if a previous warning comment exists, edit it to show
# the review passed so the PR doesn't have a stale ⚠️ sitting around.
# Skipped on fork PRs — no comment was ever posted to update.
- name: Update previous warning to passed
if: steps.label-check.outputs.reviewed == 'true' && github.event.pull_request.head.repo.fork != true
env:
GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT || github.token }}
run: |
set -euo pipefail
PR="${{ github.event.pull_request.number }}"
MARKER="<!-- ci-review-bot -->"
# Find an existing comment with our marker.
COMMENT_ID=$(gh api \
"repos/${{ github.repository }}/issues/${PR}/comments" \
--paginate --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" \
| head -1 || true)
if [ -n "$COMMENT_ID" ]; then
BODY="${MARKER}
## ✅ CI-sensitive file review passed
The \`ci-reviewed\` label is present on this PR."
gh api --method PATCH \
"repos/${{ github.repository }}/issues/comments/${COMMENT_ID}" \
-f body="$BODY"
fi

81
.github/workflows/lockfile-diff.yml vendored
View file

@ -7,22 +7,25 @@ name: Lockfile diff
# the ``packages`` map at the merge base and at HEAD and set-diffs the
# {install path: version} maps instead.
#
# The comment is upserted: the script embeds a hidden HTML marker and the
# workflow PATCHes the existing comment when one is found, so a PR gets
# exactly one lockfile-diff comment that tracks the latest push instead
# of a stack of stale ones. When a later push reverts all lockfile
# changes, the comment is updated to say so (deleting it would be more
# surprising than telling the reviewer it's resolved).
# The semantic diff is exposed as a workflow_call output ``review_status``
# (a JSON array in the unified status format) and an artifact
# (``lockfile-diff`` containing the markdown fragment) for the step
# summary.
#
# Never blocking — this is review signal, not enforcement. Exit is 0 even
# when commenting fails (fork PRs get a read-only GITHUB_TOKEN).
# Never blocking — this is review signal, not enforcement.
on:
workflow_call:
outputs:
changed:
description: Whether package-lock.json changed relative to the target branch.
value: ${{ jobs.diff.outputs.changed }}
review_status:
description: JSON array of review status objects for the unified PR comment.
value: ${{ jobs.diff.outputs.review_status }}
permissions:
contents: read
pull-requests: write # post/update the diff comment
concurrency:
group: lockfile-diff-${{ github.event.pull_request.number || github.ref }}
@ -33,6 +36,9 @@ jobs:
name: package-lock.json semantic diff
runs-on: ubuntu-latest
timeout-minutes: 5
outputs:
changed: ${{ steps.diff.outputs.changed }}
review_status: ${{ steps.emit-status.outputs.review_status }}
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@ -54,45 +60,36 @@ jobs:
--output /tmp/lockfile-diff.md
if [ -s /tmp/lockfile-diff.md ]; then
echo "changed=true" >> "$GITHUB_OUTPUT"
cat /tmp/lockfile-diff.md >> "$GITHUB_STEP_SUMMARY"
{
echo "## package-lock.json semantic diff"
echo ""
cat /tmp/lockfile-diff.md
} >> "$GITHUB_STEP_SUMMARY"
else
echo "changed=false" >> "$GITHUB_OUTPUT"
: > /tmp/lockfile-diff.md
fi
- name: Post or update PR comment
env:
GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }}
REPO: ${{ github.repository }}
PR: ${{ github.event.pull_request.number }}
CHANGED: ${{ steps.diff.outputs.changed }}
- name: Emit review_status
id: emit-status
run: |
set -euo pipefail
MARKER='<!-- hermes-lockfile-diff -->'
CHANGED="${{ steps.diff.outputs.changed }}"
STATUS="[]"
# Find our previous comment (paginated — busy PRs exceed one page).
EXISTING=$(gh api --paginate "repos/${REPO}/issues/${PR}/comments" \
--jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \
| head -1 || true)
if [ "$CHANGED" != "true" ]; then
if [ -n "$EXISTING" ]; then
# A previous push changed the lockfile but the latest one
# doesn't — update the comment rather than leave stale info.
printf '%s\n✅ package-lock.json changes from an earlier push have been reverted — locked versions now match the target branch.\n' "$MARKER" > /tmp/lockfile-diff.md
else
echo "No lockfile changes and no existing comment — nothing to do."
exit 0
fi
fi
if [ -n "$EXISTING" ]; then
echo "Updating existing comment ${EXISTING}"
gh api --method PATCH "repos/${REPO}/issues/comments/${EXISTING}" \
-F body=@/tmp/lockfile-diff.md > /dev/null \
|| echo "::warning::Could not update PR comment (expected for fork PRs — GITHUB_TOKEN is read-only)"
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.\"}]}"
else
echo "Creating new comment"
gh api "repos/${REPO}/issues/${PR}/comments" \
-F body=@/tmp/lockfile-diff.md > /dev/null \
|| echo "::warning::Could not post PR comment (expected for fork PRs — GITHUB_TOKEN is read-only)"
STATUS="[]"
fi
echo "review_status=${STATUS}" >> "$GITHUB_OUTPUT"
- name: Upload diff artifact
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
with:
name: lockfile-diff
path: /tmp/lockfile-diff.md
retention-days: 1
overwrite: true

139
.github/workflows/osv-scanner.yml vendored
View file

@ -14,14 +14,14 @@ name: OSV-Scanner
# code patterns in PR diffs) by covering the orthogonal "currently-pinned
# dep became known-vulnerable" case.
#
# Steps below are inlined from Google's officially-recommended reusable
# workflow (google/osv-scanner-action/.github/workflows/osv-scanner-reusable.yml),
# rather than called via `uses:` so we can set a `timeout-minutes` in the
# degenerate case where this job hangs.
# Uses Google's officially-recommended reusable workflow, pinned by SHA.
# Findings land in the repo's Security tab (Code Scanning > OSV-Scanner).
# fail-on-vuln is disabled so the job does not block merges on pre-existing
# vulnerabilities in pinned deps that we may need to patch deliberately.
#
# The reusable workflow can't emit custom outputs, so a wrapper job
# downloads the SARIF result and summarizes the vulnerability count into
# a review_status for the unified PR comment.
on:
workflow_call:
@ -40,62 +40,85 @@ permissions:
jobs:
scan:
name: Scan lockfiles
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false
uses: google/osv-scanner-action/.github/workflows/osv-scanner-reusable.yml@9a498708959aeaef5ef730655706c5a1df1edbc2 # v2.3.8
with:
# Scan explicit lockfiles rather than recursing, so we only look at
# the three sources of truth and skip vendored / test / worktree dirs.
scan-args: |-
--lockfile=uv.lock
--lockfile=package-lock.json
--lockfile=website/package-lock.json
fail-on-vuln: false
- name: 'Run scanner'
uses: google/osv-scanner-action/osv-scanner-action@9a498708959aeaef5ef730655706c5a1df1edbc2 # v2.3.8
emit-status:
name: Emit review status
runs-on: ubuntu-latest
needs: scan
if: always()
outputs:
review_status: ${{ steps.emit.outputs.review_status }}
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- name: Download SARIF result
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
with:
# Scan explicit lockfiles rather than recursing, so we only look at
# the three sources of truth and skip vendored / test / worktree dirs.
scan-args: |-
--output=results.json
--format=json
--lockfile=uv.lock
--lockfile=package-lock.json
--lockfile=website/package-lock.json
name: osv-results
path: /tmp/osv-results
continue-on-error: true
- name: 'Run osv-scanner-reporter'
uses: google/osv-scanner-action/osv-reporter-action@9a498708959aeaef5ef730655706c5a1df1edbc2 # v2.3.8
with:
scan-args: |-
--output=results.sarif
--new=results.json
--gh-annotations=false
--fail-on-vuln=false
# Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF
# format to the repository Actions tab.
- name: 'Upload artifact'
id: 'upload_artifact'
if: ${{ !cancelled() }}
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
with:
name: OSV Scanner SARIF file
path: results.sarif
retention-days: 5
# Upload the results to GitHub's code scanning dashboard.
- name: 'Upload to code-scanning'
if: ${{ !cancelled() }}
uses: github/codeql-action/upload-sarif@cdefb33c0f6224e58673d9004f47f7cb3e328b89 # v4.31.10
with:
sarif_file: results.sarif
- name: 'Print Code Scanning URL'
if: ${{ !cancelled() }}
- name: Emit review_status
id: emit
run: |
echo "View the OSV-Scanner results in the 'Security' tab, using the following link:"
echo "${{ github.server_url }}/${{ github.repository }}/security/code-scanning?query=is%3Aopen+branch%3A${GITHUB_REF_NAME}+tool%3Aosv-scanner"
env:
GITHUB_REF_NAME: ${{ github.ref_name }}
set -euo pipefail
STATUS="[]"
- name: 'Error troubleshooter'
if: ${{ always() && steps.upload_artifact.outcome == 'failure' }}
run: |
echo "::error::Artifact upload failed. This is most likely caused by a error during scanning earlier in the workflow."
exit 1
if [ -f /tmp/osv-results/osv-results.sarif ]; then
# Count vulnerabilities from the SARIF file
VULN_COUNT=$(python3 -c "
import json, sys
try:
with open('/tmp/osv-results/osv-results.sarif') as f:
data = json.load(f)
count = 0
vulns = []
for run in data.get('runs', []):
for result in run.get('results', []):
count += 1
rule_id = result.get('ruleId', 'unknown')
message = result.get('message', {}).get('text', '')
loc = result.get('locations', [{}])[0].get('physicalLocation', {}).get('artifactLocation', {}).get('uri', '')
vulns.append(f'- {rule_id} in {loc}: {message}')
print(count)
if vulns:
print('\n'.join(vulns[:20]), file=sys.stderr)
except Exception:
print(0)
")
VULN_DETAIL=""
if [ "$VULN_COUNT" -gt 0 ] 2>/dev/null; then
VULN_PLURAL=$([ "$VULN_COUNT" -eq 1 ] && echo "y" || echo "ies")
VULN_DETAIL=$(python3 -c "
import json, sys
try:
with open('/tmp/osv-results/osv-results.sarif') as f:
data = json.load(f)
vulns = []
for run in data.get('runs', []):
for result in run.get('results', []):
rule_id = result.get('ruleId', 'unknown')
loc = result.get('locations', [{}])[0].get('physicalLocation', {}).get('artifactLocation', {}).get('uri', '')
vulns.append(f'- {rule_id} in {loc}')
print(json.dumps('\n'.join(vulns[:20])))
except Exception:
print(json.dumps(''))
")
STATUS="[{\"source\":\"osv scan\",\"results\":[{\"kind\":\"warning\",\"title\":\"OSV vulnerability scan\",\"summary\":\"${VULN_COUNT} known vulnerabilit${VULN_PLURAL} found in pinned dependencies.\",\"detail\":${VULN_DETAIL},\"how_to_fix\":\"Review the findings in the [Security tab](../../security/code-scanning). Update the affected dependencies if a patched version is available.\"}]}]"
else
STATUS="[]"
fi
fi
echo "review_status=${STATUS}" >> "$GITHUB_OUTPUT"

92
.github/workflows/review-labels.yml vendored Normal file
View file

@ -0,0 +1,92 @@
name: Review labels
# Require explicit maintainer review when CI-sensitive files or the MCP
# catalog change. Previously this was split across two jobs in two
# workflows: ``ci-review`` in lint.yml (gated on ``ci_review``) and
# ``mcp-catalog-review`` in supply-chain-audit.yml (gated on
# ``mcp_catalog``). Both checked for their own label.
#
# Now consolidated: a single ``ci-reviewed`` label covers both. The
# comment sections tell the reviewer exactly what to verify per area,
# so one label is enough — the human reads the comment, not the label
# name.
#
# Outputs:
# ci_reviewed — "true" / "false" / "" (empty when neither lane ran)
# review_status — JSON array of status objects consumed by the review
# comment assembler. See scripts/ci/emit_review_status.py.
on:
workflow_call:
inputs:
ci_review:
description: Whether CI-sensitive files (eslint config, workflows, actions) changed.
type: boolean
default: false
mcp_catalog:
description: Whether the MCP catalog / installer changed.
type: boolean
default: false
outputs:
ci_reviewed:
description: Whether the ci-reviewed label is present. Empty when neither input was true.
value: ${{ jobs.check.outputs.ci_reviewed }}
review_status:
description: JSON array of status objects for the review comment assembler.
value: ${{ jobs.check.outputs.review_status }}
permissions:
contents: read
pull-requests: read # read PR labels
jobs:
check:
name: Review label gate
if: inputs.ci_review || inputs.mcp_catalog
runs-on: ubuntu-latest
timeout-minutes: 2
outputs:
ci_reviewed: ${{ steps.label-check.outputs.ci_reviewed }}
review_status: ${{ steps.build-status.outputs.review_status }}
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- name: Check ci-reviewed label
id: label-check
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
run: |
set -euo pipefail
PR="${{ github.event.pull_request.number }}"
LABELS=$(gh pr view "$PR" --repo "$REPO" --json labels --jq '.labels[].name' || true)
if echo "$LABELS" | grep -Fxq 'ci-reviewed'; then
echo "ci-reviewed label present."
echo "ci_reviewed=true" >> "$GITHUB_OUTPUT"
else
echo "ci-reviewed label missing."
echo "ci_reviewed=false" >> "$GITHUB_OUTPUT"
fi
- name: Build review_status JSON
id: build-status
env:
CI_REVIEW: ${{ inputs.ci_review }}
MCP_CATALOG: ${{ inputs.mcp_catalog }}
LABEL_PRESENT: ${{ steps.label-check.outputs.ci_reviewed }}
run: |
set -euo pipefail
ARGS=""
if [ "$CI_REVIEW" = "true" ]; then ARGS="$ARGS --ci-review"; fi
if [ "$MCP_CATALOG" = "true" ]; then ARGS="$ARGS --mcp-catalog"; fi
if [ "$LABEL_PRESENT" = "true" ]; then ARGS="$ARGS --label-present"; fi
python3 scripts/ci/emit_review_status.py $ARGS --output "$GITHUB_OUTPUT"
- name: Fail on missing label
if: steps.label-check.outputs.ci_reviewed != 'true'
run: |
echo "::error::CI-sensitive changes require the ci-reviewed label. Add the label and re-run this check."
exit 1

168
.github/workflows/supply-chain-audit.yml vendored
View file

@ -10,9 +10,17 @@ name: Supply Chain Audit
# advisory-only workflow instead.
#
# Path-gating is handled centrally by the ``ci.yml`` orchestrator's
# ``detect`` job. The orchestrator passes ``scan`` / ``deps`` /
# ``mcp_catalog`` booleans as inputs; this workflow's jobs gate on those
# inputs instead of re-computing the diff.
# ``detect`` job. The orchestrator passes ``scan`` / ``deps`` booleans as
# inputs; this workflow's jobs gate on those inputs instead of re-computing
# the diff. MCP catalog review was previously here but has moved to
# ``review-labels.yml`` so it can be rerun independently.
#
# Outputs:
# review_status — JSON array of status objects consumed by the review
# comment assembler (scripts/ci/assemble_review_comment.py).
# Each job (``scan``, ``dep-bounds``) emits its own
# array; an ``aggregate`` job merges them into the
# workflow-level output.
on:
workflow_call:
@ -29,10 +37,10 @@ on:
description: Whether pyproject.toml changed.
type: boolean
required: true
mcp_catalog:
description: Whether the MCP catalog / installer changed.
type: boolean
required: true
outputs:
review_status:
description: JSON array of review status objects for the review comment assembler.
value: ${{ jobs.aggregate.outputs.review_status }}
permissions:
pull-requests: write
@ -44,6 +52,8 @@ jobs:
if: inputs.scan
runs-on: ubuntu-latest
timeout-minutes: 15
outputs:
review_status: ${{ steps.emit-status.outputs.review_status }}
steps:
- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@ -61,7 +71,7 @@ jobs:
HEAD="${{ github.event.pull_request.head.sha }}"
# Added lines only, excluding lockfiles.
# Three-dot diff (base...head) diffs from the merge base to HEAD,
# Three-point diff (base...head) diffs from the merge base to HEAD,
# so only changes introduced by this PR are included — not changes
# that landed on main after the PR branched off.
DIFF=$(git diff "$BASE"..."$HEAD" -- . ':!uv.lock' ':!*.lock' ':!package-lock.json' ':!yarn.lock' || true)
@ -139,26 +149,41 @@ jobs:
echo "found=false" >> "$GITHUB_OUTPUT"
fi
- name: Post critical finding comment
if: steps.scan.outputs.found == 'true'
- name: Emit review_status
id: emit-status
if: always()
env:
GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }}
FOUND: ${{ steps.scan.outputs.found }}
run: |
BODY="## 🚨 CRITICAL Supply Chain Risk Detected
python3 - <<'PYEOF'
import json, os
This PR contains a pattern that has been used in real supply chain attacks. A maintainer must review the flagged code carefully before merging.
found = os.environ.get("FOUND", "") == "true"
$(cat /tmp/findings.md)
if found:
with open("/tmp/findings.md", encoding="utf-8") as f:
detail = f.read()
status = [{
"source": "supply chain",
"results": [{
"kind": "error",
"title": "Critical supply chain risk",
"summary": "Critical supply chain risk patterns detected in this PR.",
"detail": detail,
"how_to_fix": "Review the flagged code carefully. If intentional, add the `ci-reviewed` label."
}]
}]
else:
status = []
---
*Scanner only fires on high-signal indicators: .pth files, base64+exec/eval combos, subprocess with encoded commands, or install-hook files. Low-signal warnings were removed intentionally — if you're seeing this comment, the finding is worth inspecting.*"
gh pr comment "${{ github.event.pull_request.number }}" --body "$BODY" || echo "::warning::Could not post PR comment (expected for fork PRs — GITHUB_TOKEN is read-only)"
with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f:
f.write(f"review_status={json.dumps(status)}\n")
PYEOF
- name: Fail on critical findings
if: steps.scan.outputs.found == 'true'
run: |
echo "::error::CRITICAL supply chain risk patterns detected in this PR. See the PR comment for details."
echo "::error::CRITICAL supply chain risk patterns detected in this PR. See the review comment for details."
exit 1
dep-bounds:
@ -166,6 +191,8 @@ jobs:
if: inputs.deps
runs-on: ubuntu-latest
timeout-minutes: 15
outputs:
review_status: ${{ steps.emit-status.outputs.review_status }}
steps:
- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@ -188,7 +215,7 @@ jobs:
exit 0
fi
# Match PyPI dep specs that have >= but no < ceiling.
# Match PyPI dep specs that have >= and no < ceiling.
# Pattern: "package>=version" without a following ",<" bound.
# Excludes git+ URLs (which use commit SHAs) and comments.
UNBOUNDED=$(echo "$ADDED" | grep -oE '"[a-zA-Z0-9_-]+(\[[^\]]*\])?>=[ 0-9.]+"' | grep -v ',<' || true)
@ -200,26 +227,36 @@ jobs:
echo "found=false" >> "$GITHUB_OUTPUT"
fi
- name: Post unbounded dep warning
if: steps.bounds.outputs.found == 'true'
- name: Emit review_status
id: emit-status
if: always()
env:
GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }}
FOUND: ${{ steps.bounds.outputs.found }}
run: |
BODY="## ⚠️ Unbounded PyPI Dependency Detected
python3 - <<'PYEOF'
import json, os
This PR adds PyPI dependencies without a \`<next_major\` upper bound. Per our [supply chain policy](../blob/main/CONTRIBUTING.md#dependency-pinning-policy-supply-chain-hardening), all PyPI deps must be pinned as \`>=floor,<next_major\`.
found = os.environ.get("FOUND", "") == "true"
**Unbounded specs found:**
\`\`\`
$(cat /tmp/unbounded.txt)
\`\`\`
if found:
with open("/tmp/unbounded.txt", encoding="utf-8") as f:
detail = f.read()
status = [{
"source": "supply chain",
"results": [{
"kind": "action_required",
"title": "Unbounded PyPI dependencies",
"summary": "This PR adds PyPI dependencies without upper bounds.",
"detail": detail,
"how_to_fix": 'Add a `<next_major` upper bound, e.g. `"package>=1.2.0,<2"`. See CONTRIBUTING.md dependency pinning policy.'
}]
}]
else:
status = []
**Fix:** Add an upper bound, e.g. \`"package>=1.2.0,<2"\`
---
*See PR #2810 and CONTRIBUTING.md for the full policy rationale.*"
gh pr comment "${{ github.event.pull_request.number }}" --body "$BODY" || echo "::warning::Could not post PR comment (expected for fork PRs)"
with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f:
f.write(f"review_status={json.dumps(status)}\n")
PYEOF
- name: Fail on unbounded deps
if: steps.bounds.outputs.found == 'true'
@ -227,45 +264,36 @@ jobs:
echo "::error::PyPI dependencies without upper bounds detected. Add <next_major ceiling per CONTRIBUTING.md policy."
exit 1
mcp-catalog-review:
name: MCP catalog security review
if: inputs.mcp_catalog
aggregate:
name: Aggregate review statuses
needs: [scan, dep-bounds]
if: always()
runs-on: ubuntu-latest
timeout-minutes: 15
outputs:
review_status: ${{ steps.merge.outputs.review_status }}
steps:
- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 0
- name: Require explicit MCP catalog review label
- name: Merge review statuses
id: merge
env:
# Read-only label lookup. Use the built-in GITHUB_TOKEN (present and
# read-only on forks) so the gate works on fork PRs; fall back to it
# when AUTOFIX_BOT_PAT is empty. `|| true` degrades an API blip to
# "label absent" rather than hard-failing the step.
GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT || github.token }}
SCAN_STATUS: ${{ needs.scan.outputs.review_status }}
DEP_STATUS: ${{ needs.dep-bounds.outputs.review_status }}
run: |
set -euo pipefail
PR="${{ github.event.pull_request.number }}"
LABELS=$(gh pr view "$PR" --json labels --jq '.labels[].name' || true)
if echo "$LABELS" | grep -Fxq 'mcp-catalog-reviewed'; then
echo "MCP catalog review label present."
exit 0
fi
python3 - <<'PYEOF'
import json, os
BODY="## ⚠️ MCP catalog security review required
merged = []
for key in ("SCAN_STATUS", "DEP_STATUS"):
raw = os.environ.get(key, "")
if not raw:
continue
try:
data = json.loads(raw)
except (json.JSONDecodeError, TypeError):
continue
if isinstance(data, list):
merged.extend(data)
This PR changes the bundled MCP catalog or MCP catalog installer code. MCP entries can define local commands that users later install into \`mcp_servers\`, so this needs explicit maintainer review before merge.
A maintainer should verify:
- any new/changed \`optional-mcps/**/manifest.yaml\` command and args are expected,
- stdio transports do not use shell+egress/exfiltration payloads,
- git install refs are pinned and bootstrap commands are minimal,
- requested env vars/secrets match the upstream MCP's documented needs.
After review, add the \`mcp-catalog-reviewed\` label and re-run this check."
gh pr comment "$PR" --body "$BODY" || echo "::warning::Could not post PR comment (expected for fork PRs)"
echo "::error::MCP catalog changes require the mcp-catalog-reviewed label."
exit 1
with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f:
f.write(f"review_status={json.dumps(merged)}\n")
PYEOF

11
.github/workflows/uv-lockfile-check.yml vendored
View file

@ -45,6 +45,10 @@ name: uv.lock check
on:
workflow_call:
outputs:
review_status:
description: "JSON review status for the review-status aggregator"
value: ${{ jobs.check.outputs.review_status }}
permissions:
contents: read
@ -58,6 +62,8 @@ jobs:
name: uv lock --check
runs-on: ubuntu-latest
timeout-minutes: 5
outputs:
review_status: ${{ steps.verify.outputs.review_status }}
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@ -73,6 +79,7 @@ jobs:
# of this file) — failures often mean "your branch is behind main,
# rebase and regenerate uv.lock."
- name: Verify uv.lock is up-to-date
id: verify
run: |
# uv lock --check re-resolves against PyPI (network). Retry so a
# registry blip doesn't read as "lockfile stale". A genuinely stale
@ -117,5 +124,9 @@ jobs:
on `main` post-merge.
EOF
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"
exit 1
fi
review_status='[]'
echo "review_status=${review_status}" >> "$GITHUB_OUTPUT"

424
scripts/ci/assemble_review_comment.py Normal file
View file

@ -0,0 +1,424 @@
#!/usr/bin/env python3
"""Assemble the unified CI review comment for a pull request.
Every CI job that wants to appear in the review comment emits a
``review_status`` output: a JSON array of objects, each with a ``source``
(the workflow name, used for dedup) and a ``results`` array of typed
result objects::
[
{
"source": "review-label-gate",
"results": [
{"kind": "action_required", "title": "...", "summary": "...",
"how_to_fix": "..."},
{"kind": "info", "title": "...", "summary": "..."}
]
},
{
"source": "ci-timings",
"results": [
{"kind": "warning", "title": "CI timings", "summary": "...",
"detail": "...", "link": "..."}
]
}
]
Each result object has:
kind: "error" | "action_required" | "warning" | "info"
title: section heading
summary: one-line description
detail: markdown detail (optional)
how_to_fix: markdown checklist (optional)
link: URL (optional)
link_label: label for the link (optional, default "View logs")
The assembler flattens all results into a flat list of ReviewItems,
grouped by severity in the comment. Jobs that failed (from the
``needs`` context) but didn't emit any status get synthesized ❌ Error
items. Jobs that DID emit a status are excluded from the synthesized
error list — their own output is the authority for their classification.
Exits 0 always — comment posting is best-effort (fork PRs are read-only).
"""
from __future__ import annotations
import argparse
import json
import sys
from dataclasses import dataclass
from pathlib import Path
# Hidden marker the comment system uses to find-and-edit its
# previous comment instead of stacking new ones on each run.
MARKER = "<!-- hermes-ci-review-bot -->"
# Severity ordering for display.
_SEVERITY_ORDER = ["error", "action_required", "warning", "info"]
# Severities that trigger the "blocking issues" layout (vs. the
# "looks good!" banner).
_BLOCKING_SEVERITIES = ("error", "action_required", "warning")
_SEVERITY_GROUP_HEADER = {
"error": "## ❌ Job failures",
"action_required": "## ⚠️ Action required",
"warning": "## ⚠️ Warnings",
"info": "## ℹ️ Details",
}
@dataclass
class ReviewItem:
"""A single piece of review information with a severity tag."""
severity: str # "error" | "action_required" | "warning" | "info"
title: str # short section title, e.g. "package-lock.json"
summary: str # one-line summary
detail: str = "" # optional markdown detail (tables, bullet lists, etc.)
link: str = "" # optional URL emitted by the job (e.g. report URL)
link_label: str = "View report" # label for the emitted link
how_to_fix: str = "" # optional markdown checklist for action_required items
source: str = "" # workflow that declared this status (for dedup)
job_url: str = "" # auto-attached per-job log link (from the live poller)
# ---------------------------------------------------------------------------
# Collectors — each returns a list of ReviewItems (possibly empty)
# ---------------------------------------------------------------------------
def collect_from_statuses(review_statuses_json: str) -> tuple[list[ReviewItem], set[str]]:
"""Parse the nested review_status JSON into flat ReviewItems.
The input is a JSON array of ``{source, results: [...]}`` objects.
Each entry in ``results`` becomes one ReviewItem, tagged with the
parent's ``source``.
Returns ``(items, sources)`` where ``sources`` is the set of source
values — used by :func:`collect_failed_jobs` to exclude jobs that
already declared their own status (so a failing job that emitted an
``action_required`` status doesn't also show as a synthesized ❌ Error).
"""
if not review_statuses_json:
return [], set()
try:
data = json.loads(review_statuses_json)
except (json.JSONDecodeError, TypeError):
return [], set()
if not isinstance(data, list):
return [], set()
items: list[ReviewItem] = []
sources: set[str] = set()
for entry in data:
if not isinstance(entry, dict):
continue
source = entry.get("source", "")
if source:
sources.add(source)
for r in entry.get("results", []):
if not isinstance(r, dict):
continue
kind = r.get("kind", "info")
if kind not in _SEVERITY_ORDER:
kind = "info"
items.append(ReviewItem(
severity=kind,
title=r.get("title", "Unknown"),
summary=r.get("summary", ""),
detail=r.get("detail", ""),
link=r.get("link", ""),
link_label=r.get("link_label", "View logs"),
how_to_fix=r.get("how_to_fix", ""),
source=source,
))
return items, sources
def collect_failed_jobs(
needs_json: str,
run_url: str,
exclude_sources: set[str] | None = None,
job_urls: dict[str, str] | None = None,
) -> list[ReviewItem]:
"""Build error items for failed CI jobs from the ``needs`` context.
``needs_json`` is the JSON string emitted by ``all-checks-pass`` — a
``{job_name: result}`` dict where result is ``success`` / ``failure``
/ ``skipped``. Only ``failure`` entries become error items.
``exclude_sources`` is a set of ``source`` values from status objects
declared by workflow_call jobs. Job names containing any of these
source strings are excluded — their failure is already covered by their
own status output.
``job_urls`` is an optional ``{job_name: html_url}`` dict from the
live poller. When a job's name is in this dict, the ❌ Error link
points directly to that job's logs page instead of the whole run.
Falls back to ``run_url`` when no per-job URL is available.
"""
if not needs_json:
return []
try:
needs = json.loads(needs_json)
except (json.JSONDecodeError, TypeError):
return []
# Pre-normalize exclude sources once: lowercase + hyphens→spaces, so
# "review-label-gate" matches "Review label gate / Review label gate".
norm_sources = {
src.lower().replace("-", " ") for src in (exclude_sources or set())
}
items: list[ReviewItem] = []
for name, result in sorted(needs.items()):
if result != "failure":
continue
if norm_sources:
norm = name.lower().replace("-", " ")
if any(src in norm for src in norm_sources):
continue
job_url = (job_urls or {}).get(name, run_url)
items.append(ReviewItem(
severity="error",
title=name,
summary=f"Job **{name}** failed.",
job_url=job_url,
))
return items
# ---------------------------------------------------------------------------
# Rendering
# ---------------------------------------------------------------------------
def _render_item(item: ReviewItem) -> str:
"""Render a single ReviewItem as a markdown block.
The group header (``## ❌ Job failures`` etc.) carries the severity
emoji, so items don't repeat it. Links are shown inline next to the
title. Layout per item::
### {title} · [View report](url) · [View job](url)
{summary}
{detail}
**How to fix:**
{how_to_fix}
"""
title = f"### {item.title}"
# Build inline links next to the title.
links: list[str] = []
if item.link:
links.append(f"[{item.link_label}]({item.link})")
if item.job_url:
links.append(f"[View job]({item.job_url})")
if links:
title += " · " + " · ".join(links)
parts = [title, "", item.summary]
if item.detail:
parts += ["", item.detail]
if item.how_to_fix:
parts += ["", "**How to fix:**", "", item.how_to_fix]
return "\n".join(parts)
def _render_group(header: str, items: list[ReviewItem]) -> str:
"""Render a severity group: ``##`` header + items separated by ``---``."""
blocks = [_render_item(i) for i in items]
return f"{header}\n\n" + "\n\n---\n\n".join(blocks)
def _render_info_details(items: list[ReviewItem]) -> str:
"""Render each info item as its own collapsible ``<details>`` block."""
blocks = []
for item in items:
inner = _render_item(item)
blocks.append(
f"<details>\n<summary>{item.title}</summary>\n\n{inner}\n\n</details>"
)
return "\n\n".join(blocks)
def _render_pending_items(pending_jobs: list[str]) -> str:
"""Render the dimmed ``<sub>`` items for jobs still running."""
job_list = ", ".join(f"`{j}`" for j in sorted(pending_jobs))
return f"\n\n---\n\n<sub>Still running {len(pending_jobs)} job{'s' if len(pending_jobs) != 1 else ''}: {job_list}</sub>\n"
def render_comment(items: list[ReviewItem], pending_jobs: list[str] | None = None, commit_info: str = "") -> str:
"""Render the full comment body from a list of review items.
Items are grouped by severity under ``##`` group headers, separated
by ``---``. Errors and action_required items are always visible.
Warnings are shown only when present. Info items are in a collapsible
``<details>`` block. If ``pending_jobs`` is non-empty, a dimmed
``<sub>`` footer is appended listing jobs still running.
When there are no errors, action_required, or warnings (only info
items, or nothing at all), a "looks good!" banner is shown at the top,
and info items (if any) follow in a collapsible ``<details>`` block.
"""
pending = pending_jobs or []
# Group by severity
by_severity: dict[str, list[ReviewItem]] = {s: [] for s in _SEVERITY_ORDER}
for item in items:
by_severity.setdefault(item.severity, []).append(item)
info = by_severity.get("info", [])
has_blocking = any(by_severity.get(s) for s in _BLOCKING_SEVERITIES)
body = f"{MARKER}\n# ૮ >ﻌ< ა ci review\n\n"
if commit_info:
body += f"{commit_info}\n\n"
if not items and not pending:
return f"{body}looks good to me!"
sections: list[str] = []
for sev in _BLOCKING_SEVERITIES:
group = by_severity.get(sev, [])
if group:
sections.append(_render_group(_SEVERITY_GROUP_HEADER[sev], group))
# Info: collapsible <details>
if info:
sections.append(_render_info_details(info))
if pending:
body += _render_pending_items(pending)
if sections:
body += "\n\n---\n\n".join(sections)
return body
# ---------------------------------------------------------------------------
# Assembly
# ---------------------------------------------------------------------------
def _attach_job_urls(items: list[ReviewItem], job_urls: dict[str, str], run_url: str) -> None:
"""Fill in per-job log links for all items.
Uses the same case-insensitive, hyphen-normalized matching as
:func:`collect_failed_jobs`: the item's ``source`` is matched against
job names in ``job_urls``. Sets ``job_url`` on the item — this is
separate from ``link`` (the job-emitted URL, e.g. a report artifact),
so both can appear in the rendered comment.
"""
if not job_urls and not run_url:
return
# Pre-normalize job_url keys once.
norm_urls: dict[str, str] = {}
for name, url in job_urls.items():
norm_urls[name.lower().replace("-", " ")] = url
for item in items:
if item.job_url:
continue
src = item.source.lower().replace("-", " ")
# Try exact match first, then substring match.
if src in norm_urls:
item.job_url = norm_urls[src]
continue
for norm_name, url in norm_urls.items():
if src and src in norm_name:
item.job_url = url
break
# If no per-job URL found, fall back to run_url for items with a source.
if not item.job_url and item.source and run_url:
item.job_url = run_url
def assemble(
needs_json: str = "",
run_url: str = "",
job_urls: dict[str, str] | None = None,
review_statuses_json: str = "",
pending_jobs: list[str] | None = None,
commit_info: str = "",
) -> str:
"""Assemble the full comment body from all available inputs."""
items: list[ReviewItem] = []
# 1. Structured statuses from workflow_call jobs (review-labels, etc.)
status_items, sources = collect_from_statuses(review_statuses_json)
items.extend(status_items)
# 2. Synthesized error items for failed jobs not covered by statuses
items.extend(collect_failed_jobs(needs_json, run_url, exclude_sources=sources, job_urls=job_urls))
# 3. Attach per-job log links to all items (not just synthesized errors)
_attach_job_urls(items, job_urls or {}, run_url)
return render_comment(items, pending_jobs, commit_info)
# ---------------------------------------------------------------------------
# CLI
# ---------------------------------------------------------------------------
def main() -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument(
"--needs-json",
default="",
help="JSON string of {job_name: result} from the all-checks-pass job.",
)
parser.add_argument(
"--run-url",
default="",
help="URL to the CI run summary page (for failed job links).",
)
parser.add_argument(
"--review-statuses-json",
default="",
help="JSON array of {source, results: [...]} objects from workflow_call jobs.",
)
parser.add_argument(
"--pending-jobs",
default="",
help="Comma-separated list of job names still running (shown in a dimmed footer).",
)
parser.add_argument(
"--output",
type=Path,
required=True,
help="Output file for the assembled comment body.",
)
args = parser.parse_args()
pending = [j.strip() for j in args.pending_jobs.split(",") if j.strip()] if args.pending_jobs else None
body = assemble(
needs_json=args.needs_json,
run_url=args.run_url,
review_statuses_json=args.review_statuses_json,
pending_jobs=pending,
)
args.output.write_text(body)
print(f"Wrote {len(body)} chars to {args.output}")
return 0
if __name__ == "__main__":
sys.exit(main())

143
scripts/ci/emit_review_status.py Normal file
View file

@ -0,0 +1,143 @@
#!/usr/bin/env python3
"""Emit review_status JSON for the review-labels workflow.
Builds a JSON array with one entry::
[
{
"source": "review-label-gate",
"results": [
{"kind": "action_required", "title": "...", "summary": "...",
"how_to_fix": "..."},
{"kind": "info", "title": "...", "summary": "..."}
]
}
]
The ``source`` field is the workflow name that declared the status; the
assembler uses it to exclude the corresponding job from the synthesized
❌ Error list (the job already has its own status section).
The array can contain 0, 1, or 2 results — one per lane that ran
(``ci_review``, ``mcp_catalog``). When the ``ci-reviewed`` label is
present, the kind is ``info``; when missing, it's ``action_required``
with the verification checklist.
"""
from __future__ import annotations
import argparse
import json
import sys
# The source identifier used for error-synthesis exclusion. This must
# match (as a normalized substring) the job name as it appears in the
# GitHub Actions API. The ci.yml job key is ``review-labels`` with
# ``name: Review label gate``, and the reusable workflow's job is also
# ``name: Review label gate``, so the API shows the job as
# "Review label gate / Review label gate". Normalizing "review-label-gate"
# (lowercase, hyphens→spaces) gives "review label gate", which is a
# substring of "review label gate / review label gate".
SOURCE = "review-label-gate"
def build_results(
ci_review: bool,
mcp_catalog: bool,
label_present: bool,
) -> list[dict]:
"""Build the list of result objects for this source."""
results: list[dict] = []
if ci_review:
if label_present:
results.append({
"kind": "info",
"title": "CI-sensitive file review",
"summary": "`ci-reviewed` label is present.",
})
else:
results.append({
"kind": "action_required",
"title": "CI-sensitive file review",
"summary": (
"This PR changes CI-sensitive files (eslint config, "
"workflow YAMLs, or composite actions). These influence "
"what the js-autofix job executes and pushes to main."
),
"how_to_fix": (
"Add the `ci-reviewed` label after verifying:\n"
"- no new eslint rules with custom `fix` functions that write outside linted paths,\n"
"- no workflow changes that widen permissions or remove guards,\n"
"- no composite action changes that alter what gets executed."
),
})
if mcp_catalog:
if label_present:
results.append({
"kind": "info",
"title": "MCP catalog security review",
"summary": "`ci-reviewed` label is present.",
})
else:
results.append({
"kind": "action_required",
"title": "MCP catalog security review",
"summary": (
"This PR changes the bundled MCP catalog or MCP catalog "
"installer code. MCP entries can define local commands "
"that users later install into `mcp_servers`, so this "
"needs explicit maintainer review before merge."
),
"how_to_fix": (
"Add the `ci-reviewed` label after verifying:\n"
"- any new/changed `optional-mcps/**/manifest.yaml` command and args are expected,\n"
"- stdio transports do not use shell+egress/exfiltration payloads,\n"
"- git install refs are pinned and bootstrap commands are minimal,\n"
"- requested env vars/secrets match the upstream MCP's documented needs."
),
})
return results
def build_statuses(
ci_review: bool,
mcp_catalog: bool,
label_present: bool,
) -> list[dict]:
"""Build the full review_status array (one entry with a results list)."""
results = build_results(ci_review, mcp_catalog, label_present)
if not results:
return []
return [{"source": SOURCE, "results": results}]
def main() -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--ci-review", action="store_true",
help="Whether CI-sensitive files changed.")
parser.add_argument("--mcp-catalog", action="store_true",
help="Whether the MCP catalog / installer changed.")
parser.add_argument("--label-present", action="store_true",
help="Whether the ci-reviewed label is present.")
parser.add_argument("--output", default="-",
help="Output file ('-' for stdout, or a GITHUB_OUTPUT path).")
args = parser.parse_args()
statuses = build_statuses(args.ci_review, args.mcp_catalog, args.label_present)
json_str = json.dumps(statuses)
if args.output == "-":
print(json_str)
else:
# GITHUB_OUTPUT format: key=value\n
with open(args.output, "a", encoding="utf-8") as f:
f.write(f"review_status={json_str}\n")
return 0
if __name__ == "__main__":
sys.exit(main())

526
scripts/ci/live_comment.py Normal file
View file

@ -0,0 +1,526 @@
#!/usr/bin/env python3
"""Live-updating CI review comment.
Polls the GitHub Actions API for job statuses in the current 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
as each job finishes.
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.
Architecture:
- :func:`classify_jobs` (pure, testable) — takes a list of raw API job
dicts and returns ``(completed, pending, job_urls)`` where ``completed``
is a ``{name: result}`` dict (for :func:`assemble_review_comment.assemble`)
and ``pending`` is a list of job names still running.
- :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:`run` — the polling loop. Calls the API, classifies, assembles,
upserts, sleeps, repeats. Exits when all jobs are completed.
The orchestrator job names (detect, all-checks-pass, comment-live, etc.)
are excluded from the comment — they're infrastructure, not review signal.
"""
from __future__ import annotations
import argparse
import json
import os
import subprocess
import sys
import time
import urllib.error
import urllib.request
from pathlib import Path
API_BASE = "https://api.github.com"
# Job names that are infrastructure (this script, the gate, the detector)
# and should never appear in the review comment.
_INFRA_JOBS = frozenset({
"detect",
"all-checks-pass",
"comment-pending",
"comment-results",
"comment-live",
"CI review comment (pending)",
"CI review comment (results)",
"CI review comment (live)",
"All required checks pass",
"Detect affected areas",
})
# Map GitHub API conclusion values to our result strings.
_CONCLUSION_MAP = {
"success": "success",
"failure": "failure",
"skipped": "skipped",
"cancelled": "skipped",
"neutral": "skipped",
"timed_out": "failure",
"action_required": "skipped",
}
def classify_jobs(api_jobs: list[dict]) -> tuple[dict[str, str], list[str], dict[str, str]]:
"""Classify raw API job dicts into completed + pending + job_urls.
Returns ``(completed, pending, job_urls)``:
- ``completed``: ``{job_name: result}`` where result is
``"success"`` / ``"failure"`` / ``"skipped"``. Only non-infra jobs
that have finished.
- ``pending``: list of job names still running (in_progress / queued
/ waiting). Excludes infra jobs.
- ``job_urls``: ``{job_name: html_url}`` — direct links to each
job's logs page, for the assembler to use in ❌ Error links.
The API returns orchestrator-level jobs and sub-workflow jobs
(workflow_call) in separate runs — :func:`collect_run_jobs` merges
them. Each sub-workflow job has a ``_workflow_name`` prefix so the
display name is ``"Workflow / job"``.
"""
completed: dict[str, str] = {}
pending: list[str] = []
job_urls: dict[str, str] = {}
for job in api_jobs:
name = job.get("name", "unknown")
if job.get("_workflow_name"):
name = f"{job['_workflow_name']} / {name}"
if name in _INFRA_JOBS:
continue
status = job.get("status", "")
conclusion = job.get("conclusion", "")
html_url = job.get("html_url", "")
if html_url:
job_urls[name] = html_url
if status in ("in_progress", "queued", "waiting"):
pending.append(name)
elif status == "completed":
result = _CONCLUSION_MAP.get(conclusion, "skipped")
completed[name] = result
# else: unknown status → skip
return completed, pending, job_urls
# ---------------------------------------------------------------------------
# API helpers
# ---------------------------------------------------------------------------
def _api_request(url: str, token: str) -> dict:
"""Authenticated GitHub API GET (single page)."""
req = urllib.request.Request(url, headers={
"Authorization": f"Bearer {token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
"User-Agent": "ci-live-comment",
})
with urllib.request.urlopen(req) as resp:
data: dict = json.loads(resp.read())
return data
def _api_get_paginated(url: str, token: str, list_key: str | None = None) -> list:
"""Authenticated GitHub API GET with pagination."""
results: list = []
while url:
req = urllib.request.Request(url, headers={
"Authorization": f"Bearer {token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
"User-Agent": "ci-live-comment",
})
with urllib.request.urlopen(req) as resp:
data = json.loads(resp.read())
link_header = resp.headers.get("Link", "")
if list_key:
results.extend(data.get(list_key, []))
elif isinstance(data, list):
results.extend(data)
else:
return data
next_url = None
for part in link_header.split(","):
part = part.strip()
if 'rel="next"' in part:
next_url = part[part.find("<") + 1:part.find(">")]
break
url = next_url
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.
Returns a flat list of job dicts (same shape as the API returns, plus
``_workflow_name`` on sub-workflow jobs).
"""
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
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.
for job in orch_jobs:
steps = job.get("steps") or []
if any(s.get("name", "").startswith("Run ./.github/") 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",
token, list_key="jobs",
)
for j in sr_jobs:
j["_workflow_name"] = sr_name
all_jobs.append(j)
return all_jobs
def find_comment_id(token: str, repo: str, pr_number: str) -> int | None:
"""Find our existing review comment by marker prefix."""
owner, repo_name = repo.split("/")
comments = _api_get_paginated(
f"{API_BASE}/repos/{owner}/{repo_name}/issues/{pr_number}/comments",
token,
)
for c in comments:
body = c.get("body", "") if isinstance(c, dict) else ""
if body.startswith("<!-- hermes-ci-review-bot -->"):
return c.get("id") if isinstance(c, dict) else None
return None
def upsert_comment(
token: str, repo: str, pr_number: str, body: str, comment_id: int | None = None
) -> int | None:
"""Create or update the review comment. Returns the comment ID."""
owner, repo_name = repo.split("/")
if comment_id is None:
comment_id = find_comment_id(token, repo, pr_number)
if comment_id:
url = f"{API_BASE}/repos/{owner}/{repo_name}/issues/comments/{comment_id}"
method = "PATCH"
else:
url = f"{API_BASE}/repos/{owner}/{repo_name}/issues/{pr_number}/comments"
method = "POST"
data = json.dumps({"body": body}).encode("utf-8")
req = urllib.request.Request(url, data=data, method=method, headers={
"Authorization": f"Bearer {token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
"Content-Type": "application/json",
"User-Agent": "ci-live-comment",
})
try:
with urllib.request.urlopen(req) as resp:
result = json.loads(resp.read())
return result.get("id")
except urllib.error.HTTPError as e:
print(f" API error {e.code}: {e.reason}", file=sys.stderr)
return None
# ---------------------------------------------------------------------------
# Artifact fetching (ci-timings 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.
"""
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 []
except Exception:
return []
status_file = Path("/tmp/artifact-dl/review-status.json")
if not status_file.exists():
return []
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)
if isinstance(statuses, list):
return statuses
except (json.JSONDecodeError, OSError):
pass
return []
# ---------------------------------------------------------------------------
# Comment assembly
# ---------------------------------------------------------------------------
def _import_assembler():
"""Import assemble_review_comment.py from the same directory."""
here = Path(__file__).resolve().parent
sys.path.insert(0, str(here))
import assemble_review_comment as asm
return asm
def build_comment_body(
asm_mod,
completed: dict[str, str],
pending: list[str],
run_url: str,
job_urls: dict[str, str],
review_statuses_json: str,
commit_info: str = "",
) -> str:
"""Assemble the comment body from current job states + static inputs."""
needs_json = json.dumps(completed) if completed else ""
return asm_mod.assemble(
needs_json=needs_json,
run_url=run_url,
job_urls=job_urls,
review_statuses_json=review_statuses_json,
pending_jobs=pending if pending else None,
commit_info=commit_info,
)
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 ""
# ---------------------------------------------------------------------------
# Polling loop
# ---------------------------------------------------------------------------
def run(
token: str,
repo: str,
run_id: str,
pr_number: str,
run_url: str,
review_statuses_json: str = "",
commit_info: str = "",
interval: int = 15,
timeout: int = 1800,
dry_run: bool = False,
) -> int:
"""Poll for job statuses and update the PR comment until all done.
Returns 0 always — comment posting is best-effort.
"""
asm = _import_assembler()
start = time.time()
last_body = ""
# 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")
while True:
elapsed = time.time() - start
if elapsed > timeout:
print(f"Timeout ({timeout}s) reached — stopping poll.", file=sys.stderr)
break
try:
jobs = collect_run_jobs(token, repo, run_id)
except Exception as e:
print(f" API error collecting jobs: {e}", file=sys.stderr)
time.sleep(interval)
continue
completed, pending, job_urls = classify_jobs(jobs)
total = len(completed) + len(pending)
print(f" [{elapsed:.0f}s] {len(completed)} completed, {len(pending)} pending "
f"({total} total jobs)")
# Try to fetch ci-timings artifact statuses (may not exist yet).
artifact_statuses = _fetch_artifact_statuses(
token, repo, run_id, "ci-timings-report",
)
if artifact_statuses:
print(f" Found ci-timings artifact with {len(artifact_statuses)} status entries")
merged_json = _merge_statuses(base_statuses, artifact_statuses)
body = build_comment_body(
asm, completed, pending, run_url, job_urls,
merged_json,
commit_info,
)
if body != last_body:
if dry_run:
print("--- DRY RUN — comment body ---")
print(body)
print("--- END ---")
else:
cid = upsert_comment(token, repo, pr_number, body)
if cid:
print(f" Updated comment {cid}")
else:
print(" Failed to update comment (will retry)", file=sys.stderr)
last_body = body
else:
print(" No change since last poll.")
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.")
break
time.sleep(interval)
return 0
def main() -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--interval", type=int, default=15,
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()
token = os.environ.get("GITHUB_TOKEN", "")
repo = os.environ.get("GITHUB_REPOSITORY", "")
run_id = os.environ.get("GITHUB_RUN_ID", "")
pr_number = os.environ.get("PR_NUMBER", "")
run_url = os.environ.get("RUN_URL", "")
if not args.dry_run:
if not token:
print("GITHUB_TOKEN is required", file=sys.stderr)
return 1
if not repo:
print("GITHUB_REPOSITORY is required", file=sys.stderr)
return 1
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
# 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", "")
commit_url = os.environ.get("COMMIT_URL", "")
commit_info = ""
if commit_sha:
short_sha = commit_sha[:7]
if commit_msg:
# Truncate commit message to first line, max 60 chars.
first_line = commit_msg.split("\n")[0][:60]
if commit_url:
commit_info = f"<sub>running on [{short_sha}]({commit_url}) — {first_line}</sub>"
else:
commit_info = f"<sub>running on {short_sha} — {first_line}</sub>"
elif commit_url:
commit_info = f"<sub>running on [{short_sha}]({commit_url})</sub>"
else:
commit_info = f"<sub>running on {short_sha}</sub>"
return run(
token=token,
repo=repo,
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,
dry_run=args.dry_run,
)
if __name__ == "__main__":
sys.exit(main())

34
scripts/ci/lockfile_diff.py
View file

@ -15,11 +15,11 @@ Usage (from a checkout that still has the base ref available):
--output diff.md [--repo-root .]
Reads every ``package-lock.json`` tracked at either ref (top-level and
nested — the repo has several), diffs each, and writes a Markdown report
to ``--output``. Exits 0 always; an empty report file means "no version
changes" (the caller uses that to decide whether to post/update the PR
comment). The report embeds ``COMMENT_MARKER`` so the workflow can find
and update its own previous comment instead of stacking new ones.
nested — the repo has several), diffs each, and writes a Markdown fragment
to ``--output``. Exits 0 always; an empty output file means "no version
changes" (the caller uses that to decide whether to include the section).
The fragment is consumed by ``scripts/ci/assemble_review_comment.py``,
which wraps it in a section with a header and action note.
"""
from __future__ import annotations
@ -29,9 +29,6 @@ import json
import subprocess
import sys
# Hidden marker used to locate the bot's previous comment for in-place update.
COMMENT_MARKER = "<!-- hermes-lockfile-diff -->"
def parse_lockfile(text: str) -> dict[str, str]:
"""Reduce lockfile JSON to ``{install path: version}``.
@ -79,21 +76,24 @@ def _display_name(path: str) -> str:
def render_markdown(diffs: dict[str, dict[str, list]]) -> str:
"""Render per-lockfile diffs as a Markdown PR comment body.
"""Render per-lockfile diffs as a Markdown fragment.
``diffs`` maps lockfile repo-path → the output of :func:`diff_locks`.
Lockfiles with no version changes are omitted. Returns ``""`` when
nothing changed anywhere (caller skips commenting entirely).
nothing changed anywhere (caller skips the section entirely).
The output is a fragment — per-lockfile ``####`` subsections with
tables — not a standalone comment. The ``assemble_review_comment``
script wraps this in a section with its own header and action note,
so no top-level header or comment marker is emitted here.
"""
sections = []
total = 0
for lockfile, d in sorted(diffs.items()):
added, removed, updated = d["added"], d["removed"], d["updated"]
n = len(added) + len(removed) + len(updated)
if n == 0:
continue
total += n
lines = [f"### `{lockfile}`", ""]
lines = [f"#### `{lockfile}`", ""]
lines.append("| Package | Before | After |")
lines.append("| --- | --- | --- |")
for path, old, new in updated:
@ -107,13 +107,7 @@ def render_markdown(diffs: dict[str, dict[str, list]]) -> str:
if not sections:
return ""
header = (
f"{COMMENT_MARKER}\n"
f"## ⚠️ `package-lock.json` changes ({total} package"
f"{'s' if total != 1 else ''})\n\n"
"This PR changes locked npm dependency versions."
)
return header + "\n" + "\n\n".join(sections) + "\n"
return "\n\n".join(sections) + "\n"
def _git_show(ref: str, path: str, repo_root: str) -> str | None:

93
scripts/ci/timings_report.py
View file

@ -895,6 +895,86 @@ def generate_summary(timings: dict, baseline: dict | None = None) -> str:
return "\n".join(lines)
# ---------------------------------------------------------------------------
# Review status JSON for the unified PR comment
# ---------------------------------------------------------------------------
# Wall-time regressions above this fraction of baseline are "warning" severity.
_TIMINGS_WARN_PCT = 0.25
def generate_review_status(
timings: dict, baseline: dict | None, report_url: str | None = None
) -> list[dict]:
"""Produce a review_status JSON array for the CI timings review section.
Returns a list with one ``{source, results: [...]}`` entry. The
result kind is ``"info"`` or ``"warning"`` (timings is never error —
it's an observability job). *summary* is a single short line suitable
for the PR comment. *detail* has the per-job deltas as a markdown
fragment.
"""
stats = compute_stats(timings, baseline)
if baseline is None:
severity = "info"
summary = f"Wall time {fmt_dur(stats['wall'])} (no baseline yet)."
else:
wall = stats["wall"]
bl_wall = stats["bl_wall"] or 0
if bl_wall > 0:
pct = (wall - bl_wall) / bl_wall * 100
wall_str = f"Wall time {fmt_dur(wall)} vs {fmt_dur(bl_wall)} ({pct:+.1f}%)."
if pct > _TIMINGS_WARN_PCT * 100:
severity = "warning"
else:
severity = "info"
else:
wall_str = f"Wall time {fmt_dur(wall)}."
severity = "info"
if stats["slower"]:
wall_str += f" {stats['slower']} job(s) slower,"
if stats["faster"]:
wall_str += f" {stats['faster']} faster,"
if stats["unchanged"]:
wall_str += f" {stats['unchanged']} unchanged."
summary = wall_str
# Per-job delta detail (top 5 by absolute change)
detail_lines: list[str] = []
if baseline:
bl_map = {j["name"]: j for j in baseline.get("jobs", [])}
deltas: list[tuple[float, str, str]] = []
for j in timings.get("jobs", []):
if is_skipped(j):
continue
bl = bl_map.get(j["name"])
if not bl or is_skipped(bl):
continue
cur = j.get("duration_s") or 0
bl_d = bl.get("duration_s") or 0
diff = cur - bl_d
if abs(diff) < 1.0:
continue
deltas.append((abs(diff), j["name"], f"{diff:+.1f}s"))
deltas.sort(reverse=True)
for _, name, delta_str in deltas[:5]:
detail_lines.append(f"- {name}: {delta_str}")
result: dict = {
"kind": severity,
"title": "CI timings",
"summary": summary,
"detail": "\n".join(detail_lines),
}
if report_url:
result["link"] = report_url
result["link_label"] = "View report"
return [{"source": "ci timing", "results": [result]}]
# ---------------------------------------------------------------------------
# Main
# ---------------------------------------------------------------------------
@ -916,6 +996,8 @@ def main():
help="JSON output path (default: ci-timings.json)")
parser.add_argument("--summary-out", default="ci-timings-summary.md",
help="Markdown summary output path (default: ci-timings-summary.md)")
parser.add_argument("--review-status-out", default="",
help="If set, write a review-status JSON for the unified PR comment.")
args = parser.parse_args()
# Collect or load timings
@ -974,6 +1056,17 @@ def main():
f.write(summary)
print(f"Wrote summary to {args.summary_out}")
# Write review status for the unified PR comment.
# The output goes to GITHUB_OUTPUT (or a file with the same key=value
# format) so the ci-timings job can expose it as a workflow_call output.
if args.review_status_out:
report_url = os.environ.get("CI_TIMINGS_REPORT_URL", "")
statuses = generate_review_status(timings, baseline, report_url)
json_str = json.dumps(statuses)
with open(args.review_status_out, "a", encoding="utf-8") as f:
f.write(f"review_status={json_str}\n")
print(f"Wrote review status to {args.review_status_out}")
if __name__ == "__main__":
main()

643
tests/ci/test_assemble_review_comment.py Normal file
View file

@ -0,0 +1,643 @@
"""Tests for scripts/ci/assemble_review_comment.py.
The assembler collects status from every CI sub-workflow into ReviewItems
classified by severity (error / action_required / warning / info), then
renders them into a single PR comment body.
Status data comes from two sources:
1. --review-statuses-json: JSON array of {source, results: [...]} objects
from workflow_call jobs. Each result has kind/title/summary/detail/
how_to_fix/link. The assembler flattens all results into ReviewItems.
2. --needs-json: {job_name: result} from all-checks-pass. Failed jobs not
claimed by any status become synthesized ❌ Error items.
Layout rules tested here:
- group headers: ## ❌ Job failures, ## ⚠️ Action required, ## ⚠️ Warnings
- each item is a ### section under its group header
- errors + action_required always visible
- warnings shown only when present
- info in a collapsible <details> block
- sections separated by ---
- how_to_fix rendered at bottom of action_required items
- empty → clean banner
- jobs with declared statuses excluded from failed-jobs list
- per-job URLs used for failed job links when available
"""
from __future__ import annotations
import importlib.util
import json
import sys
from pathlib import Path
import pytest
_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "assemble_review_comment.py"
_spec = importlib.util.spec_from_file_location("assemble_review_comment", _PATH)
if _spec is None or _spec.loader is None:
raise ImportError("Failed to load assemble_review_comment.py")
_mod = importlib.util.module_from_spec(_spec)
sys.modules["assemble_review_comment"] = _mod
_spec.loader.exec_module(_mod)
MARKER = _mod.MARKER
ReviewItem = _mod.ReviewItem
def _status(source: str, results: list[dict]) -> str:
"""Helper: build a review_statuses JSON string with one source entry."""
return json.dumps([{"source": source, "results": results}])
# ─── collect_from_statuses ──────────────────────────────────────────
def test_statuses_empty_json():
items, sources = _mod.collect_from_statuses("")
assert items == []
assert sources == set()
def test_statuses_bad_json():
items, sources = _mod.collect_from_statuses("not json")
assert items == []
assert sources == set()
def test_statuses_action_required():
statuses = _status("review-label-gate", [{
"kind": "action_required",
"title": "CI-sensitive file review",
"summary": "Changes detected.",
"how_to_fix": "Add the label.",
}])
items, sources = _mod.collect_from_statuses(statuses)
assert len(items) == 1
assert items[0].severity == "action_required"
assert items[0].title == "CI-sensitive file review"
assert items[0].how_to_fix == "Add the label."
assert items[0].source == "review-label-gate"
assert sources == {"review-label-gate"}
def test_statuses_info():
statuses = _status("review-label-gate", [{
"kind": "info",
"title": "CI-sensitive file review",
"summary": "Label present.",
}])
items, sources = _mod.collect_from_statuses(statuses)
assert len(items) == 1
assert items[0].severity == "info"
assert sources == {"review-label-gate"}
def test_statuses_multiple_results_same_source():
"""One source can emit multiple results of different kinds."""
statuses = _status("review-label-gate", [
{"kind": "action_required", "title": "CI review", "summary": "Missing label."},
{"kind": "action_required", "title": "MCP review", "summary": "Missing label."},
])
items, sources = _mod.collect_from_statuses(statuses)
assert len(items) == 2
assert sources == {"review-label-gate"}
def test_statuses_mixed_kinds_same_source():
"""One source can emit both a warning and an info."""
statuses = _status("ci-timings", [
{"kind": "warning", "title": "CI timings", "summary": "Slower."},
{"kind": "info", "title": "Baseline", "summary": "OK."},
])
items, sources = _mod.collect_from_statuses(statuses)
assert len(items) == 2
assert items[0].severity == "warning"
assert items[1].severity == "info"
assert sources == {"ci-timings"}
def test_statuses_multiple_sources():
statuses = json.dumps([
{"source": "review-label-gate", "results": [
{"kind": "action_required", "title": "CI review", "summary": "Missing."},
]},
{"source": "lockfile-diff", "results": [
{"kind": "info", "title": "package-lock.json", "summary": "No changes."},
]},
])
items, sources = _mod.collect_from_statuses(statuses)
assert len(items) == 2
assert sources == {"review-label-gate", "lockfile-diff"}
def test_statuses_unknown_kind_becomes_info():
statuses = _status("some-job", [{
"kind": "bogus",
"title": "X",
"summary": "Y",
}])
items, _ = _mod.collect_from_statuses(statuses)
assert items[0].severity == "info"
def test_statuses_no_source():
"""Status without a source — still rendered, just not excluded from errors."""
statuses = json.dumps([{
"results": [{"kind": "info", "title": "X", "summary": "Y"}],
}])
items, sources = _mod.collect_from_statuses(statuses)
assert len(items) == 1
assert sources == set()
def test_statuses_passes_through_optional_fields():
statuses = _status("ci-timings", [{
"kind": "warning",
"title": "CI timings",
"summary": "Slower.",
"detail": "- job: +5s",
"link": "https://report",
"link_label": "View report",
"how_to_fix": "Optimize.",
}])
items, _ = _mod.collect_from_statuses(statuses)
assert items[0].detail == "- job: +5s"
assert items[0].link == "https://report"
assert items[0].link_label == "View report"
assert items[0].how_to_fix == "Optimize."
# ─── collect_failed_jobs ─────────────────────────────────────────────
def test_failed_jobs_empty_needs():
assert _mod.collect_failed_jobs("", "https://run") == []
def test_failed_jobs_no_failures():
needs = json.dumps({"tests": "success", "lint": "skipped"})
assert _mod.collect_failed_jobs(needs, "https://run") == []
def test_failed_jobs_collects_only_failures():
needs = json.dumps({"tests": "success", "lint": "failure", "js-tests": "failure"})
items = _mod.collect_failed_jobs(needs, "https://run/123")
assert len(items) == 2
assert all(i.severity == "error" for i in items)
# sorted by name
names = [i.title for i in items]
assert names == ["js-tests", "lint"]
assert all(i.job_url == "https://run/123" for i in items)
def test_failed_jobs_bad_json():
assert _mod.collect_failed_jobs("not json", "https://run") == []
def test_failed_jobs_excluded_by_source():
"""Jobs whose name contains a declared source are excluded."""
needs = json.dumps({
"Review label gate / Review label gate": "failure",
"tests": "failure",
})
items = _mod.collect_failed_jobs(needs, "https://run", exclude_sources={"review-label-gate"})
assert len(items) == 1
assert items[0].title == "tests"
def test_failed_jobs_no_exclusion_without_sources():
"""Without exclude_sources, all failures are shown."""
needs = json.dumps({"review-label-gate": "failure", "tests": "failure"})
items = _mod.collect_failed_jobs(needs, "https://run")
assert len(items) == 2
def test_failed_jobs_per_job_url():
"""When job_urls is provided, the link points to the specific job."""
needs = json.dumps({"tests": "failure", "lint": "failure"})
job_urls = {"tests": "https://run/1/job/2", "lint": "https://run/1/job/3"}
items = _mod.collect_failed_jobs(needs, "https://fallback", job_urls=job_urls)
assert len(items) == 2
urls = {i.title: i.job_url for i in items}
assert urls["tests"] == "https://run/1/job/2"
assert urls["lint"] == "https://run/1/job/3"
def test_failed_jobs_fallback_to_run_url():
"""Jobs not in job_urls fall back to run_url."""
needs = json.dumps({"tests": "failure", "lint": "failure"})
job_urls = {"tests": "https://run/1/job/2"}
items = _mod.collect_failed_jobs(needs, "https://fallback", job_urls=job_urls)
urls = {i.title: i.job_url for i in items}
assert urls["tests"] == "https://run/1/job/2"
assert urls["lint"] == "https://fallback"
# ─── render_comment ───────────────────────────────────────────────────
def test_render_empty_shows_clean_banner():
"""Completely clean — dog kaomoji + 'looks good' banner, no sections."""
body = _mod.render_comment([])
assert body.startswith(MARKER)
assert "૮ >ﻌ< ა" in body
assert "looks good to me!" in body
assert "##" not in body # no section headers
def test_render_info_only_shows_details():
"""Info items only — header + collapsible details, no blocking sections."""
items = [
ReviewItem(severity="info", title="lockfile", summary="No changes."),
ReviewItem(severity="info", title="timings", summary="OK."),
]
body = _mod.render_comment(items)
assert "૮ >ﻌ< ა" in body
assert "<details>" in body
assert "</details>" in body
assert "No changes." in body
assert "OK." in body
# No blocking sections
assert "## ❌" not in body
assert "## ⚠️" not in body
def test_render_info_only_with_pending_shows_details_plus_footer():
items = [ReviewItem(severity="info", title="lockfile", summary="No changes.")]
body = _mod.render_comment(items, pending_jobs=["ci-timings"])
assert "૮ >ﻌ< ა" in body
assert "Still running" in body
assert "<details>" in body
assert "Still running" in body
assert "`ci-timings`" in body
def test_render_group_header_for_errors():
"""Errors appear under a '## ❌ Job failures' group header."""
items = [
ReviewItem(severity="error", title="tests", summary="Job **tests** failed.", link="https://run"),
ReviewItem(severity="error", title="lint", summary="Job **lint** failed.", link="https://run"),
]
body = _mod.render_comment(items)
assert "## ❌ Job failures" in body
assert "### tests" in body
assert "### lint" in body
assert body.index("## ❌ Job failures") < body.index("### tests")
def test_render_group_header_for_action_required():
items = [
ReviewItem(severity="action_required", title="CI review", summary="Need label."),
]
body = _mod.render_comment(items)
assert "## ⚠️ Action required" in body
assert "### CI review" in body
def test_render_group_header_for_warnings():
items = [
ReviewItem(severity="warning", title="CI timings", summary="Slower."),
]
body = _mod.render_comment(items)
assert "## ⚠️ Warnings" in body
assert "### CI timings" in body
assert "<details>" not in body
items2 = [ReviewItem(severity="info", title="x", summary="y")]
body2 = _mod.render_comment(items2)
assert "## ⚠️ Warnings" not in body2
def test_render_no_duplicated_severity_in_item_body():
"""Items don't repeat the severity label — the group header carries it."""
items = [ReviewItem(severity="error", title="tests", summary="Job failed.", link="https://run")]
body = _mod.render_comment(items)
assert "### tests" in body
assert "Job failed." in body
assert "**❌ Error**" not in body
def test_render_how_to_fix_at_bottom():
items = [
ReviewItem(severity="action_required", title="CI review", summary="Need label.",
how_to_fix="Add the `ci-reviewed` label."),
]
body = _mod.render_comment(items)
assert "**How to fix:**" in body
assert "Add the `ci-reviewed` label." in body
assert body.index("Need label.") < body.index("How to fix")
def test_render_sections_separated_by_hr():
items = [
ReviewItem(severity="error", title="tests", summary="failed."),
ReviewItem(severity="action_required", title="CI review", summary="need label."),
]
body = _mod.render_comment(items)
assert "\n\n---\n\n" in body
def test_render_errors_always_visible():
items = [
ReviewItem(severity="error", title="tests", summary="Job **tests** failed.", job_url="https://run"),
ReviewItem(severity="info", title="lockfile", summary="No changes."),
]
body = _mod.render_comment(items)
assert "## ❌ Job failures" in body
assert "### tests" in body
assert "Job **tests** failed." in body
assert "[View job](https://run)" in body
assert "<details>" in body
assert "No changes." in body
def test_render_info_in_collapsible_details():
"""Each info item is its own <details> block."""
items = [
ReviewItem(severity="info", title="lockfile", summary="No changes."),
ReviewItem(severity="info", title="timings", summary="OK."),
]
body = _mod.render_comment(items)
assert body.count("<details>") == 2
assert body.count("</details>") == 2
assert "<summary>lockfile</summary>" in body
assert "<summary>timings</summary>" in body
assert "No changes." in body
assert "OK." in body
def test_render_order_errors_then_action_then_warn_then_info():
items = [
ReviewItem(severity="info", title="i", summary="info"),
ReviewItem(severity="warning", title="w", summary="warn"),
ReviewItem(severity="action_required", title="a", summary="action"),
ReviewItem(severity="error", title="e", summary="error"),
]
body = _mod.render_comment(items)
error_pos = body.index("## ❌ Job failures")
action_pos = body.index("## ⚠️ Action required")
warn_pos = body.index("## ⚠️ Warnings")
info_pos = body.index("<details>")
assert error_pos < action_pos < warn_pos < info_pos
# ─── render_comment (pending jobs) ────────────────────────────────────
def test_render_pending_only_shows_header_with_clock():
"""Pending jobs only — header has 'still waiting', footer lists jobs, no sections."""
body = _mod.render_comment([], pending_jobs=["ci-timings"])
assert body.startswith(MARKER)
assert "૮ >ﻌ< ა" in body
assert "Still running" in body
assert "`ci-timings`" in body
assert "##" not in body
def test_render_pending_notif():
items = [ReviewItem(severity="info", title="lockfile", summary="No changes.")]
body = _mod.render_comment(items, pending_jobs=["ci-timings"])
assert "૮ >ﻌ< ა" in body
assert "<sub>Still running 1 job: `ci-timings`</sub>" in body
def test_render_pending_multiple_jobs_sorted():
body = _mod.render_comment([], pending_jobs=["docker", "ci-timings"])
assert "`ci-timings`" in body
assert "`docker`" in body
assert body.index("`ci-timings`") < body.index("`docker`")
def test_render_no_pending_no_footer():
items = [ReviewItem(severity="info", title="x", summary="y")]
body = _mod.render_comment(items)
assert "Still running" not in body
# ─── assemble (integration) ──────────────────────────────────────────
def test_assemble_all_skipped_clean_banner():
body = _mod.assemble()
assert body.startswith(MARKER)
assert "૮ >ﻌ< ა" in body
assert "looks good to me!" in body
assert "##" not in body
def test_assemble_failed_job_shown():
needs = json.dumps({"tests": "failure", "lint": "success"})
body = _mod.assemble(needs_json=needs, run_url="https://run/1")
assert "## ❌ Job failures" in body
assert "### tests" in body
assert "[View job](https://run/1)" in body
def test_assemble_with_review_statuses():
"""Statuses from review-labels render directly + exclude gate from errors."""
statuses = _status("review-label-gate", [{
"kind": "action_required",
"title": "CI-sensitive file review",
"summary": "Changes detected.",
"how_to_fix": "Add the label.",
}])
needs = json.dumps({
"Review label gate / Review label gate": "failure",
"tests": "success",
})
body = _mod.assemble(
needs_json=needs,
run_url="https://run",
review_statuses_json=statuses,
)
assert "## ⚠️ Action required" in body
assert "### CI-sensitive file review" in body
assert "Add the label." in body
assert "## ❌ Job failures" not in body
def test_assemble_pending_jobs():
body = _mod.assemble(pending_jobs=["ci-timings"])
assert "Still running" in body
assert "`ci-timings`" in body
def test_assemble_with_items_and_pending():
needs = json.dumps({"tests": "failure"})
body = _mod.assemble(needs_json=needs, run_url="https://run", pending_jobs=["ci-timings"])
assert "## ❌ Job failures" in body
assert "### tests" in body
assert "Still running" in body
assert "`ci-timings`" in body
def test_assemble_with_timings_status():
"""Timings status from the nested format renders as info or warning."""
statuses = _status("ci-timings", [{
"kind": "info",
"title": "CI timings",
"summary": "Wall time 3m (no baseline yet).",
"detail": "",
"link": "https://report",
}])
body = _mod.assemble(review_statuses_json=statuses)
assert "<details>" in body
assert "### CI timings" in body
assert "Wall time 3m" in body
assert "## ❌" not in body
assert "## ⚠️" not in body
def test_assemble_with_lockfile_status():
"""Lockfile no-changes status renders as info in the details block."""
statuses = _status("lockfile-diff", [{
"kind": "info",
"title": "package-lock.json",
"summary": "No lockfile changes — locked versions match the target branch.",
}])
body = _mod.assemble(review_statuses_json=statuses)
assert "<details>" in body
assert "### package-lock.json" in body
assert "No lockfile changes" in body
def test_assemble_with_lockfile_changed_status():
"""Lockfile changed status renders as action_required with ci-reviewed how_to_fix."""
statuses = _status("lockfile-diff", [{
"kind": "action_required",
"title": "package-lock.json",
"summary": "Locked npm dependency versions changed.",
"detail": "#### `package-lock.json`\n\n| col | | |",
"how_to_fix": "Add the `ci-reviewed` label after verifying the version changes are expected.",
}])
body = _mod.assemble(review_statuses_json=statuses)
assert "## ⚠️ Action required" in body
assert "### package-lock.json" in body
assert "Locked npm dependency versions changed." in body
assert "Add the `ci-reviewed` label" in body
assert "<details>" not in body # action_required is not in the collapsible block
# ─── _attach_job_urls ────────────────────────────────────────────────
def test_attach_job_urls_fills_missing_links():
"""Items without a link get one from job_urls via source matching."""
items = [
ReviewItem(severity="info", title="Supply chain scan",
summary="No risks.", source="supply chain"),
ReviewItem(severity="warning", title="CI timings",
summary="Slower.", source="ci timings",
link="https://report"), # already has a link
]
job_urls = {
"Supply Chain Audit / Scan PR for critical supply chain risks": "https://run/1/job/2",
}
_mod._attach_job_urls(items, job_urls, "https://fallback")
# First item gets the per-job URL as job_url (link untouched)
assert items[0].job_url == "https://run/1/job/2"
assert items[0].link == "" # no emitted link
# Second item keeps its existing link, job_url is set separately
assert items[1].link == "https://report"
assert items[1].job_url == "https://fallback" # fell back to run_url
def test_attach_job_urls_fallback_to_run_url():
"""Items with a source but no matching job URL fall back to run_url."""
items = [
ReviewItem(severity="info", title="X", summary="Y", source="some-job"),
]
_mod._attach_job_urls(items, {}, "https://fallback")
assert items[0].job_url == "https://fallback"
def test_attach_job_urls_no_source_no_link():
"""Items without a source don't get a link."""
items = [
ReviewItem(severity="info", title="X", summary="Y"), # no source
]
_mod._attach_job_urls(items, {"job": "https://run/1"}, "https://fallback")
assert items[0].job_url == ""
def test_assemble_attaches_links_to_all_items():
"""Integration: assemble() attaches job URLs to status items, not just errors."""
statuses = _status("supply chain", [{
"kind": "info",
"title": "Supply chain scan",
"summary": "No risks.",
}])
job_urls = {
"Supply Chain Audit / Scan PR for critical supply chain risks": "https://run/1/job/2",
}
body = _mod.assemble(
review_statuses_json=statuses,
job_urls=job_urls,
run_url="https://fallback",
)
assert "[View job](https://run/1/job/2)" in body
def test_render_commit_info_below_header():
"""Commit info is rendered below the header, above the content."""
body = _mod.render_comment(
[ReviewItem(severity="error", title="tests", summary="failed.")],
commit_info="<sub>running on [abc1234](https://commit-url) — fix: thing</sub>",
)
assert "# ૮ >ﻌ< ა ci review" in body
assert "running on [abc1234](https://commit-url)" in body
assert "fix: thing" in body
# Commit info appears before the content
assert body.index("abc1234") < body.index("## ❌")
def test_render_no_commit_info_when_empty():
"""No commit_info → no extra line below header."""
body = _mod.render_comment([])
assert "running on" not in body
def test_assemble_passes_commit_info():
"""assemble() passes commit_info through to render_comment."""
body = _mod.assemble(commit_info="<sub>running on abc1234</sub>")
assert "running on abc1234" in body
assert "looks good to me!" in body
def test_render_both_emitted_link_and_job_url():
"""An item with both an emitted link and a job_url shows both."""
item = ReviewItem(
severity="warning",
title="CI timings",
summary="Slower.",
link="https://artifact/report.html",
link_label="View report",
source="ci timings",
job_url="https://github.com/run/1/job/5",
)
body = _mod.render_comment([item])
assert "[View report](https://artifact/report.html)" in body
assert "[View job](https://github.com/run/1/job/5)" in body
# Both links on the same line, separated by ·
assert " · " in body
def test_assemble_both_links_for_ci_timings():
"""Integration: ci-timings has a report URL (link) AND gets a job_url."""
statuses = _status("ci timings", [{
"kind": "warning",
"title": "CI timings",
"summary": "Wall time 5m vs 3m (+66%).",
"detail": "- tests: +120s",
"link": "https://artifact/report.html",
"link_label": "View report",
}])
job_urls = {"CI timings": "https://github.com/run/1/job/5"}
body = _mod.assemble(
review_statuses_json=statuses,
job_urls=job_urls,
run_url="https://fallback",
)
assert "[View report](https://artifact/report.html)" in body
assert "[View job](https://github.com/run/1/job/5)" in body

162
tests/ci/test_live_comment.py Normal file
View file

@ -0,0 +1,162 @@
"""Tests for scripts/ci/live_comment.py — classify_jobs().
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.
"""
from __future__ import annotations
import importlib.util
import sys
from pathlib import Path
_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)
def _job(name: str, status: str, conclusion: str | None = None, workflow: str = "") -> dict:
"""Build a raw API job dict."""
j = {"name": name, "status": status, "conclusion": conclusion}
if workflow:
j["_workflow_name"] = workflow
return j
def test_classify_empty():
completed, pending, job_urls = _mod.classify_jobs([])
assert completed == {}
assert pending == []
assert job_urls == {}
def test_classify_success():
jobs = [_job("Python tests", "completed", "success")]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "success"}
assert pending == []
def test_classify_failure():
jobs = [_job("Python tests", "completed", "failure")]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "failure"}
assert pending == []
def test_classify_skipped():
jobs = [_job("Python tests", "completed", "skipped")]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "skipped"}
assert pending == []
def test_classify_in_progress():
jobs = [_job("Python tests", "in_progress", None)]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {}
assert pending == ["Python tests"]
def test_classify_queued():
jobs = [_job("Python tests", "queued", None)]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {}
assert pending == ["Python tests"]
def test_classify_waiting():
jobs = [_job("Python tests", "waiting", None)]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {}
assert pending == ["Python tests"]
def test_classify_mixed():
jobs = [
_job("Python tests", "completed", "success"),
_job("Python lints", "completed", "failure"),
_job("JS & TS checks", "in_progress", None),
_job("Desktop E2E", "queued", None),
]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "success", "Python lints": "failure"}
assert set(pending) == {"JS & TS checks", "Desktop E2E"}
def test_classify_infra_jobs_excluded():
"""Infra jobs (detect, all-checks-pass, comment-live) are never shown."""
jobs = [
_job("detect", "completed", "success"),
_job("Detect affected areas", "completed", "success"),
_job("all-checks-pass", "completed", "success"),
_job("All required checks pass", "completed", "success"),
_job("comment-live", "in_progress", None),
_job("CI review comment (live)", "in_progress", None),
_job("Python tests", "completed", "success"),
]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "success"}
assert pending == []
def test_classify_sub_workflow_jobs_prefixed():
"""Sub-workflow jobs get 'Workflow / job' display names."""
jobs = [
_job("test", "completed", "success", workflow="Tests"),
_job("check", "in_progress", None, workflow="JS Tests"),
]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert "Tests / test" in completed
assert completed["Tests / test"] == "success"
assert "JS Tests / check" in pending
def test_classify_captures_html_url():
"""The poller captures html_url per job for per-job log links."""
jobs = [
{**_job("Python tests", "completed", "failure"),
"html_url": "https://github.com/repo/actions/runs/1/job/2"},
_job("Python lints", "completed", "success"),
]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert job_urls["Python tests"] == "https://github.com/repo/actions/runs/1/job/2"
# Jobs without html_url are simply absent from the dict
assert "Python lints" not in job_urls
def test_classify_cancelled_treated_as_skipped():
jobs = [_job("Python tests", "completed", "cancelled")]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "skipped"}
def test_classify_timed_out_treated_as_failure():
jobs = [_job("Python tests", "completed", "timed_out")]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "failure"}
def test_classify_neutral_treated_as_skipped():
jobs = [_job("Python tests", "completed", "neutral")]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "skipped"}
def test_classify_action_required_treated_as_skipped():
jobs = [_job("Python tests", "completed", "action_required")]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {"Python tests": "skipped"}
def test_classify_unknown_status_skipped():
"""Unknown status values are silently ignored, not crashed on."""
jobs = [_job("weird-job", "unknown_status", None)]
completed, pending, job_urls = _mod.classify_jobs(jobs)
assert completed == {}
assert pending == []

7
tests/ci/test_lockfile_diff.py
View file

@ -89,15 +89,14 @@ def test_nested_dedup_is_distinct_entry():
assert d["updated"] == [("node_modules/foo/node_modules/react", "17.0.2", "17.0.3")]
def test_render_markdown_contains_marker_and_versions():
def test_render_markdown_contains_versions_and_nested_display():
d = _mod.diff_locks(
_mod.parse_lockfile(BASE),
_mod.parse_lockfile(_lock({"node_modules/react": {"version": "19.0.0"}})),
)
md = _mod.render_markdown({"apps/desktop/package-lock.json": d})
assert md.startswith(_mod.COMMENT_MARKER) # workflow finds its comment by prefix
assert "⚠️" in md
assert "`apps/desktop/package-lock.json`" in md
# Fragment starts directly with the per-lockfile subsection header.
assert md.startswith("#### `apps/desktop/package-lock.json`")
assert "`18.2.0`" in md and "`19.0.0`" in md
# nested display name keeps the parent chain visible
assert "nested under foo" in md

143
tests/ci/test_timings_report.py Normal file
View file

@ -0,0 +1,143 @@
"""Tests for scripts/ci/timings_report.py — generate_review_status().
The review status is a JSON array in the unified nested format consumed
by the review comment assembler. It classifies the CI timings result as
info/warning (never error — timings is an observability job, not a gate)
and provides a one-line summary plus optional per-job delta detail.
"""
from __future__ import annotations
import importlib.util
from datetime import datetime, timezone
from pathlib import Path
_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "timings_report.py"
_spec = importlib.util.spec_from_file_location("timings_report", _PATH)
if _spec is None or _spec.loader is None:
raise ImportError("Failed to load timings_report.py")
_mod = importlib.util.module_from_spec(_spec)
_spec.loader.exec_module(_mod)
_T0 = datetime(2025, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
def _ts(seconds: float) -> str:
"""ISO timestamp `seconds` after T0."""
dt = _T0.timestamp() + seconds
return datetime.fromtimestamp(dt, tz=timezone.utc).isoformat().replace("+00:00", "Z")
def _job(name: str, dur_s: float, start_s: float = 0.0, conclusion: str = "success") -> dict:
"""Build a normalized job dict with realistic timestamps for wall-time math."""
return {
"name": name,
"duration_s": dur_s,
"conclusion": conclusion,
"started_at": _ts(start_s),
"completed_at": _ts(start_s + dur_s),
"wait_s": 0.0,
}
def _timings(jobs: list[dict]) -> dict:
return {"run_id": "123", "head_sha": "abc", "created_at": "", "jobs": jobs}
def _result(statuses: list[dict]) -> dict:
"""Extract the single result dict from the nested format."""
assert len(statuses) == 1
assert statuses[0]["source"] == "ci timing"
results = statuses[0]["results"]
assert len(results) == 1
return results[0]
def test_no_baseline_is_info():
t = _timings([_job("tests", 60.0)])
result = _result(_mod.generate_review_status(t, None))
assert result["kind"] == "info"
assert "no baseline" in result["summary"].lower()
assert "link" not in result # no report_url → no link field
def test_no_regression_is_info():
cur = _timings([_job("tests", 60.0)])
bl = _timings([_job("tests", 60.0)])
result = _result(_mod.generate_review_status(cur, bl))
assert result["kind"] == "info"
assert "+0.0%" in result["summary"]
def test_small_regression_is_info():
cur = _timings([_job("tests", 65.0)])
bl = _timings([_job("tests", 60.0)])
result = _result(_mod.generate_review_status(cur, bl))
# +8.3% — well under the 25% warning threshold
assert result["kind"] == "info"
def test_large_regression_is_warning():
cur = _timings([_job("tests", 80.0)])
bl = _timings([_job("tests", 60.0)])
result = _result(_mod.generate_review_status(cur, bl))
# +33% — above the 25% threshold
assert result["kind"] == "warning"
assert "+33" in result["summary"]
def test_improvement_is_info():
cur = _timings([_job("tests", 40.0)])
bl = _timings([_job("tests", 60.0)])
result = _result(_mod.generate_review_status(cur, bl))
assert result["kind"] == "info"
assert "-33" in result["summary"]
def test_detail_shows_top_deltas():
cur = _timings([_job("slow-job", 120.0), _job("fast-job", 30.0, start_s=120.0)])
bl = _timings([_job("slow-job", 60.0), _job("fast-job", 60.0, start_s=60.0)])
result = _result(_mod.generate_review_status(cur, bl))
assert "slow-job" in result["detail"]
assert "fast-job" in result["detail"]
# Sorted by abs delta — slow-job (+60) before fast-job (-30)
assert result["detail"].index("slow-job") < result["detail"].index("fast-job")
def test_skipped_jobs_excluded_from_detail():
cur = _timings([_job("skipped-job", 0.0, conclusion="skipped"), _job("tests", 60.0)])
bl = _timings([_job("skipped-job", 0.0, conclusion="skipped"), _job("tests", 60.0)])
result = _result(_mod.generate_review_status(cur, bl))
assert "skipped-job" not in result["detail"]
def test_report_url_passed_through():
t = _timings([_job("tests", 60.0)])
result = _result(_mod.generate_review_status(t, None, report_url="https://artifact/123"))
assert result["link"] == "https://artifact/123"
assert result["link_label"] == "View report"
def test_never_error_severity():
"""Timings is observability — even huge regressions are warnings, not errors."""
cur = _timings([_job("tests", 600.0)])
bl = _timings([_job("tests", 60.0)])
result = _result(_mod.generate_review_status(cur, bl))
assert result["kind"] == "warning"
assert result["kind"] != "error"
def test_nested_format_structure():
"""The return value is a list with one {source, results: [...]} entry."""
t = _timings([_job("tests", 60.0)])
statuses = _mod.generate_review_status(t, None)
assert isinstance(statuses, list)
assert len(statuses) == 1
assert statuses[0]["source"] == "ci timing"
assert isinstance(statuses[0]["results"], list)
assert len(statuses[0]["results"]) == 1
r = statuses[0]["results"][0]
assert r["kind"] == "info"
assert r["title"] == "CI timings"
assert "summary" in r
assert "detail" in r