mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-27 23:30:21 +00:00
5c7980fa11
* feat(imessage): support thumb approval reactions Mirrors openclaw#85477 (WhatsApp) for the iMessage channel. iMessage can now deliver exec/plugin approval prompts via the existing imsg/BlueBubbles transport and resolve approvals from 👍 (allow-once) / 👎 (deny) tapbacks. Allow-always remains on the manual /approve <id> allow-always fallback. What changed: - New approval surfaces under extensions/imessage/src/: approval-auth.ts, approval-resolver.ts, approval-reactions.ts, approval-handler.runtime.ts, approval-native.ts (+ tests for each). - channel.ts wires base.approvalCapability to the new iMessage capability. - send.ts appends the 👍/👎 hint to outbound /approve prompts and registers the reaction binding (keyed by accountId + chat_guid/chat_identifier/ chat_id/handle + messageId) after a successful send. - monitor/monitor-provider.ts resolves approval reactions ahead of the normal inbound decision pipeline so resolution bypasses reactionNotifications gating and runs its own actor authorization. - runtime.ts now exports getIMessageRuntime / getOptionalIMessageRuntime so approval-reactions can open a persistent keyed store for binding state across gateway restarts. What did NOT change: - Core approval surfaces in src/gateway/server-methods/* and src/infra/* remain channel-agnostic; the channels.imessage.allowFrom field already exists and is reused as the approver list for reactions. - Other channels and the manual /approve sender-authorized path are untouched. * fix(imessage): address codex review findings on thumb approvals Addresses 15 findings from the multi-angle codex review: Critical (correctness / blocking): - Register CHANNEL_APPROVAL_NATIVE_RUNTIME_CONTEXT_CAPABILITY in the iMessage monitor so the gateway can actually deliver native approval prompts via approval-handler.runtime.ts (it was dead code without the context lease). - DM tapback approvals never resolved because send keyed by handle while inbound preferred chat_guid. Register and look up under EVERY available conversation key (chat_guid / chat_identifier / chat_id / handle); inbound probes them all and accepts the first hit. - Reaction binding now requires the bridge's GUID string (rejecting numeric ROWIDs) so the binding key matches inbound reacted_to_guid. - Outbound regex now requires both a canonical `ID: <approvalId>` header AND a matching `/approve <id> <decision>` line, so non-approval messages that legitimately mention /approve syntax no longer get a phantom reaction binding (and can no longer resolve a colliding live approval). - Drop is_from_me reaction events so cross-device echoes of the operator's own tap cannot self-approve when their handle is in allowFrom. High (operability / cleanup): - Non-ApprovalNotFound errors now log at warn via the runtime child logger (no longer hidden behind OPENCLAW_LOG_LEVEL=debug). - In-memory binding is cleared on successful resolve so a toggle 👍→👎 (or chat.db replay) does not refire and emit a misleading 'expired approval' log line. Removed tapbacks are also owned by the shortcut and not surfaced as noisy reaction system events. - Move resolveIMessageReactionContext (and its helpers) to a slim monitor/reaction-context.ts so approval-reactions.ts no longer transitively pulls monitor/inbound-processing.ts (14+ heavy runtime modules) into the hot channel.ts entrypoint per extensions/CLAUDE.md. Medium (consistency / future-proofing): - Native runtime exec pending payload now passes agentId, ask, and sessionKey through buildExecApprovalPendingReplyPayload so the two delivery routes produce identical operator-visible prompts. - Both delivery paths now use addIMessageApprovalReactionHintToText (single insertion point after ID:) so the hint cannot be double-emitted by the native runtime path bypassing the idempotency guard. - Extract replaceApprovalIdPlaceholder into a shared approval-text.ts that escapes `$` in the replacement string so an approvalId containing `$&`/`$1`-`$9`/`$$` cannot interpolate into the outbound text. - In-memory Map now stores TTL alongside each entry and prunes expired bindings on each register so the gateway no longer accumulates an unbounded reaction-target Map. - bindPending refuses to bind when accountId is missing or the approval is already expired, with explicit error logs instead of silent no-ops. - Reject chat_id=0 as a synthetic key value (chat.db ROWIDs start at 1). - Drop dead getIMessageRuntime export — only the optional accessor is used. Documentation: - docs/channels/imessage.md gains an 'Approval reactions (👍 / 👎)' accordion documenting the reaction emoji map, allowFrom approver requirement, the /approve <id> allow-always manual fallback, and the deliberate change to /approve command authorization for users with non-empty allowFrom. - CHANGELOG.md entry added under 2026.5.24. Tests: 411 iMessage tests pass (was 406). Added explicit coverage for the DM key-mismatch fix, the regex-tightening fix, the is_from_me guard, the clear-on-success behavior, and the approval-id `$` escape. * test(imessage): match WhatsApp approval-native test coverage Backfills the nine cases from extensions/whatsapp/src/approval-native.test.ts that weren't mirrored in iMessage: - target-mode exec + plugin prompt rendering with the canonical hint - target-mode availability when no iMessage target matches - agentFilter / sessionFilter applied to native handling - account-scoped target enabled/disabled per account - shouldSuppressForwardingFallback session-origin exact-match cases - shouldSuppressForwardingFallback off when native cannot bind (locks down the targets-only forwarding path the Lobster live deploy exercised) - both-mode explicit + unscoped target suppression - group-origin tapback approvals require explicit approvers Tests: extensions/imessage/src/approval-native.test.ts 21 passed (was 11). Total iMessage approval-specific cases now 49 (was 40). * fix(imessage): preserve service-prefixed direct handles as approvers ClawSweeper P1 review finding on #85952. normalizeIMessageApproverId was calling looksLikeIMessageExplicitTargetId() to reject conversation-target prefixes, but that helper also matches the imessage:/sms:/auto: service prefixes — which are valid direct-handle forms. Any allowFrom entry like 'imessage:+15551230000' dropped to undefined, leaving approvers empty, which: - silently denied reaction resolution ('reactions require explicit approvers'), and - let text /approve fall back to implicit same-chat authorization. Fix: normalize first via normalizeIMessageHandle (strips the service prefix), then reject only chat_id:/chat_guid:/chat_identifier: conversation-target shapes that remain after normalization. Tests: - approval-auth.test.ts: assert the resolved approver list contains the normalized handle, plus the corollary that a non-matching sender is explicitly rejected (no longer masked by the implicit-same-chat fallback). Add a separate case covering chat_id/chat_guid/ chat_identifier rejection (with and without a service prefix). - approval-reactions.test.ts: reaction resolution end-to-end with a service-prefixed allowFrom entry — proves resolveIMessageApproval is called rather than silently denied. Focused suite: 48 passed (was 47). * test(imessage): satisfy strict buildPendingPayload signature in render tests CI check:test-types caught that the render.exec/render.plugin buildPendingPayload calls were passing accountId (not in the type signature). The signature is { cfg, request, target, nowMs }. Replace accountId with target on the four render-test sites so the strict test-types pass matches the SDK contract: - it('renders thumbs-only reaction hints in exec approval prompts') - it('renders thumbs-only reaction hints in plugin approval prompts ...') - it('renders target-mode exec prompts with concrete thumbs-only ...') - it('renders target-mode plugin prompts with concrete thumbs-only ...') Verified locally with pnpm check:test-types (tsgo:core:test + tsgo:extensions:test). 49 approval-specific tests still pass. * fix(imessage): probe every tapback GUID form for approval lookup ClawSweeper P1 review finding on #85952. readApprovalReactionEvent was only using reaction.targetGuid (the first/normalized form), but resolveIMessageReactionContext produces reaction.targetGuids = [normalized, raw] for both `abc-123` and `p:0/abc-123` forms. If the imsg bridge returned 'p:0/<guid>' from send() and send.ts registered the binding under that prefixed key, the inbound resolver probing only the unprefixed form would miss and the tapback would silently fall through. Fix: - Surface every GUID candidate in IMessageApprovalReactionEvent (messageIdCandidates). - maybeResolveIMessageApprovalReaction now probes each candidate in precedence order; first hit wins. - On success / ApprovalNotFoundError, clear the binding under all candidate keys so toggle/replay does not refire. Tests: extensions/imessage/src/approval-reactions.test.ts gains a 'resolves a reaction when the binding was registered under a p:0/… prefixed GUID and the tapback surfaces both forms' regression case; 22/22 reaction tests pass. Full iMessage suite: 424/424. * fix(imessage): native approval binding requires GUID, not numeric id ClawSweeper third P1 review finding on #85952. approval-handler.runtime.ts deliverPending was using result.messageId as the approval-reaction binding key, but that field can be a numeric ROWID coerced to a string ('12345') when the imsg bridge returns only message_id. Inbound tapbacks carry reacted_to_guid which is always a GUID, so a numeric-id binding can never match. Fix mirrors the send.ts forwarding-path treatment: - IMessageSendResult now exposes a separate guid?: string field, populated from the same resolveOutboundMessageGuid helper send.ts already uses for the forwarding-path binding. The generic messageId field is unchanged so reply-cache, echo-cache, and receipt-building paths still see the broadest id form. - deliverPending now binds against result.guid; when it's undefined (numeric ROWID or 'ok'/'unknown' placeholders), the function returns null instead of binding against an id the inbound tapback can't possibly match. Tests: approval-handler.runtime.test.ts gets a deliverPending GUID-only binding describe block with three regression cases (numeric ROWID refused, GUID accepted, ok/unknown placeholders refused). vi.mock isolates sendMessageIMessage so the cases run synchronously without spawning imsg. 11 tests pass across handler.runtime + send specs. --------- Co-authored-by: Omar Shahine <10343873+omarshahine@users.noreply.github.com>