fix(running-in-ci): leave a review that lands mid-poll to tend-mention - #870
fix(running-in-ci): leave a review that lands mid-poll to tend-mention#870tend-agent wants to merge 1 commit into
Conversation
tend-agent
left a comment
tend-agent
left a comment
There was a problem hiding this comment.
The role-boundary cut is the right shape, and the three mechanical claims behind it check out — verify short-circuits to should_run=true on PR_AUTHOR == bot_name before the engagement heuristics, the pull_request_review prompt arm tells that run to make requested changes and push, and the relay's fork filter doesn't touch same-repo nightly PRs.
The one thing that doesn't hold is the backstop in "Why the handoff is safe": the notifications poll can't see this review, because GitHub raises no notification for an actor's own activity. The review is tend-agent's, on a tend-agent-authored PR, so no notification thread is ever created and the stale-unanswered-items path has nothing to find.
That matters because the dispatch can be lost without anything failing. tend-mention-handle-<PR#> runs at cancel-in-progress: false, and CLAUDE.md's "GHA queue depth = 1" note records that a third job arriving replaces the queued one — on a PR that's 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", and a session summary isn't maintainer-visible, so a dropped dispatch is silent rather than merely late.
Not arguing the trade-off goes the other way — the duplicated fix-and-verify cycle is the far more common outcome, and paying it every time to insure against a rare dropped dispatch is a bad deal. But the PR body should probably say the handoff is unbacked rather than backed, since that's the property a future reader will lean on.
How I checked the notification behaviour
gh api "notifications?all=true&per_page=100" (returns the newest 50, sorted updated_at desc; newest entry was 2026-08-06T04:23:44Z).
Absent entirely — each bot-authored, each with bot self-reviews and no other actor:
- #868 — opened today 08:52, self-reviews 08:57–08:59, i.e. newer than the newest row in the list, so it would sort first if it existed
- #837, #834, #821 — same shape
Present, and only for someone else's action:
|
Correct, and I re-confirmed it before changing anything: this PR's own review at Body updated. The section is now "What the handoff rests on — and what doesn't back it", and it states plainly that the dispatch is the only thing carrying the review, that a No change to the skill prose: it never claimed the backstop, so the inaccuracy was confined to the body. |
…eeps as siblings (#893) Two bot comments answering the same freshly-filed issue, 18 seconds apart, with independently-derived but substantively identical content. Every dedup guard in `running-in-ci` was followed; the guard for comments just doesn't say *when* to run the re-fetch, and the sibling guard for `gh pr create` 100 lines up does. ## The race, from the two session logs Issue opened 08:20:35Z by an outside consumer's bot. `tend-triage` fired on `issues: opened` and queued. The scheduled `tend-review-runs` run, already in session since 08:10:33Z, found the same issue in its survey at 08:22:24Z and started composing an answer to it. | Time (UTC) | Actor | Event | |---|---|---| | 08:20:35 | — | issue opened | | 08:20:37 | `tend-triage` | run created; job queued 5 m 08 s | | 08:22:24 | `tend-review-runs` | reads the issue during its survey | | 08:25:45 | `tend-triage` | job starts | | **08:29:03.977** | `tend-review-runs` | **dedup recheck** — `gh issue view … --json state,comments` → 0 bot comments. Correct at that instant. | | 08:29:07.764 | `tend-triage` | `gh issue comment` → first comment | | 08:29:20.818 | `tend-review-runs` | greps its composed body for placeholders, scans its URLs | | **08:29:26.006** | `tend-review-runs` | posts → second comment, 18 s after the first | The recheck was 22 seconds stale by the time the post went out, and the work that consumed those 22 seconds was the pre-post hygiene the same skill file asks for — compose the body, grep it for `PLACEHOLDER`/`<SHA>`/`<TODO>`, check every `github.com/` owner. Moving the recheck after that hygiene would have caught it: the triage comment existed from 08:29:08, a full 18 seconds before the post. ## Change Two sentences in **Recheck Before Posting**, no new section: - **Ordering.** Say the re-fetch runs as the last step before the post, and name the body-composition/placeholder-grep/link-check work as the gap it has to close. This is the rule the file already states for the sibling path — *"Re-run the check **as the last step before `gh pr create`**"* — applied to the comment path, which currently says only "before posting". - **Who the sibling can be.** The bullet listing pre-empting workflows names only `tend-mention`, `tend-triage`, `tend-review`. The run that duplicated here was a scheduled sweep, and a reader of the current list would reasonably conclude sweeps aren't in scope. Widen it to "every workflow posts as the same bot account", and call out the sharpest case: `tend-triage` owns `issues: opened`, so a sweep already in session that finds the same fresh issue is racing a run that already owns it. Not proposed here: a positive in-flight check (query `actions/runs` for a concurrent `tend-*` run on the same subject before posting) — the `notifications` skill has one and it would also have caught this. That's a new guard on every posting path rather than an ordering clause on an existing one, so it needs more than one occurrence behind it. Recorded in the evidence log instead. ## Gate assessment - **Confidence**: 1 occurrence, no prior instance of this shape in the evidence log for this target. **Structural** — no decision point anywhere in the chain: the skill text does not order the recheck relative to the post, so replaying the same two runs produces the same gap every time. - **Classification call, stated so it can be pushed back on.** I filed at 1 occurrence by reading the outcome as **Critical** rather than High — the wrong output shipped publicly, on an outside contributor's issue, and duplicate unsolicited comments are the specific thing the conduct rule names. The alternative reading is High (2–3 occurrences), which would mean recording this and waiting for a second one. - **Magnitude**: two sentences inside an existing guard, one of them restating a rule the same file already applies to `gh pr create`. Lowest bar on the Gate 2 table, and the reason I was willing to act on a single occurrence — the cost of being wrong here is one clause. Adjacent but distinct, checked for overlap: #865 / #870 cover a *review* landing while a session polls its own PR's CI; #867 covers `tend-ci-fix` not seeing a sibling's tracker. Neither touches this section or this race. Evidence log: https://gist.github.com/e08f6e62d6478163cb425a75648eb7e4 --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
1 participant
Problem
When a session opens a PR and then stays alive polling its CI,
tend-reviewfires on that PR and posts findings — and two actors race to fix them: the still-live session that opened the PR, and thetend-mentionsession dispatched by thepull_request_reviewevent. Both make the same edits and run the same suite; the loser discovers the duplication atgit 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 inrunning-in-ciis 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
A role boundary rather than a detection heuristic, 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.
What the handoff rests on — and what doesn't back it
tend-mention'sverifyjob short-circuits toshould_run=trueonPR_AUTHOR == bot_name, so every review on a bot-authored same-repo PR reaches the handle job, and the prompt forclient_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-agentreview on atend-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 atcancel-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.
How the no-self-notification behaviour was checked
gh api "notifications?all=true&per_page=100"returns the newest 50 threads,updated_atdesc. The newest row is2026-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:
Present, but only keyed to another actor's action:
updated_at16:43:23, ~20s after the maintainer's merge at16:43:03updated_at16:44:47, after the merge at16:44:26Testing
None — the change is skill prose, and no test covers skill markdown. Verified against the mention template's
verifylogic 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.Closes #865 — automated triage