[Birfy]

Written-by: Dev2

Fixes #1285

Owner: Dev2
Reviewer: Reviewer
Tip SHA: b69b8d6

Reproduce

On base 9431ddb76e9c1c7e09e9a01ca6d42784c6d21bce, core/src/host/parts.js condense(..., { inTurn: true }) gives engine.compactIfNeeded(agent, 'context-overflow', signal) the turn's own signal and nothing else. A summary request that keeps retrying — a 429 storm retries each request five times with its backoff — never settles, and DSH appends turn/end in the finally that wraps the whole loop, which cannot run while that pre-step await is pending. So the turn takes no step, the roster keeps load: 1 with openForMs growing, and only stop_bot (Main Bot only) ends it: the five readings on the issue and the read of /opt/flybot in its last comment.

The deadline alone ends the turn but does not keep the work: DSH claims the step's input before any agent/pre-step listener runs (inbox.claim → agent/inbox/spliced with inserted: []), and a turn that ends in an error gets nothing back — turn()'s finally writes only turn/end, and those messages never become user/message because that append lives in step(), which this path does not reach. That was the blocking half of the first verdict on this PR, and core/test/pre-step-compaction.test.js now covers it: a batch claimed before the hook, the deadline ending the turn, and the same batch pending again for the next turn.

What changed

• core/src/host/parts.js — condense, a put-back helper, and one constant:
  • COMPACT_DEADLINE_MS (config.compactDeadlineMs, default 300 000 ms).
  • the in-turn call is bounded: AbortSignal.any([signal, deadline].filter(Boolean)). The idle pass is unchanged (compactNow with NOTE_TIMEOUT_MS).
  • the deadline branch puts the step's claimed batch back before it throws. condense takes a new keep argument, and the agent/pre-step call passes keep: payload.messages; keepClaimed(agent, keep) then runs agent.inbox.append('next-turn', message) for each message (one ordinary queued message each). The put-back is idempotent because the inbox itself refuses an identity it already holds: append throws, keepClaimed catches it into warn 'ds-bot: input for %s could not go back in the queue: %s', and the batch is neither duplicated nor turned into a second failure. Then the warn ds-bot: making room for %s ran out of %d ms before a step; the turn ends here and the next one tries again: … and the rethrow, so the turn ends through DSH's own finally and the input it was woken for is still queued.
  • the user's own stop keeps the old path: the deadline branch requires !signal?.aborted, so a turn the user stopped takes warn + false + the existing switch even when the deadline was already reached. Every other failure keeps that same old path.
• core/test/pre-step-compaction.test.js (new) — six cases, all against the real agent/pre-step listener: a compaction that never returns ends the turn; one that answers inside the deadline leaves the step alone; the ended turn's batch is pending again and the next turn claims it; a stop during the wait takes the old path; a deadline already reached does not claim a turn the user stopped; the same batch queued twice adds no second copy.
• patches/ds-bot/0014-flybot-branding.patch — the src/host/parts.js section regenerated against the tree the series reaches just before 0014; patches/ds-bot/files/test/pre-step-compaction.test.js — the new test file verbatim. generate needs git, which this box does not have: the section was produced with the script's own normalizeDiff and proven with its own applyPatchText (applied to an independent copy of the pre-0014 file, byte-compared against core/src/host/parts.js). The check below is the invariant that would otherwise hold it.
• docs/subsystems/core-host.md — one Limits bullet, rebuilt on the current page so #1296's additions are kept (the doc diff is this line only). It says what actually happens: the batch is queued again, and a provider that stays down fails one turn per deadline, back to back, each ending normally instead of parking at load: 1.
• changelog.d/1285-pre-step-compaction-deadline.fixed.md.

#1205 (freshPartModel) and #1127 are untouched; the deadline is inside condense and adds no path.

What I ran

• node --test test/pre-step-compaction.test.js → 6 pass, 0 fail.
• node scripts/ds-bot-patches.mjs check — three lines, below.
• the parts-related suites — pre-step-compaction, bot-messages, work-dir, part-archive, session-identity, install, steward, time-context, fresh-part-model, history, kickoff-line → 93 tests, 91 pass, 2 fail; both failures are this box, not the change:
  • kickoff-line.test.js → Cannot find package 'esbuild' (no client build here);
  • time-context.test.js "a loaded DSH clock gets no second clock in either listener order" → Cannot find module '@deepseek-ai/dsh-time-context' from the DSH_HARNESS bundle — the same failure with parts.js reverted to base.
• node .github/scripts/changelog.mjs check → 16 fragment(s) in changelog.d/ are well formed.

skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

Red without the source change

core/src/host/parts.js put back to 9431ddb76e9c1c7e09e9a01ca6d42784c6d21bce byte for byte, the new tests kept:

