From c539cb8149bbd4df3ffa3172c3a58c8f6e580019 Mon Sep 17 00:00:00 2001 From: nova28 Date: Fri, 21 Aug 2026 18:06:13 +0800 Subject: [PATCH] feat(scripts): add pr-await CI gate to collapse PR polling into one call (#2873) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio --- .agents/agents/pr-poller.md | 24 +- .agents/skills/pr-fixup/SKILL.md | 58 ++- AGENTS.md | 2 +- Makefile | 1 + scripts/pr-await | 789 ++++++++++++++++++++++++++++ scripts/pr-await.test.sh | 859 +++++++++++++++++++++++++++++++ scripts/pr-state | 1 + scripts/pr-state.test.sh | 1 + 8 files changed, 1710 insertions(+), 25 deletions(-) create mode 100755 scripts/pr-await create mode 100755 scripts/pr-await.test.sh diff --git a/.agents/agents/pr-poller.md b/.agents/agents/pr-poller.md index 0ca5984720..fe33ac5ea5 100644 --- a/.agents/agents/pr-poller.md +++ b/.agents/agents/pr-poller.md @@ -16,15 +16,21 @@ Do not read source code, edit files, push, post or resolve GitHub comments, trigger workflows, fetch full CI logs, or spawn subagents. Use `scripts/pr-state --summary ` and `scripts/pr-resolve list ` as the -primary sources. Poll at a 30-second cadence for at most 20 minutes. In the -default mode, return early for a failed check, merge conflict, actionable -review feedback, or a terminal clean state. If the caller says "wait N -minutes" or "then fix up", use strict-deadline mode: calculate and include the -absolute deadline in the polling prompt, accumulate findings, and do not return -early for findings, pending checks, or a clean snapshot. Stop early only if the -PR is merged/closed or access is revoked. At the deadline, return the latest -named pending checks and named actionable review findings, not only aggregate -CI and review states. +primary sources. In the default mode, poll at a 60-second cadence for at most +20 minutes and return early for a failed check, merge conflict, actionable +review feedback, or a terminal clean state. Keep polling directly in this mode: +`scripts/pr-await` waits for every check to finish, which cannot return early on +a failure or conflict while other checks are still pending. If the caller says "wait N +minutes" or "then fix up", use strict-deadline mode instead: the 20-minute cap +does not apply there — honor the caller's own N-minute deadline. Those +semantics are exactly what `scripts/pr-await --deadline-min N` implements, +so prefer it there and report its single output. Without that script, still poll +at a 60-second cadence but for the caller's full N minutes: calculate and +include the absolute deadline in the polling prompt, accumulate findings, and +do not return early for findings, pending checks, or a clean snapshot. Stop +early only if the PR is merged/closed or access is revoked. At the deadline, +return the latest named pending checks and named actionable review findings, +not only aggregate CI and review states. Before the first GitHub request, obtain any runtime network approval required by the platform. A denied, cancelled, or interrupted approval is terminal: do not diff --git a/.agents/skills/pr-fixup/SKILL.md b/.agents/skills/pr-fixup/SKILL.md index 4125f6f04e..968ee1ab6e 100644 --- a/.agents/skills/pr-fixup/SKILL.md +++ b/.agents/skills/pr-fixup/SKILL.md @@ -50,19 +50,45 @@ If the fresh mergeability query reports `mergeable=CONFLICTING` or result, push it, and restart this section with a new PR-state snapshot. Do not triage comments or checks against a conflicted head. -For pending CI, do not run a rapid polling loop. Wait at a reasonable interval, -then run the same summary again. Stop after about 20 minutes and report the -exact pending checks as "CI in progress." Use two monitoring modes: default -monitoring may return early for a failed check, merge conflict, actionable -finding, or clean terminal state; strict-deadline monitoring applies when the -user says "wait N minutes" or "then fix up." In strict mode, calculate and -pass an absolute deadline, accumulate failures/comments, and do not return -early for findings, pending checks, or a clean snapshot; stop early only when -the PR is merged/closed or access is revoked. Do not use interactive `gh pr -checks --watch` in the primary conversation: its TTY redraws make captured -output unusable. Use saved `scripts/pr-state --summary` snapshots about 60–90 -seconds apart, or the read-only `pr-poller` only when the user explicitly asked -to wait or monitor. +For pending CI, wait with `scripts/pr-await `. It blocks below the +conversation and prints one report when every check is terminal, so a wait of +any length costs a single round-trip. Do not re-run `scripts/pr-state +--summary` on a timer instead: each snapshot is a separate model turn that +re-reads the whole context, and waiting is only about 6% of what such a turn +actually does. + +Default `--mode all-terminal` is the cost-correct choice and returns only when +no check is pending, so every failure arrives in one report and is fixed in one +pass. It is also the correct choice for a reason unrelated to cost: a failed job +can be visible while its parent workflow is still running, so an early return +reports a partial failure set. Use `--mode first-failure` only when the user +wants the first failure interactively; it costs an extra full CI cycle per +failure. Pass `--deadline-min` for a user-stated limit; on exit 2 report the +named pending checks as "CI in progress." + +Read its exit code rather than re-deriving state: 0 clean, 1 terminal with +findings, 2 deadline with checks still pending, 3 blocked. Exit 3 covers every +case where a clean answer cannot be trusted: the PR merged or closed, a workflow +needs approval, access was lost, the merge-state query failed, the snapshot +reported `errors` or a null unresolved-thread count, or the host toolchain +cannot run `pr-state` correctly. Never downgrade an exit 3 to "probably clean." + +Exit 1 counts blocking reviews, not only threads: a current-head +`CHANGES_REQUESTED` review can exist with no unresolved thread attached. + +Every report records the jq and bash versions it ran under. That is provenance, +not a gate: `pr-state` used to fail silently on jq 1.6 and on the bash 3.2 that +macOS ships as `/bin/bash`, both of which made a dirty PR look clean, and both +are fixed at the source. What guards against a degraded `pr-state` now is the +snapshot itself, which is stronger than any version check: a summary reporting +`errors`, one whose unresolved-thread count is null, and one that does not parse +are all treated as unknown rather than clean. A blocked report on an old +toolchain adds a note naming it as a possible cause. A push during the wait +restarts the gate against the new head and is reported. + +Do not use interactive `gh pr checks --watch` in the primary conversation: its +TTY redraws make captured output unusable. Use the read-only `pr-poller` only +when the user explicitly asked to wait or monitor and `pr-await` is unavailable. Treat a poller's unresolved/pending snapshot as provisional: it can predate a primary-session push or thread resolution. Re-run `scripts/pr-state --summary ` at the current head before acting on it or declaring completion. @@ -82,8 +108,10 @@ then re-run the summary and require jobs to materialize before polling. `gh run approve` is not a valid command. Treat the state as clean only when the current head has no failed or pending -checks, no merge conflict, no actionable review thread or issue comment, and -qualifying exact-head semantic evidence where PR delivery requires it. +checks, no merge conflict, no blocking review (an active `CHANGES_REQUESTED` +or a review blocked at the exact current head), no actionable review thread or +issue comment, and qualifying exact-head semantic evidence where PR delivery +requires it. ## 2. Fix CI Failures diff --git a/AGENTS.md b/AGENTS.md index f614b36d4b..af04055727 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -164,7 +164,7 @@ For multiline Markdown issue or PR bodies, write the body to a file and pass it with the relevant `gh ... --body-file ` option. Do not send escaped newlines through `--body`; GitHub will render them literally. -For PR review/fixup workflows, prefer the repo helpers before manually querying GitHub/GraphQL: `scripts/pr-state --summary ` for checks and unresolved-thread state, `scripts/pr-state --comment ` for a full review-comment body, `scripts/pr-resolve list ` for actionable unresolved review threads, and `scripts/pr-resolve reply ""` to reply, resolve, and react in one call. +For PR review/fixup workflows, prefer the repo helpers before manually querying GitHub/GraphQL: `scripts/pr-await ` to block until CI is terminal and get one report (do not manually poll `pr-state` on a timer in the primary conversation; preserve the documented `pr-poller` fallback when `pr-await` is unavailable), `scripts/pr-state --summary ` for checks and unresolved-thread state, `scripts/pr-state --comment ` for a full review-comment body, `scripts/pr-resolve list ` for actionable unresolved review threads, and `scripts/pr-resolve reply ""` to reply, resolve, and react in one call. When a Kandev system message references an MCP tool that is not visible in the active tool list, use the runtime's tool discovery mechanism, such as `tool_search` when available, before falling back to a less specific workflow. Some task messaging and platform helpers are exposed on demand. diff --git a/Makefile b/Makefile index cc8ad301c8..0e2e27e7d7 100644 --- a/Makefile +++ b/Makefile @@ -560,6 +560,7 @@ test-scripts: @printf "$(CYAN)Running script tests...$(RESET)\n" @python3 .github/scripts/lint-action-pinning_test.py @bash scripts/pr-state.test.sh + @bash scripts/pr-await.test.sh @bash scripts/run-quiet.test.sh @bash scripts/dev-prod-db-path.test.sh @bash scripts/opencode-code-review.test.sh diff --git a/scripts/pr-await b/scripts/pr-await new file mode 100755 index 0000000000..6085db1fb7 --- /dev/null +++ b/scripts/pr-await @@ -0,0 +1,789 @@ +#!/usr/bin/env bash +# Block until a pull request's CI reaches a terminal state, then emit one snapshot. +# +# Usage: +# scripts/pr-await [options] +# +# Options: +# --mode all-terminal | first-failure +# all-terminal (default) waits until no check is pending, +# so every failure lands in a single report and a single +# fix pass. first-failure returns as soon as one check +# fails; it exists for the interactive case and costs an +# extra full CI cycle per failure. +# --deadline-min N Hard stop, in minutes (default 45). +# --interval-sec N Seconds between polls (default 60). +# --format text | json Report format (default text). +# --quiet Suppress per-poll progress on stderr. +# +# Why this exists: `pr-state --summary` is a one-shot snapshot, so waiting for CI +# costs one model round-trip per snapshot. Measured over 6,658 such round-trips, +# each re-read ~177k of cached context at ~$0.26. This script moves the waiting +# below the model: one invocation, one report, regardless of how long CI runs. +# +# stdout carries ONLY the final report, so a caller can capture it verbatim. +# Progress goes to stderr. +# +# Exit codes: +# 0 terminal and clean - no failed checks, no unresolved threads +# 1 terminal with findings - failures and/or review threads to act on +# 2 deadline reached - checks still pending; names reported +# 3 blocked - merged/closed, approval required, or access lost +# +# Requires: gh (authenticated), jq, and scripts/pr-state alongside this script. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PR_STATE="${PR_AWAIT_PR_STATE:-$SCRIPT_DIR/pr-state}" +GH="${PR_AWAIT_GH:-gh}" +SLEEP="${PR_AWAIT_SLEEP:-sleep}" +ACTIVE_READ_PID='' +ACTIVE_READ_GROUP='' +ACTIVE_WATCHDOG_PID='' + +# Blocked results leave the poll loop before a snapshot exists, but a caller +# that asked for JSON still needs to parse the answer. +emit_blocked() { + local outcome=$1 message=$2 + if [[ "${FORMAT:-text}" == 'json' ]]; then + local waited_sec="${WAITED:-0}" + local polls="${POLLS:-0}" + local head_changes="${HEAD_CHANGES:-0}" + local deadline_sec="${DEADLINE_SEC:-0}" + local interval_sec="${INTERVAL_SEC:-0}" + local mergeable_value='UNKNOWN' + local merge_status_value='UNKNOWN' + if jq -e . >/dev/null 2>&1 <<<"${MERGE_JSON:-}"; then + mergeable_value="$(jq -r '.mergeable // "UNKNOWN"' <<<"$MERGE_JSON")" + merge_status_value="$(jq -r '.mergeStateStatus // "UNKNOWN"' <<<"$MERGE_JSON")" + fi + jq -n \ + --argjson pr "$PR" \ + --arg outcome "$outcome" \ + --arg mode "${MODE:-all-terminal}" \ + --argjson waited_sec "$waited_sec" \ + --argjson polls "$polls" \ + --argjson head_changes "$head_changes" \ + --argjson exit_code 3 \ + --arg message "$message" \ + --arg toolchain "${TOOLCHAIN:-unverified}" \ + --argjson deadline_sec "$deadline_sec" \ + --argjson interval_sec "$interval_sec" \ + --arg mergeable "$mergeable_value" \ + --arg merge_state_status "$merge_status_value" \ + '{pr: $pr, outcome: $outcome, mode: $mode, waited_sec: $waited_sec, + polls: $polls, head_changes: $head_changes, exit_code: $exit_code, + message: $message, toolchain: $toolchain, evidence_complete: false, + deadline_sec: $deadline_sec, interval_sec: $interval_sec, + mergeable: $mergeable, merge_state_status: $merge_state_status, + summary: null}' + else + printf 'PR %s\n%s\n' "$PR" "$message" + toolchain_hint + fi + exit 3 +} + +usage() { + sed -n '2,/^# Requires:/p' "$0" | sed 's|^# \{0,1\}||' + exit "${1:-1}" +} + +PR='' +MODE='all-terminal' +DEADLINE_MIN=45 +INTERVAL_SEC=60 +FORMAT='text' +QUIET=0 + +while [[ $# -gt 0 ]]; do + case "$1" in + --mode) + shift + [[ $# -gt 0 ]] || usage 1 + case "$1" in + all-terminal | first-failure) MODE=$1 ;; + *) usage 1 ;; + esac + shift + ;; + --deadline-min) + shift + [[ $# -gt 0 && "$1" =~ ^[0-9]+$ ]] || usage 1 + # 10# forces base 10. Without it "09" aborts arithmetic under set -e and + # "010" silently means 8, because Bash reads a leading zero as octal. + DEADLINE_MIN=$((10#$1)) + shift + ;; + --interval-sec) + shift + [[ $# -gt 0 && "$1" =~ ^[0-9]+$ && $((10#$1)) -gt 0 ]] || usage 1 + INTERVAL_SEC=$((10#$1)) + shift + ;; + --format) + shift + [[ $# -gt 0 ]] || usage 1 + case "$1" in + text | json) FORMAT=$1 ;; + *) usage 1 ;; + esac + shift + ;; + --quiet) + QUIET=1 + shift + ;; + -h | --help) usage 0 ;; + -*) usage 1 ;; + *) + [[ -z "$PR" ]] || usage 1 + PR=$1 + shift + ;; + esac +done + +[[ -n "$PR" && "$PR" =~ ^[0-9]+$ ]] || usage 1 +# Normalize so a value like 0012 is 12 everywhere, including `jq --argjson`. +PR=$((10#$PR)) +# "0" and "00" both pass the pattern above and normalize to 0, which is not a +# PR number GitHub ever issues; reject it here instead of chasing it through +# three failed pr-state reads as a confusing blocked-access outcome. +[[ "$PR" -gt 0 ]] || usage 1 + +progress() { + [[ "$QUIET" -eq 1 ]] || printf '%s\n' "$*" >&2 +} + +# Mergeability is deliberately absent from `pr-state --summary`, so it is +# queried separately below rather than letting a caller assume a clean merge +# state. There is no `|| {}` fallback: a failed query is unknown state, and the +# caller must treat it that way instead of reading an empty object as +# "not conflicting". + +TIMEOUT_BIN='' +TIMEOUT_RESOLVED=0 +resolve_timeout_bin() { + [[ "$TIMEOUT_RESOLVED" -eq 0 ]] || return 0 + TIMEOUT_RESOLVED=1 + # Set PR_AWAIT_NO_TIMEOUT=1 to exercise the shell fallback on a host that does + # have timeout(1). The fallback is the path stock macOS takes, so it needs + # coverage somewhere other than stock macOS. + [[ -z "${PR_AWAIT_NO_TIMEOUT:-}" ]] || return 0 + local candidate + for candidate in timeout gtimeout; do + if command -v "$candidate" >/dev/null 2>&1; then + TIMEOUT_BIN="$candidate" + return 0 + fi + done +} + +stop_active_read() { + local pid="${ACTIVE_READ_PID:-}" + local group="${ACTIVE_READ_GROUP:-}" + local watchdog="${ACTIVE_WATCHDOG_PID:-}" + + # The bounded command is a job-control process-group leader on the fallback + # path. Stop the group first so a pr-state shell cannot leave its gh child + # running. The KILL fallback makes cancellation deterministic if a child + # ignores TERM. The direct kill also covers timeout(1), whose child group is + # not portable across implementations. + if [[ -n "$group" ]]; then + kill -TERM -- "-$group" 2>/dev/null || true + kill -KILL -- "-$group" 2>/dev/null || true + fi + [[ -z "$pid" ]] || kill -TERM "$pid" 2>/dev/null || true + [[ -z "$pid" ]] || kill -KILL "$pid" 2>/dev/null || true + [[ -z "$watchdog" ]] || kill -KILL "$watchdog" 2>/dev/null || true +} + +# Run a command with a wall-clock bound, keeping its stdout. Used for the GitHub +# reads so a stalled `pr-state` or `gh pr view` cannot outlive the caller's +# deadline: the poll loop can only check the clock between reads, never during. +run_bounded_capture() { + local secs=$1 outfile=$2 + shift 2 + [[ -z "$outfile" ]] || : >"$outfile" + + resolve_timeout_bin + if [[ -n "$TIMEOUT_BIN" ]]; then + local had_monitor=0 + case "$-" in + *m*) had_monitor=1 ;; + esac + set -m + if [[ -n "$outfile" ]]; then + "$TIMEOUT_BIN" "$secs" "$@" >"$outfile" 2>/dev/null & + else + "$TIMEOUT_BIN" "$secs" "$@" >/dev/null 2>&1 & + fi + local pid=$! + [[ "$had_monitor" -eq 1 ]] || set +m + ACTIVE_READ_PID="$pid" + ACTIVE_READ_GROUP="$pid" + local rc=0 + wait "$pid" 2>/dev/null || rc=$? + ACTIVE_READ_PID='' + ACTIVE_READ_GROUP='' + return "$rc" + fi + + # Fallback for hosts with no timeout(1), stock macOS among them. + # + # Job control makes the background job a process-group leader so the watchdog + # can kill the whole tree; killing the pr-state shell alone leaves the `gh` it + # is blocked on running. The watchdog polls in one-second steps and exits by + # itself once the command finishes, because a shell blocked in `sleep N` does + # not act on a signal until that sleep returns. Its fds are closed because + # callers capture this with $(...), and any background process holding that + # pipe keeps the substitution blocked afterwards. + local had_monitor=0 + case "$-" in + *m*) had_monitor=1 ;; + esac + set -m + if [[ -n "$outfile" ]]; then + "$@" >"$outfile" 2>/dev/null & + else + "$@" >/dev/null 2>&1 & + fi + local pid=$! + [[ "$had_monitor" -eq 1 ]] || set +m + ACTIVE_READ_PID="$pid" + ACTIVE_READ_GROUP="$pid" + + ( + waited=0 + while [[ "$waited" -lt "$secs" ]]; do + sleep 1 + kill -0 "$pid" 2>/dev/null || exit 0 + waited=$((waited + 1)) + done + kill -9 -"$pid" 2>/dev/null || kill -9 "$pid" 2>/dev/null + ) >/dev/null 2>&1 & + ACTIVE_WATCHDOG_PID=$! + + local rc=0 + wait "$pid" 2>/dev/null || rc=$? + kill "$ACTIVE_WATCHDOG_PID" 2>/dev/null || true + wait "$ACTIVE_WATCHDOG_PID" 2>/dev/null || true + ACTIVE_WATCHDOG_PID='' + ACTIVE_READ_PID='' + ACTIVE_READ_GROUP='' + # 137 is SIGKILL, which here can only have come from the watchdog. + [[ "$rc" -ne 137 ]] || return 124 + return "$rc" +} + +# Seconds left before the caller's deadline, floored at zero. +remaining_budget() { + local left=$((DEADLINE_SEC - (SECONDS - START_SEC))) + [[ "$left" -gt 0 ]] || left=0 + printf '%s' "$left" +} + +# Cap on a single GitHub read. A healthy `pr-state --summary` returns in about +# ten seconds; three minutes means it is wedged, and waiting out a 45 minute +# default deadline on one wedged read helps nobody. +READ_CAP=180 + +# A zero or already-spent deadline still gets exactly one snapshot, so the +# report can name the pending checks rather than saying nothing at all. That +# read is bounded tightly: a healthy read returns in about ten seconds, and a +# caller who asked for no waiting should not get a long one. +READ_FLOOR="${PR_AWAIT_READ_FLOOR:-15}" + +# Recorded in every report as provenance, not as a gate. `pr-state` used to fail +# silently on jq 1.6 and on the bash 3.2 that macOS ships as /bin/bash, and this +# script refused to run there. Both are fixed at the source now, and blocking on +# a version would only turn away working installations. +# +# What replaced the gate is stronger than it was: the poll loop rejects a +# snapshot that reports `errors`, one whose unresolved-thread count is null, and +# one that does not parse. Those test the data rather than a proxy for it, so +# they catch a degraded pr-state whatever the cause, including a checkout that +# predates the fix. +record_toolchain() { + local jq_bin="${PR_AWAIT_JQ:-jq}" + local bash_bin="${PR_AWAIT_PROBE_BASH:-bash}" + JQ_VERSION="$("$jq_bin" --version 2>/dev/null || printf 'unknown')" + BASH_VERSION_SEEN="$("$bash_bin" -c 'printf "%s" "${BASH_VERSION%%(*}"' 2>/dev/null || printf 'unknown')" + TOOLCHAIN="$JQ_VERSION, bash $BASH_VERSION_SEEN" +} + +# Appended to a blocked report only. A reader whose pr-state produced nothing +# usable is the one person who needs to know their toolchain is a known cause. +toolchain_hint() { + local hint='' + case "$JQ_VERSION" in + jq-1.[0-6] | jq-1.[0-6].* | jq-1.[0-6]-*) + hint="$JQ_VERSION loops on an empty-matching gsub; a pr-state predating that fix hangs on it." ;; + esac + case "$BASH_VERSION_SEEN" in + [123].*) + hint="${hint:+$hint }bash $BASH_VERSION_SEEN errors on an empty array under set -u; a pr-state predating that fix loses review threads there." ;; + esac + [[ -z "$hint" ]] || printf '\nToolchain note: %s\n' "$hint" +} + +TOOLCHAIN='unverified' +JQ_VERSION='unknown' +BASH_VERSION_SEEN='unknown' +record_toolchain + +READ_TMP="$(mktemp "${TMPDIR:-/tmp}/pr-await-read-XXXXXX")" +cleanup_read_tmp() { + [[ -z "${READ_TMP:-}" ]] || rm -f "$READ_TMP" +} + +handle_signal() { + local code=$1 + stop_active_read + exit "$code" +} + +trap cleanup_read_tmp EXIT +trap 'handle_signal 130' INT +trap 'handle_signal 143' TERM + +DEADLINE_SEC=$((DEADLINE_MIN * 60)) + +# Sleep until the next poll unless the deadline arrives first, capping the last +# sleep to whatever time is left. Returns 1 when the caller must stop, so a +# retry path cannot outlive a limit the caller asked for. +sleep_or_stop() { + local remaining=$((DEADLINE_SEC - (SECONDS - START_SEC))) + [[ "$remaining" -gt 0 ]] || return 1 + if [[ "$remaining" -lt "$INTERVAL_SEC" ]]; then + "$SLEEP" "$remaining" + else + "$SLEEP" "$INTERVAL_SEC" + fi +} +START_SEC=$SECONDS +POLLS=0 +CONSECUTIVE_ERRORS=0 +MERGE_ERRORS=0 +MERGE_TIMED_OUT=0 +EVIDENCE_ERRORS=0 +MERGE_UNKNOWN=0 +HEAD_CHANGES=0 +FIRST_HEAD='' +SUMMARY_JSON='' +MERGE_JSON='' +OUTCOME='' + +while :; do + POLLS=$((POLLS + 1)) + ELAPSED=$((SECONDS - START_SEC)) + + # A spent deadline still buys one snapshot, but one for the whole iteration: + # granting the floor again to the second query would let --deadline-min 0 wait + # out two floor-length reads. + floor_start='' + budget="$(remaining_budget)" + if [[ "$budget" -le 0 ]]; then + if [[ -n "$SUMMARY_JSON" ]]; then + OUTCOME='deadline' + break + fi + floor_start=$SECONDS + budget="$READ_FLOOR" + fi + [[ "$budget" -le "$READ_CAP" ]] || budget="$READ_CAP" + + read_rc=0 + run_bounded_capture "$budget" "$READ_TMP" "$PR_STATE" --summary "$PR" || read_rc=$? + if [[ "$read_rc" -eq 124 ]]; then + progress "poll ${POLLS}: pr-state exceeded its ${budget}s bound" + OUTCOME='deadline' + [[ -n "$SUMMARY_JSON" ]] || OUTCOME='blocked-access' + break + fi + summary_body="$(cat "$READ_TMP")" + # A non-zero read is a failed read, whatever it printed. Checking only the + # parse would let a pr-state that exited 1 through on an empty snapshot. + # Staged in a local first: assigning straight into SUMMARY_JSON would + # clobber the last good snapshot with this failed read's garbage, so a + # deadline reached right after would lose evidence a working poll had + # already produced. + if [[ "$read_rc" -ne 0 ]] || ! jq -e . >/dev/null 2>&1 <<<"$summary_body"; then + CONSECUTIVE_ERRORS=$((CONSECUTIVE_ERRORS + 1)) + progress "poll ${POLLS}: pr-state failed (${CONSECUTIVE_ERRORS} in a row)" + # Three consecutive failures is a lost connection, not a transient blip. + # Reporting "blocked" beats looping silently against revoked access. + if [[ "$CONSECUTIVE_ERRORS" -ge 3 ]]; then + OUTCOME='blocked-access' + break + fi + if ! sleep_or_stop; then + OUTCOME='blocked-access' + break + fi + continue + fi + SUMMARY_JSON="$summary_body" + CONSECUTIVE_ERRORS=0 + + budget="$(remaining_budget)" + if [[ "$budget" -le 0 ]]; then + if [[ -n "$MERGE_JSON" ]]; then + OUTCOME='deadline' + break + fi + if [[ -n "$floor_start" ]]; then + # Whatever the first read left of this iteration's single floor. + budget=$((READ_FLOOR - (SECONDS - floor_start))) + [[ "$budget" -gt 0 ]] || budget=1 + else + floor_start=$SECONDS + budget="$READ_FLOOR" + fi + fi + [[ "$budget" -le "$READ_CAP" ]] || budget="$READ_CAP" + + read_rc=0 + run_bounded_capture "$budget" "$READ_TMP" \ + "$GH" pr view "$PR" --json state,mergeable,mergeStateStatus,headRefOid || read_rc=$? + if [[ "$read_rc" -eq 124 ]]; then + progress "poll ${POLLS}: gh pr view exceeded its ${budget}s bound" + OUTCOME='blocked-merge-state' + MERGE_TIMED_OUT=1 + break + fi + merge_body="$(cat "$READ_TMP")" + # Staged in a local first, same reason as the pr-state read above: a failed + # read must not overwrite the last good MERGE_JSON with unusable text. + if [[ "$read_rc" -ne 0 ]] || ! jq -e . >/dev/null 2>&1 <<<"$merge_body"; then + MERGE_ERRORS=$((MERGE_ERRORS + 1)) + progress "poll ${POLLS}: gh pr view failed (${MERGE_ERRORS} in a row)" + if [[ "$MERGE_ERRORS" -ge 3 ]]; then + OUTCOME='blocked-merge-state' + break + fi + if ! sleep_or_stop; then + OUTCOME='blocked-merge-state' + break + fi + continue + fi + MERGE_JSON="$merge_body" + MERGE_ERRORS=0 + + pr_state="$(jq -r '.state // ""' <<<"$MERGE_JSON")" + head_oid="$(jq -r '.headRefOid // ""' <<<"$MERGE_JSON")" + [[ -n "$head_oid" ]] || head_oid="$(jq -r '.pr.head_ref_oid // ""' <<<"$SUMMARY_JSON")" + [[ -n "$FIRST_HEAD" ]] || FIRST_HEAD="$head_oid" + + # A push during the wait restarts CI, so every check counted so far belongs to + # a head that no longer exists. Discard that evidence, but not the caller's + # clock: --deadline-min is a hard stop, and a PR receiving periodic commits + # would otherwise defer it forever. + if [[ -n "$head_oid" && "$head_oid" != "$FIRST_HEAD" ]]; then + HEAD_CHANGES=$((HEAD_CHANGES + 1)) + progress "poll ${POLLS}: head moved ${FIRST_HEAD:0:9} -> ${head_oid:0:9}; discarding stale evidence" + FIRST_HEAD="$head_oid" + EVIDENCE_ERRORS=0 + MERGE_UNKNOWN=0 + # The snapshot in hand describes the previous head. Keeping it would let a + # deadline reached before the next read print those checks under the new + # SHA; with no snapshot the run reports blocked, which is the truth. + SUMMARY_JSON='' + if ! sleep_or_stop; then + OUTCOME='deadline' + break + fi + continue + fi + + if [[ "$pr_state" == 'MERGED' || "$pr_state" == 'CLOSED' ]]; then + OUTCOME='blocked-pr-state' + break + fi + + failed="$(jq -r '.failed_checks | length' <<<"$SUMMARY_JSON")" + pending="$(jq -r '.pending_checks | length' <<<"$SUMMARY_JSON")" + check_count="$(jq -r '.check_count // 0' <<<"$SUMMARY_JSON")" + passed="$(jq -r '.passed_check_count // 0' <<<"$SUMMARY_JSON")" + complete="$(jq -r '.checks_snapshot_complete' <<<"$SUMMARY_JSON")" + checks_head="$(jq -r '.checks_head_sha // ""' <<<"$SUMMARY_JSON")" + + progress "poll ${POLLS} (+$((SECONDS - START_SEC))s): ${passed} passed, ${failed} failed, ${pending} pending" + + # Everything below decides on check evidence, so a snapshot describing a head + # that has already been replaced is discarded outright rather than guarded + # against case by case. Every snapshot retained past this point is therefore + # current-head by construction, and a deadline reached with none in hand + # reports blocked instead of printing the previous head's checks under the + # new SHA. + if [[ -z "$checks_head" || "$checks_head" != "$head_oid" ]]; then + progress "poll ${POLLS}: snapshot is for ${checks_head:0:9}, head is ${head_oid:0:9}; discarding" + SUMMARY_JSON='' + if ! sleep_or_stop; then + OUTCOME='deadline' + break + fi + continue + fi + + if [[ "$(jq -r '.approval_required_runs | length' <<<"$SUMMARY_JSON")" -gt 0 ]]; then + # action_required reads as green-or-skipped in a naive check count. It is + # blocked verification: waiting for it to finish would never terminate. + OUTCOME='blocked-approval' + break + fi + + # GitHub can expose a new head before Actions has registered any check. An + # empty rollup satisfies "nothing pending" without any evidence behind it; + # in this repository the required jobs always run, so zero means not yet. + if [[ "$check_count" -eq 0 ]]; then + progress "poll ${POLLS}: no checks registered at this head yet; waiting" + if ! sleep_or_stop; then + OUTCOME='deadline' + break + fi + continue + fi + + # A failed job can be visible while its parent workflow is still running, so + # in all-terminal mode a failure is never on its own a reason to stop. + if [[ "$MODE" == 'first-failure' && "$failed" -gt 0 ]]; then + snapshot_errors="$(jq -r '.errors // [] | length' <<<"$SUMMARY_JSON")" + threads_known="$(jq -r 'if .unresolved_review_thread_count == null then "no" else "yes" end' <<<"$SUMMARY_JSON")" + if [[ "$complete" != 'true' || "$snapshot_errors" -gt 0 || "$threads_known" == 'no' ]]; then + EVIDENCE_ERRORS=$((EVIDENCE_ERRORS + 1)) + progress "poll ${POLLS}: first failure visible but evidence incomplete (${EVIDENCE_ERRORS} in a row)" + if [[ "$EVIDENCE_ERRORS" -ge 3 ]]; then + OUTCOME='blocked-evidence' + break + fi + if ! sleep_or_stop; then + OUTCOME='blocked-evidence' + break + fi + continue + fi + EVIDENCE_ERRORS=0 + OUTCOME='terminal' + break + fi + + if [[ "$pending" -eq 0 && "$complete" == 'true' ]]; then + # Terminal by check state, but only trustworthy if the snapshot is whole. + # A partial fetch reports `errors`, and a failed thread query leaves the + # unresolved count null - which `// 0` would otherwise launder into "clean". + snapshot_errors="$(jq -r '.errors // [] | length' <<<"$SUMMARY_JSON")" + threads_known="$(jq -r 'if .unresolved_review_thread_count == null then "no" else "yes" end' <<<"$SUMMARY_JSON")" + if [[ "$snapshot_errors" -gt 0 || "$threads_known" == 'no' ]]; then + EVIDENCE_ERRORS=$((EVIDENCE_ERRORS + 1)) + progress "poll ${POLLS}: checks terminal but evidence incomplete (${EVIDENCE_ERRORS} in a row)" + if [[ "$EVIDENCE_ERRORS" -ge 3 ]]; then + OUTCOME='blocked-evidence' + break + fi + if ! sleep_or_stop; then + OUTCOME='blocked-evidence' + break + fi + continue + fi + EVIDENCE_ERRORS=0 + + # GitHub reports UNKNOWN while it is still computing mergeability. Deciding + # on it would call a PR clean before the answer exists. + if [[ "$(jq -r '.mergeable // "UNKNOWN"' <<<"$MERGE_JSON")" == 'UNKNOWN' ]]; then + MERGE_UNKNOWN=$((MERGE_UNKNOWN + 1)) + progress "poll ${POLLS}: mergeability still UNKNOWN (${MERGE_UNKNOWN} in a row)" + if [[ "$MERGE_UNKNOWN" -ge 3 ]]; then + OUTCOME='blocked-merge-unknown' + break + fi + if ! sleep_or_stop; then + OUTCOME='blocked-merge-unknown' + break + fi + continue + fi + MERGE_UNKNOWN=0 + + OUTCOME='terminal' + break + fi + + if ! sleep_or_stop; then + OUTCOME='deadline' + break + fi +done + +WAITED=$((SECONDS - START_SEC)) + +# A failed read can leave a partial, unparseable body behind, and that value +# survives the `continue` that follows it. Normalize before the report rather +# than letting `jq --argjson` abort under `set -e` with no output at all. +jq -e . >/dev/null 2>&1 <<<"$MERGE_JSON" || MERGE_JSON='{}' + +if [[ -z "$SUMMARY_JSON" ]] || ! jq -e . >/dev/null 2>&1 <<<"$SUMMARY_JSON"; then + emit_blocked 'blocked-access' \ +"BLOCKED: no usable pr-state snapshot was obtained; treat CI state as unknown." +fi + +if [[ "$OUTCOME" == 'blocked-access' ]]; then + emit_blocked 'blocked-access' \ +"BLOCKED: pr-state failed $CONSECUTIVE_ERRORS times in a row; treat CI state as unknown." +fi + +if [[ "$OUTCOME" == 'blocked-merge-state' ]]; then + if [[ "$MERGE_TIMED_OUT" -eq 1 ]]; then + emit_blocked 'blocked-merge-state' \ +"BLOCKED: gh pr view exceeded its read bound, so merge state is unknown. +A conflicted or closed PR is indistinguishable from a clean one here." + fi + emit_blocked 'blocked-merge-state' \ +"BLOCKED: gh pr view failed $MERGE_ERRORS times in a row, so merge state is unknown. +A conflicted or closed PR is indistinguishable from a clean one here." +fi + +failed_n="$(jq -r '.failed_checks | length' <<<"$SUMMARY_JSON")" +pending_n="$(jq -r '.pending_checks | length' <<<"$SUMMARY_JSON")" +passed_n="$(jq -r '.passed_check_count // 0' <<<"$SUMMARY_JSON")" +# `unresolved_review_thread_count` is every unresolved thread (pr-state:922); +# `hidden_unresolved_threads` is the subset filtered out of `unresolved_threads`, +# not an addition to it. Summing the two counts the hidden ones twice. +threads_total_n="$(jq -r '.unresolved_review_thread_count // 0' <<<"$SUMMARY_JSON")" +visible_n="$(jq -r '.unresolved_threads | length' <<<"$SUMMARY_JSON")" +hidden_n="$(jq -r '.hidden_unresolved_threads | length' <<<"$SUMMARY_JSON")" +comments_n="$(jq -r '.actionable_issue_comment_count // 0' <<<"$SUMMARY_JSON")" +changes_req_n="$(jq -r '.review_evidence.active_changes_requested_count // 0' <<<"$SUMMARY_JSON")" +blocked_rev_n="$(jq -r '.review_evidence.blocked_exact_current_head_review_count // 0' <<<"$SUMMARY_JSON")" +evidence_errors_n="$(jq -r '.errors // [] | length' <<<"$SUMMARY_JSON")" +error_sources="$(jq -r '[.errors // [] | .[].source] | join(", ")' <<<"$SUMMARY_JSON")" +threads_known_final="$(jq -r 'if .unresolved_review_thread_count == null then "no" else "yes" end' <<<"$SUMMARY_JSON")" +base_advanced="$(jq -r 'if .pr.base_advanced_since_head == true then 1 else 0 end' <<<"$SUMMARY_JSON")" +mergeable="$(jq -r '.mergeable // "UNKNOWN"' <<<"$MERGE_JSON")" +merge_status="$(jq -r '.mergeStateStatus // "UNKNOWN"' <<<"$MERGE_JSON")" + +conflicted=0 +[[ "$mergeable" == 'CONFLICTING' || "$merge_status" == 'DIRTY' ]] && conflicted=1 + +# A CHANGES_REQUESTED review, or one whose body carries an explicit blocker, +# can exist with no unresolved thread and no issue comment attached. Counting +# only threads would report such a PR clean. +findings=$((failed_n + threads_total_n + comments_n + conflicted + changes_req_n + blocked_rev_n + base_advanced)) + +case "$OUTCOME" in + blocked-pr-state | blocked-approval | blocked-evidence | blocked-merge-unknown) code=3 ;; + deadline) code=2 ;; + *) [[ "$findings" -gt 0 ]] && code=1 || code=0 ;; +esac + +if [[ "$FORMAT" == 'json' ]]; then + jq -n \ + --argjson pr "$PR" \ + --argjson summary "$SUMMARY_JSON" \ + --argjson merge "$MERGE_JSON" \ + --arg outcome "$OUTCOME" \ + --arg mode "$MODE" \ + --argjson waited_sec "$WAITED" \ + --argjson polls "$POLLS" \ + --argjson head_changes "$HEAD_CHANGES" \ + --argjson exit_code "$code" \ + --arg toolchain "$TOOLCHAIN" \ + --argjson deadline_sec "$DEADLINE_SEC" \ + --argjson interval_sec "$INTERVAL_SEC" \ + --argjson evidence_complete "$([[ "$evidence_errors_n" -eq 0 && "$threads_known_final" == 'yes' ]] && echo true || echo false)" \ + '{pr: $pr, outcome: $outcome, mode: $mode, waited_sec: $waited_sec, polls: $polls, + head_changes: $head_changes, exit_code: $exit_code, + message: null, toolchain: $toolchain, evidence_complete: $evidence_complete, + deadline_sec: $deadline_sec, interval_sec: $interval_sec, + mergeable: ($merge.mergeable // "UNKNOWN"), + merge_state_status: ($merge.mergeStateStatus // "UNKNOWN"), + summary: $summary}' + exit "$code" +fi + +branch="$(jq -r '.pr.head_ref_name // "?"' <<<"$SUMMARY_JSON")" +base="$(jq -r '.pr.base_ref_name // "?"' <<<"$SUMMARY_JSON")" +printf 'PR %s @ %s (%s -> %s)\n' "$PR" "${FIRST_HEAD:0:9}" "$branch" "$base" +printf 'gate: %s | outcome: %s | waited %dm%02ds over %d poll(s)\n' \ + "$MODE" "$OUTCOME" "$((WAITED / 60))" "$((WAITED % 60))" "$POLLS" +[[ "$HEAD_CHANGES" -gt 0 ]] && printf 'note: head moved %d time(s) during the wait\n' "$HEAD_CHANGES" +printf 'checks: %s passed, %s failed, %s pending\n' "$passed_n" "$failed_n" "$pending_n" +printf 'merge: %s / %s\n' "$mergeable" "$merge_status" +printf 'toolchain: %s\n' "$TOOLCHAIN" + +if [[ "$failed_n" -gt 0 ]]; then + printf '\nFAILED CHECKS:\n' + jq -r '.failed_checks[] | " \(.name) run=\(.run_id // "?") job=\(.job_id // "?") \(.conclusion // "?")"' <<<"$SUMMARY_JSON" +fi + +if [[ "$pending_n" -gt 0 ]]; then + printf '\nSTILL PENDING:\n' + jq -r '.pending_checks[] | " \(.name) run=\(.run_id // "?")"' <<<"$SUMMARY_JSON" +fi + +if [[ "$threads_total_n" -gt 0 ]]; then + printf '\nUNRESOLVED THREADS (%s total: %s visible, %s hidden):\n' \ + "$threads_total_n" "$visible_n" "$hidden_n" + jq -r '(.unresolved_threads + .hidden_unresolved_threads)[] + | " \(.thread_id) c=\(.comment_id) \(.author) \(.path // "-")"' <<<"$SUMMARY_JSON" +fi + +[[ "$comments_n" -gt 0 ]] && printf '\nACTIONABLE ISSUE COMMENTS: %s\n' "$comments_n" + +if [[ "$base_advanced" -eq 1 ]]; then + # code-review/SKILL.md: mergeable=MERGEABLE proves conflict compatibility, not + # that the merged result was tested. Green checks here ran against an older base. + printf '\nBASE ADVANCED since this head: checks did not run against the current base.\n' + printf ' Validate the merge result before treating this PR as ready.\n' +fi + +if [[ "$changes_req_n" -gt 0 || "$blocked_rev_n" -gt 0 ]]; then + printf '\nBLOCKING REVIEWS: %s active changes requested, %s blocked at this head\n' \ + "$changes_req_n" "$blocked_rev_n" + jq -r '.review_evidence.blocked_exact_current_head_reviews // [] | .[] + | " \(.author) \(.verdict // .state // "?")"' <<<"$SUMMARY_JSON" 2>/dev/null || true +fi + +if [[ "$evidence_errors_n" -gt 0 || "$threads_known_final" == 'no' ]]; then + printf '\nINCOMPLETE EVIDENCE: %s\n' "${error_sources:-unresolved review thread count is null}" +fi + +printf '\nNEXT: ' +case "$OUTCOME" in + blocked-pr-state) printf 'PR is %s. Do not push a stale head.\n' "$pr_state" ;; + blocked-approval) printf 'workflow approval required; CI cannot complete unattended.\n' ;; + blocked-evidence) + printf 'checks finished but review evidence is incomplete (%s). Treat as unknown, not clean.\n' \ + "${error_sources:-null unresolved thread count}" ;; + blocked-merge-unknown) + printf 'GitHub still reports mergeability as UNKNOWN. Re-run before treating this head as clean.\n' ;; + deadline) + if [[ "$(jq -r '.check_count // 0' <<<"$SUMMARY_JSON")" -eq 0 ]]; then + printf 'deadline hit with no checks registered yet at this head. Re-run or raise --deadline-min.\n' + else + printf 'deadline hit with %s check(s) pending. Re-run or raise --deadline-min.\n' "$pending_n" + fi + ;; + *) + if [[ "$conflicted" -eq 1 ]]; then + printf 'resolve the merge conflict before triaging checks or threads.\n' + elif [[ "$findings" -eq 0 ]]; then + printf 'clean at this head.\n' + elif [[ "$failed_n" -eq 0 && "$threads_total_n" -eq 0 ]]; then + # The finding is entirely from a category the "N failed / M thread" + # phrasing below cannot name: an issue comment, a blocking review, or an + # advanced base. Saying "fix 0 failed check(s) and 0 thread(s)" here + # would tell the caller there is nothing to do while findings > 0. + printf 'review the findings reported above (comments, blocking reviews, or an advanced base), then push once.\n' + else + printf 'fix %s failed check(s) and %s thread(s) in one pass, then push once.\n' \ + "$failed_n" "$threads_total_n" + fi + ;; +esac + +exit "$code" diff --git a/scripts/pr-await.test.sh b/scripts/pr-await.test.sh new file mode 100755 index 0000000000..0f70a5dcb3 --- /dev/null +++ b/scripts/pr-await.test.sh @@ -0,0 +1,859 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SCRIPT="$ROOT_DIR/scripts/pr-await" + +pass() { printf 'ok - %s\n' "$1"; } +fail() { + printf 'not ok - %s\n' "$1" >&2 + [[ $# -lt 2 ]] || printf '%s\n' "$2" >&2 + exit 1 +} + +TMP_DIRS=() +cleanup() { [[ "${#TMP_DIRS[@]}" -eq 0 ]] || rm -rf "${TMP_DIRS[@]}"; } +trap cleanup EXIT + +make_tmp_dir() { + local d + d="$(mktemp -d)" + TMP_DIRS+=("$d") + printf '%s' "$d" +} + +# A snapshot generator. Each call to the fake pr-state pops the next file from +# a numbered sequence, so a test can describe CI as it evolves over time. +# The last snapshot repeats once the sequence is exhausted. +setup_fake() { + local dir=$1 + mkdir -p "$dir/seq" + cat >"$dir/pr-state" <<'FAKE' +#!/usr/bin/env bash +n=$(cat "$FAKE_DIR/counter" 2>/dev/null || echo 0) +n=$((n + 1)) +printf '%s' "$n" > "$FAKE_DIR/counter" +last=$(ls "$FAKE_DIR"/seq/*.json 2>/dev/null | wc -l | tr -d ' ') +[[ $n -le $last ]] || n=$last +if [[ -f "$FAKE_DIR/seq/$n.fail" ]]; then exit 1; fi +cat "$FAKE_DIR/seq/$n.json" +FAKE + cat >"$dir/gh" <<'FAKEGH' +#!/usr/bin/env bash +cat "$FAKE_DIR/gh.json" 2>/dev/null || printf '{"state":"OPEN","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"aaaaaaaaaaaa"}' +FAKEGH + cat >"$dir/jq-ok" <<'JQOK' +#!/usr/bin/env bash +[ "$1" = "--version" ] && { echo "jq-1.7.1-fake"; exit 0; } +exit 0 +JQOK + cat >"$dir/jq-bad" <<'JQBAD' +#!/usr/bin/env bash +[ "$1" = "--version" ] && { echo "jq-1.6-fake"; exit 0; } +exec sleep 30 +JQBAD + cat >"$dir/bash-ok" <<'BASHOK' +#!/usr/bin/env bash +case "$2" in + *BASH_VERSION*) printf '5.9.9-fake' ;; + *) exit 0 ;; +esac +BASHOK + cat >"$dir/bash-bad" <<'BASHBAD' +#!/usr/bin/env bash +case "$2" in + *BASH_VERSION*) printf '3.2.57-fake' ;; + *) exit 1 ;; +esac +BASHBAD + chmod +x "$dir/jq-ok" "$dir/jq-bad" "$dir/bash-ok" "$dir/bash-bad" + cat >"$dir/sleep" <<'FAKESLEEP' +#!/usr/bin/env bash +printf 'slept %s\n' "$1" >> "$FAKE_DIR/sleeps" +FAKESLEEP + chmod +x "$dir/pr-state" "$dir/gh" "$dir/sleep" +} + +snapshot() { + # snapshot [complete] [head] [extra-json] + local dir=$1 idx=$2 passed=$3 failed=$4 pending=$5 + local complete=${6:-true} head=${7:-aaaaaaaaaaaa} extra=${8:-'{}'} + local failed_arr='[]' pending_arr='[]' + [[ "$failed" -eq 0 ]] || failed_arr="$(jq -nc --argjson n "$failed" '[range($n) | {name: "Failing \(.)", workflow: "w", status: "completed", conclusion: "failure", run_id: 100, job_id: 200}]')" + [[ "$pending" -eq 0 ]] || pending_arr="$(jq -nc --argjson n "$pending" '[range($n) | {name: "Pending \(.)", workflow: "w", status: "in_progress", run_id: 101, job_id: 201}]')" + jq -n --argjson f "$failed_arr" --argjson p "$pending_arr" --argjson passed "$passed" \ + --argjson failed "$failed" --argjson pending "$pending" \ + --argjson complete "$complete" --arg head "$head" --argjson extra "$extra" \ + '{pr: {number: 1, head_ref_name: "feat", base_ref_name: "main", head_ref_oid: $head}, + failed_checks: $f, pending_checks: $p, passed_check_count: $passed, + check_count: ($passed + $failed + $pending), + checks_head_sha: $head, checks_snapshot_complete: $complete, + approval_required_runs: [], unresolved_review_thread_count: 0, + unresolved_threads: [], hidden_unresolved_threads: [], + actionable_issue_comment_count: 0, errors: [], + review_evidence: {active_changes_requested_count: 0, + blocked_exact_current_head_review_count: 0}} * $extra' > "$dir/seq/$idx.json" +} + +run_await() { + local dir=$1; shift + FAKE_DIR="$dir" PR_AWAIT_PR_STATE="$dir/pr-state" PR_AWAIT_GH="$dir/gh" \ + PR_AWAIT_SLEEP="$dir/sleep" \ + PR_AWAIT_JQ="${PROBE_JQ:-$dir/jq-ok}" PR_AWAIT_PROBE_BASH="${PROBE_BASH:-$dir/bash-ok}" \ + "$SCRIPT" "$@" +} + +assert_json_envelope() { + local name=$1 json=$2 + jq -e ' + (keys | sort) == [ + "deadline_sec", "evidence_complete", "exit_code", "head_changes", + "interval_sec", "merge_state_status", "mergeable", "message", "mode", + "outcome", "polls", "pr", "summary", "toolchain", "waited_sec" + ] + and (.pr | type) == "number" + and (.message == null or (.message | type) == "string") + and (.summary == null or (.summary | type) == "object") + ' <<<"$json" >/dev/null || fail "$name" "$json" +} + +# --- argument validation --------------------------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +run_await "$d" >/dev/null 2>&1 && fail "missing PR should exit non-zero" || pass "missing PR argument is rejected" +run_await "$d" 12 --mode bogus >/dev/null 2>&1 && fail "bad --mode accepted" || pass "invalid --mode is rejected" +run_await "$d" notanumber >/dev/null 2>&1 && fail "non-numeric PR accepted" || pass "non-numeric PR is rejected" +run_await "$d" 0 >/dev/null 2>&1 && fail "PR 0 accepted" || pass "PR #0 is rejected as a usage error" +run_await "$d" 00 >/dev/null 2>&1 && fail "PR 00 accepted" || pass "PR #00 is rejected as a usage error" +run_await "$d" 12 --interval-sec 0 >/dev/null 2>&1 && fail "zero interval accepted" || pass "zero --interval-sec is rejected" +"$SCRIPT" --help >/dev/null 2>&1 && pass "--help exits 0" || fail "--help should exit 0" + +# --- the documented race: a failure while the workflow is still running ----- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 10 1 5 # a job has failed, but 5 checks still pending +snapshot "$d" 2 12 1 3 +snapshot "$d" 3 14 2 0 # terminal: a SECOND failure only appeared at the end +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "all-terminal with findings should exit 1, got $rc" "$out" +grep -q '2 failed' <<<"$out" || fail "should report BOTH failures, not just the first" "$out" +grep -q '0 pending' <<<"$out" || fail "should only return when nothing is pending" "$out" +pass "all-terminal waits past an early failure and reports every failure at once" + +# --- first-failure returns early (and therefore reports less) -------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 10 1 5 +snapshot "$d" 2 14 2 0 +out="$(run_await "$d" 12 --mode first-failure --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "first-failure with a failure should exit 1, got $rc" "$out" +grep -q '1 failed' <<<"$out" || fail "first-failure should return on the first failure" "$out" +grep -q '5 pending' <<<"$out" || fail "first-failure returns with checks still pending" "$out" +pass "first-failure returns on the first failing check" + +# --- first-failure requires complete evidence before returning -------------- +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3; do + snapshot "$d" "$i" 10 1 2 false aaaaaaaaaaaa \ + '{"errors":[{"source":"review_threads","message":"graphql failed"}]}' +done +out="$(run_await "$d" 12 --mode first-failure --interval-sec 1 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "incomplete first-failure evidence should block, got $rc" "$out" +[[ "$(jq -r '.outcome' <<<"$out")" == 'blocked-evidence' ]] \ + || fail "incomplete first-failure evidence should report blocked-evidence" "$out" +assert_json_envelope "blocked-evidence must use the JSON envelope" "$out" +pass "first-failure waits for complete evidence before returning a failure" + +# --- clean terminal state --------------------------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "clean state should exit 0, got $rc" "$out" +grep -q 'clean at this head' <<<"$out" || fail "clean state should say so" "$out" +pass "clean terminal state exits 0" + +# --- an incomplete snapshot is not terminal -------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 false +snapshot "$d" 2 20 0 0 true +out="$(run_await "$d" 12 --interval-sec 1 2>"$d/err")" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "expected 0 after snapshot completed, got $rc" "$out" +grep -q '2 poll' <<<"$out" || fail "should have polled twice, not stopped on the incomplete snapshot" "$out" +pass "checks_snapshot_complete=false keeps waiting" + +# --- deadline --------------------------------------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 10 0 4 +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 2 ]] || fail "deadline should exit 2, got $rc" "$out" +grep -q 'STILL PENDING' <<<"$out" || fail "deadline should name pending checks" "$out" +grep -q 'Pending 0' <<<"$out" || fail "deadline should list check names" "$out" +pass "deadline exits 2 and names the pending checks" + +# --- approval-required is blocked, not green -------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 0 0 0 true aaaaaaaaaaaa \ + '{"approval_required_runs":[{"run_id":9,"name":"E2E","conclusion":"action_required"}]}' +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "approval_required should exit 3, got $rc" "$out" +grep -q 'approval required' <<<"$out" || fail "should explain the block" "$out" +pass "approval_required_runs exits 3 instead of reading as clean" + +# --- merged/closed PR ------------------------------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 10 0 2 +printf '{"state":"MERGED","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"aaaaaaaaaaaa"}' > "$d/gh.json" +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "merged PR should exit 3, got $rc" "$out" +grep -q 'MERGED' <<<"$out" || fail "should report the merged state" "$out" +pass "a merged PR exits 3 rather than waiting forever" + +# --- merge conflict --------------------------------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +printf '{"state":"OPEN","mergeable":"CONFLICTING","mergeStateStatus":"DIRTY","headRefOid":"aaaaaaaaaaaa"}' > "$d/gh.json" +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "conflict should exit 1, got $rc" "$out" +grep -q 'resolve the merge conflict' <<<"$out" || fail "conflict should lead the NEXT line" "$out" +pass "a merge conflict is surfaced even when every check passed" + +# --- transient pr-state failures recover; sustained ones block -------------- +d="$(make_tmp_dir)"; setup_fake "$d" +: > "$d/seq/1.fail"; snapshot "$d" 1 0 0 0 +snapshot "$d" 2 20 0 0 +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "one transient failure should be retried, got $rc" "$out" +pass "a transient pr-state failure is retried" + +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3; do : > "$d/seq/$i.fail"; snapshot "$d" "$i" 0 0 0; done +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "three consecutive failures should exit 3, got $rc" "$out" +grep -q 'unknown' <<<"$out" || fail "should say CI state is unknown" "$out" +pass "three consecutive pr-state failures exit 3 as unknown, never as clean" + +# --- a push during the wait restarts the gate ------------------------------ +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 10 0 3 true aaaaaaaaaaaa +snapshot "$d" 2 2 0 8 true bbbbbbbbbbbb +snapshot "$d" 3 20 0 0 true bbbbbbbbbbbb +cat > "$d/gh" <<'GH2' +#!/usr/bin/env bash +n=$(cat "$FAKE_DIR/counter" 2>/dev/null || echo 1) +if [[ $n -le 1 ]]; then oid=aaaaaaaaaaaa; else oid=bbbbbbbbbbbb; fi +printf '{"state":"OPEN","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"%s"}' "$oid" +GH2 +chmod +x "$d/gh" +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "expected clean after the new head settled, got $rc" "$out" +grep -q 'head moved 1 time' <<<"$out" || fail "should report the head change" "$out" +grep -q 'bbbbbbbbb' <<<"$out" || fail "should report the NEW head, not the stale one" "$out" +pass "a push during the wait restarts the gate and reports the new head" + +# --- stdout carries only the report ---------------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 10 0 2 +snapshot "$d" 2 12 0 0 +out="$(run_await "$d" 12 --interval-sec 1 2>/dev/null)" +grep -q '^poll ' <<<"$out" && fail "progress leaked into stdout" "$out" +err="$(run_await "$d" 12 --interval-sec 1 2>&1 >/dev/null)" +grep -q '^poll ' <<<"$err" || fail "progress should go to stderr" "$err" +pass "progress goes to stderr; stdout carries only the final report" + +# --- --quiet silences progress --------------------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 10 0 0 +err="$(run_await "$d" 12 --interval-sec 1 --quiet 2>&1 >/dev/null)" +[[ -z "$err" ]] || fail "--quiet should emit nothing on stderr" "$err" +pass "--quiet silences per-poll progress" + +# --- json format ----------------------------------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 14 2 0 +out="$(run_await "$d" 12 --interval-sec 1 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "json mode should keep the exit code, got $rc" "$out" +jq -e . >/dev/null <<<"$out" || fail "json mode must emit valid JSON" "$out" +[[ "$(jq -r '.outcome' <<<"$out")" == 'terminal' ]] || fail "outcome missing" "$out" +[[ "$(jq -r '.summary.failed_checks | length' <<<"$out")" == '2' ]] || fail "summary should be embedded whole" "$out" +assert_json_envelope "terminal JSON must use the shared envelope" "$out" +[[ "$(jq -r '.pr' <<<"$out")" == '12' ]] || fail "terminal JSON must include the PR number" "$out" +[[ "$(jq -r '.message' <<<"$out")" == 'null' ]] || fail "terminal JSON message must be null" "$out" +pass "--format json embeds the full pr-state summary and keeps the exit code" + +# --- the cost claim: one invocation, not one per poll ---------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3 4 5 6 7; do snapshot "$d" "$i" "$i" 0 $((8 - i)); done +snapshot "$d" 8 20 0 0 +run_await "$d" 12 --interval-sec 1 --quiet >/dev/null 2>&1 || true +[[ "$(wc -l < "$d/sleeps" | tr -d ' ')" -eq 7 ]] || fail "expected 7 internal sleeps" "$(cat "$d/sleeps")" +pass "eight polls happen inside a single invocation (7 internal sleeps, 0 caller round-trips)" + +# --- P1: a snapshot carrying errors is never clean --------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3 4; do + snapshot "$d" "$i" 20 0 0 true aaaaaaaaaaaa \ + '{"errors":[{"source":"review_threads","message":"graphql failed"}]}' +done +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "errors in snapshot must not exit 0, got $rc" "$out" +grep -q 'review_threads' <<<"$out" || fail "should name the failing source" "$out" +pass "a pr-state snapshot with non-empty errors exits 3, never clean" + +# --- P1: a transient error that clears is fine ------------------------------ +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 true aaaaaaaaaaaa '{"errors":[{"source":"x","message":"blip"}]}' +snapshot "$d" 2 20 0 0 +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "a cleared transient error should exit 0, got $rc" "$out" +pass "an error that clears on a later poll does not block" + +# --- P1: an unknown thread count is unknown, not zero ----------------------- +# This is the observed bash-3.2 pr-state failure: pagination dies non-fatally +# and the count comes back null while GitHub has unresolved threads. +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3 4; do + snapshot "$d" "$i" 20 0 0 true aaaaaaaaaaaa '{"unresolved_review_thread_count":null}' +done +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "null thread count must not read as 0/clean, got $rc" "$out" +pass "a null unresolved_review_thread_count blocks instead of counting as zero" + +# --- P1: blocking reviews count as findings --------------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 true aaaaaaaaaaaa \ + '{"review_evidence":{"active_changes_requested_count":1,"blocked_exact_current_head_review_count":0}}' +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "CHANGES_REQUESTED with no threads should exit 1, got $rc" "$out" +grep -qi 'changes requested' <<<"$out" || fail "should report the blocking review" "$out" +pass "an active CHANGES_REQUESTED review is a finding even with zero threads" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 true aaaaaaaaaaaa \ + '{"review_evidence":{"active_changes_requested_count":0,"blocked_exact_current_head_review_count":2}}' +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "blocked exact-head reviews should exit 1, got $rc" "$out" +pass "blocked exact-current-head reviews count as findings" + +# --- P1: a failed merge-state query is unknown, not clean ------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +cat > "$d/gh" <<'GHFAIL' +#!/usr/bin/env bash +exit 1 +GHFAIL +chmod +x "$d/gh" +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "failed gh pr view must not exit 0, got $rc" "$out" +grep -qi 'merge state' <<<"$out" || fail "should say merge state is unknown" "$out" +pass "a failed merge-state query exits 3 instead of reporting clean" + +# --- P2: first-failure must not act on a stale head ------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +# snapshot 1 describes the OLD head; gh already reports the NEW one. +snapshot "$d" 1 10 1 5 true aaaaaaaaaaaa +snapshot "$d" 2 20 0 0 true bbbbbbbbbbbb +cat > "$d/gh" <<'GH3' +#!/usr/bin/env bash +printf '{"state":"OPEN","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"bbbbbbbbbbbb"}' +GH3 +chmod +x "$d/gh" +out="$(run_await "$d" 12 --mode first-failure --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "stale first-failure snapshot must be discarded, got $rc" "$out" +grep -q '0 failed' <<<"$out" || fail "should report the NEW head, not the stale failure" "$out" +pass "first-failure discards a snapshot whose checks belong to a superseded head" + +# --- P2: leading-zero arguments are decimal, not octal --------------------- +# Assert the parsed values rather than timing the wait: "010" as octal is 8, +# which a duration test would only catch by waiting out the difference. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 09 --quiet --format json 2>"$d/err")" || true +grep -q 'value too great for base' "$d/err" && fail "09 must not be read as octal" "$(cat "$d/err")" +[[ "$(jq -r '.deadline_sec' <<<"$out")" == '540' ]] \ + || fail "--deadline-min 09 should be 540s, got $(jq -r '.deadline_sec' <<<"$out")" "$out" +pass "--deadline-min 09 parses as 9 minutes, not an octal error" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(run_await "$d" 12 --interval-sec 010 --deadline-min 010 --quiet --format json 2>"$d/err")" || true +[[ "$(jq -r '.interval_sec' <<<"$out")" == '10' ]] \ + || fail "--interval-sec 010 should be 10, got $(jq -r '.interval_sec' <<<"$out")" "$out" +[[ "$(jq -r '.deadline_sec' <<<"$out")" == '600' ]] \ + || fail "--deadline-min 010 should be 600s (octal would be 480), got $(jq -r '.deadline_sec' <<<"$out")" "$out" +pass "--interval-sec 010 and --deadline-min 010 parse as decimal 10, not octal 8" + + +# --- toolchain preflight: pr-state degrades silently, so refuse to run ------ +# An old toolchain is recorded, not refused. pr-state works on jq 1.6 and bash +# 3.2 now, so a version gate would only turn away working installations; the +# snapshot-quality guards below cover a degraded pr-state whatever its cause. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(PROBE_JQ="$d/jq-bad" run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "an old jq must not block a healthy run, got $rc" "$out" +grep -q 'jq-1.6-fake' <<<"$out" || fail "the old jq should still be recorded" "$out" +pass "an old jq is recorded in the report rather than refused" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(PROBE_BASH="$d/bash-bad" run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "an old bash must not block a healthy run, got $rc" "$out" +grep -q '3.2.57-fake' <<<"$out" || fail "the old bash should still be recorded" "$out" +pass "an old bash is recorded in the report rather than refused" + +# But a reader whose pr-state produced nothing usable needs to know the +# toolchain is a known cause of exactly that. +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3; do : > "$d/seq/$i.fail"; snapshot "$d" "$i" 0 0 0; done +out="$(PROBE_JQ="$d/jq-bad" PROBE_BASH="$d/bash-bad" run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "expected blocked, got $rc" "$out" +grep -qi 'Toolchain note' <<<"$out" || fail "a blocked report should name the toolchain risk" "$out" +grep -qi 'empty-matching gsub' <<<"$out" || fail "should explain the jq risk" "$out" +grep -qi 'empty array under set -u' <<<"$out" || fail "should explain the bash risk" "$out" +pass "a blocked report explains a known-bad toolchain as a possible cause" + +# A current toolchain gets no note, so the hint stays signal. +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3; do : > "$d/seq/$i.fail"; snapshot "$d" "$i" 0 0 0; done +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "expected blocked, got $rc" "$out" +grep -qi 'Toolchain note' <<<"$out" && fail "a current toolchain must not emit a note" "$out" +pass "a current toolchain produces no toolchain note" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "a good toolchain should not block, got $rc" "$out" +grep -q 'toolchain: jq-1.7.1-fake, bash 5.9.9-fake' <<<"$out" \ + || fail "the report must record the verified toolchain" "$out" +pass "a verified toolchain is recorded in every report" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(run_await "$d" 12 --interval-sec 1 --quiet --format json 2>/dev/null)" +[[ "$(jq -r '.toolchain' <<<"$out")" == 'jq-1.7.1-fake, bash 5.9.9-fake' ]] \ + || fail "json report must carry the toolchain" "$out" +[[ "$(jq -r '.evidence_complete' <<<"$out")" == 'true' ]] || fail "evidence_complete missing" "$out" +pass "json report carries toolchain and evidence_complete" + + +# --- mergeable UNKNOWN is inconclusive, not clean --------------------------- +# GitHub returns UNKNOWN while it computes mergeability; merge-conflicts.md +# says query again rather than deciding. +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3 4; do snapshot "$d" "$i" 20 0 0; done +printf '{"state":"OPEN","mergeable":"UNKNOWN","mergeStateStatus":"UNKNOWN","headRefOid":"aaaaaaaaaaaa"}' > "$d/gh.json" +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "persistent mergeable=UNKNOWN must not exit 0, got $rc" "$out" +grep -qi 'mergeability' <<<"$out" || fail "should explain the unknown mergeability" "$out" +pass "a persistently UNKNOWN mergeability exits 3 instead of clean" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0; snapshot "$d" 2 20 0 0 +cat > "$d/gh" <<'GHU' +#!/usr/bin/env bash +n=$(cat "$FAKE_DIR/counter" 2>/dev/null || echo 1) +if [[ $n -le 1 ]]; then m=UNKNOWN; else m=MERGEABLE; fi +printf '{"state":"OPEN","mergeable":"%s","mergeStateStatus":"CLEAN","headRefOid":"aaaaaaaaaaaa"}' "$m" +GHU +chmod +x "$d/gh" +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "UNKNOWN that resolves should exit 0, got $rc" "$out" +pass "an UNKNOWN mergeability that resolves does not block" + +# --- the deadline is honored while retrying, not only while polling --------- +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3 4 5; do : > "$d/seq/$i.fail"; snapshot "$d" "$i" 0 0 0; done +out="$(run_await "$d" 12 --interval-sec 30 --deadline-min 0 --quiet 2>/dev/null)" && rc=0 || rc=$? +: > "$d/sleeps.seen"; [[ -f "$d/sleeps" ]] || : > "$d/sleeps" +[[ "$rc" -eq 3 ]] || fail "expected blocked, got $rc" "$out" +sleeps=$(wc -l < "$d/sleeps" 2>/dev/null | tr -d ' ' || echo 0) +[[ "${sleeps:-0}" -le 1 ]] || fail "a zero deadline must not burn 3 retry intervals, slept $sleeps times" +pass "a retry path stops at the deadline instead of sleeping out its full strike count" + +# --- the jq probe works without timeout(1) on PATH -------------------------- +# Stock macOS ships no /usr/bin/timeout, so a probe that depends on it would +# silently skip on the exact hosts that carry the broken jq. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +cat > "$d/pr-state" <<'HANGNT' +#!/usr/bin/env bash +exec sleep 120 +HANGNT +chmod +x "$d/pr-state" +start=$SECONDS +out="$(PR_AWAIT_NO_TIMEOUT=1 PR_AWAIT_READ_FLOOR=2 run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -ne 0 ]] || fail "a stalled read must be bounded without timeout(1), got $rc" "$out" +[[ $((SECONDS - start)) -lt 12 ]] || fail "the shell fallback did not bound the read" +pass "a stalled read is bounded without timeout(1) available" + +# The fallback must also complete a normal run, not merely catch a hang. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(PR_AWAIT_NO_TIMEOUT=1 run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "the shell fallback must handle a healthy read, got $rc" "$out" +pass "the shell fallback completes a normal run without timeout(1)" + +# --- the suite is wired into make test-scripts ------------------------------ +grep -q 'bash scripts/pr-await.test.sh' "$ROOT_DIR/Makefile" \ + || fail "pr-await.test.sh must be registered in make test-scripts" +pass "the suite runs under make test-scripts" + + +# --- the deadline is absolute, not restarted by every push ----------------- +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3 4 5 6; do snapshot "$d" "$i" 2 0 5 true "head$i"; done +cat > "$d/gh" <<'GHMOVE' +#!/usr/bin/env bash +n=$(cat "$FAKE_DIR/counter" 2>/dev/null || echo 1) +printf '{"state":"OPEN","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"head%s"}' "$n" +GHMOVE +chmod +x "$d/gh" +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 2 ]] || fail "a repeatedly-moving head must still hit the deadline, got $rc" "$out" +pass "a repeatedly-moving head still reaches the deadline" + +# The behavioural test above cannot separate "clock reset" from "clock kept" +# without waiting out a real deadline, so assert the invariant directly: the +# deadline is absolute for the invocation, and a push discards stale evidence, +# never the caller's time limit. +[[ "$(grep -c 'START_SEC=\$SECONDS' "$ROOT_DIR/scripts/pr-await")" -eq 1 ]] \ + || fail "the deadline clock must be assigned once, not reset on head change" +pass "the deadline clock is assigned exactly once (a push cannot defer the hard stop)" + +# --- blocked outcomes still honor --format json ---------------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3; do : > "$d/seq/$i.fail"; snapshot "$d" "$i" 0 0 0; done +out="$(run_await "$d" 12 --interval-sec 1 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "expected 3, got $rc" "$out" +jq -e . >/dev/null <<<"$out" || fail "blocked-access must still emit JSON" "$out" +[[ "$(jq -r '.outcome' <<<"$out")" == 'blocked-access' ]] || fail "outcome missing" "$out" +[[ "$(jq -r '.exit_code' <<<"$out")" == '3' ]] || fail "exit_code missing" "$out" +assert_json_envelope "blocked-access JSON must use the shared envelope" "$out" +pass "a blocked-access result is still valid JSON under --format json" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +out="$(PROBE_JQ="$d/jq-bad" run_await "$d" 12 --interval-sec 1 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "an old jq must not block the JSON path, got $rc" "$out" +jq -e . >/dev/null <<<"$out" || fail "must emit valid JSON" "$out" +[[ "$(jq -r '.outcome' <<<"$out")" == 'terminal' ]] || fail "outcome should be terminal" "$out" +grep -q 'jq-1.6-fake' <<<"$(jq -r '.toolchain' <<<"$out")" || fail "toolchain should record the jq" "$out" +pass "an old jq is recorded in the JSON toolchain field, not treated as blocked" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +cat > "$d/gh" <<'GHF' +#!/usr/bin/env bash +exit 1 +GHF +chmod +x "$d/gh" +out="$(run_await "$d" 12 --interval-sec 1 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +jq -e . >/dev/null <<<"$out" || fail "merge-state failure must emit JSON" "$out" +[[ "$(jq -r '.outcome' <<<"$out")" == 'blocked-merge-state' ]] || fail "outcome missing" "$out" +assert_json_envelope "blocked-merge-state JSON must use the shared envelope" "$out" +pass "a blocked merge-state result is still valid JSON under --format json" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 true aaaaaaaaaaaa \ + '{"approval_required_runs":[{"run_id":9,"name":"E2E","conclusion":"action_required"}]}' +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "blocked-approval JSON should exit 3, got $rc" "$out" +[[ "$(jq -r '.outcome' <<<"$out")" == 'blocked-approval' ]] || fail "blocked-approval outcome missing" "$out" +assert_json_envelope "blocked-approval JSON must use the shared envelope" "$out" +pass "blocked-approval produces the shared JSON envelope" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +printf '{"state":"MERGED","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"aaaaaaaaaaaa"}' > "$d/gh.json" +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "blocked-pr-state JSON should exit 3, got $rc" "$out" +[[ "$(jq -r '.outcome' <<<"$out")" == 'blocked-pr-state' ]] || fail "blocked-pr-state outcome missing" "$out" +assert_json_envelope "blocked-pr-state JSON must use the shared envelope" "$out" +pass "blocked-pr-state produces the shared JSON envelope" + +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3; do snapshot "$d" "$i" 20 0 0; done +printf '{"state":"OPEN","mergeable":"UNKNOWN","mergeStateStatus":"UNKNOWN","headRefOid":"aaaaaaaaaaaa"}' > "$d/gh.json" +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "blocked-merge-unknown JSON should exit 3, got $rc" "$out" +[[ "$(jq -r '.outcome' <<<"$out")" == 'blocked-merge-unknown' ]] || fail "blocked-merge-unknown outcome missing" "$out" +assert_json_envelope "blocked-merge-unknown JSON must use the shared envelope" "$out" +pass "blocked-merge-unknown produces the shared JSON envelope" + + +# --- hidden threads are a subset of the total, not an addition ------------- +# pr-state:922 counts every unresolved thread; hidden_unresolved_threads is the +# subset filtered out of unresolved_threads. Summing both double-counts. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 true aaaaaaaaaaaa \ + '{"unresolved_review_thread_count":4,"unresolved_threads":[], + "hidden_unresolved_threads":[{"thread_id":"t1","comment_id":1,"author":"bot","path":"a.go"}, + {"thread_id":"t2","comment_id":2,"author":"bot","path":"b.go"}, + {"thread_id":"t3","comment_id":3,"author":"bot","path":"c.go"}, + {"thread_id":"t4","comment_id":4,"author":"bot","path":"d.go"}]}' +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "expected findings, got $rc" "$out" +grep -q 'fix 0 failed check(s) and 4 thread(s)' <<<"$out" \ + || fail "4 unresolved threads must count as 4, not 8" "$out" +grep -q '4 total: 0 visible, 4 hidden' <<<"$out" \ + || fail "the header must not label the total as visible" "$out" +pass "hidden threads are counted once, and the total is not labelled visible" + +# --- an empty check rollup is not terminal --------------------------------- +# Right after a push GitHub can expose the new head before Actions registers +# any checks. Zero of everything is "not yet", not "clean". +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3; do snapshot "$d" "$i" 0 0 0; done +snapshot "$d" 4 20 0 0 +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "expected clean once checks appeared, got $rc" "$out" +grep -q '4 poll' <<<"$out" || fail "must wait for checks to appear, not stop at the empty rollup" "$out" +pass "an empty check rollup keeps waiting instead of reporting clean" + +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3 4 5; do snapshot "$d" "$i" 0 0 0; done +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -ne 0 ]] || fail "a permanently empty rollup must never exit 0" "$out" +pass "a rollup that never populates does not become clean at the deadline" +grep -qi 'no checks registered yet' <<<"$out" \ + || fail "a zero-check deadline must say no checks registered, not '0 check(s) pending'" "$out" +grep -q '0 check(s) pending' <<<"$out" \ + && fail "a zero-check deadline must not be phrased as pending checks" "$out" +pass "a zero-check deadline is distinguished from a deadline with pending checks" + +# Skipped and neutral checks are terminal evidence even though pr-state does +# not include them in passed_check_count. The separate check_count prevents the +# await loop from treating such a rollup as if Actions had not registered it. +for conclusion in skipped neutral; do + d="$(make_tmp_dir)"; setup_fake "$d" + snapshot "$d" 1 0 0 0 true aaaaaaaaaaaa "{\"check_count\":1,\"terminal_conclusions\":[\"$conclusion\"]}" + out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? + [[ "$rc" -eq 0 ]] || fail "$conclusion-only check rollup should be terminal, got $rc" "$out" + [[ "$(jq -r '.outcome' <<<"$out")" == 'terminal' ]] || fail "$conclusion-only rollup should emit terminal" "$out" +done +pass "skipped and neutral terminal checks do not look like an empty rollup" + +# --- a base that advanced leaves the merge result untested ----------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 true aaaaaaaaaaaa '{"pr":{"base_advanced_since_head":true}}' +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "base_advanced_since_head must prevent a clean verdict, got $rc" "$out" +grep -qi 'base advanced' <<<"$out" || fail "should report the advanced base" "$out" +pass "an advanced base is reported and prevents a clean verdict" + +# --- approval-required must belong to the current head --------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 10 0 2 true aaaaaaaaaaaa \ + '{"approval_required_runs":[{"run_id":9,"name":"E2E","conclusion":"action_required"}]}' +snapshot "$d" 2 20 0 0 true bbbbbbbbbbbb +cat > "$d/gh" <<'GHAP' +#!/usr/bin/env bash +printf '{"state":"OPEN","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"bbbbbbbbbbbb"}' +GHAP +chmod +x "$d/gh" +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 0 ]] || fail "approval for a superseded head must be discarded, got $rc" "$out" +pass "an approval-required run from a superseded head is not actionable" + +# --- a leading-zero PR number survives the JSON path ----------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +for i in 1 2 3; do : > "$d/seq/$i.fail"; snapshot "$d" "$i" 0 0 0; done +out="$(run_await "$d" 0012 --interval-sec 1 --quiet --format json 2>/dev/null)" && rc=0 || rc=$? +jq -e . >/dev/null <<<"$out" || fail "leading-zero PR must not break the JSON envelope" "$out" +[[ "$(jq -r '.pr' <<<"$out")" == '12' ]] || fail "PR should normalize to 12" "$out" +pass "a leading-zero PR number normalizes and keeps the JSON envelope valid" + + +# The bounded reads are always captured with $(...). A watchdog that keeps that +# pipe open blocks the substitution long after the command itself has finished, +# so a fast run must stay fast when captured, not only when redirected. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +start=$SECONDS +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 30 --quiet 2>/dev/null)" && rc=0 || rc=$? +elapsed=$((SECONDS - start)) +[[ "$rc" -eq 0 ]] || fail "expected clean, got $rc" "$out" +[[ "$elapsed" -lt 10 ]] || fail "a captured fast run must not wait on the watchdog, took ${elapsed}s" +pass "a bounded read does not hold the command substitution open after it finishes" + +# --- a stalled GitHub read cannot outlive the deadline --------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +cat > "$d/pr-state" <<'HANG' +#!/usr/bin/env bash +exec sleep 120 +HANG +chmod +x "$d/pr-state" +start=$SECONDS +out="$(PR_AWAIT_READ_FLOOR=2 run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet 2>/dev/null)" && rc=0 || rc=$? +elapsed=$((SECONDS - start)) +[[ "$rc" -ne 0 ]] || fail "a stalled pr-state must never exit 0, got $rc" "$out" +[[ "$elapsed" -lt 12 ]] || fail "a zero deadline must not wait out a 120s read, took ${elapsed}s" +pass "a stalled pr-state read cannot exceed the deadline" + +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +cat > "$d/gh" <<'HANGGH' +#!/usr/bin/env bash +exec sleep 120 +HANGGH +chmod +x "$d/gh" +start=$SECONDS +out="$(PR_AWAIT_READ_FLOOR=2 run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet 2>/dev/null)" && rc=0 || rc=$? +elapsed=$((SECONDS - start)) +[[ "$rc" -ne 0 ]] || fail "a stalled gh pr view must never exit 0, got $rc" "$out" +[[ "$elapsed" -lt 12 ]] || fail "a zero deadline must not wait out a 120s gh read, took ${elapsed}s" +pass "a stalled gh pr view read cannot exceed the deadline" +grep -qi 'exceeded its read bound' <<<"$out" \ + || fail "a timed-out gh pr view must say so, not report a bare failure count" "$out" +grep -q 'failed 0 times' <<<"$out" \ + && fail "a pure gh pr view timeout must not be reported as 'failed 0 times in a row'" "$out" +pass "a timed-out gh pr view is reported as a timeout, not a miscounted failure" + +# Structural guard: both external reads must go through the bounded helper, or +# the deadline is only enforced between reads and not during them. +grep -q 'run_bounded_capture "\$budget" "\$READ_TMP" "\$PR_STATE"' "$ROOT_DIR/scripts/pr-await" \ + || fail "the pr-state read must run through run_bounded_capture" +grep -A2 'run_bounded_capture "\$budget" "\$READ_TMP"' "$ROOT_DIR/scripts/pr-await" \ + | grep -q '"\$GH" pr view' \ + || fail "the gh pr view read must run through run_bounded_capture" +pass "every external GitHub read is bounded by the remaining deadline" + +# Structural guard: a failed read must be validated in a local before it can +# replace SUMMARY_JSON/MERGE_JSON. Otherwise a transient failure right before +# the deadline destroys evidence a working poll already produced, and the +# deadline-clock check ("if -n SUMMARY_JSON: OUTCOME=deadline") wrongly trusts +# garbage instead of falling back to the last good snapshot. +grep -q 'summary_body="\$(cat "\$READ_TMP")"' "$ROOT_DIR/scripts/pr-await" \ + || fail "the pr-state read must stage into a local before committing to SUMMARY_JSON" +grep -q 'merge_body="\$(cat "\$READ_TMP")"' "$ROOT_DIR/scripts/pr-await" \ + || fail "the gh pr view read must stage into a local before committing to MERGE_JSON" +pass "a failed GitHub read is staged before it can clobber the last good snapshot" + + + +# --- a head change voids the snapshot in hand ------------------------------ +# Otherwise a deadline reached before the next read renders the old head's +# checks under the new SHA. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 3 2 7 true aaaaaaaaaaaa +snapshot "$d" 2 3 2 7 true aaaaaaaaaaaa +cat > "$d/gh" <<'GHMOVED' +#!/usr/bin/env bash +printf '{"state":"OPEN","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"bbbbbbbbbbbb"}' +GHMOVED +chmod +x "$d/gh" +out="$(run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 3 ]] || fail "a deadline with only stale evidence must block, got $rc" "$out" +grep -q 'Failing' <<<"$out" && fail "must not report the old head's checks under the new SHA" "$out" +pass "a head change voids the snapshot, so a deadline cannot report stale checks" + +# --- a timed-out read takes its children with it --------------------------- +# On timeout-less hosts, killing only pr-state's shell leaves its gh child +# running and still issuing requests. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +cat > "$d/pr-state" <<'SPAWN' +#!/usr/bin/env bash +sleep 300 & +printf '%s' "$!" > "$FAKE_DIR/childpid" +wait +SPAWN +chmod +x "$d/pr-state" +PR_AWAIT_NO_TIMEOUT=1 PR_AWAIT_READ_FLOOR=2 run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet >/dev/null 2>&1 || true +child="$(cat "$d/childpid" 2>/dev/null || echo '')" +[[ -n "$child" ]] || fail "the fake did not record its child pid" +sleep 1 +if kill -0 "$child" 2>/dev/null; then + kill -9 "$child" 2>/dev/null || true + fail "a timed-out read left its child process running (pid $child)" +fi +pass "a timed-out read terminates the whole process group, not just the shell" + +# --- cancellation stops an active bounded read ----------------------------- +# A caller that cancels the waiter must not leave a child GitHub read alive. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +cat > "$d/pr-state" <<'CANCEL' +#!/usr/bin/env bash +sleep 120 & +printf '%s' "$!" > "$FAKE_DIR/childpid" +wait "$!" +CANCEL +chmod +x "$d/pr-state" +FAKE_DIR="$d" PR_AWAIT_PR_STATE="$d/pr-state" PR_AWAIT_GH="$d/gh" \ + PR_AWAIT_SLEEP="$d/sleep" PR_AWAIT_JQ="$d/jq-ok" PR_AWAIT_PROBE_BASH="$d/bash-ok" \ + PR_AWAIT_NO_TIMEOUT=1 PR_AWAIT_READ_FLOOR=30 "$SCRIPT" 12 --interval-sec 1 --deadline-min 30 --quiet >"$d/out" 2>"$d/err" & +await_pid=$! +for _ in 1 2 3 4 5; do + [[ -s "$d/childpid" ]] && break + /bin/sleep 1 +done +[[ -s "$d/childpid" ]] || { kill -KILL "$await_pid" 2>/dev/null || true; fail "the fake did not start a bounded child read"; } +child="$(<"$d/childpid")" +kill -TERM "$await_pid" +for _ in 1 2 3 4 5; do + kill -0 "$await_pid" 2>/dev/null || break + /bin/sleep 1 +done +if kill -0 "$await_pid" 2>/dev/null; then + kill -KILL "$await_pid" 2>/dev/null || true + kill -KILL "$child" 2>/dev/null || true + wait "$await_pid" 2>/dev/null || true + fail "SIGTERM must stop an active bounded read" +fi +wait "$await_pid" && rc=0 || rc=$? +[[ "$rc" -eq 143 ]] || fail "SIGTERM should return 143, got $rc" "$(<"$d/err")" +if kill -0 "$child" 2>/dev/null; then + kill -KILL "$child" 2>/dev/null || true + fail "SIGTERM left the bounded child read running (pid $child)" +fi +pass "SIGTERM stops the waiter and its active bounded read" + + +# --- a spent deadline grants one read floor, not one per query ------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +# pr-state SUCCEEDS but eats most of the floor; gh then stalls. A shared budget +# leaves gh the remainder; a fresh grant hands it a whole second floor. +cat > "$d/pr-state" <<'SLOWPS' +#!/usr/bin/env bash +sleep 3 +cat "$FAKE_DIR/seq/1.json" +SLOWPS +cat > "$d/gh" <<'SLOWGH' +#!/usr/bin/env bash +exec sleep 120 +SLOWGH +chmod +x "$d/pr-state" "$d/gh" +start=$SECONDS +PR_AWAIT_READ_FLOOR=4 run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet >/dev/null 2>&1 || true +elapsed=$((SECONDS - start)) +[[ "$elapsed" -lt 6 ]] || fail "a zero deadline granted a second read floor, took ${elapsed}s (budget was 4)" +pass "a spent deadline grants a single read floor shared across both queries" + +# --- every --format json exit path emits parseable JSON -------------------- +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 +cat > "$d/gh" <<'GARBAGE' +#!/usr/bin/env bash +printf 'not json at all' +GARBAGE +chmod +x "$d/gh" +out="$(PR_AWAIT_READ_FLOOR=2 run_await "$d" 12 --interval-sec 1 --deadline-min 0 --quiet --format json 2>/dev/null)" || true +jq -e . >/dev/null 2>&1 <<<"$out" || fail "a garbage merge-state read must still yield parseable JSON" "$out" +pass "a garbage merge-state read still yields parseable JSON" + +# --- a comment-only finding gets an accurate NEXT message ------------------- +# Zero failed checks and zero unresolved threads must not be reported as +# "fix 0 failed check(s) and 0 thread(s)" when an actionable issue comment +# (or a blocking review, conflict, or advanced base) is the actual finding. +d="$(make_tmp_dir)"; setup_fake "$d" +snapshot "$d" 1 20 0 0 true aaaaaaaaaaaa '{"actionable_issue_comment_count":1}' +out="$(run_await "$d" 12 --interval-sec 1 --quiet 2>/dev/null)" && rc=0 || rc=$? +[[ "$rc" -eq 1 ]] || fail "an actionable issue comment with clean checks should exit 1, got $rc" "$out" +grep -q 'fix 0 failed check(s) and 0 thread(s)' <<<"$out" \ + && fail "must not claim nothing to fix when a comment is the only finding" "$out" +grep -qi 'review the findings reported above' <<<"$out" \ + || fail "should point the caller at the reported findings instead" "$out" +pass "a comment-only finding is not reported as '0 failed check(s) and 0 thread(s)'" + +printf '\nall tests passed\n' diff --git a/scripts/pr-state b/scripts/pr-state index f04d0951ad..09b7c3724b 100755 --- a/scripts/pr-state +++ b/scripts/pr-state @@ -583,6 +583,7 @@ summarize_state_json() { | select(successful_check) | {name, workflow, status, conclusion, run_id, job_id, details_url, target_url} ], + check_count: ([effective_checks[]] | length), passed_check_count: ([effective_checks[] | select(successful_check)] | length), unresolved_review_thread_count, filtered_review_thread_count, diff --git a/scripts/pr-state.test.sh b/scripts/pr-state.test.sh index 15a1ca8728..eb9f746556 100644 --- a/scripts/pr-state.test.sh +++ b/scripts/pr-state.test.sh @@ -1248,6 +1248,7 @@ test_summary_mode_returns_compact_fixup_state() { assert_jq "summary pending check count" '.pending_checks | length == 2' "$json" assert_jq "summary pending check" '.pending_checks[0] | .name == "claude-review" and .status == "in_progress" and .run_id == "27340000003"' "$json" assert_jq "summary pending status context keeps target url" '.pending_checks[] | select(.name == "external pending") | .status == "pending" and .details_url == null and .target_url == "https://ci.example.test/build/1"' "$json" + assert_jq "summary counts every effective check" '.check_count == 7' "$json" assert_jq "summary exposes normalized passing checks" '(.passed_check_count == (.successful_checks | length)) and any(.successful_checks[]; .name == "web lint" and .conclusion == "success") and any(.successful_checks[]; .name == "opencode-review-same-repo" and .conclusion == "success")' "$json" assert_jq "summary unresolved count" '.unresolved_review_thread_count == 2' "$json" assert_jq "summary filtered thread count" '.filtered_review_thread_count == 1' "$json"