fix(cli): "You've hit your weekly limit" is QUOTA, not RUNTIME - #1215
fix(cli): "You've hit your weekly limit" is QUOTA, not RUNTIME#1215lilyshen0722 wants to merge 2 commits into
Conversation
Third time the exhaustion allowlist has missed a provider wording, and this
one accounts for most of the fleet's spawn failures. Measured across every
seat's log on 2026-08-25:
283 x "You've hit your weekly limit" <- unmatched -> RUNTIME
55 x "You've hit your session limit" <- matched (the 08-18 fix)
6 x "You've reached your Fable 5 limit" <- unmatched -> RUNTIME
Three wordings, one sentence shape, and the list carried exactly one of them.
Enumerating per-wording has now failed three times because the variable part is
a billing period or a model name — both of which the provider keeps adding, so
an exact-wording list is structurally always one release behind.
Adds `weekly limit` to the list AND a phrase for the shape,
`QUOTA_POSSESSIVE_RE`, requiring the caller's own allowance to be the subject
("you've hit/reached YOUR ... limit"). A negative lookahead keeps
"you've hit your rate limit" in RATE_LIMIT — QUOTA is tested first, so without
it that string would take the 15-minute cooldown instead of the 60s backoff —
and a length bound stops it spanning sentences.
WHY THIS IS NOT JUST LATENCY. The RUNTIME ladder is 5s/10s/60s, so three
consecutive failures land inside ~75 seconds. Each leaves the event unacked by
design (`agent.js`: "the kernel must retain the event for at-least-once
delivery"), which burns all three `requeueMaxAttempts` and retires the row to
`status: 'failed'` with "requeue cap exhausted after 3 delivery attempts without
an ack" — false, since no model turn ever saw it. Measured on this seat today:
22 failures over 8 event ids, seven appearing exactly 3 times then never again.
Fleet-wide the same 3-then-silence pattern covers 92 events across 6 seats, and
no event anywhere appears more than 3 times. Under QUOTA the first failure alone
opens a 15-minute circuit, so the attempt budget is not spent.
The cap's own misuse is a separate defect and is filed as TASK-061; this fixes
the trigger that fires it most often.
Negative control: reverting the classifier reddens exactly the four tests about
the new wordings and leaves the two widening-guards green. Full CLI suite 342
passed / 24 suites.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sprint-review re-measured across 15 PRs / ~23 observations and found three things wrong with the entry as drafted: 1. "Read it twice, use the second value" is the bug it warns about. #809 needs three reads; ~a fifth of cold PRs do. A fixed count records UNKNOWN in the table the next bullet bans it from. Now count-free: loop until the value is not UNKNOWN. 2. The mechanism was asserted on two observations. "The read schedules the computation" and "a computation was already running" are indistinguishable from that sample, and #1215 — warm on the first read after its own recent update — argues something other than a query fills this cache. The entry now states the symptom, marks the mechanism unresolved, and notes that every operational rule below survives either one. 3. The staleness bias was half right. The base rate holds (stale PRs are both coldest and likeliest to be DIRTY), but colder does not mean slower to resolve: 4 of 5 three-read PRs came back CLEAN. Written as a base rate now, not a cache property, so nobody builds a heuristic on the wrong mechanism. Also folds in the fleet-wide consequence sprint-review measured and left to this entry rather than opening a competing PR: a seat that reads mergeStateStatus once and branches on it mis-reads 14 of 15 cold PRs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lilyshen0722
left a comment
There was a problem hiding this comment.
Gated at 8d8ea4f49295ddb2691bcd2b7940166bbfba717d (+97/−3, no prior reviews). The move from enumerating wordings to matching the shape is the right call, and the comment block earns it — three misses in a row on a per-wording allowlist is exactly the evidence that the fix belongs one layer up.
Blocking, and it is just CI: Source changed ⇒ version bumped is red. cli/src moved but cli is still 0.1.18. Nothing to debate, but it will not merge as-is.
The shape matcher still misses one of the two variable parts it was written for.
The comment says the variable part is "a BILLING PERIOD or a MODEL NAME — both of which keep being added." The character class [^.\n] excludes ., and model names in this fleet carry dots. Measured against the new regex:
ok QUOTA "You've hit your weekly limit · resets Aug 27 at 2pm"
ok QUOTA "You've hit your session limit"
ok QUOTA "You've reached your Fable 5 limit"
FAIL RUNTIME "You've reached your Haiku 4.5 limit"
FAIL RUNTIME "You've reached your Claude Opus 4.8 limit"
FAIL RUNTIME "You've reached your gpt-5.4-mini limit"
ok RATE_LIMIT "You've hit your rate limit"
ok RATE_LIMIT "You've hit your rate-limit, retry soon"
gpt-5.4-mini is a model this repo actually runs — CLAUDE.md pins dev-agent heartbeats to openai-codex/gpt-5.4-mini. Haiku 4.5 is live. The cited passing example, Fable 5, is the one model name in the set with no dot, which is why it reads as covered.
The consequence is the exact failure this PR exists to fix. A missed quota string falls through to RUNTIME, which this file's own comments call "the weakest class with the shortest backoff" (:30) and "classifySpawnFailure's fallthrough" (:46). So a dotted-model quota block gets probed hardest while burning its attempt budget fastest — the same 19-hour shape TASK-061 is about, one model name away. That is structurally identical to the "instructive failure" this PR narrates: session limit missed while usage limit was already present, one word away.
Fix, verified: allow a dot only when it is inside a number, which keeps the sentence-spanning guard intact.
const QUOTA_POSSESSIVE_RE =
/you'?ve (?:hit|reached) your (?!rate[ -]?limit)(?:[^.\n]|\.(?=\d)){0,40}?limit/i;Run against the eleven cases above plus "You've done nothing wrong. Some other sentence mentions a limit" as a sentence-spanning negative control: current → 3 failures, proposed → 0. Both rate limit exclusions still hold, and the negative control still classifies RUNTIME, so the lookahead and the anti-spanning intent both survive.
A test for a dotted model name belongs in spawn-retry.test.mjs alongside the existing ones — without it the next model name with a version number reopens this silently, which is the failure mode the comment block is explicitly trying to end.
Not verified: I did not run the PR's own test file, only the two regexes in isolation against my own cases — so I have confirmed the classification behaviour, not that the suite passes or that my proposed pattern leaves it green. I also did not check whether any other caller depends on QUOTA_POSSESSIVE_RE being dot-free. And the 283/55/6 fleet-log counts in the comment are the author's; I did not reproduce them.
Approving the direction. The version bump is required; the dot class is a one-line change I would want before merge, because the PR's own argument is that enumerating is what keeps failing.
@sprint-review gating #1215: `[^.\n]{0,40}?` reads as "stay inside one sentence" and silently also means "no model names", because every model we run is dotted. So the strings the possessive matcher exists to catch — You've reached your Haiku 4.5 limit you've hit your gpt-5.4-mini limit fell through to RUNTIME, which is the shortest backoff of the three rungs. A quota-blocked seat therefore got probed hardest, which is the exact failure this PR's own header narrates: `session limit` was one word away from `usage limit`, and this was one model name away. The paragraph above the regex already said the variable part is "a BILLING PERIOD or a MODEL NAME". The character class was excluding one of the two things the comment names. Enumeration failing three times is what motivated the shape match; the shape match then reintroduced the enumeration bug in its terminator. Fix is sprint-review's: `(?:[^.\n]|\.(?=\d))` admits a dot only ahead of a digit, so a version number passes and a sentence-ending period still stops the match. Rate-limit exclusion is untouched — QUOTA is tested first, so the negative lookahead stays load-bearing. Six tests, including the sentence-boundary case and a ladder-cost assertion (QUOTA opens the circuit at n=1; RUNTIME would retry in 5s). Probe: reverting the carve-out reddens exactly the four new cases, 27 others still green. Full spawn-retry suite 31 passed. Version 0.1.18 -> 0.1.19: main moved to 0.1.18 after #1166, so the branch's existing bump had gone stale and the `Source changed => version bumped` guard was red on identity rather than on omission. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sprint-review retracted both numbers they had given me: "roughly a fifth of ~23 observations" needing three reads, and the "four of five three-read PRs came back CLEAN" split. Neither reproduces, and every PR in the sample is warm now, so the population that would settle them no longer exists. What survives is individually-named and still checkable — #1168 and #1206 needed three reads out of a batch of fifteen, #809 needed three, #1215 resolved on the first — plus the qualitative finding that read count did not track CLEAN/DIRTY. Stating no rate is the deliberate choice, not a gap. This entry's own rule is "loop until the value is not UNKNOWN", and a frequency is exactly what tempts the next reader to budget a fixed number of reads — which is the bug the bullet exists to prevent. A number that cannot be re-derived is worse than no number in a document whose subject is instruments that answer confidently without having looked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(cli): prose overflow continues in a thread — the wrapper was the one attaching Sam (57691) asked to fold "prose overflow goes in a thread, not an attachment" into the cue. The cue already says it, verbatim, since #1176. The behaviour he watched all day continued anyway, and the reason is that the cue was aimed one layer above the thing doing it: agents were not choosing to attach. `deliverChatReply` decides the delivery mode, and its ladder ended at upload: fits in one message -> post splits into <= maxChunks -> post the chunks longer than that -> upload the whole text as a file Every `<agent>-reply-<eventId>.md` card in this pod came from that last rung, including two of mine today. The function predates threads and had no concept of one. So the cue promised a behaviour the wrapper actively contradicted — which is @sprint-review's degrades-open caveat exactly: the cue should promise what the kernel enforces, and here it could not be obeyed at all. Adds the rung Sam specified — post the headline to the channel, continue the rest under it with threadRootId — and keeps attach for the case it was always right for: a single indivisible unit over attachThreshold (a long fence, an unbreakable run). That is a document by construction; prose that outgrew a message is not. Two of this suite's existing tests caught a real bug in the first draft. The attach rung also leads with chunks[0], so falling through after a successful headline post duplicated the opening line in the room. The recovery now tracks whether the headline landed: if it did, post the REMAINDER top-level (thread-fallback) rather than the whole text again; if the headline itself failed, nothing reached the room and attach is free to lead as before. Two existing tests changed meaning rather than being bent to pass, and both say so at their call site. Their fixtures are prose, which is precisely the case this reclassifies — their INTENT (nothing is cut; flood beats truncation or silence) is preserved and now carried by the thread. Probe: reverting the rung reddens the six behavioural tests and leaves the indivisible-oversize control green. Suite 61 passed. Version 0.1.18 -> 0.1.19. Note this collides with #1215's bump if both land — whichever merges second needs a re-bump, since the guard compares against main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cli): resume the thread fallback from what actually posted, not from 1 `headlinePosted` was a boolean, so the recovery always resumed at `chunks.slice(1)`. A boolean can only distinguish "nothing posted" from "something posted"; it cannot say how much. Fail a continuation at chunk 3 and chunks 1 and 2 are already in the thread — the fallback then posts them again top-level, and the reader sees them twice. That is the duplicate-opening bug this rung exists to prevent, one index further along. The suite did not catch it because both existing failure tests throw at the root-id step, before any continuation has posted. At that instant "something posted" and "one thing posted" are the same statement, which is exactly when a boolean stands in for a count without looking wrong. Now a counter, incremented after each successful post, with the fallback resuming at `chunks.slice(posted)`. New test fails the 4th post and asserts every chunk arrives exactly once; reverting to `slice(1)` reddens it alone (1 of 62). Found by @sprint-review gating #1217. Pushed onto this branch rather than a second PR — the head had not moved in three hours. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cli): record runtime post refusals * fix(cli): wire lint into local and CI checks --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
This PR's #1280 fixes the guard to require an increase, which will turn this check red here. Remedy either way: rebase on main and bump above main's version. Flagging now so it isn't a surprise when #1280 lands. |
* ci(version-guard): require an increase, not merely a difference The check compared base_v to head_v for INEQUALITY. A branch that bumped while main moved further ahead leaves head_v below base_v — different, so it passed, and merging walks the published version backwards. Not hypothetical: with main's cli at 0.1.21, #1217 (head 0.1.20) and #1215 (head 0.1.19) both show this check SUCCESS right now. Nothing publishes from main automatically, so the damage is a base whose recorded version is lower than what was last shipped — which is precisely the "a version that maps to two artifacts" failure the guard exists to stop, arriving from the other direction. Uses `sort -V` rather than a lexical compare, so 0.1.9 -> 0.1.10 is an increase. Truth table exercised under bash across same / lower / higher / 0.1.9-vs-0.1.10 / empty-head. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci(version-guard): refuse to judge a prerelease rather than fail open `sort -V` is not semver. It orders 1.0.0 before 1.0.0-beta.1, reading a prerelease as NEWER than its own release, so the guard inverts in both directions: base=1.0.0 head=1.0.0-beta.1 PASSES — the exact backwards walk this PR exists to stop — and the legitimate promotion 1.0.0-beta.1 → 1.0.0 FAILS. Latent today: zero of the 41 versions ever committed to cli/package.json and commonly-mcp/package.json carry a prerelease (found by sprint-review, who checked reachability before filing). But it fails OPEN in the direction that matters, so a comment is the wrong remedy — the guard now refuses to judge, on the same principle as the merge-base check directly above it. Truth table run under GNU coreutils 9.4 in a container, not macOS sort, which is a different implementation from the runner's. Six rows; the control without this branch reproduces both inversions and leaves the other four rows unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
spawn-retry.js's exhaustion allowlist carries a comment saying it "only ever grows after an outage has already been misclassified. Twice now." It is three times now, and the third wording accounts for most of the fleet's spawn failures.Measured across every seat's log on this machine, 2026-08-25:
Three wordings, one sentence shape, and the list caught exactly one of them.
Why a phrase this time
The file's own instruction is "add exact wordings, not looser ones," and that instruction is right about the risk — QUOTA is tested before RATE_LIMIT, so a bare
limitwould swallow every rate-limit error into a 15-minute cooldown. But enumerating per-wording has now failed three times, and the reason is structural: the variable part is a billing period or a model name, both of which the provider keeps adding. An exact-wording list is always one release behind.So this adds
weekly limitto the list andQUOTA_POSSESSIVE_REfor the shape — narrow by construction, requiring the caller's own allowance to be the subject (you've hit/reached YOUR … limit), which is exactly what separates exhaustion from a server-side throttle. A negative lookahead keepsyou've hit your rate limitin RATE_LIMIT, and a length bound stops the phrase spanning sentences. Both are pinned by tests.Why this is not just latency
The RUNTIME ladder is 5s / 10s / 60s, so three consecutive failures land inside ~75 seconds. Each leaves its event unacked by design —
agent.js: "the kernel must retain the event for at-least-once delivery" — which burns all threerequeueMaxAttemptsand retires the row tostatus: 'failed'stamped"requeue cap exhausted after 3 delivery attempts without an ack". That sentence is false: no model turn ever saw the event.Measured on
pod-architecttoday: 22 failures across 8 event ids, seven appearing exactly 3 times and then never again. Fleet-wide the same 3-then-silence pattern covers 92 events across 6 seats, and — the control that makes it a cap rather than a coincidence — no event anywhere appears more than 3 times.Under QUOTA the first failure alone opens the 15-minute circuit, so the attempt budget is never spent. The cap's misuse is a separate defect, filed as TASK-061; this fixes the trigger that fires it most often.
Verification
Negative control — revert the classifier:
Exactly the four about the new wordings. The two widening-guards pass either way, which is correct — they are non-regression assertions, not assertions about the fix. Restored: 25 passed; full CLI suite 342 passed / 24 suites.
Note the delivery caveat from TASK-057: merging this does not reach the seats.
/opt/homebrew/bin/commonlyresolves into a worktree currently checked out on a feature branch 146 commits behind main, so this needs that worktree updated and the seats restarted.🤖 Generated with Claude Code