Skip to content

fix(agents): apply stale-run liveness check to aborted subagent orphan recovery#90817

Merged
steipete merged 16 commits into
openclaw:mainfrom
Pick-cat:fix/90766-orphan-recovery-stale-runs
Jul 10, 2026
Merged

fix(agents): apply stale-run liveness check to aborted subagent orphan recovery#90817
steipete merged 16 commits into
openclaw:mainfrom
Pick-cat:fix/90766-orphan-recovery-stale-runs

Conversation

@Pick-cat

@Pick-cat Pick-cat commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #90766.

After long gateway downtime, restart-aborted subagent runs with abortedLastRun: true were 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:

  1. resolveSubagentRunOrphanReason skips stale-unended-run classification when abortedLastRun === true (src/agents/subagent-registry-helpers.ts:192).
  2. recoverOrphanedSubagentSessions checks abortedLastRun then goes straight to evaluateSubagentRecoveryGate without applying isStaleUnendedSubagentRun, 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. The abortedLastRun exemption 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 the abortedLastRun !== true condition from resolveSubagentRunOrphanReason so stale aborted runs are classified as stale-unended-run like 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.
  • Registry lifecycle: Persist an explicit interrupted-recovery terminal owner before task projection, arbitrate late provider/kill/yield callbacks through the existing completion lock, and preserve partial or conflicting terminal evidence. If restart admission is already draining, defer and replay only the matching exact terminal transition after admission reopens.
  • Persistence restore: Preserve valid interrupted-recovery ownership and normalize matching flat/nested terminal evidence so cleanup and delivery can resume after process loss.
  • docs/tools/subagents.md: Update the restart-recovery contract so docs match the runtime.
  • Tests: Cover stale/fresh same-session generations, persistence rollback and replay, restart-drain deferral, partial/conflicting terminal winners, bounded retry, and a faithful restart-path integration test driving the real registry + real session store + real liveness policy.

User Impact

  • Stale aborted subagent runs older than the 2h stale window are finalized (terminal error outcome) instead of being resumed after gateway restart. Parents no longer see late, meaningless recovery output for abandoned child work.
  • Fresh aborted runs (within the stale window) are unchanged — they still resume through orphan recovery as today.
  • Persisted restart-aborted rows older than the stale window will be pruned/finalized on the first restart after upgrade. This is a one-time transition in recovery semantics for old state; no config or migration is required.

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.ts

Result: 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:changed run passed dependency guards, core and core-test typechecks, and all core lint shards after its sole sort() finding was changed to toSorted() 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 real recoverOrphanedSubagentSessions against the real subagent registry, real isStaleUnendedSubagentRun policy, 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:

[proof] before recovery: stale run endedAt=undefined outcome=undefined
[proof] after recovery: result={"recovered":0,"failed":0,"skipped":1,"failedRuns":[]} endedAt=<timestamp> outcome=error
[subagent-interrupted-resume] found orphaned subagent session: agent:main:subagent:fresh-aborted (run=run-fresh-aborted)
[subagent-interrupted-resume] resumed orphaned session: agent:main:subagent:fresh-aborted
[subagent-interrupted-resume] orphan recovery complete: recovered=1 failed=0 skipped=0
[proof] fresh recovery: result={"recovered":1,"failed":0,"skipped":0,"failedRuns":[]} gatewayCalls=2

The stale aborted run (startedAt 3h ago) is finalized in the real registry — endedAt goes from undefined to a real timestamp, outcome=error — and is never resumed (no gateway agent call). 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

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 6, 2026
@clawsweeper

clawsweeper Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: stale review; fresh review needed.

Summary
The latest durable ClawSweeper review was for head e95137cbf88d88b000e69ebcc531229f336ee65f, but the PR head is now 3e199d7d3909033a9aa2afc58c22ed9111ab8112. Its old verdict and PR readiness labels are no longer current.

Next step
Run or wait for a fresh ClawSweeper review on the current PR head.

Review history (7 earlier review cycles)
  • reviewed 2026-07-02T15:32:25.259Z sha 27a709f :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T07:37:57.260Z sha e9b35ad :: needs maintainer review before merge. :: none
  • reviewed 2026-07-10T16:57:02.126Z sha b64e76c :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-10T17:06:59.542Z sha b64e76c :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-10T19:47:38.172Z sha 6457084 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-10T19:59:26.970Z sha 6457084 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-10T20:22:42.800Z sha 4064056 :: needs real behavior proof before merge. :: [P1] Preserve restart-drain terminal retries

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 6, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 6, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 6, 2026
@Pick-cat

Pick-cat commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

@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.

@clawsweeper

clawsweeper Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@Pick-cat

Copy link
Copy Markdown
Contributor Author

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

@Pick-cat

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed P2 finding in 8ebc547:

  • await finalizeInterruptedSubagentRun for stale aborted runs
  • report failedRuns when finalization returns 0 or throws (scheduler retry path)
  • added orphan-recovery regression tests (39/39 pass)

PR body updated with expanded real behavior proof.

@clawsweeper

clawsweeper Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. labels Jun 15, 2026
@github-actions github-actions Bot added the dependencies-changed PR changes dependency-related files label Jul 10, 2026
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

  • Current SHA: 3e199d7d3909033a9aa2afc58c22ed9111ab8112

@steipete
steipete force-pushed the fix/90766-orphan-recovery-stale-runs branch from fac2dcc to 13adddc Compare July 10, 2026 18:50
@openclaw-barnacle openclaw-barnacle Bot removed channel: nextcloud-talk Channel integration: nextcloud-talk app: android App: android app: ios App: ios app: macos App: macos app: web-ui App: web-ui gateway Gateway runtime labels Jul 10, 2026
Pick-cat and others added 16 commits July 10, 2026 21:40
…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]>
@steipete

Copy link
Copy Markdown
Contributor

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: orphan recovery can resurrect stale subagent runs after long downtime

2 participants