From 87e2a9b7194ad0ab8796ce3c820d815e43003563 Mon Sep 17 00:00:00 2001 From: don-petry <{}+don-petry@users.noreply.github.com> Date: Fri, 9 Oct 2026 01:59:03 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20implement=20issue=20#2050=20?= =?UTF-8?q?=E2=80=94=20dev-lead-retry=20runs=20from=20main=20but=20dispatc?= =?UTF-8?q?hes=20to=20the=20stable-pinned=20harness:=20every=20fix-bot-com?= =?UTF-8?q?ment=20retry=20has=20failed=20since=20#2017=20merged=20(17=20fa?= =?UTF-8?q?ilures=20in=203=20h)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/dev-lead-retry.yml | 30 +++++ AGENTS.md | 11 ++ .../unit/test_dev_lead_retry_channel.bats | 116 ++++++++++++++++++ 3 files changed, 157 insertions(+) create mode 100644 tests/dev-lead/unit/test_dev_lead_retry_channel.bats diff --git a/.github/workflows/dev-lead-retry.yml b/.github/workflows/dev-lead-retry.yml index bbbc30788..dba6d4b35 100644 --- a/.github/workflows/dev-lead-retry.yml +++ b/.github/workflows/dev-lead-retry.yml @@ -67,9 +67,39 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 steps: + # Run the sweep at the SAME release as the harness it dispatches to (#2050). + # The dispatched runs execute scripts/ at the channel dev-lead.yml pins via + # `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. + - name: Fetch dev-lead caller stub + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.repository.default_branch }} + path: .channel-pin + sparse-checkout: .github/workflows/dev-lead.yml + sparse-checkout-cone-mode: false + token: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS || secrets.GITHUB_TOKEN }} + persist-credentials: false + + - name: Resolve pinned dev-lead channel + id: channel + env: + STUB_FILE: .channel-pin/.github/workflows/dev-lead.yml + run: | + set -euo pipefail + ref="$(sed -nE "s/^[[:space:]]*agent_ref:[[:space:]]*[\"']?([^\"'[:space:]#]*).*/\1/p" "$STUB_FILE" | head -n 1)" + if [[ ! "$ref" =~ ^dev-lead/v[0-9]+-(next|ring0|ring1|stable)$ ]]; then + echo "::error::dev-lead.yml agent_ref '${ref}' is empty or not a dev-lead channel tag (dev-lead/v-next|ring0|ring1|stable)" + exit 1 + fi + echo "Sweep runs scripts/ at pinned channel: ${ref}" + echo "agent_ref=${ref}" >>"$GITHUB_OUTPUT" + - name: Checkout agent repo uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + ref: ${{ steps.channel.outputs.agent_ref }} token: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS || secrets.GITHUB_TOKEN }} persist-credentials: false diff --git a/AGENTS.md b/AGENTS.md index 8a8f65563..e3fd6bff7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -945,6 +945,17 @@ even though every ref looks valid, because the stub is ahead of the channel it p pinned channel resolves to a commit that declares the input. Doing these out of order (forwarding first) is exactly the skew this rule prevents. +- **Same order for a new `client_payload` field from a dispatching sweep (#2050).** A sweep + such as `scripts/dev-lead-retry.sh` sends `repository_dispatch` payloads to the harness at + the pinned channel. A new field the sweep sends follows the same order: **(1)** the harness + change that reads the field (e.g. `scripts/dev-lead-intent.sh`) merges and **reaches the + pinned channel first**; **(2)** only then does the sweep change that sends it merge. A + sweep and harness change merged together breaks every dispatched run until the channel + catches up (#2017: every fix-bot-comment retry failed against `v139-stable`), and the + canary gate charges those failures to the next candidate. As a structural backstop, + `dev-lead-retry.yml` checks out `scripts/` at the channel `dev-lead.yml` pins (`agent_ref`), + not `main`, so the sweep it runs is the same release as the harness + (`tests/dev-lead/unit/test_dev_lead_retry_channel.bats`). - **Enforcement.** The dev-lead prompt guardrail (part C, #1254) stops the agent from introducing the skew at the source; the **Part A CI guard (#1253)** is the belt-and-braces check that fails a PR whose caller-stub `with:` forwards an input the pinned channel does not declare. diff --git a/tests/dev-lead/unit/test_dev_lead_retry_channel.bats b/tests/dev-lead/unit/test_dev_lead_retry_channel.bats new file mode 100644 index 000000000..ab317ccd6 --- /dev/null +++ b/tests/dev-lead/unit/test_dev_lead_retry_channel.bats @@ -0,0 +1,116 @@ +#!/usr/bin/env bats +# Channel-skew guard for the dev-lead retry sweep (#2050). +# +# dev-lead-retry.yml runs scripts/dev-lead-retry.sh, which dispatches runs that +# execute the harness at the channel this repo's caller stub pins +# (dev-lead.yml `agent_ref`). Running the sweep from `main` let it send payload +# fields the pinned harness could not read (#2017: every fix-bot-comment retry +# failed until stable caught up). The sweep must therefore check out scripts/ at +# that same channel. These tests pin: +# • the workflow reads `agent_ref` from dev-lead.yml on the default branch; +# • the agent checkout's `ref:` is that resolved value (never main, never a +# hard-coded major); +# • the resolve step (executed from the workflow text) accepts the live stub +# and other majors, and fails on an empty or malformed value. +# +# Run: bats tests/dev-lead/unit/test_dev_lead_retry_channel.bats + +bats_require_minimum_version 1.5.0 + +REPO_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)" +WF="$REPO_ROOT/.github/workflows/dev-lead-retry.yml" +STUB="$REPO_ROOT/.github/workflows/dev-lead.yml" + +setup() { + export GITHUB_OUTPUT="$BATS_TEST_TMPDIR/github_output" + : >"$GITHUB_OUTPUT" + RESOLVE_SCRIPT="$BATS_TEST_TMPDIR/resolve.sh" + yq -r '.jobs.retry.steps[] | select(.id == "channel") | .run' "$WF" >"$RESOLVE_SCRIPT" +} + +# Write a minimal caller stub fixture pinning agent_ref to $1. +_stub_fixture() { + local f="$BATS_TEST_TMPDIR/dev-lead.yml" + cat >"$f" <"$GITHUB_OUTPUT" + STUB_FILE="$(_stub_fixture "$ch")" run bash "$RESOLVE_SCRIPT" + [ "$status" -eq 0 ] + grep -qxF "agent_ref=$ch" "$GITHUB_OUTPUT" + done +} + +@test "resolve step fails when agent_ref is missing" { + local f="$BATS_TEST_TMPDIR/dev-lead.yml" + printf 'jobs:\n dev-lead:\n uses: x\n' >"$f" + STUB_FILE="$f" run bash "$RESOLVE_SCRIPT" + [ "$status" -ne 0 ] + [ ! -s "$GITHUB_OUTPUT" ] +} + +@test "resolve step fails on a malformed or non-channel agent_ref" { + local bad + for bad in main dev-lead/v139 dev-lead/vX-stable dev-lead/v139-canary pr-review/v1-stable 'dev-lead/v139-stable;x'; do + : >"$GITHUB_OUTPUT" + STUB_FILE="$(_stub_fixture "$bad")" run bash "$RESOLVE_SCRIPT" + [ "$status" -ne 0 ] + [ ! -s "$GITHUB_OUTPUT" ] + done +} From c7f6133e3b62048adaa627d7d06c4e5d9551b9eb Mon Sep 17 00:00:00 2001 From: don-petry Date: Fri, 9 Oct 2026 03:47:57 +0000 Subject: [PATCH 2/2] fix(dev-lead-retry): address #2083 review findings (#2050) - tests: assert the exact failure status (exit 1) instead of any non-zero, so grep or resolve-step errors cannot pass the negative tests; skip the file locally when yq is missing, but fail in CI. - AGENTS.md: describe the #2017 skew as the pre-backstop failure and note the ordering still matters for target repos pinned to an older channel. - dev-lead-retry.yml: document that one pin covers the whole sweep (per-target-repo resolution tracked in #2086). Test-Change-Justification: review threads on #2083 (gemini-code-assist and cubic) asked for exact exit-status assertions and a yq guard in this PR's own new test file. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019K6UsD7r25pAcRjJL3G6Kx --- .github/workflows/dev-lead-retry.yml | 2 ++ AGENTS.md | 16 +++++++++------- .../unit/test_dev_lead_retry_channel.bats | 14 +++++++++++--- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/.github/workflows/dev-lead-retry.yml b/.github/workflows/dev-lead-retry.yml index dba6d4b35..ab19e0fca 100644 --- a/.github/workflows/dev-lead-retry.yml +++ b/.github/workflows/dev-lead-retry.yml @@ -72,6 +72,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). - name: Fetch dev-lead caller stub uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/AGENTS.md b/AGENTS.md index e3fd6bff7..b519c45f9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -949,13 +949,15 @@ even though every ref looks valid, because the stub is ahead of the channel it p such as `scripts/dev-lead-retry.sh` sends `repository_dispatch` payloads to the harness at the pinned channel. A new field the sweep sends follows the same order: **(1)** the harness change that reads the field (e.g. `scripts/dev-lead-intent.sh`) merges and **reaches the - pinned channel first**; **(2)** only then does the sweep change that sends it merge. A - sweep and harness change merged together breaks every dispatched run until the channel - catches up (#2017: every fix-bot-comment retry failed against `v139-stable`), and the - canary gate charges those failures to the next candidate. As a structural backstop, - `dev-lead-retry.yml` checks out `scripts/` at the channel `dev-lead.yml` pins (`agent_ref`), - not `main`, so the sweep it runs is the same release as the harness - (`tests/dev-lead/unit/test_dev_lead_retry_channel.bats`). + pinned channel first**; **(2)** only then does the sweep change that sends it merge. Before + the backstop below, the sweep ran from `main`, so a sweep change that merged ahead of the + channel broke every dispatched run until the channel caught up (#2017: every + fix-bot-comment retry failed against `v139-stable`), and the canary gate charged those + failures to the next candidate. As a structural backstop, `dev-lead-retry.yml` now checks + out `scripts/` at the channel `dev-lead.yml` pins (`agent_ref`), not `main`, so the sweep + and the harness it dispatches to in this repo are the same release + (`tests/dev-lead/unit/test_dev_lead_retry_channel.bats`). The order still matters for + target repos that pin an older channel than this one (#2086). - **Enforcement.** The dev-lead prompt guardrail (part C, #1254) stops the agent from introducing the skew at the source; the **Part A CI guard (#1253)** is the belt-and-braces check that fails a PR whose caller-stub `with:` forwards an input the pinned channel does not declare. diff --git a/tests/dev-lead/unit/test_dev_lead_retry_channel.bats b/tests/dev-lead/unit/test_dev_lead_retry_channel.bats index ab317ccd6..89016a7e6 100644 --- a/tests/dev-lead/unit/test_dev_lead_retry_channel.bats +++ b/tests/dev-lead/unit/test_dev_lead_retry_channel.bats @@ -25,6 +25,14 @@ setup() { export GITHUB_OUTPUT="$BATS_TEST_TMPDIR/github_output" : >"$GITHUB_OUTPUT" RESOLVE_SCRIPT="$BATS_TEST_TMPDIR/resolve.sh" + # yq extracts the resolve step: skip locally without it, but never silently in CI. + if ! command -v yq >/dev/null 2>&1; then + if [ -n "${CI:-}" ]; then + echo "yq is required in CI" >&2 + return 1 + fi + skip "yq not installed" + fi yq -r '.jobs.retry.steps[] | select(.id == "channel") | .run' "$WF" >"$RESOLVE_SCRIPT" } @@ -75,7 +83,7 @@ EOF @test "workflow does not hard-code a dev-lead major version" { run grep -nE 'dev-lead/v[0-9]+' "$WF" - [ "$status" -ne 0 ] + [ "$status" -eq 1 ] } @test "resolve step returns the live stub's agent_ref" { @@ -101,7 +109,7 @@ EOF local f="$BATS_TEST_TMPDIR/dev-lead.yml" printf 'jobs:\n dev-lead:\n uses: x\n' >"$f" STUB_FILE="$f" run bash "$RESOLVE_SCRIPT" - [ "$status" -ne 0 ] + [ "$status" -eq 1 ] [ ! -s "$GITHUB_OUTPUT" ] } @@ -110,7 +118,7 @@ EOF for bad in main dev-lead/v139 dev-lead/vX-stable dev-lead/v139-canary pr-review/v1-stable 'dev-lead/v139-stable;x'; do : >"$GITHUB_OUTPUT" STUB_FILE="$(_stub_fixture "$bad")" run bash "$RESOLVE_SCRIPT" - [ "$status" -ne 0 ] + [ "$status" -eq 1 ] [ ! -s "$GITHUB_OUTPUT" ] done }