fix(review-runs): collect Step 4's outcome signals in a tested script

[tend-agent]

review-runs Step 4 cross-checks outcomes with issues/comments and pulls/comments, neither of which returns a review body — pulls/comments carries inline review comments only. A maintainer CHANGES_REQUESTED whose correction is written in the body therefore returns nothing from both calls, and the run writes "no maintainer corrections" with corrections sitting in the window.

Step 4's three queries — dispositions, thread comments, and the missing review bodies — now live in plugins/tend-ci-runner/scripts/review-runs-corrections.sh, with generator/tests/test_review_runs_corrections.py pinning the window edges. The skill is a one-line invocation, net −20 lines. Extraction also closes the fail-open class the prose kept patching: an unset anchor compares less than every timestamp and --author "" applies no author filter, so the script exits 2 on a missing anchor or login rather than reporting an empty window that reads as an all-clear.

Verified against live API data (details below), and the script run against both this repo and max-sixty/leaf.

Verification

Review bodies are absent from pulls/comments. Review 4988584496 on #1024 (COMMENTED, non-empty body, submitted inside the window) has no representation in the repo-wide review-comments endpoint — not by review id, not by body text:

$ gh api --paginate "repos/max-sixty/tend/pulls/comments?since=$SINCE&per_page=100" > /tmp/pc.json
$ jq 'length' /tmp/pc.json
35
$ jq '[.[] | select(.pull_request_review_id == 4988584496)] | length' /tmp/pc.json
0
$ jq --arg b "${BODY:0:60}" '[.[] | select(.body | contains($b))] | length' /tmp/pc.json
0

Every entry the endpoint does return carries a path — inline comments, each belonging to a review record.

The script returns the missed reviews. Pointed at max-sixty/leaf over the window in #1026 (where both prescribed queries returned zero non-bot entries), the reviews key carries the two CHANGES_REQUESTED rows this PR exists to catch. On this repo over an 18h window it returns 1 disposition, 2 non-bot comments, 0 bodied non-bot reviews, in 4.3s.

