fix(agents): apply stale-run liveness check to aborted subagent orphan recovery#90817
Conversation
|
Codex review: stale review; fresh review needed. Summary Next step Review history (7 earlier review cycles)
|
|
@clawsweeper re-review RBP check is now passing (success). The proof section has been updated with real behavior evidence. Please refresh the review verdict and labels. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Enhanced proof with 6 edge cases calling the actual production isStaleUnendedSubagentRun function. Covers stale/fresh/ended/empty/custom-timeout scenarios. Full persistence test suite passes (18/18). @clawsweeper re-review |
|
@clawsweeper re-review Addressed P2 finding in 8ebc547:
PR body updated with expanded real behavior proof. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Dependency graph guard clearedThis PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh
|
fac2dcc to
13adddc
Compare
…very Skip stale unended subagent runs during orphan recovery and registry restore, even when they carry abortedLastRun. Previously, restart-aborted runs were exempt from the stale-unended age check, allowing hours-old aborted child sessions to be resurrected after long downtime. Fixes openclaw#90766 Co-Authored-By: Claude Opus 4.7 <[email protected]>
Previously the stale-run check in recoverOrphanedSubagentSessions only incremented the skipped counter. Stale active runs were left unended because scheduleOrphanRecovery only retries failedRuns, not skipped runs. Now stale runs are finalized via finalizeInterruptedSubagentRun so they don't remain orphaned in the registry. Ref: openclaw#90766 review feedback Co-Authored-By: Claude Opus 4.7 <[email protected]>
Await finalizeInterruptedSubagentRun for stale aborted runs and report failedRuns when finalization does not update the registry, so the scheduler retry path can recover from finalization failures. Co-authored-by: Cursor <[email protected]>
Drive the real recoverOrphanedSubagentSessions against the real subagent registry, the real isStaleUnendedSubagentRun policy, and a real on-disk session store, mocking only the outbound gateway transport and transcript reader. Proves finalizeInterruptedSubagentRun actually ends the stale aborted run in the registry (endedAt set, outcome error) instead of resuming it, while a fresh aborted run still resumes. Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
Merged via squash.
|
What Problem This Solves
Fixes #90766.
After long gateway downtime, restart-aborted subagent runs with
abortedLastRun: truewere exempt from the stale-unended pruning that every other stale run goes through. As a result, very old aborted child sessions (hours past the 2h stale window) were resurrected by orphan recovery and sent a synthetic resume message — producing late, meaningless recovery output for work that was abandoned long ago.Two gaps in current
main:resolveSubagentRunOrphanReasonskipsstale-unended-runclassification whenabortedLastRun === true(src/agents/subagent-registry-helpers.ts:192).recoverOrphanedSubagentSessionschecksabortedLastRunthen goes straight toevaluateSubagentRecoveryGatewithout applyingisStaleUnendedSubagentRun, and fire-and-forget finalization can leave stale runs unended with no scheduler retry (src/agents/subagent-orphan-recovery.ts:251).Why This Change Was Made
The stale-run liveness policy (
isStaleUnendedSubagentRun) already encodes the cutoff beyond which an unended run is not worth recovering. TheabortedLastRunexemption was a special case that bypassed this policy at two boundaries (registry restore and orphan recovery), creating a path where stale aborted runs could be resurrected indefinitely across restarts.This PR removes the exemption and reuses the existing liveness helper at both boundaries:
subagent-registry-helpers.ts: Drop theabortedLastRun !== truecondition fromresolveSubagentRunOrphanReasonso stale aborted runs are classified asstale-unended-runlike any other stale run.subagent-orphan-recovery.ts: Evaluate a frozen stale-first scan snapshot before recovery. Stale aborted runs are finalized via the exact run ID, and failed terminal transitions remain generation-scoped across bounded scheduler retries. Fresh aborted successors still pass through normal recovery.docs/tools/subagents.md: Update the restart-recovery contract so docs match the runtime.User Impact
erroroutcome) instead of being resumed after gateway restart. Parents no longer see late, meaningless recovery output for abandoned child work.Evidence
Maintainer proof targets public head
3e199d7d3909033a9aa2afc58c22ed9111ab8112.Blacksmith Testbox focused run:
pnpm test \ src/agents/subagent-delivery-state.test.ts \ src/agents/subagent-orphan-recovery.test.ts \ src/agents/subagent-orphan-recovery.restart-integration.test.ts \ src/agents/subagent-registry-lifecycle.test.ts \ src/agents/subagent-registry.persistence.test.ts \ src/agents/subagent-registry.store.sqlite.test.ts \ src/agents/subagent-registry.test.tsResult: 7 files / 261 tests pass. Exact-head targeted Oxlint also passes with 0 warnings and 0 errors under all 242 rules. A near-final
check:changedrun passed dependency guards, core and core-test typechecks, and all core lint shards after its solesort()finding was changed totoSorted()and re-proved at the final head.Blacksmith lease evidence: https://github.com/openclaw/openclaw/actions/runs/29120499066
The restart-path integration test (
subagent-orphan-recovery.restart-integration.test.ts) drives the realrecoverOrphanedSubagentSessionsagainst the real subagent registry, realisStaleUnendedSubagentRunpolicy, and a real on-disk session store. Only the outbound gateway WebSocket (callGateway) and transcript reader (readSessionMessagesAsync) are mocked — the genuine process boundaries a single-process test cannot stand up.Captured logs:
The stale aborted run (
startedAt3h ago) is finalized in the real registry —endedAtgoes fromundefinedto a real timestamp,outcome=error— and is never resumed (no gatewayagentcall). The fresh aborted run (55s old) passes the stale gate and resumes normally (recovered=1).Disclosed seams: outbound gateway WebSocket and transcript reader are mocked; the clock is seeded via run timestamps. A literal multi-hour live-gateway downtime is not exercised (the stale window is 2h), but every decision boundary — restore pruning, stale finalization, and fresh resume — runs through production code.
Source-blind validation did not substitute a weaker scenario: supported public surfaces cannot seed stale and fresh registered generations on one exact child session while controlling predecessor age and restart-aborted state. The faithful integration test is therefore the executable proof for that topology.
Fixes: #90766