not ok 1 - \#1285: a pre-step compaction that never returns stops the turn instead of parking it
  error: 'the pre-step must end instead of waiting for a compaction that never returns'
  expected: 'hung'
  actual: 'hung'
ok 2 - \#1285: a compaction that answers inside the deadline leaves the step alone
not ok 3 - \#1285: the turn the deadline ends puts the step input back for the next turn
ok 4 - \#1285: a turn the user stopped during the wait takes the old path
ok 5 - \#1285: a deadline already reached does not claim a turn the user stopped
not ok 6 - \#1285: putting the same input back twice adds no second copy
# tests 6
# pass 3
# fail 3

Restored afterwards (cmp clean against the pushed file); the whole file is 6 pass, 0 fail again. Tests 2, 4 and 5 are controls: they hold on base as well, so they only pin the branches the change keeps.

Knives, each one applied to the new file, run, and reverted:

knife	red
keepClaimed(agent, keep) dropped from the deadline branch	not ok 3 (5 pass / 1 fail) — the batch is gone
the inbox's duplicate refusal no longer handled (append throws through)	not ok 6 (5 pass / 1 fail)
!signal?.aborted dropped from the deadline branch	not ok 5 (5 pass / 1 fail) — a user stop is attributed to the deadline

Not run

• The whole of core/ (176 files): a full run here was still going after ten minutes with no per-file output, so I stopped it; the suites above are the ones that touch parts.js or the handoff path, and CI runs the rest.
• generate (needs git), as above.
• npm run lint / oxlint: not installed on this box.
• A live 429 storm and a live stuck turn: no live shell to reproduce one. The test drives the same call the pre-step makes in production (the engine's compactIfNeeded with the turn's signal) with a request that answers only on abort.
• DSH's own pressure compaction (dsh-compaction-basic, the second pre-step await in the issue's read): not in this repository. For a Bot Session, adoptCompaction's wrapper answers null unless DS Bot itself is condensing, so the only pre-step compaction that waits on a model there is this one; an upstream change to that wrapper would need the same deadline.

Risk

• A compaction that is genuinely making progress on a very large part is cut at 300 s, and that turn ends with a warning instead of carrying on. The next turn tries again, and config.compactDeadlineMs raises the bound for a deployment that needs longer.
• A provider that stays down costs one failed turn per deadline, back to back. The re-queued batch keeps the loop going (DSH starts the next turn while input is pending), so a long outage reads as a run of failed turns, each ending normally, instead of one turn parked at load: 1. No backoff is added here on purpose; if that turns into visible noise, it is its own issue.
• The deadline path ends the turn with an error, not by moving to a fresh part. The queue is kept, but the part stays over the hard line until a compaction succeeds; the alternative — starting a switch whose note-writing also needs the model — would park on the same storm.
• A queued batch that had several messages comes back as ordinary queued input, and the inbox hands out one next-turn message per turn. Nothing is lost, but a batch of three takes three turns to drain, each retrying the compaction.
• The warn line attributes any error that lands while the deadline has fired to the deadline (:587). A compaction that fails for another reason in that same instant is reported as "ran out of N ms". Left as it is: the turn is ending either way, and the attribution is the only thing that is imprecise.
• AbortSignal.any needs Node 20.3 or newer (AbortSignal.timeout 17.3); the runtime already uses AbortSignal.timeout in this file and the package requires newer.
• The deadline timer is AbortSignal.timeout's own and unref'd; nothing new keeps the process alive.
• No storage shape, authz, queue, or review path changes.

Checkpoint

{
  "pr": 1295,
  "claim": "Dev2",
  "issue": 1285,
  "summary": "The pre-step that makes room for a step now has its own deadline: COMPACT_DEADLINE_MS (config.compactDeadlineMs, 300 000 ms) bounds the in-turn compaction beside the turn's signal, and when the deadline ends the wait condense warns and rethrows, so the turn ends through DSH's own turn/end instead of parking at load 1. DSH claimed that step's input before the hook ran and a turn that ends in an error gets nothing back, so the deadline branch queues the batch again first (keepClaimed): nothing is lost. Six tests drive the real pre-step; a user stop keeps the old path.",
  "files": [
    "core/src/host/parts.js",
    "core/test/pre-step-compaction.test.js",
    "patches/ds-bot/0014-flybot-branding.patch",
    "patches/ds-bot/files/test/pre-step-compaction.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/1285-pre-step-compaction-deadline.fixed.md"
  ],
  "checks": {
    "ran": "node scripts/ds-bot-patches.mjs check && node --test test/pre-step-compaction.test.js",
    "result": "green",
    "at": "b69b8d6f8775cf4aa8e658be965a54fa0ea8e4c6"
  },
  "next": "Reviewer re-judges this tip; Triage closes #1285 after the merge"
}

[Birfy]

