Skip to content

fix(tasks): wake parent on blocked subagent delivery#95080

Closed
849261680 wants to merge 2 commits into
openclaw:mainfrom
849261680:fix/90178-subagent-announce-giveup
Closed

fix(tasks): wake parent on blocked subagent delivery#95080
849261680 wants to merge 2 commits into
openclaw:mainfrom
849261680:fix/90178-subagent-announce-giveup

Conversation

@849261680

@849261680 849261680 commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Wakes the parent/requester session when a subagent completion announce gives up and the required completion is marked blocked.
  • Keeps normal subagent successful completions and ordinary missing/progress-only blocked completions on the existing announce path, avoiding duplicate task delivery.
  • Re-queues only the real give-up sequence where delivery was already marked failed, then the same run becomes terminalOutcome: blocked with the required-completion delivery-failure summary.
  • Preserves idempotency after the blocked event is queued; replaying the terminal update does not re-open delivery.
  • Intentionally out of scope: adding retry/expiry config knobs from Subagent announce give-up (retry-limit/expiry) does not wake parent waiting on sessions_yield → permanent deadlock #90178, which ClawSweeper flagged as a separate maintainer/product decision.

Reviewers should focus on the subagent blocked completion delivery policy and the failed -> pending -> session_queued transition in the task registry.

Linked context

Addresses the parent wake-up bug in #90178.

Related: #91650, #87330.

Requested by maintainer workflow through issue #90178.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: subagent announce give-up can mark required completion delivery blocked without waking the parent waiting through sessions_yield.
  • Real environment tested: local OpenClaw source checkout on this branch using real task registry/system-event/heartbeat modules with an isolated OPENCLAW_STATE_DIR.
  • Exact steps or command run after this patch:
    • OPENCLAW_STATE_DIR=$(mktemp -d) node --import tsx --eval '<create subagent task; mark delivery failed; mark terminalOutcome blocked; print final task/events/heartbeat>'
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):
{
  "deliveryStatus": "session_queued",
  "terminalOutcome": "blocked",
  "eventCount": 2,
  "events": [
    "Background task blocked: Subagent task (run run-suba). Required completion delivery failed before reaching the requester: retry-limit.",
    "Task needs follow-up: Subagent task (run run-suba). Required completion delivery failed before reaching the requester: retry-limit."
  ],
  "heartbeatWake": true
}
  • Observed result after fix: the blocked subagent completion is queued into the parent session and requests an immediate heartbeat wake, so a yielded parent has a bounded failure/follow-up event instead of remaining silent.
  • What was not tested: live Signal/K8s multi-agent timing with real transport delivery.
  • Proof limitations or environment constraints: the proof is a local runtime-level task/session wake proof, not a full transport capture.
  • Before evidence (optional but encouraged): current main had shouldAutoDeliverTaskTerminalUpdate() return false for non-cancelled subagent tasks, so the blocked task update created by the give-up path was not auto-delivered to the requester session.

Tests and validation

  • pnpm install --frozen-lockfile
  • node scripts/run-vitest.mjs src/agents/subagent-registry-cleanup.test.ts src/agents/subagent-registry-lifecycle.test.ts src/tasks/task-executor-policy.test.ts src/tasks/task-registry.test.ts
  • node scripts/run-vitest.mjs src/tasks/task-executor-policy.test.ts src/tasks/task-registry.test.ts
  • pnpm -s exec oxfmt --check src/tasks/task-executor-policy.ts src/tasks/task-executor-policy.test.ts src/tasks/task-registry.ts src/tasks/task-registry.test.ts
  • git diff --check
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base upstream/main

Regression coverage added:

  • src/tasks/task-executor-policy.test.ts: blocked subagent completions are eligible for terminal delivery only when they carry the explicit delivery-failure summary and recorded error; normal subagent success and ordinary blocked completions remain suppressed.
  • src/tasks/task-registry.test.ts: real failed-delivery then blocked-terminal sequence queues parent events, triggers heartbeat, and stays idempotent on replay.
  • src/tasks/task-registry.test.ts: ordinary progress-only blocked subagent completion stays pending with no task event, no heartbeat wake, and no direct channel send.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. A previously silent subagent completion give-up now surfaces as a blocked background task event in the parent session.

Did config, environment, or migration behavior change? (Yes/No)

No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

Duplicate or misrouted completion delivery.

How is that risk mitigated?

The policy still suppresses ordinary non-cancelled subagent terminal updates. The recovery path requires subagent + succeeded + terminalOutcome: blocked plus the required-completion delivery-failure summary and recorded error. The registry only requeues from deliveryStatus: failed, and tests cover replay idempotency plus the ordinary blocked-completion sibling path.

Current review state

What is the next action?

ClawSweeper re-review on head 30ce8faa80.

What is still waiting on author, maintainer, CI, or external proof?

Nothing known from the author side beyond normal CI/ClawSweeper re-review. Maintainers may still decide separately whether #90178 should add retry/expiry configurability.

Which bot or reviewer comments were addressed?

Addressed ClawSweeper's finding to constrain blocked subagent task delivery to the announce give-up recovery path, and added sibling regression coverage for ordinary blocked required completions.

@openclaw-barnacle openclaw-barnacle Bot added size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 20, 2026, 9:29 PM ET / 01:29 UTC.

Summary
The PR adds a narrow task-delivery exception and registry transition so a failed subagent completion delivery that becomes a blocked required completion can queue parent-session wake events, with regression coverage.

PR surface: Source +20, Tests +136. Total +156 across 4 files.

Reproducibility: yes. at source level: current main records failed subagent announce delivery and a blocked required completion, but suppresses non-cancelled subagent terminal fallback before requester wake delivery. I did not run the live Signal/K8s timing scenario.

Review metrics: 1 noteworthy metric.

  • Task Terminal Policy Exception: 1 added. The new exception changes when a subagent terminal update can wake the requester, which is the maintainer-visible behavior change before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90178
Summary: This PR is a candidate fix for the canonical subagent announce give-up parent-wake bug; related items overlap but do not replace this narrow branch.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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] Let exact-head required checks complete before landing.

Mantis proof suggestion
A live transport transcript would materially verify that forced announce give-up wakes the yielded parent once without duplicate completion text. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram live: force a subagent completion announce give-up and verify the yielded parent wakes once with no duplicate completion text.

Risk before merge

  • [P1] The PR intentionally changes when blocked subagent completion failures enqueue requester-session system events and heartbeat wakes; a bad guard could duplicate, suppress, or misroute completion messages.
  • [P1] The contributor proof is runtime-level and source-backed, but it does not exercise the reported live Signal/K8s transport timing path.
  • [P1] The linked canonical issue also asks for retry and expiry configurability; that remains a separate product/config decision and should not be implied by this narrow bug fix.

Maintainer options:

  1. Land Narrow Recovery After Checks (recommended)
    If exact-head required checks pass, maintainers can accept the bounded policy exception because it targets only the failed-delivery give-up recovery path and does not add retry or expiry config surface.
  2. Request Live Transport Proof
    Ask for a redacted live transport transcript that forces announce give-up and shows the yielded parent wakes once without duplicate completion text before merging.
  3. Pause For Broader Yield Direction
    If maintainers want retry/expiry configurability or a formal yielded-parent lifecycle contract first, pause this PR and keep the broader direction on the canonical issue.

Next step before merge

  • [P2] No narrow code repair remains from this read-only review; maintainers should decide whether the focused runtime proof is enough for the session/message-delivery risk.

Security
Cleared: The diff changes task delivery policy and tests only; no dependency, workflow, package, secret, permission, or supply-chain surface is touched.

Review details

Best possible solution:

Land the constrained failed-delivery wake path after exact-head checks if maintainers accept the runtime-level proof, while keeping retry and expiry configurability as separate canonical-issue work.

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

Yes, at source level: current main records failed subagent announce delivery and a blocked required completion, but suppresses non-cancelled subagent terminal fallback before requester wake delivery. I did not run the live Signal/K8s timing scenario.

Is this the best way to solve the issue?

Yes for the narrow bug-fix slice: the PR reuses the existing terminal delivery path and gates the new behavior to the required-completion delivery-failure contract. Broader retry/expiry configurability or a formal yielded-parent lifecycle contract should stay separate.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a yielded-parent/subagent completion delivery deadlock that can stall active real-user workflows.
  • merge-risk: 🚨 session-state: The diff changes queued system-event and heartbeat wake behavior for yielded requester sessions.
  • merge-risk: 🚨 message-delivery: The diff changes when failed subagent completion delivery is surfaced to the requester, so mistakes could suppress, duplicate, or misroute completion messages.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 includes copied after-fix runtime output showing the failed-delivery then blocked-terminal path queues parent events and requests a heartbeat wake.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix runtime output showing the failed-delivery then blocked-terminal path queues parent events and requests a heartbeat wake.
Evidence reviewed

PR surface:

Source +20, Tests +136. Total +156 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 21 1 +20
Tests 2 136 0 +136
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 157 1 +156

What I checked:

  • Repository policy read: Root AGENTS.md was read fully and applied to the deep source review, proof assessment, and session/message-delivery merge-risk treatment for this PR. (AGENTS.md:1, e012f2cd3cc8)
  • Current main suppresses the affected fallback: Current main returns false for non-cancelled subagent terminal task delivery, so a blocked subagent terminal update cannot wake the requester through the generic terminal delivery path. (src/tasks/task-executor-policy.ts:112, e012f2cd3cc8)
  • Give-up path marks failed then blocked: The subagent lifecycle give-up path records failed delivery and then marks required-completion delivery blocked with the delivery-failure reason, matching the reported retry-limit/expiry sequence. (src/agents/subagent-registry-lifecycle.ts:601, e012f2cd3cc8)
  • Delivery-failure terminal contract: The required-completion failure helper emits the blocked terminal summary prefix that the PR uses to distinguish give-up recovery from ordinary progress-only blocked completions. (src/tasks/task-completion-contract.ts:84, e012f2cd3cc8)
  • Terminal delivery queues wake events: The existing terminal delivery callee queues system events, blocked follow-up events, and heartbeat wakes before marking the task session_queued when direct delivery is unavailable. (src/tasks/task-registry.ts:1333, e012f2cd3cc8)
  • PR narrows the policy exception: The PR allows subagent auto-delivery only for succeeded blocked tasks with a non-empty error and the required-completion delivery-failure summary prefix. (src/tasks/task-executor-policy.ts:116, a849bfd57d18)

Likely related people:

  • vincentkoc: GitHub path history shows task delivery policy extraction and recent task-registry work in the affected files. (role: task delivery policy contributor; confidence: high; commits: 817ac551b655, 745b01163276; files: src/tasks/task-executor-policy.ts, src/tasks/task-registry.ts, src/tasks/task-completion-contract.ts)
  • steipete: Recent path history includes subagent lifecycle, cleanup, timeout, and task-registry state work that this fix relies on. (role: recent subagent lifecycle contributor; confidence: high; commits: 589b1f6aec69, 465a5456fe0f, 5e139e32dc5f; files: src/agents/subagent-registry-lifecycle.ts, src/agents/subagent-registry-cleanup.ts, src/tasks/task-registry.ts)
  • IWhatsskill: Commit 36e76ef added the required-completion blocked and delivery-failure terminal contract that the PR keys on. (role: required-completion contract contributor; confidence: medium; commits: 36e76ef42409; files: src/tasks/task-completion-contract.ts, src/agents/subagent-registry-lifecycle.ts)
  • anyech: Commit 5697ab8 changed requester-visible task terminal delivery behavior for a neighboring parent-review path. (role: adjacent task delivery contributor; confidence: medium; commits: 5697ab810e4f; files: 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.

@849261680

849261680 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 19, 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. labels Jun 19, 2026
@clawsweeper clawsweeper Bot added P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 19, 2026
@849261680
849261680 force-pushed the fix/90178-subagent-announce-giveup branch from bd85be5 to 30ce8fa Compare June 19, 2026 19:43
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026
@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. and removed 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 Jun 19, 2026
This was referenced Jun 20, 2026
Open
Open
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed 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 Jun 20, 2026
@849261680

849261680 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated head: a849bfd

Author-side fix:

  • Fixed the direct no-promise-executor-return lint blocker in src/tasks/task-registry.test.ts by making the delay Promise executor block-bodied. Runtime behavior is unchanged.

Fresh local verification:

  • node scripts/run-vitest.mjs src/agents/subagent-registry-cleanup.test.ts src/agents/subagent-registry-lifecycle.test.ts src/tasks/task-executor-policy.test.ts src/tasks/task-registry.test.ts
  • node scripts/run-vitest.mjs src/agents/subagent-orphan-recovery.test.ts
  • node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json src/tasks/task-registry.test.ts
  • pnpm -s exec oxfmt --check src/tasks/task-registry.test.ts
  • git diff --check
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base upstream/main

@clawsweeper

clawsweeper Bot commented Jun 21, 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 rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 21, 2026
@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 21, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 22, 2026
Open
This was referenced Jun 23, 2026
Open
Open
@rosenlo rosenlo mentioned this pull request Jun 27, 2026
Merged
This was referenced Jun 27, 2026
Open
Open
Closed
@yetval yetval mentioned this pull request Jun 29, 2026
Closed
@liuhao1024 liuhao1024 mentioned this pull request Jun 29, 2026
Closed
1 task
This was referenced Jun 29, 2026
Closed
Closed
@lzyyzznl lzyyzznl mentioned this pull request Jun 29, 2026
Closed
7 tasks
This was referenced Jul 1, 2026
Closed
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jul 9, 2026
Open
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 11, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 11, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 12, 2026
Open
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 19, 2026

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

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

merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S stale Marked as stale due to inactivity 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.

1 participant

@849261680