fix: narrow reply-root dedupe to queued reruns by livingghost · Pull Request #47920 · openclaw/openclaw

livingghost · 2026-03-16T05:23:56Z

Summary

dedupe queued reruns by explicit reply targets before falling back to thread roots
keep recent reply-root suppression on queued/followup paths so fresh inbound messages still run normally
only mark a reply root as sent after an outbound delivery actually returns a message result, and cover the behavior with new tests

Test plan

.\node_modules.bin\vitest.cmd run --config vitest.config.ts src/auto-reply/reply/route-reply.test.ts
.\node_modules.bin\vitest.cmd run --config vitest.config.ts src/auto-reply/reply/reply-root-dedupe.test.ts
.\node_modules.bin\vitest.cmd run --config vitest.config.ts src/auto-reply/reply/followup-runner.test.ts
.\node_modules.bin\vitest.cmd run --config vitest.config.ts src/auto-reply/reply/reply-flow.test.ts
.\node_modules.bin\vitest.cmd run --config vitest.config.ts src/infra/outbound/deliver.lifecycle.test.ts
.\node_modules.bin\vitest.cmd run --config vitest.e2e.config.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts

Track recent reply roots across queued and routed sends so the same inbound thread does not trigger duplicate agent replies. Surface the reply-root key into message hooks so plugin-side send guards can keep using the same identifier.

Avoid dropping fresh inbound messages that happen to share a reply root, prefer explicit reply targets over thread roots, and only mark a reply root as sent after an outbound delivery actually occurs.

livingghost · 2026-03-16T05:31:14Z

Closing and recreating this PR with the repository template body.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 78264ed959

ℹ️ 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-03-16T05:32:42Z