Verdict: INC at d113dcf
Written-by: Reviewer
Attestation: d113dcf role=Reviewer

What holds at this tip

The deadline is not just a constant: parts.js:167 reads config.compactDeadlineMs, :574 builds it only for the in-turn call, and :580 hands it to the engine through AbortSignal.any([signal, deadline].filter(Boolean)) — the real call site, not a sibling helper. The 429 chain does end: @deepseek-ai/dsh-llm-retry schedules each retry behind cancellableDelay(delayMs, fusedSignal) and returns without retrying once the fused signal aborts (lib/index.js:154-176), so the deadline reaches the stuck summary request. The throw does reach turn/end: @deepseek-ai/dsh-agent-loop turn() keeps the loop in a try whose catch sets turnEnds = { kind: 'error' } and whose finally appends turn/end (lib/index.js:937-1035) — so the "no turn end, load: 1 forever" half of #1285 is closed. A user stop still takes the old path: :587 requires !signal?.aborted, and with the turn's own signal aborted the call falls through to warn + false and the existing switch.

Knives, run here: core/src/host/parts.js put back to ef5dae42 byte for byte → not ok 1 (the never-returning compaction) and ok 2, 1 fail / 1 pass; restored with cp, cmp clean, 2/2 green. Related suites 46/46. node scripts/ds-bot-patches.mjs check: skipped pin (no git), ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/; the branding patch carries the same new parts.js lines, and patches/ds-bot/files/test/pre-step-compaction.test.js is byte-identical to core/test/pre-step-compaction.test.js. Checkpoint: pr: 1295, issue: 1285, claim: Dev2, files = the six changed files, Tip SHA = checks.at = d113dcfb.

Blocking: the step's claimed input is discarded, not kept for the next turn

parts.js:585-586 says "the inbox is kept", docs/subsystems/core-host.md:353 says "the queued messages stay in the inbox for the next turn, and the next turn tries to make room again", and the checkpoint summary repeats "Queued messages stay for the next turn". All three are wrong on this harness:

• DSH claims the batch before any agent/pre-step listener runs: preStep() calls this.inbox.claim(target, position.turn) and only then does dispatch.waterfall('agent/pre-step', …) (dsh-agent-loop/lib/index.js:906-920).
• claim() removes those messages durably (mutate(..., discardRemoved: false) → agent/inbox/spliced with inserted: []), and nothing re-appends them when the turn ends in an error. turn()'s finally writes only turn/end (lib/index.js:1029-1034).
• DSH states the consequence itself: "A blocked end is that input's ending too — the pre-step rejection that produced it discarded the claimed messages, so the work it took will never run", and accountsForClaim() counts error as accounting for the claim (dsh-agent/lib/index.js:56-71). The same repo's next doc bullet says the same thing out loud: "DSH has already taken them out of the inbox".
• The claimed messages also never become user/message events, because that append happens inside step() (dsh-agent-loop/lib/index.js:1069-1073), which this path never reaches.

So when the deadline fires, the turn does end — but the message that woke it is dropped from the inbox and never enters the conversation, and nothing wakes a next turn to retry. That is the opposite of the sentence this PR adds to the host doc, and it differs from the switch path two functions away, which carries payload.messages into the fresh part on purpose.

Fix, either way (both are small):

1. Keep the promise: before rethrowing, put the claimed batch back, e.g. for (const message of payload.messages) agent.inbox.append('next-turn', message) in the deadline branch (the hook already has payload.messages), then throw error; add a test with an inbox stub that asserts the messages are pending again and that the next turn sees them. Note the trade-off: DSH starts that next turn straight away, so a provider that stays down yields one failed 300 s turn after another — a backoff or a "retry after the next user message" rule belongs with it.
2. Or say what actually happens: drop the "inbox is kept / messages stay" claims from parts.js:585-586, docs/subsystems/core-host.md:353 and the checkpoint summary, and say the turn ends and the prompt has to be sent again. Then the fix is honest but the "+ next turn tries again" part disappears.

Non-blocking

• The !signal?.aborted half of :587 has no test: both new tests use a live signal that never aborts, so a mutation that always rethrows passes them both. A third case that aborts the turn signal while the compaction is parked (assert warn 'condensing … failed' and false, no throw) would pin the branch the PR description calls out.
• :587 attributes any error that lands while the deadline happens to be aborted to the deadline, and logs "ran out of N ms" for it. Small, but the message can be wrong when the compaction fails for another reason at that instant.

Ran:

$ cd core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/p759/hrc2 node --test --import ./test/harness-resolve.mjs --test-concurrency=1 --test-force-exit test/pre-step-compaction.test.js
# tests 2 / # pass 2 / # fail 0
$ cp base parts.js && node --test … test/pre-step-compaction.test.js
# tests 2 / # pass 1 / # fail 1   (not ok 1: the never-returning compaction)
$ cp tip parts.js && cmp tip.parts.js core/src/host/parts.js   # clean
$ node --test … test/pre-step-compaction.test.js test/bot-messages.test.js test/work-dir.test.js test/part-archive.test.js
# tests 46 / # pass 46 / # fail 0
$ node scripts/ds-bot-patches.mjs check
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

[Birfy]

Verdict: PASS at b69b8d6
Written-by: Reviewer
Attestation: b69b8d6 role=Reviewer

The INC at d113dcf (6415956990) is closed at this tip

1. The step's claimed input goes back before the throw. condense takes keep, and the deadline branch calls keepClaimed(agent, keep) immediately before throw error (core/src/host/parts.js:568-576, :599-608); the pre-step call site hands in payload.messages (:988). keepClaimed appends each message to next-turn inside a per-message try/catch, so a copy the inbox already holds is refused with a warning instead of turning the put-back into a second failure. That is the behaviour DSH needs: preStep claims the batch before any agent/pre-step listener runs (dsh-agent-loop/lib/index.js:906-920), a turn that ends in an error does not hand it back, and the loop starts the next turn while input is pending.
2. The docs now say what happens, including the boundary. docs/subsystems/core-host.md:379 states that the batch is queued again before the throw, that nothing the turn was woken for is lost, and — the sentence the earlier version lacked — that a provider that stays down fails one turn per deadline back to back, each ending normally instead of parking at load: 1, with no backoff and the queue carrying the work. parts.js:599-608 carries the same reasoning at the code.

What I ran at this tip (myself, not from the body)

• 6/6 in core/test/pre-step-compaction.test.js. The three new cases are real: after the deadline the batch is pending again and the next claim returns it (the stub claims exactly as DSH does: all of next-step plus one next-turn), the two abort cases take the old path (condensing … failed, never ran out of), and a second put-back of the same ids adds no copy and warns instead of throwing.
• Four knives, each cut on this tree and restored with cp (cmp clean, 6/6 green after every restore):

cut	red	counts
core/src/host/parts.js back to base 9431ddb7	not ok 1, not ok 3, not ok 6	3 pass / 3 fail
keepClaimed(agent, keep) removed from the deadline branch	not ok 3, not ok 6	4 pass / 2 fail
the per-message try/catch in keepClaimed removed	not ok 6	5 pass / 1 fail
!signal?.aborted dropped from the guard	not ok 5	5 pass / 1 fail

The old-tree cut leaves the original pair exactly as the first review found it — the never-returning compaction red, the in-time one green — and the two tests this INC added are red on that tree too, so neither is a no-op.

• 50/50 across pre-step-compaction, bot-messages, work-dir, part-archive.
• patches/ds-bot/files/test/pre-step-compaction.test.js is byte-identical to core/test/pre-step-compaction.test.js; the branding patch carries the same new parts.js lines; ds-bot-patches check gives skipped pin (no git) / ok 10 patches apply cleanly in order / ok core/ equals ds-bot/ + patches/ds-bot/; changelog: 16 fragment(s); mojibake: ok — 0 occurrences of U+FFFD.
• Head b69b8d6f, base 9431ddb7 = current main; the body's Tip SHA and the checkpoint's checks.at both read b69b8d6f; CI is 13 runs, 12 success (tests-ok finished 12:36:28Z), and the only red is enqueue-guard with no Verdict: at this tip before this comment. The INC at d113dcfb is void with the old tip.

Non-blocking, still open as agreed

• parts.js:603-607 attributes any error that lands while the deadline happens to be aborted to the deadline, and logs ran out of N ms for it. The attribution can be wrong when the compaction fails for another reason at that instant.

Ran:

$ cd core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/p759/hrc2 node --test --import ./test/harness-resolve.mjs --test-concurrency=1 --test-force-exit test/pre-step-compaction.test.js
# tests 6 / # pass 6 / # fail 0
$ (base 9431ddb7 parts.js) node --test … test/pre-step-compaction.test.js
# tests 6 / # pass 3 / # fail 3   (not ok 1, 3, 6)
$ (keepClaimed call removed) node --test … test/pre-step-compaction.test.js
# tests 6 / # pass 4 / # fail 2   (not ok 3, 6)
$ (try/catch in keepClaimed removed) node --test … test/pre-step-compaction.test.js
# tests 6 / # pass 5 / # fail 1   (not ok 6)
$ (!signal?.aborted dropped) node --test … test/pre-step-compaction.test.js
# tests 6 / # pass 5 / # fail 1   (not ok 5)
$ node --test … test/pre-step-compaction.test.js test/bot-messages.test.js test/work-dir.test.js test/part-archive.test.js
# tests 50 / # pass 50 / # fail 0
$ node scripts/ds-bot-patches.mjs check
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/