feat(hooks): wire outbound message lifecycle hooks#12584
feat(hooks): wire outbound message lifecycle hooks#12584vincentkoc wants to merge 15 commits intoopenclaw:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (1)
Also appears in Prompt To Fix With AIThis is a comment left during a code review.
Path: src/gateway/server-methods/send.ts
Line: 168:191
Comment:
**Missing session context propagation**
`deliverOutboundPayloads` now accepts `sessionKey`/`sessionId` for message hook attribution, but this call site doesn’t pass either, so outbound `message_sending`/`message_sent` hooks invoked from the gateway `send` endpoint will still have undefined session fields. If hook consumers rely on session attribution (per PR description), this path won’t work as intended.
Also appears in `src/infra/outbound/message.ts:162-179`, `src/auto-reply/reply/route-reply.ts:118-136`, `src/infra/heartbeat-runner.ts:803-820`, `src/cron/isolated-agent/run.ts:528-537`.
How can I resolve this? If you propose a fix, please make it concise. |
Additional Comments (1)
In the chunked send loops, Also affects Also appears in Prompt To Fix With AIThis is a comment left during a code review.
Path: src/infra/outbound/deliver.ts
Line: 324:327
Comment:
**Incorrect failure hook content**
In the chunked send loops, `message_sent` is only emitted on the success path. If `handler.sendText(...)` (or `sendSignal(...)` in `sendSignalTextChunks`) throws, control jumps to the outer payload `catch`, which calls `runMessageSentHook(attemptedSendContent, ...)`. But `attemptedSendContent` is only tracked for the payload/media paths, not for per-chunk text sends, so the failure `message_sent.content` can be wrong (often the full original payload text rather than the specific chunk that failed). This breaks downstream auditing/guardrails that expect the attempted chunk content.
Also affects `sendSignalTextChunks` where rewritten chunks are sent in an inner loop without updating any payload-scoped attempted content.
Also appears in `src/infra/outbound/deliver.ts` around 379-402.
How can I resolve this? If you propose a fix, please make it concise. |
Additional Comments (1)
This file still does Prompt To Fix With AIThis is a comment left during a code review.
Path: src/auto-reply/reply/dispatch-from-config.ts
Line: 68:72
Comment:
**Inconsistent session key lookup**
This file still does `store[sessionKey.toLowerCase()] ?? store[sessionKey]` when reading the session store, but elsewhere the PR notes a “session-key normalization touch-up”. If other code paths now normalize differently (e.g., via a shared helper), leaving this ad-hoc normalization here can cause mismatches for mixed-case keys and duplicate entries. Align this lookup with the canonical session-key normalization used by the session store APIs.
How can I resolve this? If you propose a fix, please make it concise. |
|
@greptileai fixed sendTextChunks so it now mirrors the Signal chunk behavior, patching the last issue you raised. PR should be ready to merge, can you update the PR score please? |
Additional Comments (1)
In Prompt To Fix With AIThis is a comment left during a code review.
Path: src/infra/outbound/message.ts
Line: 162:165
Comment:
**Missing session attribution**
In `sendMessage()`, the delivery call only forwards a session key when mirroring is enabled. Since calls without mirroring are allowed, outbound message hooks invoked from this path will always receive an undefined session key in their context, preventing reliable session attribution for those sends.
How can I resolve this? If you propose a fix, please make it concise. |
Additional Comments (1)
In Prompt To Fix With AIThis is a comment left during a code review.
Path: src/infra/outbound/message.ts
Line: 162:168
Comment:
**Session context not propagated**
In `sendMessage()`, `deliverOutboundPayloads` is called with `sessionKey` sourced from `params.mirror?.sessionKey` (see `src/infra/outbound/message.ts:162-168`). This makes outbound hooks receive a session key only when mirroring is enabled, rather than from the actual send/session context used by normal agent/chat flows. As a result, many `message_sending` / `message_sent` events will have missing session attribution even after this PR.
How can I resolve this? If you propose a fix, please make it concise. |
vincentkoc
force-pushed
the
vincentkoc-code/hooks-message-lifecycle
branch
from
5ec6821 to
60002ac
Compare
When a message_sending plugin hook cancels outbound delivery by
returning { cancel: true }, the message_sent hook was silently
skipped. Plugins tracking delivery outcomes (audit logging,
analytics) had no visibility into cancelled sends.
Fire message_sent with { success: false, error: "canceled by
message_sending hook" } before the continue, following the pattern
established in PR openclaw#12584.
Closes openclaw#16596
thewilloftheshadow
force-pushed
the
main
branch
from
bfc1ccb to
f92900f
Compare
When a message_sending plugin hook cancels outbound delivery by
returning { cancel: true }, the message_sent hook was silently
skipped. Plugins tracking delivery outcomes (audit logging,
analytics) had no visibility into cancelled sends.
Fire message_sent with { success: false, error: "canceled by
message_sending hook" } before the continue, following the pattern
established in PR openclaw#12584.
Closes openclaw#16596
1 participant
Why
Message lifecycle hooks were not fully wired across outbound delivery paths, limiting observability and intervention points for message guardrails. This PR completes outbound lifecycle wiring and propagates session context so hook consumers can attribute events reliably.
Detailed Changes
message_sendingmessage_sent(success/failure/cancel-aware)sessionKeysessionIdsessionKeyintomessage_receivedhook context from dispatch flowRelated Links and Issues
Greptile Overview
Greptile Summary
This PR completes wiring for outbound message lifecycle plugin hooks by invoking
message_sending(with cancel/modify support) andmessage_sent(success/failure/cancel-aware) across outbound delivery paths, including text chunking, Signal chunk formatting, and media sends. It also extends hook context withsessionKey/sessionIdand propagates these fields through several call sites (gateway send, cron isolated-agent, heartbeat, auto-reply routing) so hook consumers can better attribute events.Core changes live in
src/infra/outbound/deliver.ts, which now runs the hooks around per-chunk/per-attachment sends and ensures failures are attributed to the attempted content. Tests were expanded substantially insrc/infra/outbound/deliver.test.tsto cover chunk rewrite limits, cancel behavior, and accurate failure reporting.Confidence Score: 4/5
deliver.tsare internally consistent and are backed by new regression tests for chunk rewrite/cancel/failure attribution. However,sendMessage()appears to passsessionKeyfrom the mirror config rather than from the actual session context, so hook consumers will still miss session attribution in common direct-send flows unless this is corrected.