refactor: consolidate duplicated plugin state and doctor migration plumbing onto SDK seams by steipete · Pull Request #99850 · openclaw/openclaw

steipete · 2026-07-04T06:29:07Z

What Problem This Solves

Resolves the largest verbatim code-duplication clusters in the bundled plugins, where channel and doctor plumbing was copy-pasted per plugin instead of flowing through plugin SDK family seams:

Slack (x2), Mattermost, and MS Teams each hand-rolled the same dual-layer presence cache (global in-memory dedupe + best-effort persistent keyed store with disable-on-error). Slack and Mattermost were ~95% identical whole files and the top jscpd hit in the repo (40-line verbatim clone).
Matrix reimplemented the approval-reaction target store that openclaw/plugin-sdk/approval-reaction-runtime already provides and imessage/signal/whatsapp already use. The local copy had drifted: its in-memory map was unbounded and never expired entries.
fileExists was copied verbatim into 10 extension doctor-contract-api.ts files and archiveLegacySource into 9, with per-plugin label prefixes baked into otherwise identical doctor message templates.
extensions/memory-core/src/memory/qmd-runtime-cache.ts validated the same cache-entry envelope twice in two 50-line normalizers.

Every copy was a drift bug waiting to happen; extensions/AGENTS.md explicitly calls this out ("stop copying the logic. Extract one shared helper and migrate both call sites in the same change").

Why This Change Was Made

One canonical owner per behavior, per the repo architecture rules:

New createPersistentDedupeCache family helper in openclaw/plugin-sdk/dedupe-runtime (memory-first presence checks, best-effort persistence that disables itself on the first state failure so message handling never breaks, re-prime on persistent hits). The four presence-cache files shrink to thin key-shape wrappers; persisted record shapes and namespaces are unchanged, so no state migration is needed.
Matrix now builds on createApprovalReactionTargetStore like its sibling channels, keeping only the matrix-specific emoji bindings local (checkmark/cross render reliably across Matrix clients). The persisted {version: 1, target} wire format is unchanged and asserted by existing tests. The test-only sync resolver export was dropped; production callers always used the persistence-aware resolver.
legacyStateFileExists / archiveLegacyStateSource are now exported from openclaw/plugin-sdk/runtime-doctor (implementation in src/plugins/doctor-state-migration-fs.ts, next to the doctor contract registry) and all ten doctor contracts consume them. Doctor change/warning strings are preserved via the label parameter (asserted by the existing doctor-contract tests).

Reviewed and intentionally left as-is, with reasons: the msteams-local estimateBase64DecodedBytes copy (documented hot-path load-cost tradeoff vs the broad media-runtime barrel), the discord components-registry store wrapper (linked two-store disable + richer error formatting = different contract), and the core-side OpenAI OAuth TLS preflight twin (src/plugins/provider-openai-chatgpt-oauth-tls.ts), which needs a doctor-contract ownership decision tracked in #99841.

User Impact

No user-visible behavior change. Channel presence caches, approval reactions, and doctor migrations behave as before; two internal hardening side effects: the Matrix approval-reaction memory layer is now bounded/TTL-pruned like its siblings, and the MS Teams sent-message memory layer moves from unbounded per-conversation maps to one globally bounded dedupe cache (20k entries). Net -539 lines of production/test code.

Evidence

git diff --numstat vs base: 380 added / 919 deleted (net -539).
Verbatim-clone proof: targeted jscpd re-scan (min 15 lines / 120 tokens) over all touched packages reports 0 clones involving the refactored files (was: 40L slack<->mattermost, 32L qmd self-clone, 24L active-memory<->nostr, 18L matrix cluster).
Repo gates: node scripts/check-duplicates.mjs (production/tests/src-mixed all 0 clones), check-architecture-smells.mjs, check-database-first-legacy-stores.mjs, check-no-extension-src-imports.ts, check-extension-plugin-sdk-boundary.mjs, check-plugin-extension-import-boundary.mjs, check-src-extension-import-boundary.mjs, check-import-cycles.ts - all clean.
Types: all four tsgo lanes clean (core, extensions, core-test, extensions-test).
Build: node scripts/build-all.mjs green, including the check-plugin-sdk-exports gate.
Tests (node scripts/run-vitest.mjs): new src/plugin-sdk/dedupe-runtime.test.ts (6 tests: layering, re-prime timestamps, disable-on-error, test reset); existing suites for all migrated files pass unchanged except for three matrix test files updated to the async resolver and one memory-wiki doctor wording assertion ("legacy record" -> canonical "legacy source"): slack sent-thread/inbound-delivery + send/upload/blocks/message-handler + dispatch.streaming (13+50+26), mattermost thread-participation + monitor-gating (5 incl. persisted-shape assertions, +17 shard), msteams sent-message-cache + thread-parent/thread-session handlers (4+7+17 shard), matrix approval-reactions/handler/reaction-events (14+7), all 10 doctor-contract suites, memory-core qmd-runtime-cache (10), plugin-sdk subpath contracts (18) and api-baseline (3).
Autoreview (Codex, gpt-5.5, full bundle): "patch is correct (0.82) - No actionable defects found. The shared persistent dedupe helper preserves the existing memory-first behavior, handles persistence failures as best-effort, and the migrated call sites keep their prior TTL and fallback semantics."

clawsweeper · 2026-07-04T06:32:24Z

Codex review: needs real behavior proof before merge. Reviewed July 4, 2026, 4:49 AM ET / 08:49 UTC.

Summary
The branch adds shared Plugin SDK helpers for persistent dedupe caches and doctor legacy-state filesystem migration, migrates bundled plugin/channel/doctor callers, and updates focused docs, tests, and SDK surface budgets.

Reproducibility: not applicable. this is a refactor PR for source-level duplication rather than a runtime bug report. Source review and the linked issue establish the duplicated cache/doctor plumbing being cleaned up.

Review metrics: 2 noteworthy metrics.

Plugin SDK public surface: 4 exports added, 0 removed. The dedupe-runtime and runtime-doctor additions are public plugin-facing API and must stay aligned with docs, exports, and API baseline checks.
Migration breadth: 4 presence caches and 10 doctor contracts migrated. The refactor centralizes behavior across multiple channel and doctor paths, so parity proof matters before merge.

Stored data model
Persistent data-model change detected: migration/backfill/repair: .agents/skills/openclaw-changelog-update/scripts/verify-release-notes.mjs, migration/backfill/repair: src/plugin-sdk/runtime-doctor.ts, migration/backfill/repair: src/plugins/doctor-state-migration-fs.ts, persistent cache schema: src/infra/session-cost-usage.test.ts, persistent cache schema: src/plugin-sdk/dedupe-runtime.test.ts, serialized state: src/infra/session-cost-usage.test.ts, and 5 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99841
Summary: This PR is the implementation candidate for the open cleanup issue that describes the same bundled-plugin persistent-state and doctor migration SDK seam duplication.

Members:

canonical: Bundled plugins duplicate persistent-state and doctor migration plumbing instead of SDK family seams #99841 - The PR body uses closing syntax for this issue and implements the three cleanup bullets described there.

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: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

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

Rank-up moves:

Regenerate and commit the Plugin SDK API baseline hash for the new exports.
Attach redacted real behavior proof for one migrated doctor or channel path, such as terminal output or logs from a representative run.
[P1] Keep the public SDK helper shape on the maintainer review path before merge.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body lists tests/build/static checks, but I did not find after-fix real setup proof such as a redacted terminal transcript, logs, or linked artifact for a migrated doctor or channel path. 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.

Risk before merge

[P1] The branch adds public Plugin SDK exports, so compatibility, docs, export checks, and baseline artifacts need maintainer-visible alignment before merge.
[P1] The shared cache and Matrix approval-reaction refactor changes message and approval delivery state paths; a parity miss could suppress, duplicate, or misroute follow-up replies or approval decisions.
[P1] The PR body lists tests/build/static checks, but no after-fix real setup transcript, logs, or artifact proves a migrated doctor or channel path running outside unit coverage.
[P1] The tracked Plugin SDK API baseline hash is still unchanged, so the API check can report drift for the intended new exports.

Maintainer options:

Regenerate Baseline And Add Proof (recommended)
Update the Plugin SDK API baseline hash and attach redacted real behavior proof for one migrated doctor or channel path before merge.
Approve The SDK Shape Explicitly
Maintainers can accept the additive dedupe-runtime and runtime-doctor helpers after confirming they are the right plugin-facing seam.
Pause If The Seam Is Not Settled
If these helpers should stay internal or be shaped differently, pause this PR and keep the linked issue as the design owner.

Next step before merge

[P1] Manual review and contributor follow-up are required because automation can regenerate a baseline but cannot supply the contributor's real behavior proof or approve the public SDK seam.

Security
Cleared: The actual current-main merge diff does not change dependencies, lockfiles, workflow permissions, secrets handling, downloaded artifacts, or package execution paths.

Review findings

[P1] Regenerate the SDK API baseline — src/plugin-sdk/dedupe-runtime.ts:31

