Matrix: tighten fallback resolution and ACP lookup

This commit is contained in:
Gustavo Madeira Santana 2026-03-12 10:01:38 +00:00
parent a7edb677b0
commit ee0568c62e
No known key found for this signature in database
9 changed files with 183 additions and 21 deletions

View File

@ -30,10 +30,19 @@ describe("matrix client storage paths", () => {
}
});
function setupStateDir(): string {
function setupStateDir(
cfg: Record<string, unknown> = {
channels: {
matrix: {},
},
},
): string {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-matrix-storage-"));
tempDirs.push(dir);
setMatrixRuntime({
config: {
loadConfig: () => cfg,
},
logging: {
getChildLogger: () => ({
info: () => {},
@ -157,6 +166,74 @@ describe("matrix client storage paths", () => {
expect(fs.existsSync(path.join(legacyRoot, "crypto"))).toBe(true);
});
it("refuses fallback migration when multiple Matrix accounts need explicit selection", async () => {
const stateDir = setupStateDir({
channels: {
matrix: {
accounts: {
ops: {},
work: {},
},
},
},
});
const storagePaths = resolveMatrixStoragePaths({
homeserver: "https://matrix.example.org",
userId: "@bot:example.org",
accessToken: "secret-token",
accountId: "ops",
env: {},
});
const legacyRoot = path.join(stateDir, "matrix");
fs.mkdirSync(path.join(legacyRoot, "crypto"), { recursive: true });
fs.writeFileSync(path.join(legacyRoot, "bot-storage.json"), '{"legacy":true}');
await expect(
maybeMigrateLegacyStorage({
storagePaths,
env: {},
}),
).rejects.toThrow(/defaultAccount is not set/i);
expect(maybeCreateMatrixMigrationSnapshotMock).not.toHaveBeenCalled();
expect(fs.existsSync(path.join(legacyRoot, "bot-storage.json"))).toBe(true);
});
it("refuses fallback migration for a non-selected Matrix account", async () => {
const stateDir = setupStateDir({
channels: {
matrix: {
defaultAccount: "ops",
homeserver: "https://matrix.default.example.org",
accessToken: "default-token",
accounts: {
ops: {
homeserver: "https://matrix.ops.example.org",
accessToken: "ops-token",
},
},
},
},
});
const storagePaths = resolveMatrixStoragePaths({
homeserver: "https://matrix.default.example.org",
userId: "@default:example.org",
accessToken: "default-token",
env: {},
});
const legacyRoot = path.join(stateDir, "matrix");
fs.mkdirSync(path.join(legacyRoot, "crypto"), { recursive: true });
fs.writeFileSync(path.join(legacyRoot, "bot-storage.json"), '{"legacy":true}');
await expect(
maybeMigrateLegacyStorage({
storagePaths,
env: {},
}),
).rejects.toThrow(/targets account "ops"/i);
expect(maybeCreateMatrixMigrationSnapshotMock).not.toHaveBeenCalled();
expect(fs.existsSync(path.join(legacyRoot, "bot-storage.json"))).toBe(true);
});
it("reuses an existing token-hash storage root after the access token changes", () => {
const stateDir = setupStateDir();
const oldStoragePaths = resolveMatrixStoragePaths({

View File

@ -3,7 +3,10 @@ import os from "node:os";
import path from "node:path";
import {
maybeCreateMatrixMigrationSnapshot,
normalizeAccountId,
requiresExplicitMatrixDefaultAccount,
resolveMatrixAccountStorageRoot,
resolveMatrixDefaultOrOnlyAccountId,
resolveMatrixLegacyFlatStoragePaths,
} from "openclaw/plugin-sdk/matrix";
import { getMatrixRuntime } from "../../runtime.js";
@ -31,6 +34,26 @@ function resolveLegacyStoragePaths(env: NodeJS.ProcessEnv = process.env): {
return { storagePath: legacy.storagePath, cryptoPath: legacy.cryptoPath };
}
function assertLegacyMigrationAccountSelection(params: { accountKey: string }): void {
const cfg = getMatrixRuntime().config.loadConfig();
if (!cfg.channels?.matrix || typeof cfg.channels.matrix !== "object") {
return;
}
if (requiresExplicitMatrixDefaultAccount(cfg)) {
throw new Error(
"Legacy Matrix client storage cannot be migrated automatically because multiple Matrix accounts are configured and channels.matrix.defaultAccount is not set.",
);
}
const selectedAccountId = normalizeAccountId(resolveMatrixDefaultOrOnlyAccountId(cfg));
const currentAccountId = normalizeAccountId(params.accountKey);
if (selectedAccountId !== currentAccountId) {
throw new Error(
`Legacy Matrix client storage targets account "${selectedAccountId}", but the current client is starting account "${currentAccountId}". Start the selected account first so flat legacy storage is not migrated into the wrong account directory.`,
);
}
}
function scoreStorageRoot(rootDir: string): number {
let score = 0;
if (fs.existsSync(path.join(rootDir, "bot-storage.json"))) {
@ -175,6 +198,10 @@ export async function maybeMigrateLegacyStorage(params: {
return;
}
assertLegacyMigrationAccountSelection({
accountKey: params.storagePaths.accountKey,
});
const logger = getMatrixRuntime().logging.getChildLogger({ module: "matrix-storage" });
await maybeCreateMatrixMigrationSnapshot({
trigger: "matrix-client-fallback",

View File

@ -121,4 +121,31 @@ describe("updateMatrixAccountConfig", () => {
});
expect(updated.channels?.["matrix"]?.accounts?.ops?.rooms).toBeUndefined();
});
it("reuses and canonicalizes non-normalized account entries when updating", () => {
const cfg = {
channels: {
matrix: {
accounts: {
Ops: {
homeserver: "https://matrix.ops.example.org",
accessToken: "ops-token",
},
},
},
},
} as CoreConfig;
const updated = updateMatrixAccountConfig(cfg, "ops", {
deviceName: "Ops Bot",
});
expect(updated.channels?.["matrix"]?.accounts?.Ops).toBeUndefined();
expect(updated.channels?.["matrix"]?.accounts?.ops).toMatchObject({
homeserver: "https://matrix.ops.example.org",
accessToken: "ops-token",
deviceName: "Ops Bot",
enabled: true,
});
});
});

View File

@ -1,6 +1,7 @@
import { DEFAULT_ACCOUNT_ID } from "openclaw/plugin-sdk/account-id";
import { normalizeAccountId } from "openclaw/plugin-sdk/matrix";
import type { CoreConfig, MatrixConfig } from "../types.js";
import { findMatrixAccountConfig } from "./account-config.js";
export type MatrixAccountPatch = {
name?: string | null;
@ -113,7 +114,7 @@ export function updateMatrixAccountConfig(
): CoreConfig {
const matrix = cfg.channels?.matrix ?? {};
const normalizedAccountId = normalizeAccountId(accountId);
const existingAccount = (matrix.accounts?.[normalizedAccountId] ??
const existingAccount = (findMatrixAccountConfig(cfg, normalizedAccountId) ??
(normalizedAccountId === DEFAULT_ACCOUNT_ID ? matrix : {})) as MatrixConfig;
const nextAccount: Record<string, unknown> = { ...existingAccount };
@ -191,6 +192,14 @@ export function updateMatrixAccountConfig(
}
}
const nextAccounts = Object.fromEntries(
Object.entries(matrix.accounts ?? {}).filter(
([rawAccountId]) =>
rawAccountId === normalizedAccountId ||
normalizeAccountId(rawAccountId) !== normalizedAccountId,
),
);
if (shouldStoreMatrixAccountAtTopLevel(cfg, normalizedAccountId)) {
const { accounts: _ignoredAccounts, defaultAccount, ...baseMatrix } = matrix;
return {
@ -215,7 +224,7 @@ export function updateMatrixAccountConfig(
...matrix,
enabled: true,
accounts: {
...matrix.accounts,
...nextAccounts,
[normalizedAccountId]: nextAccount as MatrixConfig,
},
},

View File

@ -122,6 +122,31 @@ describe("registerMatrixAutoJoin", () => {
expect(joinRoom).toHaveBeenCalledWith("!room:example.org");
});
it("retries alias resolution after an unresolved lookup", async () => {
const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub();
resolveRoom.mockResolvedValueOnce(null).mockResolvedValueOnce("!room:example.org");
registerMatrixAutoJoin({
client,
accountConfig: {
autoJoin: "allowlist",
autoJoinAllowlist: ["#allowed:example.org"],
},
runtime: {
log: vi.fn(),
error: vi.fn(),
} as unknown as import("openclaw/plugin-sdk/matrix").RuntimeEnv,
});
const inviteHandler = getInviteHandler();
expect(inviteHandler).toBeTruthy();
await inviteHandler!("!room:example.org", {});
await inviteHandler!("!room:example.org", {});
expect(resolveRoom).toHaveBeenCalledTimes(2);
expect(joinRoom).toHaveBeenCalledWith("!room:example.org");
});
it("does not trust room-provided alias claims for allowlist joins", async () => {
const { client, getInviteHandler, joinRoom, resolveRoom } = createClientStub();
resolveRoom.mockResolvedValue("!different-room:example.org");

View File

@ -23,7 +23,7 @@ export function registerMatrixAutoJoin(params: {
const autoJoinAllowlist = new Set(rawAllowlist);
const allowedRoomIds = new Set(rawAllowlist.filter((entry) => entry.startsWith("!")));
const allowedAliases = rawAllowlist.filter((entry) => entry.startsWith("#"));
const resolvedAliasRoomIds = new Map<string, string | null>();
const resolvedAliasRoomIds = new Map<string, string>();
if (autoJoin === "off") {
return;
@ -40,7 +40,9 @@ export function registerMatrixAutoJoin(params: {
return resolvedAliasRoomIds.get(alias) ?? null;
}
const resolved = await params.client.resolveRoom(alias);
resolvedAliasRoomIds.set(alias, resolved);
if (resolved) {
resolvedAliasRoomIds.set(alias, resolved);
}
return resolved;
};

View File

@ -83,11 +83,6 @@ function normalizeMatrixThreadTarget(raw: string): string | undefined {
return normalized || undefined;
}
function normalizeMatrixDirectUserTarget(raw: string): string | undefined {
const normalized = normalizeMatrixThreadTarget(raw);
return normalized?.startsWith("@") ? normalized : undefined;
}
export function resolveMatrixAutoThreadId(params: {
to: string;
toolContext?: ChannelThreadingToolContext;
@ -101,15 +96,11 @@ export function resolveMatrixAutoThreadId(params: {
if (!target || !currentChannel) {
return undefined;
}
// Matrix user:@ targets resolve to a DM room at send time, which can differ
// from the current room after DM recreation or stale m.direct ordering.
// Only auto-thread when the explicit room target already matches.
if (target.toLowerCase() !== currentChannel.toLowerCase()) {
const directTarget = normalizeMatrixDirectUserTarget(params.to);
const currentDirectUserId = normalizeMatrixDirectUserTarget(context.currentDirectUserId ?? "");
if (!directTarget || !currentDirectUserId) {
return undefined;
}
if (directTarget.toLowerCase() !== currentDirectUserId.toLowerCase()) {
return undefined;
}
return undefined;
}
return context.currentThreadTs;
}

View File

@ -309,7 +309,7 @@ describe("runMessageAction threading auto-injection", () => {
expect(call?.ctx?.params?.threadId).toBe("$explicit");
});
it("injects threadId for matching Matrix dm user target", async () => {
it("skips threadId for Matrix dm user targets until the resolved room matches", async () => {
mockHandledSendAction();
const call = await runThreadingAction({
@ -322,8 +322,8 @@ describe("runMessageAction threading auto-injection", () => {
toolContext: defaultMatrixDmToolContext,
});
expect(call?.threadId).toBe("$thread");
expect(call?.ctx?.params?.threadId).toBe("$thread");
expect(call?.threadId).toBeUndefined();
expect(call?.ctx?.params?.threadId).toBeUndefined();
});
it("skips threadId for different Matrix dm user target", async () => {

View File

@ -115,6 +115,10 @@ export {
resolveMatrixLegacyFlatStoragePaths,
sanitizeMatrixPathSegment,
} from "../infra/matrix-storage-paths.js";
export {
requiresExplicitMatrixDefaultAccount,
resolveMatrixDefaultOrOnlyAccountId,
} from "../infra/matrix-account-selection.js";
export {
hasActionableMatrixMigration,
hasPendingMatrixMigration,