fix: enable ACP completion relay for main sessions by bluk1020 · Pull Request #53256 · openclaw/openclaw

bluk1020 · 2026-03-24T00:41:28Z

Problem

mode=run ACP spawns from main sessions (e.g. Discord-bound agent sessions) fire successfully but never relay completion results back to the spawning session.

The root cause: implicitStreamToParent in resolveAcpSpawnStreamPlan requires requesterIsSubagentSession, which is false for main sessions. This means the completion relay is never set up, and the spawning session never receives the result.

Fix

Relax the implicitStreamToParent guard to enable the relay for any session with a valid parentSessionKey, not just subagent sessions.

The original subagent-specific heuristic (heartbeat enabled + relay route usable + no active binding) is preserved as a nested condition that only applies when the requester IS a subagent session. All other sessions (including main) get the relay unconditionally.

Uses isSubagentSessionKey(parentSessionKey) instead of the compound requesterIsSubagentSession flag to correctly handle legacy subagent:* key formats.

Testing

Tested on v2026.3.22 by patching the compiled dist
ACP mode=run spawn from main Discord session → CC runs → completion relays back ✅
Previously: spawn accepted, CC runs and completes, result silently dropped
Regression test added for main session relay path

Community Impact

This is a widely reported issue across the ACP ecosystem. Our fix directly addresses or unblocks:

Directly fixes

[bug] ACP oneshot completion callback never fires — lifecycle event race condition #52967 — ACP oneshot completion callback never fires (our original report)
ACP sessions never trigger auto-announce (missing lifecycle event in dispatch-acp path) #40693 — ACP sessions never trigger auto-announce (missing lifecycle event in dispatch-acp path)

Addresses the core problem described in

RFC: ACP completion relay — unified approach to close the parent notification loop #49782 — RFC: ACP completion relay — unified approach to close the parent notification loop (8 comments, community RFC)
Parent sessions can drop replies after sessions_spawn child completions and emit NO_REPLY #43547 — Parent sessions drop replies after sessions_spawn child completions and emit NO_REPLY (closed but pattern recurs)

Unblocks / partially resolves

ACP/coding-agent unusable from Telegram DM: run mode ends in ACP_TURN_FAILED, session mode blocked by thread binding #47348 — ACP/coding-agent unusable from Telegram DM (run mode completes but result never relayed)
ACP sessions inherit parent delivery context, ignoring acp.delivery.mode #33859 — ACP sessions inherit parent delivery context, ignoring acp.delivery.mode
ACP runs fail with ACP_TURN_FAILED via OpenClaw, while direct acpx CLI succeeds #28708 — ACP runs fail silently via OpenClaw while direct acpx CLI succeeds

The implicitStreamToParent gate being locked to subagent sessions is the root cause of the "ACP works but parent never hears back" pattern reported across Discord, Telegram, and other channel surfaces.

This PR fixes a silent result-drop bug where mode=run ACP spawns triggered from main sessions (e.g. Discord-bound agents) would execute successfully but never relay their completion back to the spawning session. The root cause was that implicitStreamToParent was gated on requesterIsSubagentSession, which is false for any non-subagent caller.

The fix replaces the compound requesterIsSubagentSession flag with a direct isSubagentSessionKey(parentSessionKey) check: subagent sessions retain the original heartbeat-gated path (!requesterHasActiveSubagentBinding && requesterHeartbeatEnabled && requesterHeartbeatRelayRouteUsable), while all other sessions with a valid parentSessionKey receive the relay unconditionally.

Key correctness points:

Legacy subagent:* keys are properly handled — isSubagentSessionKey returns true for them because the raw string starts with "subagent:", routing them to the heartbeat-gated branch where requesterHeartbeatRelayRouteUsable stays false (no parseable requesterAgentId), so the no-relay behavior is preserved and the existing test at line 862 continues to pass.
Regression test added — the new test at line 879 verifies the main-session relay path end-to-end, addressing the prior reviewer concern.

Both outstanding review concerns from previous threads are resolved in this iteration.

Confidence Score: 4/5

