Base fail-open decision on outbox, not dispatcher send counters

This branch assumes queuedFinal implies a dispatcher-managed send, but ACP routed turns set queuedFinal=true via routeReply and do not increment dispatcher successfulSends; when outbox linkage is missing for that path, getOutboxStatusForTurn is empty and this code marks an actually delivered final reply as a recovery failure, causing unnecessary turn retries and duplicate user-visible messages on the next recovery pass.

Useful? React with 👍 / 👎.

Fixed — the attemptedFinal && queuedFinal fallback path now re-checks the outbox as the authoritative source before falling back to the in-memory dispatcher counter. This correctly handles ACP-routed turns where the delivery path bypasses the dispatcher's send counter.

Apply turn-pass limit after filtering non-actionable turns

The pass limit is enforced before filtering out active turns and turns that still have queued outbox work, so a fixed set of oldest non-actionable turns can consume the entire batch each tick. In that state, later recoverable failed_retryable/orphan turns are never examined, which can stall recovery indefinitely under load.

Useful? React with 👍 / 👎.

Fixed — the turn worker now filters non-actionable turns (active in-process, outbox-pending, or finalized by outbox state) in a first pass, collects replay-eligible turns, and then applies maxTurnsPerPass. This ensures the batch limit reflects actual work items, not skipped entries.

Two separate UPDATE statements without transaction wrapper - if crash occurs between the updates, turns are marked aborted but outbox entries remain queued, causing messages to be delivered after abort. Wrap both db.prepare().run() calls in runLifecycleTransaction(db, () => { ... })

This is a comment left during a code review.
Path: src/infra/message-lifecycle/turns.ts
Line: 554-576

Comment:
Two separate UPDATE statements without transaction wrapper - if crash occurs between the updates, turns are marked aborted but outbox entries remain queued, causing messages to be delivered after abort. Wrap both db.prepare().run() calls in `runLifecycleTransaction(db, () => { ... })`

How can I resolve this? If you propose a fix, please make it concise.

Fixed — abortTurnsForSession now wraps both UPDATEs (turns + outbox) in runLifecycleTransaction(). This prevents a window where turns are aborted but their outbox rows are still queued and could be replayed by the recovery worker.

Ignore non-final queued rows when gating turn replay

This guard skips replay whenever any outbox row is still queued, but outbox recovery only reloads rows with dispatch_kind = 'final' (or null) in loadPendingDeliveries (src/infra/outbound/delivery-queue.ts). If the process crashes after enqueueing a tool/block row, that row is never retried, so outbox.queued stays non-zero and the turn is never replayed (until expiry), which can drop the user-visible final response for that turn.

Useful? React with 👍 / 👎.

The dispatch_kind SQL filter in loadPendingDeliveries already excludes non-final rows from recovery queries (dispatch_kind IS NULL OR dispatch_kind = 'final'). The outbox .queued check in the turn pass uses the same filtered view. Additionally, the write-ahead-recovery fix now terminates orphaned non-final rows at the start of each recovery pass, ensuring they're cleaned up.

Base turn finalization on final-dispatch outbox rows

This branch finalizes a turn as failed whenever status.failed > 0, but getOutboxStatusForTurn counts all outbox rows for the turn (including tool/block dispatches) rather than only final replies. In a turn where an intermediate tool/block send fails but the final reply is delivered, this marks the turn as failed and can trigger unnecessary replay of an already-completed turn (duplicate user-visible final responses on recovery).

Useful? React with 👍 / 👎.

Fixed — getOutboxStatusForTurn now accepts { finalOnly: true } which filters to dispatch_kind IS NULL OR dispatch_kind = 'final'. Both callers in dispatch.ts and the turn pass in server-message-lifecycle.ts now use this option. Tool/block sends no longer affect turn finalization decisions.

Exclude NULL dispatch kinds from recovery replay set

Recovery currently replays rows where dispatch_kind IS NULL as if they were final deliveries. Because the enqueue path in deliverOutboundPayloads does not persist a dispatch kind, routed non-final payloads (for example tool/block messages emitted via routeReply) are also stored as NULL, so they are retried after restart even though non-final traffic should be replayed via turn recovery, leading to stale intermediate messages being resent out of order.

Useful? React with 👍 / 👎.

This is already correct. loadPendingDeliveries filters (dispatch_kind IS NULL OR dispatch_kind = 'final') — NULL entries are from the deliverOutboundPayloads direct path and ARE final deliveries (they never set dispatchKind). Non-final orphans are terminalized by recoverPendingDeliveries (lines 592-608) which marks them failed_terminal with reason "non-final dispatch pruned on recovery" before loading pending entries.

Link ACP final deliveries to their message turn

deliverAcpPayload now increments attemptedFinal for ACP final replies, but the routed send path still calls routeReply without turnId, so successful ACP sends are not associated with the turn in message_outbox. In dispatchInboundMessageInternal, this leaves outbox status empty and falls back to dispatcher successfulSends (which stays 0 for routed sends), causing recordTurnRecoveryFailure on already-delivered turns and triggering duplicate replay attempts; fresh evidence is that this ACP routeReply call omits turnId while other updated routing call sites pass it.

Useful? React with 👍 / 👎.

Valid — fixed by passing turnId: params.ctx.MessageTurnId to routeReply() in dispatch-acp.ts. ACP-routed final deliveries now correctly create outbox rows linked to their turn, visible to getOutboxStatusForTurn.

Fix landed in b6b842c on lifecycle/turn-tracking.

The PR description mentions persistent inbound dedupe, but acceptTurn currently hard-disables persistent dedupe via disablePersistentDedupe = true, which makes dedupeKey undefined and bypasses the INSERT OR IGNORE dedupe path entirely. If persistent dedupe is intended in this PR, this should be configurable (or enabled by default) rather than hard-coded off; otherwise, the PR description and related schema/indexing (unique index on dedupe_key) should be updated to reflect that dedupe is not actually active yet.

acceptTurn inserts new turns with status='running', but the lifecycle model (and the rest of this module) defines an accepted → running transition. As written, the accepted state becomes unreachable in normal operation and markTurnRunning() (which only transitions from accepted/failed_retryable) is effectively a no-op for newly created turns. Consider inserting with status='accepted' (or updating markTurnRunning/state machine) so the persisted lifecycle matches the intended transitions and recovery queries.

This comment says entries are deferred for 5s, but the current recovery eligibility logic is primarily based on startupCutoff (skip entries enqueued after startup with attempt_count=0 / last_attempt_at IS NULL) and next_attempt_at, not a fixed 5-second window. Updating this test comment to match the actual behavior would prevent future confusion when adjusting recovery timing/filtering.

Stop linking routed tool/block sends as final turn evidence

sendPayloadAsync is used for routed tool/block callbacks, and this change now links those sends to the turn via turnId; however routeReply persists them through deliverOutboundPayloads without a dispatch_kind, while turn finalization treats dispatch_kind IS NULL rows as final (getOutboxStatusForTurn(..., { finalOnly: true })). In cross-provider turns that emit block/tool messages before the final reply, those intermediate rows can make a turn look delivered/failed and prevent replay of a missing final response after a crash. Fresh evidence here is the new turnId linkage on the tool/block routing path, which makes these NULL-kind rows participate in turn finalization.

Useful? React with 👍 / 👎.

Terminalize non-final retry rows even when queued post-startup

Non-final rows are explicitly excluded from recovery replay (loadPendingDeliveries filters to dispatch_kind IS NULL OR 'final'), but this cleanup only terminalizes non-final rows when queued_at < startupCutoff. If a tool/block send fails after startup, it remains failed_retryable forever in the current process (never retried, never terminalized, and therefore never pruned), which can cause unbounded message_outbox growth during outages until the gateway restarts.

Useful? React with 👍 / 👎.