Skip to content

fix(agents): preserve subagent wake delivery route#91381

Closed
ooiuuii wants to merge 1 commit into
openclaw:mainfrom
ooiuuii:agent/xiaozhua/sessions-yield-route-preservation
Closed

fix(agents): preserve subagent wake delivery route#91381
ooiuuii wants to merge 1 commit into
openclaw:mainfrom
ooiuuii:agent/xiaozhua/sessions-yield-route-preservation

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes #91356.

This preserves the parent turn's original delivery route when a subagent completion wakes an active requester session through sessions_yield / the active reply-run queue bridge.

Changes:

  • carry queue message options through queueReplyRunMessage(...) and ReplyBackendHandle.queueMessage(...) instead of dropping them on the active reply-run bridge
  • pass a resolved DeliveryContext into active subagent completion wake queue options
  • prefer user-facing external routes over internal completion/webchat origins for active wake context
  • allow subagent_announce user-facing inter-session provenance to inherit the established external route, matching the existing sessions_send route-preservation behavior

This is the same turn lifecycle visible-outcome reconciliation invariant as #91333 and #91357: once a parent channel turn has an established user-visible route, internal resume/completion bookkeeping should not silently replace it.

Real behavior proof

Behavior or issue addressed: Parent replies after a sessions_yield / subagent completion wake can resume under an internal completion/webchat context and lose the original external channel route.

Real environment tested: Local OpenClaw source checkout on branch agent/xiaozhua/sessions-yield-route-preservation, head 3728ae01a858e2679b196eb9f29283f33ef5cc33, using the repository focused Vitest runner and changed-file lint gate.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs src/auto-reply/reply/effective-reply-route.test.ts src/agents/embedded-agent-runner/runs.test.ts src/agents/subagent-announce-delivery.test.ts
node scripts/run-vitest.mjs $(cat /tmp/pr91381-state-routing-files.txt)
pnpm exec oxlint src/auto-reply/reply/effective-reply-route.ts src/auto-reply/reply/effective-reply-route.test.ts src/auto-reply/reply/reply-run-registry.ts src/agents/embedded-agent-runner/run-state.ts src/agents/embedded-agent-runner/runs.ts src/agents/embedded-agent-runner/runs.test.ts src/agents/subagent-announce-delivery.ts src/agents/subagent-announce-delivery.test.ts
git diff --check

Evidence after fix:

[test] passed focused affected shards: 15 + 124 tests passed
[test] passed CI state/routing shard locally: 534 tests passed
Found 0 warnings and 0 errors.

Observed result after fix: The active requester wake path now queues the completion with the parent external DeliveryContext, the reply-run bridge preserves those queue options, and subagent_announce inter-session provenance inherits the established external route when the live resume context is internal webchat. The added regression preserves the requester external route when the completion origin is internal verifies an internal completion origin no longer replaces the requester external route.

What was not tested: I did not run a live Telegram/Slack end-to-end delivery against production credentials. The proof is source-level route preservation plus focused regression coverage for the affected queue and route-resolution paths.

Tests

  • node scripts/run-vitest.mjs src/auto-reply/reply/effective-reply-route.test.ts src/agents/embedded-agent-runner/runs.test.ts src/agents/subagent-announce-delivery.test.ts
  • pnpm exec oxlint src/auto-reply/reply/effective-reply-route.ts src/auto-reply/reply/effective-reply-route.test.ts src/auto-reply/reply/reply-run-registry.ts src/agents/embedded-agent-runner/run-state.ts src/agents/embedded-agent-runner/runs.ts src/agents/embedded-agent-runner/runs.test.ts src/agents/subagent-announce-delivery.ts src/agents/subagent-announce-delivery.test.ts
  • git diff --check

Related: #91333, #91357

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 8, 2026
@clawsweeper

clawsweeper Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 8, 2026, 5:41 AM ET / 09:41 UTC.

Summary
The branch widens active reply-run queue message options, passes a resolved delivery context for subagent wake steering, and extends effective reply-route inheritance for subagent/inter-session handoffs.

PR surface: Source +74, Tests +98. Total +172 across 8 files.

Reproducibility: yes. at source level: the linked report describes a live sessions_yield route-loss path, and current main shows subagent wake steering plus effective route resolution can be driven by internal subagent_announce context. I did not run a live channel reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Queue option surface: 1 internal queue API widened. queueReplyRunMessage and ReplyBackendHandle.queueMessage now carry delivery options, so maintainers should verify every real backend consumes the new route metadata correctly.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Plumb or prove queued deliveryContext consumption in the real active embedded/reply-run steering path.
  • [P1] Add redacted real external-channel or runtime-log proof that the parent final after sessions_yield reaches the original route.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body now has the required proof fields, but the after-fix evidence is focused Vitest/lint output only; add redacted real external-channel proof, runtime logs, terminal output, or a linked artifact showing the sessions_yield parent final returning to the original chat, then update the PR body to trigger re-review.