Safe to merge; the fix is targeted, the two prior review concerns are resolved, and a regression test guards the new path.
The logic change is minimal and well-reasoned: replacing requesterIsSubagentSession (which incorrectly excluded legacy keys and all main sessions) with isSubagentSessionKey(parentSessionKey) is strictly correct. Legacy subagent key semantics are preserved because isSubagentSessionKey matches raw subagent:* prefixes, sending them to the heartbeat-gated branch where they still evaluate to no-relay. The regression test covers the newly enabled main-session path. One minor nit: the inline loadSessionStoreMock re-setup in the new test duplicates the beforeEach default and adds no new coverage, but it does not affect correctness. Score is 4 rather than 5 only because the broader impact of unconditionally enabling relay for all non-subagent session types (cron, ACP, etc.) is not explicitly tested, though this appears to be intentional by design.
No files require special attention.

_{Reviews (2): Last reviewed commit: "fix: use isSubagentSessionKey for legacy..." | Re-trigger Greptile}

greptile-apps · 2026-03-24T00:44:48Z

+    (!requesterIsSubagentSession ||
+      (!requesterHasActiveSubagentBinding &&
+        requesterHeartbeatEnabled &&
+        requesterHeartbeatRelayRouteUsable));


Legacy subagent session keys unintentionally get implicit relay

requesterIsSubagentSession is defined as Boolean(requesterParsedSession) && isSubagentSessionKey(parentSessionKey). requesterParsedSession is the result of parseAgentSessionKey, which returns null for legacy-format keys that start with subagent: but don't match the agent:<id>:<rest> pattern.

For such a legacy key, requesterIsSubagentSession evaluates to false, so !requesterIsSubagentSession becomes true, and the relay is now unconditionally enabled — changing the intentional "no relay" behavior that the existing test at line 862 ("does not implicitly stream for legacy subagent requester session keys") documents and asserts. That test will now fail.

If the intent is to apply the heartbeat-gated path to any session that semantically looks like a subagent (including legacy-format keys), the condition should call isSubagentSessionKey(parentSessionKey) directly rather than the compound requesterIsSubagentSession variable:

Suggested change

(!requesterIsSubagentSession ||

(!requesterHasActiveSubagentBinding &&

requesterHeartbeatEnabled &&

requesterHeartbeatRelayRouteUsable));

(!isSubagentSessionKey(parentSessionKey) ||

(!requesterHasActiveSubagentBinding &&

requesterHeartbeatEnabled &&

requesterHeartbeatRelayRouteUsable));

This way:

Main/non-subagent sessions get the relay unconditionally (the intended fix).

New-format subagent keys preserve the heartbeat-gated path.

Legacy subagent:* keys also hit the heartbeat-gated path (keeping existing behavior).

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/acp-spawn.ts Line: 497-500 Comment: **Legacy subagent session keys unintentionally get implicit relay** `requesterIsSubagentSession` is defined as `Boolean(requesterParsedSession) && isSubagentSessionKey(parentSessionKey)`. `requesterParsedSession` is the result of `parseAgentSessionKey`, which returns `null` for legacy-format keys that start with `subagent:` but don't match the `agent:<id>:<rest>` pattern. For such a legacy key, `requesterIsSubagentSession` evaluates to `false`, so `!requesterIsSubagentSession` becomes `true`, and the relay is now unconditionally enabled — changing the intentional "no relay" behavior that the existing test at line 862 ("does not implicitly stream for legacy subagent requester session keys") documents and asserts. That test will now fail. If the intent is to apply the heartbeat-gated path to any session that semantically looks like a subagent (including legacy-format keys), the condition should call `isSubagentSessionKey(parentSessionKey)` directly rather than the compound `requesterIsSubagentSession` variable: ```suggestion (!isSubagentSessionKey(parentSessionKey) || (!requesterHasActiveSubagentBinding && requesterHeartbeatEnabled && requesterHeartbeatRelayRouteUsable)); ``` This way: - Main/non-subagent sessions get the relay unconditionally (the intended fix). - New-format subagent keys preserve the heartbeat-gated path. - Legacy `subagent:*` keys also hit the heartbeat-gated path (keeping existing behavior). How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-03-24T00:44:49Z

  const implicitStreamToParent =
    !streamToParentRequested &&
    spawnMode === "run" &&
    !requestThreadBinding &&
-    requesterIsSubagentSession &&
-    !requesterHasActiveSubagentBinding &&
+    Boolean(parentSessionKey) &&
    !requesterHasThreadContext &&
