fix(dev-lead-retry): CI-retry payloads carry the check run id and URL again

Merged
DO
2d ago
+147-12

Problem

lookup_check_run_details in scripts/dev-lead-retry.sh passed --arg name "$check_name" to gh api --jq. gh api has no --arg: it reads --arg as the jq filter and then rejects the call:

$ gh api repos/x/y/commits/abc/check-runs --jq --arg name CI '.check_runs'
accepts 1 arg(s), received 4

The 2>/dev/null hid that, and the fallback {"id":"","details_url":""} always won. So every dev-lead-ci-failure dispatch from the retry sweep has sent an empty details_url and a null check run id, and fix-ci.sh could not fetch the failed run's logs or annotations. Found while fixing #2462 (recorded in its body); the bug predates it.

Fix

  • Fetch the check runs with gh api --paginate (a commit can carry more than 100) and filter all pages with jq -s --arg. The check name is data, never filter text, so names with quotes, backslashes or \(...) match literally.
  • An API error body still yields the fallback object: it has no .check_runs, so jq fails and the output is replaced.
  • When no failed run is found, or the check runs cannot be read, it prints one bounded ::warning:: line on stderr naming the repo, head SHA and check. Stdout still carries only the JSON object.

Why only failure conclusions

The lookup selects conclusion == "failure" on purpose. Every CI retry this sweep sends started as a failure:

  • The original dev-lead-ci-failure dispatch comes from the ci-relay job in .github/workflows/dev-lead-reusable.yml, which runs only when github.event.check_run.conclusion == 'failure'.
  • The sweep re-dispatches only from a fix-ci status=rate-limited ... check=<name> marker. That marker's check name comes from the payload of that original dispatch (CHECKS_JSON[0].name in dev-lead-fix-ci.sh).
  • dispatch_ci_retry itself sends conclusion: "failure".

timed_out, cancelled and action_required runs are never relayed, so they are never retried here.

Risk

Low code risk: the change is confined to one lookup helper and its output shape is unchanged. The behavioural risk is that this turns on a path that has effectively never run. Until now, every retry payload was empty, so fix-ci.sh always fell back to the PR diff. After merge, retries carry a real check run id and details_url, so fix-ci will fetch the failed run's logs and annotations. That content comes from CI output (untrusted) and now reaches the agent on retries, as it already does on first dispatches from ci-relay.

Known limitation (pre-existing, not changed here): the sweep reads the check name from the marker with check=([^\s"<>]+), so a check name containing a space is truncated and will not match. The new warning makes those cases visible; widening that capture is a follow-up.

Test plan

tests/dev-lead/unit/test_dev_lead_retry.bats gains _strict_gh_stub, a gh stub that parses arguments the way the real gh does (value flags take the next word; exactly one positional, else accepts 1 arg(s), received N; later pages only with --paginate). Cases:

  • Latest failed run of the named check is returned (successes and other checks ignored), with no warning.
  • A failure listed only on a later page is found (fails if --paginate is removed).
  • Name with quotes, backslashes and \(...) matches literally.
  • No failed run of the check gives exactly {"id":"","details_url":""} and one warning naming the check.
  • An API error body gives the fallback object and a warning naming repo, SHA and check.
  • A hostile name (newline, ::error::, %0A, 300 chars) stays one bounded warning line.
  • dispatch_ci_retry sends checks[0].id and checks[0].details_url from the run.

The original three cases fail on main. Locally: tests/dev-lead/unit/ passes in full (2698 tests, bash 5.3). shellcheck --severity=warning -x, check-model-pins.sh, check-retry-payload-fields.sh, check-bats-negation.sh, check-duplicate-decls.sh and check-fixed-tmp-paths.sh are clean.

Rollback

Revert this PR. The script goes back to sending empty details_url / null id, which fix-ci already handles by falling back to the PR diff. No state, markers or payload schema change.

Monitoring

After merge, watch the first few dev-lead-ci-failure dispatches from the retry sweep (dev-lead-retry workflow runs) and the fix-ci runs they start in the target repos:

  • The dispatch payload should carry a non-null checks[0].id and a details_url.
  • fix-ci should fetch that run's logs and annotations instead of falling back to the diff.
  • Since this brings untrusted CI log content to the agent on retries for the first time, check that those runs stay within the usual fix-ci scope.
  • ::warning::dev-lead-retry: no failed check run ... lines in sweep runs mark retries that still went out without context (for example, check names with spaces).

šŸ¤– Generated with Claude Code