Review details

Best possible solution:

Land the shared SDK seam refactor only after regenerating the API baseline, preserving docs/export contracts, adding real behavior proof for a representative migrated path, and getting maintainer sign-off on the public SDK shape.

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

Not applicable: this is a refactor PR for source-level duplication rather than a runtime bug report. Source review and the linked issue establish the duplicated cache/doctor plumbing being cleaned up.

Is this the best way to solve the issue?

No as-is: the shared-helper direction is likely the right owner-boundary solution, but the missing API baseline update, missing real behavior proof, and public SDK seam decision make this head not merge-ready.

Full review comments:

[P1] Regenerate the SDK API baseline — src/plugin-sdk/dedupe-runtime.ts:31
This remains from the prior review: the patch adds public SDK exports in openclaw/plugin-sdk/dedupe-runtime and openclaw/plugin-sdk/runtime-doctor, and even raises the surface budgets, but it still does not update docs/.generated/plugin-sdk-api-baseline.sha256. The baseline check compares the rendered public exports with that tracked hash, so pnpm plugin-sdk:api:check can continue to report drift until the baseline is regenerated for these intentional exports.
Confidence: 0.92

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 3d6a2216ea77.

Label changes

Label changes:

add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

P3: This is low-priority maintainability and SDK-boundary cleanup, not an urgent user-facing outage.
merge-risk: 🚨 compatibility: The PR adds public Plugin SDK exports and changes doctor helper ownership, which can affect plugin authors and upgrade contracts.
merge-risk: 🚨 message-delivery: The PR moves channel presence caches and Matrix approval reaction target storage through shared helpers used on message and approval delivery paths.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists tests/build/static checks, but I did not find after-fix real setup proof such as a redacted terminal transcript, logs, or linked artifact for a migrated doctor or channel path. 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.

Evidence reviewed

What I checked:

Live PR state: Live metadata shows this PR is open, authored by a contributor account, mergeable, and labeled maintainer, P3, needs proof, compatibility risk, and message-delivery risk. (08102f2e5830)
Actual current-main merge surface: GitHub's potential merge commit against current main changes 27 central SDK/plugin-doctor/channel files, so the stale 148-file branch surface is not treated as a deletion/regression finding by itself. (88b5a1af1068)
Public dedupe SDK export added: The PR adds createPersistentDedupeCache and PersistentDedupeCache to the public dedupe-runtime SDK subpath. (src/plugin-sdk/dedupe-runtime.ts:31, 08102f2e5830)
Runtime doctor SDK exports added: The PR adds archiveLegacyStateSource and legacyStateFileExists to the public runtime-doctor SDK subpath. (src/plugin-sdk/runtime-doctor.ts:32, 08102f2e5830)
API baseline hash still unchanged: The tracked plugin SDK API baseline hash is identical at the prior reviewed head and current head, and the potential merge diff does not include docs/.generated/plugin-sdk-api-baseline.sha256. Public docs: docs/.generated/plugin-sdk-api-baseline.sha256. (docs/.generated/plugin-sdk-api-baseline.sha256, 08102f2e5830)
Baseline check contract: The API baseline writer computes the rendered JSON/JSONL hash and reports drift in check mode when the tracked hash content differs. (src/plugin-sdk/api-baseline.ts:591, 3d6a2216ea77)

Likely related people:

