Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
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
11 changes: 9 additions & 2 deletions .github/workflows/dev-lead-retry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ name: Dev-Lead Retry (rate-limited)
# whose registered reviewer-bot issue comment has no covering disposition (and no
# info_status_pattern match) gets one deduplicated fix-bot-comment retry per scan,
# addressed by the comment's node id (scan_pr_for_undispositioned_bot_comments).
#
# Each target repo parses a payload at its OWN dev-lead.yml pin, so the sweep
# holds back (with a ::warning::) any payload carrying a client_payload field the
# target's pinned parser does not read (#2086, scripts/lib/dispatch-channel-gate.sh).

on:
schedule:
Expand Down Expand Up @@ -72,8 +76,8 @@ jobs:
# `agent_ref`; running the sweep from main let it send payload fields that
# channel could not read yet (#2017). Read the pin from the default branch
# and check scripts/ out at it. Never hard-code the major here.
# One pin covers the whole sweep: target repos whose dev-lead.yml pins an
# older channel than this repo can still see the skew (#2086).
# Target repos pinned to another channel are covered per payload by the
# gate the run step turns on with SWEEP_AGENT_REF (#2086).
- name: Fetch dev-lead caller stub
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
Expand Down Expand Up @@ -106,4 +110,7 @@ jobs:
persist-credentials: false

- name: Scan and retry rate-limited PRs
env:
# Turns on the per-target payload-field gate (#2086).
SWEEP_AGENT_REF: ${{ steps.channel.outputs.agent_ref }}
run: bash scripts/dev-lead-retry.sh
42 changes: 42 additions & 0 deletions scripts/dev-lead-retry.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ set -euo pipefail
# DISPATCH_DELAY_SEC — seconds between repo dispatches (default: 30) to
# prevent cascading org-wide rate-limit hits
# DRY_RUN — if "true", log what would be dispatched but don't send
# SWEEP_AGENT_REF — the dev-lead channel the sweep's scripts/ run at;
# turns on the #2086 per-target payload-field gate
# (dev-lead-retry.yml passes it; unset = gate off)
# NOW_ISO — override current time for testing (ISO-8601 UTC)
# BOT_COMMENT_RETRY_CLAIM_SETTLE_SEC — seconds to wait after posting a
# bot-comment retry marker before re-listing markers
Expand Down Expand Up @@ -78,6 +81,13 @@ set -euo pipefail
# threads with no reply from our automation, and dispatches one deduplicated
# fix-reviews pass per PR (lib/bot-thread-retry.sh). Threads that exhaust their
# attempts get a single visible notice, so the stall is never silent.
#
# Each payload is gated on the target repo's own dev-lead.yml pin (#2086). The
# sweep runs one release of scripts/ (this repo's pinned channel), but every
# target parses a payload with dev-lead-intent.sh at its own pin. A payload
# carrying a client_payload field that parser does not read is held back, with
# a ::warning:: naming the field (lib/dispatch-channel-gate.sh); payloads every
# release reads still reach older-pinned repos.

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# Escalation gate (#946): pr_has_escalation_label / NEEDS_HUMAN_REVIEW_LABEL.
Expand All @@ -96,6 +106,9 @@ source "$SCRIPT_DIR/lib/bot-comment-retry.sh"
# Unreplied bot review-thread retry decision + thread fetch (#2046).
# shellcheck source=lib/bot-thread-retry.sh
source "$SCRIPT_DIR/lib/bot-thread-retry.sh"
# Per-target payload-field gate (#2086): dcg_init / dcg_payload_allowed / dcg_target_reads.
# shellcheck source=lib/dispatch-channel-gate.sh
source "$SCRIPT_DIR/lib/dispatch-channel-gate.sh"

