Skip to content

fix(running-in-ci): batch pushes so the author doesn't cancel its own reviewer - #868

Merged
max-sixty merged 2 commits into
mainfrom
hourly/review-31085980872
Aug 7, 2026
Merged

fix(running-in-ci): batch pushes so the author doesn't cancel its own reviewer#868
max-sixty merged 2 commits into
mainfrom
hourly/review-31085980872

Conversation

@tend-agent

Copy link
Copy Markdown
Collaborator

Problem

tend-review triggers on pull_request_target: synchronize under a per-PR concurrency group at cancel-in-progress: true (review.yaml.j2). Every push to a bot-authored PR therefore starts a replacement review run and cancels the one already going. Nothing in the skills tells the author side to batch its pushes, so a session that opens a PR and then pushes a follow-up commit thirty seconds later spawns a reviewer purely to kill it.

On PRQL/prql in the 07:44Z–08:44Z window, 5 of the 8 tend-review runs on bot-authored PRs were cancelled, and 4 of those posted nothing at all. Each cancellation traces to the same bot pushing again while its own reviewer was running.

PR head review run started ended lifetime review posted
#6149 43b1738a 31082377982 07:49:05 cancelled 07:49:45 40s no
#6150 1979e288 31084139419 08:15:14 cancelled 08:15:47 33s no
#6150 ce92ec84 31084169295 08:15:39 cancelled 08:15:53 14s no
#6150 270aa2b1 31085384920 08:33:29 cancelled 08:35:43 2m14s no
#6147 94ceeca5 31081526145 07:35:42 cancelled 07:51:42 16m00s no

The sharpest case is #6150. The session created the PR at 08:15:09, pushed ce92ec84 (chore: changelog entry) ~28s later, then force-pushed it as eaf11545 ~12s after that — gh api repos/PRQL/prql/compare/ce92ec84...88867eeb returns status: diverged, confirming the rewrite. Three review runs fired inside 42 seconds; two died before doing any work. The changelog entry had no reason not to be in the initial push.

The 16-minute loss on #6147 is the same rule from the other end: run 31081526145 had been reviewing for 16 minutes when the author session pushed 0efb4aea, and its analysis went with it.

Solution

A new Batch the push subsection under running-in-ci's "Pushing to PR Branches", stating the mechanism once and two rules: commit everything before gh pr create, and make the commits then push once rather than pushing after each. It notes explicitly that amends and rebases fire synchronize too, since the #6150 case went through a force-push. A closing line keeps the legitimate case intact — a push that acts on review feedback should invalidate the running review; the ask is to land it as one push, not several.

running-in-ci is the right home because every session that pushes reads it, and the sessions producing this were tend-mention handle legs rather than any one workflow's skill.

Relationship to existing work

This is the author-side counterpart to #830 / #834, not a duplicate. Those cover a review session cancelling itself by pushing a fix before submitting its review; the ordering rule there protects the pusher. Here the pusher is a different session entirely, and the victim is a reviewer that has no way to protect itself — so the rule has to live on the author side. The two compose: #834 makes the reviewer push last, this makes the author push once.

