[codex] Fix stale active work session recovery#95778
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 27, 2026, 4:44 AM ET / 08:44 UTC. Summary PR surface: Source +52, Tests +94. Total +146 across 6 files. Reproducibility: yes. at source level: current main keeps model-call recovery gated on active embedded-run ownership, and the PR head computes the new routed JSON summary from raw task rows. No live Telegram/provider reproduction was supplied for this head. 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. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Either narrow this PR to a corrected task JSON summary that uses the intended inspectable projection, or restore a bounded diagnostic recovery fix with live proof and coordinate it with the canonical stale model-call issue. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main keeps model-call recovery gated on active embedded-run ownership, and the PR head computes the new routed JSON summary from raw task rows. No live Telegram/provider reproduction was supplied for this head. Is this the best way to solve the issue? No. The current head no longer implements stale active-work recovery, and the new JSON summary should summarize the same inspectable projection operators rely on before becoming a public output field. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c5d34c8376f8. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +52, Tests +94. Total +146 across 6 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
|
…rk-recovery # Conflicts: # src/commands/tasks-json.ts
|
@clawsweeper re-review Fixed the task JSON mismatch from the latest review: Proof: |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Addressed the owner-boundary finding: ownerless processing-session model/tool markers are no longer active-abort eligible. Stale owned model calls still recover through the embedded-run owner path, while idle queued lanes blocked by stale orphaned markers continue to recover through stale-session-state handling. Proof passed on PR head node scripts/run-vitest.mjs src/logging/diagnostic.test.ts src/logging/diagnostic-session-attention.test.ts src/commands/tasks-json.test.ts
pnpm format:check src/logging/diagnostic.ts src/logging/diagnostic.test.ts src/commands/tasks-json.ts src/commands/tasks-json.test.ts
pnpm check:test-typesRemaining proof gap: the live Telegram/Mantis behaviour proof requested in the previous review still needs maintainer/live-runtime capture. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Closing as superseded by the current focused recovery path. The current head no longer carries the original stale active-work recovery fix in a form that should land independently, and the remaining task/status JSON pieces overlap the narrower recovery work being kept open, especially #96853. Keeping this PR open creates duplicate review overhead. If a unique task-status output improvement is still needed, it should be folded into #96853 or reopened as a small task-status-only PR with current proof. |
What Problem This Solves
Telegram topic sessions could remain blocked behind stale active work even when the underlying model call, worker, or active run was no longer making progress. In that state, a lane could be classified as stalled but still fail to release because model-call recovery required an active embedded-run marker.
This meant a topic could stay silent behind a persisted busy/running state or queued work even though recovery should have cleared only that affected lane.
Why This Change Was Made
The diagnostic heartbeat already tracks meaningful progress and has per-lane recovery plumbing. The gap was that stalled
model_callrecovery was tied tohasActiveEmbeddedRun === true, so orphaned model work without that marker was observed but not actively released.This change makes stalled model calls and orphaned active work recovery-eligible once their last meaningful progress age reaches the configured stuck-session abort threshold. It also aligns JSON task output with the operator inspection projection so stale/lost tasks do not look like real active work forever.
User Impact
Evidence
Validated on a fresh worktree based on current
origin/main:Result:
src/logging/diagnostic-session-attention.test.ts: 11 passedsrc/logging/diagnostic.test.ts: 70 passedsrc/commands/tasks-json.test.ts: 2 passedgit diff --check origin/main...HEAD: passedAdditional local verification after the task JSON summary fix:
Results: formatting, focused tasks JSON tests, and test type checks passed. Latest pushed commit:
8b893230e2d.Additional local verification after the owner-boundary repair:
Results: focused diagnostic/session-attention/tasks JSON tests passed, formatting passed, and test type checks passed. Latest pushed commit:
8b893230e2d.