TARGET_ORG="${TARGET_ORG:-petry-projects}"
DELEGATION_ORGS="${DELEGATION_ORGS:-}"
Expand Down Expand Up @@ -231,6 +244,8 @@ dispatch_ci_retry() {
app_slug: "github-actions", id: $check_run_id}]
}
}')
# The target's pinned parser must read every field sent (#2086).
dcg_payload_allowed "$repo" "$payload" || return 1
if ! echo "$payload" | gh api --method POST "repos/${repo}/dispatches" --input - >/dev/null 2>&1; then
echo " [warn] dispatch failed for PR ${pr_number} in ${repo}" >&2
return 1
Expand Down Expand Up @@ -261,6 +276,8 @@ dispatch_reviews_retry() {
intent_type: $intent_type
}
}')
# The target's pinned parser must read every field sent (#2086).
dcg_payload_allowed "$repo" "$payload" || return 1
if ! echo "$payload" | gh api --method POST "repos/${repo}/dispatches" --input - >/dev/null 2>&1; then
echo " [warn] dispatch failed for PR ${pr_number} in ${repo}" >&2
return 1
Expand Down Expand Up @@ -292,6 +309,8 @@ dispatch_issue_retry() {
attempt: $attempt
}
}')
# The target's pinned parser must read every field sent (#2086).
dcg_payload_allowed "$repo" "$payload" || return 1
if ! echo "$payload" | gh api --method POST "repos/${repo}/dispatches" --input - >/dev/null 2>&1; then
echo " [warn] dispatch failed for issue ${issue_number} in ${repo}" >&2
fi
Expand Down Expand Up @@ -589,6 +608,11 @@ withdraw_bot_comment_retry_marker() {
fi
}

# The client_payload keys dispatch_bot_comment_retry sends, so the scan can check
# the target reads them (#2086) BEFORE it posts a retry marker. Keep in step with
# the payload below (tests/dev-lead/unit/test_dispatch_channel_gate.bats checks).
BOT_COMMENT_RETRY_FIELDS=(pr_number head_sha repo intent_type comment_node_id)

