Skip to content

fix(ci-status): wait for the re-run of a failed full run before carrying its failure #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
kyle-sexton wants to merge 1 commit into
base: main
Choose a base branch
from
+211 −38
Draft

fix(ci-status): wait for the re-run of a failed full run before carrying its failure #647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 11 additions & 6 deletions .github/actions/ci-status/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,11 @@ inputs:
Ceiling, in seconds, on the carry-forward wait. In carry-forward mode this
action polls every 15 seconds. Each poll lists the runs of this same
workflow on this same head SHA that are still incomplete, excluding this
run, and then reads the `status-context` status list. A `success` or
`failure` is a settled verdict and ends the poll; otherwise the run stops
run, and then reads the `status-context` status list. A `success` is a
settled verdict and ends the poll. A `failure` or `error` ends it too,
unless the full run that wrote it (the run its `target_url` names) is
re-running with an attempt that started after the status was created;
then the poll waits on that run alone. Otherwise the run stops
when nothing is in flight, which is the immediate fail on an absent status
that this action has always had when no full run is coming, and sleeps
when something is.
Expand All @@ -72,10 +75,12 @@ inputs:
status and flips to `completed` moments later, and the other order lets
that completion land between the two calls and report both no status and
nothing in flight.
Reaching the ceiling fails the run; there is no pass-on-timeout path. The
one case that runs to the ceiling is two or more contract-only runs on a
SHA with no full run in flight to write a verdict for them, and it fails
closed. `0` disables the wait and goes straight to a single status read.
Reaching the ceiling fails the run; there is no pass-on-timeout path. Two
cases run to the ceiling and fail closed: two or more contract-only runs
on a SHA with no status and no full run in flight to write one, and a
writer re-run slower than the ceiling. Every red names both remedies:
re-run the full workflow, or, once the status is `success`, re-run this
run. `0` disables the wait and goes straight to a single status read.
The calling job needs `actions: read`: under an explicit `permissions:`
block the default is none, and a 403 warns naming the scope and degrades
to the status read alone.
Expand Down
87 changes: 66 additions & 21 deletions .github/actions/ci-status/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@
# 1. List the runs of this same workflow on this same head SHA that are still
# incomplete, excluding this run. Any run id; see below.
# 2. Read the newest `github-actions[bot]` status for `status-context`. A
# `success` or `failure` is a settled verdict, so stop polling and apply
# it under the usual rules.
# `success` is a settled verdict, so stop polling and apply it. A
# `failure` or `error` is settled too, unless the full run that wrote it
# (the run its `target_url` names) is in flight again with a current
# attempt that started after the status was created: that is a re-run
# about to replace the verdict, so the wait narrows to that one run.
# 3. Otherwise, if step 1 found nothing in flight, stop: no run that could
# still write a verdict exists, and an absent status fails the run as it
# always has. If something IS in flight, sleep and poll again.
Expand All @@ -36,23 +39,36 @@
# breaks a mutual wait: once the full run writes its verdict, both contract-only
# runs read it and stop.
#
# Only the writer's re-run holds a settled failure open. Only a full run writes
# the status, so a contract-only sibling is never that run, and two contract-only
# runs on a failed SHA still stop at once instead of waiting on each other. A
# `run_attempt > 1` test would also catch a re-run contract-only run and bring
# that mutual wait back. The start-time test keeps the writer's own first
# attempt, which writes the status a moment before it completes, out of the
# wait. The known limit: a re-run of a DIFFERENT full run on the same SHA does
# not hold a failure open, and the run fails at once as it did before.
#
# Step 1 before step 2 WITHIN a poll. A sibling writes the status and flips to
# `completed` a moment later. Listing after reading would let that completion
# land in the gap between the two calls and report both "no status" and "nothing
# in flight", failing a SHA that does carry a verdict. Listing first closes the
# gap, because a status written before the listing is still read after it.
#
# The loop never turns a verdict green. Reaching the ceiling exits 1 rather than
# passing on an unsettled status; there is no pass-on-timeout path. The one case
# that runs to the ceiling is two or more contract-only runs on a SHA with no
# full run in flight to write a verdict for them: they wait on each other and
# then both fail closed.
# passing on an unsettled status; there is no pass-on-timeout path. Two cases
# run to the ceiling: two or more contract-only runs on a SHA with no status and
# no full run in flight to write one (they wait on each other and then both fail
# closed), and a writer re-run that outlasts the ceiling.
#
# Reading a settled status before the wait set empties is a deliberate trade: it
# releases the mutual wait above, and it can carry an older `success` forward
# Reading a settled `success` before the wait set empties is a deliberate trade:
# it releases the mutual wait above, and it can carry an older `success` forward
# while a re-run of the same SHA is in flight to overwrite it. The defenses
# against a forged status (context, creator login, Bot type, newest id) hold.
#
# Every red names both remedies. The run cannot see a later verdict, so the
# message says to re-run the full workflow, and to re-run this run instead once
# `status-context` on the SHA is `success`.
#
# `same-repo` false is a fork pull request. Its token is read-only on
# `pull_request` whatever `permissions:` requests, so it cannot record lane
# state; it aggregates, reports the lanes verdict, and writes nothing. The
Expand Down Expand Up @@ -193,12 +209,20 @@ require_pattern carry-forward-wait-seconds "$CARRY_FORWARD_WAIT_SECONDS" '^[0-9]
# echoing it so the caller can distinguish "read failed" from "read empty"
# through the return status.
carried_state=""
# When that entry was created, and the run id its `target_url` names (the full
# run that wrote it). Empty when absent; either one empty means the wait never
# treats a run as the writer's re-run.
carried_created_at=""
carried_writer_run_id=""
# Whether `carried_state` holds a completed read. The poll loop reads the status
# itself, so the caller below must not read a second time and overwrite what the
# loop decided on.
carried_state_read=false
read_carried_state() {
local entry
carried_state=""
carried_created_at=""
carried_writer_run_id=""
carried_state_read=false
# The LIST endpoint, not the combined one: `commits/<sha>/status` collapses to
# one entry per context and exposes no author, so any collaborator with write
Expand All @@ -221,9 +245,13 @@ read_carried_state() {
# explicit check a malformed or truncated body would make jq exit nonzero,
# that status would be discarded, and the function would report a completed
# read of an empty state. A body this function cannot parse is a failed read.
carried_state="$(jq -r --arg context "$STATUS_CONTEXT" --arg creator "$STATUS_CREATOR" \
'[ .[] | select(.context == $context and (.creator.login // "") == $creator and (.creator.type // "") == "Bot") ] | (max_by(.id).state // "")' \
# One pass, `|`-joined: tab is IFS whitespace, so `read` would collapse an
# empty middle field and shift the ones after it.
entry="$(jq -r --arg context "$STATUS_CONTEXT" --arg creator "$STATUS_CREATOR" \
'[ .[] | select(.context == $context and (.creator.login // "") == $creator and (.creator.type // "") == "Bot") ] | (max_by(.id) // {})
| [(.state // ""), (.created_at // ""), ((.target_url // "") | capture("/actions/runs/(?<id>[0-9]+)").id // "")] | join("|")' \
<"$gh_stdout")" || return 1
IFS='|' read -r carried_state carried_created_at carried_writer_run_id <<<"$entry"
carried_state_read=true
}

Expand Down Expand Up @@ -307,6 +335,9 @@ wait_for_sibling_runs() {
ids="$(jq -r --argjson incomplete "$INCOMPLETE_RUN_STATUSES" --argjson self "$run_id" \
'[ .workflow_runs[]? | select(.status as $s | $incomplete | index($s)) | select((.id // $self) != $self) | .id ] | sort | join(" ")' \
<"$gh_stdout")"
# Kept for the settled-failure check below, which runs after the status
# read has overwritten gh_stdout.
cp -- "$gh_stdout" "$scratch/runs.json"
# The status read comes AFTER the listing above, never before it: a sibling
# writes the status and flips to `completed` moments later, and the other
# order lets that completion land between the two calls.
Expand All @@ -329,11 +360,19 @@ wait_for_sibling_runs() {
cat "$gh_stderr" >&2
break
fi
# A settled verdict ends the wait whatever is still in flight. This is what
# releases two contract-only runs that would otherwise wait on each other.
# `error` is settled alongside `failure`: both are terminal states the API
# accepts, and neither passes below, so waiting on one only delays a red.
if [[ "$carried_state" == success || "$carried_state" == failure || "$carried_state" == error ]]; then
# A failure is stale while the full run that wrote it is re-running, so the
# wait narrows to that one run; see the header for why only the writer.
# `error` goes with `failure`: both are terminal states the API accepts, and
# neither passes below.
if [[ "$carried_state" == failure || "$carried_state" == error ]]; then
ids="$(jq -r --argjson incomplete "$INCOMPLETE_RUN_STATUSES" --arg writer "$carried_writer_run_id" --arg since "$carried_created_at" \
'[ .workflow_runs[]? | select(.status as $s | $incomplete | index($s)) | select($since != "" and (.id | tostring) == $writer and (.run_started_at // "") > $since) | .id ] | join(" ")' \
<"$scratch/runs.json")"
fi
# A settled verdict with no writer re-run in flight ends the wait whatever
# else is in flight. This is what releases two contract-only runs that
# would otherwise wait on each other.
if [[ "$carried_state" == success || ("$carried_state" =~ ^(failure|error)$ && -z "$ids") ]]; then
if [[ "$carry_forward_waited" -gt 0 ]]; then
echo "The ${STATUS_CONTEXT} status on ${SHA} settled after ${carry_forward_waited}s."
fi
Expand Down Expand Up @@ -366,6 +405,15 @@ wait_for_sibling_runs() {
set_wait_note "$all_ids"
}

# Every carry-forward red. The run cannot see a verdict recorded after it, so
# the message names both remedies: a failed or missing full run needs the full
# workflow again, and once the status is `success` re-running this run is
# enough, where a new commit would re-run every lane.
fail_carry_forward() {
echo "::error::no successful ${STATUS_CONTEXT} status on ${SHA}; re-run the full workflow${carry_forward_wait_note}. Once ${STATUS_CONTEXT} on ${SHA} is success, re-run this run instead."
exit 1
}

if [[ "$contract_only" == true ]]; then
echo "Contract-only event: reading the ${STATUS_CONTEXT} status on ${SHA} instead of aggregating skipped lanes."
if [[ "$CARRY_FORWARD_WAIT_SECONDS" -gt 0 ]]; then
Expand All @@ -375,8 +423,7 @@ if [[ "$contract_only" == true ]]; then
if [[ "$carry_forward_wait_status" -ne 0 ]]; then
# Never pass on timeout: a run that could still write this SHA's verdict
# is in flight, so whatever is on the SHA right now is not settled.
echo "::error::no successful ${STATUS_CONTEXT} status on ${SHA}; re-run the full workflow${carry_forward_wait_note}"
exit 1
fail_carry_forward
fi
fi
# Only when the loop did not read it: a failed Actions read or a mid-loop
Expand All @@ -385,16 +432,14 @@ if [[ "$contract_only" == true ]]; then
# shellcheck disable=SC2310 # read_carried_state handles its own errexit; the caller classifies the status.
if ! read_carried_state; then
cat "$gh_stderr" >&2
echo "::error::no successful ${STATUS_CONTEXT} status on ${SHA}; re-run the full workflow${carry_forward_wait_note}"
exit 1
fail_carry_forward
fi
fi
if [[ "$carried_state" == success ]]; then
echo "Carried forward: ${STATUS_CONTEXT} is success on ${SHA} (recorded by ${STATUS_CREATOR})."
exit 0
fi
echo "::error::no successful ${STATUS_CONTEXT} status on ${SHA}; re-run the full workflow${carry_forward_wait_note}"
exit 1
fail_carry_forward
fi

# ---------------------------------------------------------------------------
Expand Down
109 changes: 107 additions & 2 deletions .github/actions/ci-status/run.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,14 @@ user_status() {
printf '{"id":%s,"context":"ci-lanes","state":"%s","creator":{"login":"a-collaborator","type":"User"}}' "$1" "$2"
}

# writer_status <id> <state> <created_at> <writer-run-id>
# A bot status as a full run records it: `target_url` names the run that wrote
# it, which is how the wait recognizes that run's re-run.
writer_status() {
printf '{"id":%s,"context":"ci-lanes","state":"%s","created_at":"%s","target_url":"https://github.com/%s/actions/runs/%s","creator":{"login":"github-actions[bot]","type":"Bot"}}' \
"$1" "$2" "$3" "$repository" "$4"
}

# --- carry-forward wait fixtures -------------------------------------------

statuses_key="GET_repos_melodic-software_ci-workflows_commits_${sha}_statuses"
Expand Down Expand Up @@ -313,6 +321,13 @@ run_entry() {
printf '{"id":%s,"status":"%s","created_at":"%s"}' "$1" "$2" "$3"
}

# attempt_entry <id> <status> <created_at> <run_attempt> <run_started_at>
# `run_started_at` is when the CURRENT attempt started; a re-run moves it and
# leaves `created_at` alone.
attempt_entry() {
printf '{"id":%s,"status":"%s","created_at":"%s","run_attempt":%s,"run_started_at":"%s"}' "$1" "$2" "$3" "$4" "$5"
}

earlier_full_run="$(run_entry 4000 in_progress 2026-09-05T12:00:00Z)"

# --- full mode -------------------------------------------------------------
Expand Down Expand Up @@ -588,8 +603,8 @@ run_case 1 'skipped skipped' pass true true CARRY_FORWARD_WAIT_SECONDS=60
expect_log 'Waiting 15s for in-flight run(s) 4000'
expect_log "::error::no successful ci-lanes status on ${sha}; re-run the full workflow (waited 15s of 60s on in-flight run(s): 4000)"

# A deliberate trade: a settled verdict ends the wait even with a sibling
# incomplete, so an older verdict can carry forward while a re-run is in flight.
# A settled failure ends the wait even with a sibling incomplete, when that
# sibling is not the writer re-running (this status names no writer at all).
# An absent or `pending` status still waits, as the case above shows.
echo 'case: a settled status ends the wait even with a sibling still incomplete'
clear_status_fixtures
Expand Down Expand Up @@ -760,6 +775,96 @@ run_case 1 'skipped skipped' pass true true CARRY_FORWARD_WAIT_SECONDS=60
expect_no_log 'Waiting '
expect_log "::error::no successful ci-lanes status on ${sha}; re-run the full workflow"

# --- carry-forward: a stale failure while its writer re-runs ----------------
#
# Full run 4000 fails and records ci-lanes=failure at 12:00:10Z. Someone re-runs
# it: attempt 2 starts at 12:00:20Z. A contract-only run arrives meanwhile.

stale_failure="$(writer_status 100 failure 2026-09-05T12:00:10Z 4000)"
writer_rerun="$(attempt_entry 4000 in_progress 2026-09-05T11:59:00Z 2 2026-09-05T12:00:20Z)"

# Without holding a settled failure open for the writer's re-run, this reads
# the stale failure on the first poll and goes red while the re-run that
# replaces it is still in flight.
echo 'case: a stale failure waits for the in-flight re-run of its writer and carries its success'
clear_status_fixtures
clear_run_fixtures
current_run
status_list_on_call 1 "[${stale_failure}]"
status_list_on_call 2 "[$(writer_status 200 success 2026-09-05T12:04:00Z 4000),${stale_failure}]"
workflow_runs_on_call 1 "[${writer_rerun}]"
workflow_runs_on_call 2 '[]'
run_case 0 'skipped skipped' pass true true CARRY_FORWARD_WAIT_SECONDS=60
expect_log "Waiting 15s for in-flight run(s) 4000 on ${sha} to finish (waited 0s of 60s)."
expect_log "The ci-lanes status on ${sha} settled after 15s."
expect_log "Carried forward: ci-lanes is success on ${sha}"

# Without the ceiling a re-run slower than it holds the run forever, and
# without "never pass on timeout" it would have to resolve to something.
echo 'case: a writer re-run that outlasts the ceiling fails closed naming it'
clear_status_fixtures
clear_run_fixtures
current_run
status_list "[${stale_failure}]"
workflow_runs "[${writer_rerun}]"
run_case 1 'skipped skipped' pass true true CARRY_FORWARD_WAIT_SECONDS=30
expect_log '::warning::reached the 30s carry-forward-wait-seconds ceiling with in-flight run(s) 4000'
expect_log "::error::no successful ci-lanes status on ${sha}; re-run the full workflow (waited 30s of 30s on in-flight run(s): 4000). Once ci-lanes on ${sha} is success, re-run this run instead."

# Without the start-time term this waits on the writer's own first attempt,
# which records the status a moment before it completes, and turns a prompt
# red into a wait.
echo 'case: the writer attempt that recorded the failure is not waited on'
clear_status_fixtures
clear_run_fixtures
current_run
status_list "[${stale_failure}]"
workflow_runs "[$(attempt_entry 4000 in_progress 2026-09-05T11:59:00Z 1 2026-09-05T11:59:00Z)]"
run_case 1 'skipped skipped' pass true true CARRY_FORWARD_WAIT_SECONDS=60
expect_no_log 'Waiting '
expect_log "::error::no successful ci-lanes status on ${sha}; re-run the full workflow"

# Without narrowing the wait to the writer, a failed SHA brings back the mutual
# wait: each contract-only run holds the failure open for the other until the
# ceiling. 4300 is itself a re-run started after the failure, so a
# `run_attempt > 1` test in place of the writer test waits on it too.
echo 'case: two contract-only siblings on a failed SHA do not wait on each other'
clear_status_fixtures
clear_run_fixtures
current_run
status_list "[${stale_failure}]"
workflow_runs "[$(attempt_entry 4100 in_progress "$current_run_created_at" 1 "$current_run_created_at"),$(attempt_entry 4300 in_progress 2026-09-05T12:00:00Z 2 2026-09-05T12:01:00Z)]"
run_case 1 'skipped skipped' pass true true CARRY_FORWARD_WAIT_SECONDS=60
expect_no_log 'Waiting '
expect_log "::error::no successful ci-lanes status on ${sha}; re-run the full workflow."

# Without the unsettled-state fail a re-run of the full run that ends without
# recording anything would leave an absent status to pass. No status names no
# writer, so this is the ordinary wait on any sibling, and it still fails.
echo 'case: a missing status still fails after the re-run ends without recording one'
clear_status_fixtures
clear_run_fixtures
current_run
status_list '[]'
workflow_runs_on_call 1 "[${writer_rerun}]"
workflow_runs_on_call 2 '[]'
run_case 1 'skipped skipped' pass true true CARRY_FORWARD_WAIT_SECONDS=60
expect_log "Waiting 15s for in-flight run(s) 4000 on ${sha} to finish (waited 0s of 60s)."
expect_log "::error::no successful ci-lanes status on ${sha}; re-run the full workflow (waited 15s of 60s on in-flight run(s): 4000)."
expect_no_log 'Carried forward'

# Without the second remedy the message only ever says to re-run the full
# workflow, and a red that outlived its failure gets a new commit (every lane
# again) where re-running this run is enough.
echo 'case: the failure message names the re-run-this-run remedy for a later success'
clear_status_fixtures
clear_run_fixtures
current_run
status_list "[${stale_failure}]"
workflow_runs '[]'
run_case 1 'skipped skipped' pass true true CARRY_FORWARD_WAIT_SECONDS=60
expect_log "::error::no successful ci-lanes status on ${sha}; re-run the full workflow. Once ci-lanes on ${sha} is success, re-run this run instead."

# The documented trade, pinned deliberately from the passing side. A recorded
# success ends the wait even with a sibling in flight, so a re-run of this SHA
# that is about to overwrite it does not hold the run. Without that the loop
Expand Down
Loading
Loading