fix(diagnostics): reclaim wedged session lanes with a stale leaked active run#86056
Conversation
|
Codex review: needs maintainer review before merge. Reviewed May 25, 2026, 9:15 AM ET / 13:15 UTC. Summary PR surface: Source +74, Tests +130, Docs +1. Total +205 across 5 files. Reproducibility: Do we have a high-confidence way to reproduce the issue? Yes at source and harness level: current main keeps Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land the focused liveness-gated recovery after maintainer acceptance of the abort-threshold policy, keeping the existing diagnostics threshold and regression coverage. Do we have a high-confidence way to reproduce the issue? Do we have a high-confidence way to reproduce the issue? Yes at source and harness level: current main keeps Is this the best way to solve the issue? Is this the best way to solve the issue? Yes, subject to maintainer policy acceptance: it reuses the existing diagnostic activity signal and AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 8fe4f34af224. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +74, Tests +130, Docs +1. Total +205 across 5 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Clockwork Proofling Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
55f4171 to
87df9d4
Compare
c0db2f3 to
9cfee88
Compare
9cfee88 to
e9ad954
Compare
e9ad954 to
a81d041
Compare
a81d041 to
42709dc
Compare
42709dc to
97d0d34
Compare
…tive run A group session lane could wedge permanently (openclaw#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes openclaw#85639
4c1d613 to
09084c5
Compare
|
Verification for PR #86056 on head Behavior addressed: stale active-run and reply-work bookkeeping no longer leaves queued session work wedged when there is no forward progress. node scripts/run-vitest.mjs src/logging/diagnostic-stuck-session-recovery.runtime.test.ts
git diff --check origin/main...HEAD
./node_modules/.bin/oxfmt --check --threads=1 src/logging/diagnostic-stuck-session-recovery.runtime.test.ts
/Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode branch --base origin/mainEvidence after fix: focused diagnostics regression suite passed with 20 tests; diff check and oxfmt check passed; autoreview reported no accepted/actionable findings; CI run |
…tive run (#86056) * fix(diagnostics): reclaim wedged session lanes with a stale leaked active run A group session lane could wedge permanently (#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes #85639 * test(diagnostics): cover stale active run recovery --------- Co-authored-by: Peter Steinberger <[email protected]>
…tive run (openclaw#86056) * fix(diagnostics): reclaim wedged session lanes with a stale leaked active run A group session lane could wedge permanently (openclaw#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes openclaw#85639 * test(diagnostics): cover stale active run recovery --------- Co-authored-by: Peter Steinberger <[email protected]>
…tive run (openclaw#86056) * fix(diagnostics): reclaim wedged session lanes with a stale leaked active run A group session lane could wedge permanently (openclaw#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes openclaw#85639 * test(diagnostics): cover stale active run recovery --------- Co-authored-by: Peter Steinberger <[email protected]>
…tive run (openclaw#86056) * fix(diagnostics): reclaim wedged session lanes with a stale leaked active run A group session lane could wedge permanently (openclaw#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes openclaw#85639 * test(diagnostics): cover stale active run recovery --------- Co-authored-by: Peter Steinberger <[email protected]>
…tive run (openclaw#86056) * fix(diagnostics): reclaim wedged session lanes with a stale leaked active run A group session lane could wedge permanently (openclaw#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes openclaw#85639 * test(diagnostics): cover stale active run recovery --------- Co-authored-by: Peter Steinberger <[email protected]>
…tive run (openclaw#86056) * fix(diagnostics): reclaim wedged session lanes with a stale leaked active run A group session lane could wedge permanently (openclaw#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes openclaw#85639 * test(diagnostics): cover stale active run recovery --------- Co-authored-by: Peter Steinberger <[email protected]>
…tive run (openclaw#86056) * fix(diagnostics): reclaim wedged session lanes with a stale leaked active run A group session lane could wedge permanently (openclaw#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes openclaw#85639 * test(diagnostics): cover stale active run recovery --------- Co-authored-by: Peter Steinberger <[email protected]>
…tive run (openclaw#86056) * fix(diagnostics): reclaim wedged session lanes with a stale leaked active run A group session lane could wedge permanently (openclaw#85639): an embedded run that dies abnormally leaves a stale ACTIVE_EMBEDDED_RUNS handle, so the diagnostic heartbeat classifies the lane stale_session_state (recoveryEligible without allowActiveAbort) while stuck-session recovery reads the leaked isEmbeddedPiRunActive flag and skips with active_reply_work — a tautology that keeps the lane forever. The age-based escape never fires because ageMs (last-activity) resets on every incoming queued message. Make the active-run skip a liveness check: before keeping the lane, consult the run's real forward-progress age (lastProgressAgeMs, not refreshed by incoming messages). If a run flagged active has made no forward progress past the resolved diagnostics.stuckSessionAbortMs threshold (threaded through the recovery request; falls back to a 5-minute floor) with queued work waiting, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run, or one within an operator-raised threshold, is kept. Fixes openclaw#85639 * test(diagnostics): cover stale active run recovery --------- Co-authored-by: Peter Steinberger <[email protected]>
Summary
queueDepth=1) but never dispatch. Stuck-session recovery detects the wedge but skips withreason=active_reply_work action=keep_lane, repeating every ~2 minutes (the reporter saw 23+ days). The bot goes silent with no user-visible error. (Group session lane wedges indefinitely: stuck-recovery skips with active_reply_work false positive #85639)ACTIVE_EMBEDDED_RUNSkeeps a leaked handle andisEmbeddedPiRunActive(...)staystrue, while the separate diagnostic run-activity tracking already shows no active run.stale_session_state(queued_work_without_active_run), which isrecoveryEligiblewithoutallowActiveAbort(that grant is reserved forsession.stalledclassifications).recoverStuckDiagnosticSessionthen reads the leakedisEmbeddedPiRunActiveflag and, becauseallowActiveAbort !== true, skips withactive_reply_work— a tautology: it keeps the lane because the lane looks active.ageMsis the session's last-activity age, which resets every time a new message queues, so it never crosses a timeout (the reporter's age stayed ~124s for weeks). The existing progress-based abort (lastProgressAgeMs) only runs forsession.stalledclassifications, notstale_session_state.active_embedded_run/active_reply_work, consult the run's real forward-progress age (getDiagnosticSessionActivitySnapshot(...).lastProgressAgeMs, which is touched by tool/model/chunk events, not by incoming queued messages). If a run flagged active has made no forward progress for ≥ a staleness window and there is queued work, treat it as a leaked/dead handle and reclaim it (abort + drain + force-clear) instead of skipping. A genuinely progressing run (recentlastProgressAgeMs) is still kept.src/logging/diagnostic-stuck-session-recovery.runtime.ts: addisActiveRunProgressStale(queued work + stalelastProgressAgeMs); both theactive_embedded_runandactive_reply_workskip branches reclaim instead of skipping when the active run's progress is stale.src/logging/diagnostic-session-recovery.ts+src/logging/diagnostic.ts: thread the resolveddiagnostics.stuckSessionAbortMsinto the recovery request (staleActiveProgressAbortMs) so the reclaim honors an operator-raised abort threshold; it falls back to theMIN_STALLED_EMBEDDED_RUN_ABORT_MS(5 min) floor when not supplied, matching the existing stalled-run abort policy.src/logging/diagnostic-stuck-session-recovery.runtime.test.ts: regression coverage.allowActiveAbortpath, the diagnostic classifier, the heartbeat, and the run registries are unchanged. The reclaim is additive: it only fires when an active flag is paired with stale forward progress and queued work, so existing skip/abort behavior is otherwise identical.Reproduction
ACTIVE_EMBEDDED_RUNShandle is never cleared (diagnostic activity already shows no active run).stale_session_state, recovery reads the leaked active flag and skips withactive_reply_work— the lane never frees (permanent wedge).Real behavior proof
active_reply_work; a still-progressing run must not be reclaimed.recoverStuckDiagnosticSessiondecision path exercised under Vitest (Linux, Node 22). The run/abort registry and lane seams are mocked exactly as the existing 15-case suite in this file does; the recovery logic, the new liveness gate, andlastProgressAgeMsthresholding are real.pnpm test src/logging/diagnostic-stuck-session-recovery.runtime.test.ts.c91c3c6e5a): the new staleness-reclaim case fails because the unpatched recovery skips (the wedge); the two control cases (still-progressing, no queued work) pass because the unpatched path also keeps the lane:tsx /tmp/qmd85639/repro.mts, Linux/Node 22) drives the realrecoverStuckDiagnosticSession, the real embedded-run registry (setActiveEmbeddedRunleaks a handle to simulate abnormal death), and the real diagnostic run-activity tracker; the clock is advanced so the run's reallastProgressAgeMscrosses the window. FRESH (recent progress) keeps the lane; STALE (10 min, no progress, queued work) reclaims it (real abort + force-clear), after which the handle is gone (active=false):(The STALE run also emitted the real gateway logs
stuck session recovery reclaiming stale active run …and… action=abort_embedded_run aborted=true forceCleared=true.)reclaiming stale active …); the still-progressing run and the no-queued-work cases still keep the lane (no false reclaim).diagnostics.stuckSessionAbortMs(default ≥ 5 min) so a live-but-slow run is not reclaimed.Risk / Mitigation
lastProgressAgeMs, which is not refreshed by incoming queued messages) is stale for ≥ the resolveddiagnostics.stuckSessionAbortMs(operator-configurable; default ≥ 5 min) and there is queued work; a run making any recent progress is kept. An operator who raises the threshold to protect slow active work is honored (covered by a raised-threshold test: stale at 10 min is kept, at 25 min is reclaimed). This mirrors the existingblocked_tool_callabort which already triggers at the samelastProgressAgeMs/threshold scale. Without queued work, nothing is reclaimed.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Fixes #85639