Skip to content

[codex] Dedupe messaging tool replies by route#75579

Merged
steipete merged 1 commit into
mainfrom
codex/suppress-message-tool-final-payload
May 1, 2026
Merged

[codex] Dedupe messaging tool replies by route#75579
steipete merged 1 commit into
mainfrom
codex/suppress-message-tool-final-payload

Conversation

@steipete

@steipete steipete commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • centralize the messaging-tool reply dedupe decision so normal replies and follow-up/media wake delivery use the same route-aware rule
  • suppress the final assistant payload only when a messaging tool already delivered to the originating conversation
  • preserve the requester-facing final reply when the tool sent text, image, video, or audio to another channel or target

Why

The old fix treated any successful messaging-tool send as enough to suppress the final payload. That prevents duplicate sends for same-route deliveries, but it is too broad for proactive or cross-target sends: posting media somewhere else should not erase the reply back to the requester.

Validation

  • pnpm install --frozen-lockfile
  • pnpm test src/auto-reply/reply/reply-payloads.test.ts src/auto-reply/reply/agent-runner-payloads.test.ts src/auto-reply/reply/followup-delivery.test.ts
  • pnpm exec oxfmt --check --threads=1 src/auto-reply/reply/reply-payloads-dedupe.ts src/auto-reply/reply/reply-payloads.ts src/auto-reply/reply/reply-payloads-dedupe.runtime.ts src/auto-reply/reply/agent-runner-payloads.ts src/auto-reply/reply/followup-delivery.ts src/auto-reply/reply/reply-payloads.test.ts src/auto-reply/reply/followup-delivery.test.ts
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS maintainer Maintainer-authored PR labels May 1, 2026
@clawsweeper

clawsweeper Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge.

What this changes:

The PR adds a shared route-aware messaging-tool payload dedupe decision and uses it from normal reply payload building and follow-up delivery, with tests for same-route suppression and cross-target text/media preservation.

Required change before merge:

A repair worker can safely add the missing one-line changelog entry without changing the TypeScript logic; maintainer review and required checks still gate merge afterward.

Security review:

Security review cleared: Security review cleared: the diff is limited to TypeScript reply-dedupe logic and colocated tests, with no CI, dependency, credential, install, permission, or package-publishing changes.

Review findings:

  • [P3] Add the required changelog entry — src/auto-reply/reply/reply-payloads-dedupe.ts:208
Review details

Best possible solution:

Land the route-aware dedupe helper after adding a single active changelog entry and waiting for required checks, keeping normal replies and follow-up/media wake delivery aligned on the same same-route suppression rule.

Do we have a high-confidence way to reproduce the issue?

Yes. On current main, call resolveFollowupDeliveryPayloads with a payload whose text or media matches sentTexts/sentMediaUrls while sentTargets points to another provider or target; the current function filters the content before route metadata can preserve the requester-facing reply.

Is this the best way to solve the issue?

Yes, with the changelog follow-up. Centralizing the decision into shouldDedupePayloads and suppressReplies is the narrow maintainable fix because both normal replies and follow-up delivery can share the same route-aware rule while preserving legacy content dedupe when target metadata is absent.

Full review comments:

  • [P3] Add the required changelog entry — src/auto-reply/reply/reply-payloads-dedupe.ts:208
    This changes user-visible messaging reply delivery: cross-target message-tool sends no longer suppress follow-up text/media replies. OpenClaw policy requires an active CHANGELOG.md entry for user-facing fixes before landing, but this PR does not update the changelog.
    Confidence: 0.82

Overall correctness: patch is correct
Overall confidence: 0.84

Acceptance criteria:

  • git diff --check
  • sed -n '1,80p' CHANGELOG.md

What I checked:

Likely related people:

  • steipete: Recent history for the central route and auto-reply payload surfaces includes route-helper publishing, route normalization, silent voice payload preservation, block-stream media dedupe, and the follow-up delivery seam, beyond this PR authorship. (role: recent maintainer and route/dedupe refactor owner; confidence: high; commits: e27c32b9b0d9, 387668263559, 28bf71d74b06; files: src/auto-reply/reply/reply-payloads-dedupe.ts, src/auto-reply/reply/agent-runner-payloads.ts, src/auto-reply/reply/followup-delivery.ts)
  • vincentkoc: Current local blame is rooted in a recent Vincent Koc commit, and GitHub history shows follow-up type/import and dedupe-helper split work in the same reply surface. (role: adjacent reply/follow-up maintainer; confidence: medium; commits: 7f58e89731, 159e6bc09931, 125e778fe618; files: src/auto-reply/reply/reply-payloads-dedupe.ts, src/auto-reply/reply/followup-delivery.ts, src/auto-reply/reply/reply-payloads.test.ts)
  • mcaxtr: Recent history for agent-runner-payloads.ts includes outbound media normalization and final media delivery work that overlaps the media-dedupe side of this PR. (role: adjacent media reply maintainer; confidence: medium; commits: 18c98316f7ac; files: src/auto-reply/reply/agent-runner-payloads.ts)

Remaining risk / open question:

  • The required active CHANGELOG.md entry for this user-facing fix is still missing.
  • Some GitHub checks for the exact head SHA were still in progress during review, so required checks should finish before merge.
  • The protected maintainer label means explicit maintainer handling is still required even after the mechanical changelog repair.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 7b3dfbf21407.

@steipete
steipete force-pushed the codex/suppress-message-tool-final-payload branch from 577c687 to 97dae1f Compare May 1, 2026 11:34
@openclaw-barnacle openclaw-barnacle Bot added size: S and removed agents Agent runtime and tooling size: XS labels May 1, 2026
@steipete steipete changed the title [codex] Suppress final payload after message tool send [codex] Dedupe messaging tool replies by route May 1, 2026
@steipete
steipete marked this pull request as ready for review May 1, 2026 11:41
@steipete
steipete merged commit 72f6016 into main May 1, 2026
160 of 161 checks passed
@steipete
steipete deleted the codex/suppress-message-tool-final-payload branch May 1, 2026 11:44
@steipete

steipete commented May 1, 2026

Copy link
Copy Markdown
Contributor Author

Landed via rebase onto main.

  • Gate: GitHub CI passed on 97dae1f1c13d8f6c211082bf03effa0981c0fbf9; targeted local proof before push: pnpm test src/auto-reply/reply/reply-payloads.test.ts src/auto-reply/reply/agent-runner-payloads.test.ts src/auto-reply/reply/followup-delivery.test.ts, pnpm exec oxfmt --check --threads=1 ..., git diff --check
  • PR head: 97dae1f1c13d8f6c211082bf03effa0981c0fbf9
  • Land commit: 72f6016ce5964a39e61ac55d213879e798b1dfe1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant