Repository navigation
fix(parts): archive a job-held part once its job ends - #1436
Conversation
|
Written-by: Dev Claimed. Branch: workflow-settings-canvas-editor. Owner screenshot attached on the agent task; replace the prompt-wall Settings page with a canvas-first drag-drop editor. |
bb24a31 to
c3c2d32
Compare
|
Self-test: PASS at c3c2d32 Measured on this tree, not read off the PR body. This is our team's self-test: it is not the repository's formal verdict, and it authorizes no merge. Ran: from Where this tip standshead Red, then green, then the knife
The four reds, by name and line: case 3 Knife — only the schedule-only retry block reverted in place ( The base suite is green on its own: base The seven behaviours, with the lines
Two smaller things checked in the same pass: Gates at this head
Process items
as of my read: 1 comment, newest |
c3c2d32 to
4eff9e9
Compare
…ries it (ThinkFlowLab#1500) fix(parts): ask the context steward again before moving a part on — carrying ThinkFlowLab#1449 with the review fixes Fixes ThinkFlowLab#1447 This branch carries **ThinkFlowLab#1449** (author: @HugoXK, `Written-by: Dev-2`) — the same seven-file change, byte for byte — plus the three fixes the review asked for at `12c266e6`. It is opened here because ThinkFlowLab#1449's head lives in `HugoXK/FlyBot`, where this account has `pull` only: git object writes answer 404 and a ref update answers 422, and `PUT /pulls/1449/update-branch` can only merge `main` in, 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: 87fd310 ## What was wrong (from ThinkFlowLab#1449) One throttled steward call moved the whole part on. `decide()` wrapped the ask in a single `try`, and any exception — including a per-minute 429 that recovers in seconds — fell to `return 'switch'`; `checkContext()` then called `startSwitch()` 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: `writeNote` has `NOTE_ATTEMPTS` (2) and `NOTE_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 `12c266e6` A real provider route reports a failed request as a **terminal `finish` chunk**: `{reason:{kind:'error', failure:{message, code:'RATE_LIMIT', status?, providerRetryAfterMs?}}}`. `askSteward` (`core/src/host/parts.js:556`) threw `new Error(finish.failure?.message)`, keeping only the message, so `transientFailure` never saw `code`/`status` and 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 ThinkFlowLab#1449 only covered `llm.stream` throwing synchronously, which the provider does not do. ## What this adds on top (`8a94c490`, 6 files, +153/−28) 1. `core/src/host/parts.js` `askSteward` — the chunk's `failure` is carried onto the thrown Error (`Object.assign(new Error(message), finish.failure)` through a `stewardFailure` helper), so `code`, `status` and `providerRetryAfterMs` survive. 2. `transientFailure` — reads `status` first (429 / 5xx), then the harness codes `RATE_LIMIT`, `SERVER`, `TRANSPORT`, `TIMEOUT`; the raw adapter codes stay. 3. `stewardRetryDelay` — reads `providerRetryAfterMs` (already milliseconds) before the adapters' `retryAfter` (seconds). 4. `core/test/steward-retry.test.js` and its `patches/ds-bot/files/` mirror — two real-shape cases: a finish chunk with `failure.code = 'RATE_LIMIT'`, and one carrying only `status: 429` plus `providerRetryAfterMs: 400`, which also asserts the wait is that 400 ms. 5. `patches/ds-bot/0014-flybot-branding.patch` updated by hand for the same hunks (no `git` on this box); `docs/subsystems/core-host.md` and the changelog fragment follow the behaviour. ## Ran (Rework, at tree `87fd310e`) - Red first: `# tests 5 / # pass 3 / # fail 2` — exactly the two new real-shape cases. After the fix: `5/5`. - Whole `core` suite, serially: `1844 tests / 1839 pass / 5 fail`; all five fail identically at `12c266e6` in this sandbox and are environmental (`check-arch-docs` without `git`, three suites needing `jsdom`, the `usage.sqlite` fixture). - `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 --check` exit 0; `changelog.mjs check` → 43 fragments well formed; lint 0 errors. - **Not run**: `core/dev/bench.sh smoke` — no harness checkout in this sandbox (read-only mount), the same limitation as ThinkFlowLab#1388, ThinkFlowLab#1392 and ThinkFlowLab#1436. ## Boundaries - ThinkFlowLab#1449 stays open and stays its author's; nothing is closed or reworded there, and no force-push is possible from here anyway. - ThinkFlowLab#1436 (`dev2/fix/1433-job-part-archive`, tip `c3c2d32`) is not touched: it also edits `core/src/host/parts.js`, so whichever lands first makes the other rebase mechanically, logic unchanged. ## Checkpoint <!-- flybot:checkpoint --> { "pr": 1500, "claim": "Chief", "issue": 1447, "summary": "One throttled steward call moved the whole part on. The retry added in ThinkFlowLab#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; ThinkFlowLab#1449 is superseded by this head once it lands" } <!-- /flybot:checkpoint --> --------- Co-authored-by: Day Dreamer <87640074+HugoXK@users.noreply.github.com>
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.
4eff9e9 to
6b0111a
Compare
|
Written-by: Rework Replayed onto main What the replay had to merge: main's #1500 (steward retry) also edits Ran at CI on this tip: |
|
Verdict: PASS at 6b0111a Ran at this tip (2026-10-27T23:12Z): 审查要点:
|
Fixes #1433
Written-by: Dev-2
Owner: Chief
Reviewer: Reviewer
Base: 55e1c27
Tip: 6b0111a (the replayed head; its only parent is the base above, and it carries tree
2c5e9f967b5e403b55864f4751908d29e42686bd)Tree: 2c5e9f967b5e403b55864f4751908d29e42686bd
What was wrong
The handoff archived the earlier part and, when DSH refused because the Session was still active, answered only a refusal a scheduled task alone caused (
{ stopActivity: true }, the #896 rule). A refusal naming ajob, aturnor asubagentreturned atparts.js:827and nothing ever read that Session id again, so the part stayed active for the life of the instance (#1433). An instance logged one at 11:09:36Z onsession-cb45538c…with(job).The trigger, named
ctx.get('jobs').events.subscribe({ owners: 'all' }, …), insidectx.effect(…)atparts.js:1149: on an event withtype === 'settled'whosejob.owneris a waiting Session id, onectx.workspaceRegistry.archiveSession(id)with no options.@deepseek-ai/dsh-jobsis also what answersworkspace/session-activitywithkind: 'job', so the thing that held the part is the thing that reports it gone.agent/statusidle handler (parts.js:1186, before thert.isBotSessioncheck, since a part that was handed over is no longer the Bot's current Session): the same single attempt, for aturnorsubagentholder.attemptingArchiveguard), no timer and no loop. At most 32 ids wait (PENDING_ARCHIVE_MAX), oldest dropped from the insertion-ordered set; the table can only fill from holder-end signals, never grow on its own.error.activitystill names a job, a turn or a subagent, the holder has not ended after all, so the id stays for the next signal instead of spending the one chance (parts.js:1167). It leaves when the attempt lands, or when the refusal names no activity at all, while a scheduled task alone still takes the same stop-and-archive the handoff uses ({ stopActivity: true },parts.js:832) — so a task that could not be moved never leaves the part active, and the id is never what keeps it there.stopActivityis untouched. A schedule-only refusal takes{ stopActivity: true }at the handoff (parts.js:832) and on the retry; a job, turn or subagent is never stopped —h.stoppedstays[]in all three job cases, and the one case that does stop pins the stop to the scheduled task.Why the idle event alone cannot carry this
DSH derives
agent/statusfrom the loop phase (@deepseek-ai/dsh-agent-loop:idle/maintenance→idle, anything else →running). The handoff's archive runs insiderunMaintenance, so the Session is already idle when the refusal arrives — the idle transition was published before the attempt and is not published again for it. Ajobis a separate admission family (@deepseek-ai/dsh-jobsanswers from its own registry), so it keeps holding the Session while that Session sits idle. And the part that was handed over takes no further turn at all: input for it is forwarded to the fresh part (forward(),parts.js:747). So the ordering Chief asked about — an idle that arrives while the job is still running — is the normal case, not the corner: the job's settlement is the only signal that names the end, and the keep-on-refusal rule is what stops an early idle from spending it (case 4 below stands exactly there).The tests
core/test/part-archive.test.js— 7 cases; with the source taken from the base0915151, 4 fail (cases 3, 4, 5, 6) and 3 pass; at this tip the file is 7/7 green, and the knife (this tip with only the retry's schedule branch reverted) leaves case 6 alone red:#896: a part whose task moved is archived once the move finished— unchanged.#896: a failed task move does not leave an armed earlier part active— unchanged.#896: a part held by a job is never force-archived— the base:240assertion (archivedEntry(h.archived, 'b') === undefined, "stays active, as before this fix") is rewritten in the diff to "the part stays active while the job holds it" and then asserts the part is archived once the job settles;:241h.stopped === []and:242h.activities.b === ['job'](job still running) are kept as they were. Nothing is stopped, and the archive carries no options.#1433: an idle before the job ended spends nothing, and the part is archived when it ends— idle while the job runs archives nothing and stops nothing, the retry really happened (2archiveSessioncalls forb), and the id survives to the settlement.#1433: one idle after the job ended archives the part, and the id is dropped once it lands— one idle after the end archives it; a later idle and a later settlement ask nothing more (the call count does not move).#1433: a scheduled task alone still holding the part takes the handoff stop on the retry— the task move fails (failPut), so the task stays armed on the earlier part; at the handoff the job refuses the archive and nothing is stopped, then the job ends and the schedule-only refusal takes{ stopActivity: true }: the part lands archived with its task stopped, andh.stoppedholds that one stop.#1169: a folder under the harness home refuses the move and names itself— unchanged.core/test/install.test.js: the newctx.effectis a registration, so the pinned order inapply registers listeners, tools, and effects in a fixed ordergains its one line.docs/subsystems/core-host.md:410said a refusal caused by a job, turn or subagent "is not retried"; that sentence is now false, so it is corrected in this diff rather than waved through with aDocs-not-needed:trailer.Files (8, not the 5 in the plan)
changelog.d/1433-job-part-archive.fixed.mdcore/src/host/parts.jscore/test/install.test.jscore/test/part-archive.test.jsdocs/subsystems/core-host.mdpatches/ds-bot/0001-core-fixes.patchpatches/ds-bot/0014-flybot-branding.patchpatches/ds-bot/files/test/part-archive.test.jsThe two extra files are the pinned registration order and the now-false doc sentence; the patch mirrors are
generateoutput. Total +429 / −14.Ran at the reviewed tip (tree
3b94f195, local commit9a0c01f, pushed asc3c2d32; before the replay onto main)cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/part-archive.test.js— 7/7 green here; withcore/src/host/parts.jstaken from0915151(that file is byte-identical on405e408fand0915151,sha256 5cca6335d5a6dea6…): 4 fail, 3 pass (#896: a part held by a job is never force-archived→until: the condition did not hold within 2 s,#1433: one idle after the job ended…→ same,#1433: a scheduled task alone…→ same,#1433: an idle before the job ended…→the retry was actually attempted / 1 !== 2). The knife — this tip with only the retry's schedule branch reverted (a85f912'sparts.js) — leaves case 6 alone red (6/7). Restoring the file byte-for-byte (sha256 17c8065773b621dc…before and after) returns 7/7.TZ=UTC npm run verify— exit 0: core 1828/1828 (1825 on the base: three cases are new), social 525, tree 125, timeline 125, workflow 55 (Settings → Workflows: rename and delete user-owned Workflows #1435 added three),.github/scripts545, interop 37, live-e2e 24 — every suite# fail 0; lint 0 errors;arch-docs: OK (8 changed file(s)).node scripts/ds-bot-patches.mjs generatethencheck— threeoklines (ds-bot/pin, 10 patches apply,core/equalsds-bot/+ patches).405e408f→0915151, Settings → Workflows: rename and delete user-owned Workflows #1435): the branch was rebuilt on the new tip,patches/ds-botwas regenerated from scratch, and Settings → Workflows: rename and delete user-owned Workflows #1435 touches neithercore/src/host/parts.jsnorcore/test/part-archive.test.js, so the red/knife measurements above hold unchanged on it.Not run
core/dev/bench.sh smoke— this sandbox has no harness checkout at../deepseek-harness,DSH_HARNESSis unset, and the mounted harness is[EROFS](read-only): the bench cannot be started here at all, the same limitation as workflow:reap()skipsrunningruns, and it only runs once at plugin load #1388 and fix(issue-ref): read a bare #1234 only when the number stands alone #1392. Thesrc/host/change is covered by the unit suite instead.Replay onto main (Rework)
Replayed onto
55e1c27a09acb86757aa3c07caef5a073860fc49(main) as6b0111a68013a3e5abde53218df2aa2504fca27e; the new commit's only parent is thathead, and the change set is still the eight files above. Main's #1447/#1500
(steward retry) edits
core/src/host/parts.jsanddocs/subsystems/core-host.md, so those two files are merges of both changes;the two
patches/ds-bot/*.patchmirrors were rebuilt from the merged core. Theother four files keep their reviewed blobs byte for byte (
core/test/install.test.js,core/test/part-archive.test.js, the vendored copy and the changelog fragmentare identical at both tips).
Ran at
6b0111a68013(parent55e1c27a):part-archive.test.js7/7; with the merged
parts.jsswapped for main's, 3 pass / 4 fail (cases3-6);
install.test.js4/4;part-archive,install,steward-retry,steward,pre-step-compaction,fresh-part-model,bot-messagesandsession-identityserially 68/68;ds-bot-patchescheck ok (10 patchesapply in order, core equals ds-bot/ + patches). Not run:
core/dev/bench.sh smoke(no harness checkout here).
Checkpoint
{
"pr": 1436,
"claim": "Chief",
"issue": 1433,
"summary": "A handoff left a part active for the life of the instance when a job, turn or subagent refused the archive: only a schedule-only refusal was answered, with a stop. The Session id is now remembered and the archive is asked for once more with no options when its holder is reported ended — the job registry's settled event, or the Session's next idle — keeping the id while a refusal still names a holder and holding at most 32, oldest dropped. Nothing is ever stopped for a job, a turn or a subagent. The subsystem doc sentence that said such a refusal is not retried is corrected.",
"files": [
"core/src/host/parts.js",
"core/test/part-archive.test.js",
"core/test/install.test.js",
"docs/subsystems/core-host.md",
"changelog.d/1433-job-part-archive.fixed.md",
"patches/ds-bot/0014-flybot-branding.patch",
"patches/ds-bot/0001-core-fixes.patch",
"patches/ds-bot/files/test/part-archive.test.js"
],
"checks": {
"ran": "Rework replay at the new tip: part-archive 7/7 (base source 3 pass / 4 fail), install 4/4, affected parts suites 68/68, ds-bot-patches check ok",
"result": "green",
"at": "6b0111a68013a3e5abde53218df2aa2504fca27e"
},
"next": "fresh Verdict at this replayed tip, then enqueue"
}