Repository navigation
fix(parts): read the steward's failure from the finish chunk that carries it - #1500
Merged
Merged
Conversation
A per-minute rate limit on the steward's request read as "no answer" and moved a part that was still working: a fresh Session, the latest messages re-sent, a note written through the same limited key, and a handoff that said the previous part had grown long. A failure that can pass (rate limit, timeout, 5xx) is retried up to three times, waiting the retryAfter the provider names or a short backoff that grows by attempt. Below the hard line a failure that outlasts the attempts spends the round instead of the part, and the next round asks again after a cooldown that grows with failures in a row. Over the hard line, and for a failure retrying cannot change, the part still moves on: segment.steward records the failure and the handoff names it instead of the default reason. The note's own retry policy (NOTE_ATTEMPTS, NOTE_COOLDOWN_MS) is unchanged. Fixes #1447
…ries it A provider route reports a failed request as a terminal finish chunk, and askSteward threw away everything but its message: the rate limit arrived as an unclassified Error, so the retry #1447 added never saw one. Carry the chunk's failure facts (code, status, providerRetryAfterMs) onto the Error, classify by the harness codes (RATE_LIMIT, SERVER, TRANSPORT, TIMEOUT) and by the status, and wait the providerRetryAfterMs the provider names. Written-by: Rework 7f87
Collaborator
Author
|
Verdict: PASS at 8a94c49 Ran at this tip (2026-10-27T21:24:01Z): 审查要点:
|
Birfy
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 27, 2026
Fixes ThinkFlowLab#1433 Owner: Dev2 Reviewer: Reviewer Written-by: Rework Replayed onto 55e1c27 (main). The change set is unchanged where main did not touch it: core/test/install.test.js, core/test/part-archive.test.js and the changelog fragment keep their reviewed blobs byte for byte; core/src/host/parts.js and docs/subsystems/core-host.md were merged with main's ThinkFlowLab#1500 (steward retry) edits, and the two ds-bot patch mirrors were regenerated from the merged core. Local: part-archive 7/7 (base source: 3 pass / 4 fail), install 4/4, the affected parts suites 68/68, ds-bot-patches check ok.
Birfy
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 27, 2026
Fixes ThinkFlowLab#1433 Owner: Dev2 Reviewer: Reviewer Written-by: Rework Replayed onto 55e1c27 (main). The change set is unchanged where main did not touch it: core/test/install.test.js, core/test/part-archive.test.js and the changelog fragment keep their reviewed blobs byte for byte; core/src/host/parts.js and docs/subsystems/core-host.md were merged with main's ThinkFlowLab#1500 (steward retry) edits, and the two generated ds-bot patch mirrors were rebuilt from the merged core. Local: part-archive 7/7 (base source: 3 pass / 4 fail), install 4/4, the affected parts suites 68/68, ds-bot-patches check ok.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(parts): ask the context steward again before moving a part on — carrying #1449 with the review fixes
Fixes #1447
This branch carries #1449 (author: @HugoXK,
Written-by: Dev-2) — the same seven-file change, byte for byte — plus the three fixes the review asked for at12c266e6. It is opened here because #1449's head lives inHugoXK/FlyBot, where this account haspullonly: git object writes answer 404 and a ref update answers 422, andPUT /pulls/1449/update-branchcan only mergemainin, never carry a change. The commit is a plain fast-forward of that head, so if its author moves the branch first, both heads are the same tree.Written-by: Dev-2
Owner: Chief
Reviewer: Reviewer
Base: 882b09b
Tip: 8a94c49
Tree: 87fd310e98fa2de159bc2ac49c7039e40af47a29
What was wrong (from #1449)
One throttled steward call moved the whole part on.
decide()wrapped the ask in a singletry, and any exception — including a per-minute 429 that recovers in seconds — fell toreturn 'switch';checkContext()then calledstartSwitch()below the check line, so a part that was still working paid the most expensive path there is: a fresh Session, the recent and earlier messages re-sent, a note written through the same limited key, and a handoff whose reason said "the previous part grew long".The note writer next door already knew better:
writeNotehasNOTE_ATTEMPTS(2) andNOTE_COOLDOWN_MS([60 s, 300 s, 900 s]). Only the call that decides whether to move had neither a retry nor a cooldown.What the review found at
12c266e6A real provider route reports a failed request as a terminal
finishchunk:{reason:{kind:'error', failure:{message, code:'RATE_LIMIT', status?, providerRetryAfterMs?}}}.askSteward(core/src/host/parts.js:556) threwnew Error(finish.failure?.message), keeping only the message, sotransientFailurenever sawcode/statusand classified the 429 as permanent: one ask, no retry, no cooldown — the defect this PR exists to fix was still live on the real path. The two added cases in #1449 only coveredllm.streamthrowing synchronously, which the provider does not do.What this adds on top (
8a94c490, 6 files, +153/−28)core/src/host/parts.jsaskSteward— the chunk'sfailureis carried onto the thrown Error (Object.assign(new Error(message), finish.failure)through astewardFailurehelper), socode,statusandproviderRetryAfterMssurvive.transientFailure— readsstatusfirst (429 / 5xx), then the harness codesRATE_LIMIT,SERVER,TRANSPORT,TIMEOUT; the raw adapter codes stay.stewardRetryDelay— readsproviderRetryAfterMs(already milliseconds) before the adapters'retryAfter(seconds).core/test/steward-retry.test.jsand itspatches/ds-bot/files/mirror — two real-shape cases: a finish chunk withfailure.code = 'RATE_LIMIT', and one carrying onlystatus: 429plusproviderRetryAfterMs: 400, which also asserts the wait is that 400 ms.patches/ds-bot/0014-flybot-branding.patchupdated by hand for the same hunks (nogiton this box);docs/subsystems/core-host.mdand the changelog fragment follow the behaviour.Ran (Rework, at tree
87fd310e)# tests 5 / # pass 3 / # fail 2— exactly the two new real-shape cases. After the fix:5/5.coresuite, serially:1844 tests / 1839 pass / 5 fail; all five fail identically at12c266e6in this sandbox and are environmental (check-arch-docswithoutgit, three suites needingjsdom, theusage.sqlitefixture).node scripts/ds-bot-patches.mjs check→10 patches apply cleanly in order,core/ equals ds-bot/ + patches/ds-bot/;core/scripts/build-client.mjs --checkexit 0;changelog.mjs check→ 43 fragments well formed; lint 0 errors.core/dev/bench.sh smoke— no harness checkout in this sandbox (read-only mount), the same limitation as workflow:reap()skipsrunningruns, and it only runs once at plugin load #1388, fix(issue-ref): read a bare #1234 only when the number stands alone #1392 and fix(parts): archive a job-held part once its job ends #1436.Boundaries
dev2/fix/1433-job-part-archive, tipc3c2d32) is not touched: it also editscore/src/host/parts.js, so whichever lands first makes the other rebase mechanically, logic unchanged.Checkpoint
{
"pr": 1500,
"claim": "Chief",
"issue": 1447,
"summary": "One throttled steward call moved the whole part on. The retry added in #1449 never fired on the real path: a provider failure arrives as a terminal finish chunk, and askSteward kept only its message, so a 429 looked permanent. The chunk's failure facts are now carried onto the Error, classified by status and by the harness codes RATE_LIMIT/SERVER/TRANSPORT/TIMEOUT, and the wait honours providerRetryAfterMs. Red first 3 pass / 2 fail on the two new real-shape cases, 5/5 after.",
"files": ["core/src/host/parts.js", "core/test/steward-retry.test.js", "docs/subsystems/core-host.md", "changelog.d/1447-steward-retry.fixed.md", "patches/ds-bot/0014-flybot-branding.patch", "patches/ds-bot/files/test/steward-retry.test.js", "patches/ds-bot/topics.json"],
"checks": { "ran": "steward-retry red/green, core serially 1844/1839 with environment-only failures, ds-bot-patches check, build-client --check, changelog check, lint", "result": "green", "at": "8a94c490007c74b7bc382a28d1a42ca20c85ea00" },
"next": "fresh PASS at this tip, then enqueue; #1449 is superseded by this head once it lands"
}