Repository navigation
feat: implement issue #2081 — CI check: fail when the dev-lead retry sweep sends a client_payload field the pinned channel's intent parser does not read #2085
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
| # scripts/check-retry-payload-fields.sh (#2081) — fail when the dev-lead retry | ||
| # sweep sends a client_payload field that the pinned channel's intent parser | ||
| # does not read. | ||
| # | ||
| # The sweep (scripts/dev-lead-retry.sh and the libs it sources) dispatches | ||
| # repository_dispatch events whose client_payload is parsed by | ||
| # scripts/dev-lead-intent.sh at the channel tag dev-lead.yml pins via agent_ref, | ||
| # not at main. A field the sweep on main sends but the pinned parser ignores is | ||
| # dropped, and the dispatched run fails or misroutes (#2050: comment_node_id | ||
| # broke every fix-bot-comment retry). Order: harness support reaches the pinned | ||
| # channel first, the sweep change merges second (AGENTS.md "Caller-stub input | ||
| # forwarding across channel pins"). | ||
| # | ||
| # The channel is read from the stub's agent_ref, so a major bump needs no edit | ||
| # here. Only top-level client_payload keys are compared; nested keys (e.g. | ||
| # checks[].details_url) are consumed downstream of the parser. | ||
| # | ||
| # Usage: check-retry-payload-fields.sh [root] (root defaults to the repo root) | ||
| # Exit: 0 every sent field is read · 1 one or more unread fields (each printed) | ||
| # 2 setup error (agent_ref missing/malformed, tag unresolvable, nothing | ||
| # parsed), so the sweep cannot be certified | ||
|
|
||
| ROOT="${1:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" | ||
| STUB=".github/workflows/dev-lead.yml" | ||
| SWEEP="scripts/dev-lead-retry.sh" | ||
| PARSER="scripts/dev-lead-intent.sh" | ||
|
|
||
| # Fields the sweep sends for logs and audit only; no harness code reads them, at | ||
| # any channel. Adding a field here asserts that nothing downstream needs it. | ||
| # repo — redundant with the dispatch target repository | ||
| # attempt — retry counter carried for the run log (#781) | ||
| INFORMATIONAL_FIELDS=(repo attempt) | ||
|
|
||
| die() { echo "check-retry-payload-fields: $*" >&2; exit 2; } | ||
|
|
||
| [ -f "$ROOT/$STUB" ] || die "$STUB not found under $ROOT" | ||
| channel=$(sed -n 's/^[[:space:]]*agent_ref:[[:space:]]*["'\'']\{0,1\}\([^"'\''[:space:]#]*\).*/\1/p' "$ROOT/$STUB" | head -n1) | ||
| [ -n "$channel" ] || die "no agent_ref in $STUB" | ||
| [[ "$channel" =~ ^dev-lead/v[0-9]+-(next|ring0|ring1|stable)$ ]] \ | ||
| || die "agent_ref '$channel' in $STUB is not a dev-lead channel tag" | ||
|
|
||
| # CI checkouts carry no tags, and a local tag may be stale after a promote, so | ||
| # refresh from origin when there is one. A failed fetch falls back to the local | ||
| # tag; a missing tag is a setup error. | ||
| if git -C "$ROOT" remote get-url origin >/dev/null 2>&1; then | ||
| git -C "$ROOT" fetch --quiet --force --depth=1 origin "refs/tags/${channel}:refs/tags/${channel}" 2>/dev/null \ | ||
| || echo "check-retry-payload-fields: warning: could not fetch ${channel} from origin; using the local tag" >&2 | ||
|
don-petry marked this conversation as resolved.
Outdated
don-petry marked this conversation as resolved.
Outdated
|
||
| fi | ||
| parser_src=$(git -C "$ROOT" show "refs/tags/${channel}:${PARSER}" 2>/dev/null) \ | ||
| || die "cannot read ${PARSER} at ${channel}" | ||
|
|
||
| # The sweep plus every lib it sources (transitively), relative to ROOT. | ||
| files=() | ||
| queue=("$SWEEP") | ||
| while [ "${#queue[@]}" -gt 0 ]; do | ||
| f="${queue[0]}"; queue=("${queue[@]:1}") | ||
| case " ${files[*]-} " in *" $f "*) continue ;; esac | ||
| [ -f "$ROOT/$f" ] || die "$f not found under $ROOT" | ||
| files+=("$f") | ||
| while IFS= read -r lib; do | ||
| queue+=("scripts/$lib") | ||
| done < <(sed -n 's/^[[:space:]]*\(source\|\.\)[[:space:]]\{1,\}"\$SCRIPT_DIR\/\([^"]*\.sh\)".*/\2/p' "$ROOT/$f") | ||
|
don-petry marked this conversation as resolved.
Outdated
|
||
| done | ||
|
|
||
| # Emit "<file>:<line> <field>" for every top-level key the sweep sends, from jq | ||
| # object literals (`client_payload: { key: …, … }`, keys at depth 1 only) and | ||
| # the gh -f form (`client_payload[key]=`). | ||
| sent=$( | ||
| for f in "${files[@]}"; do | ||
| awk -v file="$f" ' | ||
| { | ||
| line = $0 | ||
| while (match(line, /client_payload\[[A-Za-z_][A-Za-z0-9_]*\]/)) { | ||
| k = substr(line, RSTART + 15, RLENGTH - 16) | ||
| print file ":" NR " " k | ||
| line = substr(line, RSTART + RLENGTH) | ||
| } | ||
| i = 1 | ||
| if (!inblk) { | ||
| if (!match($0, /client_payload[[:space:]]*:[[:space:]]*\{/)) next | ||
|
don-petry marked this conversation as resolved.
Outdated
|
||
| inblk = 1; depth = 1; expect = 1; tok = "" | ||
| i = RSTART + RLENGTH | ||
| } | ||
| n = length($0) | ||
| for (; i <= n && inblk; i++) { | ||
| c = substr($0, i, 1) | ||
| if (c == "{" || c == "[" || c == "(") { depth++; tok = ""; expect = 0; continue } | ||
|
don-petry marked this conversation as resolved.
Outdated
don-petry marked this conversation as resolved.
Outdated
|
||
| if (c == "}" || c == "]" || c == ")") { | ||
| if (depth == 1 && expect && tok != "") print file ":" NR " " tok | ||
| depth--; tok = "" | ||
| if (depth == 0) inblk = 0 | ||
| continue | ||
| } | ||
|
don-petry marked this conversation as resolved.
Outdated
|
||
| if (depth != 1) continue | ||
| if (c == ",") { | ||
| if (expect && tok != "") print file ":" NR " " tok | ||
| expect = 1; tok = ""; continue | ||
| } | ||
| if (!expect) continue | ||
| if (c == ":") { if (tok != "") print file ":" NR " " tok; expect = 0; tok = ""; continue } | ||
| if (c ~ /[A-Za-z0-9_]/) { tok = tok c; continue } | ||
| if (c == "$" || c == "\"" || c ~ /[[:space:]]/) continue | ||
| expect = 0; tok = "" | ||
| } | ||
|
don-petry marked this conversation as resolved.
Outdated
don-petry marked this conversation as resolved.
Outdated
|
||
| }' "$ROOT/$f" | ||
| done | ||
| ) | ||
| [ -n "$sent" ] || die "no client_payload fields found in ${files[*]}; the parser may need updating" | ||
|
|
||
| read_fields=$(grep -oE 'client_payload\.[A-Za-z_][A-Za-z0-9_]*' <<<"$parser_src" | sed 's/^client_payload\.//' | sort -u || true) | ||
|
don-petry marked this conversation as resolved.
Outdated
don-petry marked this conversation as resolved.
Outdated
don-petry marked this conversation as resolved.
Outdated
|
||
|
|
||
| missing=0 | ||
| while read -r loc field; do | ||
| case " ${INFORMATIONAL_FIELDS[*]} " in *" $field "*) continue ;; esac | ||
| if ! grep -qxF "$field" <<<"$read_fields"; then | ||
| echo "client_payload.${field} (sent at ${loc}) is not read by ${PARSER} at ${channel}" | ||
| missing=1 | ||
| fi | ||
| done <<<"$sent" | ||
|
|
||
| if [ "$missing" -ne 0 ]; then | ||
| echo "Land the parser change and promote ${channel} before the sweep sends these fields (AGENTS.md \"Caller-stub input forwarding across channel pins\")." | ||
| exit 1 | ||
| fi | ||
| echo "OK: every client_payload field the sweep sends is read by ${PARSER} at ${channel}" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| #!/usr/bin/env bats | ||
|
don-petry marked this conversation as resolved.
|
||
| # scripts/check-retry-payload-fields.sh (#2081) — fails when the dev-lead retry | ||
| # sweep on main sends a client_payload field that scripts/dev-lead-intent.sh at | ||
| # the channel pinned by dev-lead.yml's agent_ref does not read. | ||
|
|
||
| REPO_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)" | ||
| CHECK="$REPO_ROOT/scripts/check-retry-payload-fields.sh" | ||
|
|
||
| setup() { | ||
| SANDBOX="$(mktemp -d)" | ||
| mkdir -p "$SANDBOX/.github/workflows" "$SANDBOX/scripts/lib" | ||
| git -C "$SANDBOX" init -q | ||
| git -C "$SANDBOX" config user.email t@example.com | ||
| git -C "$SANDBOX" config user.name t | ||
| } | ||
|
|
||
| teardown() { | ||
| rm -rf "$SANDBOX" | ||
| } | ||
|
|
||
| # write_stub <agent_ref> | ||
| write_stub() { | ||
| cat > "$SANDBOX/.github/workflows/dev-lead.yml" <<EOF | ||
| jobs: | ||
| dev-lead: | ||
| uses: petry-projects/.github-private/.github/workflows/dev-lead-reusable.yml@$1 | ||
| with: | ||
| agent_ref: $1 | ||
| EOF | ||
| } | ||
|
|
||
| # write_intent <field>... — an intent parser that reads each named field. | ||
| write_intent() { | ||
| { | ||
| echo '#!/usr/bin/env bash' | ||
| for f in "$@"; do | ||
| echo "x=\$(jq -r '.client_payload.${f} // empty' \"\$EVENT_PATH\")" | ||
| done | ||
| } > "$SANDBOX/scripts/dev-lead-intent.sh" | ||
| } | ||
|
|
||
| # write_sweep <field>... — a sweep that sends each named field in one payload. | ||
| write_sweep() { | ||
| { | ||
| echo '#!/usr/bin/env bash' | ||
| echo 'source "$SCRIPT_DIR/lib/helper.sh"' | ||
| echo "payload=\$(jq -n '{" | ||
| echo ' event_type: "dev-lead-reviews-retry",' | ||
| echo ' client_payload: {' | ||
| local f | ||
| for f in "$@"; do echo " ${f}: \$${f},"; done | ||
| echo ' checks: [{name: $name, conclusion: "failure",' | ||
|
don-petry marked this conversation as resolved.
Outdated
|
||
| echo ' id: $check_run_id}]' | ||
| echo ' }' | ||
| echo "}')" | ||
| } > "$SANDBOX/scripts/dev-lead-retry.sh" | ||
| [ -f "$SANDBOX/scripts/lib/helper.sh" ] || echo '#!/usr/bin/env bash' > "$SANDBOX/scripts/lib/helper.sh" | ||
| } | ||
|
|
||
| # tag_channel <tag> — commit the tree and point the channel tag at it. | ||
| tag_channel() { | ||
| git -C "$SANDBOX" add -A | ||
| git -C "$SANDBOX" commit -qm "release" --allow-empty | ||
| git -C "$SANDBOX" tag -f "$1" >/dev/null | ||
| } | ||
|
|
||
| @test "passes when every field the sweep sends is read by the pinned parser" { | ||
| write_stub dev-lead/v7-stable | ||
| write_intent pr_number head_sha checks | ||
| write_sweep pr_number head_sha | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 0 ] | ||
| } | ||
|
|
||
| # The #2050 defect: main's parser learns the field, the sweep sends it, and the | ||
| # pinned channel still points at the old parser. | ||
| @test "fails when the sweep sends a field only main's parser reads" { | ||
| write_stub dev-lead/v7-stable | ||
| write_intent pr_number head_sha checks | ||
| write_sweep pr_number head_sha | ||
| tag_channel dev-lead/v7-stable | ||
| write_intent pr_number head_sha checks comment_node_id | ||
| write_sweep pr_number head_sha comment_node_id | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 1 ] | ||
| [[ "$output" == *"client_payload.comment_node_id"* ]] | ||
| [[ "$output" == *"scripts/dev-lead-retry.sh:"* ]] | ||
| [[ "$output" == *"dev-lead/v7-stable"* ]] | ||
| [[ "$output" != *"client_payload.pr_number"* ]] | ||
| } | ||
|
|
||
| @test "checks the libs the sweep sources, not just the sweep itself" { | ||
| write_stub dev-lead/v7-stable | ||
| write_intent pr_number checks | ||
| write_sweep pr_number | ||
| cat > "$SANDBOX/scripts/lib/helper.sh" <<'EOF' | ||
| #!/usr/bin/env bash | ||
| payload=$(jq -n '{event_type: "x", client_payload: {pr_number: $p, new_field: $n}}') | ||
| EOF | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 1 ] | ||
| [[ "$output" == *"client_payload.new_field"* ]] | ||
| [[ "$output" == *"scripts/lib/helper.sh:2"* ]] | ||
| } | ||
|
|
||
| @test "catches the gh -f client_payload[field]= form" { | ||
| write_stub dev-lead/v7-stable | ||
| write_intent pr_number checks | ||
| write_sweep pr_number | ||
| echo 'gh api x -f "client_payload[other_field]=1"' >> "$SANDBOX/scripts/dev-lead-retry.sh" | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 1 ] | ||
| [[ "$output" == *"client_payload.other_field"* ]] | ||
| } | ||
|
|
||
| # Nested keys (checks[].name, checks[].id) are read by fix-ci, not the intent | ||
| # parser; only top-level client_payload keys are compared. | ||
| @test "ignores keys nested inside a top-level field" { | ||
| write_stub dev-lead/v7-stable | ||
| write_intent pr_number checks | ||
| write_sweep pr_number | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" != *"client_payload.name"* ]] | ||
| [[ "$output" != *"client_payload.id"* ]] | ||
| } | ||
|
|
||
| @test "allowlisted informational fields (repo, attempt) do not fail the check" { | ||
| write_stub dev-lead/v7-stable | ||
| write_intent pr_number checks | ||
| write_sweep pr_number repo attempt | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 0 ] | ||
| } | ||
|
|
||
| # AC2: the channel comes from the stub, so a major bump needs no edit here. | ||
| @test "follows the stub's agent_ref to another major and channel" { | ||
| write_intent pr_number checks | ||
| write_sweep pr_number | ||
| tag_channel dev-lead/v7-stable | ||
| write_intent pr_number checks comment_node_id | ||
| write_sweep pr_number comment_node_id | ||
| write_stub dev-lead/v8-next | ||
| tag_channel dev-lead/v8-next | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"dev-lead/v8-next"* ]] | ||
| } | ||
|
|
||
| @test "exits 2 when the stub has no agent_ref" { | ||
| echo 'jobs: {}' > "$SANDBOX/.github/workflows/dev-lead.yml" | ||
| write_intent pr_number | ||
| write_sweep pr_number | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 2 ] | ||
| [[ "$output" == *"agent_ref"* ]] | ||
| } | ||
|
|
||
| @test "exits 2 when agent_ref is not a dev-lead channel tag" { | ||
| write_stub main | ||
| write_intent pr_number | ||
| write_sweep pr_number | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 2 ] | ||
| [[ "$output" == *"main"* ]] | ||
| } | ||
|
|
||
| @test "exits 2 when the channel tag cannot be resolved" { | ||
| write_stub dev-lead/v9-stable | ||
| write_intent pr_number | ||
| write_sweep pr_number | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 2 ] | ||
| [[ "$output" == *"dev-lead/v9-stable"* ]] | ||
| } | ||
|
|
||
| @test "exits 2 when the sweep sends no client_payload fields at all" { | ||
| write_stub dev-lead/v7-stable | ||
| write_intent pr_number | ||
| echo '#!/usr/bin/env bash' > "$SANDBOX/scripts/dev-lead-retry.sh" | ||
| tag_channel dev-lead/v7-stable | ||
| run bash "$CHECK" "$SANDBOX" | ||
| [ "$status" -eq 2 ] | ||
| } | ||
|
|
||
| @test "lint.yml runs the check" { | ||
| grep -q 'bash scripts/check-retry-payload-fields.sh' "$REPO_ROOT/.github/workflows/lint.yml" | ||
| } | ||
|
|
||
| @test "AGENTS.md ordering rule points at the check" { | ||
| grep -q 'check-retry-payload-fields.sh' "$REPO_ROOT/AGENTS.md" | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.