fix(review-runs,review-reviewers): dedup against merged PRs, and against tend before filing upstream - #891
Conversation
tend-agent
left a comment
There was a problem hiding this comment.
The review-reviewers half is right — that skill runs in max-sixty/tend with tend as gh's default repo, so gh pr list --state all does reach #875.
The review-runs half doesn't fix the case in the PR body. review-runs is a generated workflow (generator/src/tend/config.py lists it in the enabled set), so it runs in each adopter's checkout — a bare gh pr list there returns the adopter's PRs. The cited duplicate was max-sixty/tend#883 filed from a max-sixty/cargo-affected run, duplicating max-sixty/tend#875. Changing --state open to --state all doesn't help: cargo-affected's PR list never contained #875 at any state. So the body's "Even had it run Step 5's PR line verbatim, --state open would not have returned #875" holds for a reason the patch doesn't address — the repo, not the state filter.
Step 6 routes bundled-skill defects upstream (file the fix against tend per **Filing Issues in Other Repos**), but nothing in Step 5 — or in running-in-ci's dedup recipes, which are all local-repo — tells the agent to dedup in the target repo before filing there. That's the actual gap that produced #883.
Second, smaller: the new paragraph is written from tend's vantage point, but in review-runs the reader is the adopter, and the local gh pr list it follows has nothing to do with pinned refs. Suggested reword below scopes it to the upstream repo, which is where the pinning argument actually bites. The review-reviewers copy is fine as-is.
…cts upstream review-runs is a generated workflow, so its dedup queries run against the adopter's own repo and can never see an upstream tend PR. Add the cross-repo pair, and scope the pinning note to the upstream repo where it applies.
Problem
Two dedup blocks were blind in two different ways, and the cited duplicate needed both fixed.
State filter.
review-runsStep 5 andreview-reviewersStep 4 both deduped against PRs withgh 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 allso closed and merged siblings show up"); these two recipes contradicted it.Repo scope.
review-runsis a generated workflow (generator/src/tend/config.py:24lists it in the enabled set), so it runs in each adopter's checkout and an unqualifiedgh pr listreturns the adopter's PRs. Step 6 routes bundled-skill defects upstream to tend, but neither Step 5 nor any ofrunning-in-ci's dedup recipes — all local-repo — told the agent to dedup in the target repo before filing there.--state allalone does not close this: the adopter's PR list never contained the upstream fix at any state.review-reviewersis unaffected by the second half. It runs inmax-sixty/tendand files onto tend, so its unqualifiedgh pr listalready 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'stend-review-runsrun 31160677649 (08:11:33Z → 08:21:41Z) hit the| lastevidence-log mis-selection: it appended ~12 KB of run evidence into the nightly's unrelated comment on target #73, noticed on its post-verify read, restored comment5188771252, and re-appended to the real log5150650688. Good recovery. It then filed #883 upstream, whose "Proposed fix" is a## Runheading predicate on the comment selector.#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 broadergh issue list --state alltitle regex, and a finalgh issue list --state openrecheck). All three weregh issue list, which never returns PRs — and all three ran againstmax-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
gh pr list --state open→--state all, projectingstate,mergedAtso a merged hit is legible.review-runsonly: 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-runsonly: the pinning note is scoped to the upstream repo, since in that skill the reader is the adopter and the localgh pr listabove it has nothing to do with pinned refs.review-reviewerskeeps 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
| last, so any other bot comment on the tracker forks the log #883's body).gh pr list --state opendeterministically 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 fix(review-runs): find the evidence log by its heading, not by recency #875 ten times. There is no decision point.running-in-ci, rather than introducing new policy.Not covered by
No open PR touches either dedup block. #845/#850/#838 are the run-window cluster; #849, #856, #857, #864, #869, #876, #809, #837, #821, #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