Skip to content

fix(agents): skip text-direct fallback for sessions_yield completions#91370

Open
openperf wants to merge 3 commits into
openclaw:mainfrom
openperf:fix/90944-sessions-yield-text-direct
Open

fix(agents): skip text-direct fallback for sessions_yield completions#91370
openperf wants to merge 3 commits into
openclaw:mainfrom
openperf:fix/90944-sessions-yield-text-direct

Conversation

@openperf

@openperf openperf commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

  • Problem: Issue [Bug]: sessions_yield resume reply recorded but not delivered; auto-announce direct-text mirror delivered instead → user gets child raw summary, not parent reply #90944 reports that when a `sessions_yield`-paused subagent completes, raw child text is delivered directly to the DM channel before the parent's intended resume reply. Users see a duplicate/out-of-order message: child output appears ~672ms ahead of the parent's reply.
  • Root Cause: `completeSubagentRun` in `src/agents/subagent-registry-lifecycle.ts` clears `entry.pauseReason` before triggering the announce cleanup flow. By the time `startSubagentAnnounceCleanupFlow` reads the entry, `pauseReason` is already `undefined`, so the yield state is lost. `runSubagentAnnounceFlow` falls through to the text-direct fallback because the parent's resume reply travels through the normal session output path and never appears as gateway payload evidence.
  • Fix: Stamp `entry.completedFromYieldPause = true` on the run record before clearing `pauseReason`; read it in `startSubagentAnnounceCleanupFlow` and forward as `requesterPausedForYield` into `runSubagentAnnounceFlow`. When the flag is set, the announce flow returns `true` after both descendant phases — active-descendant deferral and `wakeOnDescendantSettle` re-wake — so a yielded run still defers while descendants are pending and still synthesizes settled child findings before the yield skip takes over. When the settled-descendant wake is attempted but fails, the flow returns `false` to keep cleanup retryable rather than treating the failed wake as a successful skip. `finalizeSubagentCleanup` runs with `skipAnnounce: true`, clearing pending delivery state without crediting `delivery.status = "delivered"`. `completedFromYieldPause` is reset in `replaceSubagentRunAfterSteer` so a steered run cannot carry the flag into its replacement.
  • What changed:
    • `src/agents/subagent-registry.types.ts` — add `completedFromYieldPause?: boolean` to `SubagentRunRecord`
    • `src/agents/subagent-registry-lifecycle.ts` — stamp flag before clearing `pauseReason`; forward as `requesterPausedForYield`; pass `skipAnnounce: true` to `finalizeSubagentCleanup` for yield completions
    • `src/agents/subagent-announce.ts` — add `requesterPausedForYield` param; yield skip placed after both descendant phases; failed settled-descendant wake returns `false` and clears `shouldDeleteChildSession` to prevent the `finally` block deleting the child session during a retryable cleanup
    • `src/agents/subagent-registry-run-manager.ts` — reset `completedFromYieldPause` alongside other terminal fields in `replaceSubagentRunAfterSteer`
    • `src/agents/subagent-announce.test.ts` — `requesterPausedForYield: true` returns `true` and skips announce; with pending descendants returns `false` and defers
    • `src/agents/subagent-announce.format.e2e.test.ts` — yield + settled descendants still wakes before skip; failed wake returns `false`; `cleanup:"delete"` run with failed wake does not delete the child session
    • `src/agents/subagent-announce-delivery.test.ts` — non-yield DM + no payload fires `sendMessage` text-direct (baseline unaffected)
    • `src/agents/subagent-registry-lifecycle.test.ts` — `completeSubagentRun` with `pauseReason:"sessions_yield"` stamps flag, forwards `requesterPausedForYield`, does not credit delivery
    • `src/agents/subagent-registry.steer-restart.test.ts` — `replaceSubagentRunAfterSteer` clears `completedFromYieldPause` on replacement
  • What did NOT change (scope boundary): `subagent-announce-delivery.ts` has no production changes. Config surface unchanged. Plugin surface unchanged. Gateway protocol unchanged.

Reproduction

  1. Agent A spawns agent B using `sessions_yield`, suspending A with `pauseReason: "sessions_yield"` on B's run record.
  2. Agent B completes; `completeSubagentRun` clears `entry.pauseReason` and triggers the announce flow.
  3. The announce flow has no gateway payload evidence (the parent's resume reply is in A's session output queue, not the gateway response).
  4. Before this PR: the text-direct fallback fires unconditionally, sending B's raw output ~672ms ahead of A's resume reply; users see a duplicate/out-of-order message.
  5. After this PR: `runSubagentAnnounceFlow` returns `true` after the descendant phases because `requesterPausedForYield === true`; text-direct fallback is skipped; only A's resume reply reaches the user.

Real behavior proof

Behavior addressed (#90944): after a `sessions_yield` subagent completes, the DM channel no longer receives a duplicate raw-child-text message; the text-direct fallback is bypassed, the descendant-pending defer/retry invariant is preserved, failed settled-descendant wakes stay retryable without deleting the child session, and B's delivery state is not credited without external proof.

Real environment tested (Linux, Node 22 — Vitest agents suite against production `runSubagentAnnounceFlow`, `completeSubagentRun`, `startSubagentAnnounceCleanupFlow`, `finalizeSubagentCleanup`, and `replaceSubagentRunAfterSteer`): `pnpm test src/agents/subagent-announce.test.ts src/agents/subagent-announce.format.e2e.test.ts src/agents/subagent-announce-delivery.test.ts src/agents/subagent-registry-lifecycle.test.ts src/agents/subagent-registry.steer-restart.test.ts`

Exact steps or command run after this patch: `pnpm test src/agents/subagent-announce.test.ts src/agents/subagent-announce.format.e2e.test.ts src/agents/subagent-announce-delivery.test.ts src/agents/subagent-registry-lifecycle.test.ts src/agents/subagent-registry.steer-restart.test.ts`

Evidence after fix (verbatim output):

Test Files  4 passed (4)
      Tests  163 passed (163)
   Duration  8.37s

Test Files  1 passed (1)
      Tests  82 passed (82)
   Duration  9.81s

Observed result after fix: 245 tests pass across 5 files. For a `sessions_yield` completion: `runSubagentAnnounceFlow` returns `true` after descendant phases; `finalizeSubagentCleanup` runs with `skipAnnounce: true`; no `sendMessage` for child text; no `delivery.status = "delivered"`. With pending descendants: returns `false` and defers. With settled descendants: still wakes via `replaceSubagentRunAfterSteer` before the yield skip. Failed wake: returns `false`, child session not deleted. After steer restart: replacement run carries no `completedFromYieldPause`.

What was not tested: live Feishu/Discord DM round-trip with a real `sessions_yield` agent pair.

Repro confirmation: each new test case fails on `main` (no `requesterPausedForYield` guard, `shouldDeleteChildSession` not cleared on failed wake) and passes after this patch.

Risk / Mitigation

  • Risk: Parent resume reply fails to reach the external channel after child fallback is suppressed. Mitigation: A's delivery is tracked through A's own session output path, independent of B's announce lifecycle.
  • Risk: Failed settled-descendant wake silently discards child findings. Mitigation: returning `false` keeps cleanup retryable; the next announce retry will re-attempt the wake with fresh registry state.
  • Risk: `completedFromYieldPause` persists in `payload_json` after cleanup completes. Mitigation: read only in `startSubagentAnnounceCleanupFlow` at completion; reset on steer restart; inert once `cleanupCompletedAt` is set.

Change Type (select all)

  • Bug fix

Scope (select all touched areas)

  • agents

Linked Issue/PR

Fixes #90944

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

clawsweeper Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 3:36 PM ET / 19:36 UTC.

Summary
Adds a durable yielded-completion marker, forwards it into subagent announce cleanup to skip raw child text-direct fallback for sessions_yield completions, resets the marker on steer restart, and adds focused agent regression tests.

PR surface: Source +26, Tests +312. Total +338 across 9 files.

Reproducibility: yes. Source inspection shows current main clears sessions_yield pause state before announce cleanup and still has the :text-direct child-output fallback, while the linked issue includes live Feishu/WhatsApp evidence for the same failure mode.

Review metrics: 1 noteworthy metric.

  • Persisted Run-State Fields: 1 added. completedFromYieldPause is persisted run-record state and affects cleanup decisions for yielded completions across retries or restarts.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90944
Summary: This PR is the active fix candidate for the canonical yielded-completion child :text-direct fallback report; related subagent delivery reports overlap but cover broader direct-run, retry, passive-context, or descendant-wake behavior.

Members:

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

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

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

Mantis proof suggestion
A live Telegram transcript would materially prove that yielded completion delivery suppresses raw child fallback without dropping the parent reply. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram live: run a sessions_yield parent/subagent DM completion and verify only the parent resume reply is delivered, with no raw child fallback.

Risk before merge

  • [P2] Suppressing the child text-direct fallback for yielded completions makes the parent resume path authoritative; maintainers should accept that the previous child fallback will no longer mask parent-delivery failures.
  • [P1] The new completedFromYieldPause field is durable run state and changes cleanup/retry behavior across restart until cleanup completes.
  • [P1] No live external-channel round trip is attached; the PR body reports targeted production-function Vitest proof, but not a Feishu, Telegram, Discord, QQBot, or WhatsApp transport run.

Maintainer options:

  1. Accept Parent-Owned Yield Delivery (recommended)
    Maintainers can accept that yielded child completions suppress the raw child fallback and rely on the parent session output path for the visible reply.
  2. Request Live Transport Proof
    Before merge, a maintainer can ask for a Telegram or comparable DM proof showing exactly one parent resume reply and no raw child fallback.
  3. Pause For Wider Direct-Run Policy
    If maintainers want to solve broader non-yield raw-child fallback behavior at the same time, pause this branch and route that decision through the wider subagent delivery cluster.

Next step before merge

  • [P2] Human maintainer review should decide whether to accept parent-owned yielded delivery and the durable marker; there is no narrow automated repair to queue.

Security
Cleared: The diff changes agents runtime/types/tests only and does not add dependency, workflow, secret, package, install, or supply-chain surface.

Review details

Best possible solution:

Land this branch or an equivalent narrow fix after maintainers accept parent-owned yielded completion delivery and any desired live transport proof is captured.

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

Yes. Source inspection shows current main clears sessions_yield pause state before announce cleanup and still has the :text-direct child-output fallback, while the linked issue includes live Feishu/WhatsApp evidence for the same failure mode.

Is this the best way to solve the issue?

Yes. Capturing yield-origin state before pauseReason is cleared is the narrow owner-boundary fix, and placing the skip after descendant deferral/wake preserves the existing retry semantics; a delivery-layer dedupe would be broader and still would not preserve yield-origin state for cleanup retries.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a user-facing agent/channel workflow where a yielded parent reply can be replaced or preempted by raw child output.
  • merge-risk: 🚨 session-state: The diff adds durable run state and changes cleanup/retry behavior for yielded subagent completions.
  • merge-risk: 🚨 message-delivery: The diff intentionally suppresses a direct-message fallback and relies on the parent resume path for user-visible delivery.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply because the PR author is a repository MEMBER; the PR body provides targeted Vitest proof but no live transport artifact.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The generic agents change affects visible Telegram DM completion behavior and can be demonstrated with a short yielded parent/subagent transcript.
Evidence reviewed

PR surface:

Source +26, Tests +312. Total +338 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 4 26 0 +26
Tests 5 312 0 +312
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 9 338 0 +338

What I checked:

  • Repository policy read: Root AGENTS.md was read fully and applied; its PR review policy requires whole-path review, scoped AGENTS.md, current behavior, tests, history, and merge-risk treatment for session/message-delivery changes. (AGENTS.md:1, 5a89484eb31d)
  • Scoped agents policy and Telegram note read: The scoped agents guide and Telegram maintainer note were inspected; the Telegram note supports requiring live Telegram proof for visible Telegram behavior, but no Telegram-specific runtime decision is contradicted by this diff. (src/agents/AGENTS.md:1, 5a89484eb31d)
  • Current main loses yield state before cleanup: On current main, completeSubagentRun clears entry.pauseReason without preserving whether it was sessions_yield, before the announce cleanup flow uses the run record. (src/agents/subagent-registry-lifecycle.ts:1196, 5a89484eb31d)
  • Current main still has child text-direct fallback: The current delivery helper resolves child task completion text and sends it with a :text-direct idempotency key for direct-message targets when no visible messaging-tool evidence is present. (src/agents/subagent-announce-delivery.ts:949, 5a89484eb31d)
  • Current main announce flow has no yielded-requester guard: runSubagentAnnounceFlow on current main has no requesterPausedForYield parameter and proceeds to deliverSubagentAnnouncement after descendant handling. (src/agents/subagent-announce.ts:233, 5a89484eb31d)
  • Documented owner boundary: Subagent docs say completions return as internal parent-session events and human-visible replies are owned by the parent/requester agent's normal delivery policy, which supports suppressing raw child fallback for yielded completions. Public docs: docs/tools/subagents.md. (docs/tools/subagents.md:65, 5a89484eb31d)

Likely related people:

  • steipete: Git history shows the largest volume of work in the implicated subagent registry, announce, and delivery files, including lifecycle and cleanup stabilization work. (role: feature-history contributor; confidence: high; commits: c1467389967e, d6bb36730b50, ca3e5ffd892a; files: src/agents/subagent-registry-lifecycle.ts, src/agents/subagent-announce.ts, src/agents/subagent-announce-delivery.ts)
  • vincentkoc: History shows repeated recent work around subagent lifecycle, delivery, and registry performance paths adjacent to this fix. (role: recent adjacent contributor; confidence: medium; commits: 48042c387556, a8977cde64a0, b3faf20d914a; files: src/agents/subagent-registry-lifecycle.ts, src/agents/subagent-announce-delivery.ts, src/agents/subagent-registry-run-manager.ts)
  • openperf: The PR author also appears in prior subagent-related main history and authored the current targeted fix branch, so they are relevant for follow-up details beyond merely opening this PR. (role: linked fix author and adjacent subagent contributor; confidence: medium; commits: 17f086c02180, f3c63aa4c295, 940468ebe933; files: src/agents/subagent-registry-lifecycle.ts, src/agents/subagent-announce.ts, src/agents/subagent-registry-run-manager.ts)
  • Agustin Rivera: Current blame on the reviewed current-main lifecycle, announce, and direct fallback regions points to a recent commit that touched these files, though the commit title suggests broad adjacent churn rather than original feature ownership. (role: recent area contributor; confidence: low; commits: e87b0df67575; files: src/agents/subagent-registry-lifecycle.ts, src/agents/subagent-announce.ts, src/agents/subagent-announce-delivery.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 (1 earlier review cycle)
  • reviewed 2026-06-21T16:30:02.903Z sha 940468e :: needs maintainer review before merge. :: none

@openperf
openperf force-pushed the fix/90944-sessions-yield-text-direct branch from f86ff43 to 422a8ee Compare June 8, 2026 08:45
@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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 8, 2026
@openperf
openperf force-pushed the fix/90944-sessions-yield-text-direct branch from 422a8ee to ad188b0 Compare June 8, 2026 09:06
@clawsweeper clawsweeper Bot added 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: 🦐 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. labels Jun 8, 2026
@openperf
openperf force-pushed the fix/90944-sessions-yield-text-direct branch from ad188b0 to 424a4bd Compare June 8, 2026 12:17
@clawsweeper clawsweeper Bot added 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. 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 8, 2026
@openperf
openperf force-pushed the fix/90944-sessions-yield-text-direct branch from 424a4bd to 320fef8 Compare June 8, 2026 13:34
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jun 8, 2026
@openperf
openperf force-pushed the fix/90944-sessions-yield-text-direct branch from 320fef8 to dd2d701 Compare June 8, 2026 14:03
@openperf
openperf force-pushed the fix/90944-sessions-yield-text-direct branch from dd2d701 to 0772257 Compare June 8, 2026 14:40
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 8, 2026
@clawsweeper clawsweeper Bot removed the rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. label Jun 9, 2026
@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. 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 status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. 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. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 20, 2026
openperf added 2 commits June 21, 2026 21:23
When a sessions_yield-paused subagent completes, the announce flow's
text-direct fallback fired unconditionally because the parent's resume
reply travels through the normal session output path and never appears
as gateway payload evidence, delivering raw child text to the DM channel
ahead of the parent's intended reply.

completeSubagentRun cleared entry.pauseReason before the announce flow
ran, so yield state could not be derived at announce time. Stamp a durable
entry.completedFromYieldPause (persisted in payload_json) before the clear,
forward it as requesterPausedForYield into runSubagentAnnounceFlow, and
return early to skip the announce/text-direct fallback. The early return
sits after the descendant-pending deferral so a yielded run with unsettled
descendants still defers and retries. finalizeSubagentCleanup runs with
skipAnnounce for yield completions, so delivery is not credited on the
child run without external evidence; the parent session owns delivery
through its own output path.

Fixes openclaw#90944
@openperf
openperf force-pushed the fix/90944-sessions-yield-text-direct branch from b9b1acd to 7cc26a9 Compare June 21, 2026 13:28
@clawsweeper clawsweeper Bot added 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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. and removed 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. labels Jun 21, 2026
…ake fails

shouldDeleteChildSession was not cleared before the retryable return false in
the failed-wake-under-yield path; the finally block would delete the child
session despite cleanup being retried. Mirrors the convention of all other
deferral paths.
@openclaw-barnacle

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 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling mantis: telegram-visible-proof Mantis should capture Telegram visible proof. 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. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: M 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

1 participant