diff --git a/CHANGELOG.md b/CHANGELOG.md index 77d89bb8eff..38ab71353fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,7 @@ Docs: https://docs.openclaw.ai - Auth/failover: persist selected fallback overrides before retrying, shorten `auth_permanent` lockouts, and refresh websocket/shared-auth sessions only when real auth changes occur so retries and secret rotations behave predictably. (#60404, #60323, #60387) Thanks @extrasmall0 and @mappel-nv. - CLI/Commander: preserve Commander-computed exit codes for argument and help-error paths, and cover the user-argv parse mode in the regression tests so invalid CLI invocations no longer report success when exits are intercepted. (#60923) Thanks @Linux2010. - CLI/skills JSON: route `skills list --json`, `skills info --json`, and `skills check --json` output to stdout instead of stderr so machine-readable consumers receive JSON on the expected stream again. (#60914; fixes #57599; landed from contributor PR #57611 by @Aftabbs) Thanks @Aftabbs. +<<<<<<< HEAD - Config/All Settings: keep the raw config view intact when sensitive fields are blank instead of corrupting or dropping the rendered snapshot. (#28214) Thanks @solodmd. - Control UI/avatar: honor `ui.assistant.avatar` when serving `/avatar/:agentId` so Appearance UI avatar paths stop falling back to initials placeholders. (#60778) Thanks @hannasdev. - Control UI/chat: add a per-session thinking-level picker in the chat header and mobile chat settings, and keep the browser bundle on UI-local thinking/session-key helpers so Safari no longer crashes on Node-only imports before rendering chat controls. @@ -175,6 +176,7 @@ Docs: https://docs.openclaw.ai - WhatsApp: restore `channels.whatsapp.blockStreaming` and reset watchdog timeouts after reconnect so quiet chats stop falling into reconnect loops. (#60007, #60069) Thanks @MonkeyLeeT and @mcaxtr. - 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. +- 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. - 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. ## 2026.4.2 diff --git a/docs/channels/matrix.md b/docs/channels/matrix.md index a0fcf13e2af..543cff92b42 100644 --- a/docs/channels/matrix.md +++ b/docs/channels/matrix.md @@ -19,16 +19,6 @@ packaged builds do not need a separate install. If you are on an older build or a custom install that excludes Matrix, install it manually: -## Exec approvals - -When `channels.matrix.execApprovals.enabled` is on, Matrix approval prompts seed reaction shortcuts on the pending approval message: - -- `✅` = allow once -- `❌` = deny -- `♾️` = allow always when that decision is allowed by the effective exec policy - -The original `/approve ...` commands stay in the message as a fallback. - Install from npm: ```bash @@ -689,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/extensions/matrix/src/approval-reactions.test.ts b/extensions/matrix/src/approval-reactions.test.ts index d9ec9588a4d..cd2d9846e37 100644 --- a/extensions/matrix/src/approval-reactions.test.ts +++ b/extensions/matrix/src/approval-reactions.test.ts @@ -1,10 +1,17 @@ -import { describe, expect, it } from "vitest"; +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([ @@ -20,73 +27,81 @@ describe("matrix approval reactions", () => { ); }); - it("resolves a reaction back to the approval decision exposed in the prompt text", () => { - const text = [ - "Approval required.", - "", - "Run:", - "```txt", - "/approve req-123 allow-once", - "```", - "", - "Other options:", - "```txt", - "/approve req-123 allow-always", - "/approve req-123 deny", - "```", - ].join("\n"); + 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(text, "✅")).toEqual({ + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "✅", + }), + ).toEqual({ approvalId: "req-123", decision: "allow-once", }); - expect(resolveMatrixApprovalReactionTarget(text, "♾️")).toEqual({ + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "♾️", + }), + ).toEqual({ approvalId: "req-123", decision: "allow-always", }); - expect(resolveMatrixApprovalReactionTarget(text, "❌")).toEqual({ + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "❌", + }), + ).toEqual({ approvalId: "req-123", decision: "deny", }); }); - it("ignores reactions that are not available in the prompt text", () => { - const text = [ - "Approval required.", - "", - "Run:", - "```txt", - "/approve req-123 allow-once", - "```", - "", - "Other options:", - "```txt", - "/approve req-123 deny", - "```", - ].join("\n"); + 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(text, "♾️")).toBeNull(); + expect( + resolveMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", + reactionKey: "♾️", + }), + ).toBeNull(); }); - it("reuses the shared command parser for mention and legacy alias forms", () => { - const text = [ - "Approval required.", - "", - "Run:", - "```txt", - "/approve@claw req-123 allow-once", - "```", - "", - "Other options:", - "```txt", - "/approve req-123 always", - "/approve req-123 deny", - "```", - ].join("\n"); - - expect(resolveMatrixApprovalReactionTarget(text, "♾️")).toEqual({ + it("stops resolving reactions after the approval anchor event is unregistered", () => { + registerMatrixApprovalReactionTarget({ + roomId: "!ops:example.org", + eventId: "$approval-msg", approvalId: "req-123", - decision: "allow-always", + 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 index 6d5425be2fd..03b8c68cfeb 100644 --- a/extensions/matrix/src/approval-reactions.ts +++ b/extensions/matrix/src/approval-reactions.ts @@ -1,24 +1,5 @@ import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-runtime"; -const APPROVE_COMMAND_REGEX = - /\/approve(?:@[^\s]+)?\s+([A-Za-z0-9][A-Za-z0-9._:-]*)\s+(allow-once|allow-always|always|deny)\b/i; - -function parseExecApprovalCommandText( - raw: string, -): { approvalId: string; decision: ExecApprovalReplyDecision } | null { - const trimmed = raw.trim(); - const match = trimmed.match(APPROVE_COMMAND_REGEX); - if (!match) { - return null; - } - const rawDecision = (match[2] ?? "").toLowerCase(); - return { - approvalId: match[1] ?? "", - decision: - rawDecision === "always" ? "allow-always" : (rawDecision as ExecApprovalReplyDecision), - }; -} - const MATRIX_APPROVAL_REACTION_META = { "allow-once": { emoji: "✅", @@ -51,6 +32,22 @@ export type MatrixApprovalReactionResolution = { 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[] { @@ -94,32 +91,68 @@ export function resolveMatrixApprovalReactionDecision( return null; } -export function resolveMatrixApprovalReactionTarget( - messageText: string, - reactionKey: string, -): MatrixApprovalReactionResolution | null { - const allowedDecisions = new Set(); - let approvalId: string | null = null; - for (const line of messageText.split(/\r?\n/)) { - const parsed = parseExecApprovalCommandText(line); - if (!parsed) { - continue; - } - if (approvalId && approvalId !== parsed.approvalId) { - return null; - } - approvalId = parsed.approvalId; - allowedDecisions.add(parsed.decision); +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; } - if (!approvalId || allowedDecisions.size === 0) { + 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 decision = resolveMatrixApprovalReactionDecision(reactionKey, Array.from(allowedDecisions)); + const target = matrixApprovalReactionTargets.get(key); + if (!target) { + return null; + } + const decision = resolveMatrixApprovalReactionDecision( + params.reactionKey, + target.allowedDecisions, + ); if (!decision) { return null; } return { - approvalId, + approvalId: target.approvalId, decision, }; } + +export function clearMatrixApprovalReactionTargetsForTest(): void { + matrixApprovalReactionTargets.clear(); +} diff --git a/extensions/matrix/src/exec-approvals-handler.test.ts b/extensions/matrix/src/exec-approvals-handler.test.ts index 3c521916669..20e2e2427f7 100644 --- a/extensions/matrix/src/exec-approvals-handler.test.ts +++ b/extensions/matrix/src/exec-approvals-handler.test.ts @@ -1,5 +1,6 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { clearMatrixApprovalReactionTargetsForTest } from "./approval-reactions.js"; import { MatrixExecApprovalHandler } from "./exec-approvals-handler.js"; const baseRequest = { @@ -57,6 +58,7 @@ function createHandler(cfg: OpenClawConfig, accountId = "default") { afterEach(() => { vi.useRealTimers(); + clearMatrixApprovalReactionTargetsForTest(); }); describe("MatrixExecApprovalHandler", () => { @@ -303,6 +305,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 = { @@ -334,6 +392,50 @@ 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("honors request decision constraints in pending approval text", async () => { const cfg = { channels: { diff --git a/extensions/matrix/src/exec-approvals-handler.ts b/extensions/matrix/src/exec-approvals-handler.ts index 341b03a13b0..57eea6fc8b4 100644 --- a/extensions/matrix/src/exec-approvals-handler.ts +++ b/extensions/matrix/src/exec-approvals-handler.ts @@ -15,6 +15,8 @@ import { matrixNativeApprovalAdapter } from "./approval-native.js"; import { buildMatrixApprovalReactionHint, listMatrixApprovalReactionBindings, + registerMatrixApprovalReactionTarget, + unregisterMatrixApprovalReactionTarget, } from "./approval-reactions.js"; import { isMatrixExecApprovalClientEnabled, @@ -32,7 +34,8 @@ type ApprovalRequest = ExecApprovalRequest; type ApprovalResolved = ExecApprovalResolved; type PendingMessage = { roomId: string; - messageId: string; + messageIds: readonly string[]; + reactionEventId: string; }; type PreparedMatrixTarget = { @@ -41,6 +44,7 @@ type PreparedMatrixTarget = { threadId?: string; }; type PendingApprovalContent = { + approvalId: string; text: string; allowedDecisions: readonly ("allow-once" | "allow-always" | "deny")[]; }; @@ -61,6 +65,10 @@ export type MatrixExecApprovalHandlerDeps = { repairDirectRooms?: typeof repairMatrixDirectRooms; }; +function normalizePendingMessageIds(entry: PendingMessage): string[] { + return Array.from(new Set(entry.messageIds.map((messageId) => messageId.trim()).filter(Boolean))); +} + function isHandlerConfigured(params: { cfg: OpenClawConfig; accountId: string }): boolean { return isMatrixExecApprovalClientEnabled(params); } @@ -96,6 +104,7 @@ function buildPendingApprovalContent(params: { const hint = buildMatrixApprovalReactionHint(allowedDecisions); const text = payload.text ?? ""; return { + approvalId: params.request.id, text: hint ? `${text}\n\n${hint}` : text, allowedDecisions, }; @@ -191,10 +200,25 @@ export class MatrixExecApprovalHandler { 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(); + registerMatrixApprovalReactionTarget({ + 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, result.messageId, emoji, { + await this.reactMessage(result.roomId, reactionEventId, emoji, { cfg: this.opts.cfg as CoreConfig, accountId: this.opts.accountId, client: this.opts.client, @@ -204,7 +228,8 @@ export class MatrixExecApprovalHandler { ); return { roomId: result.roomId, - messageId: result.messageId, + messageIds, + reactionEventId, }; }, finalizeResolved: async ({ request, resolved, entries }) => { @@ -275,11 +300,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, + unregisterMatrixApprovalReactionTarget({ + 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", + }); + }), + ]); }), ); } @@ -287,12 +330,20 @@ 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", + unregisterMatrixApprovalReactionTarget({ + 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", + }); + }), + ); }), ); } diff --git a/extensions/matrix/src/matrix/monitor/reaction-events.test.ts b/extensions/matrix/src/matrix/monitor/reaction-events.test.ts index 6b43fa31474..41cc38ed662 100644 --- a/extensions/matrix/src/matrix/monitor/reaction-events.test.ts +++ b/extensions/matrix/src/matrix/monitor/reaction-events.test.ts @@ -1,4 +1,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + clearMatrixApprovalReactionTargetsForTest, + registerMatrixApprovalReactionTarget, +} from "../../approval-reactions.js"; +import type { CoreConfig } from "../../types.js"; import { handleInboundMatrixReaction } from "./reaction-events.js"; const resolveMatrixExecApproval = vi.fn(); @@ -11,9 +16,10 @@ vi.mock("../../exec-approval-resolver.js", () => ({ beforeEach(() => { resolveMatrixExecApproval.mockReset(); + clearMatrixApprovalReactionTargetsForTest(); }); -function buildConfig() { +function buildConfig(): CoreConfig { return { channels: { matrix: { @@ -28,7 +34,7 @@ function buildConfig() { }, }, }, - }; + } as CoreConfig; } function buildCore() { @@ -52,26 +58,17 @@ function buildCore() { 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 required.", - "", - "Run:", - "```txt", - "/approve req-123 allow-once", - "```", - "", - "Other options:", - "```txt", - "/approve req-123 allow-always", - "/approve req-123 deny", - "```", - ].join("\n"), - }, + content: { body: "approval prompt" }, }), } as unknown as Parameters[0]["client"]; @@ -156,14 +153,22 @@ describe("matrix approval reactions", () => { it("still resolves approval reactions when generic reaction notifications are off", async () => { const core = buildCore(); const cfg = buildConfig(); - cfg.channels.matrix.reactionNotifications = "off"; + 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: "/approve req-123 deny", - }, + content: { body: "approval prompt" }, }), } as unknown as Parameters[0]["client"]; diff --git a/extensions/matrix/src/matrix/monitor/reaction-events.ts b/extensions/matrix/src/matrix/monitor/reaction-events.ts index a714cb9b6a6..c4edfbdc6a8 100644 --- a/extensions/matrix/src/matrix/monitor/reaction-events.ts +++ b/extensions/matrix/src/matrix/monitor/reaction-events.ts @@ -28,30 +28,13 @@ export function resolveMatrixReactionNotificationMode(params: { return accountConfig.reactionNotifications ?? matrixConfig?.reactionNotifications ?? "own"; } -function readTargetEventText(event: MatrixRawEvent | null): string { - if (!event?.content || typeof event.content !== "object") { - return ""; - } - const content = event.content as { - body?: unknown; - "m.new_content"?: { - body?: unknown; - }; - }; - const body = - typeof content.body === "string" - ? content.body - : typeof content["m.new_content"]?.body === "string" - ? content["m.new_content"].body - : ""; - return body.trim(); -} - async function maybeResolveMatrixApprovalReaction(params: { cfg: CoreConfig; accountId: string; senderId: string; + roomId: string; reactionKey: string; + targetEventId: string; targetEvent: MatrixRawEvent | null; targetSender: string; selfUserId: string; @@ -69,10 +52,11 @@ async function maybeResolveMatrixApprovalReaction(params: { ) { return false; } - const target = resolveMatrixApprovalReactionTarget( - readTargetEventText(params.targetEvent), - params.reactionKey, - ); + const target = resolveMatrixApprovalReactionTarget({ + roomId: params.roomId, + eventId: params.targetEventId, + reactionKey: params.reactionKey, + }); if (!target) { return false; } @@ -138,7 +122,9 @@ export async function handleInboundMatrixReaction(params: { cfg: params.cfg, accountId: params.accountId, senderId: params.senderId, + roomId: params.roomId, reactionKey: reaction.key, + targetEventId: reaction.eventId, targetEvent: targetEvent as MatrixRawEvent | null, targetSender, selfUserId: params.selfUserId, 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 = {