fix(reply): dedupe duplicate non-streaming final replies#100828
Conversation
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs changes before merge. Reviewed July 6, 2026, 7:16 AM ET / 11:16 UTC. Summary PR surface: Source +45, Tests +104, Docs +1. Total +150 across 3 files. Reproducibility: yes. at source level: current main sends every replyResult array entry through final delivery, and the linked issue includes duplicate media-delivery logs from an affected channel. I did not run a live affected-channel reproduction in this read-only review. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Remove the release-owned changelog line, keep the shared dispatcher-level dedupe with identity-preservation tests, and have a maintainer accept whether source-level proof is enough before merge. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main sends every replyResult array entry through final delivery, and the linked issue includes duplicate media-delivery logs from an affected channel. I did not run a live affected-channel reproduction in this read-only review. Is this the best way to solve the issue? Likely yes: the shared dispatch loop is the maintainable choke point because buildReplyPayloads only dedupes message-tool/direct-block evidence and custom resolvers still flow through dispatchReplyFromConfig. The PR should drop the release-owned changelog edit before merge. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 827402243d7f. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +45, Tests +104, Docs +1. Total +150 across 3 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
f4a33b5 to
d347f57
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Merged via squash.
|
…0828) * fix(reply): dedupe duplicate non-streaming final replies * fix(reply): preserve distinct final delivery identities * chore(reply): drop release-owned changelog entry * chore(reply): drop release-owned changelog entry --------- Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com>
…0828) * fix(reply): dedupe duplicate non-streaming final replies * fix(reply): preserve distinct final delivery identities * chore(reply): drop release-owned changelog entry * chore(reply): drop release-owned changelog entry --------- Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com>
Fixes #84623.
What Problem This Solves
Non-streaming
dispatchReplyFromConfigcould deliver byte-identical final payload entries twice for one turn.Why This Change Was Made
The shared final-dispatch loop now keeps a per-turn key over delivery-visible payload fields and routing identity. Exact duplicate finals are suppressed, while same-content replies remain deliverable when they come from distinct assistant messages, route to different accounts, or carry different explicit reply-threading identity.
User Impact
Channels using non-streaming final replies no longer send duplicate identical text or media payloads for one inbound turn, without collapsing intentional repeated replies.
Evidence
pnpm check:changedpassed in the guarded validation lane.a5f6c13efb61e60f4ad454bc8cebc3d6a1798190.