fix(hooks): emit internal message:sent for dispatcher-only reply paths#30668
fix(hooks): emit internal message:sent for dispatcher-only reply paths#30668ou524u wants to merge 2 commits intoopenclaw:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfaf5dd755
ℹ️ 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".
| createInternalHookEvent("message", "sent", sessionKey, { | ||
| to: ctx.To ?? ctx.From ?? "", | ||
| content: "", | ||
| success: true, |
There was a problem hiding this comment.
Emit message:sent only after delivery outcome is known
This new hook is emitted with success: true immediately after sendFinalReply/routing bookkeeping, but sendFinalReply only confirms enqueueing and returns before delivery runs (createReplyDispatcher enqueues at src/auto-reply/reply/reply-dispatcher.ts and executes deliver asynchronously). In any transport failure after enqueue, internal hooks will still receive a successful message:sent, which can trigger downstream automations/metrics as false positives.
Useful? React with 👍 / 👎.
| to: ctx.To ?? ctx.From ?? "", | ||
| content: "", | ||
| success: true, | ||
| channelId: channel, | ||
| accountId: ctx.AccountId, |
There was a problem hiding this comment.
Use routed target fields for message:sent context
When replies are routed to a different originating channel (routeReply uses originatingChannel/originatingTo), this hook still reports to and channelId from the inbound context (ctx.To/ctx.From and channel). That produces incorrect metadata for cross-channel sends, so any internal hook filtering by destination/channel can execute against the wrong conversation.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR successfully fixes missing Key changes:
Implementation notes:
Confidence Score: 5/5
Last reviewed commit: deb5208 |
…ndition
- Use Symbol.for('openclaw.internal-hooks.handlers') on globalThis so
register and trigger always share one Map even across bundled chunks.
- Widen message:sent trigger from (queuedFinal || routedFinalCount > 0)
to (deliveredCount > 0) covering tool/block/final counts.
- Add singleton registry test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deb5208616
ℹ️ 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".
| const deliveredCount = counts.tool + counts.block + counts.final; | ||
| if (sessionKey && deliveredCount > 0) { |
There was a problem hiding this comment.
Exclude unsent routed payloads from message:sent emission
The new deliveredCount > 0 gate can report a sent message even when nothing was actually delivered on routed paths: routeReply returns ok: true for payloads that normalize to empty and are skipped (src/auto-reply/reply/route-reply.ts:85-99), and those ok results are counted before this hook is fired. In that case, message:sent is emitted with success: true although no outbound message was sent, which can trigger false positives in internal automations/metrics for cross-channel routing.
Useful? React with 👍 / 👎.
Summary
This fixes missing
message:sentinternal hook events for reply paths that use channel-provided dispatchers (not the standarddeliverOutboundPayloadsflow).In practice, Feishu uses a custom dispatcher deliver callback for streaming/card/direct sends, so
deliverOutboundPayloads-levelemitMessageSentis bypassed and internal hooks never seemessage:sent.Root cause
message:sentinternal hooks were emitted from delivery plumbing that is only exercised bydeliverOutboundPayloads/deliverOutboundPayloadsCore.Map) was module-scoped, so when bundling splits code across multiple chunks,registerInternalHookandtriggerInternalHookcould reference different Map instances — register succeeds but trigger finds no handlers.Changes
1. Emit
message:sentat dispatch layer (dispatch-from-config.ts)After successful completion (
recordProcessed("completed")), emit an internalmessage:senthook when:sessionKeyis present, andcounts.tool + counts.block + counts.final > 0).2. Global singleton hook registry (
internal-hooks.ts)Use
Symbol.for("openclaw.internal-hooks.handlers")onglobalThisso thatregisterInternalHookandtriggerInternalHookalways share one Map, even across bundled chunks.Why
dispatch-from-config.tsdispatchReplyFromConfigis the common orchestration point across both:routeReply), anddispatcher.sendFinalReply).Emitting here ensures internal hooks observe completion in both paths.
Validation
message:received→ 👏 CLAP reaction addedmessage:sent→ ✅ OK reaction addedRelated
message:sentnever fires due to splithandlersMap between bundle chunks #30624