# dispatch_bot_comment_retry <repo> <pr_number> <head_sha> <comment_node_id>
# Re-dispatches a fix-bot-comment pass for ONE bot comment (#2017). Reuses the
# dev-lead-reviews-retry type every caller stub already subscribes to, so no stub
Expand Down Expand Up @@ -618,6 +642,8 @@ dispatch_bot_comment_retry() {
comment_node_id: $comment_node_id
}
}')
# The target's pinned parser must read every field sent (#2086).
dcg_payload_allowed "$repo" "$payload" || return 1
if ! echo "$payload" | gh api --method POST "repos/${repo}/dispatches" --input - >/dev/null 2>&1; then
echo " [warn] dispatch failed for PR ${pr_number} in ${repo}" >&2
return 1
Expand Down Expand Up @@ -763,6 +789,13 @@ scan_pr_for_undispositioned_bot_comments() {
now_iso="${NOW_ISO:-$(date -u +%Y-%m-%dT%H:%M:%SZ)}"
echo " [retry] bot-comment ${cid} ($(jq -r '.login' <<< "$pick")) on PR ${pr_number}: undispositioned → fix-bot-comment attempt ${attempt}" >&2

# A target pinned to a parser that drops comment_node_id would fail the run
# (#2050); hold before the marker, which would block the comment for the whole
# pending window (#2086).
if ! dcg_target_reads "$repo" "${BOT_COMMENT_RETRY_FIELDS[@]}"; then
echo "0"; return 0
fi

# Record the attempt BEFORE dispatching so a concurrent caller (the cron and
# pr-review's gate verdict) sees it as pending and does not duplicate it.
local marker_id=""
Expand Down Expand Up @@ -1257,6 +1290,15 @@ main() {
local repo_count="${#all_repos[@]}"
echo "[retry] scanning ${repo_count} repo(s) across org(s)"

# Hold each retry payload back from a target whose pinned parser does not
# read all its fields (#2086). Off unless the workflow passes SWEEP_AGENT_REF.
dcg_init || exit 1
if [ -n "$DCG_SWEEP_REF" ]; then
echo "[retry] sweep channel: ${DCG_SWEEP_REF} (a payload goes only to repos whose pinned parser reads all its fields)"
else
echo "[retry] SWEEP_AGENT_REF unset: per-target payload-field gate off (#2086)"
fi

local repo_index=0
for repo in "${all_repos[@]}"; do
if [ "$repo_index" -gt 0 ] && [ "$DISPATCH_DELAY_SEC" -gt 0 ]; then
Expand Down
207 changes: 207 additions & 0 deletions scripts/lib/dispatch-channel-gate.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
#!/usr/bin/env bash
# dispatch-channel-gate.sh — hold a retry payload back from a target repo whose
# pinned dev-lead harness does not read every field in it (#2086).
#
# WHY THIS EXISTS
# dev-lead-retry.yml runs scripts/ at ONE release: the channel this repo's
# dev-lead.yml pins via `agent_ref` (#2050). The sweep then sends
# repository_dispatch events to every repo in the org, and each target parses
# them with scripts/dev-lead-intent.sh at ITS OWN dev-lead.yml pin (the stub
# contract allows a per-repo ring/channel pin). A target pinned older than the
# sweep can be sent a client_payload field its parser drops — the #2017 / #2050
# skew again, across repos instead of across time.
#
# WHAT THIS LIBRARY DECIDES
# Per payload, not per repo: most retry payloads carry only fields every
# release reads, so a target on an older channel (the fleet sits on stable)
# keeps its retries; only a payload with a field the target's parser lacks is
# held back. For each target:
# pin its dev-lead.yml `agent_ref` (the reusable's default, main, when
# the stub passes none) — the ref its harness checks scripts/ out at
# fields the `.client_payload.<key>` reads in dev-lead-intent.sh and the
# lib/*.sh files it names, at that pin (comments stripped; the
# same reading scripts/check-retry-payload-fields.sh does in CI)
# A payload goes out only when the target's parser reads all its fields
# (INFORMATIONAL ones aside). Pin == the sweep's channel needs no lookup: the
# sweep and that parser ship together, and CI checks them against each other.
# no dev-lead.yml nothing in that repo receives it → hold, no warning
# (a 404 counts only when the repo itself is readable)
# pin or parser unreadable compatibility unproven → hold, warn (fail closed)
# a field not read hold, warn once per repo and field set
# A held payload leaves its markers in place, so the retry resumes once the
# target is repinned to a channel that reads the field.
#
# The gate is OFF (every payload allowed, no API reads) until dcg_init runs with
# SWEEP_AGENT_REF set — dev-lead-retry.yml passes its resolved channel. Unit
# tests and scripts/check-retry-payload-fields.sh source the sweep without it.
#
# Env:
# SWEEP_AGENT_REF the channel tag the sweep's scripts/ run at; enables the gate
# DCG_HOST_REPO repo hosting the dev-lead scripts and channel tags (default:
# $GITHUB_REPOSITORY, else petry-projects/.github-private)

DCG_HOST_REPO="${DCG_HOST_REPO:-${GITHUB_REPOSITORY:-petry-projects/.github-private}}"
# Same channel-tag shape dev-lead-retry.yml accepts for the sweep's own pin.
DCG_CHANNEL_RE='^dev-lead/v[0-9]+-(next|ring0|ring1|stable)$'
DCG_PARSER="scripts/dev-lead-intent.sh"
# Sent for logs and audit only; no harness code reads them. Mirrors
# INFORMATIONAL_FIELDS in scripts/check-retry-payload-fields.sh.
DCG_INFORMATIONAL_FIELDS=(repo attempt)
DCG_SWEEP_REF=""
# repo -> pin | "!<rc>" (2 = no stub, 1 = unreadable); ref -> fields | "!"
declare -gA _DCG_PIN_CACHE=() _DCG_FIELDS_CACHE=() _DCG_WARNED=()

# dcg_init: turn the gate on for the sweep's channel. Returns 1 when
# SWEEP_AGENT_REF is set but is not a channel tag (a configuration error).
dcg_init() {
DCG_SWEEP_REF=""
[ -n "${SWEEP_AGENT_REF:-}" ] || return 0
if [[ ! "$SWEEP_AGENT_REF" =~ $DCG_CHANNEL_RE ]]; then
echo "::error::[retry] SWEEP_AGENT_REF '${SWEEP_AGENT_REF}' is not a dev-lead channel tag (dev-lead/v<N>-next|ring0|ring1|stable)" >&2
return 1
fi
DCG_SWEEP_REF="$SWEEP_AGENT_REF"
}

# dcg_parse_agent_ref: stdin = dev-lead.yml text; prints the ref its harness
# runs at — `agent_ref`, else the reusable's default `main`. Returns 1 when the
# value is not a plain ref (an expression, say), so it cannot be resolved here.
dcg_parse_agent_ref() {
local text ref
text="$(cat)"
ref="$(sed -nE "s/^[[:space:]]*agent_ref:[[:space:]]*[\"']?([^\"'[:space:]#]*).*/\1/p" <<<"$text" | head -n 1)"
if ! grep -qE "^[[:space:]]*agent_ref:" <<<"$text"; then
ref="main"
fi
[[ "$ref" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]] || return 1
printf '%s\n' "$ref"
}

# dcg_target_pin <repo>: prints the ref <repo>'s dev-lead harness runs at.
# Returns 2 when the (readable) repo has no dev-lead.yml, 1 when the pin is
# unreadable — the repo itself included.
# Cached per sweep.
dcg_target_pin() {
local repo="$1" out rc=0
if [ -n "${_DCG_PIN_CACHE[$repo]+set}" ]; then
out="${_DCG_PIN_CACHE[$repo]}"
if [[ "$out" == "!"* ]]; then return "${out#!}"; fi
printf '%s\n' "$out"; return 0
fi
out="$(gh api "repos/${repo}/contents/.github/workflows/dev-lead.yml" \
-H "Accept: application/vnd.github.raw" 2>&1)" || rc=$?
if [ "$rc" -ne 0 ]; then
rc=1
# GitHub also answers 404 for a repo the token cannot read; only a readable
# repo's 404 means the stub is absent.
if [[ "$out" == *"HTTP 404"* ]] \
&& gh api "repos/${repo}" --jq '.full_name' >/dev/null 2>&1; then
rc=2
fi
elif ! out="$(dcg_parse_agent_ref <<<"$out")"; then
rc=1
fi
if [ "$rc" -ne 0 ]; then
_DCG_PIN_CACHE[$repo]="!${rc}"
return "$rc"
fi
_DCG_PIN_CACHE[$repo]="$out"
printf '%s\n' "$out"
}

# _dcg_fetch_at <path> <ref>: raw file content from the host repo at <ref>.
_dcg_fetch_at() {
local path="$1" ref="$2" enc
enc="$(jq -rn --arg r "$ref" '$r | @uri')" || return 1
gh api "repos/${DCG_HOST_REPO}/contents/${path}?ref=${enc}" \
-H "Accept: application/vnd.github.raw" 2>/dev/null
}

# dcg_fields_read_at <ref>: the top-level client_payload keys the intent parser
# (and the lib/*.sh files it names, transitively) reads at <ref>, one per line.
# Returns 1 when any of those files cannot be read. Cached per sweep.
dcg_fields_read_at() {
local ref="$1" src all="" lib
if [ -n "${_DCG_FIELDS_CACHE[$ref]+set}" ]; then
[ "${_DCG_FIELDS_CACHE[$ref]}" != "!" ] || return 1
printf '%s' "${_DCG_FIELDS_CACHE[$ref]}"; return 0
fi
local -a queue=("$DCG_PARSER") seen=()
while [ "${#queue[@]}" -gt 0 ]; do
lib="${queue[0]}"; queue=("${queue[@]:1}")
case " ${seen[*]} " in *" $lib "*) continue ;; esac
seen+=("$lib")
if ! src="$(_dcg_fetch_at "$lib" "$ref")" || [ -z "$src" ]; then
_DCG_FIELDS_CACHE[$ref]="!"
return 1
fi
# Drop shell comments (not `$#`), so prose naming a field is not a read.
src="$(sed -E 's/(^|[^$])#.*$/\1/' <<<"$src")"
all+="$src"$'\n'
mapfile -t -O "${#queue[@]}" queue < <(grep -oE '/lib/[A-Za-z0-9_.-]+\.sh' <<<"$src" | sed 's|^|scripts|' | sort -u)
done
all="$(grep -oE '\.client_payload\.[A-Za-z_][A-Za-z0-9_]*' <<<"$all" \
| sed 's/^\.client_payload\.//' | sort -u || true)"
[ -z "$all" ] || all+=$'\n'
_DCG_FIELDS_CACHE[$ref]="$all"
printf '%s' "$all"
}

# _dcg_warn_once <key> <message>: a ::warning:: per key per sweep, so a repo with
# many held PRs logs its reason once.
_dcg_warn_once() {
[ -z "${_DCG_WARNED[$1]+set}" ] || return 0
_DCG_WARNED[$1]=1
echo "::warning::$2" >&2
}

# dcg_target_reads <repo> <field>...: 0 when <repo>'s pinned parser reads every
# field (informational ones aside), or the gate is off; 1 to hold the payload.
dcg_target_reads() {
local repo="$1"; shift
[ -n "$DCG_SWEEP_REF" ] || return 0
local pin rc=0
pin="$(dcg_target_pin "$repo")" || rc=$?
case "$rc" in
0) ;;
2)
echo " [hold] ${repo}: no dev-lead.yml, so nothing there receives a retry (#2086)" >&2
return 1 ;;
*)
_dcg_warn_once "pin:${repo}" "[retry] holding retries for ${repo}: cannot read the ref its dev-lead.yml runs the harness at (agent_ref), so the fields its parser reads are unproven (#2086)"
return 1 ;;
esac
[ "$pin" != "$DCG_SWEEP_REF" ] || return 0