Mantis proof suggestion
A real transport transcript would materially prove that the generic route fix delivers the parent final back to the original chat, which unit tests do not show. 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 live: verify a parent turn that uses sessions_spawn plus sessions_yield receives the child completion and sends the final reply back to the original Telegram chat.

Risk before merge

  • [P1] The patch changes generic parent-wake message routing, so a mistake can still drop or misroute final replies across external channel plugins even if focused unit tests pass.
  • [P2] The new queue deliveryContext is observable in mocks but not consumed by the real active steering helper on current source, leaving the active requester wake route dependent on persisted fallback behavior rather than the queued route option.

Maintainer options:

  1. Fix active wake route consumption before merge (recommended)
    Wire the queued deliveryContext into the real active embedded/reply-run steering path, or add proof that the persisted route fallback fully covers that path without the option.
  2. Accept source-level fallback with live proof
    Maintainers could accept the persisted-route fallback as the intended owner boundary, but should require redacted live external-channel or runtime-log proof that the final parent reply reaches the original route.
  3. Pause if the route owner is still unsettled
    If active steering cannot safely own delivery context, pause this PR and decide whether requester route state belongs in the queue message, session store, or dispatch route resolver.

Next step before merge

  • [P1] Human review is needed because the active-route consumption finding blocks merge and the missing real-environment proof cannot be supplied by an automated repair lane.

Security
Cleared: No dependency, workflow, secret-handling, or supply-chain surface changed; the remaining concern is functional message routing, covered separately.

Review findings

  • [P1] Consume the queued wake delivery context in the real active run — src/agents/subagent-announce-delivery.ts:637
Review details

Best possible solution:

Land a narrow agents/reply routing fix that actually carries requester delivery context through the active wake consumption path, with focused regression coverage and redacted external-channel or runtime proof before merge.

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

Yes, at source level: the linked report describes a live sessions_yield route-loss path, and current main shows subagent wake steering plus effective route resolution can be driven by internal subagent_announce context. I did not run a live channel reproduction in this read-only review.

Is this the best way to solve the issue?

No, not yet: preserving the requester route is the right fix direction, but this branch currently proves a mock receives deliveryContext while the real active steering helper ignores it. The safer solution is to consume that route in the active wake path or prove the persisted-route fallback covers the active path end to end.

Full review comments:

  • [P1] Consume the queued wake delivery context in the real active run — src/agents/subagent-announce-delivery.ts:637
    This new deliveryContext reaches the queue options, but the real active requester path then goes through attempt.ts into steerActiveSessionWithOptionalDeliveryWait, whose option type only uses deliveryTimeoutMs and waitForTranscriptCommit before calling activeSession.steer(text). That means the regression only proves a mock received the requester route; an active sessions_yield wake can still run without the queued route unless persisted session fallback happens to save it. Please plumb this context into the actual active steering/turn-source path or add an end-to-end source test proving the fallback covers that path.
    Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.82

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P1: The PR addresses a live agent/channel workflow where parent final replies after sessions_yield can be generated but not delivered to the originating external chat.
  • add merge-risk: 🚨 message-delivery: The diff changes route precedence and active wake delivery options, so merging it could still drop or misroute parent replies if the active-run path is incomplete.
  • add mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. Although the diff is generic agents routing, the user-visible outcome can be demonstrated as a Telegram chat receiving the parent final after a subagent sessions_yield wake.

Label justifications:

  • P1: The PR addresses a live agent/channel workflow where parent final replies after sessions_yield can be generated but not delivered to the originating external chat.
  • merge-risk: 🚨 message-delivery: The diff changes route precedence and active wake delivery options, so merging it could still drop or misroute parent replies if the active-run path is incomplete.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body now has the required proof fields, but the after-fix evidence is focused Vitest/lint output only; add redacted real external-channel proof, runtime logs, terminal output, or a linked artifact showing the sessions_yield parent final returning to the original chat, then update the PR body to trigger re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. Although the diff is generic agents routing, the user-visible outcome can be demonstrated as a Telegram chat receiving the parent final after a subagent sessions_yield wake.
Evidence reviewed

PR surface:

Source +74, Tests +98. Total +172 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 5 90 16 +74
Tests 3 100 2 +98
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 8 190 18 +172

What I checked:

  • Repository policy read: Root AGENTS.md and the scoped src/agents/AGENTS.md were read; the review applied the guidance to inspect owner boundaries, active-run callers/callees, adjacent tests, and delivery-route behavior instead of relying on the diff only. (AGENTS.md:1, b75d1a0b85b0)
  • PR queues a delivery context: The PR head computes queueDeliveryContext and includes it in active subagent wake queue options before calling the active requester queue path. (src/agents/subagent-announce-delivery.ts:632, 3728ae01a850)
  • PR forwards options into the reply-run bridge: The PR head changes queueReplyRunMessage to accept optional queue options and pass them to the attached backend when present. (src/auto-reply/reply/reply-run-registry.ts:538, 3728ae01a850)
  • Real active steering ignores route context: Current main's active embedded-run steering helper accepts only deliveryTimeoutMs and waitForTranscriptCommit, then calls activeSession.steer(text) without consuming deliveryContext, so the PR's new queue option is not proven to restore the active turn route. (src/agents/embedded-agent-runner/run/attempt.queue-message.ts:218, b75d1a0b85b0)
  • Route fallback extension is real but narrower than the queue claim: The PR head broadens resolveEffectiveReplyRoute so subagent_announce and other user-facing inter-session handoffs can inherit persisted external route state when the live surface is internal webchat. (src/auto-reply/reply/effective-reply-route.ts:37, 3728ae01a850)
  • Related bug context: The linked issue describes live channel-plugin parent finals after sessions_yield becoming visible in web/control transcript but not delivered back to the original external channel route.

Likely related people:

  • lanzhi-lee: The related merged subagent dispatch refactor introduced the current subagent-announce-delivery, effective-reply-route, reply-run registry, and active queue helper surfaces now being modified or relied on. (role: recent feature contributor; confidence: high; commits: 58bab0c2761e; files: src/agents/subagent-announce-delivery.ts, src/auto-reply/reply/effective-reply-route.ts, src/auto-reply/reply/reply-run-registry.ts)
  • Vincent Koc: Recent current-main history shows adjacent work in src/agents/embedded-agent-runner/run/attempt.ts, which owns the live queue handle that receives the new options. (role: recent adjacent contributor; confidence: medium; commits: 8b03fd1f5f83; files: src/agents/embedded-agent-runner/run/attempt.ts)
  • steipete: The same merged subagent dispatch commit records Peter as a co-author, making him a likely routing candidate for the recent area decision without implying fault. (role: adjacent co-author; confidence: medium; commits: 58bab0c2761e; files: src/agents/subagent-announce-delivery.ts, src/auto-reply/reply/effective-reply-route.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.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 8, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 8, 2026
@ooiuuii
ooiuuii force-pushed the agent/xiaozhua/sessions-yield-route-preservation branch from d8f1172 to 3728ae0 Compare June 8, 2026 09:30
@ooiuuii

ooiuuii commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated after the initial review:

  • Added a ## Real behavior proof section with the required fields; local proof evaluator passes and the GitHub Real behavior proof check is now green.
  • Fixed the no-options queueMessage(text) compatibility path so the existing registry call-shape test passes.
  • Tightened active wake route precedence so an internal completion/webchat origin does not replace the requester external route, with regression coverage for that case.

Additional local verification after the update:

  • Full local CI state/routing shard: 534 passed
  • Focused affected Vitest: 15 + 124 passed
  • Changed-file oxlint and git diff --check passed

@clawsweeper

clawsweeper Bot commented Jun 8, 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.

Re-review progress:

@clawsweeper clawsweeper Bot added mantis: telegram-visible-proof Mantis should capture Telegram visible proof. 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
@clawsweeper clawsweeper Bot mentioned this pull request Jun 8, 2026
Open
2 tasks
@zackchiutw zackchiutw mentioned this pull request Jun 8, 2026
Closed
@ooiuuii

ooiuuii commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by a clean replacement PR from current main: #92251

The replacement drops the broad/ambiguous parts of this PR and follows the maintainer boundary more tightly:

It also avoids claiming #91356 as a proven current-main core regression after Josh's current-main Telegram check. Closing this PR once the replacement is available for review.

@ooiuuii

ooiuuii commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of the clean replacement PR #92251.

@ooiuuii ooiuuii closed this Jun 11, 2026
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 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: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Parent final reply after sessions_yield can lose original channel delivery routing

1 participant

@ooiuuii