fix(agents): skip text-direct fallback for sessions_yield completions#91370
fix(agents): skip text-direct fallback for sessions_yield completions#91370openperf wants to merge 3 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 3:36 PM ET / 19:36 UTC. Summary PR surface: Source +26, Tests +312. Total +338 across 9 files. Reproducibility: yes. Source inspection shows current main clears Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest 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 Is this the best way to solve the issue? Yes. Capturing yield-origin state before AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5a89484eb31d. Label changesLabel justifications:
Evidence reviewedPR surface: Source +26, Tests +312. Total +338 across 9 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
f86ff43 to
422a8ee
Compare
422a8ee to
ad188b0
Compare
ad188b0 to
424a4bd
Compare
424a4bd to
320fef8
Compare
320fef8 to
dd2d701
Compare
dd2d701 to
0772257
Compare
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
b9b1acd to
7cc26a9
Compare
…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.
|
This pull request has been automatically marked as stale due to inactivity. |
Summary
Reproduction
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):
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
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Fixes #90944