test(think): remove timing races from Durable Object tests - #2192
Merged
Conversation
Replaces wall-clock sleeps and tight poll budgets with effect polling and promise gates across the think workers suite: - waitForAgentToolRun: 20x10ms poll (that silently returned non-terminal snapshots) -> vi.waitFor with an 8s deadline - skipped/cancelled agent-tool tests: 5ms-sleep-inside-a-50ms-window races -> a beforeStep promise gate on ThinkTestAgent that parks the child turn deterministically while the test resets/cancels - WS chat clear + streaming message-id: fixed 200ms sleeps used as synchronization -> vi.waitFor polling for the persisted effect - triggerFiberRecovery: returns per-callback scheduled-job counts read inside the same RPC, so tests no longer race the zero-delay recovery alarm that the workers pool can fire as soon as the DO is released - probeSubmissionAlarmOwnershipForTest: ~200ms alarm-latency budget -> ~5s Claude-Session: https://claude.ai/code/session_011QZUJztM1rMTsHEC7mbcbz
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
mattzcarey
added a commit
that referenced
this pull request
Sep 1, 2026
Preserve #2192's exact atomic helper shape while checking the Task transport through a separate same-RPC method. Align chat peer ranges with the pending changesets release batch, which publishes this work in agents 0.23.0.
mattzcarey
added a commit
that referenced
this pull request
Sep 2, 2026
* fix(tasks): contain memory-limit loops with the alarm breaker Two confirmed gaps (pinned red-first in memory-limit.test.ts): 1. The driver's in-process dispatch retry converted a memory-limit reset into a silent success — the retry found the half-claimed run not due, returned void, and the wake was deleted without the breaker ever engaging. Memory-limit resets now defer to the alarm boundary like code-update resets: the isolate is condemned either way. 2. The run row outlives the breaker's queue-row policy: startup reconciliation re-derives due-now wakes from it, resurrecting a doomed run through backoff and past sealing. MemoryLimitContext now carries the striking job's identity, and Tasks applies the breaker to the run itself — demoted to the backoff wake on a strike (claim stripped so reconcile honors the deadline), terminally failed (task:failed, TaskMemoryLimitSealed) when the breaker seals. Generic for every definition: the striking run is the one that exhausted memory. Claude-Session: https://claude.ai/code/session_011QZUJztM1rMTsHEC7mbcbz * refactor(chat): run recovery continuations on Tasks Replace root-agent recovery schedule rows with chained runs of the reserved __cf_internal_chat_recovery Task definition shared by AI Chat and Think. Initial attempts deduplicate by incident, delayed retries use step.sleep, and the existing bounded callback methods still detach at model handoff. Legacy and routed dynamic-agent schedules remain as compatibility shims. Make Tasks breaker-safe for recovery definitions: condemned-isolate failures escape journal retries, flagged framework definitions carry queue membership, and onMemoryLimit aligns or seals their authoritative run rows so startup cannot resurrect purged work. Think's submission sweep now inspects Tasks rather than Scheduler rows. AI Chat and Think require agents >=0.23.1, the release containing the shared definition and internal enqueue aperture. * fix(tasks): release non-retained terminal runs Apply retain: false through one terminal cleanup path shared by completed, failed, and cancelled runs. A memory-limit seal now emits task:failed, removes the run and journal, cancels its wake, and releases its idempotency key. Make the breaker regression tolerate only the intentional workerd isolate reset and use per-attempt Durable Object names so Vitest retries cannot collide with durable rows from the failed attempt. * chore: restack recovery tasks on main Preserve #2192's exact atomic helper shape while checking the Task transport through a separate same-RPC method. Align chat peer ranges with the pending changesets release batch, which publishes this work in agents 0.23.0. * fix(recovery): preserve queue attempt ownership Upgrade existing Lifecycle job tables before reading the recovery-loop flag. Let Task wakes defer after one JobDriver attempt because ReplayStep owns their durable retry budget, including reconciliation of older wake rows. Keep pre-handoff chat failures on the current Task or schedule, and enqueue a replacement only after the bounded callback has handed off to the model turn. * fix(tasks): carry late memory resets to breaker When bounded Task dispatch returns before an attempt, preserve a durable late-memory-limit marker if that detached attempt later OOMs. The next alarm rethrows the canonical signal inside JobDriver so existing strike, backoff, and sealing policy remains authoritative. Mark detached job outcomes so their alarm is not treated as a clean breaker cycle, preserve marker wakes through startup reconciliation, and cover queued and warm attempts plus marker cleanup. * fix(tasks): clear stale strikes after detached settlement When work previously detached from an alarm settles without a memory reset, enqueue one no-op Task wake. That wake restores any authoritative run deadline and gives the existing JobDriver a clean alarm boundary to clear stale strikes. Late OOMs keep using the existing late-memory-limit marker. * refactor(lifecycle): own bounded alarm work Keep Lifecycle's deadman alarm armed while it joins promises registered at bounded job handoffs. Classify the whole dynamically growing alarm batch once: all clean work clears prior strikes, while any memory reset enters the existing breaker once with its captured executing job. Tasks now only registers attempts at its five-second handoff, and AI Chat and Think register post-handoff model work so Task and Scheduler transports share the same breaker. Remove the Task-specific late marker jobs and their state. * refactor(lifecycle,tasks): track handed-off alarm work without holding the alarm Lifecycle: `trackAlarmWork` no longer holds the alarm invocation open or coalesces physical re-arms. Handed-off work is classified when it settles: a memory reset records a strike against the job that handed it off, one strike per reset however many flows observe it, and strikes clear only once no handed-off work is outstanding and the last of it settled clean. Attribution follows the dispatch's async context and alarms in flight are counted, so overlapping invocations (tests drive alarm() by hand while the pool auto-fires the physical alarm) stay correct. Tasks: drop the per-definition recoveryLoop grouping and its composition root plumbing (setTaskRecoveryLoopDefinitionResolver, Agent._recoveryLoopTaskDefinitions, the recoveryLoop option on _registerInternalTaskDefinition). onMemoryLimit acts only on the run whose wake struck, stripping the claim and pushing the deadline so the reclaim still sees an interrupted attempt. onJob hands off one canonical promise per attempt, #syncWake skips same-values pushes, and runAttached actually attaches instead of racing the warm start. Chat: both hosts run the bounded recovery callbacks through the shared dispatchChatRecoveryToHandoff and register the recovery Task with hooks keyed by callback name. Design docs, changeset, stale Scheduler/recovery-engine comments and tests updated; new tests pin one-strike-per-reset and the startup re-arm. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * fix(lifecycle): attribute strikes correctly under overlapping alarms Two correctness fixes from Devin Review, both against the previous commit's job-driver.ts: - Overlapping alarm() invocations attributed a memory-limit strike to whichever job a shared instance field (#executingRow) last pointed at, not the job that actually struck — the struck job could escape backoff/sealing while an unrelated one absorbed it. Replaced with an AttributedPlatformFailure carrying the row out of the throw itself, correct by construction regardless of overlap. - The quiescence-clear check for the strike counter read stale snapshots of #alarmsInFlight/#outstandingAlarmWork depending on which of two transitions (an alarm ending, a handoff settling) ran first, so a strike recorded in the gap between checks could never clear. Fixed by having each transition re-check both counters fresh, with no yield point between the mutation and the check on either side. Fixing the second issue introduced a regression caught by the full suite: the quiescence check started running unconditionally in runAlarm's finally, including right after a strike was just recorded, immediately clearing what had just been set. Restored the placement-based guard (only a clean pass triggers the check) that the prior design relied on. New regression test isolates the attribution fix from Tasks' own active-attempt tracking, which independently and correctly re-tracks a Task run an overlapping alarm re-dispatches and was masking the bug in two earlier attempts at this test. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * fix(lifecycle): preserve a strike through a later clean sibling settling Third correctness fix from Devin Review against job-driver.ts, following the same shape as the two already fixed: a strike could be fully recorded (durable counter written, isolate reset scheduled via setTimeout(0)), and then a slower, unrelated clean handoff still outstanding at that moment could settle in the gap before the reset actually lands, find the alarm domain quiescent, and clear what was just recorded. Fixed with #strikeRecordedThisIsolate: once any flow records a strike in this isolate, no later settlement may clear the durable counter for the rest of this isolate's life. The isolate always resets shortly after a recorded strike, so the next genuinely clean cycle runs in a fresh isolate with a fresh JobDriver and the flag back at its default — there is no cross-isolate state to reset. New regression test (oomBeforeCleanSibling) orders a run's own strike distinctly before a separately tracked clean sibling; verified against the pre-fix code that it exercises the intended scenario and passes correctly with the fix. I could not force this specific narrow race window (bounded by workerd's own near-instant setTimeout(0) teardown scheduling) to reproducibly fail pre-fix within this test harness after several timing attempts, so this is a positive regression guard rather than a failing-then-passing proof like the other two fixes in this PR. The fix itself is verified by direct code tracing, matching Devin's own independent diagnosis of the same race. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * refactor(tasks): add Tasks#register for framework-reserved definitions Replaces the imperative _registerInternalTaskDefinition/_internalTaskDefinitions mechanism on Agent (a Map populated via a protected method, five near-identical one-line wrapper methods across AIChatAgent and Think just to call it) with a plain `register(name, definition)` method directly on Tasks. Framework code calls this.tasks.register(...) once per reserved name from its own constructor; it throws if the name lacks the `__cf` prefix or is already registered, so registration composes correctly no matter how many subclass layers exist or what any of them do with their own taskDefinitions field — it no longer depends on Agent's own field-override machinery at all. Agent's constructor keeps setTaskDefinitionResolver, but its only remaining job is bridging the end user's own overridable taskDefinitions field, which genuinely cannot be read at Tasks-construction time (a further-downstream subclass's field initializer runs only after every constructor up the chain returns) — register() has no such problem since it's called eagerly from each host's own constructor, after this.tasks already exists. design/rfc-fibers.md updated to describe the new mechanism (an adversarial review caught the stale reference to the old resolver-based path). New tests cover register()'s own validation branches directly (missing __cf prefix, empty name, duplicate registration, collision with a constructor-declared definition, and that a registered definition is reachable only through the internal aperture, never the public run()) — the only real definitions that previously exercised it (chat turn, chat recovery, messenger reply) never hit any of its failure paths. No public API surface changes: docs/agents/tasks.md and the existing changeset need no edits, matching every other __DO_NOT_USE_WILL_BREAK__- style internal aperture on Tasks. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * chore(lifecycle): remove dead ALTER TABLE migration for cf_agents_jobs cf_agents_jobs (and its recovery_loop column) has never shipped in a release — the whole table was introduced in this same unreleased branch (#2175, then extended with recovery_loop in #2190/#2194). No deployed Durable Object can have this table without the column already present: CREATE TABLE IF NOT EXISTS already includes it in the same statement. The pragma_table_info probe + ALTER TABLE fallback was defending against a schema history that cannot exist yet. Removes the dead branch, its regression test (which only proved the migration path itself, not anything a real caller depends on), and the two doc/changeset sentences describing it. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * feat(tasks): support routed wakes so facet chat recovery drops the Scheduler bridge Tasks now mirrors a routed sub-agent's run deadline to the root's job queue while the run row and step journal stay put: only the root owns the physical alarm, so only the wake needs to cross that boundary. onRoute handles syncWake/dispatch/memoryLimit; a routed strike forwards to the owning facet's own onAlarmMemoryLimit hook, the same bridge Scheduler already used, since the facet's own Lifecycle never observes the root's alarm directly. AIChatAgent and Think's _enqueueChatRecovery now always uses Tasks, removing the parentPath-gated Scheduler fallback and the now-dead chatRecoverySchedulePolicy/chatRecoveryRedeferPolicy helpers. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * fix(ai-chat): import AgentContext for AIChatAgentToolChild's constructor Fixes a typecheck failure in packages/ai-chat/src/tests/tsconfig.json CI caught: the test-only OOM Task definition added in the routed-wake work needed the child's own constructor to register it, which used AgentContext without importing it. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * fix(tasks): fix three Devin-flagged routed-dispatch bugs - onRoute's "dispatch" case awaited #executeRun directly, bypassing onJob's active-run claim refresh and its five-second dispatch budget. A routed run already active on the facet lost its wake when the root's mirror job settled with no reschedule, and a long-running routed handler blocked the root's whole alarm cycle. Extracted the shared bounded-dispatch logic into #dispatchRun, used by both onJob locally and onRoute's "dispatch" case, whose wake outcome now flows back as the RPC's own return value instead of being discarded. - onMemoryLimit only forwarded a routed run's SEALED strike to its owning facet. A non-sealed strike backed off the root's mirror job but left the facet's own claim (generation, next_at) untouched; any facet startup before the backoff elapsed read that claim as an interrupted attempt and reconciled it due again now, resurrecting the run through the breaker. Forward every strike, sealed or not. Verified the backoff fix is real: reverting to sealed-only forwarding reproduces the failure (generation stays set) in the new "backs off a routed facet's own claim" test. The active-run and dispatch-budget fixes reuse #dispatchRun verbatim from the local path, already covered by the existing local dispatch tests; a dedicated routed-side regression test for the concurrent-active and >5s-budget cases was not added given the cost of reliably constructing those races across a real DO-RPC boundary in this harness. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * fix(tasks): track routed dispatch on the root alarm, not the facet; cancel stale wakes on facet deletion Two more Devin-flagged bugs in the routed-Tasks work: - Routed tasks bypassed the memory breaker. A routed dispatch that exceeded the five-second budget called trackAlarmWork from inside onRoute on the facet, but that call only attaches to a live alarm invocation's AsyncLocalStorage scope — onRoute runs in response to an incoming RPC, not the facet's own alarm, so the call silently did nothing. Every redispatch of a routed run goes through this same path (root owns the physical alarm), so a routed run that regularly overran the budget could OOM repeatedly without the breaker ever seeing it. Moved the budget race to the root side instead: root now races its own await of the routed RPC call, and on budget keeps the still-pending call itself tracked via trackAlarmWork, which works because onJob runs inside root's own live alarm scope. The facet's onRoute dispatch handler no longer needs its own budget or tracking at all — it just fully awaits, since the call keeps running on the facet regardless of whether root is still waiting on it. Verified this is actually safe with a deployed repro (not simulated via ctx.abort(), which is deliberately deferred and wouldn't reject an in-flight caller): a callee DO whose isolate is killed by the platform's real memory-limit enforcement while mid-flight on an RPC call reliably rejects the caller's pending promise with the platform's own "exceeded its memory limit" text — exactly what the breaker already matches on. Confirmed across 3 trials against a real deployment, then deleted the repro worker. Added a regression test whose failure lands after root's own budget elapses (a real >5s delay, not a simulated abort) and confirmed it fails when reverted to the old (silently-inert) tracking call. - Deleting a facet left its routed Task wake mirrored on the root forever, since only Scheduler's routed rows were cleaned up on facet-subtree deletion. Added the same __DO_NOT_USE_WILL_BREAK__cleanupRoutePrefix aperture to Tasks, mirroring Scheduler's, and wired it into the same cleanupPrefix call site. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * fix(tasks,chat): mirror a routed run's claim deadline before dispatch; retry a failed post-handoff redefer Two more Devin-flagged bugs from the latest routed-Tasks review pass: - Once root started racing its own await of a routed dispatch (the RPC-tracking fix earlier today), a budget win now returns undefined as root's own job outcome. JobDriver deletes the mirror job on an undefined outcome — but only when the row is still marked "running" from that same dispatch. #executeRun now pushes the claim deadline via #syncWake immediately after claiming, before the handler runs: that push clears the row's in-flight marker (job-queue's own "newer durable intent wins" guard), so root's later stale undefined outcome no-ops against it instead of deleting a still-live claim. A hung or interrupted routed attempt keeps its alarm either way now, independent of whether this specific attempt happens to settle cleanly, fail, or never resolve at all. - dispatchChatRecoveryToHandoff swallowed a failed post-handoff redefer entirely. The Task that dispatched it has already settled by the time redefer runs, so nothing else owned that incident — a transient failure to enqueue the replacement abandoned recovery silently. Wrapped it in a bounded retry (tryN, 3 attempts) and surface the final failure through the same onDetachedError channel an unowned detached failure already uses, rather than a bare swallowed catch. Both verified non-vacuous: reverting each fix reproduces the exact failure Devin described (the mirror job actually gets deleted; the redefer failure is actually silent) in new regression tests, then passes again restored. The claim-mirroring test uses a real 6.5s delayed failure, not a simulated one. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * fix(chat): dedupe a retried post-handoff redefer instead of creating duplicate runs The redefer retry added last commit (tryN, 3 attempts) has its own bug: Tasks acceptance can throw after already durably inserting the run row — most likely on the wake-mirror push, not the insert itself — so a rejected enqueue does not prove nothing was created. Retrying the same unkeyed "redefer" enqueue (intentionally unkeyed for a genuinely new attempt, per chatRecoveryTaskRunOptions) could create up to three replacement runs for one incident instead of joining the first. dispatchChatRecoveryToHandoff now generates one dedupe key per failure and passes it to every retried redefer call; chatRecoveryTaskRunOptions keys the run by it (runId) when supplied, so a retry after a partial success joins that same row instead of duplicating it. Verified non-vacuous: reverting chatRecoveryTaskRunOptions's use of the key reproduces exactly the gap in the new "keys the run by dedupeKey" test. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * fix(tasks): repair a missing wake mirror when acceptance joins an existing run The dedupe fix from the last commit prevents a retried enqueue from creating a duplicate run, but joining the existing row alone doesn't help if that row's wake was never durably pushed in the first place. #accept can throw after already inserting the run — most likely on the wake-mirror #syncWake call itself, not the insert — so a retry that finds the existing row and returns accepted:false was reporting success against a run nothing would ever wake again. The join branch now calls #syncWake before returning, for every caller (runId or idempotencyKey match), not just chat recovery's retried redefer — any Tasks caller retrying acceptance after a partial failure benefits the same way. Cheap in the common case (#syncWake already no-ops when the mirror already matches). Verified non-vacuous: reverting the added #syncWake call and rerunning the new "repairs a missing wake mirror" capability test reproduces the gap exactly — the mirror job stays missing after the retry joins the run. Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB * refactor(scheduler): remove the retired routed chat-recovery memory-limit bridge Scheduler's routed memory-limit compatibility bridge existed to deliver a sealed strike to a dynamic agent whose chat-recovery schedule row was purged, back when routed chat recovery ran on Scheduler. Chat recovery now always uses Tasks (this PR), which has its own routed memory-limit bridge (setTaskRoutedMemoryLimitHandler), so this path carries no live traffic. Removed setSchedulerRoutedMemoryLimitHandler, the WeakMap backing it, the "memoryLimit" SchedulerRouteMessage variant, and Scheduler's onMemoryLimit/onRoute handling of it, plus Agent's wiring. Deliberately left in place: LEGACY_RECOVERY_LOOP_CALLBACKS' migration of pre-existing legacy schedule rows (still needed regardless of routing, so the local alarm breaker still counts them), and MemoryLimitContext.purgedRecoveryLoopJobs itself (now unread by any capability, but a generic hook a future routed capability with purge-as-a-pack semantics could still use — Tasks backs off one run at a time instead). Full suites still pass (2012 agents, 661 ai-chat, 888 think). Claude-Session: https://claude.ai/code/session_011DVPXpp9SyXM1ChxvkQCTB
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
1 participant
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.
What
Hardens the flaky timing-dependent tests in
packages/think/src/tests(test-only change, no changeset). These tests predate the facilities now available in the installed@cloudflare/vitest-pool-workersera (per-file storage isolation, deterministic DO helpers,vi.waitForin the workerd isolate), and several of them used wall-clock sleeps or tight fixed poll budgets as synchronization. Each fix below replaces a race with either polling for the observable effect or an explicit ordering primitive, without weakening what any test proves.Fixed races
1.
waitForAgentToolRuntight poll budget (agent-tools.test.ts)startAgentToolRunreturns{status: "running"}immediately and runs the whole child turn detached; the helper polled 20×10ms (~200ms) and then silently returned whateverinspectAgentToolRunsaid — non-terminal or not — so callers'toMatchObject({status: "completed"})failed whenever a full child turn took >200ms under CI load. Nowvi.waitForwith an 8s deadline asserts the status is terminal before returning: free when fast, and a clear timeout instead of a misleading non-terminal snapshot when not. Every caller still asserts the specific terminal status and payload afterward.2.
beforeSteppromise gate replaces 5ms-sleep-in-a-50ms-window (agent-tools.test.ts× 2,agents/think-session.ts)The "marks skipped agent-tool turns as errors" and "preserves explicit agent-tool cancellation as aborted" tests parked the child turn on a 50ms
beforeStepdelay and slept 5ms before resetting/cancelling — pure wall-clock ordering with a ~45ms margin. If the isolate stalled, the turn completed first andresetTurnStateForTest/cancelAgentToolRunno-oped on a sealed row (completedinstead oferror/aborted).ThinkTestAgentnow exposes a promise gate (holdBeforeStepForTest/hasEnteredBeforeStepForTest/releaseBeforeStepForTest): the test arms the gate, starts the run, polls until the turn is provably parked insidebeforeStep, performs the reset/cancel, then releases. Releasing after the reset means the resumed turn observes the generation bump and seals promptly. The cancellation test additionally waits for the released turn's finish path to complete (bookkeeping maps empty) and re-assertsaborted, preserving the original coverage that the guarded finalizer (UPDATE … WHERE completed_at IS NULL) never clobbers the aborted seal.3. WS chat clear used a 200ms sleep (
assistant-agent.test.ts)The clear frame travels the WebSocket while
getMessages()is an RPC — independent transports with no ordering guarantee, and_handleClearawaits_clearHistory()server-side. The fixed sleep is nowvi.waitForpolling until the persisted history reads 0 (messages only reach 0 via the clear, so it cannot pass spuriously; a never-applied clear still fails within 5s).4. Assistant-message persistence raced a 200ms sleep (
streaming-message-id.test.ts)Think broadcasts the
donechat frame before_persistAssistantMessagelands, so readinggetMessages()after a fixed sleep could miss the assistant row. Now the test polls for the persisted assistant message (8s deadline, explicit 15s test timeout) and then asserts the id equality; a wrongly-stamped start id still fails the equality, and a persistence regression fails at the poll deadline.5. Scheduled-recovery job counts raced the zero-delay alarm (
channel-recovery,agent-tool-reattach-recovery,action-pause-recovery,run-turn-recovery,think-sessiontests + both test agents)scheduleRecoveryarms a zero-delay alarm-backedcf_agents_jobsrow, and the workers pool fires due alarms as soon as the trigger RPC releases the DO — so a follow-upgetScheduledChatRecoveryCountForTest()RPC could observe the row already consumed and read 0 instead of 1.triggerFiberRecovery()now reads and returns the per-callback counts synchronously inside the same RPC invocation (before the DO is released), and the count-after-trigger assertions use the returned value. No test-only alarm suppression was added — the real alarm path stays exactly aseviction-recovery.test.tsexercises it, and the manual recovery drivers remain idempotent against an alarm that wins.6. Submission-drain alarm probe capped at ~200ms (
agents/think-session.ts)probeSubmissionAlarmOwnershipForTestwaited at most 20×10ms for the platform-scheduled drain alarm before the test assertedalarmDrainCalls === 1; alarm firing latency is not bounded by the probe's timer ticks. The poll now runs up to ~5s (200×25ms), still exiting on the first tick after the alarm fires and still fitting under the 10s test timeout with the follow-up submission wait. The test still proves alarm ownership — the platform-scheduled invocation, not an inline path, runs the drain.Deliberately not changed
retry: 3invitest.config.tsstays: the review notes for it conflicted (one endorsed lowering to 1, the later one endorsed keeping it for the main lane), and its companion recommendations (a nightly--retry=0lane, retry-count reporting, randomizing fixed DO names — claimed at ~65 sites but actually 320 across nine files, many with intentional intra-test reuse) are CI-config or bulk changes beyond this test-only pass.Verification
pnpm run test:workersinpackages/think: two consecutive fully green runs (887/887 both).pnpm run typecheck packages/think: green.oxfmt --checkandoxlintclean on the tests directory.https://claude.ai/code/session_011QZUJztM1rMTsHEC7mbcbz