Skip to content

Fix subagent DM completion delivery after yield#88182

Merged
joshavant merged 1 commit into
mainfrom
fix/issue-88042-subagent-yield-completion
May 30, 2026
Merged

Fix subagent DM completion delivery after yield#88182
joshavant merged 1 commit into
mainfrom
fix/issue-88042-subagent-yield-completion

Conversation

@joshavant

@joshavant joshavant commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Require message-tool delivery for direct-message subagent completion handoffs instead of treating an automatic parent reply as sufficient.
  • Keep the existing direct text fallback for successful DM subagent completions when the announce agent omits the child result.
  • Add regression coverage for the issue 88042 NO_REOUTPUT shape and the new message-tool-only direct-DM contract.

Fixes #88042.

Verification

  • .agents/skills/autoreview/scripts/autoreview --mode local
    • First run found a privacy risk in an earlier raw-result fallback approach; accepted and changed the design.
    • Final run: autoreview clean: no accepted/actionable findings reported.
  • node scripts/run-vitest.mjs src/agents/subagent-announce-delivery.test.ts
    • 2 passed, 178 passed.
  • node scripts/run-vitest.mjs src/agents/subagent-announce-delivery.test.ts -t "directly delivers direct-message subagent text when the announce agent omits the result|requires message-tool delivery for direct subagent completions"
    • 2 passed, 4 passed, 174 skipped.
  • git diff --check
  • pnpm build

Real behavior proof

Behavior addressed: direct-message subagent completion after sessions_yield remains externally visible when the parent does not re-output the child result.

Real environment tested: local live Telegram DM using openai/gpt-5.5 and the patched checkout. AWS/Azure Crabbox live proof was attempted but blocked by coordinator cost_limit_exceeded; Blacksmith Testbox refused secret env forwarding.

Exact steps or command run after this patch: started a temporary live Telegram/OpenAI harness from the patched checkout, sent a DM prompt that spawned a child running sleep 90; echo TG88042_CHILD_9855743C, then forced the parent to reply only TG88042_NO_REOUTPUT_9855743C if it had to produce a final answer.

Evidence after fix: harness summary reported fixed: true, child task completed, task progress summary TG88042_CHILD_9855743C, parent TG88042_NO_REOUTPUT_9855743C generated internally, and Telegram outbound send succeeded after child completion.

Observed result after fix: human Telegram DM view showed only TG88042_CHILD_9855743C; it did not show TG88042_NO_REOUTPUT_9855743C.

What was not tested: completed remote Crabbox live proof, due coordinator budget limit; non-DM channel surfaces beyond the focused regression suite.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels May 30, 2026
@clawsweeper

clawsweeper Bot commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Summary
The PR changes direct-message subagent completion handoffs to require message-tool delivery after yield, with a child-result direct fallback when the announce agent omits visible output, and adds focused regression tests.

PR surface: Source +27, Tests +56. Total +83 across 2 files.

Reproducibility: yes. from source and linked issue evidence, but I did not run a live reproduction in this read-only review. Current main still treats direct-message subagent completions as automatic final delivery, while the linked report shows that path can lose the visible completion after yield.

Review metrics: 1 noteworthy metric.

  • Direct-message delivery contract: 1 route changed. The diff changes direct subagent completions from automatic final delivery to message-tool-only delivery plus fallback, which maintainers should notice before merge.

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:

  • Maintainer should decide whether to accept the message-tool-only direct-message completion contract across Telegram and Discord.

Mantis proof suggestion
A native Telegram Desktop proof would independently show that only the child completion appears after yield and the parent NO_REOUTPUT marker stays internal. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

telegram desktop proof: verify that a yielded subagent DM completion sends the child result and does not show the parent NO_REOUTPUT marker.

Risk before merge

  • [P2] The PR intentionally changes direct-message subagent completion semantics from automatic final delivery to message-tool-only delivery plus child-result fallback, so maintainers should accept that delivery contract across Telegram and Discord before merge.
  • [P1] The PR has local live Telegram proof and focused shared-core tests, but no completed remote Crabbox proof and no live Discord DM proof.

Maintainer options:

  1. Accept the DM delivery contract (recommended)
    Land the PR if maintainers agree that DM subagent completions should require message-tool delivery and fall back to the child result when the announce agent omits visible output.
  2. Request independent transport proof
    Ask for a Mantis Telegram Desktop proof or equivalent live Discord/Telegram proof if maintainers want independent confirmation before changing delivery semantics.
  3. Redirect the contract choice
    Pause or close this PR only if maintainers prefer preserving automatic DM final delivery and fixing the linked message-loss path another way.

Next step before merge

  • [P2] The PR is protected by the maintainer label and the remaining action is maintainer acceptance of a user-visible delivery contract, not an automation repair.

Security
Cleared: The diff changes agent delivery logic and colocated tests only; it does not add dependencies, workflows, permissions, scripts, package metadata, or secret handling.

Review details

Best possible solution:

Land the narrow fix if maintainers agree that direct-message subagent completions after yield should use message-tool delivery and child-result fallback rather than automatic parent final delivery.

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

Yes from source and linked issue evidence, but I did not run a live reproduction in this read-only review. Current main still treats direct-message subagent completions as automatic final delivery, while the linked report shows that path can lose the visible completion after yield.

Is this the best way to solve the issue?

Unclear until maintainer review accepts the contract change. The patch is narrow and reuses the existing message-tool-only delivery machinery, but it deliberately changes user-visible DM completion semantics.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6f3f4f74207a.

Label changes

