fix(review-reviewers): give each matrix leg its own PR branch name - #858
Conversation
The workflow matrixes over five target repos and every leg shares one $GITHUB_RUN_ID, so the Step 5 recipe named the same branch for all of them. Two legs that both find something race one ref: one push is rejected, or worse, two unrelated concerns land on a single branch. Append the target repo name to the recipe.
tend-agent
left a comment
There was a problem hiding this comment.
The diagnosis checks out against the traces — #856 sits on hourly/review-31058673934 and #857 on hourly/review-31058673934-cargo-affected, exactly the rejected-push-then-hand-suffix recovery the description describes. The scoping claim holds too: tend-review-runs.yaml has no strategy.matrix, so daily/review-runs-$GITHUB_RUN_ID can't collide the same way.
One gap: the discriminator is per leg, but the same section sanctions up to two PRs from one leg ("Limit to at most 2 PRs per run"). A leg that finds two things computes one branch name for both, so the second gh pr create fails against the head branch the first already claimed — and if the second fix is committed before that failure surfaces, two unrelated concerns share a branch, which is the atomic-PR breakage this PR is closing. Suggestion inline adds a per-PR component so the recipe covers both axes.
…he whole matrix (#877) `review-reviewers` lost **8 h 15 m** of coverage today across all five target repos, and the cause was a two-line gate in its own workflow rather than anything in a skill. ## What happened A critical GitHub Actions incident ([status page](https://stspg.io/rcz3fcm83sff), created 2026-08-06T15:22:49Z, still `investigating`) made runner acquisition unreliable. Four consecutive `review-reviewers` ticks then failed identically — `init-tracking` sat unscheduled for ~15 minutes, GitHub marked it `cancelled`, and because the matrix job has a hard `needs: init-tracking`, all five legs were `skipped` without ever trying for a runner: | Run | Created | `init-tracking` | matrix | |---|---|---|---| | [31122877870](https://github.com/max-sixty/tend/actions/runs/31122877870) | 17:21:15Z | `cancelled` | `skipped` ×5 | | [31125649607](https://github.com/max-sixty/tend/actions/runs/31125649607) | 18:17:43Z | `cancelled` | `skipped` ×5 | | [31126978811](https://github.com/max-sixty/tend/actions/runs/31126978811) | 19:34:17Z | `cancelled` | `skipped` ×5 | | [31128259981](https://github.com/max-sixty/tend/actions/runs/31128259981) | 21:22:19Z | `cancelled` | `skipped` ×5 | That's 20 leg-analyses discarded. The legs were not themselves unschedulable: in [31119653960](https://github.com/max-sixty/tend/actions/runs/31119653960) (16:23Z) `init-tracking` did succeed, and matrix legs went on to acquire runners and run — `numbagg/numbagg` completed successfully at 16:34Z during the same incident. ## Why the gate isn't worth its cost `init-tracking` exists to win a find-or-create race that can only bite on the **first cron tick of a month** — before that tick, no tracking issue exists for the new month and N legs would each try to create one. The `review-reviewers` skill still carries its own find-or-create fallback, sorting lowest-numbered-first precisely so a lost race degrades to a duplicate rather than a crash. This change makes that path load-bearing on a first-tick failure rather than a pure safety net, so the skill's wording is updated in the same commit to say so. So the gate trades *up to N-1 duplicate tracking issues (four, at today's five legs), once a month* against *the entire analysis window for every target repo, every time the job can't get a runner*. This change keeps `needs:` — so `init-tracking` still runs first and still wins the race on every normal tick — and only relaxes the failure propagation. `!cancelled()` rather than `always()` is deliberate: a user cancelling the run should still stop the matrix. ## Gate assessment - **Evidence level: High** — 4 occurrences in a single window, all with byte-identical job shape. - **Structural, not stochastic.** A hard `needs:` gate always propagates; there is no decision point for the agent to get right. Confirmed against the last 14 `review-reviewers` failures: the `init=cancelled → matrix=skipped` shape appears **only** on today's four, while every 2026-08-05 failure ran `init=success` with the matrix reaching the agent. The gate is the amplifier; the incident is only the trigger. - **Change type: removal of a gate** — 2 functional lines. Under the magnitude gate a removal clears at 1 occurrence; this has 4. - **Not an incident workaround.** The skill's own rule is to record transient upstream incidents rather than commit code around them, and this run recorded the rest of the incident's damage without acting on it. This one is different in kind: it removes a permanent single point of failure that will amplify the *next* infrastructure hiccup the same way, and it stays correct after the incident resolves. - **Dedup**: no open or closed tend issue or PR mentions `init-tracking`. #845 (cadence) and #858 (per-leg branch names) touch this workflow but not this gate. Evidence log: https://gist.github.com/192514ea2c36586f9b7f842a482d62ab Found while analysing `PRQL/prql` in run [31132205674](https://github.com/max-sixty/tend/actions/runs/31132205674) — the run that had to absorb all four lost windows. --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
…a silent survey (#885) This hour's Step 2 survey subagent reported **"No visible bot output in time window"** across 23 successful runs on `max-sixty/tend`. The window in fact contained 9 reviews (two `APPROVED`), 11 inline review comments, and 1 conversation comment from `tend-agent` — one of the busiest windows in the series. ## Evidence Window 2026-08-07T07:27Z → 08:16Z. The survey's verdict, verbatim: > **No visible bot output in time window.** All 23 successful runs (tend-mention, tend-review, tend-review-runs, tend-notifications) executed and completed but produced zero comments, reviews, or inline code comments between 2026-08-07T07:27:00Z and 08:16:00Z on tracked PRs/issues (809, 818, 834, 858, 863, 868, 875, 877, 878, 881, 816, 830). Two run-independent calls, taking no run ID at all, contradict it immediately: ``` gh api "repos/max-sixty/tend/issues/comments?since=2026-08-07T07:27:00Z&per_page=100" → 1 tend-agent row gh api "repos/max-sixty/tend/pulls/comments?since=2026-08-07T07:27:00Z&per_page=100" → 11 tend-agent rows ``` Plus reviews `4880965990` (COMMENTED), `4881043005` (COMMENTED) and `4881079314` (APPROVED) on [#881](#881), `4881113428` (APPROVED) on [#878](#878), and five more (all COMMENTED) on [#809](#809). The survey had listed 881, 878 and 809 among the PRs it checked, so the numbers were right and the reads came back empty anyway. The second-order cost is worse than the omission. Having established silence, the survey reasoned *from* it: it flagged the window's seven bot-PR merges as "merged by max-sixty without formal review workflow", concluding "**direct push/merge bypassing review requirement**, or **review workflow override via branch protection rule bypass**". Six of the seven (#818, #834, #858, #868, #875, #877) carry bot `COMMENTED` reviews predating the window, which is why a `since`-filtered read missed them; the seventh (#863) is the skill-authorized silence on a self-authored PR with no concerns. None reads `APPROVED` because GitHub blocks self-approval — the ordinary shape for a bot PR, not a bypassed control. Acted on, that summary is a false security finding against the maintainer. ## Root cause Every path Step 2 offers is run-keyed: run → `headBranch` → PR → endpoint. That chain is fine when it works, but it has one failure mode with no floor — break it anywhere and *every* run returns empty simultaneously. Uniform absence is exactly what a genuinely quiet hour looks like, so the summary that comes back is self-consistent and carries no signal that anything went wrong. The existing sanity-check line ("note if zero bot activity found across all runs") did fire here, and the subagent talked itself out of it in the same paragraph — a prompt to notice absence can't distinguish the two causes, because nothing in a run-keyed survey can. ## Change Adds a sweep block to the top of the Step 2 prompt that takes no run ID — the two `?since=` comment endpoints, bounded on `created_at` at both ends; a `pr list --search "updated:>"` for the candidate list the review queries need; and a loop over those candidates counting bot reviews submitted inside the window, since neither comment endpoint returns review submissions and an empty-body `APPROVE` is `tend-review`'s most common output — with instruction to report all four counts and to re-map from what they found rather than reporting those runs silent. Adds one sentence at the main-agent review point: an all-quiet report without the counts isn't usable, and absence isn't a finding to reason from. This is the check that caught the failure this run. It is four counts off two run-independent endpoints and one search, and it fails independently of the mapping it is checking. ## Relation to the other open Step 2 PRs Distinct problems, non-overlapping edits. [#864](#864) fixes *who* accepted (named non-bot actor); [#869](#869) fixes *which run* produced an output (confirm from the posting run's log). Both still start from a candidate PR list reached by run-keyed mapping — neither makes "no output at all" falsifiable, which is the failure here. ## Gate assessment - **Evidence level**: High — survey unreliability is recorded in the evidence gist across prior windows, cumulative **4 → 5** with this one. High needs 2–3. Prior occurrences were omissions of individual runs and one mislabelled silence; this is the first categorical zero-output claim, and the first to produce a fabricated inference from the absence. - **Structural**: the *specific* empty read is stochastic, but the skill's exposure is not — Step 2 offers only run-keyed paths, so any mapping break yields a plausible, uniform, unfalsifiable silence. Replay it and the summary is equally convincing every time. - **Change type**: targeted fix (one query block, one sentence) — normal bar, met. - **Passes both gates.** Evidence: https://gist.github.com/e08f6e62d6478163cb425a75648eb7e4 --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
…nst tend before filing upstream (#891) ## Problem Two dedup blocks were blind in two different ways, and the cited duplicate needed both fixed. **State filter.** `review-runs` Step 5 and `review-reviewers` Step 4 both deduped against PRs with `gh pr list --state open`. A merged PR is never returned by that query, so a finding whose fix already landed reads as undeduped and gets filed again. `running-in-ci`'s PR-creation dedup recheck already gets this right ("with `--state all` so closed and merged siblings show up"); these two recipes contradicted it. **Repo scope.** `review-runs` is a generated workflow ([`generator/src/tend/config.py:24`](https://github.com/max-sixty/tend/blob/f65f49f/generator/src/tend/config.py#L24) lists it in the enabled set), so it runs in each adopter's checkout and an unqualified `gh pr list` returns *the adopter's* PRs. Step 6 routes bundled-skill defects upstream to tend, but neither Step 5 nor any of `running-in-ci`'s dedup recipes — all local-repo — told the agent to dedup in the target repo before filing there. `--state all` alone does not close this: the adopter's PR list never contained the upstream fix at any state. `review-reviewers` is unaffected by the second half. It runs in `max-sixty/tend` and files onto tend, so its unqualified `gh pr list` already resolves to the right repo; only the state filter was wrong there. This bites hardest on tend specifically, because of the pinning model: adopters call `max-sixty/tend/<harness>@X.Y.Z`, so a merged skill fix stays dormant on their repos until the next release tags. The bug keeps reproducing after the fix merges — which is exactly the window in which the analysis legs are looking at it, and exactly when the dedup queries are blind to the fix. ## What happened `max-sixty/cargo-affected`'s `tend-review-runs` run [31160677649](https://github.com/max-sixty/cargo-affected/actions/runs/31160677649) (08:11:33Z → 08:21:41Z) hit the `| last` evidence-log mis-selection: it appended ~12 KB of run evidence into the nightly's unrelated comment on target [#73](max-sixty/cargo-affected#73), noticed on its post-verify read, restored comment `5188771252`, and re-appended to the real log `5150650688`. Good recovery. It then filed [#883](#883) upstream, whose "Proposed fix" is a `## Run ` heading predicate on the comment selector. [#875](#875) merged that exact fix at 07:34:40Z — 46 minutes before the issue was filed — as `test("^## Run [0-9]")` on the same selector, in the same file. #883 is a duplicate of a merged PR. The run made three dedup queries before filing (`gh issue list --state all --search "tracking issue comment append"`, a broader `gh issue list --state all` title regex, and a final `gh issue list --state open` recheck). All three were `gh issue list`, which never returns PRs — and all three ran against `max-sixty/cargo-affected`. Even had it run Step 5's PR line verbatim, it would not have returned #875, for both reasons: the state filter excluded merged PRs, and the query's repo was the adopter's, not tend's. ## The fix - Both skills: `gh pr list --state open` → `--state all`, projecting `state,mergedAt` so a merged hit is legible. - `review-runs` only: add the cross-repo pair (`gh pr list`/`gh issue list --repo max-sixty/tend --state all`) so a finding heading upstream under Step 6 is deduped against tend first. - `review-runs` only: the pinning note is scoped to the upstream repo, since in that skill the reader is the adopter and the local `gh pr list` above it has nothing to do with pinned refs. `review-reviewers` keeps the original wording, where tend is the reader and "on adopters" is the correct direction. Both added commands were run against this repo to confirm they parse and return the expected shape. ## Gate assessment - **Evidence level**: High. **Occurrences: 1** direct, verified end to end (session log, both dedup query sets, #875's merge time and diff, #883's body). - **Structural, not stochastic.** `gh pr list --state open` deterministically cannot return a merged PR, and a query scoped to the adopter's repo deterministically cannot return a tend PR; replayed ten times it misses #875 ten times. There is no decision point. - **Change type**: targeted fix — query lines plus one sentence naming the pinning consequence. It brings both recipes into line with a rule the same plugin already states in `running-in-ci`, rather than introducing new policy. - **Why act at one occurrence**: the six-PR batch merged at 07:34:40Z ([#875](#875), [#834](#834), [#868](#868), [#818](#818), [#877](#877), [#858](#858)) is all unreleased, so six distinct bugs remain observable on every adopter until the next release. The first analysis leg after that batch produced the first duplicate. The exposure is six-wide and standing, not one-off. ## Not covered by No open PR touches either dedup block. [#845](https://github.com/max-sixty/tend/pull/845)/[#850](https://github.com/max-sixty/tend/pull/850)/[#838](https://github.com/max-sixty/tend/pull/838) are the run-window cluster; [#849](#849), [#856](#856), [#857](#857), [#864](#864), [#869](#869), [#876](#876), [#809](#809), [#837](#837), [#821](#821), [#836](#836) are elsewhere. #883 stays open as a maintainer call — it is redundant with #875 but the option-2 half of its body (finish the gist migration for `review-runs`) is not. Evidence: https://gist.github.com/dca23a6e6a0d8cae2665944ba31676fb --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Problem
review-reviewersmatrixes over five target repos (review-reviewers.yaml), and all five legs of a tick share one$GITHUB_RUN_ID. Step 5 of the skill hands every leg the same branch recipe:So the moment two legs of the same tick both find something worth a PR, they contend for one ref. The loser's
git pushis rejected as a non-fast-forward and it has to recover mid-session; the ordering that hurts more is the other one, where the second leg pushes before the first opens its PR and the two unrelated concerns land on a single branch — the atomic-PR rule broken by the branch name, not by any judgement the agent made.This just happened on run 31058673934: the
max-sixty/cargo-affectedleg's push tohourly/review-31058673934was rejected against a sibling leg's commit, which by then was #856. It recovered by suffixing the target name (#857), and an earlier tick shows the same improvisation baked intohourly/review-30962483562-numbagg— legs have been working around the recipe rather than following it.The run ID is non-unique along a second axis too: the same section caps a leg at two PRs (
**Limit to at most 2 PRs per run**), and a leg that acts on two findings computes one name for both — same collision, one job instead of two.Fix
Put both discriminators in the recipe:
hourly/review-$GITHUB_RUN_ID-<target-repo-name>-<topic-slug>. The target name is unique across the matrix by construction, the topic slug separates a single leg's two PRs, and the branch stays greppable back to its run.Scoped to this skill.
review-runscarries the same$GITHUB_RUN_ID-only shape, buttend-review-runs.yamlhas no matrix, so a run there is a single job and the name can't collide — no change needed and none made.Overlaps #845 on this line. That PR rewrites the same recipe to rename the prefix (
hourly/→review-reviewers/) as part of its cadence change, and keeps$GITHUB_RUN_IDas the whole discriminator — so it ships the collision under a new name. The two are orthogonal in substance and conflict only textually: whichever lands second wants<prefix>/review-$GITHUB_RUN_ID-<target-repo-name>-<topic-slug>, taking the prefix from #845 and the suffixes from here.Gate assessment
Evidence log: https://gist.github.com/dca23a6e6a0d8cae2665944ba31676fb