diff --git a/.claude/skills/ci/SKILL.md b/.claude/skills/ci/SKILL.md index 8919af4..864ae81 100644 --- a/.claude/skills/ci/SKILL.md +++ b/.claude/skills/ci/SKILL.md @@ -49,18 +49,33 @@ When pushing to both repos, always pass the SHAs to avoid a race condition where ### 2. Read Results -When the background task completes, read the output. The script emits: +When the background task completes, read the output. The script discovers +**every** workflow run GitHub triggers for the pushed commit — currently the +real test matrix (`CI`) and an automated reviewer (`Claude Code Review`) in +each repo — and waits for and reports on all of them, rather than assuming +there's exactly one relevant run and taking whichever one comes back first +(that ambiguity used to let it silently latch onto the wrong run — see +"Known pitfalls" below). + +This is default-include: if a new workflow starts firing on pushes in the +future, the script picks it up and gates on it automatically, no code change +needed. `EXCLUDE_WORKFLOWS` near the top of `monitor-ci.sh` is the escape +hatch for a workflow that fires on a push but should never gate this check +(empty by default). The script emits: ```text -[pgxntool-test] Run 12345678 found +[pgxntool-test] Run 12345678 (CI) found +[pgxntool-test] Run 12345679 (Claude Code Review) found [pgxntool-test] === BRANCHES: pgxntool-test=feature/foo pgxntool=feature/foo === -[pgxntool-test] Polling... (running: 🐘 PostgreSQL 13, 🐘 PostgreSQL 15) +[pgxntool-test] Polling... (still running: CI, Claude Code Review) +[pgxntool-test] Run 12345678 (CI) completed: SUCCESS [pgxntool-test] PASS 🐘 PostgreSQL 12 [pgxntool-test] PASS 🐘 PostgreSQL 15 [pgxntool-test] FAIL 🐘 PostgreSQL 13 -[pgxntool-test] Run completed: FAILURE -[pgxntool-test] === FAILURE: 🐘 PostgreSQL 13 === +[pgxntool-test] === FAILURE (CI): 🐘 PostgreSQL 13 === ... failure log lines ... +[pgxntool-test] Run 12345679 (Claude Code Review) completed: SKIPPED +[pgxntool-test] SKIPPED claude-review OVERALL: FAIL ``` @@ -68,9 +83,15 @@ The **last line is always `OVERALL: `**. Check this first: | OVERALL | Exit code | Meaning | |---------|-----------|---------| -| `ALL_PASS` | 0 | All jobs green — safe to proceed | -| `FAIL` | 1 | One or more jobs failed — stop and report | +| `ALL_PASS` | 0 | Every discovered run succeeded (or legitimately skipped) — safe to proceed | +| `FAIL` | 1 | One or more runs failed — stop and report | | `TIMEOUT` | 2 | Run(s) did not complete within timeout | +| `NO_RUNS` | 3 | No (non-excluded) workflow run was found for this branch/SHA after waiting | + +A `Claude Code Review` run concluding `SKIPPED` is normal, not a failure — it +no-ops on draft PRs and on PRs from untrusted forks (see +`claude-code-review.yml`'s job-level `if:`). Only `FAILURE`/`CANCELLED`/etc. +count against `OVERALL`. **Always verify the `=== BRANCHES ===` line** matches the code you just pushed — this is your primary safeguard against the `--branch` race condition. If the @@ -94,3 +115,23 @@ branches don't match, cancel the run and re-trigger: `gh run cancel --repo 2. When pushing to both repos, start two background monitors simultaneously (one per repo) 3. Pass the exact push SHA when available — `--branch` has a race condition on rapid pushes 4. The `=== BRANCHES ===` line in the output confirms which code is under test — always verify it matches your intent +5. A PR is only green once **every** workflow run triggered for that SHA has completed — don't treat `OVERALL: ALL_PASS` from an old, partial run as sufficient + +## Known pitfalls + +`gh run list --commit SHA` with no filter returns every workflow run tied to +that commit, in an order that isn't guaranteed to put the real CI run first. +Since `CI` (event `pull_request`) and `Claude Code Review` (event +`pull_request_target`) both trigger on the same push, taking `.[0]` of that +unfiltered list used to be able to silently grab the review run and report +on it as if it were the test matrix — the `=== BRANCHES ===` line would then +never appear at all, because only `CI`'s jobs emit it. The script avoids this +by discovering the full set of runs tied to the commit (settling briefly so +sibling runs GitHub hasn't indexed yet are caught) and monitoring all of them +to completion, rather than assuming there is exactly one relevant run. + +`gh run list --jq` only accepts a plain jq expression string — it does not +pass through extra jq flags like `--argjson`. Passing `--argjson` to `gh run +list` itself fails with "unknown command", so the exclude-list filter is +applied by piping `gh`'s raw `--json` output into a separate real `jq` +invocation instead. diff --git a/.claude/skills/ci/scripts/monitor-ci.sh b/.claude/skills/ci/scripts/monitor-ci.sh index 1b5a170..da19659 100755 --- a/.claude/skills/ci/scripts/monitor-ci.sh +++ b/.claude/skills/ci/scripts/monitor-ci.sh @@ -11,10 +11,12 @@ # sha_pgxntool : exact SHA pushed to pgxntool (optional) # # Exit codes: -# 0 : ALL_PASS — all jobs succeeded -# 1 : FAIL — one or more jobs failed +# 0 : ALL_PASS — every workflow run triggered by this push succeeded (or +# legitimately skipped, e.g. Claude Code Review on a draft +# PR or an untrusted fork) +# 1 : FAIL — one or more workflow runs failed # 2 : TIMEOUT — run(s) did not complete within the timeout -# 3 : NO_RUNS — no CI run found for this branch after waiting +# 3 : NO_RUNS — no workflow run was found for this branch/SHA after waiting # # Requires: gh CLI authenticated with repo access. @@ -41,6 +43,34 @@ REPO_PGXN="${_owner}/pgxntool" TIMEOUT_TEST=900 # 15 minutes TIMEOUT_PGXN=2100 # 35 minutes POLL_INTERVAL=10 # seconds between status polls +DISCOVER_INTERVAL=5 # seconds between polls while discovering/settling run set + +# Every workflow run GitHub actually triggers for a push is included by +# default and must succeed (or legitimately skip) for OVERALL: ALL_PASS - +# there is no hand-maintained list of "expected" workflow names to keep in +# sync every time a workflow is added, renamed, or removed. Add a workflow's +# display name here only for the rare case where it fires on a push but +# should never gate this check (e.g. known-flaky, or informational-only). +EXCLUDE_WORKFLOWS=() + +if [[ ${#EXCLUDE_WORKFLOWS[@]} -eq 0 ]]; then + EXCLUDE_JSON="[]" +else + EXCLUDE_JSON=$(printf '%s\n' "${EXCLUDE_WORKFLOWS[@]}" | jq -R . | jq -s .) +fi + +# ─── Helper: discover every run for a commit, excluding EXCLUDE_WORKFLOWS ──── +_list_runs() { + local repo="$1" sha="$2" + # NOTE: gh's own --jq flag is a plain jq expression string - it does not + # accept extra jq flags like --argjson. Pipe gh's raw JSON into a real jq + # invocation instead so $exclude can be bound. + gh run list --repo "$repo" --commit "$sha" \ + --json databaseId,workflowName 2>/dev/null \ + | jq --argjson exclude "$EXCLUDE_JSON" \ + '[.[] | select(([.workflowName] | inside($exclude)) | not)]' \ + || echo "[]" +} # ─── Helper: wait for a run to appear, then poll until done ────────────────── monitor_one() { @@ -51,53 +81,105 @@ monitor_one() { local label="[$repo]" local elapsed=0 - # Step 1: find the run ID. - # When a SHA is provided, wait up to 30s for GitHub to index that exact run - # before falling back to the branch lookup. Without this wait, rapid pushes - # cause the branch fallback to pick up the previous run instead of the new one. - local run_id="" - local sha_wait=0 - local SHA_INDEX_WAIT=30 # seconds to wait for SHA indexing before branch fallback - echo "$label Waiting for CI run on branch '$branch'..." - while [[ -z "$run_id" ]]; do - if [[ -n "$sha" ]]; then - run_id=$(gh run list --repo "$repo" --commit "$sha" \ - --json databaseId --jq '.[0].databaseId // empty' 2>/dev/null || true) + echo "$label Waiting for CI run(s) on branch '$branch'..." + + # Step 1a: resolve a concrete commit to key off. If no SHA was given, + # take the branch's most recent run of any workflow and use its headSha - + # this keeps "which workflows fired for this push" well defined, instead + # of `--branch --limit 1` picking a single, possibly-wrong-workflow run + # directly (the original bug this script had). + local resolved_sha="$sha" + if [[ -z "$resolved_sha" && -n "$branch" ]]; then + while [[ -z "$resolved_sha" && $elapsed -lt $timeout ]]; do + resolved_sha=$(gh run list --repo "$repo" --branch "$branch" --limit 1 \ + --json headSha --jq '.[0].headSha // empty' 2>/dev/null || true) + if [[ -z "$resolved_sha" ]]; then + sleep "$DISCOVER_INTERVAL" + elapsed=$((elapsed + DISCOVER_INTERVAL)) + fi + done + fi + if [[ -z "$resolved_sha" ]]; then + echo "$label ERROR: no CI run found after ${timeout}s" >&2 + return 3 # NO_RUNS (distinct from FAIL/TIMEOUT; see exit-code table) + fi + + # Step 1b: discover every (non-excluded) workflow run tied to that commit, + # then settle briefly to catch sibling runs GitHub hasn't indexed yet. + # Workflows triggered by the same push event normally all appear within a + # few seconds of each other; settling stops once the discovered count has + # held steady for two consecutive polls, or 30s have passed since the + # first run appeared, whichever comes first. + local run_list="[]" + local count=0 + local stable_polls=0 + local since_first_found=0 + local found_any=0 + while [[ $elapsed -lt $timeout ]]; do + local new_list new_count + new_list=$(_list_runs "$repo" "$resolved_sha") + new_count=$(echo "$new_list" | jq 'length') + + if [[ "$new_count" -gt 0 ]]; then + found_any=1 fi - if [[ -z "$run_id" && -n "$branch" && ( -z "$sha" || $sha_wait -ge $SHA_INDEX_WAIT ) ]]; then - # Only fall back to branch once the SHA wait window has elapsed (or no SHA given). - # NOTE: this can pick up a different run if two pushes happen rapidly. - run_id=$(gh run list --repo "$repo" --branch "$branch" \ - --event pull_request --limit 1 \ - --json databaseId --jq '.[0].databaseId // empty' 2>/dev/null || true) + if [[ "$new_count" -eq "$count" ]]; then + stable_polls=$((stable_polls + 1)) + else + stable_polls=0 fi - if [[ -z "$run_id" ]]; then - sleep 5 - elapsed=$((elapsed + 5)) - sha_wait=$((sha_wait + 5)) - if [[ $elapsed -ge $timeout ]]; then - echo "$label ERROR: no CI run found after ${timeout}s" >&2 - return 3 # NO_RUNS (distinct from FAIL/TIMEOUT; see exit-code table) + run_list="$new_list" + count="$new_count" + + if [[ $found_any -eq 1 ]]; then + if [[ $stable_polls -ge 2 || $since_first_found -ge 30 ]]; then + break fi + since_first_found=$((since_first_found + DISCOVER_INTERVAL)) fi + + sleep "$DISCOVER_INTERVAL" + elapsed=$((elapsed + DISCOVER_INTERVAL)) done - echo "$label Run $run_id found" - # Step 2: extract the BRANCHES line as soon as the first job starts. - # We use the direct jobs API (fast ~1s) rather than the zip-download log path - # (slow 3-10s). We only need one job — all jobs emit the same BRANCHES line. + if [[ "$count" -eq 0 ]]; then + echo "$label ERROR: no CI run found after ${timeout}s" >&2 + return 3 + fi + + declare -A run_names=() + local rid name + while IFS=$'\t' read -r rid name; do + run_names[$rid]="$name" + echo "$label Run $rid ($name) found" + done < <(echo "$run_list" | jq -r '.[] | [.databaseId, .workflowName] | @tsv') + + # Step 2: extract the BRANCHES line as soon as a job starts. We use the + # direct jobs API (fast ~1s) rather than the zip-download log path (slow + # 3-10s), checking only each run's first job — all jobs in the CI test + # matrix emit the same BRANCHES line, and it's fine to come up empty for + # runs that never emit it (e.g. Claude Code Review). local branches_line="" local attempts=0 while [[ -z "$branches_line" && $elapsed -lt $timeout ]]; do - local first_job_id - first_job_id=$(gh run view "$run_id" --repo "$repo" \ - --json jobs --jq '[.jobs[].databaseId][0] // empty' 2>/dev/null || true) + for rid in "${!run_names[@]}"; do + local first_job_id + first_job_id=$(gh run view "$rid" --repo "$repo" \ + --json jobs --jq '[.jobs[].databaseId][0] // empty' 2>/dev/null || true) + [[ -z "$first_job_id" ]] && continue - if [[ -n "$first_job_id" ]]; then - # grep may return non-zero if the line isn't present yet — that's fine. + # grep may return non-zero if the line isn't present in this run's log + # at all, or not yet — that's fine. + # No '^' anchor: the logs API prefixes every line with an ISO-8601 + # timestamp (e.g. "2026-07-29T20:54:15Z === BRANCHES: ..."), so an + # anchored match never fires. The step also gets echoed (with + # unexpanded ${VARS}) before it runs, so more than one line can match; + # `tail -1` keeps the last (actual, expanded) occurrence. branches_line=$(gh api "repos/${repo}/actions/jobs/${first_job_id}/logs" \ - 2>/dev/null | grep "^=== BRANCHES:" | tail -1 || true) - fi + 2>/dev/null | grep "=== BRANCHES:" | tail -1 \ + | sed -E 's/^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\.[0-9]+)?Z //' || true) + [[ -n "$branches_line" ]] && break + done if [[ -z "$branches_line" ]]; then attempts=$((attempts + 1)) @@ -114,73 +196,84 @@ monitor_one() { echo "$label $branches_line" fi - # Step 3: poll until all jobs complete. - local status="in_progress" - local result="" - while [[ "$status" != "completed" && $elapsed -lt $timeout ]]; do - result=$(gh run view "$run_id" --repo "$repo" \ - --json status,conclusion,jobs \ - --jq '{status: .status, conclusion: .conclusion, - jobs: [.jobs[] | {name: .name, status: .status, conclusion: .conclusion}]}' \ - 2>/dev/null || true) - - if [[ -z "$result" ]]; then - sleep "$POLL_INTERVAL" - elapsed=$((elapsed + POLL_INTERVAL)) - continue - fi + # Step 3: poll every run until each is completed. + declare -A results=() + declare -A conclusions=() + local pending=("${!run_names[@]}") + while [[ ${#pending[@]} -gt 0 && $elapsed -lt $timeout ]]; do + local still_pending=() + for rid in "${pending[@]}"; do + local result + result=$(gh run view "$rid" --repo "$repo" \ + --json status,conclusion,jobs \ + --jq '{status: .status, conclusion: .conclusion, + jobs: [.jobs[] | {name: .name, status: .status, conclusion: .conclusion}]}' \ + 2>/dev/null || true) - status=$(echo "$result" | jq -r '.status') + if [[ -z "$result" ]]; then + still_pending+=("$rid") + continue + fi - if [[ "$status" != "completed" ]]; then - local running - running=$(echo "$result" | jq -r \ - '[.jobs[] | select(.status == "in_progress") | .name] | join(", ")' || true) - if [[ -n "$running" ]]; then - echo "$label Polling... (running: $running)" + if [[ "$(echo "$result" | jq -r '.status')" == "completed" ]]; then + results[$rid]="$result" + conclusions[$rid]=$(echo "$result" | jq -r '.conclusion') + else + still_pending+=("$rid") fi + done + pending=("${still_pending[@]}") + + if [[ ${#pending[@]} -gt 0 ]]; then + local pending_desc="" + for rid in "${pending[@]}"; do pending_desc+="${run_names[$rid]}, "; done + echo "$label Polling... (still running: ${pending_desc%, })" sleep "$POLL_INTERVAL" elapsed=$((elapsed + POLL_INTERVAL)) fi done - if [[ $elapsed -ge $timeout ]]; then - echo "$label ERROR: timed out after ${timeout}s" >&2 + if [[ ${#pending[@]} -gt 0 ]]; then + local pending_desc="" + for rid in "${pending[@]}"; do pending_desc+="${run_names[$rid]}, "; done + echo "$label ERROR: timed out after ${timeout}s waiting on: ${pending_desc%, }" >&2 return 2 fi - # Step 4: report per-job outcomes. - local conclusion - conclusion=$(echo "$result" | jq -r '.conclusion') - echo "$label Run $run_id completed: $(echo "$conclusion" | tr '[:lower:]' '[:upper:]')" - echo "$result" | jq -r '.jobs[] | "\(if .conclusion == "success" then "PASS" elif .conclusion == null then .status else .conclusion | ascii_upcase end) \(.name)"' \ - | sed "s|^|$label |" - - # Step 5: for failed jobs, print the failure log (last 60 lines per job). - if [[ "$conclusion" != "success" ]]; then - local failed_job_ids - failed_job_ids=$(gh run view "$run_id" --repo "$repo" \ - --json jobs \ - --jq '[.jobs[] | select(.conclusion == "failure") | .databaseId] | .[]' \ - 2>/dev/null || true) - - for job_id in $failed_job_ids; do - local job_name - job_name=$(gh run view "$run_id" --repo "$repo" \ - --json jobs \ - --jq --argjson id "$job_id" \ - '[.jobs[] | select(.databaseId == $id) | .name] | .[0]' 2>/dev/null || true) - echo "" - echo "$label === FAILURE: ${job_name:-job $job_id} ===" - # Use --log-failed to get only the failed step output, keeping output compact. - gh run view --repo "$repo" --job "$job_id" --log-failed 2>&1 \ - | grep -v "^$" | tail -60 || true - done + # Step 4: report per-run, per-job outcomes. + # "skipped" is a legitimate outcome, not a failure: e.g. Claude Code + # Review's single job intentionally no-ops on draft PRs and PRs from + # untrusted forks (see claude-code-review.yml's job-level `if:`), which + # surfaces as the whole run concluding "skipped" rather than "success". + local overall_ok=1 + for rid in "${!run_names[@]}"; do + local name="${run_names[$rid]}" + local conclusion="${conclusions[$rid]}" + echo "$label Run $rid ($name) completed: $(echo "$conclusion" | tr '[:lower:]' '[:upper:]')" + echo "${results[$rid]}" | jq -r '.jobs[] | "\(if .conclusion == "success" or .conclusion == "skipped" then .conclusion | ascii_upcase elif .conclusion == null then .status else .conclusion | ascii_upcase end) \(.name)"' \ + | sed "s|^|$label |" - return 1 - fi + # Step 5: for a failing run, print the failure log (last 60 lines per job). + if [[ "$conclusion" != "success" && "$conclusion" != "skipped" ]]; then + overall_ok=0 + local job_id + for job_id in $(echo "${results[$rid]}" | jq -r \ + '[.jobs[] | select(.conclusion == "failure") | .databaseId] | .[]' 2>/dev/null || true); do + local job_name + job_name=$(echo "${results[$rid]}" | jq -r \ + --argjson id "$job_id" \ + '[.jobs[] | select(.databaseId == $id) | .name] | .[0]' 2>/dev/null || true) + echo "" + echo "$label === FAILURE ($name): ${job_name:-job $job_id} ===" + # Use --log-failed to get only the failed step output, keeping output compact. + gh run view --repo "$repo" --job "$job_id" --log-failed 2>&1 \ + | grep -v "^$" | tail -60 || true + done + fi + done - return 0 + [[ $overall_ok -eq 1 ]] && return 0 + return 1 } # ─── Main: run monitors in parallel or series ─────────────────────────────────