fix(list-recent-runs): recover the window on the last successful run, and stop the fetch limit re-truncating it - #838
Conversation
`--status completed` includes failures, so a run that died before its agent produced any analysis advanced the recovery anchor as if it had covered its hour. After an outage the next green run resumed one tick back and reported an all-clear for a window nothing had looked at. Anchor on the previous successful run, scan deeper than one cron period (during an outage the recent completed runs are all failures), and announce on stderr when the 6h cap clamps the recovered floor so the caller records a coverage gap instead of a false all-clear.
tend-agent
left a comment
There was a problem hiding this comment.
Two things on the recovery path, plus a smaller note. Reviewing my own PR, so this is a COMMENT.
The widened window isn't matched by the per-workflow fetch limit. The recovery now routinely pushes COMPLETED_AFTER to intended - 6h on exactly the outage path this PR targets, which makes CREATED_SINCE reach intended - 8h. But the fetch loop that actually collects the runs — the gh_retry gh run list ... --created ">=${CREATED_SINCE}" call guarded by refusing to report a partial run list, outside this diff — still passes --limit 50. gh run list returns newest-first, so more than 50 runs in that window silently drops the oldest ones: precisely the runs sitting in the recovered gap the anchor just reached back for. Measured just now on max-sixty/tend, which is itself a matrix leg in review-reviewers.yaml: tend-mention alone produced 48 runs in the last 8h, i.e. one workflow at the cap before the other seven are counted. Unlike the 6h clamp, this truncation prints nothing, so the caller records the all-clear this PR exists to prevent. gh paginates past 100 (--limit 150 returns 150 here), so raising the limit is cheap; warning when a workflow comes back with exactly --limit rows would turn a silent truncation into a visible one. Happy to push that as a commit here if you'd rather it land together.
gh run list can do the conclusion filter server-side — inline suggestion below. --status success is accepted and returns the anchor directly: I ran it against review-reviewers and got run 30893072876 at 2026-08-04T08:42:12Z, the same anchor the "after" block in the description shows. That drops the client-side conclusion filter and, more usefully, the scan-depth heuristic with it — the "50 completed runs might all be failures" reasoning stops applying, and an outage longer than 50 runs can no longer defeat the anchor. It does need the comment paragraph about the scan reaching past one cron period trimmed, and the else branch's wording changed from "among the last 50 completed ones" to something like "at all".
Smaller: the anchor query keeps 2>/dev/null || true, so a transient gh failure is indistinguishable from a genuine "no successful run" — and now it produces a confidently-worded warning naming a cause that didn't happen, plus a 6h window. The script's other gh calls deliberately route through gh_retry and exit non-zero rather than degrade, on the same reasoning this PR is applying.
…ndow Review follow-ups on the recovery path: - The fetch loop still passed `--limit 50` while recovery routinely widens the window to 8h. `gh run list` is newest-first, so a workflow over the limit silently drops its *oldest* runs — exactly the ones in the gap the anchor reached back for, restoring the false all-clear this PR exists to prevent. `tend-mention` alone produced 57 runs in an 8h window. Raise to 200 and warn when a workflow returns exactly the limit, so a truncation is visible rather than silent. - Filter the anchor query server-side with `--status success` (it accepts conclusions as well as statuses). The scan-depth heuristic goes away with it, so an outage longer than the scan can no longer bury the anchor. - Route the anchor query through `gh_retry` and fail loud. It kept `2>/dev/null || true`, so a transient API error was indistinguishable from "no successful run" and produced a confident warning naming a cause that hadn't happened. - The consuming skill still said "if empty, record all-clear", which contradicts every warning above; note that a WARNING means a coverage gap regardless of whether the list came back empty.
|
All three taken, pushed as fb26376. The fetch-limit one was the important catch — it would have restored the exact failure this PR exists to prevent, one layer down. Fetch limit. Reproduced the measurement: over an 8h window on Anchor query. Both applied as suggested — see the inline reply.
One thing the review didn't raise, folded in because it's the same false all-clear: the consuming skill still said "If empty, record the run as all-clear … then skip to Step 6", with nothing about the stderr warnings. An agent following it literally would print Branch verification (live, this checkout)
The anchor is recent again as of this run, so the recovery path needs the just-succeeded run excluded to reproduce — that's
At the real limit of 200, nothing on |
tend-agent
left a comment
There was a problem hiding this comment.
One finding on the anchor query, otherwise the follow-up commit holds up. Reviewing my own PR, so this is a COMMENT.
I exercised each branch against this repo rather than reading them: the normal path returns 55 runs with no warnings; --status success is a valid conclusion filter (it's in the flag's own value list) and an existing workflow with zero matching runs returns [] at exit 0, so the "no successful run" branch is reachable rather than shadowed by the new exit 1; the clamp branch fires correctly on tend-weekly (last success 2026-08-02, floored at 2026-08-04T17:47:00Z); and forcing RUN_LIMIT=5 produces the truncation warning per workflow. The limit raise is well sized — tend-mention returned 80 runs over an 8h window just now, so 50 was truncating by a third and 200 leaves real headroom.
A `workflow_dispatch` run has no `.schedule` in its event payload, so it takes the non-cron branch and covers a now-anchored 1h window rather than a tiled one. Anchoring on it therefore floors the next scheduled run just past a dispatch that never covered the gap — and unlike the clamp, it warns about nothing, because the anchor looks recent. Dispatching the workflow by hand to check on a fix mid-outage is the natural thing to do and is exactly when that bites, so it's the same class of hole as anchoring on a failure. Filter the anchor query with `--event schedule`. No-op today (no dispatch runs on record for `review-reviewers`, which does declare `workflow_dispatch:`), so it only bites the case above.
|
Second independent occurrence, from a different cause — reproduced live on this tick, so recording it here rather than opening a divergent PR. The motivating case was the weekly-limit outage: The anchor query on this tick, run both ways against live data: The failed 21:22Z run floors the window at 20:47Z, so Worth noting what the cap does here, since this run exercises the branch the PR added rather than the happy path: Substantive impact this time was nil, which is only luck: Evidence log: https://gist.github.com/dca23a6e6a0d8cae2665944ba31676fb |
|
The That also puts a number on the cap question raised above without needing a change: with Evidence log: https://gist.github.com/a88c03f4d0c3fb1791060ff3dd97d1c4 |
…s hit (#887) `token-report.sh` fetches each workflow's runs with `--limit 100`. `gh run list` returns newest-first and stops there silently, so on a workflow busier than 100 runs in the window the report drops the oldest ones and totals them at zero — with nothing in the output saying so. ## Measured on this repo Current 24 h window: ``` $ gh run list --workflow tend-mention --created ">=$SINCE" --status completed --json databaseId --limit 100 | jq length 100 $ gh run list --workflow tend-mention --created ">=$SINCE" --status completed --json databaseId --limit 400 | jq length 116 ``` 16 of 116 `tend-mention` runs (14%) are outside today's report, and their tokens are simply absent from the totals. `tend-review` returns 44 at both limits, so it isn't affected today — the shortfall is per workflow and moves with whichever one is chattiest. This matters because the report's output is the fleet cost figure `review-runs` records in its evidence log every day, and #801's entries have been reading it as a complete accounting. Under-reporting is also the direction that hides a problem: a workflow that suddenly runs hot is exactly the one that crosses 100 and starts having its excess dropped. ## Change Three things, all small: - **`--limit 1000`.** The limit is per workflow, not per report, so it only has to clear the busiest one. 500 was the first draft and it was already underfoot: at this script's own documented 168 h default, `tend-mention` returns **497** today, so a default-argument call would have started tripping the new warning within a day. Narrowing the documented default instead would have moved that cost out of sight rather than removed it. 1000 is the ceiling rather than a comfort margin — the Actions runs endpoint stops paginating there whatever `total_count` says, so anything larger is unreachable *and* puts the truncation guard beyond what the fetch can ever return, i.e. buys no runs and costs the warning. It is also the value that makes `-ge` trip exactly at the ceiling. - **Warn on an exact hit.** A count landing on the limit is the only symptom of truncation visible without re-querying `.total_count`, so the loop says so on stderr rather than trusting it. - **Warn on a failed fetch.** The original line swallowed any `gh run list` error into `[]` via `|| echo`, which the truncation guard reads as "0 runs, not truncated" — so an API blip removed an entire workflow from the totals with no marker at all. That is the same silent under-report at full strength, and strictly worse than the tail-drop this PR started out fixing. Branching on the exit status covers it. Warning rather than exiting, unlike the sibling in `list-recent-runs.sh`, because this script has no `gh_retry` behind it and a bare `exit 1` would make one blip fatal to a report that is otherwise still useful. Raising a limit alone would only move the cliff. The two warnings are what make the next crossing — from either direction — visible instead of silent. The residual this doesn't fix: at exactly 1000 the report is still truncated, just no longer silently. Getting the full set past the ceiling needs `.total_count` off the API or a narrower window per fetch, both more than this PR is for — and a loud partial beats a silent one. Verified all three branches against the API: `tend-review` → 52 runs, silent; a nonexistent workflow → the fetch warning; `tend-mention` at `--limit 100` → exactly 100, the truncation warning. The ceiling and the guard's reachability at the new constant, measured on this repo: ``` $ gh api ".../actions/workflows/250047576/runs?status=completed&per_page=100&page=10" --jq '.workflow_runs | length' 100 $ gh api ".../actions/workflows/250047576/runs?status=completed&per_page=100&page=11" --jq '.workflow_runs | length' 0 $ gh run list --workflow tend-mention --created ">=2000-01-01T00:00:00Z" --status completed --json databaseId --limit 2000 | jq length 1000 $ gh run list --workflow tend-mention --created ">=2000-01-01T00:00:00Z" --status completed --json databaseId --limit 1000 | jq length 1000 ``` `total_count` for that workflow is 3265, so the 1000 is the endpoint's ceiling and not the window running out. The last line is the guard firing condition met at `RUN_LIMIT=1000` — unreachable at 2000. Patched script runs clean end to end (`token-report.sh 2 "review-"`, exit 0, 48 runs, no warnings), and `shellcheck` is clean. ## Provenance and scope Found by the review on #886 — that PR fixes the same silent-truncation shape in `review-runs`' Step 1 census (30 of 110 runs, a 68-minute view of a 24-hour window), and the reviewer measured this adjacent case in Step 2 while checking it. Kept separate because it's a different file and a different fetcher. #838 is doing the equivalent work for `list-recent-runs.sh` — raise the bound, warn at the boundary — so this is the third instance of one pattern rather than a new idea. Not a dedup hit: different script, different call, no overlap in the diff. ## Gate assessment - **Evidence level: High.** Reproduced directly against the API, twice, at two limits. Structural — `gh run list` truncates at the limit deterministically, no decision point. - **Change type: targeted fix** — one constant and a four-line guard. Normal Gate 1 bar, cleared. - **Verified**: every branch exercised against the live API, and the patched script run end to end. - **Revised twice after review** on this PR. Round one caught the swallowed-fetch path on the line being edited and measured 500 against the 168 h default; both folded in as a second commit. Round two caught that the replacement constant, 2000, sat above the API's 1000-result pagination ceiling and so made the truncation guard dead code — the defect this PR fixes, relocated. Third commit caps at 1000 and names the ceiling as the reason, so the next raise hits the explanation first; a fourth carries that framing into the runtime warning, which had called 1000 "the fetch limit" — a tunable-sounding phrase inviting the same bump — and now names it as the API's pagination ceiling and points at narrowing `HOURS`, the lever that does work. --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
list-recent-runs.sh's dropped-tick recovery anchors the window floor on the previous completed run of the analyzing workflow.completedis a status, not a conclusion, so a run that failed before its agent produced any analysis advances the anchor exactly as if it had covered its hour. After an outage the next green run therefore resumes one tick back and reports an all-clear for a window nothing ever looked at — and the gap is unrecoverable, because the following run's floor advances past it too.This is not hypothetical: it just consumed ~15 hours of
review-reviewerscoverage.What happened
Every
review-reviewersrun from 30897445507 (09:41Z) through 30959115609 (23:10Z) failed — 16 consecutive runs, all five matrix legs,claude -pexiting 1 after ~4s. Theresultevent in every leg's session log is identical:is_error: true,num_turns: 1,result: "You've hit your weekly limit · resets 12am (UTC)", withtoken-usage.jsonall zeros. The agent never started, so none of those runs analyzed anything. (The outage itself is already tracked by #831 and the annotation-legibility work in #816 / #818 — this PR is only about the window the outage swallowed.)The last run that did analyze anything is 30893072876 at 08:42Z, which is also the last entry in the
numbagg/numbaggevidence gist. At this run's tick (23:47Z intended), the recovery query picked{"conclusion":"failure","createdAt":"2026-08-04T23:10:57Z","databaseId":30959115609}and floored the window at 22:47Z, solist-recent-runs.shreturned exactly one row: atend-notificationspre-check no-op.Everything
numbagg-botactually did in the swallowed window sat outside that floor — PRs #719 and #720, issue #721, three reviews and four inline comments on #719, and comments on #716, #721 and #1. That is the densest window numbagg has had in days, and per the skill's Step 1 ("If empty, record the run as all-clear … then skip to Step 6") it would have been recorded as a quiet hour. I only found it by diffing the run list against the gist's last recorded boundary by hand.The change
Anchor on the previous successful run rather than any completed one, filtering server-side with
--status success(the flag takes conclusions as well as statuses), and when the existing 6h cap clamps the recovered floor, say so on stderr so the caller records a coverage gap instead of a false all-clear. A partially-failed matrix run counts as a failure here, which only ever widens the window — overlap is re-offered work the caller dedups against its own evidence log, whereas a gap is silently unanalyzed.Widening the window then exposed a second truncation one layer down, so the fetch loop moves with it. It passed
--limit 50while a recovered window spans up to 8h, andgh run listreturns newest-first — so a workflow over the limit silently drops its oldest runs, which are exactly the ones in the gap the anchor just reached back for. Onmax-sixty/tend,tend-mentionalone produces 57 runs in an 8h window. The limit is now 200, and a workflow returning exactly the limit warns rather than truncating in silence.Two smaller pieces: the anchor query routes through
gh_retryand exits non-zero instead of2>/dev/null || true, so a transient API error can no longer masquerade as "no successful run" and emit a confident warning naming a cause that didn't happen; andreview-reviewers/SKILL.md, which still said "if empty, record all-clear" with nothing about the stderr warnings, now treats anyWARNING:as a coverage gap — without that, an agent following the skill literally would print the warning and record an all-clear anyway.When every tick fires and succeeds the anchor is the previous tick,
prev_intended == intended - 3600 == COMPLETED_AFTER, and the comparison is a no-op — output is byte-identical to today's on the healthy path.Verification against live data
Same tick, same repo, before vs. after:
TARGET_REPO=numbagg/numbagg ./list-recent-runs.shafter the change returns 5 runs instead of 1, preceded by:The recovered anchor (08:42:12Z) matches the gist's last recorded coverage boundary exactly. Also exercised: the no-successful-run branch (warns, floors at the cap, exits 0) and the unset-
GITHUB_WORKFLOWbranch (recovery skipped, unchanged).shellcheckclean;bash -nclean. Also exercised after the review follow-ups: the fetch-limit truncation warning (via a copy withRUN_LIMIT=5) and the anchor-query failure path (exits 1 rather than degrading). See the follow-up comment for the full branch table.Gates
list-recent-runssurfaces only the three merged predecessors fix(list-recent-runs): anchor completion window to intended cron tick #526, fix(list-recent-runs): recover dropped cron ticks via previous-run anchor #753 and fix(list-recent-runs): fail loud on transient gh errors instead of a false all-clear #784.Evidence log: https://gist.github.com/19b5ab297bb7ac7e1e9a44d595ccde0f