Skip to content

fix(reply): filter short trailing acks after message-tool sends#96763

Closed
zw-xysk wants to merge 1 commit into
openclaw:mainfrom
zw-xysk:fix/issue-96681-message-tool-ack-dedup
Closed

fix(reply): filter short trailing acks after message-tool sends#96763
zw-xysk wants to merge 1 commit into
openclaw:mainfrom
zw-xysk:fix/issue-96681-message-tool-ack-dedup

Conversation

@zw-xysk

@zw-xysk zw-xysk commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #96681

What Problem This Solves

After an agent uses the message tool (action=send) to deliver its main reply, models often append a short trailing ack — 已发, OK, Done, Sent, Got it, etc. These carry no information beyond the already-sent message but appear as a second visible message in the channel.

The existing filterMessagingToolDuplicates uses a 10-char minimum, so short acks escape dedup.

Evidence

Fix

  • messaging-dedupe.ts: added isMessagingToolAck() — pattern-matches short trailing acks (Chinese + English) when a message-tool send has occurred
  • reply-payloads-dedupe.ts: wired ack check alongside existing duplicate check

Test results

✅ "已发" => true
✅ "已发 #22141" => true
✅ "核心回答如下" => true
✅ "OK" => true
✅ "Done" => true
✅ "Sent #123" => true
✅ "Got it" => true
✅ "Thanks" => true
✅ "Hello full reply" => false
✅ "" => false
12/12 passed

What was not tested

End-to-end with real Telegram/Discord channel. Logic is text-only, media payloads preserved.

Changes

3 files, +30/-2. Lockfile unchanged.

@zw-xysk @claude
fix(reply): filter short trailing acks after message-tool sends
eeed816 
After an agent delivers its main reply via the message tool, models often
append a short meta-ack (已发, OK, Done, Sent, etc.) that becomes a second
visible message. Add isMessagingToolAck to detect these by pattern.

Fixes openclaw#96681

Co-Authored-By: Claude (via Anthropic API) <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Jun 25, 2026
@clawsweeper

clawsweeper Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 7:54 AM ET / 11:54 UTC.

Summary
The branch adds an exported isMessagingToolAck classifier and applies it in filterMessagingToolDuplicates to suppress short text acknowledgements after message-tool sends.

PR surface: Source +28. Total +28 across 3 files.

Reproducibility: yes. at source level: current main has a 10-character duplicate floor and final/follow-up paths have no meta-ack filter, so short acknowledgements can pass. I did not establish a live channel reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Committed Ack Coverage: 0 tests added. The diff changes outbound message suppression, so reviewers need committed positive and false-positive coverage before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96681
Summary: The linked issue is the canonical report for short post-message-tool acknowledgement duplicate replies; this PR is one candidate fix, and another open candidate exists but is not proof-positive enough to supersede it automatically.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Replace the broad regex with whole-payload acknowledgement matching and add committed positive and false-positive tests.
  • [P1] Add redacted after-fix real channel proof, such as a transcript screenshot, terminal output, logs, recording, or linked artifact.
  • [P1] Decide whether this PR or fix(reply): suppress post-tool-send meta commentary as duplicate reply #96680 should be the canonical fix path.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The only proof is an inline text-only matcher table, and the PR body explicitly says real Telegram/Discord channel behavior was not tested. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A native Telegram proof would directly show that a post-message-tool acknowledgement is suppressed without hiding legitimate short replies. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify that a message-tool main reply followed by a short ack shows one Telegram message, and that legitimate short replies still deliver.

Risk before merge

  • [P1] The broad ack matcher can drop legitimate final or follow-up messages that end with ack-like words after a message-tool send.
  • [P1] Several reported acknowledgement forms from the linked issue remain unmatched while the PR uses closing syntax.
  • [P1] The contributor has not attached inspectable after-fix Telegram, Discord, Signal, terminal, log, or artifact proof.

Maintainer options:

  1. Repair the Ack Classifier (recommended)
    Anchor the classifier to whole acknowledgement payloads, add positive and negative tests for final and follow-up delivery, and keep media payload preservation intact before merge.
  2. Converge on the Existing Candidate
    If maintainers prefer the broader tested approach in fix(reply): suppress post-tool-send meta commentary as duplicate reply #96680, pause or close this PR only after that candidate has proof and is safe to treat as canonical.
  3. Accept a Narrower Scope
    Maintainers could merge a standalone-ack-only mitigation only after narrowing the closing scope so unfixed compound examples remain tracked.

Next step before merge

  • [P1] Needs contributor real behavior proof and a maintainer/contributor repair of the message-suppression classifier before normal merge review can continue; this is not a safe autonomous repair while the external proof gate remains unmet.