local fields
if ! fields="$(dcg_fields_read_at "$pin")"; then
Comment thread
don-petry marked this conversation as resolved.
_dcg_warn_once "fields:${pin}" "[retry] holding retries for repos pinned to ${pin}: cannot read ${DCG_PARSER} and its libs there, so the fields it reads are unproven (#2086)"
return 1
fi
local f info missing=()
for f in "$@"; do
[ -n "$f" ] || continue
for info in "${DCG_INFORMATIONAL_FIELDS[@]}"; do
[ "$f" != "$info" ] || continue 2
done
grep -qxF -- "$f" <<<"$fields" || missing+=("$f")
done
[ "${#missing[@]}" -eq 0 ] && return 0
_dcg_warn_once "miss:${repo}:${missing[*]}" "[retry] holding retries for ${repo}: it pins ${pin}, whose ${DCG_PARSER} does not read client_payload field(s) ${missing[*]} that the sweep at ${DCG_SWEEP_REF} sends (#2086). Repin it to a channel that reads them."
echo " [hold] ${repo} (${pin}) does not read: ${missing[*]}" >&2
return 1
}

# dcg_payload_allowed <repo> <event_json>: dcg_target_reads over the top-level
# client_payload keys of a repository_dispatch body.
dcg_payload_allowed() {
local repo="$1" body="$2" keys
[ -n "$DCG_SWEEP_REF" ] || return 0
if ! keys="$(jq -r '.client_payload | keys[]' <<<"$body" 2>/dev/null)"; then
echo " [hold] ${repo}: unreadable retry payload (#2086)" >&2
return 1
fi
local -a fields=()
mapfile -t fields <<<"$keys"
dcg_target_reads "$repo" "${fields[@]}"
}
25 changes: 25 additions & 0 deletions tests/dev-lead/unit/test_bot_comment_retry.bats
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,31 @@ _graphql_page() {
[ "$marker_line" -lt "$dispatch_line" ]
}

@test "sweep(#2086): a target whose pinned parser drops comment_node_id gets no retry marker and no dispatch" {
_setup_sweep
export TRUSTED_BOTS="$TRUSTED"
export GRAPHQL_RESPONSE
GRAPHQL_RESPONSE="$(_graphql_page "$(_bot IC_cr coderabbitai 'Walkthrough')")"
# Gate on; the target pins an older channel whose parser reads every retry
# field except comment_node_id (the #2050 skew).
DCG_SWEEP_REF=dev-lead/v139-ring0
dcg_target_pin() { echo dev-lead/v1-stable; }
dcg_fields_read_at() { printf '%s\n' checks head_sha intent_type issue_number pr_number; }
Comment thread
don-petry marked this conversation as resolved.

run scan_pr_for_undispositioned_bot_comments "petry-projects/.github-private" 2009
[ "$status" -eq 0 ]
[ "${lines[-1]}" = "0" ]
[[ "$output" == *"::warning::"*"comment_node_id"* ]]
if grep -q '/dispatches' "$GH_LOG"; then cat "$GH_LOG"; return 1; fi
if grep -q 'dev-lead-bot-comment-retry' "$GH_LOG"; then cat "$GH_LOG"; return 1; fi

# Once that channel reads the field, the same scan dispatches.
dcg_fields_read_at() { printf '%s\n' checks comment_node_id head_sha intent_type issue_number pr_number; }
run scan_pr_for_undispositioned_bot_comments "petry-projects/.github-private" 2009
[ "${lines[-1]}" = "1" ]
[ "$(grep -c '/dispatches' "$GH_LOG")" -eq 1 ]
}

@test "sweep(#2089): a dev-lead:hands-off PR gets no retry marker and no dispatch" {
export PR_JSON='{"state":"open","head":{"sha":"abc","ref":"dev-lead/issue-2008-x","repo":{"full_name":"petry-projects/.github-private"}},"user":{"login":"don-petry"},"labels":[{"name":"dev-lead:hands-off"}]}'
_setup_sweep
Expand Down
Loading
Loading