Label changes:

  • add P1: The PR addresses linked P1 message loss for subagent completions after yield in a user-visible channel workflow.
  • add merge-risk: 🚨 message-delivery: The diff changes how direct-message subagent completions are delivered, which can affect whether users see, miss, or duplicate completion messages.

Label justifications:

  • P1: The PR addresses linked P1 message loss for subagent completions after yield in a user-visible channel workflow.
  • merge-risk: 🚨 message-delivery: The diff changes how direct-message subagent completions are delivered, which can affect whether users see, miss, or duplicate completion messages.
  • 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 includes after-fix local live Telegram DM proof showing the child completion visible externally and the internal NO_REOUTPUT marker not visible; remote Crabbox proof was attempted but blocked by coordinator limits.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix local live Telegram DM proof showing the child completion visible externally and the internal NO_REOUTPUT marker not visible; remote Crabbox proof was attempted but blocked by coordinator limits.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The change affects visible Telegram DM completion delivery and can be demonstrated in a short Telegram Desktop recording.
Evidence reviewed

PR surface:

Source +27, Tests +56. Total +83 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 32 5 +27
Tests 1 59 3 +56
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 91 8 +83

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/agents/subagent-announce-delivery.test.ts.
  • [P1] node scripts/run-vitest.mjs src/agents/subagent-announce-delivery.test.ts -t "directly delivers direct-message subagent text when the announce agent omits the result|requires message-tool delivery for direct subagent completions".
  • [P1] git diff --check.
  • [P1] pnpm build.

What I checked:

  • Protected maintainer label: The GitHub context lists the protected maintainer label, so this PR should not be closed by conservative cleanup even if it otherwise looked stale or superseded.
  • Root policy applied: Root review policy requires full AGENTS.md review, protected-label handling, proof review, and whole delivery-surface review for agent/channel changes. (AGENTS.md:13, 6f3f4f74207a)
  • Scoped agent policy applied: The scoped agents guide emphasizes preserving behavior proof and using lightweight injected tests for agent runtime paths, which matches the focused regression-test shape here. (src/agents/AGENTS.md:1, 6f3f4f74207a)
  • Current main behavior: Current main computes requiresMessageToolDelivery only from the configured completion route, so direct-message subagent completions can still request automatic final delivery. (src/agents/subagent-announce-delivery.ts:1222, 6f3f4f74207a)
  • Current main test contract: Current main still has a regression test named keeps automatic final delivery for direct subagent completions, confirming the existing DM contract the PR changes. (src/agents/subagent-announce-delivery.test.ts:4179, 6f3f4f74207a)
  • PR diff: The PR adds a subagent-DM-specific message-tool requirement, changes the direct-completion test to deliver: false plus sourceReplyDeliveryMode: message_tool_only, and adds a fallback test for omitted announce output. (src/agents/subagent-announce-delivery.ts:1229, 0896917e1ad9)

Likely related people:

  • Peter Steinberger: The central delivery file in current main appears in the available history from commit ccad5d7b638a998e956613221e89d2beece117a0, and Peter also authored recent adjacent agent terminal-outcome work. (role: recent area contributor; confidence: medium; commits: ccad5d7b638a, acb0e9c1552f; files: src/agents/subagent-announce-delivery.ts, src/agents/agent-run-terminal-outcome.ts, src/gateway/session-lifecycle-state.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 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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. labels May 30, 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. labels May 30, 2026
@joshavant joshavant merged commit b3b962a into main May 30, 2026
175 of 185 checks passed
@joshavant joshavant deleted the fix/issue-88042-subagent-yield-completion branch May 30, 2026 01:24
@joshavant joshavant mentioned this pull request May 30, 2026
Closed
@Haderach-Ram Haderach-Ram mentioned this pull request May 30, 2026
Open
@github-actions github-actions Bot mentioned this pull request May 30, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 30, 2026
@joshavant
fix subagent dm completion delivery (openclaw#88182)
45a05a8
steipete pushed a commit that referenced this pull request May 30, 2026
@joshavant @steipete
fix subagent dm completion delivery (#88182)
00d87c7
@steipete steipete mentioned this pull request May 30, 2026
Closed
steipete pushed a commit that referenced this pull request May 30, 2026
@joshavant @steipete
fix subagent dm completion delivery (#88182)
5d3a690 
(cherry picked from commit 00d87c7)
@github-actions github-actions Bot mentioned this pull request May 30, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request May 31, 2026
Closed
2 tasks
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 31, 2026
@joshavant @steipete
fix subagent dm completion delivery (openclaw#88182)
35c7d1d 
(cherry picked from commit 00d87c7)
@samuelsoaress samuelsoaress mentioned this pull request Jun 1, 2026
Closed
SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026
@joshavant
fix subagent dm completion delivery (openclaw#88182)
21f6826
SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026
@joshavant @steipete
fix subagent dm completion delivery (openclaw#88182)
0fa4c71 
(cherry picked from commit 00d87c7)
@clawsweeper clawsweeper Bot mentioned this pull request Jun 8, 2026
Open
2 tasks
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
@joshavant
fix subagent dm completion delivery (openclaw#88182)
9480204
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
@joshavant @steipete
fix subagent dm completion delivery (openclaw#88182)
f62aed8 
(cherry picked from commit 00d87c7)
@wangwllu wangwllu mentioned this pull request Jun 12, 2026
Open
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 maintainer Maintainer-authored PR mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S 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.

Subagent announce fails after sessions_yield: adapter unavailable during yield suspension

1 participant

@joshavant