Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 127 additions & 9 deletions scripts/lib/ci-status.sh
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
#!/usr/bin/env bash
# ci-status.sh — compute CI gate status from a statusCheckRollup JSON array,
# filtering out the PR Review Agent's own check runs so the cascade never
# blocks on itself (issue #469).
# filtering out first-party agents' own check runs so the reviewing cascade never
# blocks on itself (issue #469) or on another writing agent's orchestration check
# (issue #1427).
#
# Usage: source this file, then call compute_ci_status with the rollup array.

# Directory this library lives in — used to resolve the interaction contracts
# (../../interaction-contracts) relative to the script, independent of CWD.
_CI_STATUS_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

# compute_ci_status <rollup_json>
#
# Inputs:
Expand Down Expand Up @@ -67,13 +72,72 @@ _CI_STATUS_JQ_IS_OWN_CHECK='
($n | test("^PR Review (Agent|Reusable) /")) or
($n | test("^PR Review — Mention Trigger /"));'

# Shared jq `def is_agent_check` — a check run belonging to any *first-party
# agentic role's* own workflow, excluded from the CI gate so a writing agent's own
# orchestration check (e.g. dev-lead's "dev-lead / dispatch") cannot defer the
# reviewing agent (#1427, AC #1/#2). This GENERALISES the #469 self-filter above:
# rather than hard-coding each agent, the role slugs are derived from the declared
# interaction contracts (interaction-contracts/*.yml, #1404) — the single registry
# that already names each role and its workflows — and passed in via `$agent_roles`.
#
# A role's check runs surface in the rollup as the nested "<role> / <job>" form
# (caller-job / reusable-job), e.g. "dev-lead / dispatch", "dev-lead / ci-relay".
# We match that exact shape (name == role, or name starts with "<role> / ") so a
# genuine external check that merely shares a job name (bare "dispatch", or
# "Build / review") is never swallowed — the same conservative matching the
# self-filter uses for "review" vs "Build / review".
_CI_STATUS_JQ_IS_AGENT_CHECK='
def is_agent_check:
(.name // .context // "") as $n |
($agent_roles | any(. as $r | ($r | length) > 0 and
($n == $r or ($n | startswith($r + " / ")))));'

# _ci_status_agent_roles_json — JSON array of first-party agent role slugs whose
# own check runs must be excluded from the CI gate (#1427). Derived from the
# declared interaction contracts so the exclusion can never drift from a second
# hand-maintained list. Each contract file declares `role: <slug>`.
#
# Overridable for tests / callers via CI_STATUS_AGENT_ROLES_JSON (a JSON array) or
# CI_STATUS_CONTRACTS_DIR (the contracts directory). Always emits valid JSON — on
# any error it degrades to "[]" (no agent filtering), which is safe: it only
# reverts to the pre-#1427 behavior of counting those checks.
_ci_status_agent_roles_json() {
if [ -n "${CI_STATUS_AGENT_ROLES_JSON:-}" ]; then
printf '%s' "$CI_STATUS_AGENT_ROLES_JSON"
return 0
fi
local dir="${CI_STATUS_CONTRACTS_DIR:-$_CI_STATUS_LIB_DIR/../../interaction-contracts}"
local roles_json="[]"
if [ -d "$dir" ]; then
roles_json=$(
grep -hE '^role:[[:space:]]' "$dir"/*.yml 2>/dev/null \
| sed -E 's/^role:[[:space:]]*//; s/[[:space:]]+$//' \
| jq -R 'select(length > 0)' 2>/dev/null \
| jq -s 'unique' 2>/dev/null
) || roles_json="[]"
[ -n "$roles_json" ] || roles_json="[]"
fi
Comment thread
don-petry marked this conversation as resolved.
Resolved
printf '%s' "$roles_json"
}

compute_ci_status() {
local rollup_json="${1:-[]}"
jq -r "
local agent_roles
agent_roles="$(_ci_status_agent_roles_json)"
jq -r --argjson agent_roles "$agent_roles" "
# A check carrying a non-empty conclusion is terminal, whatever its status
# says (#1427, AC #3): GitHub can report a zombie check as IN_PROGRESS while
# already carrying a terminal conclusion (observed on PR #1426:
# status IN_PROGRESS + conclusion SUCCESS). A conclusion is by definition
# terminal, so it wins over a stale running status.
def is_terminal:
(.conclusion != null and .conclusion != \"\");
def is_pending:
.status == \"IN_PROGRESS\" or .status == \"QUEUED\" or .status == \"WAITING\" or
.state == \"PENDING\" or .state == \"EXPECTED\" or
(.status == \"COMPLETED\" and (.conclusion == null or .conclusion == \"\"));
(is_terminal | not) and (
.status == \"IN_PROGRESS\" or .status == \"QUEUED\" or .status == \"WAITING\" or
.status == \"COMPLETED\" or
.state == \"PENDING\" or .state == \"EXPECTED\"
);
def is_success:
.conclusion == \"SUCCESS\" or .conclusion == \"SKIPPED\" or .conclusion == \"NEUTRAL\" or
.state == \"SUCCESS\";
Expand All @@ -83,9 +147,10 @@ compute_ci_status() {
def is_cancelled:
.conclusion == \"CANCELLED\";
$_CI_STATUS_JQ_IS_OWN_CHECK
$_CI_STATUS_JQ_IS_AGENT_CHECK
if (. == null or (type != \"array\")) then \"passing\"
else
(map(select(is_own_check | not))) as \$ext |
(map(select((is_own_check or is_agent_check) | not))) as \$ext |
if (\$ext | length) == 0 then \"passing\"
elif ([\$ext[] | select(is_pending)] | length) > 0 then \"pending\"
elif all(\$ext[]; is_success or is_cancelled) then \"passing\"
Expand All @@ -95,6 +160,56 @@ compute_ci_status() {
" <<< "$rollup_json" 2>/dev/null || echo "passing"
}

# ci_pending_age_exceeded <rollup_json> <max_age_sec> [now_epoch]
#
# Bounded-deferral guard (issue #1427, AC #3/#4). Given the same external checks
# compute_ci_status would classify as pending (own + agent checks already
# excluded), returns "true" iff there is at least one such pending check AND every
# one of them has been pending longer than <max_age_sec> — i.e. the pending state
# is genuinely STUCK, not merely slow. A pending check with no usable timestamp
# counts as NOT-exceeded, so an unknown age can never trip the timeout (fail-safe).
#
# Callers use this only to bound the review DECISION, never the merge gate: a
# genuinely-pending *required* check still blocks the merge at the ruleset
# regardless of this timeout, so proceeding here can never merge a not-yet-green
# PR (AC #4).
ci_pending_age_exceeded() {
local rollup_json="${1:-[]}" max_age="${2:-1800}" now_epoch="${3:-}"
local agent_roles now_arg
agent_roles="$(_ci_status_agent_roles_json)"
case "$max_age" in ''|*[!0-9]*) max_age=1800 ;; esac
if [ -n "$now_epoch" ] && printf '%s' "$now_epoch" | grep -qE '^[0-9]+$'; then
Comment thread
don-petry marked this conversation as resolved.
Resolved
now_arg="$now_epoch"
else
now_arg="null"
fi
jq -r --argjson agent_roles "$agent_roles" --argjson max "$max_age" --argjson nowarg "$now_arg" "
def is_terminal:
(.conclusion != null and .conclusion != \"\");
def is_pending:
(is_terminal | not) and (
.status == \"IN_PROGRESS\" or .status == \"QUEUED\" or .status == \"WAITING\" or
.status == \"COMPLETED\" or
.state == \"PENDING\" or .state == \"EXPECTED\"
);
$_CI_STATUS_JQ_IS_OWN_CHECK
$_CI_STATUS_JQ_IS_AGENT_CHECK
(if \$nowarg == null then now else \$nowarg end) as \$now |
if (. == null or (type != \"array\")) then \"false\"
else
[ .[] | select((is_own_check or is_agent_check) | not) | select(is_pending) ] as \$pend |
if (\$pend | length) == 0 then \"false\"
elif all(\$pend[];
((.startedAt // .createdAt // .started_at // \"\") as \$ts |
if (\$ts | type) != \"string\" or \$ts == \"\" then false
else (\$now - (\$ts | fromdateiso8601? // \$now)) > \$max
end))
then \"true\" else \"false\"
end
end
" <<< "$rollup_json" 2>/dev/null || echo "false"
}

# Workflow names from pr-review-sweep.yml's workflow_run.workflows: — the
# complete set the fast path can key on. Any check whose workflowName is absent
# from this list (or that has no workflowName at all) is un-eventable and falls
Expand Down Expand Up @@ -128,15 +243,18 @@ _CI_STATUS_EVENTABLE_WORKFLOWS='["CI","Tests","Holdout Guard","SonarCloud Analys
# scheduled sweep can re-review these PRs
classify_rollup_eventability() {
local rollup_json="${1:-[]}"
jq -r "
local agent_roles
agent_roles="$(_ci_status_agent_roles_json)"
jq -r --argjson agent_roles "$agent_roles" "
$_CI_STATUS_JQ_IS_OWN_CHECK
$_CI_STATUS_JQ_IS_AGENT_CHECK
def is_uneventable:
(.workflowName // \"\") as \$wf |
\$wf == \"\" or
($_CI_STATUS_EVENTABLE_WORKFLOWS | index(\$wf)) == null;
if (. == null or (type != \"array\")) then \"none\"
else
(map(select(is_own_check | not))) as \$ext |
(map(select((is_own_check or is_agent_check) | not))) as \$ext |
if (\$ext | length) == 0 then \"none\"
elif ([\$ext[] | select(is_uneventable)] | length) > 0 then \"has-uneventable\"
else \"eventable-only\"
Expand Down
39 changes: 37 additions & 2 deletions scripts/review-one-pr.sh
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,36 @@ if [ "$CI_STATUS" = "failing" ]; then
fi
fi

# Bounded ci-pending deferral (issue #1427, AC #3/#4). A genuinely stuck check —
# one that stays pending indefinitely with no terminal conclusion — must not defer
# the review forever. If every remaining external pending check has been pending
# longer than CI_PENDING_MAX_AGE_SEC (default 30 min), proceed on the completed
# checks rather than skipping. This bounds only the review DECISION: the merge gate
# is enforced independently by the branch-protection ruleset, which still blocks the
# merge on any genuinely-pending *required* check, so this can never merge a
# not-yet-green PR (fail-safe, not fail-open). Own-agent and other first-party agent
# checks are already excluded by compute_ci_status / ci_pending_age_exceeded.
if [ "$CI_STATUS" = "pending" ]; then
_CI_PENDING_MAX_AGE="${CI_PENDING_MAX_AGE_SEC:-1800}"
case "$_CI_PENDING_MAX_AGE" in ''|*[!0-9]*) _CI_PENDING_MAX_AGE=1800 ;; esac
if [ "$(ci_pending_age_exceeded "$(jq '.statusCheckRollup' <<< "$PR_SNAPSHOT")" "$_CI_PENDING_MAX_AGE")" = "true" ]; then
echo "::warning::ci-pending: external check(s) have been pending longer than ${_CI_PENDING_MAX_AGE}s without completing — proceeding with the review on the completed checks (bounded deferral, #1427). The merge gate still blocks on any genuinely-pending required check at the ruleset."
# Escalate once — post a visible, deduped note so a human sees the stuck check.
_timeout_marker='<!-- pr-review-agent ci-pending-timeout -->'
if [ "${DRY_RUN:-false}" != "true" ] && \
! jq -e --arg m "$_timeout_marker" '(.comments // []) | any((.body // "") | contains($m))' <<< "$PR_SNAPSHOT" >/dev/null 2>&1; then
_tmsg=$(printf '%s\n\nSome CI checks have been pending for over %ss without completing, so this review is proceeding on the checks that did complete. This does not merge the PR — the branch-protection rules still require every genuinely-pending required check before merge. No action is needed.\n\n_Posted by the %s PR-review cascade._' \
"$_timeout_marker" "$_CI_PENDING_MAX_AGE" "${BOT_USER:-donpetry-bot}")
gh pr comment "$PR_URL" --body "$_tmsg" || echo " warn: could not post ci-pending-timeout comment"
unset _tmsg
fi
unset _timeout_marker
CI_STATUS="passing"
echo " ci-pending bounded out (stuck > ${_CI_PENDING_MAX_AGE}s) — proceeding with the review"
fi
unset _CI_PENDING_MAX_AGE
fi

# Pending CI gate: for normal runs, skip immediately.
# For FORCE_REVIEW=true (mention-triggered), poll briefly so transient check
# churn (caused by the mention burst itself) doesn't silently swallow the run
Expand Down Expand Up @@ -215,8 +245,13 @@ if [ "$CI_STATUS" = "pending" ]; then
if [ "$CI_STATUS" = "pending" ]; then
echo " force-review: CI still pending after polling — posting visible comment"
if [ "${DRY_RUN:-false}" != "true" ]; then
_msg=$(printf '<!-- pr-review-agent ci-pending-ack -->\n\nCI checks on this PR are still running. Once they complete, re-mention `@%s` to trigger a fresh review.\n\n_Posted by the %s PR-review cascade._' \
"${BOT_USER:-donpetry-bot}" "${BOT_USER:-donpetry-bot}")
# AC #5 (#1427): the ack must NOT recommend a re-mention. A re-mention is
# an issue_comment that fires dev-lead, whose own "dev-lead / dispatch"
# check going IN_PROGRESS is exactly the condition that used to re-arm this
# deferral. The PR-review sweep already re-reviews automatically once the
# checks complete, so no user action is required.
_msg=$(printf '<!-- pr-review-agent ci-pending-ack -->\n\nCI checks on this PR are still running. The PR-review sweep re-reviews this PR automatically once the checks complete — no action is needed.\n\n_Posted by the %s PR-review cascade._' \
"${BOT_USER:-donpetry-bot}")
gh pr comment "$PR_URL" --body "$_msg" || echo " warn: could not post ci-pending-ack comment"
unset _msg
fi
Expand Down
Loading
Loading