Skip to content

refactor(whatsapp): centralize inbound turn admission and history finalization#107070

Open
mcaxtr wants to merge 8 commits into
mainfrom
refactor/whatsapp-prepared-turn-kernel
Open

refactor(whatsapp): centralize inbound turn admission and history finalization#107070
mcaxtr wants to merge 8 commits into
mainfrom
refactor/whatsapp-prepared-turn-kernel

Conversation

@mcaxtr

@mcaxtr mcaxtr commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What Problem This Solves

WhatsApp duplicated two responsibilities already owned by the shared channel turn machinery: translating an SDK ingress decision into turn admission, and clearing pending group history after reply dispatch.

Keeping those decisions in the plugin made the WhatsApp receive path easier to drift from the core turn lifecycle.

What Changed

  • Derive the canonical turn admission from the resolved SDK ingress decision at the WhatsApp access-control boundary.
  • Preserve the shipped, redacted senderAccess, commandAccess, and activationAccess callback projections while adding turnAdmission. The complete resolved ingress object and its effective allowlists remain private.
  • Export the decision-only admission mapper through channel-ingress-runtime, the modern receive-path SDK owner.
  • Carry admission and group-history finalization options as data on the current ChannelTurnPlan returned through runChannelInboundEvent.
  • Let the core turn executor finalize pending group history after successful dispatch, and remove WhatsApp's duplicate clearing branches from the reply finalizer.
  • Construct the deprecated flat-message admission directly instead of fabricating an SDK ingress graph for a compatibility path that predates it.

Broadcast behavior is unchanged: each agent receives the same pending-history snapshot, per-agent core finalization is suppressed, and the existing broadcast barrier clears the shared history once after fan-out finishes.

Behavior and Compatibility

Reachable behavior remains unchanged:

  • Successful dispatches clear pending group history for both silent and visible replies.
  • Observe-only turns suppress transport delivery and retain pending group history.
  • Failed dispatches retain pending group history.
  • Existing callback consumers continue to receive the shipped redacted access projections.

Evidence

  • Focused Plugin SDK mapper and WhatsApp admission, dispatch, process-message, and broadcast tests
  • Plugin SDK API-baseline and surface checks
  • WhatsApp extension typecheck
  • Production build, including Plugin SDK declaration and export verification

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: whatsapp-web Channel integration: whatsapp-web size: L maintainer Maintainer-authored PR labels Jul 14, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 14, 2026
@clawsweeper

clawsweeper Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 12:17 PM ET / 16:17 UTC.

Summary
The branch moves WhatsApp inbound admission and post-dispatch group-history finalization into shared channel-turn planning and exports an ingress-decision-to-turn-admission mapper through the Plugin SDK.

PR surface: Source +49, Tests +108, Docs +2, Generated 0, Other +2. Total +161 across 22 files.

Reproducibility: not applicable. This PR is a refactor and API-boundary proposal, not a report of a currently reproducible user failure.

Review metrics: 1 noteworthy metric.

  • Plugin SDK exports: 1 public mapper added. This refactor adds a new public SDK contract rather than only moving internal WhatsApp code.

Stored data model
Persistent data-model change detected: serialized state: extensions/whatsapp/src/inbound/access-control.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • Obtain explicit maintainer confirmation that the new Plugin SDK mapper is the intended durable public contract.

Risk before merge

  • [P1] Merging creates a new public Plugin SDK function, so its long-term owner, stability expectation, and whether equivalent channel-specific mappings should use this API need explicit maintainer acceptance.
  • [P1] Moving group-history clearing into core-owned turn finalization is behavior-preserving by intent but remains delivery-sensitive across silent replies, failed dispatches, observe-only turns, and broadcast fan-out.

Maintainer options:

  1. Approve the SDK boundary before merge (recommended)
    Confirm that the new public mapper is the intended shared receive-path contract, then merge with the existing redaction and lifecycle tests intact.
  2. Keep admission mapping plugin-local
    Pause this PR and remove the new SDK export if maintainers do not want to support this channel-turn mapping as public API.

Next step before merge

  • [P2] A human must choose the durable Plugin SDK API boundary; no narrow mechanical repair is indicated by the current branch.

Maintainer decision needed

  • Question: Should mapChannelIngressDecisionToTurnAdmission become a supported public plugin-sdk/channel-ingress-runtime primitive for migrated channel receive paths?
  • Rationale: The implementation is a clean refactor, but exporting this mapper establishes a shared plugin-facing contract beyond WhatsApp; code review cannot safely choose that permanent API boundary without maintainer intent.
  • Likely owner: vincentkoc — The PR timeline explicitly requested this participant, making them the best available routing candidate for the SDK-boundary decision from the supplied evidence.
  • Options:
    • Adopt the shared mapper (recommended): Accept the mapper as the canonical SDK bridge from resolved ingress decisions to channel turn plans, with the current redacted callback envelope retained.
    • Keep it WhatsApp-local: Avoid widening the Plugin SDK and revise the refactor so WhatsApp owns its decision-to-turn-admission translation until a broader channel API is designed.

Security
Cleared: The reviewed patch does not add dependencies, workflow execution, secret handling, or an observable permission boundary; the callback test specifically protects allowlist redaction.

Review details

Best possible solution:

Approve one documented Plugin SDK ownership boundary for decision-to-turn-admission mapping, then land this refactor only with the existing WhatsApp lifecycle invariants preserved by focused regression coverage.

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

Not applicable: this PR is a refactor and API-boundary proposal, not a report of a currently reproducible user failure.

Is this the best way to solve the issue?