amknight: History for Slack, Mattermost, MS Teams, and Matrix persistent-state files shows the central duplicated cache/store behavior dates to commits authored by Alex Knight. (role: introduced adjacent persistent-state behavior; confidence: high; commits: d0ec3d1f09b0, 13ecb5c55eb6, 6ae09d029c38; files: extensions/slack/src/sent-thread-cache.ts, extensions/mattermost/src/mattermost/thread-participation.ts, extensions/msteams/src/sent-message-cache.ts)
steipete: Path history shows repeated recent work on Plugin SDK boundaries, runtime-doctor exports, SQLite/plugin-state migrations, and the PR/linked issue are also scoped by this account. (role: recent SDK and doctor/state refactor owner; confidence: high; commits: f0000ab72d01, b34b03dd9eb8, 05c3325b0acc; files: src/plugin-sdk/dedupe-runtime.ts, src/plugin-sdk/runtime-doctor.ts, extensions/acpx/doctor-contract-api.ts)
vincentkoc: History on approval-reaction-runtime includes approval helper dedupe and reaction prompt fixes near the Matrix approval-reaction seam reused here. (role: adjacent approval SDK contributor; confidence: medium; commits: b3fbe5325e0b, b261e9e6ddaa; files: src/plugin-sdk/approval-reaction-runtime.ts)
Ayaan Zaidi: History on runtime-doctor includes exposing persistent dedupe import helpers adjacent to the requested shared doctor/persistence helper surface. (role: adjacent runtime-doctor contributor; confidence: medium; commits: b2dec9cd5046; files: src/plugin-sdk/runtime-doctor.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.

Review history (2 earlier review cycles)

reviewed 2026-07-04T07:16:27.700Z sha f404107 :: needs real behavior proof before merge. :: [P1] Regenerate the SDK API baseline
reviewed 2026-07-04T08:42:14.926Z sha 08102f2 :: needs real behavior proof before merge. :: [P1] Regenerate the SDK API baseline

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2d9c1b5a6

ℹ️ 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-07-04T06:35:15Z

+ * open/read/write failure disables the persistent layer for the process so message
+ * handling never breaks on state errors, matching the shipped channel-cache contract.
+ */
+export function createPersistentDedupeCache<TRecord>(params: {


Regenerate the SDK API baseline

Adding this public openclaw/plugin-sdk/dedupe-runtime export (and the runtime-doctor exports in this commit) changes the Plugin SDK API surface, but docs/.generated/plugin-sdk-api-baseline.sha256 is still unchanged from the parent. The baseline checker in scripts/generate-plugin-sdk-api-baseline.ts compares the rendered public exports to that tracked hash and exits with drift, so pnpm plugin-sdk:api:check will fail until the hash is regenerated with the intentional API changes.

Useful? React with 👍 / 👎.

…nnel presence caches

…elpers

steipete · 2026-07-04T08:51:07Z

Merged via squash.

Prepared head SHA: 08102f2e58300c433400c62757d145c9589d77c2
Landed commit: eafe2a8d0bb6a67fe43784bc5d3f4a73b33dc06e

…umbing onto SDK seams (openclaw#99850) * refactor(plugin-sdk): add createPersistentDedupeCache and migrate channel presence caches * refactor(matrix): adopt SDK approval reaction target store * refactor(plugin-sdk): share doctor legacy-state migration fs helpers * refactor(memory-core): dedupe qmd cache entry envelope validation * chore(plugin-sdk): pin surface budgets for shared dedupe and doctor helpers * test(matrix): use future approval expiry fixtures for reaction targets * test(matrix): use future approval expiry fixtures for reaction targets

chatgpt-codex-connector Bot reviewed Jul 4, 2026

View reviewed changes

steipete added 5 commits July 4, 2026 08:07

refactor(plugin-sdk): add createPersistentDedupeCache and migrate cha…

a0c72c7

…nnel presence caches

refactor(matrix): adopt SDK approval reaction target store

336f80c

refactor(plugin-sdk): share doctor legacy-state migration fs helpers

8156bb5

refactor(memory-core): dedupe qmd cache entry envelope validation

011b9ed

chore(plugin-sdk): pin surface budgets for shared dedupe and doctor h…

f404107

…elpers

steipete force-pushed the claude/zen-cray-add5e0 branch from d2d9c1b to f404107 Compare July 4, 2026 07:10

openclaw-barnacle Bot added the scripts Repository scripts label Jul 4, 2026

clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 4, 2026

steipete added 2 commits July 4, 2026 08:38

test(matrix): use future approval expiry fixtures for reaction targets

f1d30e1

test(matrix): use future approval expiry fixtures for reaction targets

08102f2

steipete requested a review from a team as a code owner July 4, 2026 08:35

steipete merged commit eafe2a8 into main Jul 4, 2026
210 of 217 checks passed

steipete deleted the claude/zen-cray-add5e0 branch July 4, 2026 08:51

steipete mentioned this pull request Jul 4, 2026

refactor: consolidate core stream cleanup, watchdog locality, and approval text duplication #99901

Merged

github-actions Bot mentioned this pull request Jul 4, 2026

📡 Upstream Digest — 2026-07-04 10:01 UTC curtismercier/openclaw-mods#1203

Open

steipete mentioned this pull request Jul 4, 2026

refactor: consolidate markdown code fences, error coercion, and byte-identical helper pairs #99932

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor: consolidate duplicated plugin state and doctor migration plumbing onto SDK seams#99850

refactor: consolidate duplicated plugin state and doctor migration plumbing onto SDK seams#99850
steipete merged 7 commits into
mainfrom
claude/zen-cray-add5e0

steipete commented Jul 4, 2026

Uh oh!

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 4, 2026

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

steipete commented Jul 4, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading