Skip to content

fix(messaging): persist getUpdates replay claims on both admission lanes - #41

Merged
NewFuture merged 5 commits into
NewFuture:mainfrom
ericcaiwx-star:fix/inbound-replay-dedupe
Aug 4, 2026
Merged

NewFuture merged 5 commits into
NewFuture:mainfrom
ericcaiwx-star:fix/inbound-replay-dedupe

Conversation

@ericcaiwx-star

Copy link
Copy Markdown
Collaborator

Summary

Author-led port of Tencent/openclaw-weixin#240 onto current community main, as invited on that PR.

  • Claim a stable inbound key (message_idclient_idseq → body fingerprint) via OpenClaw createClaimableDedupe inside ordinary and approval admission lanes, then commit a 24h plugin-state tombstone (pluginId: openclaw-weixin, account-scoped namespace) so at-least-once getUpdates replays do not run the AI pipeline twice — including across process restart.
  • Failures and abort release the claim for retry; intentional re-sends with a new message_id are not swallowed.
  • Stable MessageSid uses the same key when transport ids are present.
  • Tests: key selection, TTL, account isolation, restart persistence (SQLite), ordinary+approval duplicate drop, existing lane ordering still green.

Tracks #36

Notes for reviewers

  • Uses the supported OpenClaw persistent-dedupe plugin-state API (pluginId + stateMaxEntries), not a custom JSON path under openclaw-weixin/.
  • Lane chaining remains synchronous; claim/commit/release run as the first/last steps of each lane job.
  • Community distribution only — Tencent #240 stays open independently.

Test plan

  • npm run check:fast (Node 22.22.3)
  • Maintainer smoke: one WeChat text → single agent turn
  • Restart gateway and confirm same message_id replay is dropped
  • /approve plugin:… still bypasses an active ordinary turn; duplicate approval deliveries process once

Made with Cursor

Claim/commit OpenClaw createClaimableDedupe tombstones (pluginId
openclaw-weixin, account namespace, 24h) inside ordinary and approval
lanes so at-least-once iLink replays do not run twice across restart.
Stable MessageSid follows the same transport key when present.

Port of Tencent/openclaw-weixin#240 for the community distribution.

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds persistent, account-scoped replay protection for inbound Weixin updates.

Changes:

  • Adds stable dedupe keys and 24-hour tombstones.
  • Integrates dedupe with ordinary and approval lanes.
  • Adds tests and bilingual documentation.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/monitor/monitor.ts Integrates claims into admission lanes.
src/monitor/monitor.test.ts Tests duplicate suppression across lanes.
src/messaging/inbound-dedupe.ts Implements persistent replay dedupe.
src/messaging/inbound-dedupe.test.ts Tests keys, TTL, isolation, and persistence.
src/messaging/inbound.ts Derives stable message SIDs.
src/messaging/inbound.test.ts Tests stable SID generation.
docs/architecture.md Documents tombstone storage.
CHANGELOG.md Documents the fix in Chinese.
CHANGELOG_EN.md Documents the fix in English.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/messaging/inbound-dedupe.ts Outdated
Comment thread src/messaging/inbound-dedupe.ts Outdated
Comment thread src/messaging/inbound-dedupe.ts
Use the resolveFilePath createClaimableDedupe shape so min-host CI can
typecheck, and soften Windows temp-dir cleanup when the state DB stays open.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ericcaiwx-star

Copy link
Copy Markdown
Collaborator Author

Pushed a CI fix:

  1. OpenClaw 2026.6.1 — dropped pluginId (not in that host’s API); use resolveFilePath under openclaw-weixin/replay-dedupe/ which works on 2026.6.1 and newer hosts.
  2. Windows — persistence test no longer fails the suite if rmSync of the temp state dir hits EPERM while SQLite is still open.

Wait on ClaimableDedupe inflight pending so a replay is dropped only after
the owner commits; reclaim if the owner releases. Duplicate logs keep only
non-sensitive identity kinds (mid/cid/seq/body).

Co-authored-by: Cursor <cursoragent@cursor.com>
@ericcaiwx-star

Copy link
Copy Markdown
Collaborator Author

Addressed the remaining Copilot notes:

  1. In-flight vs duplicateadmitWeixinInboundMessage waits on pending; drops only after the owner commits; reclaims if the owner releases (avoids losing the message).
  2. Privacy — duplicate logs no longer include account/user ids or the full key; only identity=mid|cid|seq|body plus booleans.

(The earlier pluginId / min-host note was already fixed in the previous commit.)

@NewFuture NewFuture left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two blocking claim/lane lifecycle issues and one conditional fallback-key collision remain. Details and suggested regression tests are inline.

Comment thread src/monitor/monitor.ts Outdated
Comment thread src/monitor/monitor.ts Outdated
Comment thread src/messaging/inbound-dedupe.ts Outdated
Observe ClaimableDedupe inflight pending outside ordinary/approval lanes
and re-enqueue only after owner release; wrap all post-admission work in
the claim lifecycle; prefer item msg_id digests and never key by sender
alone for empty-body media.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ericcaiwx-star

Copy link
Copy Markdown
Collaborator Author

Addressed the three blocking review notes:

  1. Lane blockingtryClaim returns inflight without awaiting; the lane is released immediately and pending is observed out of band. On owner release we re-enqueue; on commit we log duplicate. Added regression: A still running + replay A + B ⇒ B starts before A finishes.
  2. Claim leak — everything after successful admission (including config lookup / null-safe item typing) is inside the claim try/catch; failures release. Added regression: throw before processOneMessage, same message_id can be reclaimed.
  3. Media fallback collision — prefer canonical item msg_id digest (:items:); empty-body media without transport/item ids returns null (no sender-only key). Added regression for two identifierless/distinct media messages.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/monitor/monitor.ts:163

  • Add a regression test for this release/re-enqueue branch: let an owner release the lane via onReplyAdmitted, queue an in-flight replay, then make the owner throw and assert that the replay re-enters admission and runs exactly once. The current tests cover an in-flight owner that commits and a later delivery after preprocessing failure, but not this out-of-band retry path, so a regression here could silently discard the waiting replay.
      // Owner released — re-enter admission so the message is not lost.
      scheduleInboundMessage(full);

src/messaging/inbound-dedupe.ts:53

  • This selects the legacy resolveFilePath/fileMaxEntries API, which contradicts the PR note claiming pluginId/stateMaxEntries plugin-state persistence and “not a custom JSON path.” On the minimum 2026.6.1 host this shape writes the stated JSON file; on newer hosts it is mapped as a legacy core-owned SQLite namespace rather than state owned by openclaw-weixin. Please align the implementation and PR description—if the legacy shape is required for minimum-host compatibility, the PR note should say so explicitly.
  return createClaimableDedupe({
    ...base,
    fileMaxEntries: WEIXIN_INBOUND_DEDUPE_FILE_MAX,
    resolveFilePath: resolveReplayDedupeFilePath,
    onDiskError: onReplayDedupeDiskError,

@NewFuture

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

Resolve Unreleased changelog conflicts by keeping the docs site Added
notes from main and the inbound replay-dedupe Fixed notes from this branch.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants