feat(workflows): schedule workflow heartbeats and queue parent delivery - #2377
Conversation
Terminal evidence — real CLI, 1-minute cadenceA real interactive Atomic session driven through tmux, with a workflow authored at tmux new-session -d -s hb-e2e -x 200 -y 50
bash scripts/e2e/workflow-heartbeat-evidence.sh hb-e2e /tmp/hb-evidenceRun dispatched — the last thing typedBoth cards, one interval apart
Full captures, including the CLI start and the scrollback-joined pane the harness asserts against, are committed under |
The message_end parser and the persisted-anchor decode both walked untyped payloads with a cast at every hop, so the compiler enforced none of the shapes they depend on. Declare HostMessagePart/HostMessage/HostMessageEndEvent and PersistedAnchorOutput, validate once at each boundary, and let every consumer downstream read a declared type. unknown stays as the parameter type at both doors, because that is what genuinely arrives from the host and from a previous process; what changes is that it is narrowed exactly once instead of re-cast at each field. Runtime validation is unchanged: the same payloads are rejected, and a malformed record is still not trusted because a type claims a shape. Addresses the P2 review comment on #2377. Assistant-model: Claude Opus 5
…heckpoint that needs it The scheduler's launch-record write is asynchronous and guarded against runs with no durable progress, so it can only be issued after the run is already resumable. A process exiting inside that window left a resumable run with no record of what it launched with; the next process then read a freshly minted startedAt and whatever cadence the definition declared by then, shifting both the phase and the cadence of a run already in flight. Persist the record at the durability boundary itself, for every stage-session checkpoint rather than only a forced pause/quit capture, and await it so it is durable before that checkpoint is acknowledged. The write is write-once and returns early once present, so later passes cost one mirror read. The regression fails without this: an ordinary first checkpoint with the scheduler's write stranded resumed at the edited 30-minute cadence from a fresh start instead of the original 15-minute series. Addresses the P1 review comment on #2377, which is fixed here because this is where the checkpoint-commit boundary lives. Assistant-model: Claude Opus 5
849702c to
9097e18
Compare
Slice 2 of issue #1975. A positive `heartbeatIntervalMinutes` now produces real heartbeats in the parent chat. Cadence is derived, not stored a second time: boundaries are `startedAt + n * interval` computed from the already-persisted `RunSnapshot.startedAt` and the frozen definition value, so a slow send, a retry, or a process restart cannot shift it and there is no parallel source of truth to invalidate. A resolved interval of 0 creates no timer and no schedule record at all. One globally-next-due wake-up serves every enabled active run rather than one recurring timer per run, and each run holds at most one pending heartbeat, so a later boundary is skipped rather than queued behind an in-flight one. Delivery reuses the lifecycle-notice parent path (`triggerTurn`, `deliverAs: "steer"`, `persistWhenStreaming`) under the distinct `workflows:workflow-heartbeat` custom type with the unchanged five-field slice-1 payload, so a busy parent is queued and never interrupted. Paused runs emit nothing and receive no backfill; a resumed or restarted run takes the first future boundary. Simultaneously due heartbeats process in `scheduledAt` order with the run id as the stable tie-break, terminal state is re-checked immediately before enqueue and again immediately before processing, and nested child runs never heartbeat the parent chat. Terminal cleanup and restart-recovery invalidation remain slice 3; only the pre-enqueue and pre-process guards land here. Refs: #1975 Assistant-model: Claude Opus 5
The host resolves `sendMessage` when a protected streaming custom message is admitted into the parent's queue, not when the parent reads it (`queueProtectedStreamingCustomMessage` returns after the push). Releasing the pending slot on that resolution let the next boundary admit a second heartbeat while the first was still queued, which is exactly the stacking the objective forbids. The slot now stays occupied until the parent reports settling its turn and draining its queued messages, observed through the public `agent_settled` extension event and registered once at runtime-state construction because `pi.on` has no unsubscribe. A one-interval hold cap keeps a host that never reports settling from silencing a run: the worst case is one skipped boundary, never permanent silence. Release re-arms from `max(now, lastEnqueuedAt)`, so a skipped boundary is dropped rather than backfilled. Terminal state is now re-read before every delivery attempt, retries included, so a run that finishes inside the capped-backoff window is suppressed instead of replayed. Invalidating a heartbeat the host has already admitted remains slice 3's door; no public host seam exposes it. The three user-facing texts stated a guarantee the host does not provide and now state the one it does. The slice-1 changelog bullet's closing sentence, which claimed a positive interval does not yet emit heartbeats, is removed as contradicted by the entry directly below it; both bullets are inside [Unreleased] and no released section is touched. Refs: #1975 Assistant-model: Claude Opus 5
… durably Three defects from the round-3 review. The one-interval hold cap breached the clause it was meant to protect. Once it elapsed, `processDue` swept the held slot and admitted the next boundary while card #1 was still unread in the host's steering queue — two events for one run. "Retain exactly one pending event" is about the event, not about the scheduler's own map. The cap is gone; the hold now ends only when the parent reports picking the card up. The liveness worry the cap answered is now answered by wiring instead of by a deadline: the scheduler holds a slot only when the host actually reports parent availability, and `extension-runtime-state` derives that flag from the same condition that registers the `agent_settled` handler, so the two cannot disagree. A host that reports nothing releases on admission and keeps heartbeating; a host that reports pickup holds indefinitely. `ExtensionAPI` declares `on` as required, so the shipped host always takes the holding branch — which the tmux capture confirms with cards at 1m and 2m and no cap behind them. `nextWorkflowHeartbeatBoundary` probed only n and n+1, which is not enough when the cadence is finer than one ULP at the anchor. At a real epoch anchor near 1.7e12 one ULP is 0.000244 ms, so a 1e-9-minute interval first lands a representable step at n=3 and the function called an authoring-valid cadence unschedulable. A bounded bracket-and-narrow now finds the smallest such n, still exactly on the anchored series. The existing tests could not catch this because their 1e6 anchor has a ULP six orders of magnitude smaller. Slice 2 now also defines the durable schedule record spec section 5.5 asks for: a reserved tool checkpoint mirroring `durable/run-timing.ts`, written on enqueue only, never for a non-positive interval, and read back only as a monotone floor. A restored floor is always in the past, so the record is structurally incapable of moving a boundary — its behavioral delta is zero by construction, which is what keeps it from being a second source of truth. Both sides are best-effort, because `getDurableBackend()` throws until DBOS is ready and the scheduler installs at session start. Refs: #1975 Assistant-model: Claude Opus 5
…ng owed boundaries Five defects from the round-4 review, four of them consequences of the last-boundary durable record added in 7ed9e9278. That record is replaced by a write-once cadence *anchor*. The old shape was wrong in three separate ways at once, and all three trace to storing a value that wants updating in a store that cannot express an update: a per-boundary `checkpointId` meant one row per heartbeat rather than one per run; `recordCheckpoint` sets `completedCheckpoints = checkpoints.size`, so writing for a run with no progress of its own made it look durably resumable; and `completedAt: scheduledAt` landed on the handle as `updatedAt`, walking the 120 s foreign-liveness window backwards. The anchor has a stable checkpoint id, so the backend's duplicate-key early return makes it exactly one immutable row; it skips runs with no other checkpoint, exactly as `run-timing.ts` does and for the same reason; and it stamps write time. Its reserved name joins `RUN_TIMING_CHECKPOINT_NAME` at both reconstruction filter sites, so it never surfaces as a phantom cached tool node. The anchor also earns its place, which the last-boundary record never did. A durable resume re-dispatches under the original workflow id but mints a fresh `RunSnapshot.startedAt`, and `registerWorkflow` overwrites `createdAt` while keeping checkpoints — so a checkpoint is the only surviving carrier of the original start, and without it a resumed run silently started a new series. The scheduler now anchors at `min(run.startedAt, anchorAt)`, which can only restore the original cadence, never advance it. The floor stays `now`, so a resumed run still takes the first future boundary. A continuation carries a fresh run id and legitimately starts a new series. Separately, `refresh()` re-derived every run's schedule on every store invalidation, flooring at `now`. Any ordinary mutation landing between a boundary coming due and its armed callback advanced the entry to the following boundary and dropped the heartbeat the cadence owed. An entry that has already come due is now sticky, and keeps the `intervalMinutes` it was derived with. The test clock gained `advanceWithoutFiring`/`fireDue`, without which no test could interleave a mutation with a late timer. Refs: #1975 Assistant-model: Claude Opus 5
`agent_settled` is emitted from the prompt cycle's `finally` whether or not the queued messages were drained. Pausing the queue mid-turn (ESC) and aborting therefore released a slot whose card was still parked in the hold, and the next boundary stacked a second card behind it — two unread cards per run during one pause episode, which is exactly what the one-pending clause forbids. `message_end` is the host's injection signal: agent-core emits it at the moment a message enters the conversation, and never for a parked one. The visible display card is published on the session-listener channel only, so an extension observes the hidden reconciliation at consumption and never the card at admission — there is no admission/consumption ambiguity to disambiguate. The reconciliation copies our `content` verbatim, so the scheduler matches on the exact string it authored and releases only that run. The release door is now `notifyHeartbeatConsumed(content)`, the held slot stores that content, and `extension-runtime-state` subscribes to `message_end` instead of `agent_settled`. The no-signal fallback is unchanged: a host without `pi.on` still releases on admission rather than going silent. This was verified before it was designed in, not after. A new host-level regression drives a real `AgentSession` through stream, admit, pause, abort, resume, and driver, and asserts that the turn settles with nothing consumed while the card is parked, and that the restored card reaches extensions verbatim on consumption. Reverting the role guard in the payload narrowing fails that regression, so it exercises the production path. Refs: #1975 Assistant-model: Claude Opus 5
… the launch definition Two defects, both about a value being read at the wrong moment. The cadence anchor was persisted only when a heartbeat was delivered, so nothing was recorded for a whole interval — fifteen minutes at the default. A run that gained durable progress and then paused, quit, or crashed before its first boundary resumed on a fresh series, because a durable resume reclaims the original run id but mints a new `startedAt`. The write moves to schedule time, still behind the no-progress guard that keeps it from manufacturing resumability. Any durable progress bumps the store, which triggers a refresh, so the anchor lands on the first pass after the run becomes resumable rather than at its first delivery. A per-run persisted set stops the durable call after its first success, since a schedule pass runs on every invalidation for every running run. The interval was re-read from the live registry on every pass, so `/workflow reload`, a rename, an edit, or a delete changed or stopped a run already in flight. Nothing else in the system lets a reload reach a live run: the executor closes over its own definition for the life of the run. The scheduler now memoizes the interval the first time it resolves for a run and uses that thereafter. Only a successful resolution memoizes, so a run observed during startup warmup picks its cadence up on a later pass instead of being frozen as unresolvable. The E2E script's step-4 comment still described `agent_settled` as the load-bearing signal on the one step whose purpose is to prove `message_end` replaced it. Corrected, along with the two doc sentences that said the anchor is written at first heartbeat. Refs: #1975 Assistant-model: Claude Opus 5
…s disabling silently Two defects. The launch-cadence memo only stored a positive value, and only in process. A run launched at `0` never memoized, so editing that workflow to a positive cadence and reloading started heartbeating a run that launched with heartbeats off. And the memo is cleared at session shutdown while the durable anchor carried only `anchorAt`, so a durable resume in a later process re-resolved against whatever the registry then held — the cadence shifted across exactly the boundary the objective says it must not. The memo now records the resolved value before the positivity test, so `0` is remembered as "launched disabled", and the write-once anchor record carries `intervalMinutes` beside `anchorAt` so a positive launch cadence survives a process boundary. A persisted cadence outranks the live registry; its absence on records written by earlier builds reads as unknown and falls back to the registry, never as `0`. A disabled run still writes nothing. The acceptance criterion "disabled (0) creates no timer or record" is flat, so the zero case is fixed in process only. The residue is real and is documented rather than hidden: a run that launched disabled and is resumed in a new process adopts whatever the workflow then declares, because nothing durable may record that it launched disabled. Separately, two very different extreme intervals both silently disabled. A sub-ULP cadence has a real first series member — at `Number.MIN_VALUE` minutes it rounds to exactly one ULP past the anchor — but no representable `n` to name it with, so the bracket-and-narrow overflowed and gave up. It now returns that instant, which is the same rounding the rest of the function performs. A cadence above ~3e303 minutes has no representable boundary at all, because `intervalMs` is already Infinity; that still returns nothing, but warns under ATOMIC_WORKFLOW_DEBUG instead of being silent, and is documented. Refs: #1975 Assistant-model: Claude Opus 5
…durable write Two defects. The third finding in this round is blocked on a contract amendment and is deliberately not implemented; see the notes. Ordering was only enforced on the order attempts *start*. `processDue` sorts a due batch by scheduledAt then runId and hands identities to the delivery layer in that order, but each identity then retried independently: if the first send was rejected it entered backoff while the next identity was admitted during that window, so the parent observed B before A. Delivery now holds one FIFO admission queue and runs only its head, releasing it when that identity reaches a terminal result — admitted, suppressed by its live guard, or out of attempts. The guard still runs immediately before every attempt, including the first attempt of an identity that waited behind another, because its run may have finished while it queued. The anchor write trusted a synchronous boolean. `DbosDurableBackend.record- Checkpoint` updates its in-memory mirror first and queues the real write after, so the immediate read-back proved only that the mirror held the row: a rejected storage write was remembered as persisted and never retried. The writer now uses `recordAdditiveCheckpointBestEffort`, which awaits `recordStepOutput` and updates the mirror only after it resolves, and the scheduler marks a run persisted only when that promise resolves true. A failure clears the in-flight marker and leaves the retry to the next natural schedule pass rather than looping. The regression for the second fix is DBOS-backed on purpose: the in-memory backend cannot tell the two write paths apart, and a first attempt at this test passed against the reverted implementation. Refs: #1975 Assistant-model: Claude Opus 5
The largest cadence whose millisecond conversion is still representable schedules at Number.MAX_VALUE. Pin that this entry stays behind an ordinary due heartbeat in the global schedule: both entries coexist, the single timer arms for the one-minute boundary, and only that run reaches the parent. This narrows the regression surface while the distinct Number.MAX_VALUE minutes case remains blocked on a user decision: its minute-to-millisecond conversion is Infinity, and every proposed representation changes the literal contract. Refs: #1975 Assistant-model: GPT-5.6 Sol
A cadence above roughly 3e303 minutes overflows startedAt + interval past the largest finite timestamp, so the boundary series has no first member. Slice 1 accepts every positive finite interval and slice 2 schedules at startedAt + n * interval; both cannot hold there. Document the limit rather than saturate the boundary, which would put a value outside the cadence series into scheduledAt, the field heartbeat identity is keyed on. Behavior is unchanged: nothing is scheduled, no durable record is written, and 0 remains the only value that declares heartbeats off. Adds the real-CLI tmux evidence for the slice at a 1-minute cadence. Assistant-model: Claude Opus 5
The message_end parser and the persisted-anchor decode both walked untyped payloads with a cast at every hop, so the compiler enforced none of the shapes they depend on. Declare HostMessagePart/HostMessage/HostMessageEndEvent and PersistedAnchorOutput, validate once at each boundary, and let every consumer downstream read a declared type. unknown stays as the parameter type at both doors, because that is what genuinely arrives from the host and from a previous process; what changes is that it is narrowed exactly once instead of re-cast at each field. Runtime validation is unchanged: the same payloads are rejected, and a malformed record is still not trusted because a type claims a shape. Addresses the P2 review comment on #2377. Assistant-model: Claude Opus 5
feaba6a to
a493362
Compare
…heckpoint that needs it The scheduler's launch-record write is asynchronous and guarded against runs with no durable progress, so it can only be issued after the run is already resumable. A process exiting inside that window left a resumable run with no record of what it launched with; the next process then read a freshly minted startedAt and whatever cadence the definition declared by then, shifting both the phase and the cadence of a run already in flight. Persist the record at the durability boundary itself, for every stage-session checkpoint rather than only a forced pause/quit capture, and await it so it is durable before that checkpoint is acknowledged. The write is write-once and returns early once present, so later passes cost one mirror read. The regression fails without this: an ordinary first checkpoint with the scheduler's write stranded resumed at the edited 30-minute cadence from a fresh start instead of the original 15-minute series. Addresses the P1 review comment on #2377, which is fixed here because this is where the checkpoint-commit boundary lives. Assistant-model: Claude Opus 5
Stack for #1975
Merge bottom to top; each PR is based on the one below it.
Base of the stack is
main. #2377, #2379 and #2383 are linked as GitHub stack #2380. #2229 cannot join it because its head branch lives on a fork, so this table carries the full chain.Slice 2 of three for #1975. Stacks on
feat/workflow-heartbeat-config(PR #2229) and must merge after it.Slice 1 added the authoring contract but nothing scheduled anything, so a positive
heartbeatIntervalMinutesdelivered no heartbeats. This slice makes the cadence real.What it owns
startedAt + n × intervalboundary, computed from the run's persisted start time and never from the previous delivery, so a slow send, a retry, or a process restart cannot shift it.0creates no timer and no durable record at all.triggerTurn,deliverAs: "steer",persistWhenStreaming) under the distinctworkflows:workflow-heartbeattype with slice 1's five-field payload.scheduledAtorder with run id as the stable tie-break, and a retried heartbeat holds its place. Terminal state is re-checked before enqueue, before processing, and before every delivery attempt. Nested child runs never heartbeat the parent chat.Out of scope
Terminal cleanup and restart-recovery invalidation are slice 3, per
specs/2026-08-06-workflow-heartbeats.md. The pre-enqueue and pre-process terminal guards are here; the cleanup door is not.One design decision worth a reviewer's eye
Slice 1 accepts every positive finite interval; slice 2 schedules at
startedAt + n × interval. Above roughly3 × 10^303minutes that product overflows toInfinityand the series has no first member, so both clauses cannot hold. This documents a representable upper limit rather than saturating the boundary atNumber.MAX_VALUE. Saturating would put a value that is not a member of the cadence series intoscheduledAt— the field heartbeat identity is keyed on. Behavior is unchanged and0remains the only value that declares heartbeats off.Verification
npm run checkandnpm run test:unit(642 files, 6257 tests) pass. Scheduler and parent-pickup behavior is covered by deterministic injected-clock tests, never by real elapsed time. Real-CLI tmux evidence at a 1-minute cadence is committed underresearch/evidence/workflow-heartbeat-scheduler/and quoted in a comment below.Part of #1975
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Greptile Summary
This change adds workflow heartbeat scheduling with durable cadence metadata, ordered parent delivery, and recovery behavior that preserves a run’s original timing.
Confidence Score: 5/5
No blocking failure remains.
No accepted P0 or P1 findings remain, so the score is 5.
Reviews (3): Last reviewed commit: "refactor(workflows): give heartbeat boun..." | Re-trigger Greptile
Context used: