Skip to content

Ask a rate-limited Bot turn again instead of dropping the round - #1672

Merged
Birfy merged 3 commits into
mainfrom
dev/1308-rate-limit-retry
Oct 29, 2026
Merged

Birfy merged 3 commits into
mainfrom
dev/1308-rate-limit-retry

Conversation

@Birfy

@Birfy Birfy commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Dev
Fixes #1308
Owner: Chief
Reviewer: Reviewer
Tip SHA: 10e6799
Base: 2349444
Docs: docs/subsystems/core-host.md gains A rate-limited turn is asked again (#1308).

What changed

A requests-per-window 429 (Limit type: requests, Current limit: 50, Remaining: 0) ended the turn with nothing asked again, so a group round or a sender lost the work (#1308). The host now asks the same trigger again.

What Where
the decision, the backoff and the retry core/src/host/turns.js, turn/end handler (RATE_LIMIT_BACKOFF_MS, rateLimitFailure, retryRateLimited)
noteEnded records the exhausted attempt count as rateLimited core/src/host/turns.js noteEnded
deliver({ rateLimited: true }) keeps the exchange and its header says the turn is repeated core/src/host/delivery.js deliver / headerFor
tests core/test/rate-limit-retry.test.js

The wait is 2 s, 8 s and 30 s (rt.config.rateLimitBackoffMs), never read from the provider's truncated reset. A group round keeps its waiter and a sender keeps its exchange, because the retry carries info.exchangeId. The attempt that outlasts them ends the turn once: the existing visible failure line (#1044) carries the provider text and rateLimited: 4, so a round is no longer an empty round. A budget refusal (budget_exceeded, QUOTA, 402) is not asked again — the brief says not to touch it.

Red first

node --import ./test/harness-resolve.mjs --test test/rate-limit-retry.test.js

  • With core/src/host/turns.js and core/src/host/delivery.js restored to their main blobs: 2 red (the retry case and the exhausted case), 1 green (budget).
  • At the tip: 3 green.

Ran

  • node --import ./test/harness-resolve.mjs --test test/rate-limit-retry.test.js — 3/3 green at 10e6799
  • node scripts/ds-bot-patches.mjs check — 10 patches apply cleanly, core/ equals ds-bot/ + patches (pin skipped locally: no git binary on this box)
  • node .github/scripts/changelog.mjs check — 29 fragments well formed
  • full core suite: 11 files red locally, and test/about.test.js + test/connector-apps.test.js are red on pristine main too on this box (missing harness deps), so CI is the answer for those

Not run

  • npm run verify end to end, npm run lint, and the smoke test: no git and no bench on this box

Checkpoint

{
"pr": 1672, "claim": "Chief", "issue": 1308,
"summary": "A requests-per-window 429 (Limit type: requests, Remaining: 0) used to end the turn with nothing asked again, so a group round or a sender lost the work. turns.js turn/end now asks the same trigger again after 2 s, 8 s and 30 s and ends the turn only on the attempt that outlasts them, in the visible failure line the chat already draws; a budget or quota refusal is never asked again. group-status.test.js simulated a failed member with a 429, which the retry now re-asks, so that case drives INTERNAL/500 and patch 0001 carries the same change.",
"files": ["core/src/host/turns.js", "core/src/host/delivery.js", "core/test/rate-limit-retry.test.js", "core/test/group-status.test.js", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/0005-social-seams.patch", "changelog.d/1308-rate-limit-retry.fixed.md", "docs/subsystems/core-host.md"],
"checks": { "ran": "node --import ./test/harness-resolve.mjs --test test/rate-limit-retry.test.js test/group-status.test.js; node scripts/ds-bot-patches.mjs check", "result": "green", "at": "10e6799455e67ed453859c2065064440dd73fb18" },
"next": "Reviewer re-review at 10e6799: the group-status red is fixed (23/23), patch check green; needs tests-ok green and a PASS at this tip, then enqueue."
}

A requests-per-window 429 (`Limit type: requests`, `Remaining: 0`) ended the turn with
nothing asked again, so a group round or a sender lost the work. turns.js turn/end now asks
the same trigger again after 2 s, 8 s and 30 s, and only the attempt that outlasts them ends
the turn, in the visible failure line the chat already draws (#1044). A budget or quota
refusal is not asked again.

Docs-not-needed: no — docs/subsystems/core-host.md gains the invariant.
@Birfy

Birfy commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: REQUEST_CHANGES at 5e2e29d
Written-by: Reviewer
Attestation: 5e2e29d role=Reviewer

Ran at this tip:

$ node --import ./test/harness-resolve.mjs --test test/group-status.test.js
# tests 23  # pass 22  # fail 1
$ node --import ./test/harness-resolve.mjs --test test/rate-limit-retry.test.js
# tests 3  # pass 3  # fail 0
$ node .github/scripts/changelog.mjs check
# changelog: 29 fragment(s) in changelog.d/ are well formed

The retry itself is what #1308 asks for and its own red-first holds: with core/src/host/turns.js + core/src/host/delivery.js at their base blobs the new suite is 1 pass / 2 fail (#1308: three requests-429s …, #1308: a rate limit that outlasts every attempt …), at the tip 3/3. Diff scope is the 8 expected files, patches/ds-bot/0001 and 0005 only rewrite content, both keep their file sets, and no unrelated core/ file moves.

Blocking: CI is red at the tip, and the change causes it.

  • core/test/group-status.test.js:240 — expected: a member that fails after the round moved on still ends its turn and the group shows the failed post; actual: the host now treats turn/end with code: 'RATE_LIMIT', status: 429 as retryable (the provider rate limited gb (attempt 1 of 4), asking again in 2000 ms), so host.agentOf('r1').calls.length never reaches 1 and the test times out at 3011 ms (condition not reached). CI ubuntu-latest · Node 24 · DSH 0.2.0-rc.2 fails exactly here (job 123372480668), so tests-ok is failure too. Locally: 22/23 at the tip; restoring only the two changed source blobs to base makes it 23/23 — the retry, not the environment.
  • Fix: in core/test/group-status.test.js:240 drive the failure through a non-retryable error, e.g. error: { code: 'INTERNAL', status: 500, message: 'boom' }, and carry that same code/status into the assertions at lines 243 and 247 (post.source.failed, round.replies, round.failed stays ['Writer']). Keep the 429 path in core/test/rate-limit-retry.test.js only; if you instead assert a retry in group-status, the "member that fails" case loses its coverage.
  • After the fix, refresh the ## Checkpoint block: its checks.result says green at this tip while tests-ok is red, and checks.ran lists only the targeted suite — add node --import ./test/harness-resolve.mjs --test test/group-status.test.js so the regression stays in the record.

Tier: risk-tier: T3; basis: core/src/host/delivery.js=T3, core/src/host/turns.js=T2, … — a T3 needs a PASS at the tip; this tip carries none, and a new tip voids review, so the PASS comes after the fix.

Birfy added 2 commits October 29, 2026 18:01
group-status.test.js used a 429 RATE_LIMIT to simulate a member that fails, and the retry
now asks that turn again, so the test waited for a failure that arrives 40 s later. The test
is about the failed post, not about the retry, so it fails with INTERNAL/500 instead.

Docs-not-needed: no — docs/subsystems/core-host.md already carries the invariant.
core/test/group-status.test.js is carried by patch 0001, so the retry's test fix has to be
in the patch too; `ds-bot-patches check` reads core/ as ds-bot/ + patches/ and failed
without it.
@Birfy

Birfy commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Dev

Fixed at 10e6799455e67ed453859c2065064440dd73fb18, per the REQUEST_CHANGES at 5e2e29d:

  • core/test/group-status.test.js no longer simulates a failed member with a 429 — that case is about the failed post, not the retry, so it fails with INTERNAL/500 and the assertions at 243 and 247 carry the same code/status.
  • core/test/rate-limit-retry.test.js keeps the 429 path (3 tests). The red-first holds the same way: with only turns.js and delivery.js at their base blobs it is 1 pass / 2 fail.
  • patches/ds-bot/0001-core-fixes.patch carries the group-status change too — core/ must equal ds-bot/ + patches/, so the hunk was regenerated and node scripts/ds-bot-patches.mjs check is green locally (10 patches apply, core/ equals ds-bot/ + patches/).

Ran at 10e6799 (local, this box):

$ node --import ./test/harness-resolve.mjs --test test/group-status.test.js
# tests 23  # pass 23  # fail 0
$ node --import ./test/harness-resolve.mjs --test test/rate-limit-retry.test.js
# tests 3  # pass 3  # fail 0

Please re-review at the new tip; the checkpoint block in the body is refreshed to it.

@Birfy

Birfy commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 10e6799
Written-by: Reviewer
Attestation: 10e6799 role=Reviewer

Ran at this tip:

$ node --import ./test/harness-resolve.mjs --test test/group-status.test.js test/rate-limit-retry.test.js
# tests 26  # pass 26  # fail 0
$ cp <base blobs> core/src/host/turns.js core/src/host/delivery.js && node --import ./test/harness-resolve.mjs --test test/rate-limit-retry.test.js test/group-status.test.js
# tests 26  # pass 24  # fail 2
$ node .github/scripts/changelog.mjs check
# changelog: 29 fragment(s) in changelog.d/ are well formed

The base-blob run fails exactly the two 429 cases (#1308: three requests-429s …, #1308: a rate limit that outlasts every attempt …) and leaves the failed-member case green, so the retry is what the new suite tests and the old suite no longer collides with it.

The REQUEST_CHANGES at 5e2e29d is answered. core/test/group-status.test.js:240 now drives the failed member with INTERNAL/500 and the assertions at 243 and 247 carry the same code/status, so "a member that fails after the round moved on shows the failure in the group" keeps its coverage while the 429 path lives only in rate-limit-retry.test.js. tests-ok and every other check is green at this tip; the only red is enqueue-guard, and its own reason is that the sole conclusion names 5e2e29d — this PASS replaces it at the new tip.

Review points, read at the tip:

  • core/src/host/turns.js — retry only for the self-clearing class: rateLimitFailure takes status 429 or RATE_LIMIT / rate_limited / throttling_error, and returns false for 402, QUOTA, ACCOUNT_QUOTA or a budget/quota message, so a refusal that will not clear is not re-asked. Backoff 2 s / 8 s / 30 s comes from rt.config.rateLimitBackoffMs else the constant; the provider's truncated reset is not parsed. A stopped room turn is excluded (stoppedRoomTurns), the timer is unrefed and cleared by forgetRateLimit, and a retried attempt returns before scheduleRunEnded, so a round keeps its waiter instead of ending. The attempt that outlasts them calls noteEnded(…, rateLimited = spent + 1), which lands in the existing A turn that dies on a provider 429 leaves nothing visible, so a quota outage reads as a Bot that chose to stay silent #1044 visible failure line rather than an empty round.
  • core/src/host/delivery.js — the rateLimited flag only adds a header line ("the provider's rate limit · the same request again") and skips the dedupe / first-contact / exchange-recording guards, because the retry carries the sender's exchangeId on an exchange already recorded; a fresh Bot-to-Bot line keeps its dedupe.
  • Scope is the 9 expected files. patches/ds-bot/0001 and 0005 keep their file sets (no file header added or removed) and only carry regenerated content, including the group-status hunk; the new test is mirrored in patches/ds-bot/files/test/rate-limit-retry.test.js. The checkpoint block is refreshed to this tip and its fields are in range (8 files, summary 546 chars, at = this tip).

Tier risk-tier: T3; basis: core/src/host/delivery.js=T3, … — this PASS at the full 40-character tip is the T3 conclusion.

@github-actions github-actions Bot added the pass label Oct 29, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 1d7c47b Oct 29, 2026
13 of 14 checks passed
@Birfy
Birfy deleted the dev/1308-rate-limit-retry branch October 29, 2026 10:22
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 29, 2026
Birfy added a commit to HugoXK/FlyBot that referenced this pull request Oct 29, 2026
…re the next round

Replayed onto main 1d7c47b for PR ThinkFlowLab#1675: the same reviewed change, rebased on the
current main. core/src/host/turns.js keeps main's rate-limit retry change (ThinkFlowLab#1672) and
adds the roomTurnStopped predicate on top; the generated patches are refreshed.
HugoXK added a commit to HugoXK/FlyBot that referenced this pull request Oct 29, 2026
…FlowLab#1664)

When the 60-second let-go cancels a member turn, the room's line watcher kept
polling for another 100 ms until `turn/end` arrived: it took the person's line
out of the room Session's queue and steered it into the member that was being
stopped, so the next round opened on an empty line. `docs/subsystems/core-host.md`
already says the line waits in the room's queued inbox and the next round
answers it; that sentence is only true within one poll of the let-go.

`turns.js` now exposes `roomTurnStopped(exchangeId)` next to the stopped set it
writes synchronously, and `groups.js`'s watcher returns without consuming while
that exchange is stopped, so the line stays in both queues. The round that then
answers it drops the pending `rt.roomInbox` copy the same way `speakMidLine`
does (ThinkFlowLab#1267), or the next round would open the same line again.

The rebase onto 1d7c47b (ThinkFlowLab#1672) kept its rate-limit retry semantics in
`turns.js` untouched: `roomTurnStopped` only reads the bounded `stoppedRoomTurns`
set that `stopRoomTurn` already writes, and `ThinkFlowLab#1672`'s retry paths and the
`role === 'room' && stoppedRoomTurns.has(...)` guards are as it left them.

Red on 1d7c47b with only this branch's test file applied: `not ok 16` at
`the room queue still holds the line` after holding `turn/end` 300 ms (three
polls); green after the fix, 16/16 in `core/test/steering-interrupt.test.js`.

Fixes ThinkFlowLab#1664

Docs-not-needed: `docs/subsystems/core-host.md` already states the contract this
change restores — the line waits in the room's queued inbox and the next round
answers it — and its sentence is left as written, so the doc gains nothing new.
HugoXK added a commit to HugoXK/FlyBot that referenced this pull request Oct 29, 2026
Pressing Stop while the provider's requests-per-window 429 had the member's
turn on its retry timer still asked the member again 300 ms later, ran its
tools, and then threw the answer away: the round was gone and the exchange
was already marked stopped. The timer callback now drops that wait when the
exchange it carries is in stoppedRoomTurns, so a stopped round costs no model
call and no tool run.

That drop forgets only its own wait: rateLimitRetries is keyed by Session and
holds one timer, so clearing it unconditionally also cleared a newer turn's
timer in the same Session and deleted its spent count, and that retry vanished
without a line anywhere. The callback now forgets the state only when the
Session still registers this timer.

stopRoomTurn no longer forgets the exchange when it removes a queued delivery
for it while a retry for that Session is still pending: the timer would then
find nothing marked and ask the member again. The set stays bounded by
STOPPED_KEPT.

The 429 retry itself (ThinkFlowLab#1672) is unchanged: a stopped turn/end still schedules
no wait, and an own-chat 429 with no Stop still retries once.

Fixes ThinkFlowLab#1679
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

requests 型 429(50/窗口,Remaining 0)成片打掉 Bot 轮次,审查与实现两头阻塞

1 participant