feat(rate-limit): let a maintainer approve past the spike limit

[max-sixty]

Problem

The spike rate limit is a circuit breaker with no reset: once tripped, the only cure is the UTC rollover. A legitimate nightly sweep — six PRs filed between 00:20 and 00:24 — pushed the day's count to 16 against a limit of 15, and every tend-review run from 11:47 UTC onward died at the preflight. Six hours with no reviews, and no signal anywhere a person would look: a failed preflight writes an annotation onto a job nobody opens. #851's own review was one of the ones stranded.

Two separable gaps: no notice that the bot had stopped, and no reset short of waiting for midnight.

Solution

On a spike trip the preflight files or reopens a tend-rate-limit issue listing the runs it refused. Opening it is the notice. Closing it is the approval, and doubles the ceiling for the rest of the UTC day; each further close doubles again, so the breaker re-arms after use rather than switching off.

A doubling rather than a flat increment because the ceiling it lifts is itself proportional — 10 + 2 * daily_avg is 15 here but 40 on a repo filing 15 a day, where a flat bump would mean closing the issue all afternoon. At the formula's floor the two coincide (10 doubles to 20), so they diverge only where a flat bump stops working.

The burst limit stays non-resumable: ten PRs in twenty minutes is a loop, not a busy day, and there is nothing there to wave through. ABORT is set true at three points and never cleared, so an approval cannot release a burst trip — and a burst trip files no issue at all, since one offering to lift the ceiling would promise a recovery that closing it cannot deliver.

Approval is a check, not an instruction the bot could be talked out of. The preflight counts only closes whose actor is neither the bot nor a GitHub App. GitHub admits only an issue's author or a triage/write collaborator to close it, and the bot is the author, so what remains is exactly the maintainers — no allowlist to keep.

Three things pin that down, because the bot holds issues: write and could otherwise arrange an approval for itself:

• The anchor is an issue the preflight filed — lowest-numbered, authored by the bot, under the pause title. A label alone pins neither author nor title, so without this, labelling any issue would nominate it.
• An approval must postdate the label going on. Moving the label onto an issue closed earlier would otherwise import that close as an approval. On a real pause issue the label is applied at creation, so nothing genuine is excluded.
• The actor is matched by numeric id, and Apps are excluded by type. A rename doesn't shake the first; github-actions[bot], which a workflow's own GITHUB_TOKEN acts as, is caught by the second.

Identity comes from the credential rather than from config. The preflight runs as the bot, so gh api user yields the login the counts need and the id the approval filter compares. The configured name fed both before, and every way it could be wrong failed open: stale after a rename, the counting queries match nothing and the limit never trips at all; empty, the approval filter reduces to "login is not the empty string" and the bot's own close approves it. The preflight now publishes both facts for the rest of the job, replacing the Resolve bot ID step that re-derived the id from the configured name.

Why the bookkeeping labels moved into the generator

The rate-limit issue collects one comment per refused run, and tend-mention skipped only tend-outage — so each row would have re-triggered mention, whose handle job trips the same limit and appends another row. That is the self-sustaining loop the existing guard exists to stop, reachable again through a label it did not know about.

BOOKKEEPING_LABELS in workflows.py behind a not_bookkeeping() macro gives triage and mention one list, so the next such label reaches both at once. Rendered as contains(…) == false, since a value opening with ! is a tag indicator to YAML and only parses inside a block scalar. The worker keeps its own copy for now — unifying it needs a story for issues already filed without the label.

Testing

Ten cases in test_shared_steps.py cover the ceiling arithmetic, day-scoping, both actor exclusions, the rename case, the refusal when identity can't be read, and reopen-rather-than-duplicate. The fake gh runs the script's own --jq expression against a fixture with real jq, because that filter is the behaviour under test — a fake that pre-filtered would assert nothing.

