fix(server): persist run failures after storage recovers#19646

_{P2 Badge} Preserve the failure time across write retries

When storage stays unavailable across one or more backoffs, this retry re-executes writeFinalRunEvents, whose first step samples DateTime.now (RunExecutionService.ts:604) and uses it for the run, attempt, node, provider thread, and failure item timestamps. The eventual records therefore describe the recovery/last-retry time rather than when ingestion failed; a long outage inflates the apparent run duration and changes thread recency because packages/client-runtime/src/state/threadSort.ts:37-47 sorts using latestRun.completedAt. Capture the failure timestamp once before entering the retry and pass that stable value to each write attempt.

Useful? React with 👍 / 👎.

Fixed in a12404ff23: failure finalization now captures completedAt once before retry and passes it through the existing helper, so storage downtime does not extend the recorded run. The typed-error and defect regressions assert the original completion timestamp; both failed before this correction and pass afterward. Final focused tests: upstream 55/55 and fork carry 60/60; server typecheck and touched-file checks passed. Independent review also tightened recovery acquisition/cleanup with Effect.acquireUseRelease, preserving service shutdown and consumer cancellation.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Close eventSubscription before starting failure-finalization recovery.

input.session.subscribeEvents uses an unbounded queue. publishToSubscribers offers each provider event to that queue. When Stream.runDrain fails, Effect.catchCause retries writeFinalRunEvents without an attempt limit. The outer Effect.ensuring(eventSubscription.close) does not run until recovery finishes, so no consumer drains the queue while storage is unavailable. The queue can grow for the duration of the retry.

Move the finalizer before Effect.catchCause. close removes the subscriber, clears its queue, and ends it. Recovery does not use eventSubscription.

This queue-growth scenario is already prevented by the managed stream's inner finalizer. ProviderSessionManager.makeEventSubscription constructs events as Stream.fromQueue(queue).pipe(..., Stream.ensuring(close)). When Stream.runDrain fails, that stream scope closes before the outer Effect.catchCause enters failure-finalization recovery. close removes the subscriber, clears the queue and ends it. The later Effect.ensuring(eventSubscription.close) is an idempotent outer cleanup; delaying that second call does not keep the managed queue registered. The fallback session.events path has no managed subscription and uses Effect.void for close. Moving the outer cleanup is therefore unnecessary for the reported leak. Confirmed against a12404ff23; retaining the current ordering and resolving this as a false positive.

@Alb11747 Thanks for the clarification. The inspected code confirms your explanation.

ProviderSessionManager.makeEventSubscription attaches Stream.ensuring(close) to the managed stream. When Stream.runDrain fails, that finalizer runs before the outer Effect.catchCause starts recovery. close removes the subscriber, clears the queue, and ends it. The outer cleanup is idempotent.

My finding missed the inner finalizer. The reported queue-growth scenario does not apply, so I withdraw the finding. No ordering change is needed.

✅ Review thread resolved.

You are interacting with an AI system.

_{P2 Badge} Keep recovery alive until startup finalization persists

When ingestion fails while startRootRun is still awaiting shouldStartProviderTurn or startTurn, and that startup path also fails, the startup catch at RunExecutionService.ts:1469-1505 interrupts providerEventFiber; this release then kills the new recovery before the catch's single writeFinalRunEvents attempt. If storage is still unavailable for that attempt, the run has already been committed as running, and ProviderTurnStartService.start treats the outbox retry as a no-op for every non-starting run (ProviderTurnStartService.ts:229-231), so the run remains Running after storage recovers. Keep this recovery alive until the replacement finalization commits, or retry that replacement write as well.

Useful? React with 👍 / 👎.

Fixed in ac34e940d6. Established ingestion recovery now belongs only to the service scope, so interrupting the provider-event consumer during startup failure does not cancel its last durable failure write. It still stops when the atomic run/attempt/Running gate rejects it or when the service shuts down. The managed event subscription closes during stream unwinding; it is not retained by recovery. A new regression fails on a12404ff23: ingestion finalization and startup finalization both fail, then storage recovers while the service stays open. It now records the failed terminal. Companion coverage verifies a successful startup terminal rejects delayed recovery. Focused tests: upstream56/56, fork61/61; touched-file checks pass. Correction: CI found a test-hook type error on previous e6f1263fd0. Earlier local wrappers masked the compiler's nonzero exit, so my typecheck-pass claim was incorrect. The amended test hook now accepts the existing RunExecutionServiceV2Error union; both compiler processes exit0. Independent pre-push review is clean and the amendment is pushed; fresh CI/review is pending. This replaces the previous consumer-owned resource bracket with the simpler service-scoped fork/join; it does not add provider replay or expand startup-write retries.