diff --git a/CHANGELOG.md b/CHANGELOG.md index 77d89bb8eff..021bdb9c744 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -176,6 +176,7 @@ Docs: https://docs.openclaw.ai - Windows/restart: fall back to the installed Startup-entry launcher when the scheduled task was never registered, so `/restart` can relaunch the gateway on Windows setups where `schtasks` install fell back during onboarding. (#58943) Thanks @imechZhangLY. - Agents/Claude CLI: persist routed Claude session bindings, rotate them on `/new` and `/reset`, and keep live Claude CLI model switches moving across the configured Claude family so resumed sessions follow the real active thread and model. Thanks @vincentkoc. - Providers/Anthropic: when Claude CLI auth becomes the default, write a real `claude-cli` auth profile so local and gateway agent runs can use Claude CLI immediately without missing-API-key failures. Thanks @vincentkoc. +- Matrix/exec approvals: anchor seeded approval reactions to the primary Matrix prompt event, resolve them from event metadata instead of prompt text, and clean up chunked approval prompts correctly. (#60931) thanks @gumadeiras. ## 2026.4.2 diff --git a/docs/channels/matrix.md b/docs/channels/matrix.md index 7e1c9855571..543cff92b42 100644 --- a/docs/channels/matrix.md +++ b/docs/channels/matrix.md @@ -679,7 +679,13 @@ Delivery rules: - `target: "channel"` sends the prompt back to the originating Matrix room or DM - `target: "both"` sends to approver DMs and the originating Matrix room or DM -Matrix uses text approval prompts today. Approvers resolve them with `/approve allow-once`, `/approve allow-always`, or `/approve deny`. +Matrix approval prompts seed reaction shortcuts on the primary approval message: + +- `✅` = allow once +- `❌` = deny +- `♾️` = allow always when that decision is allowed by the effective exec policy + +Approvers can react on that message or use the fallback slash commands: `/approve allow-once`, `/approve allow-always`, or `/approve deny`. Only resolved approvers can approve or deny. Channel delivery includes the command text, so only enable `channel` or `both` in trusted rooms. diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index c2331e155f0..0d139da4445 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -24,6 +24,11 @@ host policy sources, and the effective result. If the companion app UI is **not available**, any request that requires a prompt is resolved by the **ask fallback** (default: deny). +Native chat approval clients can also expose channel-specific affordances on the +pending approval message. For example, Matrix can seed reaction shortcuts on the +approval prompt (`✅` allow once, `❌` deny, and `♾️` allow always when available) +while still leaving the `/approve ...` commands in the message as a fallback. + ## Where it applies Exec approvals are enforced locally on the execution host: diff --git a/extensions/matrix/src/approval-reactions.test.ts b/extensions/matrix/src/approval-reactions.test.ts new file mode 100644 index 00000000000..cd2d9846e37 --- /dev/null +++ b/extensions/matrix/src/approval-reactions.test.ts @@ -0,0 +1,107 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { + buildMatrixApprovalReactionHint, + clearMatrixApprovalReactionTargetsForTest, + listMatrixApprovalReactionBindings, + registerMatrixApprovalReactionTarget, + resolveMatrixApprovalReactionTarget, + unregisterMatrixApprovalReactionTarget, +} from "./approval-reactions.js"; + +afterEach(() => { + clearMatrixApprovalReactionTargetsForTest(); +}); + +describe("matrix approval reactions", () => { + it("lists reactions in stable decision order", () => { + expect(listMatrixApprovalReactionBindings(["allow-once", "deny", "allow-always"])).toEqual([ + { decision: "allow-once", emoji: "✅", label: "Allow once" }, + { decision: "allow-always", emoji: "♾️", label: "Allow always" }, + { decision: "deny", emoji: "❌", label: "Deny" }, + ]); + }); + + it("builds a compact reaction hint", () => { + expect(buildMatrixApprovalReactionHint(["allow-once", "deny"])).toBe( + "React here: ✅ Allow once, ❌ Deny", + ); + }); + + it("resolves a registered approval anchor event back to an approval decision", () => { + registerMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + approvalId: "req-123", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }); + + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "✅", + }), + ).toEqual({ + approvalId: "req-123", + decision: "allow-once", + }); + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "♾️", + }), + ).toEqual({ + approvalId: "req-123", + decision: "allow-always", + }); + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "❌", + }), + ).toEqual({ + approvalId: "req-123", + decision: "deny", + }); + }); + + it("ignores reactions that are not allowed on the registered approval anchor event", () => { + registerMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + approvalId: "req-123", + allowedDecisions: ["allow-once", "deny"], + }); + + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "♾️", + }), + ).toBeNull(); + }); + + it("stops resolving reactions after the approval anchor event is unregistered", () => { + registerMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + approvalId: "req-123", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }); + unregisterMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + }); + + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "✅", + }), + ).toBeNull(); + }); +}); diff --git a/extensions/matrix/src/approval-reactions.ts b/extensions/matrix/src/approval-reactions.ts new file mode 100644 index 00000000000..03b8c68cfeb --- /dev/null +++ b/extensions/matrix/src/approval-reactions.ts @@ -0,0 +1,158 @@ +import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-runtime"; + +const MATRIX_APPROVAL_REACTION_META = { + "allow-once": { + emoji: "✅", + label: "Allow once", + }, + "allow-always": { + emoji: "♾️", + label: "Allow always", + }, + deny: { + emoji: "❌", + label: "Deny", + }, +} satisfies Record; + +const MATRIX_APPROVAL_REACTION_ORDER = [ + "allow-once", + "allow-always", + "deny", +] as const satisfies readonly ExecApprovalReplyDecision[]; + +export type MatrixApprovalReactionBinding = { + decision: ExecApprovalReplyDecision; + emoji: string; + label: string; +}; + +export type MatrixApprovalReactionResolution = { + approvalId: string; + decision: ExecApprovalReplyDecision; +}; + +type MatrixApprovalReactionTarget = { + approvalId: string; + allowedDecisions: readonly ExecApprovalReplyDecision[]; +}; + +const matrixApprovalReactionTargets = new Map(); + +function buildReactionTargetKey(roomId: string, eventId: string): string | null { + const normalizedRoomId = roomId.trim(); + const normalizedEventId = eventId.trim(); + if (!normalizedRoomId || !normalizedEventId) { + return null; + } + return `${normalizedRoomId}:${normalizedEventId}`; +} + +export function listMatrixApprovalReactionBindings( + allowedDecisions: readonly ExecApprovalReplyDecision[], +): MatrixApprovalReactionBinding[] { + const allowed = new Set(allowedDecisions); + return MATRIX_APPROVAL_REACTION_ORDER.filter((decision) => allowed.has(decision)).map( + (decision) => ({ + decision, + emoji: MATRIX_APPROVAL_REACTION_META[decision].emoji, + label: MATRIX_APPROVAL_REACTION_META[decision].label, + }), + ); +} + +export function buildMatrixApprovalReactionHint( + allowedDecisions: readonly ExecApprovalReplyDecision[], +): string | null { + const bindings = listMatrixApprovalReactionBindings(allowedDecisions); + if (bindings.length === 0) { + return null; + } + return `React here: ${bindings.map((binding) => `${binding.emoji} ${binding.label}`).join(", ")}`; +} + +export function resolveMatrixApprovalReactionDecision( + reactionKey: string, + allowedDecisions: readonly ExecApprovalReplyDecision[], +): ExecApprovalReplyDecision | null { + const normalizedReaction = reactionKey.trim(); + if (!normalizedReaction) { + return null; + } + const allowed = new Set(allowedDecisions); + for (const decision of MATRIX_APPROVAL_REACTION_ORDER) { + if (!allowed.has(decision)) { + continue; + } + if (MATRIX_APPROVAL_REACTION_META[decision].emoji === normalizedReaction) { + return decision; + } + } + return null; +} + +export function registerMatrixApprovalReactionTarget(params: { + roomId: string; + eventId: string; + approvalId: string; + allowedDecisions: readonly ExecApprovalReplyDecision[]; +}): void { + const key = buildReactionTargetKey(params.roomId, params.eventId); + const approvalId = params.approvalId.trim(); + const allowedDecisions = Array.from( + new Set( + params.allowedDecisions.filter( + (decision): decision is ExecApprovalReplyDecision => + decision === "allow-once" || decision === "allow-always" || decision === "deny", + ), + ), + ); + if (!key || !approvalId || allowedDecisions.length === 0) { + return; + } + matrixApprovalReactionTargets.set(key, { + approvalId, + allowedDecisions, + }); +} + +export function unregisterMatrixApprovalReactionTarget(params: { + roomId: string; + eventId: string; +}): void { + const key = buildReactionTargetKey(params.roomId, params.eventId); + if (!key) { + return; + } + matrixApprovalReactionTargets.delete(key); +} + +export function resolveMatrixApprovalReactionTarget(params: { + roomId: string; + eventId: string; + reactionKey: string; +}): MatrixApprovalReactionResolution | null { + const key = buildReactionTargetKey(params.roomId, params.eventId); + if (!key) { + return null; + } + const target = matrixApprovalReactionTargets.get(key); + if (!target) { + return null; + } + const decision = resolveMatrixApprovalReactionDecision( + params.reactionKey, + target.allowedDecisions, + ); + if (!decision) { + return null; + } + return { + approvalId: target.approvalId, + decision, + }; +} + +export function clearMatrixApprovalReactionTargetsForTest(): void { + matrixApprovalReactionTargets.clear(); +} diff --git a/extensions/matrix/src/exec-approval-resolver.test.ts b/extensions/matrix/src/exec-approval-resolver.test.ts new file mode 100644 index 00000000000..e2e15f106a4 --- /dev/null +++ b/extensions/matrix/src/exec-approval-resolver.test.ts @@ -0,0 +1,59 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const gatewayRuntimeHoisted = vi.hoisted(() => ({ + requestSpy: vi.fn(), + startSpy: vi.fn(), + stopSpy: vi.fn(), + stopAndWaitSpy: vi.fn(async () => undefined), + createClientSpy: vi.fn(), +})); + +vi.mock("openclaw/plugin-sdk/gateway-runtime", () => ({ + createOperatorApprovalsGatewayClient: gatewayRuntimeHoisted.createClientSpy, +})); + +describe("resolveMatrixExecApproval", () => { + beforeEach(() => { + gatewayRuntimeHoisted.requestSpy.mockReset(); + gatewayRuntimeHoisted.startSpy.mockReset(); + gatewayRuntimeHoisted.stopSpy.mockReset(); + gatewayRuntimeHoisted.stopAndWaitSpy.mockReset().mockResolvedValue(undefined); + gatewayRuntimeHoisted.createClientSpy.mockReset().mockImplementation((opts) => ({ + start: () => { + gatewayRuntimeHoisted.startSpy(); + opts.onHelloOk?.(); + }, + request: gatewayRuntimeHoisted.requestSpy, + stop: gatewayRuntimeHoisted.stopSpy, + stopAndWait: gatewayRuntimeHoisted.stopAndWaitSpy, + })); + }); + + it("submits exec approval resolutions through the gateway approvals client", async () => { + const { resolveMatrixExecApproval } = await import("./exec-approval-resolver.js"); + + await resolveMatrixExecApproval({ + cfg: {} as never, + approvalId: "req-123", + decision: "allow-once", + senderId: "@owner:example.org", + }); + + expect(gatewayRuntimeHoisted.requestSpy).toHaveBeenCalledWith("exec.approval.resolve", { + id: "req-123", + decision: "allow-once", + }); + }); + + it("recognizes structured approval-not-found errors", async () => { + const { isApprovalNotFoundError } = await import("./exec-approval-resolver.js"); + const err = new Error("approval not found"); + (err as Error & { gatewayCode?: string; details?: { reason?: string } }).gatewayCode = + "INVALID_REQUEST"; + (err as Error & { gatewayCode?: string; details?: { reason?: string } }).details = { + reason: "APPROVAL_NOT_FOUND", + }; + + expect(isApprovalNotFoundError(err)).toBe(true); + }); +}); diff --git a/extensions/matrix/src/exec-approval-resolver.ts b/extensions/matrix/src/exec-approval-resolver.ts new file mode 100644 index 00000000000..cf065211fda --- /dev/null +++ b/extensions/matrix/src/exec-approval-resolver.ts @@ -0,0 +1,64 @@ +import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-runtime"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; +import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; +import { createOperatorApprovalsGatewayClient } from "openclaw/plugin-sdk/gateway-runtime"; + +export { isApprovalNotFoundError }; + +export async function resolveMatrixExecApproval(params: { + cfg: OpenClawConfig; + approvalId: string; + decision: ExecApprovalReplyDecision; + senderId?: string | null; + gatewayUrl?: string; +}): Promise { + let readySettled = false; + let resolveReady!: () => void; + let rejectReady!: (err: unknown) => void; + const ready = new Promise((resolve, reject) => { + resolveReady = resolve; + rejectReady = reject; + }); + const markReady = () => { + if (readySettled) { + return; + } + readySettled = true; + resolveReady(); + }; + const failReady = (err: unknown) => { + if (readySettled) { + return; + } + readySettled = true; + rejectReady(err); + }; + + const gatewayClient = await createOperatorApprovalsGatewayClient({ + config: params.cfg, + gatewayUrl: params.gatewayUrl, + clientDisplayName: `Matrix approval (${params.senderId?.trim() || "unknown"})`, + onHelloOk: () => { + markReady(); + }, + onConnectError: (err) => { + failReady(err); + }, + onClose: (code, reason) => { + failReady(new Error(`gateway closed (${code}): ${reason}`)); + }, + }); + + try { + gatewayClient.start(); + await ready; + await gatewayClient.request("exec.approval.resolve", { + id: params.approvalId, + decision: params.decision, + }); + } finally { + await gatewayClient.stopAndWait().catch(() => { + gatewayClient.stop(); + }); + } +} diff --git a/extensions/matrix/src/exec-approvals-handler.test.ts b/extensions/matrix/src/exec-approvals-handler.test.ts index 290b28ddee1..f6d4397a6d1 100644 --- a/extensions/matrix/src/exec-approvals-handler.test.ts +++ b/extensions/matrix/src/exec-approvals-handler.test.ts @@ -1,5 +1,9 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + clearMatrixApprovalReactionTargetsForTest, + resolveMatrixApprovalReactionTarget, +} from "./approval-reactions.js"; import { MatrixExecApprovalHandler } from "./exec-approvals-handler.js"; const baseRequest = { @@ -23,6 +27,7 @@ function createHandler(cfg: OpenClawConfig, accountId = "default") { .fn() .mockResolvedValueOnce({ messageId: "$m1", roomId: "!ops:example.org" }) .mockResolvedValue({ messageId: "$m2", roomId: "!dm-owner:example.org" }); + const reactMessage = vi.fn().mockResolvedValue(undefined); const editMessage = vi.fn().mockResolvedValue({ eventId: "$edit1" }); const deleteMessage = vi.fn().mockResolvedValue(undefined); const repairDirectRooms = vi.fn().mockResolvedValue({ @@ -37,16 +42,26 @@ function createHandler(cfg: OpenClawConfig, accountId = "default") { { nowMs: () => 1000, sendMessage, + reactMessage, editMessage, deleteMessage, repairDirectRooms, }, ); - return { client, handler, sendMessage, editMessage, deleteMessage, repairDirectRooms }; + return { + client, + handler, + sendMessage, + reactMessage, + editMessage, + deleteMessage, + repairDirectRooms, + }; } afterEach(() => { vi.useRealTimers(); + clearMatrixApprovalReactionTargetsForTest(); }); describe("MatrixExecApprovalHandler", () => { @@ -79,6 +94,54 @@ describe("MatrixExecApprovalHandler", () => { ); }); + it("seeds emoji reactions for each allowed approval decision", async () => { + const cfg = { + channels: { + matrix: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok", + execApprovals: { + enabled: true, + approvers: ["@owner:example.org"], + target: "channel", + }, + }, + }, + } as OpenClawConfig; + const { handler, reactMessage, sendMessage } = createHandler(cfg); + + await handler.handleRequested(baseRequest); + + expect(sendMessage).toHaveBeenCalledWith( + "room:!ops:example.org", + expect.stringContaining("React here: ✅ Allow once, ♾️ Allow always, ❌ Deny"), + expect.anything(), + ); + expect(reactMessage).toHaveBeenCalledTimes(3); + expect(reactMessage).toHaveBeenNthCalledWith( + 1, + "!ops:example.org", + "$m1", + "✅", + expect.anything(), + ); + expect(reactMessage).toHaveBeenNthCalledWith( + 2, + "!ops:example.org", + "$m1", + "♾️", + expect.anything(), + ); + expect(reactMessage).toHaveBeenNthCalledWith( + 3, + "!ops:example.org", + "$m1", + "❌", + expect.anything(), + ); + }); + it("falls back to approver dms when channel routing is unavailable", async () => { const cfg = { channels: { @@ -245,6 +308,62 @@ describe("MatrixExecApprovalHandler", () => { ); }); + it("anchors reactions on the first chunk and clears stale chunks on resolve", async () => { + const cfg = { + channels: { + matrix: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok", + execApprovals: { + enabled: true, + approvers: ["@owner:example.org"], + target: "channel", + }, + }, + }, + } as OpenClawConfig; + const { handler, sendMessage, reactMessage, editMessage, deleteMessage } = createHandler(cfg); + sendMessage.mockReset().mockResolvedValue({ + messageId: "$m3", + primaryMessageId: "$m1", + messageIds: ["$m1", "$m2", "$m3"], + roomId: "!ops:example.org", + }); + + await handler.handleRequested(baseRequest); + await handler.handleResolved({ + id: baseRequest.id, + decision: "allow-once", + resolvedBy: "matrix:@owner:example.org", + ts: 2000, + }); + + expect(reactMessage).toHaveBeenNthCalledWith( + 1, + "!ops:example.org", + "$m1", + "✅", + expect.anything(), + ); + expect(editMessage).toHaveBeenCalledWith( + "!ops:example.org", + "$m1", + expect.stringContaining("Exec approval: Allowed once"), + expect.anything(), + ); + expect(deleteMessage).toHaveBeenCalledWith( + "!ops:example.org", + "$m2", + expect.objectContaining({ reason: "approval resolved" }), + ); + expect(deleteMessage).toHaveBeenCalledWith( + "!ops:example.org", + "$m3", + expect.objectContaining({ reason: "approval resolved" }), + ); + }); + it("deletes tracked approval messages when they expire", async () => { vi.useFakeTimers(); const cfg = { @@ -276,6 +395,79 @@ describe("MatrixExecApprovalHandler", () => { ); }); + it("deletes every chunk of a tracked approval prompt when it expires", async () => { + vi.useFakeTimers(); + const cfg = { + channels: { + matrix: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok", + execApprovals: { + enabled: true, + approvers: ["@owner:example.org"], + target: "channel", + }, + }, + }, + } as OpenClawConfig; + const { handler, sendMessage, deleteMessage } = createHandler(cfg); + sendMessage.mockReset().mockResolvedValue({ + messageId: "$m3", + primaryMessageId: "$m1", + messageIds: ["$m1", "$m2", "$m3"], + roomId: "!ops:example.org", + }); + + await handler.handleRequested(baseRequest); + await vi.advanceTimersByTimeAsync(60_000); + + expect(deleteMessage).toHaveBeenCalledWith( + "!ops:example.org", + "$m1", + expect.objectContaining({ reason: "approval expired" }), + ); + expect(deleteMessage).toHaveBeenCalledWith( + "!ops:example.org", + "$m2", + expect.objectContaining({ reason: "approval expired" }), + ); + expect(deleteMessage).toHaveBeenCalledWith( + "!ops:example.org", + "$m3", + expect.objectContaining({ reason: "approval expired" }), + ); + }); + + it("clears tracked approval anchors when the handler stops", async () => { + const cfg = { + channels: { + matrix: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok", + execApprovals: { + enabled: true, + approvers: ["@owner:example.org"], + target: "channel", + }, + }, + }, + } as OpenClawConfig; + const { handler } = createHandler(cfg); + + await handler.handleRequested(baseRequest); + await handler.stop(); + + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$m1", + reactionKey: "✅", + }), + ).toBeNull(); + }); + it("honors request decision constraints in pending approval text", async () => { const cfg = { channels: { @@ -291,7 +483,7 @@ describe("MatrixExecApprovalHandler", () => { }, }, } as OpenClawConfig; - const { handler, sendMessage } = createHandler(cfg); + const { handler, sendMessage, reactMessage } = createHandler(cfg); await handler.handleRequested({ ...baseRequest, @@ -307,5 +499,25 @@ describe("MatrixExecApprovalHandler", () => { expect.not.stringContaining("allow-always"), expect.anything(), ); + expect(sendMessage).toHaveBeenCalledWith( + "room:!ops:example.org", + expect.stringContaining("React here: ✅ Allow once, ❌ Deny"), + expect.anything(), + ); + expect(reactMessage).toHaveBeenCalledTimes(2); + expect(reactMessage).toHaveBeenNthCalledWith( + 1, + "!ops:example.org", + "$m1", + "✅", + expect.anything(), + ); + expect(reactMessage).toHaveBeenNthCalledWith( + 2, + "!ops:example.org", + "$m1", + "❌", + expect.anything(), + ); }); }); diff --git a/extensions/matrix/src/exec-approvals-handler.ts b/extensions/matrix/src/exec-approvals-handler.ts index d8f29a76708..5f115d069b5 100644 --- a/extensions/matrix/src/exec-approvals-handler.ts +++ b/extensions/matrix/src/exec-approvals-handler.ts @@ -1,6 +1,8 @@ import { buildExecApprovalPendingReplyPayload, + type ExecApprovalReplyDecision, getExecApprovalApproverDmNoticeText, + resolveExecApprovalAllowedDecisions, resolveExecApprovalCommandDisplay, } from "openclaw/plugin-sdk/approval-reply-runtime"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; @@ -11,6 +13,12 @@ import { type ExecApprovalResolved, } from "openclaw/plugin-sdk/infra-runtime"; import { matrixNativeApprovalAdapter } from "./approval-native.js"; +import { + buildMatrixApprovalReactionHint, + listMatrixApprovalReactionBindings, + registerMatrixApprovalReactionTarget, + unregisterMatrixApprovalReactionTarget, +} from "./approval-reactions.js"; import { isMatrixExecApprovalClientEnabled, shouldHandleMatrixExecApprovalRequest, @@ -19,7 +27,7 @@ import { resolveMatrixAccount } from "./matrix/accounts.js"; import { deleteMatrixMessage, editMatrixMessage } from "./matrix/actions/messages.js"; import { repairMatrixDirectRooms } from "./matrix/direct-management.js"; import type { MatrixClient } from "./matrix/sdk.js"; -import { sendMessageMatrix } from "./matrix/send.js"; +import { reactMatrixMessage, sendMessageMatrix } from "./matrix/send.js"; import { resolveMatrixTargetIdentity } from "./matrix/target-ids.js"; import type { CoreConfig } from "./types.js"; @@ -27,7 +35,8 @@ type ApprovalRequest = ExecApprovalRequest; type ApprovalResolved = ExecApprovalResolved; type PendingMessage = { roomId: string; - messageId: string; + messageIds: readonly string[]; + reactionEventId: string; }; type PreparedMatrixTarget = { @@ -35,6 +44,15 @@ type PreparedMatrixTarget = { roomId: string; threadId?: string; }; +type PendingApprovalContent = { + approvalId: string; + text: string; + allowedDecisions: readonly ExecApprovalReplyDecision[]; +}; +type ReactionTargetRef = { + roomId: string; + eventId: string; +}; export type MatrixExecApprovalHandlerOpts = { client: MatrixClient; @@ -46,11 +64,33 @@ export type MatrixExecApprovalHandlerOpts = { export type MatrixExecApprovalHandlerDeps = { nowMs?: () => number; sendMessage?: typeof sendMessageMatrix; + reactMessage?: typeof reactMatrixMessage; editMessage?: typeof editMatrixMessage; deleteMessage?: typeof deleteMatrixMessage; repairDirectRooms?: typeof repairMatrixDirectRooms; }; +function normalizePendingMessageIds(entry: PendingMessage): string[] { + return Array.from(new Set(entry.messageIds.map((messageId) => messageId.trim()).filter(Boolean))); +} + +function normalizeReactionTargetRef(params: ReactionTargetRef): ReactionTargetRef | null { + const roomId = params.roomId.trim(); + const eventId = params.eventId.trim(); + if (!roomId || !eventId) { + return null; + } + return { roomId, eventId }; +} + +function buildReactionTargetRefKey(params: ReactionTargetRef): string | null { + const normalized = normalizeReactionTargetRef(params); + if (!normalized) { + return null; + } + return `${normalized.roomId}\u0000${normalized.eventId}`; +} + function isHandlerConfigured(params: { cfg: OpenClawConfig; accountId: string }): boolean { return isMatrixExecApprovalClientEnabled(params); } @@ -60,14 +100,20 @@ function normalizeThreadId(value?: string | number | null): string | undefined { return trimmed || undefined; } -function buildPendingApprovalText(params: { request: ApprovalRequest; nowMs: number }): string { - return buildExecApprovalPendingReplyPayload({ +function buildPendingApprovalContent(params: { + request: ApprovalRequest; + nowMs: number; +}): PendingApprovalContent { + const allowedDecisions = + params.request.request.allowedDecisions ?? + resolveExecApprovalAllowedDecisions({ ask: params.request.request.ask ?? undefined }); + const payload = buildExecApprovalPendingReplyPayload({ approvalId: params.request.id, approvalSlug: params.request.id.slice(0, 8), approvalCommandId: params.request.id, ask: params.request.request.ask ?? undefined, agentId: params.request.request.agentId ?? undefined, - allowedDecisions: params.request.request.allowedDecisions, + allowedDecisions, command: resolveExecApprovalCommandDisplay((params.request as ExecApprovalRequest).request) .commandText, cwd: (params.request as ExecApprovalRequest).request.cwd ?? undefined, @@ -76,7 +122,14 @@ function buildPendingApprovalText(params: { request: ApprovalRequest; nowMs: num sessionKey: params.request.request.sessionKey ?? undefined, expiresAtMs: params.request.expiresAtMs, nowMs: params.nowMs, - }).text!; + }); + const hint = buildMatrixApprovalReactionHint(allowedDecisions); + const text = payload.text ?? ""; + return { + approvalId: params.request.id, + text: hint ? `${text}\n\n${hint}` : text, + allowedDecisions, + }; } function buildResolvedApprovalText(params: { @@ -95,8 +148,10 @@ function buildResolvedApprovalText(params: { export class MatrixExecApprovalHandler { private readonly runtime: ExecApprovalChannelRuntime; + private readonly trackedReactionTargets = new Map(); private readonly nowMs: () => number; private readonly sendMessage: typeof sendMessageMatrix; + private readonly reactMessage: typeof reactMatrixMessage; private readonly editMessage: typeof editMatrixMessage; private readonly deleteMessage: typeof deleteMatrixMessage; private readonly repairDirectRooms: typeof repairMatrixDirectRooms; @@ -107,13 +162,14 @@ export class MatrixExecApprovalHandler { ) { this.nowMs = deps.nowMs ?? Date.now; this.sendMessage = deps.sendMessage ?? sendMessageMatrix; + this.reactMessage = deps.reactMessage ?? reactMatrixMessage; this.editMessage = deps.editMessage ?? editMatrixMessage; this.deleteMessage = deps.deleteMessage ?? deleteMatrixMessage; this.repairDirectRooms = deps.repairDirectRooms ?? repairMatrixDirectRooms; this.runtime = createChannelNativeApprovalRuntime< PendingMessage, PreparedMatrixTarget, - string, + PendingApprovalContent, ApprovalRequest, ApprovalResolved >({ @@ -134,7 +190,7 @@ export class MatrixExecApprovalHandler { request, }), buildPendingContent: ({ request, nowMs }) => - buildPendingApprovalText({ + buildPendingApprovalContent({ request, nowMs, }), @@ -161,15 +217,42 @@ export class MatrixExecApprovalHandler { }; }, deliverTarget: async ({ preparedTarget, pendingContent }) => { - const result = await this.sendMessage(preparedTarget.to, pendingContent, { + const result = await this.sendMessage(preparedTarget.to, pendingContent.text, { cfg: this.opts.cfg as CoreConfig, accountId: this.opts.accountId, client: this.opts.client, threadId: preparedTarget.threadId, }); + const messageIds = Array.from( + new Set( + (result.messageIds ?? [result.messageId]) + .map((messageId) => messageId.trim()) + .filter(Boolean), + ), + ); + const reactionEventId = + result.primaryMessageId?.trim() || messageIds[0] || result.messageId.trim(); + this.trackReactionTarget({ + roomId: result.roomId, + eventId: reactionEventId, + approvalId: pendingContent.approvalId, + allowedDecisions: pendingContent.allowedDecisions, + }); + await Promise.allSettled( + listMatrixApprovalReactionBindings(pendingContent.allowedDecisions).map( + async ({ emoji }) => { + await this.reactMessage(result.roomId, reactionEventId, emoji, { + cfg: this.opts.cfg as CoreConfig, + accountId: this.opts.accountId, + client: this.opts.client, + }); + }, + ), + ); return { roomId: result.roomId, - messageId: result.messageId, + messageIds, + reactionEventId, }; }, finalizeResolved: async ({ request, resolved, entries }) => { @@ -187,6 +270,7 @@ export class MatrixExecApprovalHandler { async stop(): Promise { await this.runtime.stop(); + this.clearTrackedReactionTargets(); } async handleRequested(request: ApprovalRequest): Promise { @@ -240,11 +324,29 @@ export class MatrixExecApprovalHandler { const text = buildResolvedApprovalText({ request, resolved }); await Promise.allSettled( entries.map(async (entry) => { - await this.editMessage(entry.roomId, entry.messageId, text, { - cfg: this.opts.cfg as CoreConfig, - accountId: this.opts.accountId, - client: this.opts.client, + this.untrackReactionTarget({ + roomId: entry.roomId, + eventId: entry.reactionEventId, }); + const [primaryMessageId, ...staleMessageIds] = normalizePendingMessageIds(entry); + if (!primaryMessageId) { + return; + } + await Promise.allSettled([ + this.editMessage(entry.roomId, primaryMessageId, text, { + cfg: this.opts.cfg as CoreConfig, + accountId: this.opts.accountId, + client: this.opts.client, + }), + ...staleMessageIds.map(async (messageId) => { + await this.deleteMessage(entry.roomId, messageId, { + cfg: this.opts.cfg as CoreConfig, + accountId: this.opts.accountId, + client: this.opts.client, + reason: "approval resolved", + }); + }), + ]); }), ); } @@ -252,13 +354,58 @@ export class MatrixExecApprovalHandler { private async clearPending(entries: PendingMessage[]): Promise { await Promise.allSettled( entries.map(async (entry) => { - await this.deleteMessage(entry.roomId, entry.messageId, { - cfg: this.opts.cfg as CoreConfig, - accountId: this.opts.accountId, - client: this.opts.client, - reason: "approval expired", + this.untrackReactionTarget({ + roomId: entry.roomId, + eventId: entry.reactionEventId, }); + await Promise.allSettled( + normalizePendingMessageIds(entry).map(async (messageId) => { + await this.deleteMessage(entry.roomId, messageId, { + cfg: this.opts.cfg as CoreConfig, + accountId: this.opts.accountId, + client: this.opts.client, + reason: "approval expired", + }); + }), + ); }), ); } + + private trackReactionTarget( + params: ReactionTargetRef & { + approvalId: string; + allowedDecisions: readonly ExecApprovalReplyDecision[]; + }, + ): void { + const normalized = normalizeReactionTargetRef(params); + const key = normalized ? buildReactionTargetRefKey(normalized) : null; + if (!normalized || !key) { + return; + } + registerMatrixApprovalReactionTarget({ + roomId: normalized.roomId, + eventId: normalized.eventId, + approvalId: params.approvalId, + allowedDecisions: params.allowedDecisions, + }); + this.trackedReactionTargets.set(key, normalized); + } + + private untrackReactionTarget(params: ReactionTargetRef): void { + const normalized = normalizeReactionTargetRef(params); + const key = normalized ? buildReactionTargetRefKey(normalized) : null; + if (!normalized || !key) { + return; + } + unregisterMatrixApprovalReactionTarget(normalized); + this.trackedReactionTargets.delete(key); + } + + private clearTrackedReactionTargets(): void { + for (const target of this.trackedReactionTargets.values()) { + unregisterMatrixApprovalReactionTarget(target); + } + this.trackedReactionTargets.clear(); + } } diff --git a/extensions/matrix/src/matrix/monitor/reaction-events.test.ts b/extensions/matrix/src/matrix/monitor/reaction-events.test.ts new file mode 100644 index 00000000000..e9e8493c178 --- /dev/null +++ b/extensions/matrix/src/matrix/monitor/reaction-events.test.ts @@ -0,0 +1,344 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + clearMatrixApprovalReactionTargetsForTest, + registerMatrixApprovalReactionTarget, + resolveMatrixApprovalReactionTarget, +} from "../../approval-reactions.js"; +import type { CoreConfig } from "../../types.js"; +import { handleInboundMatrixReaction } from "./reaction-events.js"; + +const resolveMatrixExecApproval = vi.fn(); + +vi.mock("../../exec-approval-resolver.js", () => ({ + isApprovalNotFoundError: (err: unknown) => + err instanceof Error && /unknown or expired approval id/i.test(err.message), + resolveMatrixExecApproval: (...args: unknown[]) => resolveMatrixExecApproval(...args), +})); + +beforeEach(() => { + resolveMatrixExecApproval.mockReset(); + clearMatrixApprovalReactionTargetsForTest(); +}); + +function buildConfig(): CoreConfig { + return { + channels: { + matrix: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok", + reactionNotifications: "own", + execApprovals: { + enabled: true, + approvers: ["@owner:example.org"], + target: "channel", + }, + }, + }, + } as CoreConfig; +} + +function buildCore() { + return { + channel: { + routing: { + resolveAgentRoute: vi.fn().mockReturnValue({ + sessionKey: "agent:main:matrix:channel:!ops:example.org", + mainSessionKey: "agent:main:matrix:channel:!ops:example.org", + agentId: "main", + matchedBy: "peer", + }), + }, + }, + system: { + enqueueSystemEvent: vi.fn(), + }, + } as unknown as Parameters[0]["core"]; +} + +describe("matrix approval reactions", () => { + it("resolves approval reactions instead of enqueueing a generic reaction event", async () => { + const core = buildCore(); + registerMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + approvalId: "req-123", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }); + const client = { + getEvent: vi.fn().mockResolvedValue({ + event_id: "$approval-msg", + sender: "@bot:example.org", + content: { body: "approval prompt" }, + }), + } as unknown as Parameters[0]["client"]; + + await handleInboundMatrixReaction({ + client, + core, + cfg: buildConfig(), + accountId: "default", + roomId: "!ops:example.org", + event: { + event_id: "$reaction-1", + origin_server_ts: 123, + content: { + "m.relates_to": { + rel_type: "m.annotation", + event_id: "$approval-msg", + key: "✅", + }, + }, + } as never, + senderId: "@owner:example.org", + senderLabel: "Owner", + selfUserId: "@bot:example.org", + isDirectMessage: false, + logVerboseMessage: vi.fn(), + }); + + expect(resolveMatrixExecApproval).toHaveBeenCalledWith({ + cfg: buildConfig(), + approvalId: "req-123", + decision: "allow-once", + senderId: "@owner:example.org", + }); + expect(core.system.enqueueSystemEvent).not.toHaveBeenCalled(); + }); + + it("keeps ordinary reactions on bot messages as generic reaction events", async () => { + const core = buildCore(); + const client = { + getEvent: vi.fn().mockResolvedValue({ + event_id: "$msg-1", + sender: "@bot:example.org", + content: { + body: "normal bot message", + }, + }), + } as unknown as Parameters[0]["client"]; + + await handleInboundMatrixReaction({ + client, + core, + cfg: buildConfig(), + accountId: "default", + roomId: "!ops:example.org", + event: { + event_id: "$reaction-1", + origin_server_ts: 123, + content: { + "m.relates_to": { + rel_type: "m.annotation", + event_id: "$msg-1", + key: "👍", + }, + }, + } as never, + senderId: "@owner:example.org", + senderLabel: "Owner", + selfUserId: "@bot:example.org", + isDirectMessage: false, + logVerboseMessage: vi.fn(), + }); + + expect(resolveMatrixExecApproval).not.toHaveBeenCalled(); + expect(core.system.enqueueSystemEvent).toHaveBeenCalledWith( + "Matrix reaction added: 👍 by Owner on msg $msg-1", + expect.objectContaining({ + contextKey: "matrix:reaction:add:!ops:example.org:$msg-1:@owner:example.org:👍", + }), + ); + }); + + it("still resolves approval reactions when generic reaction notifications are off", async () => { + const core = buildCore(); + const cfg = buildConfig(); + const matrixCfg = cfg.channels?.matrix; + if (!matrixCfg) { + throw new Error("matrix config missing"); + } + matrixCfg.reactionNotifications = "off"; + registerMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + approvalId: "req-123", + allowedDecisions: ["deny"], + }); + const client = { + getEvent: vi.fn().mockResolvedValue({ + event_id: "$approval-msg", + sender: "@bot:example.org", + content: { body: "approval prompt" }, + }), + } as unknown as Parameters[0]["client"]; + + await handleInboundMatrixReaction({ + client, + core, + cfg, + accountId: "default", + roomId: "!ops:example.org", + event: { + event_id: "$reaction-1", + origin_server_ts: 123, + content: { + "m.relates_to": { + rel_type: "m.annotation", + event_id: "$approval-msg", + key: "❌", + }, + }, + } as never, + senderId: "@owner:example.org", + senderLabel: "Owner", + selfUserId: "@bot:example.org", + isDirectMessage: false, + logVerboseMessage: vi.fn(), + }); + + expect(resolveMatrixExecApproval).toHaveBeenCalledWith({ + cfg, + approvalId: "req-123", + decision: "deny", + senderId: "@owner:example.org", + }); + expect(core.system.enqueueSystemEvent).not.toHaveBeenCalled(); + }); + + it("resolves registered approval reactions without fetching the target event", async () => { + const core = buildCore(); + registerMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + approvalId: "req-123", + allowedDecisions: ["allow-once"], + }); + const client = { + getEvent: vi.fn().mockRejectedValue(new Error("boom")), + } as unknown as Parameters[0]["client"]; + + await handleInboundMatrixReaction({ + client, + core, + cfg: buildConfig(), + accountId: "default", + roomId: "!ops:example.org", + event: { + event_id: "$reaction-1", + origin_server_ts: 123, + content: { + "m.relates_to": { + rel_type: "m.annotation", + event_id: "$approval-msg", + key: "✅", + }, + }, + } as never, + senderId: "@owner:example.org", + senderLabel: "Owner", + selfUserId: "@bot:example.org", + isDirectMessage: false, + logVerboseMessage: vi.fn(), + }); + + expect(client.getEvent).not.toHaveBeenCalled(); + expect(resolveMatrixExecApproval).toHaveBeenCalledWith({ + cfg: buildConfig(), + approvalId: "req-123", + decision: "allow-once", + senderId: "@owner:example.org", + }); + expect(core.system.enqueueSystemEvent).not.toHaveBeenCalled(); + }); + + it("unregisters stale approval anchors after not-found resolution", async () => { + const core = buildCore(); + resolveMatrixExecApproval.mockRejectedValueOnce( + new Error("unknown or expired approval id req-123"), + ); + registerMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + approvalId: "req-123", + allowedDecisions: ["deny"], + }); + const client = { + getEvent: vi.fn(), + } as unknown as Parameters[0]["client"]; + + await handleInboundMatrixReaction({ + client, + core, + cfg: buildConfig(), + accountId: "default", + roomId: "!ops:example.org", + event: { + event_id: "$reaction-1", + origin_server_ts: 123, + content: { + "m.relates_to": { + rel_type: "m.annotation", + event_id: "$approval-msg", + key: "❌", + }, + }, + } as never, + senderId: "@owner:example.org", + senderLabel: "Owner", + selfUserId: "@bot:example.org", + isDirectMessage: false, + logVerboseMessage: vi.fn(), + }); + + expect(client.getEvent).not.toHaveBeenCalled(); + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "❌", + }), + ).toBeNull(); + }); + + it("skips target fetches for ordinary reactions when notifications are off", async () => { + const core = buildCore(); + const cfg = buildConfig(); + const matrixCfg = cfg.channels?.matrix; + if (!matrixCfg) { + throw new Error("matrix config missing"); + } + matrixCfg.reactionNotifications = "off"; + const client = { + getEvent: vi.fn(), + } as unknown as Parameters[0]["client"]; + + await handleInboundMatrixReaction({ + client, + core, + cfg, + accountId: "default", + roomId: "!ops:example.org", + event: { + event_id: "$reaction-1", + origin_server_ts: 123, + content: { + "m.relates_to": { + rel_type: "m.annotation", + event_id: "$msg-1", + key: "👍", + }, + }, + } as never, + senderId: "@owner:example.org", + senderLabel: "Owner", + selfUserId: "@bot:example.org", + isDirectMessage: false, + logVerboseMessage: vi.fn(), + }); + + expect(client.getEvent).not.toHaveBeenCalled(); + expect(resolveMatrixExecApproval).not.toHaveBeenCalled(); + expect(core.system.enqueueSystemEvent).not.toHaveBeenCalled(); + }); +}); diff --git a/extensions/matrix/src/matrix/monitor/reaction-events.ts b/extensions/matrix/src/matrix/monitor/reaction-events.ts index 013c0741adc..f475f9ef975 100644 --- a/extensions/matrix/src/matrix/monitor/reaction-events.ts +++ b/extensions/matrix/src/matrix/monitor/reaction-events.ts @@ -1,4 +1,13 @@ import { getSessionBindingService } from "openclaw/plugin-sdk/conversation-runtime"; +import { + resolveMatrixApprovalReactionTarget, + unregisterMatrixApprovalReactionTarget, +} from "../../approval-reactions.js"; +import { + isApprovalNotFoundError, + resolveMatrixExecApproval, +} from "../../exec-approval-resolver.js"; +import { isMatrixExecApprovalAuthorizedSender } from "../../exec-approvals.js"; import type { CoreConfig } from "../../types.js"; import { resolveMatrixAccountConfig } from "../account-config.js"; import { extractMatrixReactionAnnotation } from "../reaction-common.js"; @@ -22,6 +31,56 @@ export function resolveMatrixReactionNotificationMode(params: { return accountConfig.reactionNotifications ?? matrixConfig?.reactionNotifications ?? "own"; } +async function maybeResolveMatrixApprovalReaction(params: { + cfg: CoreConfig; + accountId: string; + senderId: string; + target: ReturnType; + targetEventId: string; + roomId: string; + logVerboseMessage: (message: string) => void; +}): Promise { + if (!params.target) { + return false; + } + if ( + !isMatrixExecApprovalAuthorizedSender({ + cfg: params.cfg, + accountId: params.accountId, + senderId: params.senderId, + }) + ) { + return false; + } + try { + await resolveMatrixExecApproval({ + cfg: params.cfg, + approvalId: params.target.approvalId, + decision: params.target.decision, + senderId: params.senderId, + }); + params.logVerboseMessage( + `matrix: approval reaction resolved id=${params.target.approvalId} sender=${params.senderId} decision=${params.target.decision}`, + ); + return true; + } catch (err) { + if (isApprovalNotFoundError(err)) { + unregisterMatrixApprovalReactionTarget({ + roomId: params.roomId, + eventId: params.targetEventId, + }); + params.logVerboseMessage( + `matrix: approval reaction ignored for expired approval id=${params.target.approvalId} sender=${params.senderId}`, + ); + return true; + } + params.logVerboseMessage( + `matrix: approval reaction failed id=${params.target.approvalId} sender=${params.senderId}: ${String(err)}`, + ); + return true; + } +} + export async function handleInboundMatrixReaction(params: { client: MatrixClient; core: PluginRuntime; @@ -35,6 +94,31 @@ export async function handleInboundMatrixReaction(params: { isDirectMessage: boolean; logVerboseMessage: (message: string) => void; }): Promise { + const reaction = extractMatrixReactionAnnotation(params.event.content); + if (!reaction?.eventId) { + return; + } + if (params.senderId === params.selfUserId) { + return; + } + const approvalTarget = resolveMatrixApprovalReactionTarget({ + roomId: params.roomId, + eventId: reaction.eventId, + reactionKey: reaction.key, + }); + if ( + await maybeResolveMatrixApprovalReaction({ + cfg: params.cfg, + accountId: params.accountId, + senderId: params.senderId, + target: approvalTarget, + targetEventId: reaction.eventId, + roomId: params.roomId, + logVerboseMessage: params.logVerboseMessage, + }) + ) { + return; + } const notificationMode = resolveMatrixReactionNotificationMode({ cfg: params.cfg, accountId: params.accountId, @@ -43,11 +127,6 @@ export async function handleInboundMatrixReaction(params: { return; } - const reaction = extractMatrixReactionAnnotation(params.event.content); - if (!reaction?.eventId) { - return; - } - const targetEvent = await params.client.getEvent(params.roomId, reaction.eventId).catch((err) => { params.logVerboseMessage( `matrix: failed resolving reaction target room=${params.roomId} id=${reaction.eventId}: ${String(err)}`, diff --git a/extensions/matrix/src/matrix/send.test.ts b/extensions/matrix/src/matrix/send.test.ts index 3cac9f4b56c..1275b7766cd 100644 --- a/extensions/matrix/src/matrix/send.test.ts +++ b/extensions/matrix/src/matrix/send.test.ts @@ -29,6 +29,7 @@ const resolveTextChunkLimitMock = vi.fn< >(() => 4000); const resolveMarkdownTableModeMock = vi.fn(() => "code"); const convertMarkdownTablesMock = vi.fn((text: string) => text); +const chunkMarkdownTextWithModeMock = vi.fn((text: string) => (text ? [text] : [])); vi.mock("./outbound-media-runtime.js", () => ({ loadOutboundMediaFromUrl: loadOutboundMediaFromUrlMock, @@ -52,7 +53,7 @@ const runtimeStub = { resolveTextChunkLimitMock(cfg, channel, accountId), resolveChunkMode: () => "length", chunkMarkdownText: (text: string) => (text ? [text] : []), - chunkMarkdownTextWithMode: (text: string) => (text ? [text] : []), + chunkMarkdownTextWithMode: (text: string) => chunkMarkdownTextWithModeMock(text), resolveMarkdownTableMode: () => resolveMarkdownTableModeMock(), convertMarkdownTables: (text: string) => convertMarkdownTablesMock(text), }, @@ -143,6 +144,9 @@ function resetMatrixSendRuntimeMocks() { resolveTextChunkLimitMock.mockReset().mockReturnValue(4000); resolveMarkdownTableModeMock.mockReset().mockReturnValue("code"); convertMarkdownTablesMock.mockReset().mockImplementation((text: string) => text); + chunkMarkdownTextWithModeMock + .mockReset() + .mockImplementation((text: string) => (text ? [text] : [])); applyMatrixSendRuntimeStub(); } @@ -555,6 +559,28 @@ describe("sendMessageMatrix threads", () => { expect(resolveTextChunkLimitMock).toHaveBeenCalledWith(expect.anything(), "matrix", "ops"); }); + + it("returns ordered event ids for chunked text sends", async () => { + const { client, sendMessage } = makeClient(); + sendMessage + .mockReset() + .mockResolvedValueOnce("$m1") + .mockResolvedValueOnce("$m2") + .mockResolvedValueOnce("$m3"); + convertMarkdownTablesMock.mockImplementation(() => "part1|part2|part3"); + chunkMarkdownTextWithModeMock.mockImplementation((text: string) => text.split("|")); + + const result = await sendMessageMatrix("room:!room:example", "ignored", { + client, + }); + + expect(result).toMatchObject({ + roomId: "!room:example", + primaryMessageId: "$m1", + messageId: "$m3", + messageIds: ["$m1", "$m2", "$m3"], + }); + }); }); describe("sendSingleTextMessageMatrix", () => { diff --git a/extensions/matrix/src/matrix/send.ts b/extensions/matrix/src/matrix/send.ts index 50c3b73a763..4fddfde2806 100644 --- a/extensions/matrix/src/matrix/send.ts +++ b/extensions/matrix/src/matrix/send.ts @@ -202,6 +202,7 @@ export async function sendMessageMatrix( return eventId; }; + const messageIds: string[] = []; let lastMessageId = ""; if (opts.mediaUrl) { const maxBytes = resolveMediaMaxBytes(opts.accountId, cfg); @@ -259,6 +260,9 @@ export async function sendMessageMatrix( }); const eventId = await sendContent(content); lastMessageId = eventId ?? lastMessageId; + if (eventId) { + messageIds.push(eventId); + } const textChunks = useVoice ? chunks : rest; // Voice messages use a generic media body ("Voice message"), so keep any // transcript follow-up attached to the same reply/thread context. @@ -276,6 +280,9 @@ export async function sendMessageMatrix( }); const followupEventId = await sendContent(followup); lastMessageId = followupEventId ?? lastMessageId; + if (followupEventId) { + messageIds.push(followupEventId); + } } } else { for (const chunk of chunks.length ? chunks : [""]) { @@ -291,12 +298,17 @@ export async function sendMessageMatrix( }); const eventId = await sendContent(content); lastMessageId = eventId ?? lastMessageId; + if (eventId) { + messageIds.push(eventId); + } } } return { messageId: lastMessageId || "unknown", roomId, + primaryMessageId: messageIds[0] ?? (lastMessageId || "unknown"), + messageIds, }; }, ); @@ -423,6 +435,8 @@ export async function sendSingleTextMessageMatrix( return { messageId: eventId ?? "unknown", roomId: resolvedRoom, + primaryMessageId: eventId ?? "unknown", + messageIds: eventId ? [eventId] : [], }; }, ); diff --git a/extensions/matrix/src/matrix/send/types.ts b/extensions/matrix/src/matrix/send/types.ts index 5d63d42dd93..05919f4b1b9 100644 --- a/extensions/matrix/src/matrix/send/types.ts +++ b/extensions/matrix/src/matrix/send/types.ts @@ -82,6 +82,8 @@ export type ReactionEventContent = MatrixReactionEventContent; export type MatrixSendResult = { messageId: string; roomId: string; + primaryMessageId?: string; + messageIds?: string[]; }; export type MatrixSendOpts = {