fix(auto-reply): restore route replies for exec-event completions by wzfukui · Pull Request #70258 · openclaw/openclaw

wzfukui · 2026-04-22T16:24:03Z

Summary

restore route-reply delivery for async exec-event replies by falling back to persisted session delivery context
keep the existing direct routing behavior when OriginatingChannel and OriginatingTo are present on the live turn
add regression coverage for system-event replies that must return to the originating channel surface

Root cause

Async completion turns can lose direct channel metadata by the time the reply is generated. In that case the router currently skips channel route-reply delivery entirely, so channel plugins never receive the final completion message.

Testing

pnpm vitest run src/auto-reply/reply/dispatch-from-config.test.ts

greptile-apps · 2026-04-22T16:25:50Z

Greptile Summary

This PR fixes async exec-event completions silently dropping their channel route-replies by falling back to the persisted session delivery context (deliveryContext / lastChannel / lastTo / lastAccountId) when the live turn carries no OriginatingChannel or OriginatingTo. The isSystemEventTurn guard that gates the fallback is consistent with the same three-provider grouping (heartbeat, cron-event, exec-event) already used in session.ts.

Confidence Score: 5/5

Safe to merge; the only finding is a minor test assertion gap that doesn't affect production behaviour.

The implementation is correct and consistent with existing patterns. The null guard in sendPayloadAsync prevents unintended routing when no channel metadata is available at all. The single P2 finding (missing accountId assertion in the new test) is a test-completeness suggestion, not a production defect.

No files require special attention.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/auto-reply/reply/dispatch-from-config.test.ts
Line: 466-472

Comment:
**Missing `accountId` assertion in new test**

The test sets up `deliveryContext.accountId: "acc-1"` and `AccountId: undefined` precisely to exercise the `routeAccountId` fallback — but the `routeReply` expectation never asserts `accountId`. The new `routeAccountId` variable is the other half of this fix (it's passed to four `routeReply` call-sites), so leaving it unverified means the test would still pass even if the account-ID propagation regressed.

```suggestion
    expect(mocks.routeReply).toHaveBeenCalledWith(
      expect.objectContaining({
        channel: "telegram",
        to: "telegram:999",
        accountId: "acc-1",
      }),
    );
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(auto-reply): route exec-event replie..." | Re-trigger Greptile}

greptile-apps · 2026-04-22T16:25:54Z

+    expect(dispatcher.sendFinalReply).not.toHaveBeenCalled();
+    expect(mocks.routeReply).toHaveBeenCalledWith(
+      expect.objectContaining({
+        channel: "telegram",
+        to: "telegram:999",
+      }),
+    );


Missing accountId assertion in new test

The test sets up deliveryContext.accountId: "acc-1" and AccountId: undefined precisely to exercise the routeAccountId fallback — but the routeReply expectation never asserts accountId. The new routeAccountId variable is the other half of this fix (it's passed to four routeReply call-sites), so leaving it unverified means the test would still pass even if the account-ID propagation regressed.

Suggested change

expect(dispatcher.sendFinalReply).not.toHaveBeenCalled();

expect(mocks.routeReply).toHaveBeenCalledWith(

expect.objectContaining({

channel: "telegram",

to: "telegram:999",

}),

);

expect(mocks.routeReply).toHaveBeenCalledWith(

expect.objectContaining({

channel: "telegram",

to: "telegram:999",

accountId: "acc-1",

}),

);

Prompt To Fix With AI

This is a comment left during a code review. Path: src/auto-reply/reply/dispatch-from-config.test.ts Line: 466-472 Comment: **Missing `accountId` assertion in new test** The test sets up `deliveryContext.accountId: "acc-1"` and `AccountId: undefined` precisely to exercise the `routeAccountId` fallback — but the `routeReply` expectation never asserts `accountId`. The new `routeAccountId` variable is the other half of this fix (it's passed to four `routeReply` call-sites), so leaving it unverified means the test would still pass even if the account-ID propagation regressed. ```suggestion expect(mocks.routeReply).toHaveBeenCalledWith( expect.objectContaining({ channel: "telegram", to: "telegram:999", accountId: "acc-1", }), ); ``` 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: 3f07711469

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-22T16:27:34Z

      suppressDirectUserDelivery: suppressAcpChildUserDelivery,
      isRoutableChannel: routeReplyRuntime.isRoutableChannel,
    });
-  const originatingTo = ctx.OriginatingTo;
+  const originatingTo = ctx.OriginatingTo ?? fallbackOriginatingTo;
+  const routeAccountId = ctx.AccountId ?? fallbackOriginatingAccountId;


Use fallback account for final route-reply sends

For system-event turns, this change correctly derives routeAccountId from persisted delivery context, but the main final-reply path in sendFinalPayload still calls routeReply with ctx.AccountId instead of the fallback. When exec-event/cron-event/heartbeat completions lose live account metadata (the regression this commit targets), final replies can still route with an undefined or wrong account, which breaks multi-account channel delivery or applies incorrect per-account messaging config.

Useful? React with 👍 / 👎.

wzfukui · 2026-04-23T01:09:45Z

Updated the PR branch against latest main, resolved the merge conflict, and added the requested routeReply accountId assertion for the persisted delivery-context fallback test. Local targeted test passed: pnpm test src/auto-reply/reply/dispatch-from-config.test.ts.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 863429fd24

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-23T01:18:16Z

+  const effectiveOriginatingChannel = ctx.OriginatingChannel ?? fallbackOriginatingChannel;
+  const effectiveOriginatingTo = ctx.OriginatingTo ?? fallbackOriginatingTo;


Honor sendPolicy for fallback-routed system-event replies

When a heartbeat/cron-event/exec-event turn is missing live origin fields, this code now reconstructs effectiveOriginatingChannel/effectiveOriginatingTo from session state and routes outbound replies there, but sendPolicy is still resolved later from the raw context (ctx.OriginatingChannel ?? ctx.Surface ?? ctx.Provider). In that scenario, channel rules are evaluated against exec-event instead of the actual routed channel (for example, telegram), so channel-based deny rules can be bypassed and a reply is still delivered. Please feed the effective fallback route into policy resolution so suppression is evaluated against the true outbound destination.

Useful? React with 👍 / 👎.

martingarramon · 2026-04-23T18:40:39Z

Static review. LGTM-shape on the fallback direction.

One design question + two gaps:

Stale-route risk at channel level. dispatch-from-config.ts:290-293 carries an explicit warning: "Do not read thread ids from the normalised session store here: origin.threadId can be folded back into lastThreadId/deliveryContext during store normalisation and resurrect a stale route after thread delivery was intentionally cleared." The PR now reads deliveryContext.channel / .to / .accountId from that same store at channel granularity. Does the same resurrection risk apply? Failure mode: a user who moved a Slack session to Telegram mid-life gets exec-event replies fired back to Slack.
accountId revalidation. routeAccountId replaces ctx.AccountId in both route runtime and direct-dispatcher paths. If sessionStoreEntry.entry?.lastAccountId is a revoked/rotated profile, exec-event replies go through an authprofile the user meant to drop. Re-validating against current profiles would close it.
Test matrix. Happy path covered. Worth adding: (a) exec-event with no deliveryContext AND no lastChannel (should not fabricate a route), (b) persistedDeliveryContext.channel diverging from both Provider and Surface (does suppressAcpChildUserDelivery still short-circuit correctly?).

steipete · 2026-04-23T19:28:06Z

Landed via rebase onto main.

Gate: pnpm test src/auto-reply/reply/dispatch-from-config.test.ts; pnpm check:changed
Source head before merge: e85f6b6
Land commit: 2b45bc1

Thanks @wzfukui!

…xt loss Exec completion events enqueued under cron or channel-specific session keys were invisible to the heartbeat runner, which always checked the agent's main session key. This caused exec results to silently vanish instead of being relayed back to the originating channel. 1. Propagate sessionKey through scopedHeartbeatWakeOptions in bash exec, ACP spawn, gateway/node exec, and CLI watchdog paths 2. Add resolveEventSessionKey to remap cron session keys to the originating agent's main session key for event enqueueing Scope was previously broader. Dropped per maintainer review feedback on openclaw#50818 (issuecomment-4322767117): - The forceLastTargetWhenNone override for resolveHeartbeatDeliveryTarget, which routed exec completions to the session's last target even when heartbeat.target was explicitly "none". Current main and its regression tests intentionally keep target="none" internal-only, so this routing change is left for a separate focused PR if it is ever revisited. - The openclaw#39978 hook-cancel handling has already shipped via upstream PR openclaw#70258, so it is no longer part of this PR's scope. The PR now closes openclaw#52305 only. Closes openclaw#52305. Related: openclaw#18237. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

openclaw-barnacle Bot added the size: S label Apr 22, 2026

greptile-apps Bot reviewed Apr 22, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Apr 22, 2026

View reviewed changes

wzfukui force-pushed the fix/openclaw-exec-event-delivery-context branch from 3f07711 to 863429f Compare April 23, 2026 01:08

chatgpt-codex-connector Bot reviewed Apr 23, 2026

View reviewed changes

wzfukui and others added 3 commits April 23, 2026 20:21

fix(auto-reply): route exec-event replies via persisted delivery context

0dba10f

fix(auto-reply): honor recovered system-event reply routes

603c7c2

fix(auto-reply): restore exec-event route replies (openclaw#70258)

e85f6b6

steipete force-pushed the fix/openclaw-exec-event-delivery-context branch from fd6c33c to e85f6b6 Compare April 23, 2026 19:24

steipete merged commit 2b45bc1 into openclaw:main Apr 23, 2026
67 checks passed

github-actions Bot mentioned this pull request Apr 23, 2026

📡 Upstream Digest — 2026-04-23 20:44 UTC curtismercier/openclaw-mods#660

Open

steipete mentioned this pull request Apr 24, 2026

[Bug]: Discord async exec/approval follow-up replies can persist in transcript but never reach the channel #39978

Closed

Kaspre mentioned this pull request Apr 26, 2026

fix(heartbeat): remap cron-run session keys for exec/ACP/CLI events (refs #52305) #50818

Closed

8 tasks

ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026

fix(auto-reply): restore exec-event route replies (openclaw#70258)

f52bbe6

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026

fix(auto-reply): restore exec-event route replies (openclaw#70258)

cbbd12c

globalcaos pushed a commit to globalcaos/tinkerclaw that referenced this pull request May 13, 2026

fix(auto-reply): restore exec-event route replies (openclaw#70258)

cf54e7f

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026

fix(auto-reply): restore exec-event route replies (openclaw#70258)

d1d2a5b

jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026

fix(auto-reply): restore exec-event route replies (openclaw#70258)

730b4aa

sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026

fix(auto-reply): restore exec-event route replies (openclaw#70258)

40a32af

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(auto-reply): restore route replies for exec-event completions#70258

fix(auto-reply): restore route replies for exec-event completions#70258
steipete merged 3 commits into
openclaw:mainfrom
wzfukui:fix/openclaw-exec-event-delivery-context

wzfukui commented Apr 22, 2026

Uh oh!

greptile-apps Bot commented Apr 22, 2026

Uh oh!

greptile-apps Bot Apr 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 22, 2026

Uh oh!

wzfukui commented Apr 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 23, 2026

Uh oh!

martingarramon commented Apr 23, 2026

Uh oh!

Uh oh!

steipete commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		const effectiveOriginatingChannel = ctx.OriginatingChannel ?? fallbackOriginatingChannel;
		const effectiveOriginatingTo = ctx.OriginatingTo ?? fallbackOriginatingTo;

Uh oh!

Conversation

wzfukui commented Apr 22, 2026

Summary

Root cause

Testing

Uh oh!

greptile-apps Bot commented Apr 22, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

greptile-apps Bot Apr 22, 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 Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

wzfukui commented Apr 23, 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 Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

martingarramon commented Apr 23, 2026

Uh oh!

Uh oh!

steipete commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants