mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-10 11:12:56 +00:00
f55ff8dd1b88596d2f92bd7f26277d9639f8eb21
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Kevin Lin
|
40bca6d8bb |
fix(imessage): suppress duplicate native exec approvals
Fix iMessage native exec approval routing so approval prompts bind to the sent GUID without duplicate sends after RPC timeout. Also keeps chat.db GUID recovery on the local imsg path while avoiding local DB recovery for configured or detected SSH wrappers. Thanks @kevinslin. |
||
|
Kevin Lin
|
bb752c2b47 |
Revert "feat: expose plugin approval action metadata" (#87419)
This reverts commit
|
||
|
Pablo Guardiola
|
0c867eef75 |
feat: expose plugin approval action metadata
Expose plugin approval action metadata so plugins can describe richer approval actions across gateway, SDK, channel, and UI surfaces. |
||
|
Omar Shahine
|
5c7980fa11 |
feat(imessage): support thumb approval reactions (#85952)
* 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> |