mirror of
https://github.com/ZHANGTIANYAO1/teamspeak-music-bot.git
synced 2026-10-02 04:52:50 +08:00
fix(spotify): per-bot Connect device name to avoid multi-bot device collision/misroute [corner-case R2-5]
config.spotify is a single process-wide object shared by every BotInstance, so
config.spotify.deviceName was identical for all bots. On the Rust (librespot)
backend each bot spawned `librespot --name <deviceName>` with no per-bot
uniqueness, registering two Connect devices with the same name under the one
shared account. findDeviceByName()/waitForDevice() match by name, so bot A's
transfer()+play() could drive bot B's librespot (misroute) and a bot could
report ready on seeing the OTHER bot's same-named device (false readiness).
Fix: derive a per-bot-unique Connect identity from the shared base name.
- controller.ts: new exported pure helper perBotDeviceName(base, instanceId?)
(`${base}-${instanceId}` when an id is given, else base). Add optional
instanceId to SpotifyControllerOptions; buildBackend() computes the effective
name once and passes the SAME value to both the Rust and go backends so
--name, findDeviceByName, and waitForDevice all key on one identity.
- instance.ts: pass instanceId: this.id into buildController(); add instanceId
to the spotifyControllerFactory param type (test seam).
The user-configured config.spotify.deviceName base is left untouched; the suffix
applies only to the backend/Connect identity. Behavior-preserving for callers
that pass no instanceId (base name used unchanged).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
9a32f3c602
commit
2e05d276bf
4 files changed
+97
-5
No files matched your search
@@ -626,9 +626,9 @@ describe("BotInstance.seek — spotify routing (C4)", () => {
|
||||
describe("BotInstance — spotifyOAuth threading to the controller factory (C3.1)", () => {
|
||||
function makeInstanceOptions(over: Partial<BotInstanceOptions> = {}): {
|
||||
options: BotInstanceOptions;
|
||||
captured: { param?: { oauth?: SpotifyOAuth } };
|
||||
captured: { param?: { oauth?: SpotifyOAuth; instanceId?: string } };
|
||||
} {
|
||||
const captured: { param?: { oauth?: SpotifyOAuth } } = {};
|
||||
const captured: { param?: { oauth?: SpotifyOAuth; instanceId?: string } } = {};
|
||||
const provider = { platform: "netease" } as unknown as MusicProvider;
|
||||
const logger: any = {
|
||||
info() {}, warn() {}, error() {}, debug() {},
|
||||
@@ -677,6 +677,17 @@ describe("BotInstance — spotifyOAuth threading to the controller factory (C3.1
|
||||
expect(captured.param).toBeDefined();
|
||||
expect(captured.param?.oauth).toBeUndefined();
|
||||
});
|
||||
|
||||
// R2-5: the bot id must reach the controller as `instanceId` so the backend
|
||||
// derives a UNIQUE Spotify Connect device name (<base>-<id>). Without it two
|
||||
// bots share the process-global deviceName and Connect commands misroute.
|
||||
it("forwards the bot id to the controller factory as `instanceId` (corner-case R2-5)", () => {
|
||||
const { options, captured } = makeInstanceOptions({ id: "bot-xyz" });
|
||||
// eslint-disable-next-line no-new
|
||||
new BotInstance(options);
|
||||
expect(captured.param).toBeDefined();
|
||||
expect(captured.param?.instanceId).toBe("bot-xyz");
|
||||
});
|
||||
});
|
||||
|
||||
describe("spotifyPortsForBotId — per-bot go-librespot ports (Fix 3)", () => {
|
||||
|
||||
@@ -94,6 +94,7 @@ export interface BotInstanceOptions {
|
||||
workDir: string;
|
||||
configDir: string;
|
||||
logger: Logger;
|
||||
instanceId: string;
|
||||
apiPort: number;
|
||||
callbackPort: number;
|
||||
oauth?: SpotifyOAuth;
|
||||
@@ -193,6 +194,10 @@ export class BotInstance extends EventEmitter {
|
||||
workDir: spotifyWorkDir,
|
||||
configDir: spotifyConfigDir,
|
||||
logger: this.logger,
|
||||
// Per-bot id → unique Spotify Connect device name ("<base>-<id>"), so two
|
||||
// bots under the one shared account never register the same name and
|
||||
// misroute Connect commands (corner-case R2-5).
|
||||
instanceId: this.id,
|
||||
apiPort: spotifyApiPort,
|
||||
callbackPort: spotifyCallbackPort,
|
||||
oauth: options.spotifyOAuth,
|
||||
|
||||
@@ -75,7 +75,7 @@ vi.mock("./go-librespot.js", async () => {
|
||||
});
|
||||
|
||||
// Import AFTER vi.mock so the mocked binary module is used.
|
||||
const { SpotifyController } = await import("./controller.js");
|
||||
const { SpotifyController, perBotDeviceName } = await import("./controller.js");
|
||||
|
||||
const existingBin = join(tmpdir(), `tsmb-golibrespot-${process.pid}`);
|
||||
const missingBin = join(tmpdir(), `tsmb-golibrespot-missing-${process.pid}`);
|
||||
@@ -533,6 +533,49 @@ describe("SpotifyController per-bot ports (Fix 3)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("perBotDeviceName (corner-case R2-5)", () => {
|
||||
it("suffixes the base name with the instanceId", () => {
|
||||
expect(perBotDeviceName("TSMusicBot", "bot1")).toBe("TSMusicBot-bot1");
|
||||
});
|
||||
it("returns the base name unchanged when no instanceId is given", () => {
|
||||
expect(perBotDeviceName("TSMusicBot", undefined)).toBe("TSMusicBot");
|
||||
});
|
||||
});
|
||||
|
||||
describe("SpotifyController per-bot device name (corner-case R2-5)", () => {
|
||||
it("applies the per-bot suffix to the backend deviceName when instanceId is set", async () => {
|
||||
goLibrespotCtor.mockClear();
|
||||
// No injected backendFactory → the controller builds the (mocked) real backend.
|
||||
const ctrl = new SpotifyController({
|
||||
config: cfg(),
|
||||
workDir: "/tmp/work",
|
||||
configDir: "/tmp/cfg",
|
||||
logger: silentLogger,
|
||||
instanceId: "bot1",
|
||||
});
|
||||
expect(await ctrl.ensureStarted()).toBe(true);
|
||||
// The Connect identity is UNIQUE per bot ("<base>-<id>") so two bots under
|
||||
// one account never register the same name (misroute / false-readiness).
|
||||
expect(goLibrespotCtor).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ deviceName: "TSMusicBot-bot1" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("leaves the base deviceName unchanged when no instanceId (behavior-preserving)", async () => {
|
||||
goLibrespotCtor.mockClear();
|
||||
const ctrl = new SpotifyController({
|
||||
config: cfg(),
|
||||
workDir: "/tmp/work",
|
||||
configDir: "/tmp/cfg",
|
||||
logger: silentLogger,
|
||||
});
|
||||
expect(await ctrl.ensureStarted()).toBe(true);
|
||||
expect(goLibrespotCtor).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ deviceName: "TSMusicBot" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("SpotifyController.chooseBackend (platform x config matrix)", () => {
|
||||
function pick(
|
||||
backend: SpotifyConfig["backend"],
|
||||
|
||||
@@ -30,6 +30,26 @@ import {
|
||||
} from "./backend-select.js";
|
||||
export type { SpotifyBackendKind }; // keep the name exported for existing importers
|
||||
|
||||
/**
|
||||
* Derive the per-bot Spotify Connect device name from the user-configured base.
|
||||
*
|
||||
* `config.spotify` is a single process-wide object shared by every BotInstance,
|
||||
* so `config.spotify.deviceName` is identical for all bots. On the Rust
|
||||
* (librespot) backend each bot would otherwise spawn `librespot --name <base>`
|
||||
* with NO uniqueness, registering TWO Connect devices with the SAME name under
|
||||
* the one shared Premium account — so `findDeviceByName` / `waitForDevice`
|
||||
* (which match by name) could target the OTHER bot's device, misrouting
|
||||
* transfer()+play() and reporting false readiness (corner-case R2-5).
|
||||
*
|
||||
* Suffixing the base with the bot's instanceId makes the Connect identity
|
||||
* unique per bot. Pure + deterministic: same inputs → same name across
|
||||
* restarts. Returns the base unchanged when no instanceId is supplied (keeps
|
||||
* existing single-bot / non-injected behavior byte-for-byte).
|
||||
*/
|
||||
export function perBotDeviceName(base: string, instanceId?: string): string {
|
||||
return instanceId ? `${base}-${instanceId}` : base;
|
||||
}
|
||||
|
||||
/**
|
||||
* Minimal file-backed OAuth token store used when the caller does not inject a
|
||||
* SpotifyOAuth. Persists the rotating refresh-token JSON next to the bot config
|
||||
@@ -64,6 +84,10 @@ export interface SpotifyControllerOptions {
|
||||
workDir: string;
|
||||
configDir: string;
|
||||
logger: Logger;
|
||||
/** Owning bot's id; suffixed onto the shared base deviceName so each bot's
|
||||
* Spotify Connect identity is unique (avoids multi-bot device collision /
|
||||
* misroute — corner-case R2-5). Omitted → the base name is used unchanged. */
|
||||
instanceId?: string;
|
||||
/** Per-bot go-librespot control-API port (distinct per bot to avoid binds). */
|
||||
apiPort?: number;
|
||||
/** Per-bot go-librespot OAuth callback port (distinct per bot). */
|
||||
@@ -93,6 +117,7 @@ export class SpotifyController extends EventEmitter {
|
||||
private readonly workDir: string;
|
||||
private readonly configDir: string;
|
||||
private readonly logger: Logger;
|
||||
private readonly instanceId?: string;
|
||||
private readonly apiPort?: number;
|
||||
private readonly callbackPort?: number;
|
||||
private readonly injectedFactory?: () => SpotifyAudioBackend;
|
||||
@@ -115,6 +140,7 @@ export class SpotifyController extends EventEmitter {
|
||||
this.workDir = o.workDir;
|
||||
this.configDir = o.configDir;
|
||||
this.logger = o.logger;
|
||||
this.instanceId = o.instanceId;
|
||||
this.apiPort = o.apiPort;
|
||||
this.callbackPort = o.callbackPort;
|
||||
this.injectedFactory = o.backendFactory;
|
||||
@@ -179,9 +205,16 @@ export class SpotifyController extends EventEmitter {
|
||||
/** Build the concrete backend for the chosen kind (or the injected fake). */
|
||||
private buildBackend(kind: SpotifyBackendKind): SpotifyAudioBackend {
|
||||
if (this.injectedFactory) return this.injectedFactory();
|
||||
// Compute the effective (per-bot-unique) Connect device name ONCE and pass
|
||||
// the SAME value to whichever backend we build, so `librespot --name`,
|
||||
// findDeviceByName(), and waitForDevice() all key on one identity — that
|
||||
// consistency is what prevents the multi-bot misroute / false-readiness
|
||||
// (corner-case R2-5). The user-configured base (config.deviceName) is left
|
||||
// untouched; the suffix is applied only to the backend/Connect identity.
|
||||
const deviceName = perBotDeviceName(this.config.deviceName, this.instanceId);
|
||||
if (kind === "librespot") {
|
||||
return new RustLibrespotBackend({
|
||||
deviceName: this.config.deviceName,
|
||||
deviceName,
|
||||
bitrate: this.config.bitrate,
|
||||
cacheDir: join(this.workDir, "librespot-cache"),
|
||||
oauth: this.oauth,
|
||||
@@ -190,7 +223,7 @@ export class SpotifyController extends EventEmitter {
|
||||
});
|
||||
}
|
||||
return new GoLibrespotBackend({
|
||||
deviceName: this.config.deviceName,
|
||||
deviceName,
|
||||
bitrate: this.config.bitrate,
|
||||
workDir: this.workDir,
|
||||
configDir: this.configDir,
|
||||
|
||||
Reference in new issue
Block a user