nyrn
/
openclaw
mirror of https://github.com/openclaw/openclaw.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

matrix: tighten approval reaction cleanup

This commit is contained in:
Gustavo Madeira Santana 2026-04-04 12:42:28 -04:00
parent 32883da6ad
commit 1cf2c4bad9
4 changed files with 269 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
extensions/matrix/src/exec-approvals-handler.test.ts
View File

@ -1,6 +1,9 @@
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
import { afterEach, describe, expect, it, vi } from "vitest";
import { clearMatrixApprovalReactionTargetsForTest } from "./approval-reactions.js";
import {
clearMatrixApprovalReactionTargetsForTest,
resolveMatrixApprovalReactionTarget,
} from "./approval-reactions.js";
import { MatrixExecApprovalHandler } from "./exec-approvals-handler.js";
const baseRequest = {
@ -436,6 +439,35 @@ describe("MatrixExecApprovalHandler", () => {
);
});
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: {

69
extensions/matrix/src/exec-approvals-handler.ts
View File

@ -1,5 +1,6 @@
import {
buildExecApprovalPendingReplyPayload,
type ExecApprovalReplyDecision,
getExecApprovalApproverDmNoticeText,
resolveExecApprovalAllowedDecisions,
resolveExecApprovalCommandDisplay,
@ -46,7 +47,11 @@ type PreparedMatrixTarget = {
type PendingApprovalContent = {
approvalId: string;
text: string;
allowedDecisions: readonly ("allow-once" | "allow-always" | "deny")[];
allowedDecisions: readonly ExecApprovalReplyDecision[];
};
type ReactionTargetRef = {
roomId: string;
eventId: string;
};
export type MatrixExecApprovalHandlerOpts = {
@ -69,6 +74,23 @@ 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);
}
@ -126,6 +148,7 @@ function buildResolvedApprovalText(params: {
export class MatrixExecApprovalHandler {
private readonly runtime: ExecApprovalChannelRuntime<ApprovalRequest, ApprovalResolved>;
private readonly trackedReactionTargets = new Map<string, ReactionTargetRef>();
private readonly nowMs: () => number;
private readonly sendMessage: typeof sendMessageMatrix;
private readonly reactMessage: typeof reactMatrixMessage;
@ -209,7 +232,7 @@ export class MatrixExecApprovalHandler {
);
const reactionEventId =
result.primaryMessageId?.trim() || messageIds[0] || result.messageId.trim();
registerMatrixApprovalReactionTarget({
this.trackReactionTarget({
roomId: result.roomId,
eventId: reactionEventId,
approvalId: pendingContent.approvalId,
@ -247,6 +270,7 @@ export class MatrixExecApprovalHandler {
async stop(): Promise<void> {
await this.runtime.stop();
this.clearTrackedReactionTargets();
}
async handleRequested(request: ApprovalRequest): Promise<void> {
@ -300,7 +324,7 @@ export class MatrixExecApprovalHandler {
const text = buildResolvedApprovalText({ request, resolved });
await Promise.allSettled(
entries.map(async (entry) => {
unregisterMatrixApprovalReactionTarget({
this.untrackReactionTarget({
roomId: entry.roomId,
eventId: entry.reactionEventId,
});
@ -330,7 +354,7 @@ export class MatrixExecApprovalHandler {
private async clearPending(entries: PendingMessage[]): Promise<void> {
await Promise.allSettled(
entries.map(async (entry) => {
unregisterMatrixApprovalReactionTarget({
this.untrackReactionTarget({
roomId: entry.roomId,
eventId: entry.reactionEventId,
});
@ -347,4 +371,41 @@ export class MatrixExecApprovalHandler {
}),
);
}
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();
}
}

137
extensions/matrix/src/matrix/monitor/reaction-events.test.ts
View File

@ -2,6 +2,7 @@ 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";
@ -204,4 +205,140 @@ describe("matrix approval reactions", () => {
});
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<typeof handleInboundMatrixReaction>[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<typeof handleInboundMatrixReaction>[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<typeof handleInboundMatrixReaction>[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();
});
});

70
extensions/matrix/src/matrix/monitor/reaction-events.ts
View File

@ -1,5 +1,8 @@
import { getSessionBindingService } from "openclaw/plugin-sdk/conversation-runtime";
import { resolveMatrixApprovalReactionTarget } from "../../approval-reactions.js";
import {
resolveMatrixApprovalReactionTarget,
unregisterMatrixApprovalReactionTarget,
} from "../../approval-reactions.js";
import {
isApprovalNotFoundError,
resolveMatrixExecApproval,
@ -32,15 +35,12 @@ async function maybeResolveMatrixApprovalReaction(params: {
cfg: CoreConfig;
accountId: string;
senderId: string;
roomId: string;
reactionKey: string;
target: ReturnType<typeof resolveMatrixApprovalReactionTarget>;
targetEventId: string;
targetEvent: MatrixRawEvent | null;
targetSender: string;
selfUserId: string;
roomId: string;
logVerboseMessage: (message: string) => void;
}): Promise<boolean> {
if (params.targetSender !== params.selfUserId) {
if (!params.target) {
return false;
}
if (
@ -52,34 +52,30 @@ async function maybeResolveMatrixApprovalReaction(params: {
) {
return false;
}
const target = resolveMatrixApprovalReactionTarget({
roomId: params.roomId,
eventId: params.targetEventId,
reactionKey: params.reactionKey,
});
if (!target) {
return false;
}
try {
await resolveMatrixExecApproval({
cfg: params.cfg,
approvalId: target.approvalId,
decision: target.decision,
approvalId: params.target.approvalId,
decision: params.target.decision,
senderId: params.senderId,
});
params.logVerboseMessage(
`matrix: approval reaction resolved id=${target.approvalId} sender=${params.senderId} decision=${target.decision}`,
`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=${target.approvalId} sender=${params.senderId}`,
`matrix: approval reaction ignored for expired approval id=${params.target.approvalId} sender=${params.senderId}`,
);
return true;
}
params.logVerboseMessage(
`matrix: approval reaction failed id=${target.approvalId} sender=${params.senderId}: ${String(err)}`,
`matrix: approval reaction failed id=${params.target.approvalId} sender=${params.senderId}: ${String(err)}`,
);
return true;
}
@ -105,29 +101,19 @@ export async function handleInboundMatrixReaction(params: {
if (params.senderId === params.selfUserId) {
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)}`,
);
return null;
const approvalTarget = resolveMatrixApprovalReactionTarget({
roomId: params.roomId,
eventId: reaction.eventId,
reactionKey: reaction.key,
});
const targetSender =
targetEvent && typeof targetEvent.sender === "string" ? targetEvent.sender.trim() : "";
if (!targetSender) {
return;
}
if (
await maybeResolveMatrixApprovalReaction({
cfg: params.cfg,
accountId: params.accountId,
senderId: params.senderId,
roomId: params.roomId,
reactionKey: reaction.key,
target: approvalTarget,
targetEventId: reaction.eventId,
targetEvent: targetEvent as MatrixRawEvent | null,
targetSender,
selfUserId: params.selfUserId,
roomId: params.roomId,
logVerboseMessage: params.logVerboseMessage,
})
) {
@ -140,6 +126,18 @@ export async function handleInboundMatrixReaction(params: {
if (notificationMode === "off") {
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)}`,
);
return null;
});
const targetSender =
targetEvent && typeof targetEvent.sender === "string" ? targetEvent.sender.trim() : "";
if (!targetSender) {
return;
}
if (notificationMode === "own" && targetSender !== params.selfUserId) {
return;
}