diff --git a/src/bot/instance.test.ts b/src/bot/instance.test.ts index e92aa3f..9241524 100644 --- a/src/bot/instance.test.ts +++ b/src/bot/instance.test.ts @@ -626,9 +626,9 @@ describe("BotInstance.seek — spotify routing (C4)", () => { describe("BotInstance — spotifyOAuth threading to the controller factory (C3.1)", () => { function makeInstanceOptions(over: Partial = {}): { 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 (-). 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)", () => { diff --git a/src/bot/instance.ts b/src/bot/instance.ts index 53d3d7e..5de31a6 100755 --- a/src/bot/instance.ts +++ b/src/bot/instance.ts @@ -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 ("-"), 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, diff --git a/src/music/spotify/controller.test.ts b/src/music/spotify/controller.test.ts index 819a227..fa83c1d 100644 --- a/src/music/spotify/controller.test.ts +++ b/src/music/spotify/controller.test.ts @@ -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 ("-") 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"], diff --git a/src/music/spotify/controller.ts b/src/music/spotify/controller.ts index 3c41d2a..97e6b05 100644 --- a/src/music/spotify/controller.ts +++ b/src/music/spotify/controller.ts @@ -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 ` + * 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,