mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:30:42 +00:00
fix(discord): harden account and binding routing
This commit is contained in:
@@ -12,6 +12,8 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Channels/Discord: suppress duplicate gateway monitors when multiple enabled accounts resolve to the same bot token, preferring config tokens over default env fallback and reporting skipped duplicates as disabled. Supersedes #73608. Thanks @kagura-agent.
|
||||
- Channels/Discord: ignore stale route-shaped conversation bindings after a Discord channel is reconfigured to another agent, while preserving explicit focus and subagent bindings. Fixes #73626. Thanks @ramitrkar-hash.
|
||||
- NVIDIA/NIM: persist the `NVIDIA_API_KEY` provider marker and mark bundled NVIDIA Chat Completions models as string-content compatible, so NIM models load from `models.json` and OpenAI-compatible subagent calls send plain text content. Fixes #73013 and #50107; refs #73014. Thanks @bautrey, @iot2edge, @ifearghal, and @futhgar.
|
||||
- Channels/Discord: let text-only configs drop the `GuildVoiceStates` gateway intent and expose a bounded `/gateway/bot` metadata timeout with rate-limited fallback logs, reducing idle CPU and warning floods. Fixes #73709 and #73585. Thanks @sanchezm86 and @trac3r00.
|
||||
- CLI/plugins: use plugin metadata snapshots for install slot selection and add opt-in plugin lifecycle timing traces, so plugin install avoids runtime-loading the plugin registry for metadata-only decisions. Thanks @shakkernerd.
|
||||
|
||||
@@ -176,6 +176,7 @@ openclaw pairing approve discord <CODE>
|
||||
|
||||
<Note>
|
||||
Token resolution is account-aware. Config token values win over env fallback. `DISCORD_BOT_TOKEN` is only used for the default account.
|
||||
If two enabled Discord accounts resolve to the same bot token, OpenClaw starts only one gateway monitor for that token. A config-sourced token wins over the default env fallback; otherwise the first enabled account wins and the duplicate account is reported disabled.
|
||||
For advanced outbound calls (message tool/channel actions), an explicit per-call `token` is used for that call. This applies to send and read/probe-style actions (for example read/search/fetch/thread/pins/permissions). Account policy/retry settings still come from the selected account in the active runtime snapshot.
|
||||
</Note>
|
||||
|
||||
|
||||
@@ -1,10 +1,17 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
createDiscordActionGate,
|
||||
isDiscordAccountEnabledForRuntime,
|
||||
listEnabledDiscordAccounts,
|
||||
resolveDiscordAccount,
|
||||
resolveDiscordAccountDisabledReason,
|
||||
resolveDiscordMaxLinesPerMessage,
|
||||
} from "./accounts.js";
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
describe("resolveDiscordAccount allowFrom precedence", () => {
|
||||
it("uses configured defaultAccount when accountId is omitted", () => {
|
||||
const resolved = resolveDiscordAccount({
|
||||
@@ -161,3 +168,80 @@ describe("resolveDiscordMaxLinesPerMessage", () => {
|
||||
expect(resolved).toBe(80);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Discord duplicate-token account filtering", () => {
|
||||
it("keeps the config-token account over default env fallback when tokens collide", () => {
|
||||
vi.stubEnv("DISCORD_BOT_TOKEN", "same-token");
|
||||
const cfg = {
|
||||
channels: {
|
||||
discord: {
|
||||
accounts: {
|
||||
work: {
|
||||
token: "same-token",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const defaultAccount = resolveDiscordAccount({ cfg, accountId: "default" });
|
||||
const workAccount = resolveDiscordAccount({ cfg, accountId: "work" });
|
||||
|
||||
expect(isDiscordAccountEnabledForRuntime(defaultAccount, cfg)).toBe(false);
|
||||
expect(resolveDiscordAccountDisabledReason(defaultAccount, cfg)).toBe(
|
||||
'duplicate bot token; using account "work"',
|
||||
);
|
||||
expect(isDiscordAccountEnabledForRuntime(workAccount, cfg)).toBe(true);
|
||||
expect(listEnabledDiscordAccounts(cfg).map((account) => account.accountId)).toEqual(["work"]);
|
||||
});
|
||||
|
||||
it("keeps the first enabled account when duplicate tokens have the same source", () => {
|
||||
const cfg = {
|
||||
channels: {
|
||||
discord: {
|
||||
accounts: {
|
||||
first: {
|
||||
token: "same-token",
|
||||
},
|
||||
second: {
|
||||
token: "same-token",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const firstAccount = resolveDiscordAccount({ cfg, accountId: "first" });
|
||||
const secondAccount = resolveDiscordAccount({ cfg, accountId: "second" });
|
||||
|
||||
expect(isDiscordAccountEnabledForRuntime(firstAccount, cfg)).toBe(true);
|
||||
expect(isDiscordAccountEnabledForRuntime(secondAccount, cfg)).toBe(false);
|
||||
expect(resolveDiscordAccountDisabledReason(secondAccount, cfg)).toBe(
|
||||
'duplicate bot token; using account "first"',
|
||||
);
|
||||
expect(listEnabledDiscordAccounts(cfg).map((account) => account.accountId)).toEqual(["first"]);
|
||||
});
|
||||
|
||||
it("does not let disabled duplicate-token accounts suppress enabled accounts", () => {
|
||||
const cfg = {
|
||||
channels: {
|
||||
discord: {
|
||||
accounts: {
|
||||
disabled: {
|
||||
enabled: false,
|
||||
token: "same-token",
|
||||
},
|
||||
active: {
|
||||
token: "same-token",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const activeAccount = resolveDiscordAccount({ cfg, accountId: "active" });
|
||||
|
||||
expect(isDiscordAccountEnabledForRuntime(activeAccount, cfg)).toBe(true);
|
||||
expect(listEnabledDiscordAccounts(cfg).map((account) => account.accountId)).toEqual(["active"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -91,8 +91,65 @@ export function resolveDiscordMaxLinesPerMessage(params: {
|
||||
}).config.maxLinesPerMessage;
|
||||
}
|
||||
|
||||
function resolveDiscordAccountTokenOwner(params: {
|
||||
cfg: OpenClawConfig;
|
||||
token: string;
|
||||
}): string | undefined {
|
||||
const token = params.token.trim();
|
||||
if (!token) {
|
||||
return undefined;
|
||||
}
|
||||
let owner: { accountId: string; priority: number; index: number } | undefined;
|
||||
const accountIds = listDiscordAccountIds(params.cfg);
|
||||
for (const [index, accountId] of accountIds.entries()) {
|
||||
const account = resolveDiscordAccount({ cfg: params.cfg, accountId });
|
||||
const accountToken = account.token.trim();
|
||||
if (!account.enabled || accountToken !== token) {
|
||||
continue;
|
||||
}
|
||||
const priority = account.tokenSource === "config" ? 2 : account.tokenSource === "env" ? 1 : 0;
|
||||
if (!owner || priority > owner.priority) {
|
||||
owner = { accountId: account.accountId, priority, index };
|
||||
continue;
|
||||
}
|
||||
if (priority === owner.priority && index < owner.index) {
|
||||
owner = { accountId: account.accountId, priority, index };
|
||||
}
|
||||
}
|
||||
return owner?.accountId;
|
||||
}
|
||||
|
||||
export function resolveDiscordDuplicateTokenOwner(params: {
|
||||
cfg: OpenClawConfig;
|
||||
account: ResolvedDiscordAccount;
|
||||
}): string | undefined {
|
||||
const owner = resolveDiscordAccountTokenOwner({
|
||||
cfg: params.cfg,
|
||||
token: params.account.token,
|
||||
});
|
||||
return owner && owner !== params.account.accountId ? owner : undefined;
|
||||
}
|
||||
|
||||
export function isDiscordAccountEnabledForRuntime(
|
||||
account: ResolvedDiscordAccount,
|
||||
cfg: OpenClawConfig,
|
||||
): boolean {
|
||||
return account.enabled && !resolveDiscordDuplicateTokenOwner({ cfg, account });
|
||||
}
|
||||
|
||||
export function resolveDiscordAccountDisabledReason(
|
||||
account: ResolvedDiscordAccount,
|
||||
cfg: OpenClawConfig,
|
||||
): string {
|
||||
if (!account.enabled) {
|
||||
return "disabled";
|
||||
}
|
||||
const owner = resolveDiscordDuplicateTokenOwner({ cfg, account });
|
||||
return owner ? `duplicate bot token; using account "${owner}"` : "disabled";
|
||||
}
|
||||
|
||||
export function listEnabledDiscordAccounts(cfg: OpenClawConfig): ResolvedDiscordAccount[] {
|
||||
return listDiscordAccountIds(cfg)
|
||||
.map((accountId) => resolveDiscordAccount({ cfg, accountId }))
|
||||
.filter((account) => account.enabled);
|
||||
.filter((account) => isDiscordAccountEnabledForRuntime(account, cfg));
|
||||
}
|
||||
|
||||
@@ -366,6 +366,81 @@ describe("preflightDiscordMessage", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores stale route-shaped channel bindings when config now routes to another agent", async () => {
|
||||
const channelId = "channel-stale-route";
|
||||
registerSessionBindingAdapter({
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
listBySession: () => [],
|
||||
resolveByConversation: (ref) =>
|
||||
ref.conversationId === channelId
|
||||
? createThreadBinding({
|
||||
bindingId: "default:channel-stale-route",
|
||||
targetKind: "session",
|
||||
targetSessionKey: `agent:oldagent:discord:channel:${channelId}`,
|
||||
conversation: {
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
conversationId: channelId,
|
||||
},
|
||||
metadata: undefined,
|
||||
})
|
||||
: null,
|
||||
});
|
||||
|
||||
const result = await runGuildPreflight({
|
||||
channelId,
|
||||
guildId: "guild-stale-route",
|
||||
message: createDiscordMessage({
|
||||
id: "m-stale-route",
|
||||
channelId,
|
||||
content: "which agent is this?",
|
||||
author: {
|
||||
id: "user-1",
|
||||
bot: false,
|
||||
username: "alice",
|
||||
},
|
||||
}),
|
||||
cfg: {
|
||||
agents: {
|
||||
list: [{ id: "newagent" }],
|
||||
},
|
||||
bindings: [
|
||||
{
|
||||
agentId: "newagent",
|
||||
match: {
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
peer: { kind: "channel", id: channelId },
|
||||
},
|
||||
},
|
||||
],
|
||||
channels: {
|
||||
discord: {},
|
||||
},
|
||||
},
|
||||
discordConfig: {
|
||||
allowBots: true,
|
||||
} as DiscordConfig,
|
||||
guildEntries: {
|
||||
"guild-stale-route": {
|
||||
channels: {
|
||||
[channelId]: {
|
||||
enabled: true,
|
||||
requireMention: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result?.route.agentId).toBe("newagent");
|
||||
expect(result?.route.sessionKey).toBe(`agent:newagent:discord:channel:${channelId}`);
|
||||
expect(result?.boundSessionKey).toBeUndefined();
|
||||
expect(result?.threadBinding).toBeUndefined();
|
||||
});
|
||||
|
||||
it("preflights direct-message voice notes without mention gating", async () => {
|
||||
transcribeFirstAudioMock.mockResolvedValue("hello openclaw from dm audio");
|
||||
|
||||
|
||||
@@ -54,6 +54,7 @@ import {
|
||||
buildDiscordRoutePeer,
|
||||
resolveDiscordConversationRoute,
|
||||
resolveDiscordEffectiveRoute,
|
||||
shouldIgnoreStaleDiscordRouteBinding,
|
||||
} from "./route-resolution.js";
|
||||
import { resolveDiscordSenderIdentity, resolveDiscordWebhookId } from "./sender-identity.js";
|
||||
import { isRecentlyUnboundThreadWebhookMessage } from "./thread-bindings.js";
|
||||
@@ -644,7 +645,7 @@ export async function preflightDiscordMessage(
|
||||
}) ?? `user:${author.id}`)
|
||||
: messageChannelId;
|
||||
let threadBinding: SessionBindingRecord | undefined;
|
||||
const runtimeRoute = conversationRuntime.resolveRuntimeConversationBindingRoute({
|
||||
let runtimeRoute = conversationRuntime.resolveRuntimeConversationBindingRoute({
|
||||
route,
|
||||
conversation: {
|
||||
channel: "discord",
|
||||
@@ -653,6 +654,20 @@ export async function preflightDiscordMessage(
|
||||
parentConversationId: earlyThreadParentId,
|
||||
},
|
||||
});
|
||||
if (
|
||||
shouldIgnoreStaleDiscordRouteBinding({
|
||||
bindingRecord: runtimeRoute.bindingRecord,
|
||||
route,
|
||||
})
|
||||
) {
|
||||
logVerbose(
|
||||
`discord: ignoring stale route binding for conversation ${bindingConversationId} (${runtimeRoute.bindingRecord?.targetSessionKey} -> ${route.sessionKey})`,
|
||||
);
|
||||
runtimeRoute = {
|
||||
bindingRecord: null,
|
||||
route,
|
||||
};
|
||||
}
|
||||
threadBinding = runtimeRoute.bindingRecord ?? undefined;
|
||||
const configuredRoute =
|
||||
threadBinding == null
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
resolveDiscordBoundConversationRoute,
|
||||
resolveDiscordConversationRoute,
|
||||
resolveDiscordEffectiveRoute,
|
||||
shouldIgnoreStaleDiscordRouteBinding,
|
||||
} from "./route-resolution.js";
|
||||
|
||||
function buildWorkerBindingConfig(peer: {
|
||||
@@ -136,4 +137,68 @@ describe("discord route resolution helpers", () => {
|
||||
matchedBy: "binding.channel",
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores stale route-shaped bindings after the configured agent changes", () => {
|
||||
const route: ResolvedAgentRoute = {
|
||||
agentId: "newagent",
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
sessionKey: "agent:newagent:discord:channel:c1",
|
||||
mainSessionKey: "agent:newagent:main",
|
||||
lastRoutePolicy: "session",
|
||||
matchedBy: "binding.peer",
|
||||
};
|
||||
|
||||
expect(
|
||||
shouldIgnoreStaleDiscordRouteBinding({
|
||||
route,
|
||||
bindingRecord: {
|
||||
bindingId: "binding-1",
|
||||
targetSessionKey: "agent:oldagent:discord:channel:c1",
|
||||
targetKind: "session",
|
||||
conversation: {
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
conversationId: "c1",
|
||||
},
|
||||
status: "active",
|
||||
boundAt: 1,
|
||||
},
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps explicit focus bindings even when their agent differs from routing", () => {
|
||||
const route: ResolvedAgentRoute = {
|
||||
agentId: "newagent",
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
sessionKey: "agent:newagent:discord:channel:c1",
|
||||
mainSessionKey: "agent:newagent:main",
|
||||
lastRoutePolicy: "session",
|
||||
matchedBy: "binding.peer",
|
||||
};
|
||||
|
||||
expect(
|
||||
shouldIgnoreStaleDiscordRouteBinding({
|
||||
route,
|
||||
bindingRecord: {
|
||||
bindingId: "focus-binding",
|
||||
targetSessionKey: "agent:oldagent:discord:channel:c1",
|
||||
targetKind: "session",
|
||||
conversation: {
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
conversationId: "c1",
|
||||
},
|
||||
status: "active",
|
||||
boundAt: 1,
|
||||
metadata: {
|
||||
boundBy: "user-1",
|
||||
label: "oldagent",
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-types";
|
||||
import type { SessionBindingRecord } from "openclaw/plugin-sdk/conversation-runtime";
|
||||
import {
|
||||
deriveLastRoutePolicy,
|
||||
isAcpSessionKey,
|
||||
isSubagentSessionKey,
|
||||
parseAgentSessionKey,
|
||||
resolveAgentRoute,
|
||||
type ResolvedAgentRoute,
|
||||
type RoutePeer,
|
||||
@@ -98,3 +102,39 @@ export function resolveDiscordEffectiveRoute(params: {
|
||||
...(params.matchedBy ? { matchedBy: params.matchedBy } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
function hasExplicitRuntimeBindingIntent(record: SessionBindingRecord): boolean {
|
||||
if (record.targetKind === "subagent") {
|
||||
return true;
|
||||
}
|
||||
if (isAcpSessionKey(record.targetSessionKey) || isSubagentSessionKey(record.targetSessionKey)) {
|
||||
return true;
|
||||
}
|
||||
const metadata = record.metadata;
|
||||
if (!metadata || typeof metadata !== "object") {
|
||||
return false;
|
||||
}
|
||||
return (
|
||||
typeof metadata.boundBy === "string" ||
|
||||
typeof metadata.label === "string" ||
|
||||
typeof metadata.threadName === "string" ||
|
||||
metadata.pluginBindingOwner === "plugin"
|
||||
);
|
||||
}
|
||||
|
||||
export function shouldIgnoreStaleDiscordRouteBinding(params: {
|
||||
bindingRecord?: SessionBindingRecord | null;
|
||||
route: ResolvedAgentRoute;
|
||||
}): boolean {
|
||||
const bindingRecord = params.bindingRecord;
|
||||
const boundSessionKey = bindingRecord?.targetSessionKey?.trim();
|
||||
if (!bindingRecord || !boundSessionKey || hasExplicitRuntimeBindingIntent(bindingRecord)) {
|
||||
return false;
|
||||
}
|
||||
const bound = parseAgentSessionKey(boundSessionKey);
|
||||
const routed = parseAgentSessionKey(params.route.sessionKey);
|
||||
if (!bound || !routed || bound.rest !== routed.rest) {
|
||||
return false;
|
||||
}
|
||||
return bound.agentId !== params.route.agentId;
|
||||
}
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createDiscordPluginBase } from "./shared.js";
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
describe("createDiscordPluginBase", () => {
|
||||
it("owns Discord native command name overrides", () => {
|
||||
const plugin = createDiscordPluginBase({ setup: {} as never });
|
||||
@@ -26,4 +30,29 @@ describe("createDiscordPluginBase", () => {
|
||||
expect(plugin.security?.collectWarnings).toBeTypeOf("function");
|
||||
expect(plugin.security?.collectAuditFindings).toBeTypeOf("function");
|
||||
});
|
||||
|
||||
it("reports duplicate-token accounts as disabled to gateway startup", () => {
|
||||
vi.stubEnv("DISCORD_BOT_TOKEN", "same-token");
|
||||
const plugin = createDiscordPluginBase({ setup: {} as never });
|
||||
const cfg = {
|
||||
channels: {
|
||||
discord: {
|
||||
accounts: {
|
||||
work: {
|
||||
token: "same-token",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const defaultAccount = plugin.config.resolveAccount(cfg, "default");
|
||||
const workAccount = plugin.config.resolveAccount(cfg, "work");
|
||||
|
||||
expect(plugin.config.isEnabled?.(defaultAccount, cfg)).toBe(false);
|
||||
expect(plugin.config.disabledReason?.(defaultAccount, cfg)).toBe(
|
||||
'duplicate bot token; using account "work"',
|
||||
);
|
||||
expect(plugin.config.isEnabled?.(workAccount, cfg)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,9 +5,11 @@ import { createScopedChannelConfigAdapter } from "openclaw/plugin-sdk/channel-co
|
||||
import type { ChannelDoctorAdapter } from "openclaw/plugin-sdk/channel-contract";
|
||||
import { inspectDiscordAccount } from "./account-inspect.js";
|
||||
import {
|
||||
isDiscordAccountEnabledForRuntime,
|
||||
listDiscordAccountIds,
|
||||
resolveDefaultDiscordAccountId,
|
||||
resolveDiscordAccount,
|
||||
resolveDiscordAccountDisabledReason,
|
||||
type ResolvedDiscordAccount,
|
||||
} from "./accounts.js";
|
||||
import { getChatChannelMeta, type ChannelPlugin } from "./channel-api.js";
|
||||
@@ -119,6 +121,8 @@ export function createDiscordPluginBase(params: {
|
||||
...discordConfigAdapter,
|
||||
hasConfiguredState: ({ env }) =>
|
||||
typeof env?.DISCORD_BOT_TOKEN === "string" && env.DISCORD_BOT_TOKEN.trim().length > 0,
|
||||
isEnabled: (account, cfg) => isDiscordAccountEnabledForRuntime(account, cfg),
|
||||
disabledReason: (account, cfg) => resolveDiscordAccountDisabledReason(account, cfg),
|
||||
isConfigured: (account) => Boolean(account.token?.trim()),
|
||||
describeAccount: (account) =>
|
||||
describeAccountSnapshot({
|
||||
|
||||
Reference in New Issue
Block a user