+  const replyRootId = run.replyRootId?.trim();
+  if (replyRootId) {
+    return { kind: "reply-root", value: replyRootId };


Deduplicate queued runs by message id before thread root

Prioritizing replyRootId here causes distinct thread messages to be collapsed as duplicates: get-reply-run.ts now fills replyRootId from inbound reply context, and for Slack that value is the thread root (thread_ts) for every message in the thread (extensions/slack/src/threading.ts), so in active collect/followup queues a second user turn with a different messageId is rejected as already queued. This drops legitimate user input instead of only suppressing true redeliveries.

Useful? React with 👍 / 👎.

greptile-apps · 2026-03-16T05:33:19Z

Greptile Summary

This PR narrows reply-root deduplication to only suppress queued/followup re-runs (not fresh inbound messages), deduplicates queued runs by explicit reply targets before falling back to message IDs, and gates the "recently sent" mark on a confirmed outbound delivery result rather than on enqueueing. The changes are well-structured, with a new reply-root-dedupe.ts module housing the cache and key-building logic, good test coverage across the affected paths, and a clean double-guard pattern (pre-enqueue in agent-runner.ts + pre-processing in followup-runner.ts).

Key areas to pay attention to:

Channel normalization mismatch in route-reply.ts: The key stored after delivery uses normalizeChannelId(channel) (lowercased + alias-resolved), but the keys checked in agent-runner.ts and followup-runner.ts use raw run.originatingChannel. For channels with registered aliases (e.g., "google-chat" → "googlechat", "imsg" → "imessage"), the mark and check keys will not match, silently disabling reply-root dedupe for those channels. The existing enqueue.ts dedup code uses raw channel values throughout to avoid this class of mismatch.
hookMetadata spread order in deliver.ts: In applyMessageSendingHook, ...params.hookMetadata is spread after the explicit channel, accountId, and mediaUrls fields in the metadata object. Future callers that include any of those keys in hookMetadata would silently shadow the explicit values with no TypeScript warning.

Confidence Score: 3/5

Generally safe to merge; the channel normalization mismatch could silently disable dedup for aliased channel names in production but will not cause incorrect sends or data loss.
The core logic is sound and well-tested. However, a key inconsistency in route-reply.ts — storing the dedup key with a normalized channel ID while all check sites use the raw channel — means dedupe would silently fail for any user whose originatingChannel maps through a CHAT_CHANNEL_ALIASES entry (e.g., google-chat, imsg). This is a correctness gap in the feature's primary goal. The hookMetadata spread pattern is also a latent footgun. Neither issue causes incorrect message delivery, which limits the risk.
src/auto-reply/reply/route-reply.ts — channel normalization mismatch between mark and check key paths; src/infra/outbound/deliver.ts — spread order in applyMessageSendingHook.

Comments Outside Diff (1)

src/infra/outbound/deliver.ts, line 436-441 (link)

Spread order allows hookMetadata to silently override channel, accountId, mediaUrls

...params.hookMetadata is spread after the explicit channel, accountId, and mediaUrls fields. Because hookMetadata is typed as Record<string, unknown>, TypeScript provides no protection if a future caller passes metadata that includes any of those keys — the explicit values would be silently overwritten.

The message_sent side avoids this by passing hookMetadata as a distinct nested field (metadata: params.hookMetadata). Consider the same pattern here to eliminate the footgun and make both hooks behave consistently:

metadata: {
  channel: params.channel,
  accountId: params.accountId,
  mediaUrls: params.payloadSummary.mediaUrls,
  hookMetadata: params.hookMetadata,
},

Or if the flat spread is intentional, placing the explicit fields after the spread ensures they can't be overridden:

metadata: {
  ...params.hookMetadata,
  channel: params.channel,
  accountId: params.accountId,
  mediaUrls: params.payloadSummary.mediaUrls,
},

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/infra/outbound/deliver.ts
Line: 436-441

Comment:
**Spread order allows `hookMetadata` to silently override `channel`, `accountId`, `mediaUrls`**

`...params.hookMetadata` is spread **after** the explicit `channel`, `accountId`, and `mediaUrls` fields. Because `hookMetadata` is typed as `Record<string, unknown>`, TypeScript provides no protection if a future caller passes metadata that includes any of those keys — the explicit values would be silently overwritten.

The `message_sent` side avoids this by passing `hookMetadata` as a distinct nested field (`metadata: params.hookMetadata`). Consider the same pattern here to eliminate the footgun and make both hooks behave consistently:

```ts
metadata: {
  channel: params.channel,
  accountId: params.accountId,
  mediaUrls: params.payloadSummary.mediaUrls,
  hookMetadata: params.hookMetadata,
},
```

Or if the flat spread is intentional, placing the explicit fields **after** the spread ensures they can't be overridden:
```ts
metadata: {
  ...params.hookMetadata,
  channel: params.channel,
  accountId: params.accountId,
  mediaUrls: params.payloadSummary.mediaUrls,
},
```

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

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/infra/outbound/deliver.ts
Line: 436-441

Comment:
**Spread order allows `hookMetadata` to silently override `channel`, `accountId`, `mediaUrls`**

`...params.hookMetadata` is spread **after** the explicit `channel`, `accountId`, and `mediaUrls` fields. Because `hookMetadata` is typed as `Record<string, unknown>`, TypeScript provides no protection if a future caller passes metadata that includes any of those keys — the explicit values would be silently overwritten.

The `message_sent` side avoids this by passing `hookMetadata` as a distinct nested field (`metadata: params.hookMetadata`). Consider the same pattern here to eliminate the footgun and make both hooks behave consistently:

```ts
metadata: {
  channel: params.channel,
  accountId: params.accountId,
  mediaUrls: params.payloadSummary.mediaUrls,
  hookMetadata: params.hookMetadata,
},
```

Or if the flat spread is intentional, placing the explicit fields **after** the spread ensures they can't be overridden:
```ts
metadata: {
  ...params.hookMetadata,
  channel: params.channel,
  accountId: params.accountId,
  mediaUrls: params.payloadSummary.mediaUrls,
},
```

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

---

This is a comment left during a code review.
Path: src/auto-reply/reply/route-reply.ts
Line: 173-181

Comment:
**Channel key mismatch silently disables dedupe for aliased channels**

`recentReplyRootKey` is built using `channelId` (the **normalized** result of `normalizeChannelId(channel)`), which lowercases and resolves aliases (e.g., `"google-chat"` → `"googlechat"`, `"imsg"` → `"imessage"`). The *check* side in `agent-runner.ts` and `followup-runner.ts` calls `buildRecentSentReplyRootKeyForRun(run)`, which uses the **raw** `run.originatingChannel` for that same field.

When `originatingChannel` contains an alias, the mark key and the check key contain different strings and will never match — reply-root deduplication silently does nothing for those channels.

The existing dedup code in `enqueue.ts` (`buildRecentMessageIdKey`) consistently uses raw `originatingChannel` without normalization to avoid exactly this class of mismatch. Consider passing `params.channel` (the raw value) instead of the normalized `channelId` when building `recentReplyRootKey`, so the mark and check sides stay in sync.

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

_{Last reviewed commit: 78264ed}

greptile-apps · 2026-03-16T05:33:26Z

+  const recentReplyRootKey = buildRecentSentReplyRootKey({
+    scopeKey: params.replyRootScopeKey ?? params.sessionKey,
+    agentId: params.replyRootAgentId ?? resolvedAgentId,
+    channel: channelId,
+    to,
+    accountId: accountId ?? undefined,
+    threadId: threadId ?? null,
+    replyRootId: params.replyRootId,
+  });


Channel key mismatch silently disables dedupe for aliased channels

recentReplyRootKey is built using channelId (the normalized result of normalizeChannelId(channel)), which lowercases and resolves aliases (e.g., "google-chat" → "googlechat", "imsg" → "imessage"). The check side in agent-runner.ts and followup-runner.ts calls buildRecentSentReplyRootKeyForRun(run), which uses the raw run.originatingChannel for that same field.

When originatingChannel contains an alias, the mark key and the check key contain different strings and will never match — reply-root deduplication silently does nothing for those channels.

The existing dedup code in enqueue.ts (buildRecentMessageIdKey) consistently uses raw originatingChannel without normalization to avoid exactly this class of mismatch. Consider passing params.channel (the raw value) instead of the normalized channelId when building recentReplyRootKey, so the mark and check sides stay in sync.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/auto-reply/reply/route-reply.ts Line: 173-181 Comment: **Channel key mismatch silently disables dedupe for aliased channels** `recentReplyRootKey` is built using `channelId` (the **normalized** result of `normalizeChannelId(channel)`), which lowercases and resolves aliases (e.g., `"google-chat"` → `"googlechat"`, `"imsg"` → `"imessage"`). The *check* side in `agent-runner.ts` and `followup-runner.ts` calls `buildRecentSentReplyRootKeyForRun(run)`, which uses the **raw** `run.originatingChannel` for that same field. When `originatingChannel` contains an alias, the mark key and the check key contain different strings and will never match — reply-root deduplication silently does nothing for those channels. The existing dedup code in `enqueue.ts` (`buildRecentMessageIdKey`) consistently uses raw `originatingChannel` without normalization to avoid exactly this class of mismatch. Consider passing `params.channel` (the raw value) instead of the normalized `channelId` when building `recentReplyRootKey`, so the mark and check sides stay in sync. How can I resolve this? If you propose a fix, please make it concise.

livingghost added 2 commits March 16, 2026 14:02

fix: dedupe followup replies by reply root

07db1c8

Track recent reply roots across queued and routed sends so the same inbound thread does not trigger duplicate agent replies. Surface the reply-root key into message hooks so plugin-side send guards can keep using the same identifier.

fix: narrow reply-root dedupe to queued reruns

78264ed

Avoid dropping fresh inbound messages that happen to share a reply root, prefer explicit reply targets over thread roots, and only mark a reply root as sent after an outbound delivery actually occurs.

openclaw-barnacle Bot added the size: L label Mar 16, 2026

livingghost closed this Mar 16, 2026

livingghost deleted the openclaw-pr3 branch March 16, 2026 05:31

chatgpt-codex-connector Bot reviewed Mar 16, 2026

View reviewed changes

greptile-apps Bot reviewed Mar 16, 2026

View reviewed changes

livingghost restored the openclaw-pr3 branch March 16, 2026 05:33

livingghost mentioned this pull request Mar 16, 2026

fix: narrow reply-root dedupe to queued reruns #47923

Closed

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: narrow reply-root dedupe to queued reruns#47920

fix: narrow reply-root dedupe to queued reruns#47920
livingghost wants to merge 2 commits intoopenclaw:mainfrom
livingghost:openclaw-pr3

livingghost commented Mar 16, 2026

Uh oh!

livingghost commented Mar 16, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 16, 2026

Uh oh!

greptile-apps Bot commented Mar 16, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

livingghost commented Mar 16, 2026

Summary

Test plan

Uh oh!

livingghost commented Mar 16, 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 16, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Greptile Summary

Confidence Score: 3/5

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Mar 16, 2026 •

edited

Loading