Conversation
A turn whose initialization dependency never returned stayed in `initializing` forever with no timeout and no failure path, writing a presence heartbeat every 10s until the daemon restarted. Two sessions did this for 1h51m and 1h47m on 2026-09-16; the hung dependency was `dispatch.resolve_user`, whose span never ended. Initialization now carries a per-stage deadline clocked from the last published phase/detail change, so stages that report progress (a managed-runtime download's percentage) keep resetting it while a wedged one still trips. On expiry the presence controller stops its heartbeat and `notifyInitializationStalled` fails the turn through the existing `session_init_failed` path, which records a visible chat failure and returns the session to `idle`. This also unpins garbage collection: `hasActiveTurn` reads active presence, so a stalled turn made the session permanently uncollectable. Model: claude-opus-5[1m] Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 710c77e634
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| !turnRuntime.initializationStalled && | ||
| (turnRuntime.cancelRequested || | ||
| self.isTurnCancelled(sessionId, turnRuntime.turnId) || | ||
| wasInterrupted); |
There was a problem hiding this comment.
Cancel the pending create when initialization stalls
When the deadline fires while trackPendingSession is awaiting SessionManager.createSession—for example during a wedged managed-runtime install or ACP startup—the race interrupts only the Effect wrapper, while the underlying promise remains in SessionManager.pendingSessionCreates. This new condition then suppresses finalizeCancelledTurnEffect, so no cleanup is even scheduled; a retry calls createSession, receives the same hung promise from the deduplication map, and stalls again. The watchdog therefore does not provide the documented retry/self-healing behavior for these initialization stages; the stalled create must be abortable and removed from the manager, with any session that subsequently materializes terminated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 2b9aafc — the finding was correct, and the retry path was worse than "does not self-heal": it stalled again for the full budget.
Verified all three links in the chain:
pendingSessionCreatesis keyed by session id andcreateSessionreturns the cached in-flight promise (session-manager.ts:638-641), so the retry does get the same wedged promise.- The entry is cleared only by that promise's own
.finally()(session-manager.ts:647-655), which never runs for a create that never settles. requestSessionTerminate's pending-create branch did a bareawait terminationderived frompendingCreate(session-manager.ts:738-753), so the cleanup hung too — there was no escape hatch even manually.
Fix
Keeping finalizeCancelledTurnEffect skipped is still correct (it would mark the user's turn cancelled, which is wrong for a stall), but you're right that it also owned the pending-session release, and the stall is the first halt that can land while createSession is in flight. So the release moved into a dedicated finalizeStalledInitializationEffect, and wasCancelled went back to its original form now that branch ordering distinguishes the two.
SessionManager.abandonPendingSessionCreate(sessionId, reason)detaches the entry so the nextcreateSessionstarts genuinely new work.- The work itself can't be cancelled — there's no abort signal through
createSessionFromPreparationOrCold— so it's reaped: if the abandoned create ever yields a Session, that Session is terminated. The registry entry is dropped only while it still points at that instance, so a completed retry's Session is never unregistered. - Nothing awaits the wedged promise.
requestSessionTerminatenow races its wait against a 300s deadline (slowest healthy ACP start observed was 249s) using a sentinel rather than a rejection, so a genuineterminate()failure still propagates — that preserved the existingrejects when a still-starting session cannot be terminatedtest.
Tests
session-manager.test.ts covers the contract against the real dedupe map: a naive retry is handed the same wedged promise; abandoning detaches it so a retry completes; a create that materializes after abandonment is terminated; a wedged create makes requestSessionTerminate detach on its deadline instead of hanging.
session-execution-service.test.ts adds the end-to-end case: first create wedges → turn fails → the retry reaches agent.prompt and is recorded handled.
Ablation on that last one is the direct proof of your finding: removing only the abandonPendingSessionCreate call fails it on the leftover map entry, and removing that assertion too makes the retry itself hang for the full 30s vitest timeout.
Spec and Note updated in the same commit — the Spec now states the retry guarantee, that detaching is not cancellation, and the orphan-termination and bounded-teardown guarantees. pnpm check exit 0; full CLI suite 2872 passing.
Failing a stalled turn left the create itself in `SessionManager.pendingSessionCreates`. That map answers every `createSession` for a session id with the cached in-flight promise, and the entry is cleared only by the promise's own `finally`, so a wedged create was handed straight back to the user's retry — which stalled identically. The documented self-healing did not exist. `requestSessionTerminate` was no escape: its pending-create branch did a bare `await pendingCreate` and hung with it. Skipping `finalizeCancelledTurnEffect` on the stall path is still right (it would mark the turn cancelled), but that finalizer also owned the pending-session release, and the stall is the first halt that can land while `createSession` is in flight. The release moves to a dedicated `finalizeStalledInitializationEffect`. `abandonPendingSessionCreate` detaches the entry so a retry starts fresh. The work itself cannot be cancelled, so it is reaped: a Session it eventually produces is terminated rather than orphaned, and the registry entry is dropped only while it still points at that instance. `requestSessionTerminate` now races a 300s deadline through a sentinel, so a genuine terminate failure still propagates. Model: claude-opus-5[1m] Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b9aafcf97
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| this.sessions.delete(sessionId); | ||
| } | ||
| try { | ||
| await session.terminate(true); |
There was a problem hiding this comment.
Detach the orphan before terminating it
When an abandoned create later resolves after a retry has installed a replacement under the same session ID, this call terminates an instance whose SessionManager listeners are still attached. Session.terminate() emits terminated, and registerSessionEvents then unconditionally deletes sessions[event.sessionId] and forwards the event to MessageHandler, so the orphan can unregister and finalize the healthy replacement despite the identity guard above. The fresh evidence in this revision is this new reaper path; call detachSession(session) before terminating it, as the existing failed-create cleanup does.
AGENTS.md reference: apps/cli/AGENTS.md:L74-L78
Useful? React with 👍 / 👎.
Problem
A Session turn whose initialization dependency never returns has no timeout and no failure path. It stays in
initializingand keeps writing a presence heartbeat every 10 seconds until something external clears it.Verified in
~/.lody/logs/2026-09-16.log.1— session1b227b98:1h50m39s, ended only by a supervisor shutdown — not self-healing. Session
8fc8fceestalled identically from 05:02:43 and was cleared by the same shutdown.dispatch.resolve_usernever emitted itsendspan. The next attempt, in the restarted process, took 70061ms; the normal value is 1ms.resolveUserForRequestawaits that lookup unconditionally for any session with a project or parent.Across every retained log (923 initializations, 09-09..16): p50 0s, p90 4s, p99 13s. Only four samples exceed 60s — the two stalls above, a third 31-minute stall, and one genuine 249s cold
codex-acpstart.Fix
The watchdog measures silence, not duration: the clock runs from the last phase-or-detail change.
managed-runtimepublishes a rising download percentage, so a healthy transfer resets it continuously and gets unlimited legitimate wall-clock time; a wedged one still trips. Stages with no progress signal degrade to elapsed time.Per-stage budgets, because the honest worst cases differ by orders of magnitude:
initializingUSER_PROFILE_TIMEOUT_MS, the only intentionally slow dependency there; worst healthy observation was 70sacp/resuming/managed-runtimegit-cloneA flat timeout was rejected: any value tight enough to help the bookkeeping stage would kill a legitimate clone.
SessionActivePresenceControllerdetects. It already owns per-stage timing for the 120s slow-stage report and is the only module allowed to publish session presence. It does not clear presence itself (loro/AGENTS.mdreserves that for the owning Effect release) but stops its own heartbeat immediately, so the 10s wake-up of every presence subscriber ends at detection.SessionExecutionServiceenforces.awaitInitializationStallraces the turn body withEffect.raceFirstand never completes unless the watchdog fires, so a turn reachingrunningpays nothing. On a stall it goes through the existingrecordKnownChatFailureAndHaltEffectpath withsession_init_failed, producing a visible chat failure rather than a silent return toidle.Interrupting the fiber directly was rejected: the scope finalizer reads
Cause.isInterruptedand would have reported the stall as a user cancellation.initializationStalledlatches on the runtime to exclude it from that branch.Garbage collection
SessionGCManagerdoes not read presence directly, butisEligibleForCleanupcallshasActiveTurn, andMessageHandler.hasActiveTurnreturnsstate.turn.phase !== 'idle' || hasSessionActivePresence(sessionId). A stalled turn satisfies both halves, so such a session was permanently uncollectable. No separate fix needed: failing the turn releases the runtime and the presence entry.Tests
Two tests in
session-execution-service.test.tswire the realSessionActivePresenceControllerinto the service, with an injected clock and onlysetIntervalfaked so Effect's scheduler still runs — no real sleeps, no mock-call-count assertions. They assert observable state: chat failure reason and message, user-turn status, finalidlestatus, the presence clear event, that no further heartbeat is published however far the clock advances, and thathasActiveTurnreturns to false. The second drives a managed-runtime download 450s past a 60s budget while reporting progress, then wedges it.Ablation: disabling the watchdog, and separately removing the race while keeping the watchdog, each make both tests hang to the 30s vitest timeout — the production symptom.
Checks
pnpm checkexit 0;pnpm formatcleanpnpm run docs check—"errors": []Docs
specs/session-initialization-deadline.md(+ zh) — the guarantee is new externally-visible behavior2026-09-16-bounded-session-initialization.md(+ zh)apps/cli/src/lib/loro/AGENTS.mdgains the bounded-initializinginvariant (trimmed rationale elsewhere to stay under the 8 KiB gate)Not validated
The 900s and 1800s budgets have never been reached by a real download or clone, so they are bounds rather than measurements. Only the
initializingbudget is calibrated against observed stalls.🤖 Generated with Claude Code
Review follow-up (2b9aafc): detaching the wedged create
Codex flagged a P1 that was correct and, on verification, worse than reported — the retry path did not merely fail to self-heal, it stalled again for the full budget.
Failing the turn left the create in
SessionManager.pendingSessionCreates. That map answers everycreateSessionfor a session id with the cached in-flight promise, and the entry is cleared only by that promise's own.finally()— which never runs for a create that never settles.requestSessionTerminatewas no escape either: its pending-create branch did a bareawait pendingCreateand hung with it.Skipping
finalizeCancelledTurnEffecton the stall path is still correct (it would mark the turn cancelled), but it also owned the pending-session release, and the stall is the first halt that can land whilecreateSessionis in flight. That release moved into a dedicatedfinalizeStalledInitializationEffect;wasCancelledreturned to its original form now that branch ordering distinguishes the two.abandonPendingSessionCreatedetaches the entry so a retry starts genuinely new work.createSessionFromPreparationOrCold), so it is reaped: a Session it eventually yields is terminated rather than orphaned, and the registry entry is dropped only while it still points at that instance.requestSessionTerminateraces a 300s deadline through a sentinel rather than a rejection, so a genuineterminate()failure still propagates.New tests:
session-manager.test.tscovers the real dedupe map (naive retry gets the same wedged promise; abandoning lets a retry complete; a late-materializing create is terminated; teardown detaches on its deadline instead of hanging).session-execution-service.test.tsadds the end-to-end retry — wedge, fail, then the retry reachesagent.promptand is recordedhandled.Ablation proves it: removing only the
abandonPendingSessionCreatecall fails the retry test on the leftover map entry; removing that assertion too makes the retry hang for the full 30s timeout.Spec and Note updated in the same commit.
pnpm checkexit 0; full CLI suite 2872 passing.