Matrix: fix validated review comments

This commit is contained in:
Gustavo Madeira Santana
2026-03-09 08:18:21 -04:00
parent 3c6efde41d
commit d914d4f4e5
8 changed files with 164 additions and 33 deletions

View File

@@ -166,7 +166,6 @@ authoring plugins:
`openclaw/plugin-sdk/google-gemini-cli-auth`, `openclaw/plugin-sdk/googlechat`,
`openclaw/plugin-sdk/irc`, `openclaw/plugin-sdk/llm-task`,
`openclaw/plugin-sdk/lobster`, `openclaw/plugin-sdk/matrix`,
`openclaw/plugin-sdk/matrix`,
`openclaw/plugin-sdk/mattermost`, `openclaw/plugin-sdk/memory-core`,
`openclaw/plugin-sdk/memory-lancedb`,
`openclaw/plugin-sdk/minimax-portal-auth`,

View File

@@ -1,6 +1,6 @@
{
"name": "@openclaw/matrix",
"version": "2026.3.8",
"version": "2026.3.9",
"description": "OpenClaw Matrix channel plugin",
"type": "module",
"dependencies": {

View File

@@ -83,6 +83,7 @@ describe("matrix credentials storage", () => {
it("migrates legacy matrix credential files on read", async () => {
const stateDir = setupStateDir();
const legacyPath = path.join(stateDir, "credentials", "matrix", "credentials.json");
const currentPath = resolveMatrixCredentialsPath({}, "ops");
fs.mkdirSync(path.dirname(legacyPath), { recursive: true });
fs.writeFileSync(
legacyPath,
@@ -94,23 +95,23 @@ describe("matrix credentials storage", () => {
}),
);
const loaded = loadMatrixCredentials({}, "default");
const loaded = loadMatrixCredentials({}, "ops");
expect(loaded?.accessToken).toBe("legacy-token");
expect(fs.existsSync(legacyPath)).toBe(false);
expect(fs.existsSync(resolveMatrixCredentialsPath({}, "default"))).toBe(true);
expect(fs.existsSync(currentPath)).toBe(true);
});
it("clears both current and legacy credential paths", () => {
const stateDir = setupStateDir();
const currentPath = path.join(stateDir, "credentials", "matrix", "credentials.json");
const currentPath = resolveMatrixCredentialsPath({}, "ops");
const legacyPath = path.join(stateDir, "credentials", "matrix", "credentials.json");
fs.mkdirSync(path.dirname(currentPath), { recursive: true });
fs.mkdirSync(path.dirname(legacyPath), { recursive: true });
fs.writeFileSync(currentPath, "{}");
fs.writeFileSync(legacyPath, "{}");
clearMatrixCredentials({}, "default");
clearMatrixCredentials({}, "ops");
expect(fs.existsSync(currentPath)).toBe(false);
expect(fs.existsSync(legacyPath)).toBe(false);

View File

@@ -1,5 +1,7 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id";
import {
resolveMatrixCredentialsDir as resolveSharedMatrixCredentialsDir,
resolveMatrixCredentialsPath as resolveSharedMatrixCredentialsPath,
@@ -16,11 +18,44 @@ export type MatrixStoredCredentials = {
lastUsedAt?: string;
};
function resolveStateDir(env: NodeJS.ProcessEnv): string {
return getMatrixRuntime().state.resolveStateDir(env, os.homedir);
}
function resolveLegacyMatrixCredentialsPath(env: NodeJS.ProcessEnv): string | null {
return path.join(resolveMatrixCredentialsDir(env), "credentials.json");
}
function resolveLegacyMigrationSourcePath(
env: NodeJS.ProcessEnv,
accountId?: string | null,
): string | null {
const normalized = normalizeAccountId(accountId);
if (normalized === DEFAULT_ACCOUNT_ID) {
return null;
}
const legacyPath = resolveLegacyMatrixCredentialsPath(env);
return legacyPath === resolveMatrixCredentialsPath(env, accountId) ? null : legacyPath;
}
function parseMatrixCredentialsFile(filePath: string): MatrixStoredCredentials | null {
const raw = fs.readFileSync(filePath, "utf-8");
const parsed = JSON.parse(raw) as Partial<MatrixStoredCredentials>;
if (
typeof parsed.homeserver !== "string" ||
typeof parsed.userId !== "string" ||
typeof parsed.accessToken !== "string"
) {
return null;
}
return parsed as MatrixStoredCredentials;
}
export function resolveMatrixCredentialsDir(
env: NodeJS.ProcessEnv = process.env,
stateDir?: string,
): string {
const resolvedStateDir = stateDir ?? getMatrixRuntime().state.resolveStateDir(env, os.homedir);
const resolvedStateDir = stateDir ?? resolveStateDir(env);
return resolveSharedMatrixCredentialsDir(resolvedStateDir);
}
@@ -28,7 +63,7 @@ export function resolveMatrixCredentialsPath(
env: NodeJS.ProcessEnv = process.env,
accountId?: string | null,
): string {
const resolvedStateDir = getMatrixRuntime().state.resolveStateDir(env, os.homedir);
const resolvedStateDir = resolveStateDir(env);
return resolveSharedMatrixCredentialsPath({ stateDir: resolvedStateDir, accountId });
}
@@ -38,19 +73,28 @@ export function loadMatrixCredentials(
): MatrixStoredCredentials | null {
const credPath = resolveMatrixCredentialsPath(env, accountId);
try {
if (!fs.existsSync(credPath)) {
if (fs.existsSync(credPath)) {
return parseMatrixCredentialsFile(credPath);
}
const legacyPath = resolveLegacyMigrationSourcePath(env, accountId);
if (!legacyPath || !fs.existsSync(legacyPath)) {
return null;
}
const raw = fs.readFileSync(credPath, "utf-8");
const parsed = JSON.parse(raw) as Partial<MatrixStoredCredentials>;
if (
typeof parsed.homeserver !== "string" ||
typeof parsed.userId !== "string" ||
typeof parsed.accessToken !== "string"
) {
const parsed = parseMatrixCredentialsFile(legacyPath);
if (!parsed) {
return null;
}
return parsed as MatrixStoredCredentials;
try {
fs.mkdirSync(path.dirname(credPath), { recursive: true });
fs.renameSync(legacyPath, credPath);
} catch {
// Keep returning the legacy credentials even if migration fails.
}
return parsed;
} catch {
return null;
}
@@ -93,13 +137,21 @@ export function clearMatrixCredentials(
env: NodeJS.ProcessEnv = process.env,
accountId?: string | null,
): void {
const credPath = resolveMatrixCredentialsPath(env, accountId);
try {
if (fs.existsSync(credPath)) {
fs.unlinkSync(credPath);
const paths = [
resolveMatrixCredentialsPath(env, accountId),
resolveLegacyMigrationSourcePath(env, accountId),
];
for (const filePath of paths) {
if (!filePath) {
continue;
}
try {
if (fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
}
} catch {
// ignore
}
} catch {
// ignore
}
}

View File

@@ -7,6 +7,8 @@ function createMockClient(params: {
senderDirect?: boolean;
selfDirect?: boolean;
members?: string[];
roomName?: string | null;
roomNameError?: unknown;
}) {
const members = params.members ?? ["@alice:example.org", "@bot:example.org"];
return {
@@ -18,7 +20,13 @@ function createMockClient(params: {
getJoinedRoomMembers: vi.fn().mockResolvedValue(members),
getRoomStateEvent: vi
.fn()
.mockImplementation(async (_roomId: string, _event: string, stateKey: string) => {
.mockImplementation(async (_roomId: string, eventType: string, stateKey: string) => {
if (eventType === "m.room.name") {
if (params.roomNameError) {
throw params.roomNameError;
}
return params.roomName == null ? {} : { name: params.roomName };
}
if (stateKey === "@alice:example.org") {
return { is_direct: params.senderDirect === true };
}
@@ -53,6 +61,31 @@ describe("createDirectRoomTracker", () => {
expect(client.getJoinedRoomMembers).toHaveBeenCalledWith("!room:example.org");
});
it("does not classify named 2-member rooms as DMs from member count alone", async () => {
const tracker = createDirectRoomTracker(createMockClient({ isDm: false, roomName: "Project" }));
await expect(
tracker.isDirectMessage({
roomId: "!room:example.org",
senderId: "@alice:example.org",
}),
).resolves.toBe(false);
});
it("treats missing room names as DM fallback for 2-member rooms", async () => {
const tracker = createDirectRoomTracker(
createMockClient({
isDm: false,
roomNameError: { errcode: "M_NOT_FOUND" },
}),
);
await expect(
tracker.isDirectMessage({
roomId: "!room:example.org",
senderId: "@alice:example.org",
}),
).resolves.toBe(true);
});
it("uses is_direct member flags when present", async () => {
const tracker = createDirectRoomTracker(createMockClient({ senderDirect: true }));
await expect(

View File

@@ -12,6 +12,14 @@ type DirectRoomTrackerOptions = {
const DM_CACHE_TTL_MS = 30_000;
function isMatrixNotFoundError(err: unknown): boolean {
if (typeof err !== "object" || err === null) {
return false;
}
const value = err as { errcode?: string; statusCode?: number };
return value.errcode === "M_NOT_FOUND" || value.statusCode === 404;
}
export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTrackerOptions = {}) {
const log = opts.log ?? (() => {});
let lastDmUpdateMs = 0;
@@ -83,12 +91,6 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
return true;
}
const memberCount = await resolveMemberCount(roomId);
if (memberCount === 2) {
log(`matrix: dm detected via member count room=${roomId} members=${memberCount}`);
return true;
}
const selfUserId = params.selfUserId ?? (await ensureSelfUserId());
const directViaState =
(await hasDirectFlag(roomId, senderId)) || (await hasDirectFlag(roomId, selfUserId ?? ""));
@@ -97,6 +99,25 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
return true;
}
const memberCount = await resolveMemberCount(roomId);
if (memberCount === 2) {
try {
const nameState = await client.getRoomStateEvent(roomId, "m.room.name", "");
if (!nameState?.name?.trim()) {
log(`matrix: dm detected via fallback (2 members, no room name) room=${roomId}`);
return true;
}
} catch (err: unknown) {
if (isMatrixNotFoundError(err)) {
log(`matrix: dm detected via fallback (2 members, no room name) room=${roomId}`);
return true;
}
log(
`matrix: dm fallback skipped (room name check failed: ${String(err)}) room=${roomId}`,
);
}
}
log(`matrix: dm check room=${roomId} result=group members=${memberCount ?? "unknown"}`);
return false;
},

View File

@@ -242,6 +242,28 @@ describe("pairing cli", () => {
});
});
it("uses the approved request accountId for notifications when --account is omitted", async () => {
approveChannelPairingCode.mockResolvedValueOnce({
id: "123",
entry: {
id: "123",
code: "ABCDEFGH",
createdAt: "2026-01-08T00:00:00Z",
lastSeenAt: "2026-01-08T00:00:00Z",
meta: { accountId: "ops" },
},
});
await runPairing(["pairing", "approve", "--channel", "telegram", "--notify", "ABCDEFGH"]);
expect(notifyPairingApproved).toHaveBeenCalledWith({
channelId: "telegram",
id: "123",
cfg: {},
accountId: "ops",
});
});
it("defaults approve to the sole available channel when only code is provided", async () => {
listPairingChannels.mockReturnValueOnce(["slack"]);
mockApprovedPairing();

View File

@@ -166,8 +166,11 @@ export function registerPairingCli(program: Command) {
if (!opts.notify) {
return;
}
await notifyApproved(channel, approved.id, accountId || undefined).catch((err) => {
defaultRuntime.log(theme.warn(`Failed to notify requester: ${String(err)}`));
});
const approvedAccountId = String(approved.entry?.meta?.accountId ?? "").trim();
await notifyApproved(channel, approved.id, accountId || approvedAccountId || undefined).catch(
(err) => {
defaultRuntime.log(theme.warn(`Failed to notify requester: ${String(err)}`));
},
);
});
}