mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:40:44 +00:00
fix(discord): avoid blocking startup on probe (#77129)
* fix(discord): avoid blocking startup on probe * fix(discord): clear degraded probe status * test(plugin-sdk): isolate jiti loader override * test(plugin-sdk): fix circular facade fixture path * fix(plugins): preserve sdk aliases for native loads * fix(plugins): route sdk alias loads through transform
This commit is contained in:
committed by
GitHub
parent
fa689295c6
commit
605e89468e
@@ -64,6 +64,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Agents/Pi: suppress persistence for synthetic mid-turn overflow continuation prompts, so transcript-retry recovery does not write the "continue from transcript" prompt as a new user turn. Thanks @vincentkoc.
|
||||
- Agents/tools: strip reasoning text from visible rich presentation titles, blocks, buttons, and select labels before message-tool sends, so structured channel payloads cannot leak hidden planning. Thanks @vincentkoc.
|
||||
- Telegram: keep reply-dispatch lazy provider runtime chunks behind stable dist names and delete `/reasoning stream` previews after final delivery so package updates and live reasoning drafts do not leave Telegram turns broken or noisy. Thanks @BunsDev.
|
||||
- Discord: start the gateway monitor without waiting for the startup bot/application probe, so WSL2 hosts with a slow `/users/@me` REST path still bring the channel online while status enrichment finishes asynchronously. Fixes #77103. Thanks @Suited78.
|
||||
- Exec approvals: detect `env -S` split-string command-carrier risks when `-S`/`-s` is combined with other env short options, so approval explanations do not miss split payloads hidden behind `env -iS...`. Thanks @vincentkoc.
|
||||
- Google Meet: log the concrete agent-mode TTS provider, model, voice, output format, and sample rate after speech synthesis, so Meet logs show which voice backend spoke each reply.
|
||||
- Voice Call: mark realtime calls completed when the realtime provider closes normally, so Twilio/OpenAI/Google realtime stop events do not leave active call records behind. Thanks @vincentkoc.
|
||||
|
||||
@@ -379,7 +379,7 @@ describe("discordPlugin outbound", () => {
|
||||
expect(runtimeProbeDiscord).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses direct Discord startup helpers before monitoring", async () => {
|
||||
it("uses direct Discord startup helpers for async startup enrichment", async () => {
|
||||
const runtimeProbeDiscord = vi.fn(async () => {
|
||||
throw new Error("runtime Discord probe should not be used");
|
||||
});
|
||||
@@ -407,9 +407,11 @@ describe("discordPlugin outbound", () => {
|
||||
const cfg = createCfg();
|
||||
await startDiscordAccount(cfg);
|
||||
|
||||
expect(probeDiscordMock).toHaveBeenCalledWith("discord-token", 2500, {
|
||||
includeApplication: true,
|
||||
});
|
||||
await vi.waitFor(() =>
|
||||
expect(probeDiscordMock).toHaveBeenCalledWith("discord-token", 2500, {
|
||||
includeApplication: true,
|
||||
}),
|
||||
);
|
||||
expect(monitorDiscordProviderMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
token: "discord-token",
|
||||
@@ -421,6 +423,98 @@ describe("discordPlugin outbound", () => {
|
||||
expect(runtimeMonitorDiscordProvider).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not block Discord monitor startup on the startup probe", async () => {
|
||||
let resolveProbe!: (value: {
|
||||
ok: true;
|
||||
bot: { username: string };
|
||||
application: { intents: { messageContent: "limited" } };
|
||||
elapsedMs: number;
|
||||
}) => void;
|
||||
probeDiscordMock.mockReturnValue(
|
||||
new Promise((resolve) => {
|
||||
resolveProbe = resolve;
|
||||
}),
|
||||
);
|
||||
monitorDiscordProviderMock.mockResolvedValue(undefined);
|
||||
|
||||
const cfg = createCfg();
|
||||
const statusPatches: Array<Record<string, unknown>> = [];
|
||||
const ctx = createStartAccountContext({
|
||||
account: resolveAccount(cfg),
|
||||
cfg,
|
||||
statusPatchSink: (next) => statusPatches.push({ ...next }),
|
||||
});
|
||||
|
||||
await discordPlugin.gateway!.startAccount!(ctx);
|
||||
|
||||
expect(monitorDiscordProviderMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
token: "discord-token",
|
||||
accountId: "default",
|
||||
}),
|
||||
);
|
||||
await vi.waitFor(() =>
|
||||
expect(probeDiscordMock).toHaveBeenCalledWith("discord-token", 2500, {
|
||||
includeApplication: true,
|
||||
}),
|
||||
);
|
||||
expect(statusPatches.some((patch) => "bot" in patch || "application" in patch)).toBe(false);
|
||||
|
||||
resolveProbe({
|
||||
ok: true,
|
||||
bot: { username: "AsyncBob" },
|
||||
application: { intents: { messageContent: "limited" } },
|
||||
elapsedMs: 1,
|
||||
});
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(
|
||||
statusPatches.some(
|
||||
(patch) =>
|
||||
(patch.bot as { username?: string } | undefined)?.username === "AsyncBob" &&
|
||||
Boolean(patch.application),
|
||||
),
|
||||
).toBe(true),
|
||||
);
|
||||
});
|
||||
|
||||
it("clears stale Discord probe metadata when the async startup probe degrades", async () => {
|
||||
probeDiscordMock.mockResolvedValue({
|
||||
ok: false,
|
||||
status: 401,
|
||||
error: "getMe failed (401)",
|
||||
elapsedMs: 1,
|
||||
});
|
||||
monitorDiscordProviderMock.mockResolvedValue(undefined);
|
||||
|
||||
const cfg = createCfg();
|
||||
const statusPatches: Array<Record<string, unknown>> = [];
|
||||
const ctx = createStartAccountContext({
|
||||
account: resolveAccount(cfg),
|
||||
cfg,
|
||||
statusPatchSink: (next) => statusPatches.push({ ...next }),
|
||||
});
|
||||
ctx.setStatus({
|
||||
accountId: "default",
|
||||
bot: { username: "OldBot" },
|
||||
application: { intents: { messageContent: "enabled" } },
|
||||
});
|
||||
|
||||
await discordPlugin.gateway!.startAccount!(ctx);
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(
|
||||
statusPatches.some(
|
||||
(patch) =>
|
||||
"bot" in patch &&
|
||||
"application" in patch &&
|
||||
patch.bot === undefined &&
|
||||
patch.application === undefined,
|
||||
),
|
||||
).toBe(true),
|
||||
);
|
||||
});
|
||||
|
||||
it("stagger starts later accounts in multi-bot setups", async () => {
|
||||
probeDiscordMock.mockResolvedValue({
|
||||
ok: true,
|
||||
|
||||
@@ -82,6 +82,61 @@ import { parseDiscordTarget } from "./target-parsing.js";
|
||||
const REQUIRED_DISCORD_PERMISSIONS = ["ViewChannel", "SendMessages"] as const;
|
||||
const DISCORD_ACCOUNT_STARTUP_STAGGER_MS = 10_000;
|
||||
|
||||
function startDiscordStartupProbe(params: {
|
||||
accountId: string;
|
||||
token: string;
|
||||
abortSignal: AbortSignal;
|
||||
setStatus: (patch: { accountId: string; bot?: unknown; application?: unknown }) => void;
|
||||
log?: {
|
||||
warn?: (msg: string) => void;
|
||||
info?: (msg: string) => void;
|
||||
debug?: (msg: string) => void;
|
||||
};
|
||||
}): void {
|
||||
void (async () => {
|
||||
try {
|
||||
const probe = await (
|
||||
await loadDiscordProbeRuntime()
|
||||
).probeDiscord(params.token, 2500, {
|
||||
includeApplication: true,
|
||||
});
|
||||
if (params.abortSignal.aborted) {
|
||||
return;
|
||||
}
|
||||
params.setStatus({
|
||||
accountId: params.accountId,
|
||||
bot: probe.bot,
|
||||
application: probe.application,
|
||||
});
|
||||
if (probe.ok) {
|
||||
const username = probe.bot?.username?.trim();
|
||||
if (username) {
|
||||
params.log?.info?.(`[${params.accountId}] Discord bot probe resolved @${username}`);
|
||||
}
|
||||
} else if (getDiscordRuntime().logging.shouldLogVerbose()) {
|
||||
params.log?.debug?.(
|
||||
`[${params.accountId}] bot probe degraded: ${probe.error ?? `status ${probe.status ?? "unknown"}`}`,
|
||||
);
|
||||
}
|
||||
|
||||
const messageContent = probe.application?.intents?.messageContent;
|
||||
if (messageContent === "disabled") {
|
||||
params.log?.warn?.(
|
||||
`[${params.accountId}] Discord Message Content Intent is disabled; bot may not respond to channel messages. Enable it in Discord Dev Portal (Bot → Privileged Gateway Intents) or require mentions.`,
|
||||
);
|
||||
} else if (messageContent === "limited") {
|
||||
params.log?.info?.(
|
||||
`[${params.accountId}] Discord Message Content Intent is limited; bots under 100 servers can use it without verification.`,
|
||||
);
|
||||
}
|
||||
} catch (err) {
|
||||
if (getDiscordRuntime().logging.shouldLogVerbose()) {
|
||||
params.log?.debug?.(`[${params.accountId}] bot probe failed: ${String(err)}`);
|
||||
}
|
||||
}
|
||||
})();
|
||||
}
|
||||
|
||||
function shouldTreatDiscordDeliveredTextAsVisible(params: {
|
||||
kind: "tool" | "block" | "final";
|
||||
text?: string;
|
||||
@@ -551,38 +606,14 @@ export const discordPlugin: ChannelPlugin<ResolvedDiscordAccount, DiscordProbe>
|
||||
}
|
||||
}
|
||||
const token = account.token.trim();
|
||||
let discordBotLabel = "";
|
||||
try {
|
||||
const probe = await (
|
||||
await loadDiscordProbeRuntime()
|
||||
).probeDiscord(token, 2500, {
|
||||
includeApplication: true,
|
||||
});
|
||||
const username = probe.ok ? probe.bot?.username?.trim() : null;
|
||||
if (username) {
|
||||
discordBotLabel = ` (@${username})`;
|
||||
}
|
||||
ctx.setStatus({
|
||||
accountId: account.accountId,
|
||||
bot: probe.bot,
|
||||
application: probe.application,
|
||||
});
|
||||
const messageContent = probe.application?.intents?.messageContent;
|
||||
if (messageContent === "disabled") {
|
||||
ctx.log?.warn(
|
||||
`[${account.accountId}] Discord Message Content Intent is disabled; bot may not respond to channel messages. Enable it in Discord Dev Portal (Bot → Privileged Gateway Intents) or require mentions.`,
|
||||
);
|
||||
} else if (messageContent === "limited") {
|
||||
ctx.log?.info(
|
||||
`[${account.accountId}] Discord Message Content Intent is limited; bots under 100 servers can use it without verification.`,
|
||||
);
|
||||
}
|
||||
} catch (err) {
|
||||
if (getDiscordRuntime().logging.shouldLogVerbose()) {
|
||||
ctx.log?.debug?.(`[${account.accountId}] bot probe failed: ${String(err)}`);
|
||||
}
|
||||
}
|
||||
ctx.log?.info(`[${account.accountId}] starting provider${discordBotLabel}`);
|
||||
startDiscordStartupProbe({
|
||||
accountId: account.accountId,
|
||||
token,
|
||||
abortSignal: ctx.abortSignal,
|
||||
setStatus: ctx.setStatus,
|
||||
log: ctx.log,
|
||||
});
|
||||
ctx.log?.info(`[${account.accountId}] starting provider`);
|
||||
return (await loadDiscordProviderRuntime()).monitorDiscordProvider({
|
||||
token,
|
||||
accountId: account.accountId,
|
||||
|
||||
@@ -421,6 +421,9 @@ describe("loadBundledEntryExportSync", () => {
|
||||
});
|
||||
|
||||
it("can disable source-tree fallback for dist bundled entry checks", () => {
|
||||
stubPluginModuleLoaderJitiFactory(
|
||||
vi.fn(() => vi.fn(() => ({ sentinel: 42 }))) as unknown as PluginModuleLoaderFactory,
|
||||
);
|
||||
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-channel-entry-contract-"));
|
||||
tempDirs.push(tempRoot);
|
||||
|
||||
|
||||
@@ -140,7 +140,7 @@ function createCircularPluginFixture(prefix: string): TrustedBundledPluginFixtur
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(pluginRoot, "helper.js"),
|
||||
['import { marker } from "../facade.mjs";', "export const circularMarker = marker;", ""].join(
|
||||
['import { marker } from "./facade.mjs";', "export const circularMarker = marker;", ""].join(
|
||||
"\n",
|
||||
),
|
||||
"utf8",
|
||||
|
||||
@@ -99,7 +99,6 @@ import {
|
||||
restoreMemoryPluginState,
|
||||
} from "./memory-state.js";
|
||||
import { unwrapDefaultModuleExport } from "./module-export.js";
|
||||
import { tryNativeRequireJavaScriptModule } from "./native-module-require.js";
|
||||
import {
|
||||
fingerprintPluginDiscoveryContext,
|
||||
resolvePluginDiscoveryContext,
|
||||
@@ -107,7 +106,7 @@ import {
|
||||
import { withProfile } from "./plugin-load-profile.js";
|
||||
import {
|
||||
createPluginModuleLoaderCache,
|
||||
getCachedPluginSourceModuleLoader,
|
||||
getCachedPluginModuleLoader,
|
||||
type PluginModuleLoaderCache,
|
||||
} from "./plugin-module-loader-cache.js";
|
||||
import type { PluginOrigin } from "./plugin-origin.types.js";
|
||||
@@ -480,8 +479,8 @@ function runPluginRegisterSync(
|
||||
|
||||
function createPluginModuleLoader(options: Pick<PluginLoadOptions, "pluginSdkResolution">) {
|
||||
const moduleLoaders: PluginModuleLoaderCache = createPluginModuleLoaderCache();
|
||||
const loadSourceModule = (modulePath: string) => {
|
||||
return getCachedPluginSourceModuleLoader({
|
||||
const createLoaderForModule = (modulePath: string) => {
|
||||
return getCachedPluginModuleLoader({
|
||||
cache: moduleLoaders,
|
||||
modulePath,
|
||||
importerUrl: import.meta.url,
|
||||
@@ -495,18 +494,8 @@ function createPluginModuleLoader(options: Pick<PluginLoadOptions, "pluginSdkRes
|
||||
pluginSdkResolution: options.pluginSdkResolution,
|
||||
});
|
||||
};
|
||||
return (modulePath: string): unknown => {
|
||||
if (shouldPreferNativeModuleLoad(modulePath)) {
|
||||
const native = tryNativeRequireJavaScriptModule(modulePath, { allowWindows: true });
|
||||
if (native.ok) {
|
||||
return native.moduleExport;
|
||||
}
|
||||
}
|
||||
// Source .ts runtime shims import sibling ".js" specifiers that only exist
|
||||
// after build. Jiti remains the dev/source fallback because it rewrites those
|
||||
// imports against the source graph and applies SDK aliases.
|
||||
return loadSourceModule(modulePath)(toSafeImportPath(modulePath));
|
||||
};
|
||||
return (modulePath: string): unknown =>
|
||||
createLoaderForModule(modulePath)(toSafeImportPath(modulePath));
|
||||
}
|
||||
|
||||
function resolveCanonicalDistRuntimeSource(source: string): string {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import fs from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import type { createJiti } from "jiti";
|
||||
import { toSafeImportPath } from "../shared/import-specifier.js";
|
||||
@@ -47,6 +48,8 @@ export type PluginModuleLoaderStatsSnapshot = {
|
||||
const DEFAULT_PLUGIN_MODULE_LOADER_CACHE_ENTRIES = 128;
|
||||
const MAX_TRACKED_SOURCE_TRANSFORM_TARGETS = 24;
|
||||
const JITI_FACTORY_OVERRIDE_KEY = Symbol.for("openclaw.pluginModuleLoaderJitiFactoryOverride");
|
||||
const PLUGIN_SDK_IMPORT_SPECIFIER_PATTERN =
|
||||
/(?:\bfrom\s*["']|\bimport\s*\(\s*["']|\brequire\s*\(\s*["'])(?:openclaw|@openclaw)\/plugin-sdk(?:\/[^"']*)?["']/u;
|
||||
const requireForJiti = createRequire(import.meta.url);
|
||||
let createJitiLoaderFactory: PluginModuleLoaderFactory | undefined;
|
||||
const pluginModuleLoaderStats = {
|
||||
@@ -213,6 +216,29 @@ function createLazySourceTransformLoader(params: {
|
||||
};
|
||||
}
|
||||
|
||||
function shouldForceSourceTransformForPluginSdkAlias(params: {
|
||||
target: string;
|
||||
aliasMap: Record<string, string>;
|
||||
}): boolean {
|
||||
if (
|
||||
!params.aliasMap["openclaw/plugin-sdk"] &&
|
||||
!params.aliasMap["@openclaw/plugin-sdk"] &&
|
||||
!Object.keys(params.aliasMap).some(
|
||||
(key) => key.startsWith("openclaw/plugin-sdk/") || key.startsWith("@openclaw/plugin-sdk/"),
|
||||
)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
if (!/\.[cm]?js$/iu.test(params.target)) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
return PLUGIN_SDK_IMPORT_SPECIFIER_PATTERN.test(fs.readFileSync(params.target, "utf-8"));
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function createPluginModuleLoader(params: {
|
||||
loaderFilename: string;
|
||||
aliasMap: Record<string, string>;
|
||||
@@ -242,8 +268,20 @@ function createPluginModuleLoader(params: {
|
||||
// for TS / TSX sources and for the small set of require(esm) /
|
||||
// async-module fallbacks `tryNativeRequireJavaScriptModule` declines to
|
||||
// handle.
|
||||
const getLoadWithAliasTransform = createLazySourceTransformLoader({
|
||||
...params,
|
||||
tryNative: false,
|
||||
});
|
||||
return ((target: string, ...rest: unknown[]) => {
|
||||
pluginModuleLoaderStats.calls += 1;
|
||||
if (shouldForceSourceTransformForPluginSdkAlias({ target, aliasMap: params.aliasMap })) {
|
||||
pluginModuleLoaderStats.sourceTransformForced += 1;
|
||||
recordSourceTransformTarget(target);
|
||||
return (getLoadWithAliasTransform() as (t: string, ...a: unknown[]) => unknown)(
|
||||
target,
|
||||
...rest,
|
||||
);
|
||||
}
|
||||
const native = tryNativeRequireJavaScriptModule(target, { allowWindows: true });
|
||||
if (native.ok) {
|
||||
pluginModuleLoaderStats.nativeHits += 1;
|
||||
|
||||
@@ -10,7 +10,8 @@ function writeRuntimeModuleWrapper(sourcePath: string, targetPath: string): void
|
||||
const relative = `./${path.relative(path.dirname(targetPath), sourcePath).split(path.sep).join("/")}`;
|
||||
const content = [
|
||||
`export * from ${JSON.stringify(relative)};`,
|
||||
`export { default } from ${JSON.stringify(relative)};`,
|
||||
`import * as moduleExports from ${JSON.stringify(relative)};`,
|
||||
`export default moduleExports.default ?? moduleExports;`,
|
||||
"",
|
||||
].join("\n");
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user