refactor: harden outbound, matrix bootstrap, and plugin entry resolution

This commit is contained in:
Peter Steinberger
2026-03-02 19:54:58 +00:00
parent a351ab2481
commit c424836fbe
14 changed files with 194 additions and 65 deletions

View File

@@ -1,6 +1,6 @@
import { LogService } from "@vector-im/matrix-bot-sdk";
import { createMatrixClient } from "./client/create-client.js";
import { startMatrixClientWithGrace } from "./client/startup.js";
import { getMatrixLogService } from "./sdk-runtime.js";
type MatrixClientBootstrapAuth = {
homeserver: string;
@@ -39,6 +39,7 @@ export async function createPreparedMatrixClient(opts: {
await startMatrixClientWithGrace({
client,
onError: (err: unknown) => {
const LogService = getMatrixLogService();
LogService.error("MatrixClientBootstrap", "client.start() error:", err);
},
});

View File

@@ -1,7 +1,7 @@
import { MatrixClient } from "@vector-im/matrix-bot-sdk";
import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id";
import { getMatrixRuntime } from "../../runtime.js";
import type { CoreConfig } from "../../types.js";
import { loadMatrixSdk } from "../sdk-runtime.js";
import { ensureMatrixSdkLoggingConfigured } from "./logging.js";
import type { MatrixAuth, MatrixResolvedConfig } from "./types.js";
@@ -119,6 +119,7 @@ export async function resolveMatrixAuth(params?: {
if (!userId) {
// Fetch userId from access token via whoami
ensureMatrixSdkLoggingConfigured();
const { MatrixClient } = loadMatrixSdk();
const tempClient = new MatrixClient(resolved.homeserver, resolved.accessToken);
const whoami = await tempClient.getUserId();
userId = whoami;

View File

@@ -1,11 +1,10 @@
import fs from "node:fs";
import type { IStorageProvider, ICryptoStorageProvider } from "@vector-im/matrix-bot-sdk";
import {
LogService,
import type {
IStorageProvider,
ICryptoStorageProvider,
MatrixClient,
SimpleFsStorageProvider,
RustSdkCryptoStorageProvider,
} from "@vector-im/matrix-bot-sdk";
import { loadMatrixSdk } from "../sdk-runtime.js";
import { ensureMatrixSdkLoggingConfigured } from "./logging.js";
import {
maybeMigrateLegacyStorage,
@@ -14,6 +13,7 @@ import {
} from "./storage.js";
function sanitizeUserIdList(input: unknown, label: string): string[] {
const LogService = loadMatrixSdk().LogService;
if (input == null) {
return [];
}
@@ -44,6 +44,8 @@ export async function createMatrixClient(params: {
localTimeoutMs?: number;
accountId?: string | null;
}): Promise<MatrixClient> {
const { MatrixClient, SimpleFsStorageProvider, RustSdkCryptoStorageProvider, LogService } =
loadMatrixSdk();
ensureMatrixSdkLoggingConfigured();
const env = process.env;

View File

@@ -1,7 +1,15 @@
import { ConsoleLogger, LogService } from "@vector-im/matrix-bot-sdk";
import { loadMatrixSdk } from "../sdk-runtime.js";
let matrixSdkLoggingConfigured = false;
const matrixSdkBaseLogger = new ConsoleLogger();
let matrixSdkBaseLogger:
| {
trace: (module: string, ...messageOrObject: unknown[]) => void;
debug: (module: string, ...messageOrObject: unknown[]) => void;
info: (module: string, ...messageOrObject: unknown[]) => void;
warn: (module: string, ...messageOrObject: unknown[]) => void;
error: (module: string, ...messageOrObject: unknown[]) => void;
}
| undefined;
function shouldSuppressMatrixHttpNotFound(module: string, messageOrObject: unknown[]): boolean {
if (module !== "MatrixHttpClient") {
@@ -19,18 +27,20 @@ export function ensureMatrixSdkLoggingConfigured(): void {
if (matrixSdkLoggingConfigured) {
return;
}
const { ConsoleLogger, LogService } = loadMatrixSdk();
matrixSdkBaseLogger = new ConsoleLogger();
matrixSdkLoggingConfigured = true;
LogService.setLogger({
trace: (module, ...messageOrObject) => matrixSdkBaseLogger.trace(module, ...messageOrObject),
debug: (module, ...messageOrObject) => matrixSdkBaseLogger.debug(module, ...messageOrObject),
info: (module, ...messageOrObject) => matrixSdkBaseLogger.info(module, ...messageOrObject),
warn: (module, ...messageOrObject) => matrixSdkBaseLogger.warn(module, ...messageOrObject),
trace: (module, ...messageOrObject) => matrixSdkBaseLogger?.trace(module, ...messageOrObject),
debug: (module, ...messageOrObject) => matrixSdkBaseLogger?.debug(module, ...messageOrObject),
info: (module, ...messageOrObject) => matrixSdkBaseLogger?.info(module, ...messageOrObject),
warn: (module, ...messageOrObject) => matrixSdkBaseLogger?.warn(module, ...messageOrObject),
error: (module, ...messageOrObject) => {
if (shouldSuppressMatrixHttpNotFound(module, messageOrObject)) {
return;
}
matrixSdkBaseLogger.error(module, ...messageOrObject);
matrixSdkBaseLogger?.error(module, ...messageOrObject);
},
});
}

View File

@@ -1,7 +1,7 @@
import type { MatrixClient } from "@vector-im/matrix-bot-sdk";
import { LogService } from "@vector-im/matrix-bot-sdk";
import { normalizeAccountId } from "openclaw/plugin-sdk/account-id";
import type { CoreConfig } from "../../types.js";
import { getMatrixLogService } from "../sdk-runtime.js";
import { resolveMatrixAuth } from "./config.js";
import { createMatrixClient } from "./create-client.js";
import { startMatrixClientWithGrace } from "./startup.js";
@@ -81,6 +81,7 @@ async function ensureSharedClientStarted(params: {
params.state.cryptoReady = true;
}
} catch (err) {
const LogService = getMatrixLogService();
LogService.warn("MatrixClientLite", "Failed to prepare crypto:", err);
}
}
@@ -89,6 +90,7 @@ async function ensureSharedClientStarted(params: {
client,
onError: (err: unknown) => {
params.state.started = false;
const LogService = getMatrixLogService();
LogService.error("MatrixClientLite", "client.start() error:", err);
},
});

View File

@@ -1,8 +1,8 @@
import type { MatrixClient } from "@vector-im/matrix-bot-sdk";
import { AutojoinRoomsMixin } from "@vector-im/matrix-bot-sdk";
import type { RuntimeEnv } from "openclaw/plugin-sdk";
import { getMatrixRuntime } from "../../runtime.js";
import type { CoreConfig } from "../../types.js";
import { loadMatrixSdk } from "../sdk-runtime.js";
export function registerMatrixAutoJoin(params: {
client: MatrixClient;
@@ -26,6 +26,7 @@ export function registerMatrixAutoJoin(params: {
if (autoJoin === "always") {
// Use the built-in autojoin mixin for "always" mode
const { AutojoinRoomsMixin } = loadMatrixSdk();
AutojoinRoomsMixin.setupOnClient(client);
logVerbose("matrix: auto-join enabled for all invites");
return;

View File

@@ -0,0 +1,18 @@
import { createRequire } from "node:module";
type MatrixSdkRuntime = typeof import("@vector-im/matrix-bot-sdk");
let cachedMatrixSdkRuntime: MatrixSdkRuntime | null = null;
export function loadMatrixSdk(): MatrixSdkRuntime {
if (cachedMatrixSdkRuntime) {
return cachedMatrixSdkRuntime;
}
const req = createRequire(import.meta.url);
cachedMatrixSdkRuntime = req("@vector-im/matrix-bot-sdk") as MatrixSdkRuntime;
return cachedMatrixSdkRuntime;
}
export function getMatrixLogService() {
return loadMatrixSdk().LogService;
}

View File

@@ -855,6 +855,39 @@ describe("deliverOutboundPayloads", () => {
);
});
it("preserves channelData-only payloads with empty text for non-WhatsApp sendPayload channels", async () => {
const sendPayload = vi.fn().mockResolvedValue({ channel: "line", messageId: "ln-1" });
const sendText = vi.fn();
const sendMedia = vi.fn();
setActivePluginRegistry(
createTestRegistry([
{
pluginId: "line",
source: "test",
plugin: createOutboundTestPlugin({
id: "line",
outbound: { deliveryMode: "direct", sendPayload, sendText, sendMedia },
}),
},
]),
);
const results = await deliverOutboundPayloads({
cfg: {},
channel: "line",
to: "U123",
payloads: [{ text: " \n\t ", channelData: { mode: "flex" } }],
});
expect(sendPayload).toHaveBeenCalledTimes(1);
expect(sendPayload).toHaveBeenCalledWith(
expect.objectContaining({
payload: expect.objectContaining({ text: "", channelData: { mode: "flex" } }),
}),
);
expect(results).toEqual([{ channel: "line", messageId: "ln-1" }]);
});
it("emits message_sent failure when delivery errors", async () => {
hookMocks.runner.hasHooks.mockReturnValue(true);
const sendWhatsApp = vi.fn().mockRejectedValue(new Error("downstream failed"));

View File

@@ -428,12 +428,21 @@ async function deliverOutboundPayloadsCore(
})),
};
};
const normalizeWhatsAppPayload = (payload: ReplyPayload): ReplyPayload | null => {
const hasMedia = Boolean(payload.mediaUrl) || (payload.mediaUrls?.length ?? 0) > 0;
const hasMediaPayload = (payload: ReplyPayload): boolean =>
Boolean(payload.mediaUrl) || (payload.mediaUrls?.length ?? 0) > 0;
const hasChannelDataPayload = (payload: ReplyPayload): boolean =>
Boolean(payload.channelData && Object.keys(payload.channelData).length > 0);
const normalizePayloadForChannelDelivery = (
payload: ReplyPayload,
channelId: string,
): ReplyPayload | null => {
const hasMedia = hasMediaPayload(payload);
const hasChannelData = hasChannelDataPayload(payload);
const rawText = typeof payload.text === "string" ? payload.text : "";
const normalizedText = rawText.replace(/^(?:[ \t]*\r?\n)+/, "");
const normalizedText =
channelId === "whatsapp" ? rawText.replace(/^(?:[ \t]*\r?\n)+/, "") : rawText;
if (!normalizedText.trim()) {
if (!hasMedia) {
if (!hasMedia && !hasChannelData) {
return null;
}
return {
@@ -441,25 +450,14 @@ async function deliverOutboundPayloadsCore(
text: "",
};
}
if (normalizedText === rawText) {
return payload;
}
return {
...payload,
text: normalizedText,
};
};
const normalizeEmptyTextPayload = (payload: ReplyPayload): ReplyPayload | null => {
const hasMedia = Boolean(payload.mediaUrl) || (payload.mediaUrls?.length ?? 0) > 0;
const rawText = typeof payload.text === "string" ? payload.text : "";
if (!rawText.trim()) {
if (!hasMedia) {
return null;
}
return {
...payload,
text: "",
};
}
return payload;
};
const normalizedPayloads = normalizeReplyPayloadsForDelivery(payloads)
.map((payload) => {
// Strip HTML tags for plain-text surfaces (WhatsApp, Signal, etc.)
@@ -475,10 +473,7 @@ async function deliverOutboundPayloadsCore(
return { ...payload, text: sanitizeForPlainText(payload.text) };
})
.flatMap((payload) => {
const normalized =
channel === "whatsapp"
? normalizeWhatsAppPayload(payload)
: normalizeEmptyTextPayload(payload);
const normalized = normalizePayloadForChannelDelivery(payload, channel);
return normalized ? [normalized] : [];
});
const hookRunner = getGlobalHookRunner();

View File

@@ -4,7 +4,9 @@ import { openBoundaryFileSync } from "../infra/boundary-file-read.js";
import { resolveConfigDir, resolveUserPath } from "../utils.js";
import { resolveBundledPluginsDir } from "./bundled-dir.js";
import {
DEFAULT_PLUGIN_ENTRY_CANDIDATES,
getPackageManifestMetadata,
resolvePackageExtensionEntries,
type OpenClawPackageManifest,
type PackageManifest,
} from "./manifest.js";
@@ -243,14 +245,6 @@ function readPackageManifest(dir: string): PackageManifest | null {
}
}
function resolvePackageExtensions(manifest: PackageManifest): string[] {
const raw = getPackageManifestMetadata(manifest)?.extensions;
if (!Array.isArray(raw)) {
return [];
}
return raw.map((entry) => (typeof entry === "string" ? entry.trim() : "")).filter(Boolean);
}
function deriveIdHint(params: {
filePath: string;
packageName?: string;
@@ -394,7 +388,8 @@ function discoverInDirectory(params: {
}
const manifest = readPackageManifest(fullPath);
const extensions = manifest ? resolvePackageExtensions(manifest) : [];
const extensionResolution = resolvePackageExtensionEntries(manifest ?? undefined);
const extensions = extensionResolution.status === "ok" ? extensionResolution.entries : [];
if (extensions.length > 0) {
for (const extPath of extensions) {
@@ -428,8 +423,7 @@ function discoverInDirectory(params: {
continue;
}
const indexCandidates = ["index.ts", "index.js", "index.mjs", "index.cjs"];
const indexFile = indexCandidates
const indexFile = [...DEFAULT_PLUGIN_ENTRY_CANDIDATES]
.map((candidate) => path.join(fullPath, candidate))
.find((candidate) => fs.existsSync(candidate));
if (indexFile && isExtensionFile(indexFile)) {
@@ -495,7 +489,8 @@ function discoverFromPath(params: {
if (stat.isDirectory()) {
const manifest = readPackageManifest(resolved);
const extensions = manifest ? resolvePackageExtensions(manifest) : [];
const extensionResolution = resolvePackageExtensionEntries(manifest ?? undefined);
const extensions = extensionResolution.status === "ok" ? extensionResolution.entries : [];
if (extensions.length > 0) {
for (const extPath of extensions) {
@@ -529,8 +524,7 @@ function discoverFromPath(params: {
return;
}
const indexCandidates = ["index.ts", "index.js", "index.mjs", "index.cjs"];
const indexFile = indexCandidates
const indexFile = [...DEFAULT_PLUGIN_ENTRY_CANDIDATES]
.map((candidate) => path.join(resolved, candidate))
.find((candidate) => fs.existsSync(candidate));

View File

@@ -1,6 +1,5 @@
import fs from "node:fs/promises";
import path from "node:path";
import { MANIFEST_KEY } from "../compat/legacy-names.js";
import { fileExists, readJsonFile, resolveArchiveKind } from "../infra/archive.js";
import { writeFileFromPathWithinRoot } from "../infra/fs-safe.js";
import { resolveExistingInstallPath, withExtractedArchiveRoot } from "../infra/install-flow.js";
@@ -31,18 +30,20 @@ import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js";
import { extensionUsesSkippedScannerPath, isPathInside } from "../security/scan-paths.js";
import * as skillScanner from "../security/skill-scanner.js";
import { CONFIG_DIR, resolveUserPath } from "../utils.js";
import { loadPluginManifest } from "./manifest.js";
import {
loadPluginManifest,
resolvePackageExtensionEntries,
type PackageManifest as PluginPackageManifest,
} from "./manifest.js";
type PluginInstallLogger = {
info?: (message: string) => void;
warn?: (message: string) => void;
};
type PackageManifest = {
name?: string;
version?: string;
type PackageManifest = PluginPackageManifest & {
dependencies?: Record<string, string>;
} & Partial<Record<typeof MANIFEST_KEY, { extensions?: string[] }>>;
};
const MISSING_EXTENSIONS_ERROR =
'package.json missing openclaw.extensions; update the plugin package to include openclaw.extensions (for example ["./dist/index.js"]). See https://docs.openclaw.ai/help/troubleshooting#plugin-install-fails-with-missing-openclaw-extensions';
@@ -86,15 +87,14 @@ function validatePluginId(pluginId: string): string | null {
}
function ensureOpenClawExtensions(params: { manifest: PackageManifest }): string[] {
const extensions = params.manifest[MANIFEST_KEY]?.extensions;
if (!Array.isArray(extensions)) {
const resolved = resolvePackageExtensionEntries(params.manifest);
if (resolved.status === "missing") {
throw new Error(MISSING_EXTENSIONS_ERROR);
}
const list = extensions.map((e) => (typeof e === "string" ? e.trim() : "")).filter(Boolean);
if (list.length === 0) {
if (resolved.status === "empty") {
throw new Error("package.json openclaw.extensions is empty");
}
return list;
return resolved.entries;
}
function buildFileInstallResult(pluginId: string, targetFile: string): InstallPluginResult {

View File

@@ -148,6 +148,18 @@ export type OpenClawPackageManifest = {
install?: PluginPackageInstall;
};
export const DEFAULT_PLUGIN_ENTRY_CANDIDATES = [
"index.ts",
"index.js",
"index.mjs",
"index.cjs",
] as const;
export type PackageExtensionResolution =
| { status: "ok"; entries: string[] }
| { status: "missing"; entries: [] }
| { status: "empty"; entries: [] };
export type ManifestKey = typeof MANIFEST_KEY;
export type PackageManifest = {
@@ -164,3 +176,19 @@ export function getPackageManifestMetadata(
}
return manifest[MANIFEST_KEY];
}
export function resolvePackageExtensionEntries(
manifest: PackageManifest | undefined,
): PackageExtensionResolution {
const raw = getPackageManifestMetadata(manifest)?.extensions;
if (!Array.isArray(raw)) {
return { status: "missing", entries: [] };
}
const entries = raw
.map((entry) => (typeof entry === "string" ? entry.trim() : ""))
.filter(Boolean);
if (entries.length === 0) {
return { status: "empty", entries: [] };
}
return { status: "ok", entries };
}

View File

@@ -208,6 +208,43 @@ describe("bot-native-command-menu", () => {
expect(runtimeLog).not.toHaveBeenCalledWith("telegram: command menu unchanged; skipping sync");
});
it("does not cache empty-menu hash when deleteMyCommands fails", async () => {
const deleteMyCommands = vi
.fn()
.mockRejectedValueOnce(new Error("transient failure"))
.mockResolvedValue(undefined);
const setMyCommands = vi.fn(async () => undefined);
const runtimeLog = vi.fn();
const accountId = `test-empty-delete-fail-${Date.now()}`;
syncTelegramMenuCommands({
bot: { api: { deleteMyCommands, setMyCommands } } as unknown as Parameters<
typeof syncTelegramMenuCommands
>[0]["bot"],
runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() } as Parameters<
typeof syncTelegramMenuCommands
>[0]["runtime"],
commandsToRegister: [],
accountId,
botIdentity: "bot-a",
});
await vi.waitFor(() => expect(deleteMyCommands).toHaveBeenCalledTimes(1));
syncTelegramMenuCommands({
bot: { api: { deleteMyCommands, setMyCommands } } as unknown as Parameters<
typeof syncTelegramMenuCommands
>[0]["bot"],
runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() } as Parameters<
typeof syncTelegramMenuCommands
>[0]["runtime"],
commandsToRegister: [],
accountId,
botIdentity: "bot-a",
});
await vi.waitFor(() => expect(deleteMyCommands).toHaveBeenCalledTimes(2));
expect(runtimeLog).not.toHaveBeenCalledWith("telegram: command menu unchanged; skipping sync");
});
it("retries with fewer commands on BOT_COMMANDS_TOO_MUCH", async () => {
const deleteMyCommands = vi.fn(async () => undefined);
const setMyCommands = vi

View File

@@ -174,15 +174,22 @@ export function syncTelegramMenuCommands(params: {
}
// Keep delete -> set ordering to avoid stale deletions racing after fresh registrations.
let deleteSucceeded = true;
if (typeof bot.api.deleteMyCommands === "function") {
await withTelegramApiErrorLogging({
deleteSucceeded = await withTelegramApiErrorLogging({
operation: "deleteMyCommands",
runtime,
fn: () => bot.api.deleteMyCommands(),
}).catch(() => {});
})
.then(() => true)
.catch(() => false);
}
if (commandsToRegister.length === 0) {
if (!deleteSucceeded) {
runtime.log?.("telegram: deleteMyCommands failed; skipping empty-menu hash cache write");
return;
}
await writeCachedCommandHash(accountId, botIdentity, currentHash);
return;
}