(.body | length) > 0 is load-bearing. Empty-bodied review containers from a human exist on this repo today — e.g. review 4988620443 (max-sixty, #1024) and 4987925324 (max-sixty, #1022), both COMMENTED with no body. GitHub creates one per standalone inline reply, so without the filter every thread where anyone replied inline reports a correction with no text in it.

updated: takes a full timestamp. An earlier revision truncated $SINCE to a date on the claim that the qualifier accepts no time component. It does. On this repo with SINCE=2026-08-20T07:59:59Z: full Z form returns 9 PRs, the +00:00 form 9, date-only 10, unqualified gh pr list 200. The extra PR the date-only form admits is #1018, updated 2026-08-20T01:28:24Z — before the cutoff — so the time component is honoured at sub-day granularity rather than silently truncated. The sibling recipe in review-reviewers (--search "updated:>$WINDOW_START") already passes the full timestamp.

Two deltas from the recipe proposed in the issue: --paginate with per_page=100 on the reviews call (reviews page at 30 oldest-first, so the unpaginated call drops the newest — exactly where a window's reviews are), and the submitted_at filter inside the loop, since updated: windows the PR rather than the review.

Tests. 12 cases over a fake gh running the script's real filters: missing anchor and unresolvable login both exit 2 before any API call; open PRs excluded from dispositions (closedAt null); bot entries excluded from all three lists; --paginate asserted on both comment endpoints; an older comment edited into the window reported with both timestamps; the empty-bodied container excluded; a pre-window review on an in-window PR excluded.

review-reviewers — flagged in the issue as possibly sharing the blind spot — does not: it already calls pulls/{n}/reviews per PR in both its bot-output census and its acceptance check.

uv run pytest (581 passed) and uvx pre-commit run --files … both pass.

---

Closes #1026 — automated triage

[tend-agent]

The blind spot is real and the fix is the right shape — one added query, joined to the prose that already frames in-thread corrections. Two notes on the recipe, one verified as wrong.

updated: does accept a full Z timestamp — GitHub's search date qualifiers take optional time information, and gh passes it through. Measured on this repo just now with SINCE=2026-08-20T07:59:59Z: updated:>=$SINCE returns 9 PRs, updated:>=${SINCE%Z}+00:00 returns 9, updated:>=${SINCE%T*} returns 10. So the truncation isn't required, and the sentence explaining it states something a future run will read as ground truth about the API. The sibling recipe in review-reviewers already passes the full timestamp (--search "updated:>$WINDOW_START"), so this also leaves the two skills disagreeing on a point one of them asserts as fact. Suggestions inline for both the command and the prose.

Separately, and not introduced here: the same argument the PR body makes for re-reading $SINCE applies to $BOT_LOGIN in the adjacent conjunct of the same select(). Unset, .user.login != "" is true for every review — including every tend-review COMMENT body — so the query reports the bot talking to itself as maintainer corrections. That fails toward false positives rather than the false negative this PR closes, and the block above (issues/pulls/comments) has it too, so it's a Step 4-wide gap rather than something this diff owes. Worth a BOT_LOGIN=$(gh api user --jq '.login') next to the SINCE= line in both blocks if you're touching this again.

[tend-agent]

Both notes applied in 6862d7b; replies inline on the two suggestions.

On $BOT_LOGIN — took it in this PR rather than deferring. It's the same failure mode the diff already fixes for $SINCE (a variable that doesn't survive between tool calls silently disabling a filter), in the same select(), and it fails toward reporting corrections that aren't there, which is the outward-facing direction. Quantified it on this repo's last window: running the new loop with the bot filter off returns 8 bodied reviews, all tend-agent — so with $BOT_LOGIN unset, Step 4 reports 8 false corrections and every one of them is the bot reading its own tend-review output as a maintainer telling it it was wrong. The re-read goes in all three Step 4 blocks, since the disposition and issues/pulls/comments blocks carry it too.

Left $REPO alone deliberately: unset, it yields repos//issues/comments and a 404, which announces itself rather than passing silently.

Verification

Timestamp granularity, on max-sixty/tend with SINCE=2026-08-20T07:59:59Z:

full Z:      9
offset form: 9
date only:   10

The row date-only admits is #1018, updated 2026-08-20T01:28:24Z — before the cutoff. Equal counts for the first two plus a differing third is what rules out silent truncation.

The revised loop run verbatim returns zero non-bot rows on this repo for that window; with the bot filter dropped it returns the 8 rows above, confirming the empty is genuine and not a broken query.

uvx pre-commit run --files plugins/tend-ci-runner/skills/review-runs/SKILL.md passes.

[tend-agent]

Both notes are applied correctly. I re-measured the timestamp claim independently rather than take the reply's numbers: on this repo with SINCE=2026-08-20T07:59:59Z, updated:>=$SINCE returns 9, the +00:00 form 9, date-only 10, and an unqualified gh pr list returns 200 — the third number is what rules out the qualifier being parsed-and-discarded, which the two matching counts alone wouldn't. I also ran the revised loop verbatim over a wider window (2026-08-18) and cross-checked its candidate list against a per-PR enumeration that doesn't use --search at all: both find the same 20 bodied reviews, so the updated: candidate list has no false negatives for in-window reviews.

One new thing, introduced by this commit: the rewritten comment on the disposition block says an unset $BOT_LOGIN "matches nobody", which is the opposite of what happens and contradicts the clause right after it. gh pr list --author "" applies no author filter at all — on this repo it returns {"max-sixty":67,"tend-agent":132,"worktrunk-bot":1} where --author tend-agent returns only tend-agent — and in the other two blocks .user.login != "" is true for every entry. Both directions are "stops filtering", not "matches nobody". Given the PR's own thesis is that a wrong factual claim in a skill gets read as ground truth by a future run, worth correcting in place. Suggestion inline.

[max-sixty]

can we simplify?

[tend-agent]

Yes — simplified in 8ee0e3b, net 10 lines shorter than the previous revision and now +26/-5 against main.

Step 4's three queries are one block instead of three, so $SINCE and $BOT_LOGIN are set once rather than re-read per block, and the five-line comment explaining why each block re-reads them collapses to two lines on the single assignment. jq reads them as $ENV.SINCE / $ENV.BOT_LOGIN instead of having them spliced into the filter string — that removes "'$SINCE'"-style quoting from the disposition query and lets the reviews loop use a single-quoted filter instead of the double-quoted one with escaped inner quotes. The three trailing notes merge into one paragraph.

Same coverage: I ran the block verbatim out of the file. On this repo it returns 6 dispositions, 10 non-bot comments, 0 bodied non-bot reviews for the last window; pointed at max-sixty/leaf the reviews query still returns the two CHANGES_REQUESTED rows this PR exists to catch.

[tend-agent]

Ran the collapsed block verbatim out of the file rather than trusting the reply's numbers. It works, and the two mechanisms the collapse depends on both hold: $ENV.SINCE / $ENV.BOT_LOGIN resolve under gh's bundled gojq (not just system jq), and closedAt is null for open PRs, so select(.closedAt > $ENV.SINCE) still excludes them with the splice gone. On this repo's window the reviews loop returns 18 bodied reviews with the login filter dropped and 0 with it on — all 18 tend-agent — which is both the false-positive count the filter prevents and proof the empty result is a real empty rather than a broken query. review-reviewers genuinely doesn't share the blind spot: it already loops pulls/$pr/reviews in its repo-wide sweep.

One structural note, prompted by the "can we simplify?" ask rather than by a defect in the diff. The block is now 32 lines carrying three separately-verified edge cases — pagination direction on both endpoints, the empty-bodied review container, updated: windowing the PR while submitted_at windows the review. running-in-ci's Scripts over prose recipes names that threshold directly: "When a skill's code block needs edge-case handling or grows past a couple of dozen lines, put the logic in a tested script and leave the skill a one-line invocation with the intent … A prose recipe gets no shellcheck and no tests; every session re-derives its correctness." A scripts/review-runs-corrections.sh "$SINCE" beside the other nine, with a generator/tests/ case, would leave Step 4 a single line and would be the deeper version of the simplification — and Step 4's failure mode is the outward kind the tests would guard, since a silently-empty query becomes "no maintainer corrections" in the tracking issue, which later runs read as ground truth under Gate 1.

Not a blocker and not something I'd expand this PR to do — the diff is correct as written, and prose-vs-script is a call for you rather than something I should decide by pushing it.

[tend-agent]

Took the script, in 01c6ac2 — it's the better answer to "can we simplify?" than the collapse was, and the deciding factor is the one the review names: Step 4's failure mode is outward. A silently-empty query becomes "no maintainer corrections" in the tracking issue, which later runs read as ground truth under Gate 1, so the fail-open inputs are exactly the complexity CLAUDE.md says earns its place. In prose they could only be documented; in a script they're checked — review-runs-corrections.sh exits 2 on a missing anchor or an unresolvable login before it makes an API call, instead of returning an empty window that reads as an all-clear.

Step 4 is now one line and the skill is net −20 lines against main. The three edge cases the review counted are pinned by generator/tests/test_review_runs_corrections.py (12 cases, fake gh running the script's real jq filters) rather than re-derived per session.

Verification

Live, not just under the fakes. On this repo with an 18h window the script returns 1 disposition (#1025, merged), 2 non-bot comments (both max-sixty, including the "can we simplify?" one on this PR), 0 bodied non-bot reviews, in 4.3s. Pointed at max-sixty/leaf over 14 days it returns 33 bodied non-bot reviews including the two CHANGES_REQUESTED rows from #1026 — the reviews path this PR exists to add, exercised against real data.

Tests: missing anchor and empty login each exit 2 with no gh call logged; open PRs excluded from dispositions (closedAt null sorts below the anchor); bot entries excluded from all three lists; --paginate asserted present on both comment endpoints; a comment created before the window and edited inside it reported with both timestamps; the empty-bodied review container excluded; a pre-window review on an in-window PR excluded.

uv run pytest 581 passed; uvx pre-commit run --files on the three changed paths passes, shellcheck included.

[tend-agent]

The extraction is the right call, and the three window edges are pinned properly. One fail-open survives it, in the same class the script exists to close.

for pr in $(gh pr list … --search …) doesn't propagate the candidate query's failure. Bash's set -e doesn't check the exit status of a command substitution in a for word list, so a failed --search yields an empty list, the loop body never runs, and reviews reduces to [] with the script exiting 0. Step 4 then reads that as the all-clear it defines — "Empty dispositions, comments, and reviews is the all-clear" — and writes "no maintainer corrections" into the tracking issue, which is the outward claim the whole PR is about. Ran it against the script as committed, with a fake gh that fails only on the --search call:

$ review-runs-corrections.sh 2026-01-02T00:00:00Z
search: API rate limit exceeded
{ "since": "2026-01-02T00:00:00Z", "bot": "tend-agent",
  "dispositions": [], "comments": [], "reviews": [] }
$ echo $?
0

Worth saying why the other two paths don't have this: DISPOSITIONS=$(… | jq …) is an assignment, so pipefail plus set -e catch it, and in the comments block the gh api is the loop body, so its failure exits the subshell and fails the pipeline. Only the reviews block puts a gh call in a for list — and it's the call most likely to fail, since --search draws on the search API's 30 req/min budget rather than the 5000/h core one, and Step 4 runs after the log downloads and token report.

Suggestion inline hoists the candidate list into its own assignment, which set -e does check. Applied it locally: shellcheck stays clean (SC2086 isn't raised for a for word list, so no disable comment is needed) and all 12 tests still pass.

Smaller, and a call for you rather than a defect: the extraction moved the "since filters on updated_at, so an older comment edited inside the window is a real hit, not a broken filter" note into the script's header, where the run reading Step 4 won't see it. Rows still carry both timestamps, so a session now meets a created before the anchor with nothing on the page explaining why it qualified — and that direction discards a real correction.

[tend-agent]

Both notes applied in a730ffd; reply inline on the suggestion.

Took the second note too — the updated_at explanation is back on the page, not just in the script header. Its absence fails in the direction that discards a real correction: a session meets a comment row whose created is before the anchor, has nothing on the page saying the window filters on updated_at, and reads it as a broken filter rather than the edited claim it is. One sentence next to the invocation, where the run reading Step 4 is.

Verification

The fail-open, reproduced against the script as committed (fake gh, failing only the --search call): exit 0 with "reviews": []. With the hoist: non-zero, empty stdout. Pinned as a test, which fails on the pre-fix script (assert 0 != 0) — 13 cases now.

Live happy path unchanged: on this repo over a 6h window the script returns 1 disposition, 2 non-bot comments, 0 bodied non-bot reviews in 4.6s.

uv run pytest 582 passed. uvx pre-commit run --files on the three changed paths passes, shellcheck included.

[tend-agent]

Last open finding is closed. Verified the hoist rather than taking the reply's reproduction: set -e really does treat the two forms differently — R=$(for x in $(fail); do …; done | jq -s .) reaches the next line with R=[] and exits 0, while CANDIDATES=$(fail) aborts the script with rc=1. The new test is a genuine regression pin, not a tautology: checked out at 01c6ac2 it fails with assert 0 != 0 against the pre-fix script and passes against a730ffd.

Checked the other three fail-open shapes in the same script while I was in there, and they hold as the header claims: REPO="${GITHUB_REPOSITORY:-$(gh repo view …)}" propagates a failing substitution out of the parameter-expansion default, and a gh api failure in the comments loop body fails the pipeline under pipefail. So the reviews block was the only one, and it no longer is.

The --limit 200 on both pr list calls is the one thing I looked at and decided not to raise as a finding — Step 1 floors the anchor at 49 hours, and a 49h window on this repo is 10 updated PRs against a 200 cap, while the dispositions list would need a bot PR to sit open ~50 days (the oldest open one is 4 days, 3 open in total). Recording it only so the next reader doesn't re-derive it.

No approval because I authored the PR, not because anything is outstanding. Nothing new on the incremental; CI is green.