Security
Cleared: No security or supply-chain concern was found; the diff only changes internal TypeScript reply filtering.

Review findings

  • [P1] Anchor ack matching before suppressing final text — src/agents/embedded-agent-helpers/messaging-dedupe.ts:12-14
  • [P2] Cover the linked ack forms before closing the report — src/agents/embedded-agent-helpers/messaging-dedupe.ts:12
Review details

Best possible solution:

Use a route-scoped whole-text meta-ack filter with focused positive and false-positive tests in both final and follow-up paths, then add redacted real channel proof before closing the linked issue.

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

Yes at source level: current main has a 10-character duplicate floor and final/follow-up paths have no meta-ack filter, so short acknowledgements can pass. I did not establish a live channel reproduction in this read-only review.

Is this the best way to solve the issue?

No: the route-scoped dedupe seam is appropriate, but this implementation is not the best fix because the classifier is both overbroad and undercovered. A safer solution needs whole-text matching, false-positive tests, sibling-path coverage, and real channel proof.

Full review comments:

  • [P1] Anchor ack matching before suppressing final text — src/agents/embedded-agent-helpers/messaging-dedupe.ts:12-14
    isMessagingToolAck runs for every non-media reply payload once any message-tool send exists. With this pattern, normal visible text such as The migration is done or Everything looks ok matches the English suffix regex, and the Chinese regex matches its phrases anywhere in a short payload, so this can drop legitimate final or follow-up messages. Please require the whole payload to be an acknowledgement and add negative cases before filtering it out.
    Confidence: 0.9
  • [P2] Cover the linked ack forms before closing the report — src/agents/embedded-agent-helpers/messaging-dedupe.ts:12
    The linked report includes examples such as Sent above, Sent. Replied in thread., and OK, that's the fix., but the English matcher only permits the ack word, an optional period, and an optional numeric ref. Those examples would still be delivered as second messages while this PR closes the issue, so either narrow the closing scope or add a safe matcher and tests for the reported forms.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4ecb45bf7729.

Label changes

Label changes:

  • add status: 🔁 re-review loop: A fresh ClawSweeper review was explicitly requested after the latest review. Needs real behavior proof before merge: The only proof is an inline text-only matcher table, and the PR body explicitly says real Telegram/Discord channel behavior was not tested. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • remove status: 📣 needs proof: Current PR status label is status: 🔁 re-review loop.

Label justifications:

  • P2: This is a normal-priority visible duplicate-message bug fix with limited blast radius but real channel-delivery impact.
  • merge-risk: 🚨 message-delivery: The PR filters outbound text payloads after message-tool sends, so an incorrect classifier can suppress legitimate visible messages.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 🔁 re-review loop: A fresh ClawSweeper review was explicitly requested after the latest review. Needs real behavior proof before merge: The only proof is an inline text-only matcher table, and the PR body explicitly says real Telegram/Discord channel behavior was not tested. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram duplicate-message behavior that can be demonstrated in a short Telegram Desktop recording.
Evidence reviewed

PR surface:

Source +28. Total +28 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 3 30 2 +28
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 30 2 +28

What I checked:

Likely related people:

  • steipete: Authored and merged the route-aware message-tool reply dedupe PR that established the same final/follow-up reply-payload decision surface. (role: feature history owner; confidence: high; commits: 97dae1f1c13d; files: src/auto-reply/reply/reply-payloads-dedupe.ts, src/auto-reply/reply/agent-runner-payloads.ts, src/auto-reply/reply/followup-delivery.ts)
  • sandieman2: Recently changed the same reply-payload and follow-up delivery surfaces while fixing routed-thread dedupe and queued follow-up delivery behavior. (role: recent area contributor; confidence: high; commits: c67dc59b02b0, f99d95539a4c; files: src/auto-reply/reply/reply-payloads-dedupe.ts, src/auto-reply/reply/agent-runner-payloads.ts, src/auto-reply/reply/followup-delivery.ts)
  • jalehman: Local blame for the current central dedupe lines points at the checked-out grafted snapshot commit, which is weaker than merged feature history but still relevant to current source provenance. (role: current snapshot contributor; confidence: low; commits: 0cdb050bac19; files: src/agents/embedded-agent-helpers/messaging-dedupe.ts, src/auto-reply/reply/reply-payloads-dedupe.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 25, 2026
@zw-xysk

zw-xysk commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

All core CI green (lint, test, compile). @clawsweeper re-review

@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Closed
@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 27, 2026
@mushuiyu886 mushuiyu886 mentioned this pull request Jun 28, 2026
Closed
@zw-xysk zw-xysk closed this Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Duplicate reply: message-tool sends followed by short trailing acks ("已发 #xxx", "Sent above", "核心回答如下", "OK")

1 participant

@zw-xysk