Skip to content

fix(agents): reconcile stale restart-aborted subagent runs instead of resurrecting them (fixes #90766)#91611

Closed
ly-wang19 wants to merge 1 commit into
openclaw:mainfrom
ly-wang19:fix/subagent-orphan-recovery-stale-window
Closed

fix(agents): reconcile stale restart-aborted subagent runs instead of resurrecting them (fixes #90766)#91611
ly-wang19 wants to merge 1 commit into
openclaw:mainfrom
ly-wang19:fix/subagent-orphan-recovery-stale-window

Conversation

@ly-wang19

@ly-wang19 ly-wang19 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: After the gateway is down for a long time, restart/orphan recovery resurrects subagent runs that were aborted hours ago — replaying agent.wait on a run that is no longer relevant.
  • Root cause: The restore reconciliation in resolveSubagentRunOrphanReason (subagent-registry-helpers.ts) only classifies a run as stale-unended-run when abortedLastRun !== true. So restart-aborted runs were exempted from the shared stale-liveness policy and preserved indefinitely, then auto-recovered no matter how old.
  • What changed: Remove that one-line exemption so the existing isStaleUnendedSubagentRun policy (STALE_UNENDED_SUBAGENT_RUN_MS = 2h, or the run's timeout + grace) applies to aborted runs too. A long-downtime aborted run is now reconciled (pruned) instead of resurrected.
  • What did NOT change (scope boundary): Fresh restart recovery is preserved — a recently-aborted run is not yet stale, so it is still kept and recovered. The rapid re-wedge tombstone gate (evaluateSubagentRecoveryGate, 2-minute window) and non-aborted stale pruning are untouched. No new config or stale window — it reuses the existing liveness policy.

Change Type

  • Bug fix

Scope

  • Gateway / orchestration (subagent recovery)

Linked Issue

User-visible / Behavior Changes

After long downtime, the gateway no longer auto-resumes subagent runs that were aborted more than the stale-liveness window ago; they are reconciled out of the registry. Fresh restart-aborted runs (within the window) still recover as before.

Security Impact

  • New permissions/capabilities? No · Secrets handling? No · Network calls? No · Tool exec surface? No · Data access scope? No

Evidence

  • Failing test before + passing after. Updated reconciles stale unended restored runs even when restart-aborted (#90766) (3-hour aborted run → pruned, no agent.wait) fails with the exemption restored. New recovers fresh restart-aborted runs that are not yet stale (#90766) (1-minute aborted run → still recovered) stays green throughout, proving fresh restart recovery is preserved.
  • Local run (Node 22): 92 tests pass across subagent-registry.persistence.test.ts, subagent-orphan-recovery.test.ts, subagent-registry-queries.test.ts (+ co-located). oxlint + oxfmt clean.

Human Verification

  • Verified: a 3-hour aborted unended run is pruned and not resumed after restartRegistry(); a 1-minute aborted run is still kept and resumed via agent.wait; non-aborted stale pruning and the re-wedge gate are unchanged.
  • Not verified: a live multi-hour gateway-downtime reproduction (verified at unit/source level, consistent with the issue's read-only repro).

Compatibility / Migration

  • Backward compatible? Yes — only stale (older than the liveness window) aborted runs change behavior; fresh recovery and all other paths are unchanged. · Config/env changes? No · Migration needed? No

AI assistance

AI-assisted (Claude Code). Tested locally (92 tests + lint/format). Author understands the change. GitHub Codex review covers the AI-review pass.

Real behavior proof

Behavior addressed: restart/orphan recovery no longer resurrects subagent runs aborted longer ago than the shared stale-liveness window; those are reconciled instead, while fresh restart-aborted runs still recover.
Real environment tested: Local unit + source verification on Node 22.22.1 (macOS). No live multi-hour gateway-downtime run.
Exact steps or command run after this patch: node node_modules/vitest/vitest.mjs run src/agents/subagent-registry.persistence.test.ts src/agents/subagent-orphan-recovery.test.ts src/agents/subagent-registry-queries.test.ts plus oxlint + oxfmt on the touched files.
Evidence after fix: 92 tests pass including the updated stale-aborted-pruning test and the new fresh-aborted-recovery test; re-adding the abortedLastRun !== true exemption turns the stale-aborted test red (it gets resurrected) while the fresh test stays green.
Observed result after fix: a 3-hour-old aborted run is pruned from the registry (callGateway not called); a 1-minute-old aborted run is recovered (agent.wait, kept in the registry).
What was not tested: a live gateway restart after multi-hour downtime against a real registry (verified at unit/source level only).

Risks and Mitigations

  • Risk: pruning an aborted run a user still wanted to resume after a long outage.
    • Mitigation: the window matches the existing stale-liveness policy already used for non-aborted runs (and honors a longer per-run timeout), so behavior is consistent across run types; recently-aborted runs remain recoverable.

@ly-wang19 @claude
fix(agents): reconcile stale restart-aborted subagent runs instead of…
7cf5cc8 
… resurrecting them

restore reconciliation only treated a run as stale-unended when abortedLastRun
was not true, so restart-aborted runs were exempted from the shared liveness
policy and auto-recovered no matter how old. After long gateway downtime this
resurrected hours-old aborted subagent runs. Drop the exemption so the existing
isStaleUnendedSubagentRun window (2h or run timeout+grace) applies to aborted
runs too; fresh restart-aborted runs are not yet stale and still recover.

Fixes openclaw#90766

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 9, 2026
@clawsweeper

clawsweeper Bot commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 12:35 PM ET / 16:35 UTC.

Summary
The PR removes the restore-time abortedLastRun stale-pruning exemption and updates persistence coverage for stale versus fresh restart-aborted subagent runs.

PR surface: Source +2, Tests +42. Total +44 across 2 files.

Reproducibility: yes. at source level. Current main has both implementation and test evidence that a three-hour restart-aborted restored subagent run remains recoverable instead of stale-pruned; I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

  • Persisted Restart-Recovery Semantics: 1 behavior changed. Three-hour-old restart-aborted restored runs move from resume-eligible to stale-pruned, so maintainers should notice the session-state upgrade behavior before merge.

Stored data model
Persistent data-model change detected: serialized state: src/agents/subagent-registry.persistence.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90766
Summary: This PR is one candidate fix for the canonical stale subagent orphan-recovery issue; several open PRs overlap and should be reconciled by maintainers.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add the direct orphan-recovery stale-liveness gate or explicitly defer that path to the canonical linked PR.
  • Update docs/tools/subagents.md for the new stale restart-aborted recovery behavior.
  • [P1] Add redacted real or faithful gateway restart proof covering stale pruning and fresh recovery.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides unit/source verification only; before merge it needs redacted real or faithful gateway restart output, logs, terminal output, or an artifact showing stale pruning and fresh recovery. Contributors should redact private details and update the PR body so ClawSweeper can re-review automatically, or ask a maintainer to comment @clawsweeper re-review if it does not trigger.

Risk before merge

  • [P1] Direct orphan recovery can still resume stale restart-aborted active runs because this PR only changes restored-registry reconciliation.
  • [P1] Public subagent docs would describe the old stale abortedLastRun exception after this behavior change.
  • [P1] Contributor proof is unit/source-only and does not show a real or faithful gateway restart pruning stale aborted runs while preserving fresh recovery.
  • [P1] Multiple open PRs target the same stale-recovery bug, so maintainers need to choose one canonical implementation before landing competing branches.
  • [P1] Merging changes persisted restart-aborted subagent semantics: stale rows past the liveness window move from resume-eligible to pruned.

Maintainer options:

  1. Complete The Full Recovery Boundary (recommended)
    Expand this branch or choose a broader linked branch so stale restart-aborted runs are handled in both restored-registry reconciliation and direct orphan recovery before merge.
  2. Accept Restore-Only Scope
    Maintainers could intentionally land only the restore reconciliation change, but the canonical linked issue should remain open for direct recovery and docs follow-up.
  3. Pause For Canonical Selection
    If another open stale-recovery PR becomes the viable landing path, pause or close this branch after that replacement is clearly chosen.

Next step before merge

  • [P1] Human review should choose the canonical stale-recovery PR and require contributor restart-path proof; automation cannot supply the external contributor's real behavior evidence.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes subagent recovery classification and tests only.

Review findings

  • [P2] Gate direct orphan recovery with the stale cutoff — src/agents/subagent-registry-helpers.ts:192-196
  • [P2] Update the stale restart-recovery docs — src/agents/subagent-registry-helpers.ts:192-196
Review details

Best possible solution:

Land one canonical stale subagent recovery fix that applies the shared liveness cutoff at restore and direct recovery boundaries, preserves fresh and timeout-extended recovery, updates docs, and includes redacted restart-path proof.

Do we have a high-confidence way to reproduce the issue?

Yes at source level. Current main has both implementation and test evidence that a three-hour restart-aborted restored subagent run remains recoverable instead of stale-pruned; I did not run tests because this review is read-only.

Is this the best way to solve the issue?

No as submitted. Removing the restore exemption is necessary, but the best fix also gates direct orphan recovery, updates the public docs, and proves the restart path with redacted real or faithful output.

Full review comments:

  • [P2] Gate direct orphan recovery with the stale cutoff — src/agents/subagent-registry-helpers.ts:192-196
    This patch removes the restore-time exemption, but recoverOrphanedSubagentSessions can still receive active in-memory runs, check abortedLastRun, and resume them after only the rapid recovery gate. Apply the same stale liveness policy there or explicitly leave [Bug]: orphan recovery can resurrect stale subagent runs after long downtime #90766 open for that path.
    Confidence: 0.88
  • [P2] Update the stale restart-recovery docs — src/agents/subagent-registry-helpers.ts:192-196
    Changing the restore behavior leaves docs/tools/subagents.md describing the old exception where stale restored runs remain recoverable when abortedLastRun is true. Update the public docs so operators see the new stale-window behavior.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against c2ee9b0be8ae.

Label changes

Label justifications:

  • P2: This is a bounded subagent lifecycle bug fix with limited blast radius but real restart recovery impact.
  • merge-risk: 🚨 session-state: The PR changes whether persisted restart-aborted subagent run state is resumed or pruned after the stale-run window.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides unit/source verification only; before merge it needs redacted real or faithful gateway restart output, logs, terminal output, or an artifact showing stale pruning and fresh recovery. Contributors should redact private details and update the PR body so ClawSweeper can re-review automatically, or ask a maintainer to comment @clawsweeper re-review if it does not trigger.
Evidence reviewed

PR surface:

Source +2, Tests +42. Total +44 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 3 1 +2
Tests 1 50 8 +42
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 53 9 +44

What I checked:

Likely related people:

  • joeykrug: Authored the merged restart orphan-recovery work that added recoverOrphanedSubagentSessions and the initial abortedLastRun restart-resume tests. (role: introduced behavior; confidence: high; commits: 304703f1654a, c780b6a6ab2a; files: src/agents/subagent-orphan-recovery.ts, src/agents/subagent-orphan-recovery.test.ts, src/agents/subagent-registry.ts)
  • steipete: Merged the restart orphan-recovery PR and appears as committer/merger on the original recovery work. (role: merger and adjacent owner; confidence: high; commits: 304703f1654a, c780b6a6ab2a; files: src/agents/subagent-orphan-recovery.ts, src/agents/subagent-registry.ts)
  • vincentkoc: Current checkout blame attributes the present stale-liveness, registry-helper, orphan-recovery, and docs lines to a recent current-main commit, though the history is shallow/grafted so this is a routing signal rather than root-cause proof. (role: recent adjacent area contributor; confidence: medium; commits: 89a73d08c805; files: src/agents/subagent-registry-helpers.ts, src/agents/subagent-orphan-recovery.ts, src/agents/subagent-run-liveness.ts)
  • Tak Hoffman: Touched orphan-recovery retry/dedupe behavior in the same runtime path after the original restart recovery feature landed. (role: recent adjacent contributor; confidence: medium; commits: 2990446b21e8; files: src/agents/subagent-orphan-recovery.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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 9, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 9, 2026
Closed
This was referenced Jun 19, 2026
Closed
Merged
Closed
Open
@ly-wang19

ly-wang19 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Closing this. On re-evaluation against current main, it is neither mergeable nor clearly correct.

Stale / mis-targeted. This branch is ~3900 commits behind main, and its target resolveSubagentRunOrphanReason was moved from src/agents/subagent-registry-helpers.ts to src/agents/subagent-session-reconciliation.ts by the session-accessor refactor (#96182). The patch no longer applies to where the logic lives.

More importantly, the behavior it removes is intentional. The abortedLastRun !== true exemption in the stale-unended check is deliberate: it was introduced in 757aee4cdd ("fix(subagents): retire stale unended runs"), which retired stale unended runs while specifically keeping restart-aborted ones recoverable, and it is pinned by the test "keeps stale unended restored runs with abortedLastRun for restart recovery" (src/agents/subagent-registry.persistence.test.ts). That test sets up a 3-hour-old (stale) run with abortedLastRun: true and asserts it is resumed (agent.wait) and kept in the registry after restart — exactly the behavior this PR would remove.

So removing the exemption reverses a shipped, tested design and would break that test. Whether stale restart-aborted runs should age out (#90766) versus stay recoverable is an owner-level product decision, not a drop-in contributor fix — and #90766 is currently labeled clawsweeper:no-new-fix-pr.

Closing rather than force a behavior reversal that contradicts the current intended design. If a maintainer decides #90766's age-out behavior is wanted, the right shape is an owner-driven change against current main that deliberately updates the "keeps stale unended restored runs…" test, in subagent-session-reconciliation.ts.

@ly-wang19 ly-wang19 closed this Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

1 participant

@ly-wang19