mirror of
https://github.com/ZHANGTIANYAO1/teamspeak-music-bot.git
synced 2026-10-01 20:42:50 +08:00
fix(spotify): PATH-aware binary presence detection (bin/ or PATH) [whole-branch I3,m1]
rustPresent/goPresent and getBackendInfo used existsSync(findX()), which for a bare PATH command name resolves against cwd, not $PATH — so a scoop/choco/cargo/ apt install was invisible and Spotify was gated off. Add a sync PATH-aware resolveExecutable() + isLibrespotPresent()/isGoLibrespotPresent() in binary.ts and route controller.ts and web/server.ts through them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
b4c3cc0539
commit
399cf0cf41
5 files changed
+203
-20
No files matched your search
@@ -1,5 +1,7 @@
|
||||
import { describe, it, expect, vi, afterEach } from "vitest";
|
||||
import { join } from "node:path";
|
||||
import { mkdtempSync, writeFileSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import {
|
||||
isGoLibrespotSupported,
|
||||
pickGoLibrespotPath,
|
||||
@@ -13,6 +15,9 @@ import {
|
||||
checkLibrespotAvailable,
|
||||
resetLibrespotBinaryCache,
|
||||
__setLibrespotVersionProbe,
|
||||
resolveExecutable,
|
||||
isLibrespotPresent,
|
||||
isGoLibrespotPresent,
|
||||
} from "./binary.js";
|
||||
|
||||
const origPlatform = process.platform;
|
||||
@@ -20,12 +25,33 @@ function setPlatform(p: NodeJS.Platform): void {
|
||||
Object.defineProperty(process, "platform", { value: p, configurable: true });
|
||||
}
|
||||
|
||||
// Snapshot the PATH env so PATH-resolution tests can point it at temp dirs and
|
||||
// restore afterwards. PATHEXT is optional (undefined on posix); track presence.
|
||||
const origPath = process.env.PATH;
|
||||
const origPathExt = process.env.PATHEXT;
|
||||
const tmpDirs: string[] = [];
|
||||
function mkTmpDir(prefix: string): string {
|
||||
const dir = mkdtempSync(join(tmpdir(), prefix));
|
||||
tmpDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
setPlatform(origPlatform);
|
||||
__setGoLibrespotVersionProbe(null);
|
||||
resetGoLibrespotBinaryCache();
|
||||
__setLibrespotVersionProbe(null);
|
||||
resetLibrespotBinaryCache();
|
||||
process.env.PATH = origPath;
|
||||
if (origPathExt === undefined) delete process.env.PATHEXT;
|
||||
else process.env.PATHEXT = origPathExt;
|
||||
for (const dir of tmpDirs.splice(0)) {
|
||||
try {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
describe("isGoLibrespotSupported", () => {
|
||||
@@ -234,3 +260,104 @@ describe("checkLibrespotAvailable", () => {
|
||||
expect(probe).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveExecutable", () => {
|
||||
it("returns a bin/-style path (contains a separator) that exists, unchanged", () => {
|
||||
const dir = mkTmpDir("tsmb-resolve-");
|
||||
const full = join(dir, "some-binary");
|
||||
writeFileSync(full, "#!/bin/sh\n");
|
||||
// The path contains a separator so it is checked directly, not via PATH.
|
||||
expect(resolveExecutable(full)).toBe(full);
|
||||
});
|
||||
|
||||
it("returns null for a bin/-style path that does not exist", () => {
|
||||
const full = join(tmpdir(), "tsmb-resolve-missing", "nope-binary");
|
||||
expect(resolveExecutable(full)).toBeNull();
|
||||
});
|
||||
|
||||
it("resolves a BARE command name found in a PATH directory (the I3 fix)", () => {
|
||||
// This is the regression the fix targets: a PATH-installed binary (bare
|
||||
// name, no separator) must resolve against $PATH, not process.cwd().
|
||||
const dir = mkTmpDir("tsmb-resolve-path-");
|
||||
const exeName = process.platform === "win32" ? "faketool.exe" : "faketool";
|
||||
const full = join(dir, exeName);
|
||||
writeFileSync(full, "#!/bin/sh\n");
|
||||
process.env.PATH = dir;
|
||||
expect(resolveExecutable(exeName)).toBe(full);
|
||||
});
|
||||
|
||||
it("returns null for a bare name not present in any PATH directory", () => {
|
||||
process.env.PATH = mkTmpDir("tsmb-resolve-empty-");
|
||||
expect(resolveExecutable("definitely-not-a-real-binary-xyz")).toBeNull();
|
||||
});
|
||||
|
||||
it("appends PATHEXT extensions on win32 to resolve a bare (extensionless) name", () => {
|
||||
setPlatform("win32");
|
||||
const dir = mkTmpDir("tsmb-resolve-pathext-");
|
||||
const full = join(dir, "mytool.CMD");
|
||||
writeFileSync(full, "@echo hi\n");
|
||||
process.env.PATH = dir;
|
||||
process.env.PATHEXT = ".EXE;.CMD;.BAT;.COM";
|
||||
// Bare "mytool" has no extension; win32 resolution must try PATHEXT and
|
||||
// find mytool.CMD.
|
||||
expect(resolveExecutable("mytool")).toBe(full);
|
||||
});
|
||||
|
||||
it("does not touch PATH for a bin/-style relative path with a separator", () => {
|
||||
// A candidate with a separator is checked directly even if PATH is empty.
|
||||
process.env.PATH = "";
|
||||
const dir = mkTmpDir("tsmb-resolve-direct-");
|
||||
const full = join(dir, "direct-binary");
|
||||
writeFileSync(full, "#!/bin/sh\n");
|
||||
expect(resolveExecutable(full)).toBe(full);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isLibrespotPresent (PATH-aware, sync)", () => {
|
||||
it("true when the bare librespot name resolves on PATH (posix)", () => {
|
||||
setPlatform("linux");
|
||||
const dir = mkTmpDir("tsmb-librespot-path-");
|
||||
writeFileSync(join(dir, "librespot"), "#!/bin/sh\n");
|
||||
process.env.PATH = dir;
|
||||
expect(isLibrespotPresent()).toBe(true);
|
||||
});
|
||||
|
||||
it("true when librespot.exe resolves on PATH (win32)", () => {
|
||||
setPlatform("win32");
|
||||
const dir = mkTmpDir("tsmb-librespot-win-");
|
||||
writeFileSync(join(dir, "librespot.exe"), "MZ\n");
|
||||
process.env.PATH = dir;
|
||||
process.env.PATHEXT = ".EXE;.CMD;.BAT;.COM";
|
||||
expect(isLibrespotPresent()).toBe(true);
|
||||
});
|
||||
|
||||
it("false when librespot is not on PATH", () => {
|
||||
setPlatform("linux");
|
||||
process.env.PATH = mkTmpDir("tsmb-librespot-empty-");
|
||||
expect(isLibrespotPresent()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isGoLibrespotPresent (PATH-aware, sync)", () => {
|
||||
it("true when go-librespot resolves on PATH (linux)", () => {
|
||||
setPlatform("linux");
|
||||
const dir = mkTmpDir("tsmb-go-path-");
|
||||
writeFileSync(join(dir, "go-librespot"), "#!/bin/sh\n");
|
||||
process.env.PATH = dir;
|
||||
expect(isGoLibrespotPresent()).toBe(true);
|
||||
});
|
||||
|
||||
it("false on non-linux platforms regardless of PATH (unsupported gate)", () => {
|
||||
setPlatform("win32");
|
||||
const dir = mkTmpDir("tsmb-go-win-");
|
||||
writeFileSync(join(dir, "go-librespot"), "#!/bin/sh\n");
|
||||
process.env.PATH = dir;
|
||||
expect(isGoLibrespotPresent()).toBe(false);
|
||||
});
|
||||
|
||||
it("false on linux when go-librespot is not on PATH", () => {
|
||||
setPlatform("linux");
|
||||
process.env.PATH = mkTmpDir("tsmb-go-empty-");
|
||||
expect(isGoLibrespotPresent()).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -2,12 +2,44 @@ import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import { existsSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, join } from "node:path";
|
||||
import { dirname, join, delimiter } from "node:path";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
/**
|
||||
* Resolve a command to an existing absolute path, SYNCHRONOUSLY. A candidate
|
||||
* that already contains a path separator (a project bin/ path) is checked
|
||||
* directly; a BARE command name (e.g. "librespot" / "go-librespot") is searched
|
||||
* across the $PATH directories — with PATHEXT extensions appended on win32 — so
|
||||
* a scoop/choco/cargo/apt PATH install resolves. Returns the resolved path, or
|
||||
* null when nothing exists.
|
||||
*
|
||||
* This is the sync counterpart to checkLibrespotAvailable()/…GoLibrespot… and
|
||||
* is what the hot-path presence gates (chooseBackend/isAvailable) rely on: a
|
||||
* bare name must NOT be handed to existsSync() directly, since existsSync
|
||||
* resolves it against process.cwd() rather than PATH (Bug I3/m1).
|
||||
*/
|
||||
export function resolveExecutable(cmd: string): string | null {
|
||||
if (cmd.includes("/") || cmd.includes("\\")) {
|
||||
return existsSync(cmd) ? cmd : null;
|
||||
}
|
||||
const dirs = (process.env.PATH || "").split(delimiter).filter(Boolean);
|
||||
const exts =
|
||||
process.platform === "win32"
|
||||
? (process.env.PATHEXT || ".EXE;.CMD;.BAT;.COM").split(";").filter(Boolean)
|
||||
: [""];
|
||||
for (const dir of dirs) {
|
||||
for (const ext of exts) {
|
||||
const hasExt = ext && cmd.toLowerCase().endsWith(ext.toLowerCase());
|
||||
const full = join(dir, hasExt ? cmd : cmd + ext);
|
||||
if (existsSync(full)) return full;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* True only on Linux. go-librespot ships Linux-only release binaries and the
|
||||
* sidecar relies on a POSIX FIFO (mkfifo), so the Spotify audio backend is
|
||||
@@ -44,6 +76,16 @@ export function findGoLibrespot(): string {
|
||||
return pickGoLibrespotPath([binPath, "go-librespot"], existsSync);
|
||||
}
|
||||
|
||||
/**
|
||||
* SYNCHRONOUS presence gate for go-librespot: supported platform (linux) AND
|
||||
* the resolved binary (project bin/ OR a PATH install) actually exists. This is
|
||||
* what chooseBackend()/isAvailable() must use — NOT existsSync(findGoLibrespot())
|
||||
* which cannot see a bare PATH name (Bug I3/m1).
|
||||
*/
|
||||
export function isGoLibrespotPresent(): boolean {
|
||||
return isGoLibrespotSupported() && resolveExecutable(findGoLibrespot()) !== null;
|
||||
}
|
||||
|
||||
// Injectable `--version` probe. Defaults to the real execFile call; tests
|
||||
// override it so checkGoLibrespotAvailable() needs no real binary. Keeps the
|
||||
// public checkGoLibrespotAvailable() signature param-free per the contract.
|
||||
@@ -143,6 +185,16 @@ export function findLibrespot(): string {
|
||||
return pickLibrespotPath([binExe, binBare, exe], existsSync);
|
||||
}
|
||||
|
||||
/**
|
||||
* SYNCHRONOUS presence gate for Rust librespot: supported everywhere AND the
|
||||
* resolved binary (project bin/ OR a scoop/choco/cargo PATH install) actually
|
||||
* exists. This is what chooseBackend()/isAvailable() must use — NOT
|
||||
* existsSync(findLibrespot()) which cannot see a bare PATH name (Bug I3/m1).
|
||||
*/
|
||||
export function isLibrespotPresent(): boolean {
|
||||
return isRustLibrespotSupported() && resolveExecutable(findLibrespot()) !== null;
|
||||
}
|
||||
|
||||
// Injectable `--version` probe. Defaults to the real execFile call; tests
|
||||
// override it so checkLibrespotAvailable() needs no real binary. Keeps the
|
||||
// public checkLibrespotAvailable() signature param-free per the contract.
|
||||
|
||||
@@ -22,7 +22,12 @@ const bin = vi.hoisted(() => ({
|
||||
rustSupported: true,
|
||||
rustPath: "",
|
||||
}));
|
||||
vi.mock("./binary.js", () => ({
|
||||
vi.mock("./binary.js", async () => {
|
||||
// existsSync mirrors the real resolveExecutable(find*()) result for the
|
||||
// bin/-style temp paths this suite uses (existingBin exists, missingBin does
|
||||
// not), keeping the chooseBackend/isAvailable matrix behavior-identical.
|
||||
const { existsSync } = await import("node:fs");
|
||||
return {
|
||||
isGoLibrespotSupported: () => bin.supported,
|
||||
findGoLibrespot: () => bin.path,
|
||||
resetGoLibrespotBinaryCache: () => {},
|
||||
@@ -31,7 +36,12 @@ vi.mock("./binary.js", () => ({
|
||||
findLibrespot: () => bin.rustPath,
|
||||
resetLibrespotBinaryCache: () => {},
|
||||
checkLibrespotAvailable: async () => bin.rustSupported && !!bin.rustPath,
|
||||
}));
|
||||
// PATH-aware presence gates (Bug I3): supported gate AND the resolved
|
||||
// binary actually exists on disk.
|
||||
isGoLibrespotPresent: () => bin.supported && existsSync(bin.path),
|
||||
isLibrespotPresent: () => bin.rustSupported && existsSync(bin.rustPath),
|
||||
};
|
||||
});
|
||||
|
||||
// Capture options the DEFAULT factory hands to the real GoLibrespotBackend so
|
||||
// we can assert per-bot ports (Fix 3) are threaded through. Every other test
|
||||
|
||||
@@ -15,12 +15,7 @@ import type {
|
||||
SpotifyTrackEndedEvent,
|
||||
SpotifyNowPlaying,
|
||||
} from "./backend.js";
|
||||
import {
|
||||
isGoLibrespotSupported,
|
||||
findGoLibrespot,
|
||||
isRustLibrespotSupported,
|
||||
findLibrespot,
|
||||
} from "./binary.js";
|
||||
import { isGoLibrespotPresent, isLibrespotPresent } from "./binary.js";
|
||||
import { GoLibrespotBackend } from "./go-librespot.js";
|
||||
import { RustLibrespotBackend } from "./rust-librespot.js";
|
||||
import {
|
||||
@@ -151,11 +146,13 @@ export class SpotifyController extends EventEmitter {
|
||||
}
|
||||
|
||||
private goPresent(): boolean {
|
||||
return isGoLibrespotSupported() && existsSync(findGoLibrespot());
|
||||
// PATH-aware, sync presence gate (Bug I3): a bare PATH-installed binary is
|
||||
// resolved against $PATH, not existsSync()'d against process.cwd().
|
||||
return isGoLibrespotPresent();
|
||||
}
|
||||
|
||||
private rustPresent(): boolean {
|
||||
return isRustLibrespotSupported() && existsSync(findLibrespot());
|
||||
return isLibrespotPresent();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+6
-9
@@ -23,12 +23,9 @@ import { createSpotifyRouter } from "./api/spotify.js";
|
||||
import type { SpotifyOAuth } from "../music/spotify/spotify-oauth.js";
|
||||
import { resolveSpotifyBackendKind } from "../music/spotify/backend-select.js";
|
||||
import {
|
||||
isGoLibrespotSupported,
|
||||
findGoLibrespot,
|
||||
isRustLibrespotSupported,
|
||||
findLibrespot,
|
||||
isGoLibrespotPresent,
|
||||
isLibrespotPresent,
|
||||
} from "../music/spotify/binary.js";
|
||||
import { existsSync } from "node:fs";
|
||||
import { setupWebSocket } from "./websocket.js";
|
||||
import { createUserStore } from "../data/users.js";
|
||||
import { createSessionStore } from "../data/sessions.js";
|
||||
@@ -156,10 +153,10 @@ export function createWebServer(options: WebServerOptions): WebServer {
|
||||
oauth: options.spotifyOAuth,
|
||||
logger,
|
||||
getBackendInfo: () => {
|
||||
const goPresent =
|
||||
isGoLibrespotSupported() && existsSync(findGoLibrespot());
|
||||
const rustPresent =
|
||||
isRustLibrespotSupported() && existsSync(findLibrespot());
|
||||
// PATH-aware presence (Bug m1): a PATH-installed binary (bare name)
|
||||
// is resolved against $PATH, not existsSync()'d against cwd.
|
||||
const goPresent = isGoLibrespotPresent();
|
||||
const rustPresent = isLibrespotPresent();
|
||||
const resolved = resolveSpotifyBackendKind(
|
||||
options.config.spotify.backend,
|
||||
goPresent,
|
||||
|
||||
Reference in new issue
Block a user