Verified by mutation rather than by passing. Each of these reddens exactly the case named and nothing else: deleting the two actor conditions (bot, rename, App), putting the filter back on login (rename), dropping the label-ordering rule (relabel), dropping the title filter or the --author scope (foreign anchor), reverting the shared predicate to label-only (foreign anchor and reconciler), and removing the day floor (yesterday's approval).

run_issue_matching is the single definition of which issues belong to a record — label, author, title. The anchor lookup and the duplicate reconciler are the same call, so they cannot come to disagree about which issue is real.

Not yet exercised against real GitHub. No run has filed a pause issue, reopened one, or been approved end to end. tend-agent/tend-integration exists for that and would be worth a pass before this is relied on.

Deferred

TODO.md records the automatic re-runner for work a trip refused, blocked on confirming tend's re-runs work at all — building one over a broken re-run path would move the symptom from "my review never came back" to "the recovery workflow ran and my review still never came back". Recovery meanwhile is one gh run rerun <id> --failed per row in the issue's table.

Also open: an approval cannot be revoked, since reopening doesn't remove the close event. Counting only closes since the most recent reopen by a person would give a "stop" that works inside the day; left out here because it adds complexity to a security-critical script.

This was written by Claude Code on behalf of max-sixty

[tend-agent]

Three things worth a decision before this lands.

The approval anchor is something the bot can move. run_issue_canonical "$PAUSE_LABEL" all takes the lowest-numbered issue carrying tend-rate-limit in any state, with no constraint on author or title — and every generated workflow grants issues: write (the permissions() macro in macros.yaml.j2 emits issues: write), so the agent can put that label on any issue in the repo. Label one a maintainer already closed earlier today and the preflight reads that close as an approval: actor.id != BOT_ID, actor.type is "User", created_at is today — every condition satisfied, ceiling doubles, and no pause issue was ever closed. The same thing happens by accident if a maintainer labels an old issue for tracking, which then outranks the real pause issue on the lowest-number rule. Impact is bounded (one doubling per human close on whatever issue got anchored), but docs/security-model.md states the property without qualification — "The bot cannot approve itself even if a prompt injection tells it to". Constraining the pause lookup to an issue the preflight itself filed — author is the bot, title is PAUSE_TITLE — and requiring the close to postdate that issue's creation would close both the deliberate and the accidental path, and keeps the "no allowlist" property.

#857 addresses the same gap by a different route. It changes the Report failure gate to failure() && steps.security.outcome != 'failure' and cites this exact incident — "a rate-limit preflight abort held every workflow on this repo for a full UTC day without filing a thing". With both merged, a spike trip files a tend-rate-limit issue and a tend-outage issue, and that outage create counts into the next run's TODAY_POSTS. They also both rewrite report-failure.sh's header block and touch the same regions of both action files, so whichever lands second needs a rebase either way — worth deciding now whether #857 should carve the rate-limit abort back out.

bot_name is now dead as an action input. After this change inputs.bot_name has no reference left in either claude/action.yaml or codex/action.yaml, but it stays required: true in both and macros.yaml.j2 still renders bot_name: <<cfg.bot_name>> into every agent_step. Given the repo's no-backward-compatibility rule this reads as a half cut-over: dropping the input from both actions and the bot_name: line from the with: block would finish it. (The config key itself stays — the templates use cfg.bot_name in a dozen other places.)

Smaller: when the burst limit trips in the same run as the spike, the pause block still runs — it reopens the issue and appends a row, so a maintainer's close gets undone by the next refused run while the thing actually refusing the run is the non-resumable burst check, which closing the issue cannot lift. The issue body promises the opposite ("Closing this issue approves the volume and doubles the ceiling"). Gating the block on ABORT still being false, or naming the burst trip in the appended row, keeps the notice honest.

Testing looks good — running the script's real --jq against a fixture is the right call for a filter that is the security property, and the mutation check described in the PR body is the verification I'd have asked for.

[tend-agent]

The anchor fix reads right, and it holds up the way the PR body describes — deleting the label-ordering rule reddens exactly the relabel case. Two gaps in what the tests pin, and one place the tightening didn't reach.

Neither half of the new anchor is pinned by a test. pause_issue narrows the lookup three ways — the label, --author "$BOT", and title == PAUSE_TITLE — but only the label-ordering rule has a mutation-killing test. Dropping the title select(…) from the --jq, and dropping --author "$BOT" from the gh issue list line, each leave all 14 green. That's the headline property of this commit, and it's the rule the fake gh can't catch by accident: emit runs the script's real --jq against PAUSE_ISSUES_JSON, but every fixture _approve writes already carries PAUSE_TITLE, and the fake ignores --author entirely. The case suggested below fails under both mutations and passes unmutated.

The day-scoping test now passes for a reason other than its name. _approve defaults labelled_at to 2026-01-02T08:00:00Z while _closed_event(…, day="2026-01-01") closes at 2026-01-01T09:00:00Z, so that close is excluded by the label-ordering rule rather than by the ${TODAY}T00:00:00Z floor. Replacing the floor with SINCE="" keeps all 14 green. Moving the label back a day restores the kill and leaves the relabel test as the only cover for the ordering rule.

The reconciler still treats the label as the identity. pause_issue demotes the label to a finding aid — author and title are what pin the issue — but run_issue_create_and_reconcile picks its keeper from gh issue list --label "$label" --state open with no author or title constraint. If any lower-numbered open issue carries tend-rate-limit (a maintainer tracking one, or the bot putting it there), the first trip files the pause issue and the reconciler closes it again as a duplicate of that issue, so the ::error:: points at something holding none of the refused-run rows and a maintainer's unrelated issue collects a wrong duplicate comment. Recovery still works — the next run finds the real, bot-closed and therefore unapproved issue through pause_issue and reopens it — but the notice ends up split across two issues. Reconciling on the same author/title predicate would keep the two rules agreeing about which issue is the real one.

[tend-agent]

The reconciler now agrees with the anchor, but nothing holds it there. Reverting run_issue_create_and_reconcile's list to its pre-commit form — gh issue list --label "$label" --state open --json number --jq 'sort_by(.number) | .[].number' — leaves all 15 green, and so does dropping either half on its own. That is the change this commit exists to make, and the comment tying the two lookups together (“Same predicate as run_issue_canonical”) is a promise the suite does not keep: the predicate lives in two textual copies kept in sync by hand, which is the drift a test is for. The case below fails under all three mutations and passes unmutated.

A shared run_issue_matching that both call would close it from the other side instead, if one copy appeals more than one test.