Gate assessment

  • Evidence level: High — a consistent pattern across multiple independent sessions (the #6149 handle leg, the #6150 handle leg, and the #6147 author session), not a single lapse. High requires 2–3 occurrences; this window alone has 4 cancelled-with-nothing-posted runs, and the evidence gist carries one prior recorded occurrence (tend-nightly pushes to its own just-opened PR, cancelling that PR's first review mid-flight — recorded at cumulative 1 with "act at 3+"). Cumulative 5.
  • Structural vs stochastic: structural in its consequence. Given two pushes within a minute, the first review run is cancelled every time — there is no decision point in the cancellation itself, only in whether the author batches. Replaying the scenario 10 times cancels 10 times.
  • Change type: targeted fix — a missing step added to an existing section, no new section and no workflow change. Normal bar, met.
  • Passes both gates: yes.

Testing

Skill prose, so no test to add. The structural claim was verified against the generator template rather than assumed: review.yaml.j2 shows types: [opened, synchronize, ready_for_review, reopened] with group: ${{ github.workflow }}-${{ github.event.pull_request.number }} and cancel-in-progress: true. Every run ID, head SHA, and timestamp in the table above comes from gh api repos/PRQL/prql/actions/runs/<id>; the review-posted column was checked against gh api repos/PRQL/prql/pulls/<n>/reviews. Checked the co-loaded skills before adding guidance — review/SKILL.md line 370 discusses cancelled check runs, not the review session, and no skill carried an author-side batching rule.

Evidence log: https://gist.github.com/192514ea2c36586f9b7f842a482d62ab

… reviewer

Every push to a bot-authored PR fires synchronize under a
cancel-in-progress concurrency group, so a session that pushes twice in
quick succession kills a tend-review run at startup. Add a Batch the
push subsection: commit everything before gh pr create, and push once
after making the commits.
@tend-agent tend-agent added the claude-behavior Behavioral issues found by review-reviewers label Aug 6, 2026

@tend-agent tend-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mechanism checks out against the template — review.yaml.j2 has types: [opened, synchronize, ...] with a per-PR group at cancel-in-progress: true, so the causal chain in the body is right and the rule follows from it. Two wording points below.

Comment thread plugins/tend-ci-runner/skills/running-in-ci/SKILL.md Outdated
Comment thread plugins/tend-ci-runner/skills/running-in-ci/SKILL.md Outdated
@max-sixty
max-sixty merged commit 70e0271 into main Aug 7, 2026
11 checks passed
@max-sixty
max-sixty deleted the hourly/review-31085980872 branch August 7, 2026 07:34
max-sixty pushed a commit that referenced this pull request Aug 7, 2026
…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>
max-sixty pushed a commit that referenced this pull request Aug 7, 2026
…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>
max-sixty pushed a commit that referenced this pull request Aug 11, 2026
#870)

## Problem

When a session opens a PR and then stays alive polling its CI,
`tend-review` fires on that PR and posts findings — and two actors race
to fix them: the still-live session that opened the PR, and the
`tend-mention` session dispatched by the `pull_request_review` event.
Both make the same edits and run the same suite; the loser discovers the
duplication at `git push`, discards its commit, and a full
fix-and-verify cycle is paid twice. No wrong output reaches the repo —
every existing dedup guard fires — but every guard in `running-in-ci` is
anchored at the *output* boundary (`gh pr create`, `git push`, comment
posting), so the collision is only ever discovered after the cost is
sunk.

## Solution

Two rules, one per commit — the first removes the common case, the
second bounds what the first can't reach.

### 1. A role boundary: a review that lands mid-poll isn't yours to
action

Per the second of the two options in #865. The API-based sibling-run
check in the first option can't see which branch a nightly run is
working on — that's only visible from its checkout — so it can't close
the nightly-vs-mention case it was aimed at.

The rule goes in `running-in-ci`'s **CI Monitoring** section rather than
the nightly skill: the mechanism is generic to any session that opens a
PR and stays alive polling it (nightly, ci-fix, triage, weekly, the
hourly review skills), and CI Monitoring is where that "stay alive and
poll" instruction lives. The nightly skill's step 8 gets a one-line
pointer to it, since its "poll CI" step named no section to read.

The session that stands down has to be the one holding the PR: the
mention dispatch fires whether or not the holder also responds, so it is
the run that cannot be suppressed.

### 2. A cost bound: re-check the head SHA before the expensive verify

The role boundary cuts on *who owns the review*, and a [recurrence on
`max-sixty/worktrunk`](#865 (comment))
shows work colliding without the review being read at all. There,
nightly never fetched the review; `codecov/patch` — a required gate on
that repo — flagged the same new closure the reviewer had, so nightly
added the missing test as the follow-up it was gated on
([`957358b46`](max-sixty/worktrunk@957358b),
06:56) while the mention run independently built a near-identical one.
The convergence isn't luck: a reviewer and a coverage tool reading the
same new code ask for the same missing test. Rule 1 permits this by
construction, since closing a red required gate is the carve-out it
keeps.

So the second commit puts a check on the *writing* side, where the cost
is paid: record `headRefOid` before editing, re-check it immediately
before each expensive step (full suite, coverage/snapshot regeneration,
long build) rather than only at `git push`. On that occurrence it would
have caught the sibling push at ~06:56 instead of ~07:01 — before the
integration suite and the post-reset re-verify, which is where most of
the wasted ~$2.14 went.

It also records why the runs-API approach doesn't work, so it isn't
re-derived: both writing runs report `head_branch: main` (nightly runs
on `schedule`, the mention handler on `repository_dispatch`), so a live
sibling is invisible there. Only the `pull_request_target` review run
carries the PR branch — confirmed against runs
[31465475753](https://github.com/max-sixty/worktrunk/actions/runs/31465475753),
[31466280614](https://github.com/max-sixty/worktrunk/actions/runs/31466280614),
and
[31466624503](https://github.com/max-sixty/worktrunk/actions/runs/31466624503).

The other shape the report floats — making a PR-holding session
read-only after `gh pr create` — isn't taken here: that session is the
only one gated on the PR's checks, and no dispatch fires for a red gate,
so a red `codecov/patch` with no accompanying review would sit unfixed.

## What the handoff rests on — and what doesn't back it

`tend-mention`'s `verify` job short-circuits to `should_run=true` on
`PR_AUTHOR == bot_name`, so every review on a bot-authored same-repo PR
reaches the handle job, and the prompt for `client_payload.kind ==
'pull_request_review'` instructs the session to make requested changes
and push. Nightly-opened PRs are same-repo, so the fork exclusion on the
relay job doesn't apply to them.

**The handoff is unbacked — the dispatch is the only thing carrying the
review, and if it's lost nothing recovers it.** GitHub raises no
notification for an actor's own activity, so a `tend-agent` review on a
`tend-agent`-authored PR creates no notification thread at all, and the
notifications poll's "stale unanswered items" path has nothing to find.
And the dispatch can be lost silently: `tend-mention-handle-<PR#>` runs
at `cancel-in-progress: false`, and per CLAUDE.md's "GHA queue depth =
1" note a third job arriving replaces the queued one — on a PR drawing
several events, the review's handle job is exactly the one that gets
replaced. Before this change the still-live session was the fallback;
after it, the only residual is "name the outstanding review in your
summary", which lands in the session log rather than anywhere a
maintainer sees.

That is the accepted cost rather than an oversight: the duplicated
fix-and-verify cycle is the common outcome and a dropped dispatch is
rare, so paying the duplication on every review to insure against it is
the worse deal. But the property a future reader will lean on is that
the handoff has no second line of defence.

<details><summary>How the no-self-notification behaviour was
checked</summary>

`gh api "notifications?all=true&per_page=100"` returns the newest 50
threads, `updated_at` desc. The newest row is `2026-08-06T04:23:44Z`.

Absent entirely — each bot-authored, each carrying bot self-reviews
*newer* than that newest row, so they would sort first if a thread
existed:

- this PR (#870) — opened 08:56, self-review 09:02:30
- #868 — opened 08:52, self-reviews 08:57:13 / 08:59:40 / 08:59:47

Present, but only keyed to another actor's action:

- #843 — self-reviews 07:19 and 07:23 on 08-05; thread `updated_at`
`16:43:23`, ~20s after the maintainer's merge at `16:43:03`
- #840 — self-reviews 01:02–01:05; thread `updated_at` `16:44:47`, after
the merge at `16:44:26`

</details>

## Testing

None — the change is skill prose, and no test covers skill markdown.
Verified against the mention template's `verify` logic and prompt text
(`generator/src/tend/templates/mention.yaml.j2`) that the run being
handed to actually receives and actions these reviews, and against the
notifications API that no self-activity thread exists to back that
dispatch up. The second commit's premises were checked against the
public record on `max-sixty/worktrunk`: the commit order and timing on
#3799, and the `head_branch` each of the three runs reports.

---

Closes #865 — automated triage

---------

Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-behavior Behavioral issues found by review-reviewers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants