Skip to content

fix(subagents): killed subagent runs stay running in the task list#99806

Merged
steipete merged 15 commits into
openclaw:mainfrom
masatohoshino:fix/subagent-kill-task-ledger-finalize
Jul 5, 2026
Merged

fix(subagents): killed subagent runs stay running in the task list#99806
steipete merged 15 commits into
openclaw:mainfrom
masatohoshino:fix/subagent-kill-task-ledger-finalize

Conversation

@masatohoshino

@masatohoshino masatohoshino commented Jul 4, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Related: #90444

What Problem This Solves

Killing a subagent could leave its detached task row running forever, while a provider completion arriving during cancellation could race the registry, task ledger, session timing, cleanup, and delivery paths into contradictory terminal states.

Why This Change Was Made

The original PR correctly identified the missing task finalization, but direct finalization alone made cancellation sticky too early and did not survive restarts, steer replacements, split persistence, or same-session generations.

This rewrite gives each subagent run a durable task owner and monotonic generation, records direct kills as provisional reconciliation markers, and arbitrates provider completion against stable operator cancellation. Task projection commits before registry publication, interrupted task-first commits replay from durable evidence, and session writes, cleanup, hooks, list/control views, and announcement dedupe all recheck generation ownership. Failed steer cleanup remains best-effort so a custom task runtime cannot strand the old run in suppressed state.

User Impact

  • Directly killed subagent tasks no longer remain phantom running rows.
  • A provider result already won before cancellation, or accepted during the provisional direct-kill window, repairs the task and registry to one successful outcome.
  • An explicit tasks.cancel remains sticky against a later provider result.
  • Restart, steer, and same-session reuse paths no longer let stale generations publish timing, cleanup, hooks, or list/control authority.

Evidence

  • 575 focused Vitest cases passed across unit-fast, tasks, auto-reply, agents, and E2E shards.
  • Additional recovery fix proof: 208 focused registry/E2E tests and 25 task-executor tests passed.
  • Live production-path arbitration probe passed:
    • direct kill -> provisional cancellation -> late provider success repaired to succeeded;
    • tasks.cancel -> late provider success remained cancelled;
    • provider completion before cancellation remained succeeded.
  • Testbox tbx_01kwrjd9a5b535wqj88qr1x47h passed the full changed gate: production/test tsgo, exhaustive core lint, import-cycle check, database-first guard, runtime sidecar guard, media/webhook/pairing guards. Actions proof
  • Fresh Codex autoreview: clean, no accepted/actionable findings.
  • Screenshots: not applicable; no UI surface changed.

Scope

This fixes new kill/completion reconciliation and restart durability. #90444 remains open for historical stuck-row cleanup and broader maintenance repair.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jul 4, 2026
@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 5:03 AM ET / 09:03 UTC.

Summary
This PR rewrites subagent kill/completion reconciliation with durable task ownership, run generations, provisional kill markers, task-result arbitration, cleanup fences, and focused regression coverage.

PR surface: Source +1805, Tests +6146. Total +7951 across 46 files.

Reproducibility: yes. from source inspection: current main records killed subagent terminal state without finalizing the mirrored detached task row, matching the linked stuck-running task report. I did not run a live reproduction in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Persistent subagent run fields: 5 added. The PR adds durable task ownership, generation, reconciliation, delivery-suppression, and delete-dispatch fields, so restored-row upgrade behavior matters before merge.
  • Detached runtime contract changes: 1 optional finder hook, 1 optional finalizer hook, 3 suppressDelivery parameters. Registered detached task runtimes must tolerate the widened terminal-state contract without breaking custom task ownership.

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

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90444
Summary: This PR is a current fix candidate for the canonical killed-subagent task-row reconciliation issue; the older open repair overlaps but uses a different maintenance-first strategy.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • [P2] Finish any still-running current-head CI shard before merge.
  • [P2] Make the canonical-path decision against the older maintenance-first repair before landing.

Risk before merge

  • [P1] The diff changes persistent subagent run metadata, detached task runtime behavior, task-flow cancellation, and cleanup ordering, so upgrade and custom-runtime compatibility need maintainer ownership before merge.
  • [P1] The older open repair at fix(agents): finalize killed subagent task rows via maintenance to avoid kill-vs-complete race #90505 targets the same canonical issue with a different maintenance-first strategy, so maintainers should explicitly supersede or split the paths.
  • [P1] At inspection, one current-head CI shard was still in progress even though the main proof and other checked shards were green.

Maintainer options:

  1. Land after validation and supersession decision (recommended)
    Accept the rewrite only after exact-head checks finish and maintainers document whether it supersedes the older maintenance-first repair.
  2. Pause for the narrower path
    Keep this PR open but do not merge it if maintainers prefer the smaller maintenance-based reconciliation strategy.
  3. Accept a split-scope landing
    Merge this PR for new runtime races while explicitly tracking historical stuck-row repair outside this branch.

Next step before merge

  • [P2] No narrow automated repair is queued; the remaining action is maintainer canonical-path and merge-risk review for a broad session-state rewrite.

Maintainer decision needed

  • Question: Should this broad runtime-generation reconciliation be the canonical landing path for killed-subagent task-row reconciliation, superseding or narrowing fix(agents): finalize killed subagent task rows via maintenance to avoid kill-vs-complete race #90505 after exact-head validation?
  • Rationale: The source review found no concrete blocker, but the PR is a broad session-state rewrite with compatibility-sensitive runtime contract changes and an overlapping open repair using a different strategy.
  • Likely owner: steipete — steipete is assigned and authored the current rewrite commits across the affected lifecycle/task surfaces.
  • Options:
    • Use this PR as canonical (recommended): Merge this broader runtime reconciliation after current-head proof is complete, then deliberately close or narrow the older maintenance-first repair path.
    • Prefer the narrower repair: Pause this PR and continue with the maintenance-first repair if maintainers want less runtime arbitration churn.
    • Split historical cleanup: Land this PR for new kill/completion races while keeping the linked issue open for already-stuck rows or CLI-child cleanup not covered here.

Security
Cleared: No concrete security or supply-chain issue was found; the diff stays in agent/task lifecycle code and tests without workflows, lockfiles, dependency sources, permissions, or secret handling changes.

Review details

Best possible solution:

Land this runtime reconciliation only after current-head validation and a deliberate supersession or split decision for the older repair, keeping #90444 open for historical cleanup if needed.

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

Yes from source inspection: current main records killed subagent terminal state without finalizing the mirrored detached task row, matching the linked stuck-running task report. I did not run a live reproduction in this read-only review.

Is this the best way to solve the issue?

Yes, this appears to be the best owner-boundary fix because it reconciles direct kill, late provider completion, explicit task cancellation, restart, and stale-generation ownership in the lifecycle/task paths rather than relying only on later maintenance cleanup.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This fixes an operator-visible stuck subagent/task lifecycle bug with limited blast radius, not a core outage.
  • merge-risk: 🚨 compatibility: The PR widens the detached task runtime contract and changes persisted subagent row shape used during restore and upgrade.
  • merge-risk: 🚨 session-state: The patch changes how subagent runs, task rows, flows, cleanup, and cancellation races reconcile terminal state.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body, maintainer comment, and proof check include after-fix production-path arbitration evidence for direct kill, explicit task cancellation, and provider-completion ordering.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body, maintainer comment, and proof check include after-fix production-path arbitration evidence for direct kill, explicit task cancellation, and provider-completion ordering.
Evidence reviewed

PR surface:

Source +1805, Tests +6146. Total +7951 across 46 files.

View PR surface stats
Area Files Added Removed Net
Source 27 2228 423 +1805
Tests 19 6295 149 +6146
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 46 8523 572 +7951

What I checked:

  • Repository policy applied: Root AGENTS.md and scoped src/agents/AGENTS.md were read fully; their deep-review and session-state compatibility guidance applies to this PR. (AGENTS.md:28, 3957ccba4e59)
  • Current-main kill-path gap: Current main's markSubagentRunTerminated marks subagent runs killed and persists cleanup state without finalizing the mirrored detached task row in the same path. (src/agents/subagent-registry-run-manager.ts:761, 3957ccba4e59)
  • PR kill reconciliation path: At the PR head, markSubagentRunTerminated records a durable provisional kill marker, commits it first, then finalizes the subagent detached task as cancelled with the special kill marker. (src/agents/subagent-registry-run-manager.ts:857, e683793ee3ee)
  • Task arbitration path: The task registry lets canonical subagent completion replace a provisional kill cancellation while preserving an accepted operator cancellation against later provider results. (src/tasks/task-registry.ts:499, e683793ee3ee)
  • Runtime contract surface: The detached task runtime contract gains terminal suppression, a unified optional finalizer, and an optional task finder for runtime-owned task rows, which makes compatibility review relevant. (src/tasks/detached-task-runtime-contract.ts:64, e683793ee3ee)
  • Regression coverage: The PR adds targeted tests for provisional kill replay, accepted cancellation, provider-result ordering, generation ownership, and flow cancellation while kills are provisional. (src/agents/subagent-registry-lifecycle.test.ts:1050, e683793ee3ee)

Likely related people:

  • steipete: Assigned on the live PR, authored the rewrite commits, and current-main blame for the central kill path points to Peter Steinberger in the available checkout history. (role: recent area contributor and current rewrite owner; confidence: high; commits: 8a39dc67eb58, e683793ee3ee, 8251c4094a; files: src/agents/subagent-registry-run-manager.ts, src/agents/subagent-registry-lifecycle.ts, src/tasks/task-registry.ts)
  • openperf: Authored the older open maintenance-first repair for the same canonical killed-subagent task-row issue, so they are relevant to the supersession decision. (role: related repair author; confidence: medium; commits: f63c221e2978; files: src/agents/subagent-registry-run-manager.ts, src/tasks/task-registry.maintenance.ts, src/tasks/task-registry.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.
Review history (6 earlier review cycles)
  • reviewed 2026-07-04T07:04:03.399Z sha 0350f6e :: found issues before merge. :: [P2] Preserve late completion after kill
  • reviewed 2026-07-04T11:37:25.791Z sha c48ae32 :: needs changes before merge. :: [P3] Document subagent cancellation race semantics
  • reviewed 2026-07-05T07:34:40.476Z sha 19f7057 :: needs changes before merge. :: [P3] Document provisional subagent cancellation
  • reviewed 2026-07-05T08:11:11.735Z sha f5ac0a6 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T08:38:27.969Z sha f324566 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T08:58:22.913Z sha e683793 :: needs maintainer review before merge. :: none

@harjothkhara

harjothkhara commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Non-approval review note after reading the current head and the adjacent task/subagent lifecycle paths:

I do not see a source-level blocker in this pass. Moving the cancelled task-row finalization into markSubagentRunTerminated looks like the right owner boundary because killControlledSubagentRun, killSubagentRunAdmin, cascade kills, and the parent-reply abort path all converge through that primitive. The task registry side also looks safe against a late waiter: run-scoped terminal updates only let a terminal status overwrite another terminal status when the current status is succeeded, so the new cancelled row should not be overwritten by a later completeTaskRunByRunId / failTaskRunByRunId from the normal completion finalizer.

What I checked:

  • PR diff at 0350f6e406d9 for src/agents/subagent-registry-run-manager.ts and src/agents/subagent-control.test.ts.
  • Current main completion and task paths: waitForSubagentCompletion, completeSubagentRun, safeFinalizeSubagentTaskRun, finalizeTaskRunByRunId, and shouldApplyRunScopedStatusUpdate.
  • Caller paths through src/agents/subagent-control.ts, src/auto-reply/reply/abort.ts, and operator cancel handling in src/tasks/task-registry.ts.
  • Focused proof passed locally with the repo runner:
node scripts/run-vitest.mjs src/agents/subagent-control.test.ts -- -t "finalizes the detached task row|finalizes the same way|task-row finalize" --reporter=verbose

That selected 2 passing tests in src/agents/subagent-control.test.ts.

One small evidence caveat: the PR body says src/auto-reply/reply/abort.test.ts covers the parent-reply abort caller, but those tests mock markSubagentRunTerminated; the new task finalizer itself is covered through the control/admin kill path tests. I would not treat that as a source blocker, but if maintainers want the parent-abort claim pinned directly, a narrow assertion at the registry boundary or an abort-path integration test would make the evidence match the PR text exactly.

At the time I checked, the relevant CI/check shards were passing; QA Smoke CI was still pending.

@masatohoshino

masatohoshino commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

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 proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 4, 2026
@steipete steipete self-assigned this Jul 4, 2026
@steipete
steipete force-pushed the fix/subagent-kill-task-ledger-finalize branch from eb9f0a2 to c48ae32 Compare July 4, 2026 10:31
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 4, 2026
@steipete
steipete force-pushed the fix/subagent-kill-task-ledger-finalize branch from c48ae32 to 19f7057 Compare July 5, 2026 07:30
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 5, 2026
steipete and others added 15 commits July 5, 2026 10:04
@steipete @masatohoshino
fix(subagents): reconcile killed task outcomes
9770e4c 
Co-authored-by: masatohoshino <[email protected]>
@steipete
fix(subagents): retain reconciliation marker narrowing
24f443d
@steipete
fix(subagents): honor generations in latest-run views
7732cca
@steipete
test(subagents): align reconciliation fixtures
af63df0
@steipete
fix(subagents): keep steer recovery best effort
c97aa10
@steipete
style(tasks): use type-only runtime contract import
e61e3b5
@steipete
refactor(subagents): keep generation ordering leaf-only
8405f17
@steipete
fix(subagents): sanitize persisted task owner ids
fe374b3
@steipete
fix(subagents): preserve failed replay lifecycle reason
dc5d1f2
@steipete
fix(agents): clear killed lifecycle timeout grace
12e723e
@steipete
fix(tasks): preserve canonical subagent outcomes
f95a3e0
@steipete
fix(agents): continue requester-wide cancellation
f63fa47
@steipete
fix(agents): fence yield revival races
4d4b9dc
@steipete
test(agents): mock detached task lookup
6ac3a86
@steipete
fix: fence subagent cleanup deletion
7f8f505
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation app: ios App: ios app: web-ui App: web-ui scripts Repository scripts labels Jul 5, 2026
@steipete
steipete force-pushed the fix/subagent-kill-task-ledger-finalize branch from 4f3ed83 to 7f8f505 Compare July 5, 2026 09:05
@openclaw-barnacle openclaw-barnacle Bot removed docs Improvements or additions to documentation app: ios App: ios app: web-ui App: web-ui labels Jul 5, 2026
@github-actions github-actions Bot mentioned this pull request Jul 5, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jul 5, 2026
Closed
@smthfoxy smthfoxy mentioned this pull request Jul 7, 2026
Merged
@clawsweeper clawsweeper Bot mentioned this pull request Jul 8, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@masatohoshino @harjothkhara @steipete