-    requesterHeartbeatEnabled &&
-    requesterHeartbeatRelayRouteUsable;
+    // Subagent orchestrators with heartbeat relay use the optimised path;
+    // all other sessions (including main) always get the relay.
+    (!requesterIsSubagentSession ||
+      (!requesterHasActiveSubagentBinding &&
+        requesterHeartbeatEnabled &&
+        requesterHeartbeatRelayRouteUsable));


No regression test added for the fixed main-session relay path

Per the repo's bug-fix merge gate (CLAUDE.md), bug-fix PRs require a regression test when feasible. The PR does not add a test case verifying that a plain main session key now receives an implicit relay for mode=run spawns. Adding such a test would document the fix and guard against future regressions.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/acp-spawn.ts Line: 489-500 Comment: **No regression test added for the fixed main-session relay path** Per the repo's bug-fix merge gate (CLAUDE.md), bug-fix PRs require a regression test when feasible. The PR does not add a test case verifying that a plain main session key now receives an implicit relay for `mode=run` spawns. Adding such a test would document the fix and guard against future regressions. How can I resolve this? If you propose a fix, please make it concise.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f21687933

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-24T00:45:23Z

+    (!requesterIsSubagentSession ||
+      (!requesterHasActiveSubagentBinding &&
+        requesterHeartbeatEnabled &&
+        requesterHeartbeatRelayRouteUsable));


Gate implicit parent relay on session-local heartbeat safety

This condition now makes implicitStreamToParent true for any non-subagent requester, which bypasses the safety checks in hasSessionLocalHeartbeatRelayRoute that were preventing non-local delivery contexts (for example session.scope="global" or heartbeat to/accountId overrides). In those configs, a mode=run spawn from a main session can enqueue relay updates that heartbeat delivers to an unrelated shared/pinned destination, so ACP completion/progress from one requester can leak into another channel/session.

Useful? React with 👍 / 👎.

bluk1020 · 2026-03-24T00:59:26Z

Thanks for the sharp review @greptile-apps — both points were valid.

Fixed in 6cb66fb:

Legacy subagent:* key regression — Replaced !requesterIsSubagentSession with !isSubagentSessionKey(parentSessionKey) so legacy keys correctly hit the heartbeat-gated path. The existing test at line 862 should now pass.
Regression test added — New test case "implicitly streams mode=run ACP spawns for main (non-subagent) sessions" verifies that a main session key (e.g. agent:main:discord:channel:test-channel) receives the implicit relay for mode=run spawns.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6cb66fb93c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-24T01:03:59Z

+    (!isSubagentSessionKey(parentSessionKey) ||
+      (!requesterHasActiveSubagentBinding &&
+        requesterHeartbeatEnabled &&
+        requesterHeartbeatRelayRouteUsable));


Restore session-local guard for implicit parent relay

This new implicitStreamToParent condition now auto-enables relay for every non-subagent requester, which bypasses the hasSessionLocalHeartbeatRelayRoute safety gate that previously blocked non-local heartbeat delivery contexts (for example session.scope="global" or heartbeat to/accountId overrides). In those configurations, a mode="run" ACP spawn can relay completion/progress through shared or pinned heartbeat destinations rather than the spawning session, so updates from one requester can surface in unrelated conversations.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9cce22b591

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-24T01:15:34Z

