fix(linux): add user bin directories to systemd service PATH for skill installation (#1512)
* fix(linux): add user bin directories to systemd service PATH Fixes #1503 On Linux, the systemd service PATH was hardcoded to only include system directories (/usr/local/bin, /usr/bin, /bin), causing binaries installed via npm global with custom prefix or node version managers to not be found. This adds common Linux user bin directories to the PATH: - ~/.local/bin (XDG standard, pip, etc.) - ~/.npm-global/bin (npm custom prefix) - ~/bin (user's personal bin) - Node version manager paths (nvm, fnm, volta, asdf) - ~/.local/share/pnpm (pnpm global) - ~/.bun/bin (Bun) User directories are added before system directories so user-installed binaries take precedence. 🤖 AI-assisted (Claude Opus 4.5 via Clawdbot) 📋 Testing: Existing unit tests pass (7/7) * test: add comprehensive tests for Linux user bin directory resolution - Add dedicated tests for resolveLinuxUserBinDirs() function - Test path ordering (extraDirs > user dirs > system dirs) - Test buildMinimalServicePath() with HOME set/unset - Test platform-specific behavior (Linux vs macOS vs Windows) Test count: 7 → 20 (+13 tests) * test: add comprehensive tests for Linux user bin directory handling - Test Linux user directories included when HOME is set - Test Linux user directories excluded when HOME is missing - Test path ordering (extraDirs > user dirs > system dirs) - Test platform-specific behavior (Linux vs macOS vs Windows) - Test buildMinimalServicePath() with HOME in env Covers getMinimalServicePathParts() and buildMinimalServicePath() for all Linux user bin directory edge cases. Test count: 7 → 16 (+9 tests)
This commit is contained in:
Robby
GitHub
@@ -4,8 +4,98 @@ import {
|
||||
buildMinimalServicePath,
|
||||
buildNodeServiceEnvironment,
|
||||
buildServiceEnvironment,
|
||||
getMinimalServicePathParts,
|
||||
} from "./service-env.js";
|
||||
|
||||
describe("getMinimalServicePathParts - Linux user directories", () => {
|
||||
it("includes user bin directories when HOME is set on Linux", () => {
|
||||
const result = getMinimalServicePathParts({
|
||||
platform: "linux",
|
||||
home: "/home/testuser",
|
||||
});
|
||||
|
||||
// Should include all common user bin directories
|
||||
expect(result).toContain("/home/testuser/.local/bin");
|
||||
expect(result).toContain("/home/testuser/.npm-global/bin");
|
||||
expect(result).toContain("/home/testuser/bin");
|
||||
expect(result).toContain("/home/testuser/.nvm/current/bin");
|
||||
expect(result).toContain("/home/testuser/.fnm/current/bin");
|
||||
expect(result).toContain("/home/testuser/.volta/bin");
|
||||
expect(result).toContain("/home/testuser/.asdf/shims");
|
||||
expect(result).toContain("/home/testuser/.local/share/pnpm");
|
||||
expect(result).toContain("/home/testuser/.bun/bin");
|
||||
});
|
||||
|
||||
it("excludes user bin directories when HOME is undefined on Linux", () => {
|
||||
const result = getMinimalServicePathParts({
|
||||
platform: "linux",
|
||||
home: undefined,
|
||||
});
|
||||
|
||||
// Should only include system directories
|
||||
expect(result).toEqual(["/usr/local/bin", "/usr/bin", "/bin"]);
|
||||
|
||||
// Should not include any user-specific paths
|
||||
expect(result.some((p) => p.includes(".local"))).toBe(false);
|
||||
expect(result.some((p) => p.includes(".npm-global"))).toBe(false);
|
||||
expect(result.some((p) => p.includes(".nvm"))).toBe(false);
|
||||
});
|
||||
|
||||
it("places user directories before system directories on Linux", () => {
|
||||
const result = getMinimalServicePathParts({
|
||||
platform: "linux",
|
||||
home: "/home/testuser",
|
||||
});
|
||||
|
||||
const userDirIndex = result.indexOf("/home/testuser/.local/bin");
|
||||
const systemDirIndex = result.indexOf("/usr/bin");
|
||||
|
||||
expect(userDirIndex).toBeGreaterThan(-1);
|
||||
expect(systemDirIndex).toBeGreaterThan(-1);
|
||||
expect(userDirIndex).toBeLessThan(systemDirIndex);
|
||||
});
|
||||
|
||||
it("places extraDirs before user directories on Linux", () => {
|
||||
const result = getMinimalServicePathParts({
|
||||
platform: "linux",
|
||||
home: "/home/testuser",
|
||||
extraDirs: ["/custom/bin"],
|
||||
});
|
||||
|
||||
const extraDirIndex = result.indexOf("/custom/bin");
|
||||
const userDirIndex = result.indexOf("/home/testuser/.local/bin");
|
||||
|
||||
expect(extraDirIndex).toBeGreaterThan(-1);
|
||||
expect(userDirIndex).toBeGreaterThan(-1);
|
||||
expect(extraDirIndex).toBeLessThan(userDirIndex);
|
||||
});
|
||||
|
||||
it("does not include Linux user directories on macOS", () => {
|
||||
const result = getMinimalServicePathParts({
|
||||
platform: "darwin",
|
||||
home: "/Users/testuser",
|
||||
});
|
||||
|
||||
// Should not include Linux-specific user dirs even with HOME set
|
||||
expect(result.some((p) => p.includes(".npm-global"))).toBe(false);
|
||||
expect(result.some((p) => p.includes(".nvm"))).toBe(false);
|
||||
|
||||
// Should only include macOS system directories
|
||||
expect(result).toContain("/opt/homebrew/bin");
|
||||
expect(result).toContain("/usr/local/bin");
|
||||
});
|
||||
|
||||
it("does not include Linux user directories on Windows", () => {
|
||||
const result = getMinimalServicePathParts({
|
||||
platform: "win32",
|
||||
home: "C:\\Users\\testuser",
|
||||
});
|
||||
|
||||
// Windows returns empty array (uses existing PATH)
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildMinimalServicePath", () => {
|
||||
it("includes Homebrew + system dirs on macOS", () => {
|
||||
const result = buildMinimalServicePath({
|
||||
@@ -26,6 +116,51 @@ describe("buildMinimalServicePath", () => {
|
||||
expect(result).toBe("C:\\\\Windows\\\\System32");
|
||||
});
|
||||
|
||||
it("includes Linux user directories when HOME is set in env", () => {
|
||||
const result = buildMinimalServicePath({
|
||||
platform: "linux",
|
||||
env: { HOME: "/home/alice" },
|
||||
});
|
||||
const parts = result.split(path.delimiter);
|
||||
|
||||
// Verify user directories are included
|
||||
expect(parts).toContain("/home/alice/.local/bin");
|
||||
expect(parts).toContain("/home/alice/.npm-global/bin");
|
||||
expect(parts).toContain("/home/alice/.nvm/current/bin");
|
||||
|
||||
// Verify system directories are also included
|
||||
expect(parts).toContain("/usr/local/bin");
|
||||
expect(parts).toContain("/usr/bin");
|
||||
expect(parts).toContain("/bin");
|
||||
});
|
||||
|
||||
it("excludes Linux user directories when HOME is not in env", () => {
|
||||
const result = buildMinimalServicePath({
|
||||
platform: "linux",
|
||||
env: {},
|
||||
});
|
||||
const parts = result.split(path.delimiter);
|
||||
|
||||
// Should only have system directories
|
||||
expect(parts).toEqual(["/usr/local/bin", "/usr/bin", "/bin"]);
|
||||
|
||||
// No user-specific paths
|
||||
expect(parts.some((p) => p.includes("home"))).toBe(false);
|
||||
});
|
||||
|
||||
it("ensures user directories come before system directories on Linux", () => {
|
||||
const result = buildMinimalServicePath({
|
||||
platform: "linux",
|
||||
env: { HOME: "/home/bob" },
|
||||
});
|
||||
const parts = result.split(path.delimiter);
|
||||
|
||||
const firstUserDirIdx = parts.indexOf("/home/bob/.local/bin");
|
||||
const firstSystemDirIdx = parts.indexOf("/usr/local/bin");
|
||||
|
||||
expect(firstUserDirIdx).toBeLessThan(firstSystemDirIdx);
|
||||
});
|
||||
|
||||
it("includes extra directories when provided", () => {
|
||||
const result = buildMinimalServicePath({
|
||||
platform: "linux",
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
export type MinimalServicePathOptions = {
|
||||
platform?: NodeJS.Platform;
|
||||
extraDirs?: string[];
|
||||
home?: string;
|
||||
};
|
||||
|
||||
type BuildServicePathOptions = MinimalServicePathOptions & {
|
||||
@@ -33,6 +34,31 @@ function resolveSystemPathDirs(platform: NodeJS.Platform): string[] {
|
||||
return [];
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve common user bin directories for Linux.
|
||||
* These are paths where npm global installs and node version managers typically place binaries.
|
||||
*/
|
||||
export function resolveLinuxUserBinDirs(home: string | undefined): string[] {
|
||||
if (!home) return [];
|
||||
|
||||
const dirs: string[] = [];
|
||||
|
||||
// Common user bin directories
|
||||
dirs.push(`${home}/.local/bin`); // XDG standard, pip, etc.
|
||||
dirs.push(`${home}/.npm-global/bin`); // npm custom prefix (recommended for non-root)
|
||||
dirs.push(`${home}/bin`); // User's personal bin
|
||||
|
||||
// Node version managers
|
||||
dirs.push(`${home}/.nvm/current/bin`); // nvm with current symlink
|
||||
dirs.push(`${home}/.fnm/current/bin`); // fnm
|
||||
dirs.push(`${home}/.volta/bin`); // Volta
|
||||
dirs.push(`${home}/.asdf/shims`); // asdf
|
||||
dirs.push(`${home}/.local/share/pnpm`); // pnpm global bin
|
||||
dirs.push(`${home}/.bun/bin`); // Bun
|
||||
|
||||
return dirs;
|
||||
}
|
||||
|
||||
export function getMinimalServicePathParts(options: MinimalServicePathOptions = {}): string[] {
|
||||
const platform = options.platform ?? process.platform;
|
||||
if (platform === "win32") return [];
|
||||
@@ -41,12 +67,17 @@ export function getMinimalServicePathParts(options: MinimalServicePathOptions =
|
||||
const extraDirs = options.extraDirs ?? [];
|
||||
const systemDirs = resolveSystemPathDirs(platform);
|
||||
|
||||
// Add Linux user bin directories (npm global, nvm, fnm, volta, etc.)
|
||||
const linuxUserDirs = platform === "linux" ? resolveLinuxUserBinDirs(options.home) : [];
|
||||
|
||||
const add = (dir: string) => {
|
||||
if (!dir) return;
|
||||
if (!parts.includes(dir)) parts.push(dir);
|
||||
};
|
||||
|
||||
for (const dir of extraDirs) add(dir);
|
||||
// User dirs first so user-installed binaries take precedence
|
||||
for (const dir of linuxUserDirs) add(dir);
|
||||
for (const dir of systemDirs) add(dir);
|
||||
|
||||
return parts;
|
||||
@@ -59,7 +90,10 @@ export function buildMinimalServicePath(options: BuildServicePathOptions = {}):
|
||||
return env.PATH ?? "";
|
||||
}
|
||||
|
||||
return getMinimalServicePathParts(options).join(path.delimiter);
|
||||
return getMinimalServicePathParts({
|
||||
...options,
|
||||
home: options.home ?? env.HOME,
|
||||
}).join(path.delimiter);
|
||||
}
|
||||
|
||||
export function buildServiceEnvironment(params: {
|
||||
|
||||
Reference in New Issue
Block a user