mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(discord): isolate voice connections and close auto-join race (#87530)
* fix(discord): isolate voice connections by account Co-authored-by: geekhuashan <geekhuashan@gmail.com> * chore: defer Discord voice accounts changelog * fix(discord): own auto-join cleanup in lifecycle * test(discord): bind lifecycle voice mocks --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -276,6 +276,27 @@ describe("runDiscordGatewayLifecycle", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("owns and cleans up auto-join when READY preceded voice listener registration", async () => {
|
||||
waitForDiscordGatewayStopMock.mockRejectedValueOnce(new Error("gateway wait failed"));
|
||||
const { lifecycleParams } = createLifecycleHarness();
|
||||
const autoJoin = vi.fn(async () => undefined);
|
||||
const destroy = vi.fn(async () => undefined);
|
||||
const voiceManager = {
|
||||
autoJoin,
|
||||
destroy,
|
||||
} as unknown as NonNullable<LifecycleParams["voiceManager"]>;
|
||||
lifecycleParams.voiceManager = voiceManager;
|
||||
lifecycleParams.voiceManagerRef.current = voiceManager;
|
||||
|
||||
await expect(runDiscordGatewayLifecycle(lifecycleParams)).rejects.toThrow(
|
||||
"gateway wait failed",
|
||||
);
|
||||
|
||||
expect(autoJoin).toHaveBeenCalledTimes(1);
|
||||
expect(destroy).toHaveBeenCalledTimes(1);
|
||||
expect(lifecycleParams.voiceManagerRef.current).toBeNull();
|
||||
});
|
||||
|
||||
it("pushes connected status when gateway is already connected at lifecycle start", async () => {
|
||||
const { emitter, gateway } = createGatewayHarness();
|
||||
gateway.isConnected = true;
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
import { asDateTimestampMs, parseStrictPositiveInteger } from "openclaw/plugin-sdk/number-runtime";
|
||||
import { danger } from "openclaw/plugin-sdk/runtime-env";
|
||||
import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env";
|
||||
import { formatErrorMessage } from "openclaw/plugin-sdk/ssrf-runtime";
|
||||
import { attachDiscordGatewayLogging } from "../gateway-logging.js";
|
||||
import { getDiscordGatewayEmitter, waitForDiscordGatewayStop } from "../monitor.gateway.js";
|
||||
import type { DiscordVoiceManager } from "../voice/manager.js";
|
||||
@@ -420,6 +421,7 @@ export async function runDiscordGatewayLifecycle(params: {
|
||||
gatewayRuntimeReadyTimeoutMs?: number;
|
||||
}) {
|
||||
const gateway = params.gateway;
|
||||
const gatewayReadyAtLifecycleStart = gateway?.isConnected === true;
|
||||
if (gateway) {
|
||||
registerGateway(params.accountId, gateway);
|
||||
}
|
||||
@@ -515,6 +517,18 @@ export async function runDiscordGatewayLifecycle(params: {
|
||||
return;
|
||||
}
|
||||
|
||||
if (gatewayReadyAtLifecycleStart && params.voiceManager) {
|
||||
// READY may precede lazy voice-listener registration. Reconcile only after lifecycle
|
||||
// ownership begins so every startup failure still destroys the manager in `finally`.
|
||||
void params.voiceManager
|
||||
.autoJoin()
|
||||
.catch((err: unknown) =>
|
||||
params.runtime.error?.(
|
||||
danger(`discord voice: autoJoin failed: ${formatErrorMessage(err)}`),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
await waitForGatewayReady({
|
||||
gateway,
|
||||
abortSignal: params.abortSignal,
|
||||
|
||||
@@ -39,6 +39,10 @@ const {
|
||||
voiceRuntimeModuleLoadedMock,
|
||||
} = getProviderMonitorTestMocks();
|
||||
|
||||
const { voiceAutoJoinMock } = vi.hoisted(() => ({
|
||||
voiceAutoJoinMock: vi.fn(async () => undefined),
|
||||
}));
|
||||
|
||||
let monitorDiscordProvider: typeof import("./provider.js").monitorDiscordProvider;
|
||||
let providerTesting: typeof import("./provider.js").testing;
|
||||
let runtimeEnvModule: typeof import("openclaw/plugin-sdk/runtime-env");
|
||||
@@ -140,7 +144,9 @@ function expectMessagesContainAll(messages: string[], expected: string[]): void
|
||||
vi.mock("../voice/manager.runtime.js", () => {
|
||||
voiceRuntimeModuleLoadedMock();
|
||||
return {
|
||||
DiscordVoiceManager: function DiscordVoiceManager() {},
|
||||
DiscordVoiceManager: function DiscordVoiceManager() {
|
||||
return { autoJoin: voiceAutoJoinMock };
|
||||
},
|
||||
DiscordVoiceReadyListener: function DiscordVoiceReadyListener() {},
|
||||
DiscordVoiceResumedListener: function DiscordVoiceResumedListener() {},
|
||||
DiscordVoiceStateUpdateListener: function DiscordVoiceStateUpdateListener() {},
|
||||
@@ -252,6 +258,7 @@ describe("monitorDiscordProvider", () => {
|
||||
|
||||
beforeEach(() => {
|
||||
resetDiscordProviderMonitorMocks();
|
||||
voiceAutoJoinMock.mockClear();
|
||||
vi.mocked(runtimeEnvModule.logVerbose).mockClear();
|
||||
providerTesting.setFetchDiscordApplicationId(async () => "app-1");
|
||||
providerTesting.setCreateDiscordNativeCommand(((
|
||||
@@ -270,7 +277,9 @@ describe("monitorDiscordProvider", () => {
|
||||
providerTesting.setLoadDiscordVoiceRuntime(async () => {
|
||||
voiceRuntimeModuleLoadedMock();
|
||||
return {
|
||||
DiscordVoiceManager: function DiscordVoiceManager() {},
|
||||
DiscordVoiceManager: function DiscordVoiceManager() {
|
||||
return { autoJoin: voiceAutoJoinMock };
|
||||
},
|
||||
DiscordVoiceReadyListener: function DiscordVoiceReadyListener() {},
|
||||
DiscordVoiceResumedListener: function DiscordVoiceResumedListener() {},
|
||||
DiscordVoiceStateUpdateListener: function DiscordVoiceStateUpdateListener() {},
|
||||
|
||||
@@ -402,12 +402,13 @@ describe("DiscordVoiceManager", () => {
|
||||
>[0]["discordConfig"] = { voice: { enabled: true, mode: "stt-tts" } },
|
||||
clientOverride?: ReturnType<typeof createClient>,
|
||||
cfgOverride: ConstructorParameters<typeof managerModule.DiscordVoiceManager>[0]["cfg"] = {},
|
||||
accountId = "default",
|
||||
) =>
|
||||
new managerModule.DiscordVoiceManager({
|
||||
client: (clientOverride ?? createClient()) as never,
|
||||
cfg: cfgOverride,
|
||||
discordConfig,
|
||||
accountId: "default",
|
||||
accountId,
|
||||
runtime: createRuntime(),
|
||||
});
|
||||
|
||||
@@ -1020,11 +1021,30 @@ describe("DiscordVoiceManager", () => {
|
||||
|
||||
await manager.join({ guildId: "g1", channelId: "1001" });
|
||||
|
||||
expect(getVoiceConnectionMock).toHaveBeenCalledWith("g1");
|
||||
expect(getVoiceConnectionMock).toHaveBeenCalledWith("g1", "openclaw:default");
|
||||
expect(staleConnection.destroy).toHaveBeenCalledTimes(1);
|
||||
expectConnectedStatus(manager, "1001");
|
||||
});
|
||||
|
||||
it("isolates voice connections by Discord account", async () => {
|
||||
const firstManager = createManager(undefined, undefined, undefined, "first");
|
||||
const secondManager = createManager(undefined, undefined, undefined, "second");
|
||||
|
||||
await firstManager.join({ guildId: "g1", channelId: "1001" });
|
||||
await secondManager.join({ guildId: "g1", channelId: "1002" });
|
||||
|
||||
expect(getVoiceConnectionMock).toHaveBeenNthCalledWith(1, "g1", "openclaw:first");
|
||||
expect(getVoiceConnectionMock).toHaveBeenNthCalledWith(2, "g1", "openclaw:second");
|
||||
expect(joinVoiceChannelMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ group: "openclaw:first" }),
|
||||
);
|
||||
expect(joinVoiceChannelMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({ group: "openclaw:second" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("autoJoin uses the last configured channel for duplicate guild entries", async () => {
|
||||
const manager = createManager({
|
||||
voice: {
|
||||
|
||||
@@ -212,6 +212,10 @@ function startAutoJoin(manager: Pick<DiscordVoiceManager, "autoJoin">) {
|
||||
);
|
||||
}
|
||||
|
||||
function resolveVoiceConnectionGroup(accountId: string): string {
|
||||
return `openclaw:${accountId}`;
|
||||
}
|
||||
|
||||
function resolveDiscordVoiceAgentRoute(params: {
|
||||
cfg: OpenClawConfig;
|
||||
accountId: string;
|
||||
@@ -554,7 +558,8 @@ export class DiscordVoiceManager {
|
||||
existingEntry.stop();
|
||||
this.sessions.delete(guildId);
|
||||
}
|
||||
const staleConnection = voiceSdk.getVoiceConnection(guildId);
|
||||
const voiceConnectionGroup = resolveVoiceConnectionGroup(this.params.accountId);
|
||||
const staleConnection = voiceSdk.getVoiceConnection(guildId, voiceConnectionGroup);
|
||||
if (staleConnection) {
|
||||
destroyVoiceConnectionSafely({
|
||||
connection: staleConnection,
|
||||
@@ -568,6 +573,7 @@ export class DiscordVoiceManager {
|
||||
const joinedConnection = voiceSdk.joinVoiceChannel({
|
||||
channelId,
|
||||
guildId,
|
||||
group: voiceConnectionGroup,
|
||||
adapterCreator,
|
||||
selfDeaf: false,
|
||||
selfMute: false,
|
||||
@@ -995,7 +1001,10 @@ export class DiscordVoiceManager {
|
||||
await this.leave({ guildId });
|
||||
} else {
|
||||
const voiceSdk = loadDiscordVoiceSdk();
|
||||
const connection = voiceSdk.getVoiceConnection(guildId);
|
||||
const connection = voiceSdk.getVoiceConnection(
|
||||
guildId,
|
||||
resolveVoiceConnectionGroup(this.params.accountId),
|
||||
);
|
||||
if (connection) {
|
||||
destroyVoiceConnectionSafely({
|
||||
connection,
|
||||
|
||||
Reference in New Issue
Block a user