perf(review-reviewers): cut cadence to 3-hourly, and anchor the window to the cron period - #845
perf(review-reviewers): cut cadence to 3-hourly, and anchor the window to the cron period#845tend-agent wants to merge 7 commits into
Conversation
…w to the cron period Anthropic quota exhaustion has now hard-failed the fleet twice in three days. review-reviewers runs hourly across a 5-repo matrix (120 Claude sessions/day) and takes ~70-79% of fleet spend, which keeps the subscription quota saturated; event-driven workflows that land mid-window fail at startup having done nothing. Cutting to `47 */3 * * *` takes it to 40 sessions/day. Outcome-acceptance signal evolves over hours to days, so little analytical value is lost. list-recent-runs.sh only anchored its completion window for a bare hourly cron (`MM * * * *`); any other shape fell back to a now-anchored 1h window. Cutting the cron alone would therefore have left each run seeing one hour out of every three — a silent two-thirds coverage hole. Generalize the detection to `MM */N * * *` where N divides 24, and derive the window from the cron period. For the hourly form `cron_hour_step` is 1 and every expression reduces to the original arithmetic.
tend-agent
left a comment
tend-agent
left a comment
There was a problem hiding this comment.
The cadence cut is well-evidenced and the window generalisation holds up — I re-derived the tiling independently (exact tiling at 3-hourly including the midnight wrap, byte-identical arithmetic on the hourly path across 24h × 3 minute offsets × 4 cron-minute values, and */5 / MM H * * * / */15 * * * * still falling through). Three gaps, one of which is the PR's own failure mode surviving in a second place.
The "past hour" framing survives in the skill prompt. The PR's thesis is that cutting the cron without generalising the window leaves a silent two-thirds coverage hole. review-reviewers/SKILL.md updates the frontmatter description but keeps two hourly assumptions in the body that the run list no longer matches:
- The opening instruction — "Analyze tend's CI behavior on the target repo over the past hour" (immediately under
# Review Reviewers). This is the agent's own framing for the whole analysis, so it now contradicts the 3-hour run list it is handed in Step 1. - The Step 2 survey-subagent prompt — "
tend-notifications: check for recent bot comments/issue-close events in the past hour". Unlike the other three mappings in that list, this one has no run-scoped anchor (noheadBranch, no triggering comment) — it reconstructs outcomes purely from the time window. A subagent told "past hour" while the run list spans three will under-surveytend-notificationsoutcomes by exactly the two-thirds this PR exists to prevent.
Both want to be period-agnostic ("the analysis window", "the window covered by the run list") rather than restamped to "past 3 hours" — otherwise the next cadence change reopens the same hole.
The script's header docstring wasn't carried along with the detection comment. The comment at the detection site is generalised, but the module header above set -euo pipefail still documents the old contract: "whose updatedAt falls within a 1-hour completion window", "a simple hourly cron (MM * * * *)", "tile exactly: [intended-1h, intended], then [intended, intended+1h]", and "For non-schedule events or non-hourly crons, falls back to a now-anchored 1h window" — the last of which now reads as excluding the very shape the PR adds. # Fetches runs started in the past 3 hours is stale too: CREATED_SINCE is COMPLETED_AFTER - 7200, so at the 3-hourly step it reaches five hours back. That header is what a reader hits first, so it's the one that misleads.
I've pushed a commit fixing all three (the third is the inline suggestion below) — it's doc-only and doesn't touch the lines #838 rewrites, so the rebase note in the PR body still stands.
One thing I checked and am not flagging: the widened CREATED_SINCE span makes the per-workflow --limit 50 truncate silently if it ever binds, but current density on this repo is 4–10 runs per workflow per 5h, so there's an order of magnitude of headroom.
…er and the skill prompt The cron detection was generalised to MM */N * * *, but three places still described a one-hour window: - list-recent-runs.sh's module header, which still documented hourly-only detection, a 1h window, and a 3h created lookback. - review-reviewers/SKILL.md's opening instruction, which framed the whole analysis as 'the past hour'. - The Step 2 survey-subagent prompt's tend-notifications mapping, which has no run-scoped anchor and so reconstructs outcomes from the window alone — told 'past hour' against a 3h run list, it would under-survey by exactly the two-thirds this change exists to prevent. Also corrects the 6h reach-back cap comment: 21600/3600 is six periods at hourly, not one.
…e cadence The skill still told the agent to open its PRs on `hourly/review-<run-id>`. The workflow no longer runs hourly, so every branch it creates would have carried a wrong cadence in its name. Prefixing with the workflow name instead of the cadence keeps it correct across future cadence changes — the same period-agnostic treatment the opening instruction and the Step 2 survey prompt just got.
|
All three points are in 8631cab. Re-grepping the two files afterwards turned up a fourth hourly assumption the review missed, now fixed in 3a1d13a: Step 5 told the agent to open its PRs on Nothing filters on that prefix (single occurrence across the repo), so it was cosmetic rather than a coverage hole. The commit is doc-only and still clear of the lines #838 rewrites. |
tend-agent
left a comment
tend-agent
left a comment
There was a problem hiding this comment.
One finding on the incremental plus a smaller note; both land on the same missing piece. Green CI, and self-authored so this is a COMMENT rather than an approval.
Secondary — workflow_dispatch now under-covers by the same two-thirds this PR closes. A dispatch event carries no .schedule, so it takes the fallback branch and gets a now-anchored 1 h window. Under 47 * * * * that happened to equal one period, so manual and scheduled runs saw the same span; at 47 */3 * * * a dispatched run analyzes one hour of a three-hour period, and Step 1's "if empty, record the run as all-clear" means that under-coverage can be written down as a clean window. Dispatch is the natural way to re-run after a dropped tick or a quota outage, which is when coverage matters most. I don't think the dispatch path should guess a period — but surfacing the window (below) at least makes the short span legible in the log instead of silent.
Both points reduce to: the window is computed and never published. list-recent-runs.sh emits only the per-run JSON array; COMPLETED_AFTER is used by the final jq filter and discarded. One line after the if/else that sets it, before the fetch loop, would publish it without disturbing the stdout contract:
echo "Completion window: >= $(date -u -d "@$COMPLETED_AFTER" +%Y-%m-%dT%H:%M:%SZ)" >&2Step 1 then tells the agent to carry that value forward, and Step 2 substitutes it for <window-start> and for the new line-235 wording. Happy to push it if you want — it touches lines outside this diff, so I've left it as an offer.
Nothing else new: the four hourly assumptions are correctly retired, 21600 / 3600 reads six now, and the hourly/ branch prefix has no other references in the repo (review-runs keeps daily/, which is still true of its cron).
…ilter on it The window's floor was computed and then discarded — used only by the final jq filter inside the script. Two consumers needed it and neither could get it: - Step 2's corruption scan interpolates a literal `<window-start>` into five --jq filters with no stated source. - The tend-notifications outcome mapping has no run-scoped anchor, so it reconstructs outcomes from the window alone. Telling it to "use the window's actual span" was unresolvable from inside that prompt. Reconstructing the floor from the run list's own min/max updatedAt isn't equivalent: that's the span of runs that happened to fire, it collapses toward a point when one run lands in the period, and it's empty exactly when the list is empty and the all-clear gets recorded. Printing it also makes the fallback branch legible. A workflow_dispatch carries no .schedule, so it takes the now-anchored 1h window; under the old hourly cron that equalled one period, but at 47 */3 * * * a dispatched run covers one hour of three. Dispatch is how you re-run after a dropped tick or a quota outage, which is when coverage matters most. The dispatch path still doesn't guess a period — the short span is now visible rather than silent. stderr, so stdout stays the run-list JSON. Verified end-to-end against max-sixty/tend: window line on stderr, stdout parses as a 15-element array.
|
Took the offer — b9523c6 publishes the window, which closes both the
The VerificationRan it rather than inspecting it: That run took the fallback branch (this session is not a The module header now documents the stderr line alongside the window contract. |
tend-agent
left a comment
tend-agent
left a comment
There was a problem hiding this comment.
Both open findings from the last pass are closed on the incremental: the survey prompt now names the window's source and tells the subagent to substitute it, which covers all five <window-start> interpolations in the corruption scan (they're inside the same prompt block), and Step 1 now flags the workflow_dispatch under-coverage rather than leaving it silent. I checked the published floor's format against the --jq string comparisons it feeds — +%Y-%m-%dT%H:%M:%SZ sorts lexically against GitHub's created_at/submitted_at, so > does what it looks like it does — and the echo sits after the if/else, so both branches emit.
One nit inline on the new width claim.
…tick recovery The default floor is one cron period back, but dropped-tick recovery pulls it to the previous actual run's intended tick, capped at intended - 21600 — so a scheduled run can publish a window up to 6h wide, two periods at 3-hourly. Step 1's description is what the agent reasons from when judging how much ground the window covers, so it shouldn't state the default as the maximum.
…he whole matrix (#877) `review-reviewers` lost **8 h 15 m** of coverage today across all five target repos, and the cause was a two-line gate in its own workflow rather than anything in a skill. ## What happened A critical GitHub Actions incident ([status page](https://stspg.io/rcz3fcm83sff), created 2026-08-06T15:22:49Z, still `investigating`) made runner acquisition unreliable. Four consecutive `review-reviewers` ticks then failed identically — `init-tracking` sat unscheduled for ~15 minutes, GitHub marked it `cancelled`, and because the matrix job has a hard `needs: init-tracking`, all five legs were `skipped` without ever trying for a runner: | Run | Created | `init-tracking` | matrix | |---|---|---|---| | [31122877870](https://github.com/max-sixty/tend/actions/runs/31122877870) | 17:21:15Z | `cancelled` | `skipped` ×5 | | [31125649607](https://github.com/max-sixty/tend/actions/runs/31125649607) | 18:17:43Z | `cancelled` | `skipped` ×5 | | [31126978811](https://github.com/max-sixty/tend/actions/runs/31126978811) | 19:34:17Z | `cancelled` | `skipped` ×5 | | [31128259981](https://github.com/max-sixty/tend/actions/runs/31128259981) | 21:22:19Z | `cancelled` | `skipped` ×5 | That's 20 leg-analyses discarded. The legs were not themselves unschedulable: in [31119653960](https://github.com/max-sixty/tend/actions/runs/31119653960) (16:23Z) `init-tracking` did succeed, and matrix legs went on to acquire runners and run — `numbagg/numbagg` completed successfully at 16:34Z during the same incident. ## Why the gate isn't worth its cost `init-tracking` exists to win a find-or-create race that can only bite on the **first cron tick of a month** — before that tick, no tracking issue exists for the new month and N legs would each try to create one. The `review-reviewers` skill still carries its own find-or-create fallback, sorting lowest-numbered-first precisely so a lost race degrades to a duplicate rather than a crash. This change makes that path load-bearing on a first-tick failure rather than a pure safety net, so the skill's wording is updated in the same commit to say so. So the gate trades *up to N-1 duplicate tracking issues (four, at today's five legs), once a month* against *the entire analysis window for every target repo, every time the job can't get a runner*. This change keeps `needs:` — so `init-tracking` still runs first and still wins the race on every normal tick — and only relaxes the failure propagation. `!cancelled()` rather than `always()` is deliberate: a user cancelling the run should still stop the matrix. ## Gate assessment - **Evidence level: High** — 4 occurrences in a single window, all with byte-identical job shape. - **Structural, not stochastic.** A hard `needs:` gate always propagates; there is no decision point for the agent to get right. Confirmed against the last 14 `review-reviewers` failures: the `init=cancelled → matrix=skipped` shape appears **only** on today's four, while every 2026-08-05 failure ran `init=success` with the matrix reaching the agent. The gate is the amplifier; the incident is only the trigger. - **Change type: removal of a gate** — 2 functional lines. Under the magnitude gate a removal clears at 1 occurrence; this has 4. - **Not an incident workaround.** The skill's own rule is to record transient upstream incidents rather than commit code around them, and this run recorded the rest of the incident's damage without acting on it. This one is different in kind: it removes a permanent single point of failure that will amplify the *next* infrastructure hiccup the same way, and it stays correct after the incident resolves. - **Dedup**: no open or closed tend issue or PR mentions `init-tracking`. #845 (cadence) and #858 (per-leg branch names) touch this workflow but not this gate. Evidence log: https://gist.github.com/192514ea2c36586f9b7f842a482d62ab Found while analysing `PRQL/prql` in run [31132205674](https://github.com/max-sixty/tend/actions/runs/31132205674) — the run that had to absorb all four lost windows. --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
) ## Problem `review-reviewers` matrixes over five target repos ([`review-reviewers.yaml`](https://github.com/max-sixty/tend/blob/f3e309a475af03147ce6290104c85baafe6ad9f5/.github/workflows/review-reviewers.yaml)), and all five legs of a tick share one `$GITHUB_RUN_ID`. Step 5 of the skill hands every leg the same branch recipe: > **PR** (default): Branch `hourly/review-$GITHUB_RUN_ID`, fix, commit, push, … So the moment two legs of the same tick both find something worth a PR, they contend for one ref. The loser's `git push` is rejected as a non-fast-forward and it has to recover mid-session; the ordering that hurts more is the other one, where the second leg pushes *before* the first opens its PR and the two unrelated concerns land on a single branch — the atomic-PR rule broken by the branch name, not by any judgement the agent made. This just happened on run 31058673934: the `max-sixty/cargo-affected` leg's push to `hourly/review-31058673934` was rejected against a sibling leg's commit, which by then was #856. It recovered by suffixing the target name (#857), and an earlier tick shows the same improvisation baked into `hourly/review-30962483562-numbagg` — legs have been working around the recipe rather than following it. The run ID is non-unique along a second axis too: the same section caps a leg at two PRs (`**Limit to at most 2 PRs per run**`), and a leg that acts on two findings computes one name for both — same collision, one job instead of two. ## Fix Put both discriminators in the recipe: `hourly/review-$GITHUB_RUN_ID-<target-repo-name>-<topic-slug>`. The target name is unique across the matrix by construction, the topic slug separates a single leg's two PRs, and the branch stays greppable back to its run. Scoped to this skill. `review-runs` carries the same `$GITHUB_RUN_ID`-only shape, but `tend-review-runs.yaml` has no matrix, so a run there is a single job and the name can't collide — no change needed and none made. **Overlaps #845 on this line.** That PR rewrites the same recipe to rename the prefix (`hourly/` → `review-reviewers/`) as part of its cadence change, and keeps `$GITHUB_RUN_ID` as the whole discriminator — so it ships the collision under a new name. The two are orthogonal in substance and conflict only textually: whichever lands second wants `<prefix>/review-$GITHUB_RUN_ID-<target-repo-name>-<topic-slug>`, taking the prefix from #845 and the suffixes from here. ## Gate assessment - **Evidence level**: High — the collision is deterministic given two acting legs, and there are two independent traces of it (this run's rejected push, and a prior tick's hand-suffixed branch). The one-leg-two-PRs axis is structural rather than traced. - **Structural, not stochastic**: no decision point. Every leg is told to compute the same name from the same variable; replayed ten times it collides ten times. - **Change type**: targeted fix — one line of the recipe. - **Passes both gates.** Evidence log: https://gist.github.com/dca23a6e6a0d8cae2665944ba31676fb --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
…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>
# Conflicts: # plugins/tend-ci-runner/skills/review-reviewers/SKILL.md
|
Adding a measurement this PR has not had until now: the 24 h window 2026-08-07T08:02Z → 2026-08-08T08:02Z is the first clean one in the So this is the steady-state cost of
At that rate the hourly cadence alone is ~$6,400/month at API list prices. This PR's On what the cadence buys: all six PRs the workflow produced in the window came from two of the 24 runs — 31160773238 (#882, #884, #885) and 31164742947 (#891, #892, #893), which are also the window's two most expensive runs at $15.28 and $18.36. The other 22 appended to their per-target evidence gists and produced no PR or issue, which is the intended below-threshold path — the gist for Recording here rather than opening a competing PR — this is the existing lever, open since 2026-08-05, and it is |
|
why does doing it less frequently save costs? |
|
Because most of what a run costs is paid at startup, per tick, and has nothing to do with how much window it analyses. Cutting ticks removes that fixed part; it does not remove the analysis work, which just gets batched into fewer, bigger runs. That also means the projection in the PR body is wrong, and your question is the reason I checked. It said "on the same $8.87/run it lands near $71/day" — that assumes per-run cost stays flat while the window triples, which is exactly the thing that can't be true. Measured properly the cut is ~$213 → ~$120/day, about 40%, not the 67% implied by 24 ticks → 8. The 5-repo matrix is a natural experiment for the split: every leg pays the same startup (skill load, bot-login resolution, fetching the target's Regressing cost on runs-surveyed across 10 legs (two ticks, 08:02Z and 09:59Z): cost per leg ≈ $1.12 fixed + $0.12 × runs surveyed — so at typical density ~65% of an hourly run is fixed overhead.
The model lands on $207/day for the hourly cadence against the $212.79 actually measured in the clean window, which is the main reason I trust it. So the mechanism is just: 24 ticks/day pay the Worth being explicit that this weakens the cost argument for the PR by a third, though I think it still clears: ~$90/day is real, and the quota-exhaustion case — 120 Claude sessions/day saturating the seven-day limit and hard-failing the user-facing workflows — rests on session count, which does drop by the full 3×, independent of dollars. Method, data, and caveatsPer-leg costs come from each matrix job's own Run
Run
Fit on all 10 points: fixed $1.12, variable $0.119/run, R² 0.70. Dropping Caveats, both pushing the estimate around by less than the headline gap:
n=10 across two ticks, so treat $1.12 ± a bit rather than a precise constant. The structural claim — a large per-tick floor, roughly two thirds of a quiet run — is what the two independent fits agree on, and it's the part the argument needs. |
|
The quota wall came back last night, ~14 h after the comment above called the preceding 24 h the first clean window this PR could be decided on. That window did not hold a day. This one was the rolling session quota, not the seven-day one: every agent invocation from What it actually cost, all of it verified from the run list and the session-log artifacts:
The attribution is the part worth adding here. Metering the five hours preceding the first refusal (
53.5% of the window's spend from 13% of its runs — and that is against an unusually busy fleet half, three PRs (#902/#903/#904) generating 33 metered runs in five hours. On a quiet evening the share would be higher, not lower. The asymmetry that makes this a cadence argument rather than a cost argument: the workflow that drove the wall was also the only permanent casualty of it. Recorded in the evidence gist; no new PR, since this one already proposes the change. |
…t now-24h (#939) ## Problem `review-runs` Step 1 opened its window with `date -u -d '24 hours ago'`, which resolves when the *agent* runs the command — the run's start plus container boot and skill loading. The predecessor started at *its* own start time, earlier by whatever drift it saw, so the window opened strictly after the predecessor and dropped every run in the gap. The gap is never zero and never negative, so the census systematically under-counted rather than flaking. Step 2 clipped the same band independently, and by a wider margin, because `token-report.sh 24` measured 24 hours back from its own later invocation. Reproduced on this repo, independently of the reporter's measurement on `max-sixty/cargo-affected`. Yesterday's `review-runs` run started `2026-08-09T08:01:45Z`; today's ([31369350870](https://github.com/max-sixty/tend/actions/runs/31369350870)) started `08:16:33Z`, so its window opened no earlier than that — a band of at least 14m48s, and wider by however long boot and skill loading took. Eight runs sat in it: ``` 2026-08-09T08:03:08Z review-reviewers 31302588340 2026-08-09T08:12:32Z ci 31302963325 2026-08-09T08:12:32Z tend-review 31302963274 2026-08-09T08:13:08Z ci 31302989157 2026-08-09T08:13:08Z tend-review 31302989224 2026-08-09T08:14:21Z tend-mention 31303039387 2026-08-09T08:16:03Z tend-mention 31303111366 2026-08-09T08:16:11Z tend-mention 31303117289 ``` Six of those are full agent sessions. The loss is also biased toward the runs that matter most to the audit: what happens in the minutes right after `review-runs` starts is largely the work that run triggers — its own PR getting reviewed, a mention firing on that review — and that is exactly the band the next run cannot see. This is distinct from #886 and #888, which decide how much of the window is *visible*. A fully paginated census of a window that opens too late still misses these runs, and Step 2 — the fallback that recovered the truncated runs in #888 — has the same defect. ## Solution Anchor `SINCE` on the predecessor's `created_at` so consecutive windows tile, and derive Step 2's lookback from the same anchor instead of a literal `24`. Three details beyond the reported shape: - **`status=success`, not `status=completed`.** The API's `completed` includes `failure` and `cancelled`, so anchoring on it would permanently strand the band of a predecessor that died before its census. Reaching for the last *successful* run covers that band on the next pass. - **Clamp a stale or missing anchor at 49h.** A fresh repo has no predecessor, and after an outage a week-old anchor would pull in a week of runs. 49h lets the window absorb one skipped day without unbounded growth; Step 5 already dedups findings a widened window sees twice. - **Exclude `$GITHUB_RUN_ID`.** A re-run attempt of the current run can surface as completed, and anchoring on itself collapses the window to zero. The workflow id is derived from the run rather than the file name. ## Testing No test harness covers skill text, so both recipes were extracted verbatim from the edited file and executed here: - Anchored form, simulating today's `review-runs` run: `SINCE=2026-08-09T08:01:45Z` — the predecessor's start, exit 0. - Empty-predecessor branch: falls through to the `25 hours ago` default, exit 0. - Stale anchor (`2026-08-01`): clamps to the 49h floor. - `HOURS` derivation from that `SINCE`: `25`, exit 0. The `if [[ ... ]]; then ... fi` form is deliberate over `[[ ... ]] && SINCE=$FLOOR`: the latter exits 1 when the test is false, which the agent's Bash tool reports as a failed block. <details><summary>`review-reviewers` — already anchored, no change needed</summary> The report flagged `review-reviewers` as possibly sharing the defect, unmeasured. It doesn't: it gets its window from [`list-recent-runs.sh`](https://github.com/max-sixty/tend/blob/fd23cd06b8850c8db6777a57f547931b86575f5c/plugins/tend-ci-runner/scripts/list-recent-runs.sh), which anchors the completion window to the most recent *intended* cron tick rather than to `now`, precisely so scheduler drift can't shift the window relative to actual start time. #845 extends that anchoring to every-N-hours crons. </details> ## Review follow-ups Two changes landed after review, both on this branch. **The census is now on completion, not creation.** Anchoring alone made the *creation* windows tile, but Step 1's heading promised completed-since-the-predecessor and the query still filtered `created>=$SINCE`. A run created before the anchor and still in progress at the predecessor's census was dropped there by `status=completed` and dropped again here — censused by nobody, and that is precisely the long-running class Step 3 exists to hunt. Step 1 now over-fetches by `created` and filters on `updated_at`, the shape [`list-recent-runs.sh`](https://github.com/max-sixty/tend/blob/74328fc2898f386fd4569fcfdd392a88db92da80/plugins/tend-ci-runner/scripts/list-recent-runs.sh) already uses. The over-fetch floor is 24h — a whole run lifetime, not the 6h hosted-runner job cap. `created_at` starts at queue time, and a `cancel-in-progress: false` group can hold a run queued for hours before execution begins; the longest completed run on this repo in the last three days spans 819 minutes wall-clock ([31281456692](https://github.com/max-sixty/tend/actions/runs/31281456692)), which a 6h floor still misses. Diffing the two floors against the live API in one pass, 24h is a strict superset that recovers exactly that run (242 -> 243). Also confirmed GitHub's `created=` filter honors time-of-day rather than rounding to the date, which the sub-day anchor depends on. **Step 1's lead comment is trimmed** per CLAUDE.md's skill-authoring brevity rule; the argument lives here, not in a file loaded into every session. **The anchor is now persisted to a file.** Step 1 set `$SINCE` in one Bash tool call and Steps 2 and 4 read it in others, where shell state is gone. Neither failed loudly: `date -d ""` resolves to today's midnight rather than erroring, so Step 2's `HOURS` silently became hours-since-midnight — 9 rather than 25 on this cron, pricing a *narrower* band than the literal `24` it replaced. Step 4's `closedAt > "$SINCE"` compared against the empty string, which sorts below every timestamp, so that cross-check admitted every closed bot PR ever (62 here against 10 for the correct anchor) and had never been windowed at all — that half predates this PR. Step 1 now writes the clamped anchor to `/tmp/review-runs-since` and both readers `cat` it back, verified across a real call boundary. --- Closes #938 — automated triage --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
# Conflicts: # plugins/tend-ci-runner/scripts/list-recent-runs.sh # plugins/tend-ci-runner/skills/review-reviewers/SKILL.md
…n window (#965) Cuts the four complexity hotspots from the recent bot-PR audit back to the #960 vision — mechanism only where it prevents wrong outward actions — and moves the surviving logic under real tests. Everything loaded or executed in production shrinks (~420 lines across the template, generated workflows, and skill prose); the growth is tests for scripts that previously had none. - **`mention.yaml.j2` verify gates: 4 → 2.** The stacked review-path skip gates collapse into two structural rules: a bot-authored review summons a session only as the reviewer→author handoff (fresh content on a PR the bot authored), and a contentless approval is terminal whoever submitted it. GitHub rejects self-approvals, so the empty-body-APPROVED gate was already subsumed by an author-keyed rule. Outward behavior is unchanged — verified case-by-case twice (by hand, and independently by the review sweep's verifier); each rendered workflow drops ~52 lines. - **`running-in-ci`: 869 → 677 lines.** The two markdown poll recipes become tested scripts. `poll-pr-checks.sh` polls by commit OID, never the PR head, fixing the false green a concurrent push caused (closes #829) and carrying the edge cases #837's five review rounds surfaced — null merge-ref rollups, superseded check runs, STARTUP_FAILURE/ACTION_REQUIRED bucketing — so it supersedes #837. `rerun-failed-jobs.sh` owns the rerun and finds the new attempt's jobs by `run_attempt`, so a fast rerun no longer reads as "nothing re-queued" and an unregistered one can't report stale conclusions as fresh. The rarely-taken skill-PR mechanics move to `references/skill-pr-workflow.md`. - **`list-recent-runs.sh`: 207 → 113 lines.** The completion window becomes "floor = the last successful run's start, clamped at 6h, else 6h with a coverage-gap warning". Any successful run's window opens at or before its own start, so consecutive windows overlap and never gap — which makes the cron parsing, tick tiling, dropped-tick recovery, and the schedule-event restriction all unnecessary, at any cadence. The retry wrapper goes too: a transient failure now fails loud, and the next tick's floor reaches back past the lost window. This supersedes the window-anchoring half of #845; its cadence-cut half stands alone and needs no script support under anchor-based windows. - **`review-reviewers` Non-issues: 7 carve-outs → 3 structural rules** (designed no-ops at whatever layer catches them, designed silence, reviewer-role independence), with the closed menu folded into Gate 3's cost classification. Also: pre-commit's shellcheck now covers `plugins/**/*.sh` (it covered none of them), and the fake-`gh` test scaffold consolidates into `generator/tests/__init__.py` (five copies → one). ## Testing The scripts are exercised end-to-end under pytest with fake `gh`/`date`/`sleep` binaries, with the scripts' own jq filters doing the reductions: 21 cases for the poll scripts (false-green guards: >100-context page, null rollup, superseded runs, moved head), 11 for the window logic (anchoring, clamping, self-exclusion, loud failure), and the mention gate consolidation is pinned by the reworked `test_generate.py` assertions plus regenerated snapshots. Full suite green. The verify gate itself is still string-shape-tested rather than executed — extracting it to a `.sh` template like `review-gate.sh` is the natural follow-up, left out here to keep this diff reviewable. <details><summary>Review-hardening notes (later commits)</summary> An 8-angle verified code-review of the first commit found the substantive gaps in the new scripts themselves, all fixed and pinned by tests: the GraphQL query's one 100-node page could hide a red check past the cap (now routes to UNVERIFIED); a nullable `startedAt` could let a stale settled check run outrank its QUEUED replacement (a group with any non-terminal entry now reads pending); a transient API blip on a late poll iteration discarded the pending list the cap report names (the last usable rollup is kept); rerun discovery by job status misread fast reruns (now keyed on `run_attempt` advancing); and overlapping workflow prefixes double-counted runs (`unique_by`). The known remaining limit: latest-wins can't distinguish two independent runs of a group-less workflow on one SHA — documented in the script header and the skill, with the instruction to read the specific run's conclusion where that distinction must be exact. A further tend-review round on the PR added: explicit `--repo`/owner arguments on the three `gh` calls that leaned on cwd remote detection (with test assertions pinning the flags), a Step 1 sentence in `review-reviewers` covering a failing `list-recent-runs.sh` call (retry once, then record a coverage gap — the run still concludes green, so the next tick anchors past the span), and the zero-checks commit named in `poll-pr-checks.sh`'s exit-2 docs. </details> > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Superseded by #966, which pauses The window-anchoring half doesn't survive either: the workflow now runs on
|
2 participants
Anthropic quota exhaustion has now hard-failed the fleet twice in three days. The Aug 3 diagnosis on #808 pre-registered the trigger — "I've left this to a maintainer decision rather than opening a cadence PR — it's the first occurrence and the plan-vs-cadence tradeoff is yours. If it recurs it's worth landing the cron cut" — and the review-runs tracking entry carries the same condition. It recurred, an order of magnitude larger. This lands option 1 from that comment.
What happened in this window
review-reviewersfailed 15 consecutive runs × 5 matrix legs = 75 job failures across 14.5 hours (09:41Z Aug 4 → 23:10Z Aug 4), recovering on the first tick after the quota reset. Every leg dies ~4 s into theclaudestep with the same signature, read from the session-log artifacts:{"type":"rate_limit_event","rate_limit_info":{"status":"rejected","resetsAt":1785888000, "rateLimitType":"seven_day","overageStatus":"rejected","overageDisabledReason":"org_level_disabled"}}{"type":"result","is_error":true,"api_error_status":429,"num_turns":1, "result":"You've hit your weekly limit · resets 12am (UTC)"}resetsAt: 1785888000is 2026-08-05T00:00:00Z; the first green run is 30962483562 at 00:11Z. This is the seven-day quota, not the rolling session quota that caused the Aug 3 incident — the same capacity wall, hit on a longer accounting period, which is the harder one to ride out.Why cadence is the lever
Normalised over the 8 runs that actually consumed tokens (the other 15 were free failures):
At its designed rate — 24 runs/day —
review-reviewersalone projects to ~$252/day, against $79 for the entire rest of the fleet. Per-run cost is also climbing at flat cadence: $5.31 (Aug 3) → $7.86 (Aug 4) → $10.49 (Aug 5). Both drivers push the same way, and cadence is the one that can be changed deterministically.Correction (2026-08-08), in reply to "why does doing it less frequently save costs?": cadence does not cut cost proportionally, and an earlier comment on this PR projecting
$71/day was wrong to assume a flat per-run cost while the window triples. Per-leg cost measured across the matrix is **$1.12 fixed + $0.12 x runs surveyed**, so65% of a quiet run is per-tick startup and only that part is saved. The cut is **$213 -> ~$120/day (~40%)**, not the 3x the tick count implies. The session-count argument is unaffected: 120 -> 40 Claude sessions/day is the full 3x, and that is what saturates the seven-day quota. Full derivation.47 */3 * * *takes it from 120 to 40 Claude sessions/day. Outcome-acceptance signal — was a bot PR merged, was a comment answered, did a human push back — evolves over hours to days, so little analytical value is lost at 3-hourly. The workflows that lose when the quota is gone are the user-facing ones: on Aug 3 the same exhaustion dropped the nightly sweep entirely plus two PR reviews and two mention-handles.The window-anchoring change is a prerequisite, not a drive-by
list-recent-runs.shonly anchors its completion window when the cron matches^([0-9]+) \* \* \* \*$. Any other shape falls through to a now-anchored 1-hour window. Verified against the actual regex:So cutting the cron alone would have left every run analysing one hour out of every three — a silent two-thirds coverage hole, reported as an all-clear. This change generalises the detection to
MM */N * * *and derives the window from the cron period.Nis only accepted when it divides 24. Cron's*/Nrestarts at hour 0 each day, so a step like*/5fires at 0,5,10,15,20 and then wraps after 4 h — no constant period, and a floor computed from one would under-reach across midnight. Those shapes keep falling through to the now-anchored window, same as before.Verification
Hourly behaviour is unchanged.
cron_hour_stepis 1 for the hourly form, soperiodis 3600 and every expression reduces to the original arithmetic. Checked exhaustively rather than by inspection — old vs new tick computation over 24 hours × 3 minute offsets × 4 cron-minute values, plus month/quarter boundaries:Consecutive 3-hourly ticks tile exactly — no gap, no overlap, including the midnight wrap:
Non-conforming shapes still fall back:
47 */5 * * *,17 6 * * *,*/15 * * * *→ now-anchored 1 h.shellcheckclean;bash -nclean; the workflow YAML parses with the new cron.The 6 h dropped-tick reach-back cap is deliberately left as a literal. At hourly that is unchanged; at the 3-hourly step it is two full periods, so a single dropped tick is still recoverable — and leaving it untouched keeps this diff off the lines #838 rewrites.
Overlap with #838
#838 also edits
list-recent-runs.sh, in the dropped-tick recovery block (anchor on the last successful run; raise the fetch limit). Different concern, and the two are complementary — that one widens the window after an outage, this one sets the window's period. The edits are textually adjacent, so whichever lands second wants a small rebase; neither depends on the other.Gate assessment
failurehaving silently not done their work.What this does not address
The failures themselves are annotated only as
claude -p exited non-zero (exit=1)with empty stderr — the 429 text sits in the stream-json the action already has, and is discarded on the non-zero-exit path. That is #818, open since Aug 3, and it is why diagnosing this needed a second round of manual session-log archaeology. Outage tracker #831 now carries 78 rows and no cause. Cadence reduces how often the wall is hit; #818 makes it legible when it is.