Unclear: the lifecycle consolidation is a maintainable direction, but only a maintainer can decide whether the new public Plugin SDK mapper is the best permanent contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 359859d343bd.

Label changes

Label justifications:

  • P2: This is a bounded channel/SDK refactor with meaningful compatibility and message-lifecycle review scope but no demonstrated live regression.
  • merge-risk: 🚨 compatibility: The PR adds a new public Plugin SDK export and changes the normalized WhatsApp callback admission shape.
  • merge-risk: 🚨 message-delivery: History cleanup moves from WhatsApp reply finalization into shared post-dispatch turn execution.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This PR is authored by a repository MEMBER, so the external-contributor real-behavior-proof gate does not apply; supplied focused tests and checks are supplemental validation.
Evidence reviewed

PR surface:

Source +49, Tests +108, Docs +2, Generated 0, Other +2. Total +161 across 22 files.

View PR surface stats
Area Files Added Removed Net
Source 10 103 54 +49
Tests 7 148 40 +108
Docs 3 5 3 +2
Config 0 0 0 0
Generated 1 1 1 0
Other 1 4 2 +2
Total 22 261 100 +161

What I checked:

  • Public SDK surface change: The branch exports mapChannelIngressDecisionToTurnAdmission from the core message-access barrel and the public plugin-sdk/channel-ingress-runtime subpath, increasing the reported public-function export budget by one. (src/plugin-sdk/channel-ingress-runtime.ts:10, cbca97602b0a)
  • Redaction compatibility coverage: The WhatsApp admission test asserts that the new turnAdmission is exposed while effective allowlists and the second allowlisted number remain absent from the public callback envelope. (extensions/whatsapp/src/inbound/access-control.test.ts:155, cbca97602b0a)
  • Delivery-lifecycle refactor: WhatsApp removes duplicate history-clearing branches from its reply finalizer and carries group-history finalization through the shared turn plan, so successful, observe-only, failed, and broadcast delivery paths remain merge-sensitive. (extensions/whatsapp/src/auto-reply/monitor/process-message.ts:524, cbca97602b0a)
  • Prior review continuity: The prior blocker about exposing unredacted admission data was addressed in the current head: the latest commit is titled fix(whatsapp): preserve shipped admission callbacks, and the supplied follow-up review has no remaining line-level finding. (extensions/whatsapp/src/inbound/admission.ts:76, cbca97602b0a)
  • Validation status: The supplied PR checks show successful Plugin SDK API-baseline, production-type, documentation, lint, boundary, security, and build-artifact checks; remaining compact shards were still running and are not treated as a defect. (cbca97602b0a)

Likely related people:

  • mcaxtr: Authored the eight-commit WhatsApp inbound-turn and admission-contract series under review, including the compatibility repair at the current head; current-main feature-history attribution was unavailable in the read-only review environment. (role: recent area contributor; confidence: low; commits: 9015a621df62, cbca97602b0a; files: extensions/whatsapp/src/inbound/admission.ts, extensions/whatsapp/src/auto-reply/monitor/process-message.ts)
  • vincentkoc: Was explicitly requested as a participant on this PR's timeline; the supplied context does not provide enough history to establish stronger ownership of the SDK boundary. (role: likely follow-up owner; confidence: low; files: src/plugin-sdk/channel-ingress-runtime.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 (11 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-17T01:45:20.581Z sha b49e5b2 :: needs changes before merge. :: [P1] Keep the shipped admission projections redacted in the type
  • reviewed 2026-07-17T02:26:42.049Z sha b49e5b2 :: found issues before merge. :: [P1] Keep the exported admission type redacted
  • reviewed 2026-07-18T22:41:45.446Z sha 8b8b58c :: needs maintainer review before merge. :: none
  • reviewed 2026-07-18T22:48:35.488Z sha 8b8b58c :: needs maintainer review before merge. :: none
  • reviewed 2026-07-19T00:54:28.647Z sha 430f05e :: needs maintainer review before merge. :: none
  • reviewed 2026-07-19T01:18:03.533Z sha 7454ad7 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-19T05:07:16.824Z sha a46a783 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-19T05:18:23.300Z sha 0de0b73 :: needs maintainer review before merge. :: none

@BradGroux BradGroux mentioned this pull request Jul 14, 2026
Draft
@openclaw-barnacle openclaw-barnacle Bot added the scripts Repository scripts label Jul 15, 2026
@mcaxtr
mcaxtr force-pushed the refactor/whatsapp-prepared-turn-kernel branch from 1a2527f to b5252b7 Compare July 15, 2026 03:31
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P1 High-priority user-facing bug, regression, or broken workflow. labels Jul 16, 2026
@mcaxtr
mcaxtr force-pushed the refactor/whatsapp-prepared-turn-kernel branch 2 times, most recently from 8e09757 to 6510300 Compare July 17, 2026 01:22
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 17, 2026
@mcaxtr
mcaxtr force-pushed the refactor/whatsapp-prepared-turn-kernel branch from b49e5b2 to 8b8b58c Compare July 18, 2026 22:37
@clawsweeper clawsweeper Bot removed the rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. label Jul 18, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 18, 2026
@mcaxtr
mcaxtr force-pushed the refactor/whatsapp-prepared-turn-kernel branch from 8b8b58c to 430f05e Compare July 19, 2026 00:50
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 19, 2026
@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram and removed channel: telegram Channel integration: telegram labels Jul 19, 2026
@mcaxtr
mcaxtr force-pushed the refactor/whatsapp-prepared-turn-kernel branch from 0de0b73 to cbca976 Compare July 19, 2026 16:14
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

channel: whatsapp-web Channel integration: whatsapp-web docs Improvements or additions to documentation maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mcaxtr