+    (!isSubagentSessionKey(parentSessionKey) ||
+      (!requesterHasActiveSubagentBinding &&
+        requesterHeartbeatEnabled &&


Gate main-session implicit relay on session-local route safety

This branch makes implicitStreamToParent true for every non-subagent requester, bypassing the hasSessionLocalHeartbeatRelayRoute safety constraints that previously blocked non-local heartbeat delivery contexts. In configurations like session.scope="global" or heartbeat overrides with explicit to/accountId, ACP spawn relay events are still enqueued and heartbeat wake-up can route those progress/completion updates to shared or pinned destinations, exposing one requester’s run status in unrelated conversations. Keep implicit relay behind the same session-local route checks (and require explicit streamTo="parent" when unsafe). Fresh evidence: the newly added non-subagent test in src/agents/acp-spawn.test.ts asserts this unconditional path, so the bypass is now intentional behavior.

Useful? React with 👍 / 👎.

Previously, implicitStreamToParent required requesterIsSubagentSession, which meant mode=run ACP spawns from main sessions (e.g. Discord-bound agent sessions) would fire off but never relay completion results back to the spawning session. Relax the guard to enable the relay for any session with a valid parentSessionKey. The original subagent-specific heuristic (heartbeat enabled + relay route usable) is preserved as a nested condition when the requester IS a subagent session. Fixes openclaw#52967 Related: openclaw#52878

…n regression test - Replace requesterIsSubagentSession with isSubagentSessionKey(parentSessionKey) to correctly handle legacy subagent:* keys (fixes test at line 862) - Add regression test: main (non-subagent) sessions get implicit relay for mode=run - Addresses Greptile review feedback on PR openclaw#53256

…reEach) Addresses Greptile nit — the test duplicated the mock setup already handled by beforeEach.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d69dc352b7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-25T01:16:27Z

+    Boolean(params.requester.parentSessionKey) &&
    !params.requester.hasThreadContext &&
-    params.requester.heartbeatEnabled &&
-    params.requester.heartbeatRelayRouteUsable;
+    // Subagent orchestrators with heartbeat relay use the optimised path;
+    // all other sessions (including main) always get the relay.
+    (!params.requester.isSubagentSession ||


Reapply session-local guard for main implicit parent relay

This condition now enables implicitStreamToParent for every non-subagent requester with a parent session key, so the safety checks in hasSessionLocalHeartbeatRelayRoute (which reject session.scope="global" and heartbeat to/accountId overrides) are bypassed for main sessions. In those configurations, the spawned ACP relay emits system events that heartbeat can deliver to a shared or pinned destination instead of the spawning conversation, so progress/completion text from one session can surface in unrelated chats; this path is now codified by the new non-subagent implicit-stream test.

Useful? React with 👍 / 👎.

vincentkoc · 2026-03-29T07:59:35Z

Closing as no longer current against main.

The main-session symptom this PR targeted has drifted: current acp-spawn.ts already falls back to inline delivery for non-subagent run-mode ACP spawns, which substantially changes the original root-cause analysis here. I do not want to merge an older relay heuristic on top of today's delivery behavior.

If there is still a main-session-specific completion gap on current main, please reopen with a fresh repro and we can evaluate it against the current relay/lifecycle branches (#40885 and #54701).

vincentkoc · 2026-03-29T08:00:00Z

Closing as no longer current against main.

The main-session symptom this PR targeted has drifted: current acp-spawn.ts already falls back to inline delivery for non-subagent run-mode ACP spawns, which substantially changes the original root-cause analysis here. I do not want to merge an older relay heuristic on top of today's delivery behavior.

If there is still a main-session-specific completion gap on current main, please reopen with a fresh repro and we can evaluate it against the current relay/lifecycle branches (#40885 and #54701).

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Mar 24, 2026

greptile-apps Bot reviewed Mar 24, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Mar 24, 2026

View reviewed changes

openclaw-barnacle Bot added size: S and removed size: XS labels Mar 24, 2026

chatgpt-codex-connector Bot reviewed Mar 24, 2026

View reviewed changes

This was referenced Mar 24, 2026

RFC: ACP completion relay — unified approach to close the parent notification loop #49782

Closed

[Bug]: ACP concurrent session spawns — first agent fails to launch CC process #53319

Open

feat: auto-yield parent session after mode=run ACP spawn #53483

Closed

claw added 3 commits March 25, 2026 09:09

test: remove redundant loadSessionStoreMock setup (inherits from befo…

d69dc35

…reEach) Addresses Greptile nit — the test duplicated the mock setup already handled by beforeEach.

bluk1020 force-pushed the fix/acp-main-session-completion-relay branch from 9cce22b to d69dc35 Compare March 25, 2026 01:12

chatgpt-codex-connector Bot reviewed Mar 25, 2026

View reviewed changes

vincentkoc self-assigned this Mar 27, 2026

vincentkoc closed this Mar 29, 2026

Uh oh!

Conversation

bluk1020 commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Testing

Community Impact

Directly fixes

Addresses the core problem described in

Unblocks / partially resolves

Related

Uh oh!

greptile-apps Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

bluk1020 commented Mar 24, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

vincentkoc commented Mar 29, 2026

Uh oh!

vincentkoc commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bluk1020 commented Mar 24, 2026 •

edited

Loading

greptile-apps Bot commented Mar 24, 2026 •

edited

Loading